RE: Ceph and Netfslib

[Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>]

On Tue, 2024-12-24 at 12:56 +0000, Matthew Wilcox wrote:
> On Mon, Dec 23, 2024 at 11:13:47PM +0000, Viacheslav Dubeyko wrote:
> >  * On writeback, we must submit writes to the osd IN SNAP ORDER. 
> > So,
> >  * we look for the first capsnap in i_cap_snaps and write out pages
> > in
> >  * that snap context _only_.  Then we move on to the next capsnap,
> >  * eventually reaching the "live" or "head" context (i.e., pages
> > that
> >  * are not yet snapped) and are writing the most recently dirtied
> >  * pages
> 
> Speaking of writeback, ceph doesn't need a writepage operation. 
> We're
> removing ->writepage from filesystems in favour of using -
> >migrate_folio
> for migration and ->writepages for writeback.  As far as I can tell,
> filemap_migrate_folio() will be perfect for ceph (as the
> ceph_snap_context
> contains no references to the address of the memory).  And ceph
> already
> has a ->writepages.  So I think this patch should work.  Can you give
> it
> a try?
> 

Sorry for some delay.

I did the testing of this modification. As far as I can see, as ceph
related as generic xfstests are going into ceph_writepages_start
(writepages). Even if I am creating a small file (<= 4096), then it is
processed by ceph_writepages_start() again. So, as far as I can see,
this modification should be safe enough. Running xfstests didn't reveal
any critical issues related to writepage family in Ceph. If I am
missing something, then I am ready to execute additional testing.

Thanks,
Slava.

> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
> index 85936f6d2bf7..5a5a870b6aee 100644
> --- a/fs/ceph/addr.c
> +++ b/fs/ceph/addr.c
> @@ -810,32 +810,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_fs_client(inode)->write_congested) {
> -		redirty_page_for_writepage(wbc, page);
> -		return AOP_WRITEPAGE_ACTIVATE;
> -	}
> -
> -	folio_wait_private_2(page_folio(page)); /* [DEPRECATED] */
> -
> -	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.
>   *
> @@ -1584,7 +1558,6 @@ static int ceph_write_end(struct file *file,
> struct address_space *mapping,
>  const struct address_space_operations ceph_aops = {
>  	.read_folio = netfs_read_folio,
>  	.readahead = netfs_readahead,
> -	.writepage = ceph_writepage,
>  	.writepages = ceph_writepages_start,
>  	.write_begin = ceph_write_begin,
>  	.write_end = ceph_write_end,
> @@ -1592,6 +1565,7 @@ const struct address_space_operations ceph_aops
> = {
>  	.invalidate_folio = ceph_invalidate_folio,
>  	.release_folio = netfs_release_folio,
>  	.direct_IO = noop_direct_IO,
> +	.migrate_folio = filemap_migrate_folio,
>  };
>  
>  static void ceph_block_sigs(sigset_t *oldset)