Re: [PATCH] ceph: fix snapshot context missing in ceph_uninline_data

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

On Wed, 2025-09-24 at 17:58 +0800, ethanwu wrote:
> The ceph_uninline_data function was missing proper snapshot context
> handling for its OSD write operations. Both CEPH_OSD_OP_CREATE and
> CEPH_OSD_OP_WRITE requests were passing NULL instead of the appropriate
> snapshot context, which could lead to data inconsistencies in snapshots.
> 

What is the reproducing path for this issue? How is it possible to check that
something is going wrong or functionality works right?

> This fix properly acquiring the snapshot context from either pending
> cap snaps or the inode's head snapc before performing write operations.
> 

Have you split the whole fix on two patches because of different use-cases?

Thanks,
Slava.

> Signed-off-by: ethanwu <ethanwu@synology.com>
> ---
>  fs/ceph/addr.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
> index 8b202d789e93..a8aeca9654b6 100644
> --- a/fs/ceph/addr.c
> +++ b/fs/ceph/addr.c
> @@ -2202,6 +2202,7 @@ int ceph_uninline_data(struct file *file)
>  	struct ceph_osd_request *req = NULL;
>  	struct ceph_cap_flush *prealloc_cf = NULL;
>  	struct folio *folio = NULL;
> +	struct ceph_snap_context *snapc = NULL;
>  	u64 inline_version = CEPH_INLINE_NONE;
>  	struct page *pages[1];
>  	int err = 0;
> @@ -2229,6 +2230,19 @@ int ceph_uninline_data(struct file *file)
>  	if (inline_version == 1) /* initial version, no data */
>  		goto out_uninline;
>  
> +	spin_lock(&ci->i_ceph_lock);
> +	if (__ceph_have_pending_cap_snap(ci)) {
> +		struct ceph_cap_snap *capsnap =
> +				list_last_entry(&ci->i_cap_snaps,
> +						struct ceph_cap_snap,
> +						ci_item);
> +		snapc = ceph_get_snap_context(capsnap->context);
> +	} else {
> +		BUG_ON(!ci->i_head_snapc);
> +		snapc = ceph_get_snap_context(ci->i_head_snapc);
> +	}
> +	spin_unlock(&ci->i_ceph_lock);
> +
>  	folio = read_mapping_folio(inode->i_mapping, 0, file);
>  	if (IS_ERR(folio)) {
>  		err = PTR_ERR(folio);
> @@ -2244,7 +2258,7 @@ int ceph_uninline_data(struct file *file)
>  	req = ceph_osdc_new_request(&fsc->client->osdc, &ci->i_layout,
>  				    ceph_vino(inode), 0, &len, 0, 1,
>  				    CEPH_OSD_OP_CREATE, CEPH_OSD_FLAG_WRITE,
> -				    NULL, 0, 0, false);
> +				    snapc, 0, 0, false);
>  	if (IS_ERR(req)) {
>  		err = PTR_ERR(req);
>  		goto out_unlock;
> @@ -2260,7 +2274,7 @@ int ceph_uninline_data(struct file *file)
>  	req = ceph_osdc_new_request(&fsc->client->osdc, &ci->i_layout,
>  				    ceph_vino(inode), 0, &len, 1, 3,
>  				    CEPH_OSD_OP_WRITE, CEPH_OSD_FLAG_WRITE,
> -				    NULL, ci->i_truncate_seq,
> +				    snapc, ci->i_truncate_seq,
>  				    ci->i_truncate_size, false);
>  	if (IS_ERR(req)) {
>  		err = PTR_ERR(req);
> @@ -2323,6 +2337,7 @@ int ceph_uninline_data(struct file *file)
>  		folio_put(folio);
>  	}
>  out:
> +	ceph_put_snap_context(snapc);
>  	ceph_free_cap_flush(prealloc_cf);
>  	doutc(cl, "%llx.%llx inline_version %llu = %d\n",
>  	      ceph_vinop(inode), inline_version, err);