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

[David Howells <dhowells@redhat.com>]

Christoph Hellwig <hch@lst.de> 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