| Message ID | 20190122182520.59519-1-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 0b5cb3300ae59ed7e93b465dfa2384a6a4df8eb4 |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | RDMA/srp: Increase max_segment_size | expand |
On 2019-01-22 1:25 p.m., Bart Van Assche wrote: > The default behavior of the SCSI core is to set the block layer request > queue parameter max_segment_size to 64 KB. That means that elements of > scatterlists are limited to 64 KB. Since RDMA adapters support larger > sizes, increase max_segment_size for the SRP initiator. > > Notes: > - The SCSI max_segment_size parameter was introduced in kernel v5.0. See > also commit 50c2e9107f17 ("scsi: introduce a max_segment_size > host_template parameters"). > - Some other block drivers already set max_segment_size to UINT_MAX, > e.g. nbd and rbd. In my sg v4 driver rewrite one of options within the SG_SET_GET_EXTENDED ioctl is to allow the user to modify this value. It is a #define to 32 KB in the production sg v3 driver. Doug Gilbert > Cc: Christoph Hellwig <hch@lst.de> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > drivers/infiniband/ulp/srp/ib_srp.c | 1 + > include/rdma/ib_verbs.h | 13 +++++++++++++ > 2 files changed, 14 insertions(+) > > diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c > index 085dba075651..a1173e0992e6 100644 > --- a/drivers/infiniband/ulp/srp/ib_srp.c > +++ b/drivers/infiniband/ulp/srp/ib_srp.c > @@ -3819,6 +3819,7 @@ static ssize_t srp_create_target(struct device *dev, > target_host->max_id = 1; > target_host->max_lun = -1LL; > target_host->max_cmd_len = sizeof ((struct srp_cmd *) (void *) 0L)->cdb; > + target_host->max_segment_size = ib_dma_max_seg_size(ibdev); > > target = host_to_target(target_host); > > diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h > index 94b6e1dd4dab..71ea144ec823 100644 > --- a/include/rdma/ib_verbs.h > +++ b/include/rdma/ib_verbs.h > @@ -3715,6 +3715,19 @@ static inline unsigned int ib_sg_dma_len(struct ib_device *dev, > return sg_dma_len(sg); > } > > +/** > + * ib_dma_max_seg_size - Return the size limit of a single DMA transfer > + * @dev: The device to query > + * > + * The returned value represents a size in bytes. > + */ > +static inline unsigned int ib_dma_max_seg_size(struct ib_device *dev) > +{ > + struct device_dma_parameters *p = dev->dma_device->dma_parms; > + > + return p ? p->max_segment_size : UINT_MAX; > +} > + > /** > * ib_dma_sync_single_for_cpu - Prepare DMA region to be accessed by CPU > * @dev: The device for which the DMA address was created >
On Tue, 2019-01-22 at 10:25 -0800, Bart Van Assche wrote: > The default behavior of the SCSI core is to set the block layer request > queue parameter max_segment_size to 64 KB. That means that elements of > scatterlists are limited to 64 KB. Since RDMA adapters support larger > sizes, increase max_segment_size for the SRP initiator. > > Notes: > - The SCSI max_segment_size parameter was introduced in kernel v5.0. See > also commit 50c2e9107f17 ("scsi: introduce a max_segment_size > host_template parameters"). > - Some other block drivers already set max_segment_size to UINT_MAX, > e.g. nbd and rbd. Hi Jason and Doug, Can one of you have a look at this patch? Thanks, Bart.
On Wed, Jan 30, 2019 at 10:59:34AM -0800, Bart Van Assche wrote: > On Tue, 2019-01-22 at 10:25 -0800, Bart Van Assche wrote: > > The default behavior of the SCSI core is to set the block layer request > > queue parameter max_segment_size to 64 KB. That means that elements of > > scatterlists are limited to 64 KB. Since RDMA adapters support larger > > sizes, increase max_segment_size for the SRP initiator. > > > > Notes: > > - The SCSI max_segment_size parameter was introduced in kernel v5.0. See > > also commit 50c2e9107f17 ("scsi: introduce a max_segment_size > > host_template parameters"). > > - Some other block drivers already set max_segment_size to UINT_MAX, > > e.g. nbd and rbd. > > Hi Jason and Doug, > > Can one of you have a look at this patch? I was confused by the unanswered remark from Doug Gilbert - what that a request to change something? Jason
On Wed, 2019-01-30 at 12:21 -0700, Jason Gunthorpe wrote: > On Wed, Jan 30, 2019 at 10:59:34AM -0800, Bart Van Assche wrote: > > On Tue, 2019-01-22 at 10:25 -0800, Bart Van Assche wrote: > > > The default behavior of the SCSI core is to set the block layer request > > > queue parameter max_segment_size to 64 KB. That means that elements of > > > scatterlists are limited to 64 KB. Since RDMA adapters support larger > > > sizes, increase max_segment_size for the SRP initiator. > > > > > > Notes: > > > - The SCSI max_segment_size parameter was introduced in kernel v5.0. See > > > also commit 50c2e9107f17 ("scsi: introduce a max_segment_size > > > host_template parameters"). > > > - Some other block drivers already set max_segment_size to UINT_MAX, > > > e.g. nbd and rbd. > > > > Hi Jason and Doug, > > > > Can one of you have a look at this patch? > > I was confused by the unanswered remark from Doug Gilbert - what that > a request to change something? Hi Jason, My understanding is that Doug Gilbert's e-mail was not a request to change my patch but instead an announcement of something he plans to implement in the future. The functionality Doug mentioned seems useful to me but does not replace this patch. It is much easier to users to have the default set correctly instead of having to modify the default from the command line or from a udev rule. Thanks, Bart.
On Tue, 2019-01-22 at 15:47 -0500, Douglas Gilbert wrote: > On 2019-01-22 1:25 p.m., Bart Van Assche wrote: > > The default behavior of the SCSI core is to set the block layer request > > queue parameter max_segment_size to 64 KB. That means that elements of > > scatterlists are limited to 64 KB. Since RDMA adapters support larger > > sizes, increase max_segment_size for the SRP initiator. > > > > Notes: > > - The SCSI max_segment_size parameter was introduced in kernel v5.0. See > > also commit 50c2e9107f17 ("scsi: introduce a max_segment_size > > host_template parameters"). > > - Some other block drivers already set max_segment_size to UINT_MAX, > > e.g. nbd and rbd. > > In my sg v4 driver rewrite one of options within the SG_SET_GET_EXTENDED > ioctl is to allow the user to modify this value. It is a #define to > 32 KB in the production sg v3 driver. Hi Doug, This functionality seems useful to me. But it is not clear to me how it will be guaranteed that the value set by the user won't exceed the largest max_segment_size value supported by the driver? Thanks, Bart.
On Tue, Jan 22, 2019 at 10:25:20AM -0800, Bart Van Assche wrote: > The default behavior of the SCSI core is to set the block layer request > queue parameter max_segment_size to 64 KB. That means that elements of > scatterlists are limited to 64 KB. Since RDMA adapters support larger > sizes, increase max_segment_size for the SRP initiator. > > Notes: > - The SCSI max_segment_size parameter was introduced in kernel v5.0. See > also commit 50c2e9107f17 ("scsi: introduce a max_segment_size > host_template parameters"). > - Some other block drivers already set max_segment_size to UINT_MAX, > e.g. nbd and rbd. > > Cc: Christoph Hellwig <hch@lst.de> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > --- > drivers/infiniband/ulp/srp/ib_srp.c | 1 + > include/rdma/ib_verbs.h | 13 +++++++++++++ > 2 files changed, 14 insertions(+) Applied to for-next Thanks, Jason
On 2019-01-30 3:23 p.m., Bart Van Assche wrote: > On Tue, 2019-01-22 at 15:47 -0500, Douglas Gilbert wrote: >> On 2019-01-22 1:25 p.m., Bart Van Assche wrote: >>> The default behavior of the SCSI core is to set the block layer request >>> queue parameter max_segment_size to 64 KB. That means that elements of >>> scatterlists are limited to 64 KB. Since RDMA adapters support larger >>> sizes, increase max_segment_size for the SRP initiator. >>> >>> Notes: >>> - The SCSI max_segment_size parameter was introduced in kernel v5.0. See >>> also commit 50c2e9107f17 ("scsi: introduce a max_segment_size >>> host_template parameters"). >>> - Some other block drivers already set max_segment_size to UINT_MAX, >>> e.g. nbd and rbd. >> >> In my sg v4 driver rewrite one of options within the SG_SET_GET_EXTENDED >> ioctl is to allow the user to modify this value. It is a #define to >> 32 KB in the production sg v3 driver. > > Hi Doug, > > This functionality seems useful to me. But it is not clear to me how it > will be guaranteed that the value set by the user won't exceed the largest > max_segment_size value supported by the driver? The sg v4 driver exists and its code can be found at: http://sg.danny.cz/sg/sg_v40.html And the code there takes the min(<given_sgat_sz>, <following_function>) as the element size. That function is: static int max_sectors_bytes(struct request_queue *q) { unsigned int max_sectors = queue_max_sectors(q); max_sectors = min_t(unsigned int, max_sectors, INT_MAX >> 9); return max_sectors << 9; } Also <given_sgat_sz> is rounded up to PAGE_SIZE and rejected if it is not a power of 2. Does that look correct? Doug Gilbert
On Wed, 2019-01-30 at 17:50 -0500, Douglas Gilbert wrote: > On 2019-01-30 3:23 p.m., Bart Van Assche wrote: > > On Tue, 2019-01-22 at 15:47 -0500, Douglas Gilbert wrote: > > > On 2019-01-22 1:25 p.m., Bart Van Assche wrote: > > > > The default behavior of the SCSI core is to set the block layer request > > > > queue parameter max_segment_size to 64 KB. That means that elements of > > > > scatterlists are limited to 64 KB. Since RDMA adapters support larger > > > > sizes, increase max_segment_size for the SRP initiator. > > > > > > > > Notes: > > > > - The SCSI max_segment_size parameter was introduced in kernel v5.0. See > > > > also commit 50c2e9107f17 ("scsi: introduce a max_segment_size > > > > host_template parameters"). > > > > - Some other block drivers already set max_segment_size to UINT_MAX, > > > > e.g. nbd and rbd. > > > > > > In my sg v4 driver rewrite one of options within the SG_SET_GET_EXTENDED > > > ioctl is to allow the user to modify this value. It is a #define to > > > 32 KB in the production sg v3 driver. > > > > Hi Doug, > > > > This functionality seems useful to me. But it is not clear to me how it > > will be guaranteed that the value set by the user won't exceed the largest > > max_segment_size value supported by the driver? > > The sg v4 driver exists and its code can be found at: > http://sg.danny.cz/sg/sg_v40.html > > And the code there takes the min(<given_sgat_sz>, <following_function>) as > the element size. That function is: > > static int > max_sectors_bytes(struct request_queue *q) > { > unsigned int max_sectors = queue_max_sectors(q); > > max_sectors = min_t(unsigned int, max_sectors, INT_MAX >> 9); > return max_sectors << 9; > } > > Also <given_sgat_sz> is rounded up to PAGE_SIZE and rejected if it is not > a power of 2. Does that look correct? Hi Doug, Have you noticed that my patch affects max_segment_size and that your patch refers to the max_sectors limit? From the Linux kernel header file blkdev.h: static inline unsigned int queue_max_sectors(struct request_queue *q) { return q->limits.max_sectors; } [ ... ] static inline unsigned int queue_max_segment_size(struct request_queue *q) { return q->limits.max_segment_size; } Thanks, Bart.
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index 085dba075651..a1173e0992e6 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -3819,6 +3819,7 @@ static ssize_t srp_create_target(struct device *dev, target_host->max_id = 1; target_host->max_lun = -1LL; target_host->max_cmd_len = sizeof ((struct srp_cmd *) (void *) 0L)->cdb; + target_host->max_segment_size = ib_dma_max_seg_size(ibdev); target = host_to_target(target_host); diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index 94b6e1dd4dab..71ea144ec823 100644 --- a/include/rdma/ib_verbs.h +++ b/include/rdma/ib_verbs.h @@ -3715,6 +3715,19 @@ static inline unsigned int ib_sg_dma_len(struct ib_device *dev, return sg_dma_len(sg); } +/** + * ib_dma_max_seg_size - Return the size limit of a single DMA transfer + * @dev: The device to query + * + * The returned value represents a size in bytes. + */ +static inline unsigned int ib_dma_max_seg_size(struct ib_device *dev) +{ + struct device_dma_parameters *p = dev->dma_device->dma_parms; + + return p ? p->max_segment_size : UINT_MAX; +} + /** * ib_dma_sync_single_for_cpu - Prepare DMA region to be accessed by CPU * @dev: The device for which the DMA address was created
The default behavior of the SCSI core is to set the block layer request queue parameter max_segment_size to 64 KB. That means that elements of scatterlists are limited to 64 KB. Since RDMA adapters support larger sizes, increase max_segment_size for the SRP initiator. Notes: - The SCSI max_segment_size parameter was introduced in kernel v5.0. See also commit 50c2e9107f17 ("scsi: introduce a max_segment_size host_template parameters"). - Some other block drivers already set max_segment_size to UINT_MAX, e.g. nbd and rbd. Cc: Christoph Hellwig <hch@lst.de> Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- drivers/infiniband/ulp/srp/ib_srp.c | 1 + include/rdma/ib_verbs.h | 13 +++++++++++++ 2 files changed, 14 insertions(+)