[PATCH 1/4] ubifs: Convert from writepage to writepages

[Zhihao Cheng]

在 2023/6/6 22:37, Zhihao Cheng 写道:
> 在 2023/6/6 0:50, Matthew Wilcox (Oracle) 写道:
> Hi,
>> This is a simplistic conversion to separate out any effects of
>> no longer having a writepage method.
>>
>> Signed-off-by: Matthew Wilcox (Oracle) <willy at infradead.org>
>> ---
>>   fs/ubifs/file.c | 12 ++++++++++--
>>   1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
>> index 979ab1d9d0c3..8bb4cb9d528f 100644
>> --- a/fs/ubifs/file.c
>> +++ b/fs/ubifs/file.c
>> @@ -1003,8 +1003,10 @@ static int do_writepage(struct page *page, int 
>> len)
>>    * on the page lock and it would not write the truncated inode node 
>> to the
>>    * journal before we have finished.
>>    */
>> -static int ubifs_writepage(struct page *page, struct 
>> writeback_control *wbc)
>> +static int ubifs_writepage(struct folio *folio, struct 
>> writeback_control *wbc,
>> +        void *data)
>>   {
>> +    struct page *page = &folio->page;
>>       struct inode *inode = page->mapping->host;
>>       struct ubifs_info *c = inode->i_sb->s_fs_info;
>>       struct ubifs_inode *ui = ubifs_inode(inode);
>> @@ -1076,6 +1078,12 @@ static int ubifs_writepage(struct page *page, 
>> struct writeback_control *wbc)
>>       return err;
>>   }
>> +static int ubifs_writepages(struct address_space *mapping,
>> +        struct writeback_control *wbc)
>> +{
>> +    return write_cache_pages(mapping, wbc, ubifs_writepage, NULL);
>> +}
>> +
> 
> There is a small difference.
> before patch applied:
> do_writepages -> write_cache_pages -> writepage_cb:
>   ubifs_writepage
>   mapping_set_error(mapping, ret)
> 
> So, we can get error returned from errseq_check_and_advance in syncfs 
> syscall if ubifs_writepage occurs EIO.
> 
> after patch applied:
> 
> do_writepages -> ubifs_writepages -> write_cache_pages -> 
> ubifs_writepage, mapping won't be set error if ubifs_writepage failed. 
> Unfortunately, ubifs is not a block filesystem, so 
> sync_filesystem->sync_blockdev_nowait will return 0. Finally, syncfs 
> syscall will return 0.
> 

I think we can add mapping_set_error in error branch of ubifs_writepage 
to solve it.

BTW, I notice that shrink_folio_list -> pageout will try to shrink page 
by writepage method, if we remove '->writepage', the dirty page won't be 
shrinked in that way?

> 
>>   /**
>>    * do_attr_changes - change inode attributes.
>>    * @inode: inode to change attributes for
>> @@ -1636,7 +1644,7 @@ static int ubifs_symlink_getattr(struct 
>> mnt_idmap *idmap,
>>   const struct address_space_operations ubifs_file_address_operations = {
>>       .read_folio     = ubifs_read_folio,
>> -    .writepage      = ubifs_writepage,
>> +    .writepages     = ubifs_writepages,
>>       .write_begin    = ubifs_write_begin,
>>       .write_end      = ubifs_write_end,
>>       .invalidate_folio = ubifs_invalidate_folio,
>>
> 
> 
> .