On 1/7/20 6:38 PM, Damien Le Moal wrote:
On 2020/01/08 11:34, Jens Axboe wrote:
On 1/7/20 7:06 PM, Damien Le Moal wrote:
On 2020/01/08 10:25, Ming Lei wrote:
Commit 429120f3df2d starts to take account of segment's start dma address
when computing max segment size, and data type of 'unsigned long'
is used to do that. However, the segment mask may be 0xffffffff, so
the figured out segment size may be overflowed because DMA address can
be 64bit on 32bit arch.
Fixes the issue by using 'unsigned long long' to compute max segment
size.
Fixes: 429120f3df2d ("block: fix splitting segments on boundary masks")
Reported-by: Guenter Roeck <linux@xxxxxxxxxxxx>
Tested-by: Guenter Roeck <linux@xxxxxxxxxxxx>
Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
---
block/blk-merge.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 347782a24a35..b0fcc72594cb 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -159,12 +159,12 @@ static inline unsigned get_max_io_size(struct request_queue *q,
static inline unsigned get_max_segment_size(const struct request_queue *q,
struct page *start_page,
- unsigned long offset)
+ unsigned long long offset)
{
unsigned long mask = queue_segment_boundary(q);
offset = mask & (page_to_phys(start_page) + offset);
Shouldn't mask be an unsigned long long too for this to give the
expected correct result ?
Don't think so, and the seg boundary is a ulong to begin with as well.
I was referring to 32bits arch were ulong is 32bits. So we would have
offset = 32bits & 64bits;
with the patch applied. But I am not sure how gcc handles that and if
this can be a problem.
Type extension is well defined in the C standard.
The underlying problem here is that mask is 0xffffffff, and
page_to_phys(start_page) as well as offset are sometimes 0.
In this situation, mask - offset + 1 is 0 if offset is a 32 bit
variable, and 0x100000000 if offset is a 64 bit variable.
In the first case, this results in a wrong maximum segment
size of 0.
Guenter
