[PATCH AUTOSEL 6.19-6.1] ceph: supply snapshot context in ceph_uninline_data()
From: Sasha Levin
Date: Fri Feb 20 2026 - 07:42:13 EST
From: ethanwu <ethanwu@xxxxxxxxxxxx>
[ Upstream commit 305ff6b3a03c230d3c07b61457e961406d979693 ]
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 unnecessary object clone.
Reproducer:
../src/vstart.sh --new -x --localhost --bluestore
// turn on cephfs inline data
./bin/ceph fs set a inline_data true --yes-i-really-really-mean-it
// allow fs_a client to take snapshot
./bin/ceph auth caps client.fs_a mds 'allow rwps fsname=a' mon 'allow r fsname=a' osd 'allow rw tag cephfs data=a'
// mount cephfs with fuse, since kernel cephfs doesn't support inline write
ceph-fuse --id fs_a -m 127.0.0.1:40318 --conf ceph.conf -d /mnt/mycephfs/
// bump snapshot seq
mkdir /mnt/mycephfs/.snap/snap1
echo "foo" > /mnt/mycephfs/test
// umount and mount it again using kernel cephfs client
umount /mnt/mycephfs
mount -t ceph fs_a@.a=/ /mnt/mycephfs/ -o conf=./ceph.conf
echo "bar" >> /mnt/mycephfs/test
./bin/rados listsnaps -p cephfs.a.data $(printf "%x\n" $(stat -c %i /mnt/mycephfs/test)).00000000
will see this object does unnecessary clone
1000000000a.00000000 (seq:2):
cloneid snaps size overlap
2 2 4 []
head - 8
but it's expected to see
10000000000.00000000 (seq:2):
cloneid snaps size overlap
head - 8
since there's no snapshot between these 2 writes
clone happened because the first osd request CEPH_OSD_OP_CREATE doesn't
pass snap context so object is created with snap seq 0, but later data
writeback is equipped with snapshot context.
snap.seq(1) > object snap seq(0), so osd does object clone.
This fix properly acquiring the snapshot context before performing
write operations.
Signed-off-by: ethanwu <ethanwu@xxxxxxxxxxxx>
Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@xxxxxxx>
Tested-by: Viacheslav Dubeyko <Slava.Dubeyko@xxxxxxx>
Signed-off-by: Ilya Dryomov <idryomov@xxxxxxxxx>
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---
LLM Generated explanations, may be completely bogus:
Now I have a comprehensive understanding. Let me also check the
`__ceph_mark_dirty_caps` function which is where `i_head_snapc` normally
gets populated:
The pattern at `caps.c:1813-1817` shows the standard way `i_head_snapc`
gets populated:
```c
if (!ci->i_head_snapc) {
WARN_ON_ONCE(!rwsem_is_locked(&mdsc->snap_rwsem));
ci->i_head_snapc = ceph_get_snap_context(
ci->i_snap_realm->cached_context);
}
```
The fix in `ceph_uninline_data()` uses the same pattern, which confirms
it's correct.
Now let me assess the overall analysis:
## Analysis
### 1. COMMIT MESSAGE ANALYSIS
The commit message clearly explains:
- **What**: `ceph_uninline_data()` passes NULL instead of proper
snapshot context to OSD write operations
- **Why**: This causes unnecessary RADOS object clones when inline data
is uninlined in the presence of snapshots
- **Reproducer**: A detailed step-by-step reproducer is provided
- **Impact**: Unnecessary object clones (storage waste, incorrect
snapshot state)
The commit is reviewed and tested by Viacheslav Dubeyko (IBM) and signed
off by Ilya Dryomov (CephFS maintainer).
### 2. CODE CHANGE ANALYSIS
The fix:
1. Adds snapshot context acquisition before OSD write operations
(following the established pattern from `__ceph_mark_dirty_caps` and
`ceph_write_begin`)
2. Passes `snapc` instead of NULL to both `CEPH_OSD_OP_CREATE` and
`CEPH_OSD_OP_WRITE` requests
3. Properly releases the snapshot context reference with
`ceph_put_snap_context(snapc)` in the cleanup path
The pattern of snapshot context acquisition is identical to what's used
in other CephFS write paths (caps.c:1813-1817, caps.c:2779-2783).
### 3. CLASSIFICATION
This is a **bug fix** - it fixes incorrect behavior where OSD objects
are created with snap_seq=0 instead of the proper snapshot sequence
number, causing unnecessary clones.
### 4. SCOPE AND RISK ASSESSMENT
- **Lines changed**: ~20 lines added (very small)
- **Files touched**: 1 (fs/ceph/addr.c)
- **Pattern**: Well-established, used identically in multiple other
write paths
- **Risk**: Low - the snap context acquisition pattern is battle-tested
in the CephFS codebase
- **Self-contained**: Yes, no dependencies on other patches
### 5. USER IMPACT
- Affects CephFS users with inline data enabled and snapshots
- The bug causes unnecessary RADOS object clones, leading to:
- Wasted storage space
- Incorrect/unexpected snapshot state
- Could confuse backup/recovery tools that rely on snapshot state
### 6. STABILITY INDICATORS
- Reviewed-by and Tested-by from Viacheslav Dubeyko (IBM)
- Signed-off by CephFS maintainer Ilya Dryomov
- Detailed reproducer provided and verified
### 7. HISTORICAL CONTEXT
The bug was introduced in 2016 by commit `34b759b4a22b0` ("ceph: kill
ceph_empty_snapc") which replaced `ceph_empty_snapc` with NULL, claiming
equivalence. While this was true for message sizing, it meant the
snap_seq sent to OSD became 0 instead of the proper value, causing the
clone issue.
### Concerns
- The bug requires a specific (but documented) configuration: CephFS
inline data + snapshots
- Inline data is a deprecated feature in CephFS, but still supported and
used
- The unnecessary clones are a data correctness issue (wrong snapshot
state) rather than a crash/security issue
- The fix is small, well-tested, and follows established patterns
### Verification
- Verified that commit `305ff6b3a03c2` exists and modifies only
`fs/ceph/addr.c`
- Verified the snap context acquisition pattern matches
`caps.c:1813-1817` and `caps.c:2779-2783` (identical pattern)
- Verified that NULL snapc causes `snap_seq=0` to be encoded in OSD
requests (osd_client.c:2198-2199)
- Verified that `ceph_uninline_data()` is called from `ceph_open()` when
opening a file with inline data for writing (file.c:242-243)
- Verified the historical context: commit `7b06a826e7c52` (2015) changed
to `ceph_empty_snapc`, then commit `34b759b4a22b0` (2016) changed to
NULL, introducing the bug
- Verified the commit is reviewed by Viacheslav Dubeyko and signed off
by Ilya Dryomov (CephFS maintainer)
- Verified the fix follows the standard lock ordering: `snap_rwsem`
(read) → `i_ceph_lock` (spin)
- Could NOT verify whether inline_data feature is commonly used in
production stable kernel environments (unverified)
### Decision
This commit fixes a real bug in CephFS snapshot handling that causes
incorrect RADOS object clones (data correctness issue). The fix is small
(~20 lines), self-contained, follows well-established patterns used
throughout the CephFS codebase, and has been reviewed and tested. The
bug has existed since 2016 and affects all stable kernels. While the
trigger requires inline data + snapshots (a somewhat niche
configuration), the fix is low-risk and addresses incorrect behavior.
**YES**
fs/ceph/addr.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
index 63b75d2142102..faecd9025ee9c 100644
--- a/fs/ceph/addr.c
+++ b/fs/ceph/addr.c
@@ -2199,6 +2199,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;
@@ -2226,6 +2227,24 @@ int ceph_uninline_data(struct file *file)
if (inline_version == 1) /* initial version, no data */
goto out_uninline;
+ down_read(&fsc->mdsc->snap_rwsem);
+ 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 {
+ if (!ci->i_head_snapc) {
+ ci->i_head_snapc = ceph_get_snap_context(
+ ci->i_snap_realm->cached_context);
+ }
+ snapc = ceph_get_snap_context(ci->i_head_snapc);
+ }
+ spin_unlock(&ci->i_ceph_lock);
+ up_read(&fsc->mdsc->snap_rwsem);
+
folio = read_mapping_folio(inode->i_mapping, 0, file);
if (IS_ERR(folio)) {
err = PTR_ERR(folio);
@@ -2241,7 +2260,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;
@@ -2257,7 +2276,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);
@@ -2320,6 +2339,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);
--
2.51.0