On Sat, Mar 28, 2020 at 8:57 PM Tokunori Ikegami <ikegami.t@xxxxxxxxx> wrote: > > Hi, > > On 2020/03/28 11:11, Ming Lei wrote: > > On Sat, Mar 28, 2020 at 2:18 AM Keith Busch <kbusch@xxxxxxxxxx> wrote: > >> On Sat, Mar 28, 2020 at 02:50:43AM +0900, Tokunori Ikegami wrote: > >>> On 2020/03/25 1:51, Tokunori Ikegami wrote: > >>>> On 2020/03/24 9:02, Keith Busch wrote: > >>>>> We didn't have 32-bit max segments before, though. Why was 16-bits > >>>>> enough in older kernels? Which kernel did this stop working? > >>>> Now I am asking the detail information to the reporter so let me > >>>> update later. That was able to use the same command script with the > >>>> large data length in the past. > >>> I have just confirmed the detail so let me update below. > >>> > >>> The data length 20,531,712 (0x1394A00) is used on kernel 3.10.0 (CentOS > >>> 64bit). > >>> Also it is failed on kernel 10 4.10.0 (Ubuntu 32bit). > >>> But just confirmed it as succeeded on both 4.15.0 (Ubuntu 32bit) and 4.15.1 > >>> (Ubuntu 64bit). > >>> So the original 20,531,712 length failure issue seems already resolved. > >>> > >>> I tested the data length 0x10000000 (268,435,456) and it is failed > >>> But now confirmed it as failed on all the above kernel versions. > >>> Also the patch fixes only this 0x10000000 length failure issue. > >> This is actually even more confusing. We do not support 256MB transfers > >> within a single command in the pci nvme driver anymore. The max is 4MB, > >> so I don't see how increasing the max segments will help: you should be > >> hitting the 'max_sectors' limit if you don't hit the segment limit first. > > That looks a bug for passthrough req, because 'max_sectors' limit is only > > checked in bio_add_pc_page(), not done in blk_rq_append_bio(), something > > like the following seems required: > > > > diff --git a/block/blk-map.c b/block/blk-map.c > > index b0790268ed9d..e120d80b75a5 100644 > > --- a/block/blk-map.c > > +++ b/block/blk-map.c > > @@ -22,6 +22,10 @@ int blk_rq_append_bio(struct request *rq, struct bio **bio) > > struct bio_vec bv; > > unsigned int nr_segs = 0; > > > > + if (((rq->__data_len + (*bio)->bi_iter.bi_size) >> 9) > > > + queue_max_hw_sectors(rq->q)) > > + return -EINVAL; > > + > > I have just confirmed about the max_hw_sectors checking below. > It is checked by the function blk_rq_map_kern() also as below. > > if (len > (queue_max_hw_sectors(q) << 9)) > return -EINVAL; The above check doesn't take rq->__data_len into account. Thanks, Ming Lei
