lock: dump filepath name for rwsem waiters#204
lock: dump filepath name for rwsem waiters#204richl9 wants to merge 1 commit intooracle-samples:mainfrom
Conversation
Orabug: 39017612 Signed-off-by: Richard Li <tianqi.li@oracle.com>
brenns10
left a comment
There was a problem hiding this comment.
This looks good, though you don't need to scan the stack for the nameidata - details below. More importantly, can you share a bit about the motivation for this? Locking and filesystems aren't inherently related, so displaying the filename currently being looked up seems a bit out of place.
I could see it being helpful in some cases for sure, and I'm definitely not opposed to including it to help those cases. Just wanted to hear the background (and maybe include that in the commit message?)
| if filp_frame and "nd" in filp_frame.locals(): | ||
| nameidata = filp_frame["nd"] | ||
| pathname = escape_ascii_string(nameidata.name.name.string_()) |
There was a problem hiding this comment.
You're in luck -- check the field task_struct.nameidata and the functions set_nameidata() and restore_nameidata() in fs/namei.c. It's a pointer to the nameidata for the currently in-progress filename lookup. You can avoid the stack trace altogether.
I would also note that do_filp_open() has been renamed to do_file_open() since commit torvalds/linux@541003b576c3e ("rename do_filp_open() to do_file_open()"). If you want to only display the in-lookup filename for cases where a file is being opened, then you'll need to check for that name as well.
There was a problem hiding this comment.
That sounds nice! Let me look into that.
|
Oh, also please be sure to push this branch to a branch named |
|
wondering whether we should make a separate kernfs lock contention module, there you can list the process, semaphore status, and kernfs file by using the api provided by lock module, and that will leave lock module for generic lock? |
|
Or maybe what we really want is a way for different drgn-tools subsystems to detect and register the locks they care about, so any subsystem can provide context about the lock. |
|
Yeah, either way makes sense to me. We need to refactor the code a bit to make things loosely coupled. |
No description provided.