Re: [PATCH] mm: readahead: remove redundant ra_pages in file_ra_state

[YingHang Zhu]

On Thu, Oct 25, 2012 at 10:12 AM, Ni zhan Chen <nizhan.chen@xxxxxxxxx> wrote:
> On 10/25/2012 10:04 AM, YingHang Zhu wrote:
>>
>> On Thu, Oct 25, 2012 at 9:50 AM, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
>>>
>>> On Thu, Oct 25, 2012 at 08:17:05AM +0800, YingHang Zhu wrote:
>>>>
>>>> On Thu, Oct 25, 2012 at 4:19 AM, Dave Chinner <david@xxxxxxxxxxxxx>
>>>> wrote:
>>>>>
>>>>> On Wed, Oct 24, 2012 at 07:53:59AM +0800, YingHang Zhu wrote:
>>>>>>
>>>>>> Hi Dave,
>>>>>> On Wed, Oct 24, 2012 at 6:47 AM, Dave Chinner <david@xxxxxxxxxxxxx>
>>>>>> wrote:
>>>>>>>
>>>>>>> On Tue, Oct 23, 2012 at 08:46:51PM +0800, Ying Zhu wrote:
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>    Recently we ran into the bug that an opened file's ra_pages does
>>>>>>>> not
>>>>>>>> synchronize with it's backing device's when the latter is changed
>>>>>>>> with blockdev --setra, the application needs to reopen the file
>>>>>>>> to know the change,
>>>>>>>
>>>>>>> or simply call fadvise(fd, POSIX_FADV_NORMAL) to reset the readhead
>>>>>>> window to the (new) bdi default.
>>>>>>>
>>>>>>>> which is inappropriate under our circumstances.
>>>>>>>
>>>>>>> Which are? We don't know your circumstances, so you need to tell us
>>>>>>> why you need this and why existing methods of handling such changes
>>>>>>> are insufficient...
>>>>>>>
>>>>>>> Optimal readahead windows tend to be a physical property of the
>>>>>>> storage and that does not tend to change dynamically. Hence block
>>>>>>> device readahead should only need to be set up once, and generally
>>>>>>> that can be done before the filesystem is mounted and files are
>>>>>>> opened (e.g. via udev rules). Hence you need to explain why you need
>>>>>>> to change the default block device readahead on the fly, and why
>>>>>>> fadvise(POSIX_FADV_NORMAL) is "inappropriate" to set readahead
>>>>>>> windows to the new defaults.
>>>>>>
>>>>>> Our system is a fuse-based file system, fuse creates a
>>>>>> pseudo backing device for the user space file systems, the default
>>>>>> readahead
>>>>>> size is 128KB and it can't fully utilize the backing storage's read
>>>>>> ability,
>>>>>> so we should tune it.
>>>>>
>>>>> Sure, but that doesn't tell me anything about why you can't do this
>>>>> at mount time before the application opens any files. i.e.  you've
>>>>> simply stated the reason why readahead is tunable, not why you need
>>>>> to be fully dynamic.....
>>>>
>>>> We store our file system's data on different disks so we need to change
>>>> ra_pages
>>>> dynamically according to where the data resides, it can't be fixed at
>>>> mount time
>>>> or when we open files.
>>>
>>> That doesn't make a whole lot of sense to me. let me try to get this
>>> straight.
>>>
>>> There is data that resides on two devices (A + B), and a fuse
>>> filesystem to access that data. There is a single file in the fuse
>>> fs has data on both devices. An app has the file open, and when the
>>> data it is accessing is on device A you need to set the readahead to
>>> what is best for device A? And when the app tries to access data for
>>> that file that is on device B, you need to set the readahead to what
>>> is best for device B? And you are changing the fuse BDI readahead
>>> settings according to where the data in the back end lies?
>>>
>>> It seems to me that you should be setting the fuse readahead to the
>>> maximum of the readahead windows the data devices have configured at
>>> mount time and leaving it at that....
>>
>> Then it may not fully utilize some device's read IO bandwidth and put too
>> much
>> burden on other devices.
>>>>
>>>> The abstract bdi of fuse and btrfs provides some dynamically changing
>>>> bdi.ra_pages
>>>> based on the real backing device. IMHO this should not be ignored.
>>>
>>> btrfs simply takes into account the number of disks it has for a
>>> given storage pool when setting up the default bdi ra_pages during
>>> mount.  This is basically doing what I suggested above.  Same with
>>> the generic fuse code - it's simply setting a sensible default value
>>> for the given fuse configuration.
>>>
>>> Neither are dynamic in the sense you are talking about, though.
>>
>> Actually I've talked about it with Fengguang, he advised we should unify
>> the
>
>
> But how can bdi related ra_pages reflect different files' readahead window?
> Maybe these different files are sequential read, random read and so on.
I think you mean the dynamic tuning of readahead window, that's exactly the job
of readahead algorithm and it's reflected by file_ra_state.sync_size and
file_ra_state.async_size.
The ra_pages in struct file_ra_state only means the max readahead ability.

Thanks,
    Ying Zhu
>
>> ra_pages in struct bdi and file_ra_state and leave the issue that
>> spreading data
>> across disks as it is.
>> Fengguang, what's you opinion about this?
>>
>> Thanks,
>>           Ying Zhu
>>>
>>> Cheers,
>>>
>>> Dave.
>>> --
>>> Dave Chinner
>>> david@xxxxxxxxxxxxx
>>
>> --
>>
>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>> the body to majordomo@xxxxxxxxxx  For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/