lkml.org 
[lkml]   [2020]   [Aug]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] fat: Avoid oops when bdi->io_pages==0
Date
Matthew Wilcox <willy@infradead.org> writes:

> On Sun, Aug 30, 2020 at 10:54:35AM +0900, OGAWA Hirofumi wrote:
>> Matthew Wilcox <willy@infradead.org> writes:
>>
>> Hm, io_pages is limited by driver setting too, and io_pages can be lower
>> than ra_pages, e.g. usb storage.
>>
>> Assuming ra_pages is user intent of readahead window. So if io_pages is
>> lower than ra_pages, this try ra_pages to align of io_pages chunk, but
>> not bigger than ra_pages. Because if block layer splits I/O requests to
>> hard limit, then I/O is not optimal.
>>
>> So it is intent, I can be misunderstanding though.
>
> Looking at this some more, I'm not sure it makes sense to consult ->io_pages
> at all. I see how it gets set to 0 -- the admin can write '1' to
> /sys/block/<device>/queue/max_sectors_kb and that gets turned into 0
> in ->io_pages.

if (max_sectors_kb > max_hw_sectors_kb || max_sectors_kb < page_kb)
return -EINVAL;

It should not set to 0 via /sys/.../max_sectors_kb. However the default
of bdi->io_pages is 0. So it happened if a driver didn't initialized it.

> But I'm not sure it makes any sense to respect that. Looking at
> mm/readahead.c, all it does is limit the size of a read request which
> exceeds the current readahead window. It's not used to limit the
> readahead window itself. For example:
>
> unsigned long max_pages = ra->ra_pages;
> ...
> if (req_size > max_pages && bdi->io_pages > max_pages)
> max_pages = min(req_size, bdi->io_pages);
>
> Setting io_pages below ra_pages has no effect. So maybe fat should also
> disregard it?

|-----------------------| requested blocks
[before]
ra_pages |===========|===========|===========|
io_pages |---------|---------|---------|
req |---------|-|-------|---|

[after]
ra_pages |=========|=========|=========|
io_pages |---------|---------|---------|
req |---------|---------|---|

This path is known the large sequential read there. Well, anyway, this
intent is to use [after] as 3 req, instead of [before] as 4 req.

Thanks.
--
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

\
 
 \ /
  Last update: 2020-08-30 11:05    [W:0.033 / U:6.440 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site