Messages in this thread | | | Date | Mon, 13 Jun 2022 15:20:53 -0500 | From | Tyler Hicks <> | Subject | Re: [PATCH 00/06] fid refcounting improvements and fixes |
| |
On 2022-06-12 17:53:23, Dominique Martinet wrote: > So: > - I could reproduce Tyler's generic 531 leak, fixed it by the first > commit in v9fs_vfs_atomic_open_dotl > - Found another less likely leak while reworking code > - Christian's comment that it's not obvious that clunk is just a > refcount decrease was very true: I think it's worth the churn, > so I've rename this all through a new helper... > - ... with the not-so-hidden intent to improve debugging by adding > a tracepoint for them, which I have also done. > > I've also taken my comment in the other thread further and went ahead > and made it in its own commit. Tyler, if you're ok with this I'll just > squash it up. You can see rebased patches here: > https://github.com/martinetd/linux/ > > Note that I also took the permission to add back '/* clone */' as a > comment to your changing p9_client_walk's arguments: while I can agree > variables are hard to track, figuring out what the heck that boolean > argument means is much harder to me and I honestly preferred the > variable. > Having both through a comment is good enough for me if you'll accept > this: > ---- > @@ -222,7 +221,8 @@ static struct p9_fid *v9fs_fid_lookup_with_uid(struct dentry *dentry, > * We need to hold rename lock when doing a multipath > * walk to ensure none of the patch component change > */ > - fid = p9_client_walk(old_fid, l, &wnames[i], clone); > + fid = p9_client_walk(old_fid, l, &wnames[i], > + old_fid == root_fid /* clone */);
This is no problem at all. The rebased patches look good to me. Squash your fix to my fix and it should be ready to go.
Thanks again!
Tyler
> /* non-cloning walk will return the same fid */ > if (fid != old_fid) { > p9_client_clunk(old_fid); > ---- > > > The last commit is just cleanups and should be no real change. > > Dominique Martinet (6): > 9p: fix fid refcount leak in v9fs_vfs_atomic_open_dotl > 9p: fix fid refcount leak in v9fs_vfs_get_link > 9p: v9fs_fid_lookup_with_uid fix's fix suggestion > 9p fid refcount: add p9_fid_get/put wrappers > 9p fid refcount: add a 9p_fid_ref tracepoint > 9p fid refcount: cleanup p9_fid_put calls > > > (diff stats include Tyler's commits) > > fs/9p/fid.c | 71 +++++++++++++++------------- > fs/9p/fid.h | 6 +-- > fs/9p/vfs_addr.c | 4 +- > fs/9p/vfs_dentry.c | 4 +- > fs/9p/vfs_dir.c | 2 +- > fs/9p/vfs_file.c | 9 ++-- > fs/9p/vfs_inode.c | 97 +++++++++++++++++---------------------- > fs/9p/vfs_inode_dotl.c | 79 ++++++++++++------------------- > fs/9p/vfs_super.c | 8 ++-- > fs/9p/xattr.c | 8 ++-- > include/net/9p/client.h | 3 ++ > include/trace/events/9p.h | 48 +++++++++++++++++++ > net/9p/client.c | 42 +++++++++++------ > 13 files changed, 211 insertions(+), 170 deletions(-) > > -- > 2.35.1 >
| |