diff mbox series

[1/2] migration/rdma: Increase the backlog from 5 to 128

Message ID 20220201151136.52157-1-jinpu.wang@ionos.com
State New
Headers show
Series [1/2] migration/rdma: Increase the backlog from 5 to 128 | expand

Commit Message

Jinpu Wang Feb. 1, 2022, 3:11 p.m. UTC
So it can handle more incoming requests.

Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
---
 migration/rdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Pankaj Gupta Feb. 1, 2022, 6:19 p.m. UTC | #1
> So it can handle more incoming requests.
>
> Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
> ---
>  migration/rdma.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index c7c7a384875b..2e223170d06d 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -4238,7 +4238,7 @@ void rdma_start_incoming_migration(const char *host_port, Error **errp)
>
>      trace_rdma_start_incoming_migration_after_dest_init();
>
> -    ret = rdma_listen(rdma->listen_id, 5);
> +    ret = rdma_listen(rdma->listen_id, 128);

128 backlog seems too much to me. Any reason for choosing this number.
Any rationale to choose this number?

Thanks,
Pankaj

>
>      if (ret) {
>          ERROR(errp, "listening on socket!");
> --
> 2.25.1
>
Jinpu Wang Feb. 1, 2022, 8:32 p.m. UTC | #2
On Tue, Feb 1, 2022 at 7:19 PM Pankaj Gupta <pankaj.gupta@ionos.com> wrote:
>
> > So it can handle more incoming requests.
> >
> > Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
> > ---
> >  migration/rdma.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/migration/rdma.c b/migration/rdma.c
> > index c7c7a384875b..2e223170d06d 100644
> > --- a/migration/rdma.c
> > +++ b/migration/rdma.c
> > @@ -4238,7 +4238,7 @@ void rdma_start_incoming_migration(const char *host_port, Error **errp)
> >
> >      trace_rdma_start_incoming_migration_after_dest_init();
> >
> > -    ret = rdma_listen(rdma->listen_id, 5);
> > +    ret = rdma_listen(rdma->listen_id, 128);
>
> 128 backlog seems too much to me. Any reason for choosing this number.
> Any rationale to choose this number?
>
128 is the default value of SOMAXCONN, I can use that if it is preferred.

> Thanks,
> Pankaj

Thanks!
>
> >
> >      if (ret) {
> >          ERROR(errp, "listening on socket!");
> > --
> > 2.25.1
> >
Pankaj Gupta Feb. 2, 2022, 6:27 a.m. UTC | #3
> > > So it can handle more incoming requests.
> > >
> > > Signed-off-by: Jack Wang <jinpu.wang@ionos.com>
> > > ---
> > >  migration/rdma.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/migration/rdma.c b/migration/rdma.c
> > > index c7c7a384875b..2e223170d06d 100644
> > > --- a/migration/rdma.c
> > > +++ b/migration/rdma.c
> > > @@ -4238,7 +4238,7 @@ void rdma_start_incoming_migration(const char *host_port, Error **errp)
> > >
> > >      trace_rdma_start_incoming_migration_after_dest_init();
> > >
> > > -    ret = rdma_listen(rdma->listen_id, 5);
> > > +    ret = rdma_listen(rdma->listen_id, 128);
> >
> > 128 backlog seems too much to me. Any reason for choosing this number.
> > Any rationale to choose this number?
> >
> 128 is the default value of SOMAXCONN, I can use that if it is preferred.

AFAICS backlog is only applicable with RDMA iWARP CM mode. Maybe we
can increase it to 128.
Maybe you can also share any testing data for multiple concurrent live
migrations using RDMA, please.

Thanks,
Pankaj

Thanks,
Pankaj
Pankaj Gupta Feb. 2, 2022, 6:37 a.m. UTC | #4
> > > >  migration/rdma.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/migration/rdma.c b/migration/rdma.c
> > > > index c7c7a384875b..2e223170d06d 100644
> > > > --- a/migration/rdma.c
> > > > +++ b/migration/rdma.c
> > > > @@ -4238,7 +4238,7 @@ void rdma_start_incoming_migration(const char *host_port, Error **errp)
> > > >
> > > >      trace_rdma_start_incoming_migration_after_dest_init();
> > > >
> > > > -    ret = rdma_listen(rdma->listen_id, 5);
> > > > +    ret = rdma_listen(rdma->listen_id, 128);
> > >
> > > 128 backlog seems too much to me. Any reason for choosing this number.
> > > Any rationale to choose this number?
> > >
> > 128 is the default value of SOMAXCONN, I can use that if it is preferred.
>
> AFAICS backlog is only applicable with RDMA iWARP CM mode. Maybe we
> can increase it to 128.these many

Or maybe we first increase it to 20 or 32? or so to avoid memory
overhead if we are not
using these many connections at the same time.

> Maybe you can also share any testing data for multiple concurrent live
> migrations using RDMA, please.
>
> Thanks,
> Pankaj
>
> Thanks,
> Pankaj
Dr. David Alan Gilbert Feb. 2, 2022, 9:20 a.m. UTC | #5
* Pankaj Gupta (pankaj.gupta@ionos.com) wrote:
> > > > >  migration/rdma.c | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/migration/rdma.c b/migration/rdma.c
> > > > > index c7c7a384875b..2e223170d06d 100644
> > > > > --- a/migration/rdma.c
> > > > > +++ b/migration/rdma.c
> > > > > @@ -4238,7 +4238,7 @@ void rdma_start_incoming_migration(const char *host_port, Error **errp)
> > > > >
> > > > >      trace_rdma_start_incoming_migration_after_dest_init();
> > > > >
> > > > > -    ret = rdma_listen(rdma->listen_id, 5);
> > > > > +    ret = rdma_listen(rdma->listen_id, 128);
> > > >
> > > > 128 backlog seems too much to me. Any reason for choosing this number.
> > > > Any rationale to choose this number?
> > > >
> > > 128 is the default value of SOMAXCONN, I can use that if it is preferred.
> >
> > AFAICS backlog is only applicable with RDMA iWARP CM mode. Maybe we
> > can increase it to 128.these many
> 
> Or maybe we first increase it to 20 or 32? or so to avoid memory
> overhead if we are not
> using these many connections at the same time.

Can you explain why you're requiring more than 1?  Is this with multifd
patches?

Dave

> > Maybe you can also share any testing data for multiple concurrent live
> > migrations using RDMA, please.
> >
> > Thanks,
> > Pankaj
> >
> > Thanks,
> > Pankaj
>
Jinpu Wang Feb. 2, 2022, 9:55 a.m. UTC | #6
On Wed, Feb 2, 2022 at 10:20 AM Dr. David Alan Gilbert
<dgilbert@redhat.com> wrote:
>
> * Pankaj Gupta (pankaj.gupta@ionos.com) wrote:
> > > > > >  migration/rdma.c | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/migration/rdma.c b/migration/rdma.c
> > > > > > index c7c7a384875b..2e223170d06d 100644
> > > > > > --- a/migration/rdma.c
> > > > > > +++ b/migration/rdma.c
> > > > > > @@ -4238,7 +4238,7 @@ void rdma_start_incoming_migration(const char *host_port, Error **errp)
> > > > > >
> > > > > >      trace_rdma_start_incoming_migration_after_dest_init();
> > > > > >
> > > > > > -    ret = rdma_listen(rdma->listen_id, 5);
> > > > > > +    ret = rdma_listen(rdma->listen_id, 128);
> > > > >
> > > > > 128 backlog seems too much to me. Any reason for choosing this number.
> > > > > Any rationale to choose this number?
> > > > >
> > > > 128 is the default value of SOMAXCONN, I can use that if it is preferred.
> > >
> > > AFAICS backlog is only applicable with RDMA iWARP CM mode. Maybe we
> > > can increase it to 128.these many
> >
> > Or maybe we first increase it to 20 or 32? or so to avoid memory
> > overhead if we are not
> > using these many connections at the same time.
>
> Can you explain why you're requiring more than 1?  Is this with multifd
> patches?

no, I'm not using multifs patches, just code reading, I feel 5 is too
small for the backlog setting.

As Pankaj rightly mentioned, in RDMA, only iWARP CM will take some
effect, it does nothing for InfiniBand
and RoCE.

Please ignore this patch, we can revisit this when we introduce
multifid with RDMA.

Thanks!
Jinpu Wang
>
> Dave
>
> > > Maybe you can also share any testing data for multiple concurrent live
> > > migrations using RDMA, please.
> > >
> > > Thanks,
> > > Pankaj
> > >
> > > Thanks,
> > > Pankaj
> >
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
diff mbox series

Patch

diff --git a/migration/rdma.c b/migration/rdma.c
index c7c7a384875b..2e223170d06d 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -4238,7 +4238,7 @@  void rdma_start_incoming_migration(const char *host_port, Error **errp)
 
     trace_rdma_start_incoming_migration_after_dest_init();
 
-    ret = rdma_listen(rdma->listen_id, 5);
+    ret = rdma_listen(rdma->listen_id, 128);
 
     if (ret) {
         ERROR(errp, "listening on socket!");