Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

lsfd: Support pidfs #2866

Merged
merged 4 commits into from
Apr 8, 2024
Merged

lsfd: Support pidfs #2866

merged 4 commits into from
Apr 8, 2024

Conversation

xry111
Copy link
Contributor

@xry111 xry111 commented Mar 27, 2024

In Linux 6.9 pidfds are moved from the anonymous inode infrastructure to a tiny pseudo filesystem named pidfs. Recognize it properly.

Fixes #2865.

@xry111 xry111 force-pushed the xry111/pidfs branch 2 times, most recently from ae24222 to 6e05a45 Compare March 27, 2024 20:14
t-8ch
t-8ch reviewed Mar 30, 2024
View reviewed changes
misc-utils/lsfd-unkn.c Outdated Show resolved Hide resolved
@xry111
Copy link
Contributor Author

xry111 commented Mar 30, 2024

Oops, pidfd-utils.h fails to compile on Ubuntu 18.04.

Maybe I need to include something before it?

@xry111 xry111 marked this pull request as draft March 30, 2024 15:40
@xry111 xry111 marked this pull request as ready for review March 30, 2024 16:30
@xry111
Copy link
Contributor Author

xry111 commented Mar 30, 2024

Oops, pidfd-utils.h fails to compile on Ubuntu 18.04.

Maybe I need to include something before it?

Yes, it needs <unistd.h>.

@xry111 xry111 requested a review from t-8ch March 30, 2024 16:31
t-8ch
t-8ch reviewed Mar 30, 2024
View reviewed changes
misc-utils/lsfd-unkn.c Outdated Show resolved Hide resolved
@xry111
include: Include <unistd.h> in pidfd-utils.h for syscall()
10add32 
In Glibc, <sys/syscall.h> only contains SYS_* macros and the syscall()
function is in <unistd.h>.  So include it.

Signed-off-by: Xi Ruoyao <xry111@xry111.site>
@masatake
Copy link
Member

Though pidfs is introduced, lsfd still reports the SOURCE for the fd opening a file on the file system as "anon_inodefs". It should be "pidfs".

The solution may be more complicated.
We must add code to lsfd-file. The code deals with fds opening files on pidfs.
We may have to define pidfs_file_class. The definition of nsfs_file_class can be a good starting point.

The original code deals with fds opening files on anon_inodefs.
We may have to refactor the original code. Some code, like the definition of struct anon_pidfd_data, can be shared between the code for pidfs and anon_inodefs.

@xry111
Copy link
Contributor Author

xry111 commented Mar 30, 2024

Though pidfs is introduced, lsfd still reports the SOURCE for the fd opening a file on the file system as "anon_inodefs". It should be "pidfs".

The solution may be more complicated. We must add code to lsfd-file. The code deals with fds opening files on pidfs. We may have to define pidfs_file_class. The definition of nsfs_file_class can be a good starting point.

The original code deals with fds opening files on anon_inodefs. We may have to refactor the original code. Some code, like the definition of struct anon_pidfd_data, can be shared between the code for pidfs and anon_inodefs.

Yes, I've considered that but to me it needs more time (maybe days) to be designed and implemented...

So turning this into a draft for now.

@xry111 xry111 marked this pull request as draft March 30, 2024 17:35
@karelzak karelzak added the LSFD lsfd and mkfd tests related label Apr 2, 2024
@xry111
Copy link
Contributor Author

xry111 commented Apr 3, 2024

Oops, forgotten meson.

@xry111 xry111 marked this pull request as ready for review April 3, 2024 08:50
@xry111
Copy link
Contributor Author

xry111 commented Apr 3, 2024

Though pidfs is introduced, lsfd still reports the SOURCE for the fd opening a file on the file system as "anon_inodefs". It should be "pidfs".
The solution may be more complicated. We must add code to lsfd-file. The code deals with fds opening files on pidfs. We may have to define pidfs_file_class. The definition of nsfs_file_class can be a good starting point.
The original code deals with fds opening files on anon_inodefs. We may have to refactor the original code. Some code, like the definition of struct anon_pidfd_data, can be shared between the code for pidfs and anon_inodefs.

Yes, I've considered that but to me it needs more time (maybe days) to be designed and implemented...

So turning this into a draft for now.

Done. Tested on kernel 6.8.2 and 6.9-rc2.

@xry111
lsfd: Refactor the pidfd logic into lsfd-pidfd.c
bf6645d 
We'll reuse these logic for pidfd support on Linux >= 6.9.  This should
be a no-functional change.

Besides moving the code, this change also renames anon_pidfd_data to
pidfd_data, and removes a redundant nullity check for free (because
free(NULL) will just do nothing per the C standard).

Signed-off-by: Xi Ruoyao <xry111@xry111.site>
@xry111
lsfd: Support pidfs
b1a48ef 
In Linux 6.9 pidfds are moved from the anonymous inode infrastructure to
a tiny pseudo filesystem named pidfs.  Recognize it properly.

Fixes util-linux#2865.

Signed-off-by: Xi Ruoyao <xry111@xry111.site>
@xry111
lsfd: test: Adapt test cases for pidfs
04db2ba 
On Linux >= 6.9, pidfds are from pidfs instead of anonymous inode.
Thus:

STTYPE is REG on Linux >= 6.9, UNKN on Linux < 6.9.

KNAME is pidfd:[inode number] on Linux >= 6.9, anon_inode:[pidfd] on
Linux < 6.9.

And ainode_class test cannot work on Linux >= 6.9, just skip this sub
test if STTYPE is REG.

Signed-off-by: Xi Ruoyao <xry111@xry111.site>
@xry111
Copy link
Contributor Author

xry111 commented Apr 3, 2024

Had to do another push to fix make distcheck.

@karelzak
Copy link
Collaborator

karelzak commented Apr 3, 2024

@masatake, what about moving misc-utils/lsfd* files to separate lsfd/ directory (and removing the lsfd- prefix from the filenames) during work on v2.41? It's already large enough, and it seems it will grow in the future ;-)

@masatake
Copy link
Member

masatake commented Apr 3, 2024

@karelzak I see. After finishing in-flight pull requests, I will make a pull request for introducing lsfd/ directory.

@karelzak karelzak merged commit 44b92ef into util-linux:master Apr 8, 2024
32 checks passed
@jirislaby jirislaby mentioned this pull request May 17, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t-8ch t-8ch t-8ch left review comments

@masatake masatake masatake approved these changes

Assignees
No one assigned
Labels
LSFD lsfd and mkfd tests related
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

lsfd: Fail to get PIDFD.PID with mainline kernel
4 participants
@xry111 @masatake @karelzak @t-8ch