Re: [PATCH 11/11] IB/srp: Prevent mapping failures

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1507 bytes --]

On Mon, Apr 25, 2016 at 09:16:18AM -0700, Bart Van Assche wrote:
> On 04/25/2016 08:53 AM, Leon Romanovsky wrote:
> >On Mon, Apr 25, 2016 at 12:46:59PM +0300, Sagi Grimberg wrote:
> >>
> >>>>>>+		max_max_sectors = (srp_dev->max_pages_per_mr - 1) <<
> >>>>>>+				  (ilog2(srp_dev->mr_page_size) - 9);
> >>>>>
> >>>>> From my understanding, srp_dev->max_pages_per_mr can be equal to 1, in
> >>>>>case device will advertise it.
> >>>>>
> >>>>>Is this understanding correct?
> >>>>>Will the code work in such case?
> >>>>
> >>>>Hello Leon,
> >>>>
> >>>>I'm not sure a HCA driver exists for which max_pages_per_mr == 1.
> >>>>Additionally, I'm not sure such a driver would be useful because such a
> >>>>driver would limit the maximum data transfer size for the iSER protocol to 4
> >>>>KB. That is too low to reach good performance.
> >>>
> >>>Bart and Laurence,
> >>>You are raising valuable arguments about sanity of setting srp_dev->max_pages_per_mr
> >>>to be equal 1. I totally agree with you that it is insane, however my
> >>>original question was "does the code support such value?"
> >>
> >>I don't see why we should worry about imaginary devices.
> >
> >It is called corner case and the code supports it.
> 
> I can add a WARN_ON_ONCE() statement for max_pages_per_mr == 1. That will
> make it easy to figure out where to start looking if in the future a HCA
> driver would be added that causes max_pages_per_mr == 1.
> 

Thanks Bart,
It is fair enough.

> Bart.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]