On Thu 09-02-23 17:54:14, John Hubbard wrote:

On 2/9/23 04:31, Jan Kara wrote:

quoted

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@suse.cz>
---
  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 [off-list ref]
SUSE Labs, CR