Skip to content

mds: use available CInode* for uninline data#62684

Merged
vshankar merged 1 commit intoceph:mainfrom
mchangir:mds-use-already-avaialble-CInode-in-uninline-path
May 21, 2025
Merged

mds: use available CInode* for uninline data#62684
vshankar merged 1 commit intoceph:mainfrom
mchangir:mds-use-already-avaialble-CInode-in-uninline-path

Conversation

@mchangir
Copy link
Contributor

@mchangir mchangir commented Apr 5, 2025

Fixes: https://tracker.ceph.com/issues/70624
Signed-off-by: Milind Changire mchangir@redhat.com

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)
Show available Jenkins commands

@github-actions github-actions bot added the cephfs Ceph File System label Apr 5, 2025
@mchangir mchangir marked this pull request as draft April 5, 2025 15:56
@mchangir mchangir force-pushed the mds-use-already-avaialble-CInode-in-uninline-path branch from dbcb9db to 2845109 Compare April 6, 2025 04:07
@mchangir mchangir marked this pull request as ready for review April 6, 2025 04:08
@mchangir mchangir mentioned this pull request Apr 8, 2025
14 tasks
@mchangir mchangir force-pushed the mds-use-already-avaialble-CInode-in-uninline-path branch from 2845109 to 0f67a35 Compare April 9, 2025 08:01
@vshankar
Copy link
Contributor

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

Copy link
Contributor

@vshankar vshankar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vshankar
Copy link
Contributor

jenkins retest this please

1 similar comment
@vshankar
Copy link
Contributor

jenkins retest this please

@vshankar
Copy link
Contributor

jenkins test make check

@vshankar
Copy link
Contributor

vshankar commented May 6, 2025

jenkins retest this please

2 similar comments
@vshankar
Copy link
Contributor

vshankar commented May 7, 2025

jenkins retest this please

@vshankar
Copy link
Contributor

jenkins retest this please

@vshankar
Copy link
Contributor

jenkins test docs

3 similar comments
@vshankar
Copy link
Contributor

jenkins test docs

@vshankar
Copy link
Contributor

jenkins test docs

@vshankar
Copy link
Contributor

jenkins test docs

@vshankar
Copy link
Contributor

Has "jenkins test docs" stopped auto-running the docs jenkins test?

@vshankar
Copy link
Contributor

jenkins test docs

@vshankar
Copy link
Contributor

@mchangir mind pushing a rebase please? I cannot get the docs test to even run for retesting.

This is a null pointer dereference issue and it happens as follows:

Uninline Data is not a regular client request ... it is an Internal Request.
So, there's no client request struct allocated and assigned in the mdr to
begin with.

In the scrubbing path, the auth validation is already done in
ScrubStack::kick_off_scrubs() ...  and since Uninline Data path piggybacks
on the scrubbing path, we get the auth validation for free.

rdlock_path_pin_ref(), fails to lock the path if the lock is already taken.
This is what happens in the Uninline Data case. So rdlock_path_pin_ref()
creates a C_MDS_RetryRequest and this causes the request to be re-attempted
in the regular client request path where Server::handle_client_request()
assumes that the mdr->client_request member is valid ...
and hence the null pointer dereference issue.
---
Since the scrub path dequeues the CInode* from the ScrubStack, this
commit attempts to use the already available CInode*.

Fixes: https://tracker.ceph.com/issues/70624
Signed-off-by: Milind Changire <mchangir@redhat.com>
@mchangir mchangir force-pushed the mds-use-already-avaialble-CInode-in-uninline-path branch from 0f67a35 to 0ea8570 Compare May 14, 2025 16:04
@mchangir
Copy link
Contributor Author

trivial rebase

@vshankar
Copy link
Contributor

jenkins test make check arm64

@vshankar vshankar merged commit b4c65dc into ceph:main May 21, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants