| Message ID | 1515437475-25789-1-git-send-email-idryomov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/08/2018 12:51 PM, Ilya Dryomov wrote: > Commit d3834fefcfe5 ("rbd: bump queue_max_segments") bumped > max_segments (unsigned short) to max_hw_sectors (unsigned int). > max_hw_sectors is set to the number of 512-byte sectors in an object > and overflows unsigned short for 32M (largest possible) objects, making > the block layer resort to handing us single segment (i.e. single page > or even smaller) bios in that case. > > Cc: stable@vger.kernel.org > Fixes: d3834fefcfe5 ("rbd: bump queue_max_segments") > Signed-off-by: Ilya Dryomov <idryomov@gmail.com> Is it OK for max_segments to be bigger than max_hw_sectors? I didn't investigate but the question occurred to me, since you're fixing max_segments regardless of segment size, but not doing that to the other limits being set. If that is not a problem: Reviewed-by: Alex Elder <elder@linaro.org> > --- > drivers/block/rbd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c > index f40050c97bb3..0bda823a90ca 100644 > --- a/drivers/block/rbd.c > +++ b/drivers/block/rbd.c > @@ -4381,7 +4381,7 @@ static int rbd_init_disk(struct rbd_device *rbd_dev) > segment_size = rbd_obj_bytes(&rbd_dev->header); > blk_queue_max_hw_sectors(q, segment_size / SECTOR_SIZE); > q->limits.max_sectors = queue_max_hw_sectors(q); > - blk_queue_max_segments(q, segment_size / SECTOR_SIZE); > + blk_queue_max_segments(q, USHRT_MAX); > blk_queue_max_segment_size(q, segment_size); > blk_queue_io_min(q, segment_size); > blk_queue_io_opt(q, segment_size); > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 9, 2018 at 12:35 AM, Alex Elder <elder@ieee.org> wrote: > On 01/08/2018 12:51 PM, Ilya Dryomov wrote: >> Commit d3834fefcfe5 ("rbd: bump queue_max_segments") bumped >> max_segments (unsigned short) to max_hw_sectors (unsigned int). >> max_hw_sectors is set to the number of 512-byte sectors in an object >> and overflows unsigned short for 32M (largest possible) objects, making >> the block layer resort to handing us single segment (i.e. single page >> or even smaller) bios in that case. >> >> Cc: stable@vger.kernel.org >> Fixes: d3834fefcfe5 ("rbd: bump queue_max_segments") >> Signed-off-by: Ilya Dryomov <idryomov@gmail.com> > > Is it OK for max_segments to be bigger than max_hw_sectors? I think so. max_hw_sectors (or rather max_sectors -- a soft limit capped by max_hw_sectors hard limit) limits the size of the request. max_segments limits the number of segments (~ bvecs) and rbd is fine with any number. > > I didn't investigate but the question occurred to me, since you're > fixing max_segments regardless of segment size, but not doing that > to the other limits being set. Yeah, queue_max_segment_size could be UINT_MAX and we could probably disable SG merging (BLK_MQ_F_SG_MERGE) which we don't benefit from. This is a small fix intended for stable though. Thanks for the review! Ilya -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" 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/drivers/block/rbd.c b/drivers/block/rbd.c index f40050c97bb3..0bda823a90ca 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -4381,7 +4381,7 @@ static int rbd_init_disk(struct rbd_device *rbd_dev) segment_size = rbd_obj_bytes(&rbd_dev->header); blk_queue_max_hw_sectors(q, segment_size / SECTOR_SIZE); q->limits.max_sectors = queue_max_hw_sectors(q); - blk_queue_max_segments(q, segment_size / SECTOR_SIZE); + blk_queue_max_segments(q, USHRT_MAX); blk_queue_max_segment_size(q, segment_size); blk_queue_io_min(q, segment_size); blk_queue_io_opt(q, segment_size);
Commit d3834fefcfe5 ("rbd: bump queue_max_segments") bumped max_segments (unsigned short) to max_hw_sectors (unsigned int). max_hw_sectors is set to the number of 512-byte sectors in an object and overflows unsigned short for 32M (largest possible) objects, making the block layer resort to handing us single segment (i.e. single page or even smaller) bios in that case. Cc: stable@vger.kernel.org Fixes: d3834fefcfe5 ("rbd: bump queue_max_segments") Signed-off-by: Ilya Dryomov <idryomov@gmail.com> --- drivers/block/rbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)