On Thu 09-02-23 17:54:14, John Hubbard wrote: > On 2/9/23 04:31, Jan Kara wrote: > > When a folio is pinned, there is no point in trying to write it during > > memory cleaning writeback. We cannot reclaim the folio until it is > > unpinned anyway and we cannot even be sure the folio is really clean. > > On top of that writeback of such folio may be problematic as the data > > can change while the writeback is running thus causing checksum or > > DIF/DIX failures. So just don't bother doing memory cleaning writeback > > for pinned folios. > > > > Signed-off-by: Jan Kara <jack@xxxxxxx> > > --- > > fs/9p/vfs_addr.c | 2 +- > > fs/afs/file.c | 2 +- > > fs/afs/write.c | 6 +++--- > > fs/btrfs/extent_io.c | 14 +++++++------- > > fs/btrfs/free-space-cache.c | 2 +- > > fs/btrfs/inode.c | 2 +- > > fs/btrfs/subpage.c | 2 +- > > Hi Jan! > > Just a quick note that this breaks the btrfs build in subpage.c. > Because, unfortunately, btrfs creates 6 sets of functions via calls to a > macro: IMPLEMENT_BTRFS_PAGE_OPS(). And that expects only one argument to > the clear_page_func, and thus to clear_page_dirty_for_io(). > > It seems infeasible (right?) to add another argument to the other > clear_page_func functions, which include: > > ClearPageUptodate > ClearPageError > end_page_writeback > ClearPageOrdered > ClearPageChecked > > , so I expect IMPLEMENT_BTRFS_PAGE_OPS() may need to be partially > unrolled, in order to pass in the new writeback control arg to > clear_page_dirty_for_io(). Aha, thanks for catching this. So it is easy to fix this to make things compile (just a wrapper around clear_page_dirty_for_io() - done now). It will be a bit more challenging to propagate wbc into there for proper decision - that will probably need these functions not to be defined by the macros. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
