On 2020/03/29 12:01, Ming Lei wrote:
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 for the comment and noted it.
I have just confirmed the behavior on 5.6.0-rc7 as below.
It works to limit the data length size 4MB as expected basically.
Also it can be limited by the check existed below in ll_back_merge_fn().
if (blk_rq_sectors(req) + bio_sectors(bio) >
blk_rq_get_max_sectors(req, blk_rq_pos(req))) {
But there is a case that the command data length is limited by 512KB.
I am not sure about this condition so needed more investigation.
Regards,
Ikegami
