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 function calls blk_rq_append_bio().
So the max_hw_sectors will be used to check the length with the change
above.
But it seems that there is a difference also for the checking limit
condition.
It seems that it is better to check the limit by blk_rq_map_user()
instead of blk_rq_append_bio().
Or it can be changed to check the limit by blk_rq_append_bio() only
without blk_rq_map_kern().
Regards,
Ikegami
