Re: [PATCH 4/4] ceph: remove page_snap_context()

Matthew Wilcox <[email protected]>
Newsgroups gmane.linux.kernel,gmane.comp.file-systems.ceph.devel,gmane.linux.file-systems,gmane.linux.kernel.mm
Message-ID <[email protected]>
On Sun, Aug 02, 2026 at 12:50:05PM -0400, Tal Zussman wrote:
> Convert the final caller in get_writepages_data_length() to use a folio
> and ceph_folio_snap_context(), then remove page_snap_context().
> 
> This drops the last open-coded use of page->private in ceph's writeback
> path.

This one I'm deeply conflicted about.  It's adding an extra call to
compound_head() ... and we're not getting much for it.

I'd feel better about it if it started with::

 static u64 get_writepages_data_length(struct inode *inode,
                                       struct page *page, u64 start)
 {
+	struct folio *folio = page_folio(page);

and then we had a ceph_fscrypt_pagecache_folio() function and
ceph_fscrypt_folio_offset() (we already have a fscrypt_is_bounce_page())

That way we'd have this function entirely converted except for its
argument, and a future patch can do the conversion with little fuss.
And we'd get rid of one of the four remaining calls to
fscrypt_is_bounce_page()

> Signed-off-by: Tal Zussman <[email protected]>
> ---
>  fs/ceph/addr.c | 15 ++++-----------
>  1 file changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
> index f4aaf9a5f196..702cf5fc1eab 100644
> --- a/fs/ceph/addr.c
> +++ b/fs/ceph/addr.c
> @@ -29,9 +29,9 @@
>   *
>   * There are a few funny things going on here.
>   *
> - * The page->private field is used to reference a struct
> - * ceph_snap_context for _every_ dirty page.  This indicates which
> - * snapshot the page was logically dirtied in, and thus which snap
> + * The folio->private field is used to reference a struct
> + * ceph_snap_context for _every_ dirty folio.  This indicates which
> + * snapshot the folio was logically dirtied in, and thus which snap
>   * context needs to be associated with the osd write during writeback.
>   *
>   * Similarly, struct ceph_inode_info maintains a set of counters to
> @@ -68,13 +68,6 @@
>  static int ceph_netfs_check_write_begin(struct file *file, loff_t pos, unsigned int len,
>  					struct folio **foliop, void **_fsdata);
>  
> -static inline struct ceph_snap_context *page_snap_context(struct page *page)
> -{
> -	if (PagePrivate(page))
> -		return (void *)page->private;
> -	return NULL;
> -}
> -
>  static inline struct ceph_snap_context *ceph_folio_snap_context(struct folio *folio)
>  {
>  	if (folio_test_private(folio))
> @@ -697,7 +690,7 @@ static u64 get_writepages_data_length(struct inode *inode,
>  	u64 end = i_size_read(inode);
>  	u64 ret;
>  
> -	snapc = page_snap_context(ceph_fscrypt_pagecache_page(page));
> +	snapc = ceph_folio_snap_context(page_folio(ceph_fscrypt_pagecache_page(page)));
>  	if (snapc != ci->i_head_snapc) {
>  		bool found = false;
>  		spin_lock(&ci->i_ceph_lock);
> 
> -- 
> 2.39.5
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.