Messages in this thread | | | From | Miklos Szeredi <> | Subject | Re: processes hung after sys_renameat, and 'missing' processes | Date | Mon, 11 Jun 2012 18:02:53 +0200 |
| |
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Thu, Jun 7, 2012 at 12:07 AM, Miklos Szeredi <miklos@szeredi.hu> wrote: >> >> Yeah, see the comment in below patch for how it's supposed to work. I >> *think* it's correct. > > Ok, yes, that makes me happier. > > What would make me happier still is to get rid of the "save_parent" > thing entirely, though. > > And I think you should be able to. > > Why? > > You already have the rule that: > - save_parent.mnt is always same as "path->mnt" (or NULL, if not saved) > - save_parent.dentry is the same as "dir" when you use it (you have a > BUG_ON() for it not being the same) > - you currently use "save_parent.dentry != NULL" as a flag to say "do > we have the valid state" > > So as far as I can tell, you should get rid of all the refcount games > and the "struct path save_parent", and just replace the > "save_parent.dentry != NULL" thing with a flag of whether you have a > valid state. > > That would get rid of the whole > > if ((nd->flags & LOOKUP_RCU) || nd->path.mnt != path->mnt) { > path_to_nameidata(path, nd); > } else { > save_parent.dentry = nd->path.dentry; > save_parent.mnt = mntget(path->mnt); > nd->path.dentry = path->dentry; > > } > > thing, and we could just have the old simple unconditional > > path_to_nameidata(path, nd); > > back. > > And then you'd have > > if (filp == ERR_PTR(-EOPENSTALE) && save_parent_flag && !retried) { > dput(nd->path.dentry); > nd->path.dentry = dget(dir);
But 'dir' may no longer be valid here since we dput it in path_to_nameidata() earlier.
So, unfortunately, we do need to play those refcounting games.
The mntget() could be optimized away in theory, but it's tricky because complete_walk() might do a path_put(nd->path) on failure and then we'd be left with a dentry ref without a matching vfsmount ref.
Thanks, Miklos -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |