mds: inode_t flags may not be protected by the policylock during set_vxattr by batrick · Pull Request #56934 · ceph/ceph

batrick · 2024-04-16T20:03:38Z

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 · 2024-04-17T01:54:11Z

I wonder if this was the cause of missing ceph.dir.subvolume in some upstream and downstream cases.

batrick · 2024-04-17T02:11:53Z

src/mds/MDSDaemon.cc

                                     "show cache status");
  ceph_assert(r == 0);
-  r = admin_socket->register_command("quiesce path"
+  r = admin_socket->register_command("lock path"


whoops! Didn't mean to do this.

batrick · 2024-04-17T02:14:46Z

I wonder if this was the cause of missing ceph.dir.subvolume in some upstream and downstream cases.

The inode still gets projected but ... maybe?

batrick · 2024-04-17T02:15:14Z

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

batrick · 2024-04-17T12:42:40Z

jenkins test api

leonid-s-usov

Wow, that's a good catch!

Please update the name of this PR because it's not about replication but locking.

On a separate note: love the lock path admin command, very much needed!

src/mds/MDSRank.cc

batrick · 2024-04-18T00:45:45Z

jenkins test api

batrick · 2024-04-18T00:46:54Z

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

batrick · 2024-04-20T18:07:55Z

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

batrick · 2024-04-20T19:31:47Z

jenkins test make check

batrick · 2024-04-22T18:57:55Z

https://tracker.ceph.com/projects/cephfs/wiki/Main#2024-04-20

or other uses. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

So we can have a long-running lock on an inode for certain tests. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

When the policy xlock cannot be immediately acquired. Test-case: https://tracker.ceph.com/issues/65496 Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

The code logic had a serious defect that it would only execute xlock_policylock once such that a retry would then proceed to executing the vxattr setting. Fixes: https://tracker.ceph.com/issues/65496 Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

batrick · 2024-04-22T19:02:06Z

qa/suites/fs/volumes/tasks/volumes/test/basic.yaml

        - tasks.cephfs.test_volumes.TestSubvolumeGroups
        - tasks.cephfs.test_volumes.TestSubvolumes
-        - tasks.cephfs.test_subvolume.TestSubvolume
+        - tasks.cephfs.test_subvolume


Needs rerun because this was missing.

batrick · 2024-04-24T13:45:17Z

jenkins test windows

batrick · 2024-04-25T01:35:45Z

jenkins test make check arm64

batrick · 2024-04-25T01:45:31Z

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

* refs/pull/56934/head: mds: move drop_locks to directly after rdonly check qa: test quiesce.block is replicated qa: test that ceph.dir.subvolume is replicated properly mds: add debug "lock path" command qa: move reqid_tostr helper qa: return run_shell process for waiters

batrick · 2024-04-29T21:01:07Z

jenkins test make check arm64

batrick · 2024-04-29T21:09:30Z

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

batrick · 2024-04-30T16:16:18Z

https://tracker.ceph.com/issues/65694#note-2

batrick added cephfs Ceph File System needs-review labels Apr 16, 2024

batrick requested a review from a team April 16, 2024 20:03

github-actions bot added the tests label Apr 16, 2024

batrick mentioned this pull request Apr 16, 2024

mds: regular file inode flags are not replicated by the policylock #56935

Merged

4 tasks

batrick added the wip-pdonnell-testing label Apr 16, 2024

batrick force-pushed the i65496 branch from ced0e28 to 40901ca Compare April 16, 2024 23:16

batrick commented Apr 17, 2024

View reviewed changes

batrick force-pushed the i65496 branch from 40901ca to cf268bb Compare April 17, 2024 02:14

leonid-s-usov approved these changes Apr 17, 2024

View reviewed changes

src/mds/MDSRank.cc Outdated Show resolved Hide resolved

batrick changed the title ~~mds: ceph.dir.subvolume and ceph.quiesce.blocked is not properly replicated~~ mds: ceph.dir.subvolume and ceph.quiesce.blocked may not be properly locked prior to req execution Apr 17, 2024

batrick force-pushed the i65496 branch from cf268bb to 52b2455 Compare April 17, 2024 16:24

leonid-s-usov changed the title ~~mds: ceph.dir.subvolume and ceph.quiesce.blocked may not be properly locked prior to req execution~~ mds: inode_t flags may not be protected by the policylock during set_vxattr Apr 18, 2024

batrick force-pushed the i65496 branch from 52b2455 to 3e478bb Compare April 18, 2024 14:57

batrick added 6 commits April 22, 2024 15:01

qa: return run_shell process for waiters

2c2782b

or other uses. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

qa: move reqid_tostr helper

0c8d601

Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

mds: add debug "lock path" command

7f1d066

So we can have a long-running lock on an inode for certain tests. Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

qa: test that ceph.dir.subvolume is replicated properly

8a0eceb

Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

qa: test quiesce.block is replicated

5370ede

When the policy xlock cannot be immediately acquired. Test-case: https://tracker.ceph.com/issues/65496 Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

batrick force-pushed the i65496 branch from 3e478bb to 2931860 Compare April 22, 2024 19:01

batrick commented Apr 22, 2024

View reviewed changes

batrick added wip-pdonnell-testing2 and removed wip-pdonnell-testing labels Apr 22, 2024

batrick merged commit d4de8dc into ceph:main Apr 30, 2024

batrick deleted the i65496 branch April 30, 2024 16:19

Conversation

batrick commented Apr 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

mchangir commented Apr 17, 2024

Uh oh!

batrick Apr 17, 2024

Choose a reason for hiding this comment

Uh oh!

batrick commented Apr 17, 2024

Uh oh!

batrick commented Apr 17, 2024

Uh oh!

batrick commented Apr 17, 2024

Uh oh!

leonid-s-usov left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

batrick commented Apr 18, 2024

Uh oh!

batrick commented Apr 18, 2024

Uh oh!

batrick commented Apr 20, 2024

Uh oh!

batrick commented Apr 20, 2024

Uh oh!

batrick commented Apr 22, 2024

Uh oh!

batrick Apr 22, 2024

Choose a reason for hiding this comment

Uh oh!

batrick commented Apr 24, 2024

Uh oh!

batrick commented Apr 25, 2024

Uh oh!

batrick commented Apr 25, 2024

Uh oh!

batrick commented Apr 29, 2024

Uh oh!

batrick commented Apr 29, 2024

Uh oh!

batrick commented Apr 30, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

batrick commented Apr 16, 2024 •

edited

Loading

leonid-s-usov left a comment •

edited

Loading