Re: [PATCH 08/15] ubifs: Convert ubifs_write_begin() to use a folio

Zhihao Cheng
2024-01-23T02:16:23Z

在 2024/1/22 22:43, Matthew Wilcox 写道:
> On Mon, Jan 22, 2024 at 07:51:03PM +0800, Zhihao Cheng wrote:
>>> @@ -437,13 +437,14 @@ static int ubifs_write_begin(struct file *file, struct address_space *mapping,
>>> return -EROFS;
>>> /* Try out the fast-path part first */
>>> - page = grab_cache_page_write_begin(mapping, index);
>>> - if (unlikely(!page))
>>> - return -ENOMEM;
>>> + folio = __filemap_get_folio(mapping, index, FGP_WRITEBEGIN,
>>> + mapping_gfp_mask(mapping));
>>> + if (IS_ERR(folio))
>>> + return PTR_ERR(folio);
>>> - if (!PageUptodate(page)) {
>>> + if (!folio_test_uptodate(folio)) {
>>> /* The page is not loaded from the flash */
>>> - if (!(pos & ~PAGE_MASK) && len == PAGE_SIZE) {
>>> + if (pos == folio_pos(folio) && len >= folio_size(folio)) {
>>
>> Why not len == folio_size(folio)? Although 'len >= folio_size(folio)' is not
>> wrong.
>
> This is based on my experience with iomap. Today, the caller passes in
> min(length-of-write, PAGE_SIZE). In order to be able to create large
> folios in the write path, we want to change the caller of write_begin
> to pass in the length of the write, so we can see a len which is larger
> than the size of the folio we found.
>
> .
>

I guess you are preparing for adding large folio support for
generic_perform_write path, may I have a look for the demo code? I'm a
little confused for this words "we want to change the caller of
write_begin to pass in the length of the write". According to the
implementation of iomap_write_iter(), I find the write length in each
iteration is min(write_bytes, folio_size), so I guess the write length
passed in ubifs_write_begin won't exceed folio_size.