Re: [PATCH] ceph: don't implement writepage

[Xiubo Li <xiubli@redhat.com>]

On 6/7/22 7:27 PM, Jeff Layton wrote:
> Remove ceph_writepage as it's not strictly required these days.
>
> To quote from commit 21b4ee7029c9 (xfs: drop ->writepage completely):
>
>      ->writepage is only used in one place - single page writeback from
>      memory reclaim. We only allow such writeback from kswapd, not from
>      direct memory reclaim, and so it is rarely used. When it comes from
>      kswapd, it is effectively random dirty page shoot-down, which is
>      horrible for IO patterns. We will already have background writeback
>      trying to clean all the dirty pages in memory as efficiently as
>      possible, so having kswapd interrupt our well formed IO stream only
>      slows things down. So get rid of xfs_vm_writepage() completely.
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>   fs/ceph/addr.c | 25 -------------------------
>   1 file changed, 25 deletions(-)
>
> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
> index 40830cb9b599..3489444c55b9 100644
> --- a/fs/ceph/addr.c
> +++ b/fs/ceph/addr.c
> @@ -680,30 +680,6 @@ static int writepage_nounlock(struct page *page, struct writeback_control *wbc)
>   	return err;
>   }
>   
> -static int ceph_writepage(struct page *page, struct writeback_control *wbc)
> -{
> -	int err;
> -	struct inode *inode = page->mapping->host;
> -	BUG_ON(!inode);
> -	ihold(inode);
> -
> -	if (wbc->sync_mode == WB_SYNC_NONE &&
> -	    ceph_inode_to_client(inode)->write_congested)
> -		return AOP_WRITEPAGE_ACTIVATE;
> -
> -	wait_on_page_fscache(page);
> -
> -	err = writepage_nounlock(page, wbc);
> -	if (err == -ERESTARTSYS) {
> -		/* direct memory reclaimer was killed by SIGKILL. return 0
> -		 * to prevent caller from setting mapping/page error */
> -		err = 0;
> -	}
> -	unlock_page(page);
> -	iput(inode);
> -	return err;
> -}
> -
>   /*
>    * async writeback completion handler.
>    *
> @@ -1394,7 +1370,6 @@ static int ceph_write_end(struct file *file, struct address_space *mapping,
>   const struct address_space_operations ceph_aops = {
>   	.readpage = netfs_readpage,
>   	.readahead = netfs_readahead,
> -	.writepage = ceph_writepage,
>   	.writepages = ceph_writepages_start,
>   	.write_begin = ceph_write_begin,
>   	.write_end = ceph_write_end,

Sounds reasonable.

Will merge it into the testing branch.

Thanks Jeff!

-- Xiubo