Re: [PATCH 2/2] mm: Make swap_readpage() for SWP_FS_OPS use ->direct_IO() not ->readpage()

From: David Howells
Date: Thu Aug 12 2021 - 08:57:18 EST


Christoph Hellwig <hch@xxxxxx> wrote:

> On Thu, Aug 12, 2021 at 12:57:58PM +0100, David Howells wrote:
> > Make swap_readpage(), when accessing a swap file (SWP_FS_OPS) use
> > the ->direct_IO() method on the filesystem rather then ->readpage().
>
> ->direct_IO is just a helper for ->read_iter and ->write_iter, so please
> don't call it directly. It actually is slowly on its way out, with at
> at least all of the iomap implementations not using it, as well as various
> other file systems.

[Note that __swap_writepage() uses ->direct_IO().]

Calling ->write_iter is probably a bad idea here. Imagine that it goes
through, say, generic_file_write_iter(), then __generic_file_write_iter() and
then generic_file_direct_write(). It adds a number of delays into the system,
including:

- Taking the inode lock
- Removing file privs
- Cranking mtime, ctime, file version
- Doing mnt_want_write
- Setting the inode dirty
- Waiting on pages in the range that are being written
- Walking over the pagecache to invalidate the range
- Redoing the invalidation (can't be skipped since page 0 is pinned)

that we might want to skip as they'll end up being done for every page swapped
out.

> > + ki = kzalloc(sizeof(*ki), GFP_KERNEL);
> > + if (!ki)
> > + return -ENOMEM;
>
> for the synchronous case we could avoid this allocation and just use
> arguments on stack.

True.

David