On Tue, Dec 19, 2017 at 09:14:05AM +0000, Amrani, Ram wrote:
> > Hi Jason, Yuval,
> > I’ve started a review on this a couple of days ago. It’s the first time I’ve done this.
> > Did you get e-mails about it? How come it gets merged without response?
> > Please comment on my remarks.
> >
> > Thanks,
> > Ram
> >
>
> I still didn't see any e-mail.
> Probably the review doesn't work after the patch has been committed.
Have no idea why?
So, for rdma-core, i understood that PR on github is much more powerful
than linux-rdma mailing list but i don't mind running on both next time.
> My comments:
>
> + printf(" -c, --chk validate received buffer\n");
> Why not have the default set to 'validate'?
Because we don't want to break existing user-created scripts so default
should be - "the same as it was!".
> The only reason not to use validate is for performance and I don't think this application checks that...
Yeah, the validation is out of the scope of performance accounting.
> Still, if this is important then the user can choose to disable validation.
>
> + for (int i = 0; i < size; i += page_size)
> I expect (and IMHO users expect too) that data validation is on the entire buffer. What is happening here is that only a single byte per page is checked.
Because i believe (and sure i maybe mistaken here) is that errors may happen
in a page boundaries. If a full buffer validation is needed then we can add
a check validation level such as "-c 0" for page boundaries and "-c 1" for
full check.
>
> Thanks,
> Ram
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
