libcephfs/client: pin inode/dentry for an opened directory by ifed01 · Pull Request #60909 · ceph/ceph

ifed01 · 2024-12-02T16:48:12Z

Reproduces: https://tracker.ceph.com/issues/69092
Fixes: https://tracker.ceph.com/issues/69092

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows
jenkins test rook e2e

mchangir

minor issue

src/test/libcephfs/client_cache.cc

vshankar · 2024-12-27T10:07:50Z

jenkins test windows

vshankar · 2024-12-27T10:08:02Z

jenkins test make check

vshankar · 2024-12-27T16:25:33Z

This PR is under test in https://tracker.ceph.com/issues/69375.

ifed01 · 2024-12-27T17:19:18Z

jenkins test windows

patrakov · 2024-12-30T12:17:57Z

src/client/Client.cc

+    if (fhp) {
      *fhp = _create_fh(in, flags, cmode, perms);
+      // ceph_flags_sys2wire/ceph_flags_to_mode() calls above transforms O_DIRECTORY flag
+      // into CEPH_FILE_MODE_PIN mode. Although this mode is used at server size


typo: size -> side

vshankar

https://tracker.ceph.com/projects/cephfs/wiki/Main#wip-vshankar-testing-20241227162501-debug

vshankar · 2025-01-02T06:47:37Z

jenkins test windows

vshankar · 2025-01-02T06:53:02Z

jenkins test windows

vshankar · 2025-01-06T04:33:27Z

jenkins test windows

vshankar · 2025-01-06T07:01:49Z

Windows failure seems to be real

++ echo 'src/client/Client.cc
src/test/libcephfs/CMakeLists.txt
src/test/libcephfs/client_cache.cc'
++ egrep -v '^container/'
++ wc -l
+ '[' 3 -gt 0 ']'
+ return 1

I haven't looked deeper though. @ifed01

vshankar · 2025-01-07T10:36:57Z

Windows failure seems to be real

++ echo 'src/client/Client.cc
src/test/libcephfs/CMakeLists.txt
src/test/libcephfs/client_cache.cc'
++ egrep -v '^container/'
++ wc -l
+ '[' 3 -gt 0 ']'
+ return 1

I haven't looked deeper though. @ifed01

This is happening with other PRs too, so its probably unrelated to this change (however, it blocks merging).

ifed01 · 2025-01-09T10:14:57Z

jenkins test windows

Signed-off-by: Igor Fedotov <igor.fedotov@croit.io>

ifed01 · 2025-01-27T09:29:19Z

jenkins test make check

ifed01 · 2025-01-27T13:05:38Z

jenkins test api

ifed01 · 2025-01-27T13:05:54Z

jenkins test make check

ifed01 · 2025-01-27T15:58:21Z

jenkins test make check

ifed01 · 2025-01-28T06:34:14Z

jenkins test windows

vshankar · 2025-02-04T07:52:52Z

jenkins retest this please

ifed01 · 2025-02-05T11:27:01Z

The root cause for reoccurring Windows tests failures is lack of O_DIRECTORY flag translation in ceph_flags_sys2wire() (defined in src/common/ceph_fs.cc).
This function explicitly bypasses this translation under Windows:

#ifndef _WIN32
       ceph_sys2wire(O_DIRECTORY);
       ceph_sys2wire(O_NOFOLLOW);
       // In some cases, FILE_FLAG_BACKUP_SEMANTICS may be used instead
       // of O_DIRECTORY. We may need some workarounds in order to handle
       // the fact that those flags are not available on Windows.
#endif

As a result no CEPH_FILE_MODE_PIN is returned by ceph_flags_to_mode() and opened directory gets no pinning. Which in turn exposes it to cache trimming..
@petrutlucian94 - do you think we really need that "ifndef" block given the fact that O_DIRECTORY is actually defined in include/win32/fs_compat.h?

petrutlucian94 · 2025-02-05T11:40:47Z

As long as those flags don't propagate to Windows functions, I think we can remove the ifdefs.

…er Win in ceph_flags_sys2wire() Signed-off-by: Igor Fedotov <igor.fedotov@croit.io>

ifed01 · 2025-02-06T11:17:11Z

jenkins test make check

ifed01 · 2025-02-09T11:50:49Z

jenkins test make check

ifed01 · 2025-02-09T21:10:24Z

jenkins test make check

vshankar · 2025-02-10T13:05:33Z

Nice work @ifed01. Thx @petrutlucian94

ifed01 added the tests label Dec 2, 2024

github-actions bot added build/ops cephfs Ceph File System labels Dec 2, 2024

ifed01 force-pushed the wip-ifed-client-cache-trim-repro branch from 7c63a08 to 4f122a0 Compare December 2, 2024 16:54

ifed01 requested review from joscollin and vshankar December 2, 2024 16:55

ifed01 changed the title ~~test/libcephfs: reproduce an issue after client cache trimming~~ test/libcephfs: reproduce internal data inconsistency after client cache trimming Dec 2, 2024

ifed01 force-pushed the wip-ifed-client-cache-trim-repro branch 2 times, most recently from ebb8b23 to cff1f39 Compare December 3, 2024 13:03

ifed01 changed the title ~~test/libcephfs: reproduce internal data inconsistency after client cache trimming~~ libcephfs/client: pin inode/dentry for an opened directory Dec 12, 2024

batrick added the needs-review label Dec 18, 2024

batrick requested review from a team and dparmar18 December 18, 2024 14:39

vshankar self-assigned this Dec 18, 2024

dparmar18 self-assigned this Dec 19, 2024

mchangir reviewed Dec 23, 2024

View reviewed changes

src/test/libcephfs/client_cache.cc Outdated Show resolved Hide resolved

ifed01 force-pushed the wip-ifed-client-cache-trim-repro branch from 1573d65 to 78433c2 Compare December 26, 2024 12:00

vshankar added the wip-vshankar-testing1 label Dec 27, 2024

patrakov reviewed Dec 30, 2024

View reviewed changes

vshankar approved these changes Jan 2, 2025

View reviewed changes

test/libcephfs: remove warning in Windows build

c9d7345

Signed-off-by: Igor Fedotov <igor.fedotov@croit.io>

ifed01 force-pushed the wip-ifed-client-cache-trim-repro branch from 4bd086d to c9d7345 Compare January 27, 2025 08:09

ifed01 force-pushed the wip-ifed-client-cache-trim-repro branch 2 times, most recently from 9fdfbec to be97718 Compare January 28, 2025 04:29

ifed01 force-pushed the wip-ifed-client-cache-trim-repro branch 2 times, most recently from bf96ced to aa2d946 Compare February 5, 2025 10:52

github-actions bot added the common label Feb 5, 2025

ifed01 force-pushed the wip-ifed-client-cache-trim-repro branch from aa2d946 to 51e4e1a Compare February 5, 2025 10:53

common/ceph_fs: Enable O_DIRECTORY|O_NOFOLLOWUP flags translation und…

4badc83

…er Win in ceph_flags_sys2wire() Signed-off-by: Igor Fedotov <igor.fedotov@croit.io>

ifed01 force-pushed the wip-ifed-client-cache-trim-repro branch from 51e4e1a to 4badc83 Compare February 5, 2025 15:41

ifed01 added the win32 Specifix changes for the windows platform label Feb 5, 2025

vshankar merged commit cb5a62f into ceph:main Feb 11, 2025
12 checks passed

vshankar removed the wip-vshankar-testing1 label Feb 11, 2025

ifed01 deleted the wip-ifed-client-cache-trim-repro branch February 11, 2025 12:51

joscollin mentioned this pull request Oct 3, 2025

reef: libcephfs client cache trimming might remove dentries attached to opened dir inode #65766

Closed

14 tasks

Conversation

ifed01 commented Dec 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

mchangir left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vshankar commented Dec 27, 2024

Uh oh!

vshankar commented Dec 27, 2024

Uh oh!

vshankar commented Dec 27, 2024

Uh oh!

ifed01 commented Dec 27, 2024

Uh oh!

patrakov Dec 30, 2024

Choose a reason for hiding this comment

Uh oh!

vshankar left a comment

Choose a reason for hiding this comment

Uh oh!

vshankar commented Jan 2, 2025

Uh oh!

vshankar commented Jan 2, 2025

Uh oh!

vshankar commented Jan 6, 2025

Uh oh!

vshankar commented Jan 6, 2025

Uh oh!

vshankar commented Jan 7, 2025

Uh oh!

ifed01 commented Jan 9, 2025

Uh oh!

ifed01 commented Jan 27, 2025

Uh oh!

ifed01 commented Jan 27, 2025

Uh oh!

ifed01 commented Jan 27, 2025

Uh oh!

ifed01 commented Jan 27, 2025

Uh oh!

ifed01 commented Jan 28, 2025

Uh oh!

vshankar commented Feb 4, 2025

Uh oh!

ifed01 commented Feb 5, 2025

Uh oh!

petrutlucian94 commented Feb 5, 2025

Uh oh!

ifed01 commented Feb 6, 2025

Uh oh!

ifed01 commented Feb 9, 2025

Uh oh!

ifed01 commented Feb 9, 2025

Uh oh!

vshankar commented Feb 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ifed01 commented Dec 2, 2024 •

edited

Loading