| Message ID | 1488111691-23620-1-git-send-email-Ram.Amrani@cavium.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Sun, Feb 26, 2017 at 02:21:31PM +0200, Ram Amrani wrote: > Avoid setting the value '1' in the MR. If this happens in a write latency > test then the server will send two consecutive packets, regardless of the > client's state. This can cause the application to hang - If the client > reaches the busy-wait loop after the second write then it'll keep waiting > for the value of the first write forever. > > Signed-off-by: Ram Amrani <Ram.Amrani@cavium.com> > --- > src/perftest_resources.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/src/perftest_resources.c b/src/perftest_resources.c > index afae5f2..dc768c4 100755 > --- a/src/perftest_resources.c > +++ b/src/perftest_resources.c > @@ -1252,7 +1252,11 @@ int create_single_mr(struct pingpong_context *ctx, struct perftest_parameters *u > /* Initialize buffer with random numbers */ > srand(time(NULL)); > for (i = 0; i < ctx->buff_size; i++) { > - ((char*)ctx->buf[qp_index])[i] = (char)rand(); > + /* prevent the value 1 from being written into the buffer so in, > + * e.g., write latency test, the server won't send two packets > + * consecutively without receiving a packet from the client first. > + */ > + ((char*)ctx->buf[qp_index])[i] = 2 + ((char)rand() % 255); > } > > return 0; Zohar? > -- > 1.8.3.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Will push it by the EOW -----Original Message----- From: Leon Romanovsky [mailto:leon@kernel.org] Sent: Tuesday, February 28, 2017 10:01 PM To: Zohar Ben Aharon <zoharb@mellanox.com>; Ram Amrani <Ram.Amrani@cavium.com> Cc: Gil Rockah <gilr@mellanox.com>; linux-rdma@vger.kernel.org; Ariel.Elior@cavium.com Subject: Re: [PATCH perftest] Avoid configuring the MRs with 1's On Sun, Feb 26, 2017 at 02:21:31PM +0200, Ram Amrani wrote: > Avoid setting the value '1' in the MR. If this happens in a write > latency test then the server will send two consecutive packets, > regardless of the client's state. This can cause the application to > hang - If the client reaches the busy-wait loop after the second write > then it'll keep waiting for the value of the first write forever. > > Signed-off-by: Ram Amrani <Ram.Amrani@cavium.com> > --- > src/perftest_resources.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/src/perftest_resources.c b/src/perftest_resources.c index > afae5f2..dc768c4 100755 > --- a/src/perftest_resources.c > +++ b/src/perftest_resources.c > @@ -1252,7 +1252,11 @@ int create_single_mr(struct pingpong_context *ctx, struct perftest_parameters *u > /* Initialize buffer with random numbers */ > srand(time(NULL)); > for (i = 0; i < ctx->buff_size; i++) { > - ((char*)ctx->buf[qp_index])[i] = (char)rand(); > + /* prevent the value 1 from being written into the buffer so in, > + * e.g., write latency test, the server won't send two packets > + * consecutively without receiving a packet from the client first. > + */ > + ((char*)ctx->buf[qp_index])[i] = 2 + ((char)rand() % 255); > } > > return 0; Zohar? > -- > 1.8.3.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" > in the body of a message to majordomo@vger.kernel.org More majordomo > info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>Hi Ram , >I applied your patch and start to review, >did you aware that : > > ((char*)ctx->buf[qp_index])[i] = 2 + ((char)rand() % 255); > >Will put values from 2-257 ? > >Thanks, > >Zohar Ben Aharon X % 255 yields a number between 0 and 254. Adding 2 to that we get a number between 2 and 256. Since this is a char the actual number stored is in the range 2 and 255 or 0 (for 256). HOWEVER, there's still a bug in this line. (char) should be converted to (unsigned char) Explanation: Rand returns a number of 0..<some positive value> That can turn into 0xff when casting to char. This 0xff *remains* 0xff after the modulo because it is a negative number! And 0xff+2==1 and the bug occurs again. I should have verified... Will send you a fix shortly. Thank, Ram -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> I should have verified... > BTW, I did verify, just not with 'c' but in another environment that did not notice this subtlety. Ram -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/src/perftest_resources.c b/src/perftest_resources.c index afae5f2..dc768c4 100755 --- a/src/perftest_resources.c +++ b/src/perftest_resources.c @@ -1252,7 +1252,11 @@ int create_single_mr(struct pingpong_context *ctx, struct perftest_parameters *u /* Initialize buffer with random numbers */ srand(time(NULL)); for (i = 0; i < ctx->buff_size; i++) { - ((char*)ctx->buf[qp_index])[i] = (char)rand(); + /* prevent the value 1 from being written into the buffer so in, + * e.g., write latency test, the server won't send two packets + * consecutively without receiving a packet from the client first. + */ + ((char*)ctx->buf[qp_index])[i] = 2 + ((char)rand() % 255); } return 0;
Avoid setting the value '1' in the MR. If this happens in a write latency test then the server will send two consecutive packets, regardless of the client's state. This can cause the application to hang - If the client reaches the busy-wait loop after the second write then it'll keep waiting for the value of the first write forever. Signed-off-by: Ram Amrani <Ram.Amrani@cavium.com> --- src/perftest_resources.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)