Ceph » CephFS

Reproducer: https://github.com/ceph/ceph/pull/62602

There is a cap acquisition issue, in this case the execution enters case 2:

} else if (onfinish) {
    // CASE 2 - non-blocking caller with sync read from the OSD (since client
    //          does not have the required caps or the above config is set)

if (!conf->client_debug_force_sync_read &&
      conf->client_oc &&
      (have & (CEPH_CAP_FILE_CACHE | CEPH_CAP_FILE_LAZYIO))) {
    // CAES 1 - blocking or non-blocking caller with the client holding Fc caps
    //          and client_debug_force_sync_read being default (`false)

comparision b/w bugged and normal case:
normal:

2025-04-01T18:24:55.440+0530 7f5c53cb9640 10 client.4291 get_caps 0x10000000000.head(faked_ino=0 nref=8 ll_ref=2 cap_refs={4=0,1024=1,4096=0,8192=1} open={1=1,3=1} mode=100744 size=131072/4194304 nlink=1 btime=2025-04-01T18:24:54.402387+0530 mtime=2025-04-01T18:24:54.441082+0530 ctime=2025-04-01T18:24:54.441082+0530 change_attr=1 caps=pAsxLsXsxFsxcrwb(0=pAsxLsXsxFsxcrwb) dirty_caps=Fw objectset[0x10000000000 ts 0/0 objects 1 dirty_or_tx 131072] parents=0x1.head["check_async_io_with_two_clients" ref=1 ino=0x10000000000.head csg=1] 0x7f5c0c006f70) have pAsxLsXsxFsxcrwb need Fr want Fc revoking -

bugged:

2025-04-01T18:10:55.699+0530 7f2bf668b640 10 client.4301 get_caps 0x10000000000.head(faked_ino=0 nref=4 ll_ref=1 cap_refs={} open={1=1} mode=100744 size=0/0 nlink=1 btime=2025-04-01T18:10:54.637539+0530 mtime=2025-04-01T18:10:54.637539+0530 ctime=2025-04-01T18:10:54.637539+0530 change_attr=0 caps=pAsLsXsFrw(0=pAsLsXsFrw) objectset[0x10000000000 ts 0/0 objects 0 dirty_or_tx 0]

i.e. CEPH_CAP_FILE_CACHE is missing

im surprised but i see for this case especially, the call block for write is:

else {
    if (f->flags & O_DIRECT)
      _flush_range(in, offset, size);

    // simple, non-atomic sync write
    if (onfinish == nullptr) {
      // We need a safer condition to wait on.
      cond_iofinish = new C_SaferCond();
      filer_iofinish.reset(cond_iofinish);
    } else {
      //Register a wrapper callback C_Lock_Client_Finisher for the C_Write_Finisher which takes 'client_lock'.
      //Use C_OnFinisher for callbacks. The op_cancel_writes has to be called without 'client_lock' held because
      //the callback registered here needs to take it. This would cause incorrect lock order i.e., objecter->rwlock
      //taken by objecter's op_cancel and then 'client_lock' taken by callback. To fix the lock order, queue
      //the callback using the finisher
      filer_iofinish.reset(new C_OnFinisher(new C_Lock_Client_Finisher(this, iofinish.get()), &objecter_finisher));
    }

but it should be:

if (cct->_conf->client_oc &&
      (have & (CEPH_CAP_FILE_BUFFER | CEPH_CAP_FILE_LAZYIO))) {
....
....

Additional info:

if you add:
ceph_ll_getattr(r_cmount, r_inode, &stx, CEPH_STATX_BASIC_STATS, 0, r_perms);

immediately after:
ceph_ll_nonblocking_readv_writev(w_cmount, &io_info);

the problem disappears.

Venky Shankar wrote in #note-6:

This is likely a fallout from the recent path_walk changes.

I read it again, this is async IO, so probably this bug was just discovered. I will have a look. Thanks for the reproducer.

Dhairya Parmar wrote in #note-3:

There is a cap acquisition issue, in this case the execution enters case 2:
[...]
instead of case 1:
[...]

comparision b/w bugged and normal case:
normal:
[...]

bugged:
[...]

i.e. CEPH_CAP_FILE_CACHE is missing

This is expected behavior because with 1 client it can buffer writes and cache reads, but with two clients it can't. So that looks to be as expected — if it weren't doing this separate branch, that would neatly explain the described behavior!

Shachar Sharon wrote in #note-5:

Additional info:

if you add:
> ceph_ll_getattr(r_cmount, r_inode, &stx, CEPH_STATX_BASIC_STATS, 0, r_perms); >

immediately after:
> ceph_ll_nonblocking_readv_writev(w_cmount, &io_info); >

the problem disappears.

This makes me wonder if the issue is size synchronization between the clients, as the writer is extending the file size. It should be able to do the right thing in this case, but perhaps ceph_ll_nonblocking_readv_writev inadvertently skipped that logic.
I know we have major issues with simultaneous writers invoking "append" since the logic there is just "1) check size on mds, 2) issue write at that location to OSDs".

Update: This issue is also reproducible with synchronous IO interfaces.

Venky Shankar wrote in #note-10:

Update: This issue is also reproducible with synchronous IO interfaces.

Oh..how different is the reproducer? At least with the attached reproducer ceph_ll_write() and ceph_ll_read() works fine.

Anoop C S wrote in #note-11:

Venky Shankar wrote in #note-10:

Update: This issue is also reproducible with synchronous IO interfaces.

Oh..how different is the reproducer? At least with the attached reproducer ceph_ll_write() and ceph_ll_read() works fine.

I ran into this with cephfs-mirror daemon. I will share the failed teuthology run where this is seen.

just checking..

Where do we stand w.r.t the underlying issue?

Anoop C S wrote in #note-13:

just checking..

Where do we stand w.r.t the underlying issue?

Was out last week. Will prioritize for this week.

I couldn't spend focus time on this. Give me some more time and I will get it done.

The following seems buggy. In _read():

  } else if (onfinish) {
    // CASE 2 - non-blocking caller with sync read from the OSD (since client
    //          does not have the required caps or the above config is set)

    // handle _sync_read without blocking...
    // This sounds odd, but we want to accomplish what is done in the else
    // branch below but in a non-blocking fashion. The code in _read_sync
    // is duplicated and modified and exists in
    // C_Read_Sync_NonBlocking::finish().

    // trim read based on file size?
    if (std::cmp_greater_equal(offset, in->size) || (size == 0)) {
      // read is requested at the EOF or the read len is zero, therefore just
      // release managed pointers and complete the C_Read_Finisher immediately with 0 bytes

      Context *iof = iofinish.release();
      crf.release();
      iof->complete(0);

      // Signal async completion
      return 0;
    }

In this case, the client does not have Fc caps, but is freely checking in->size, which could be 0 if the MDS hasn't sent the updated mtime/size to the client as part of a write operation from another client.