On 29/01/2023 01:41, Ilya Dryomov wrote:
On Sat, Jan 28, 2023 at 4:12 AM Xiubo Li <xiubli@xxxxxxxxxx> wrote:
[...]
Hi Xiubo,
I haven't looked into the actual problem that you trying to fix here
but this patch seems wrong and very unlikely to fly. The messenger
invokes the OSD request callback outside of osd->lock and osdc->lock
critical sections on purpose to avoid deadlocks. This goes way back
and also consistent with how Objecter behaves in userspace.
Hi Ilya
This is just a draft patch here.
I didn't test other cases yet and only tested the issue here in cephfs
and it succeeded.
The root cause is the sequence issue to make the sync_filesystem()
failing to wait the last osd request, which is the last
req->r_callback() isn't finished yet the waiter is woke up. And more
detail please see my comment in
https://tracker.ceph.com/issues/58126?issue_count=405&issue_position=3&next_issue_id=58082&prev_issue_id=58564#note-7.
Hi Xiubo,
This seems to boil down to an expectation mismatch. The diagram in the
tracker comment seems to point the finger at ceph_osdc_sync() because
it appears to return while req->r_callback is still running. This can
indeed be the case but note that ceph_osdc_sync() has never waited for
any higher-level callback -- req->r_callback could schedule some
delayed work which the OSD client would know nothing about, after all.
ceph_osdc_sync() just waits for a safe/commit reply from an OSD (and
nowadays all replies are safe -- unsafe/ack replies aren't really
a thing anymore).
Hi Ilya,
Yeah, for cephfs the req->r_callback() will release all the resources
directly the unmounting thread is waiting for and it's enough.
For the higher-level callback you mentioned above in cephfs it should be
at the same time the last req->r_callback() will flush the dirty
cap/snap to MDSs, which may send some ack replies back later and will
ihold() the inodes, and this will cause another crash in
https://tracker.ceph.com/issues/58126?issue_count=405&issue_position=3&next_issue_id=58082&prev_issue_id=58564#note-5.
This is another story, if the unmounting finishes just before the acks
come they will be dropped directly, if the acks come faster while the
unmouting is not finished yet it will update the inodes and then the
inodes will be evicted. So the acks makes no sense any more and this is
why I need this patch to drop the ack replies.
Thanks
--
Best Regards,
Xiubo Li (李秀波)
Email: xiubli@xxxxxxxxxx/xiubli@xxxxxxx
Slack: @Xiubo Li
