* [PATCH 1/6] vfs: fix d_need_lookup/d_revalidate order in do_lookup
2012-03-26 10:54 [PATCH 0/6] vfs: path lookup fixes and cleanups Miklos Szeredi
@ 2012-03-26 10:54 ` Miklos Szeredi
2012-03-26 10:54 ` [PATCH 2/6] vfs: don't revalidate just looked up dentry Miklos Szeredi
` (4 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Miklos Szeredi @ 2012-03-26 10:54 UTC (permalink / raw)
To: viro; +Cc: linux-fsdevel, linux-kernel, hch, mszeredi
From: Miklos Szeredi <mszeredi@suse.cz>
Doing revalidate on a dentry which has not yet been looked up makes no sense.
Move the d_need_lookup() check before d_revalidate().
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
fs/namei.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 46ea9cc..599e4a0 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1139,6 +1139,8 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
return -ECHILD;
nd->seq = seq;
+ if (unlikely(d_need_lookup(dentry)))
+ goto unlazy;
if (unlikely(dentry->d_flags & DCACHE_OP_REVALIDATE)) {
status = d_revalidate(dentry, nd);
if (unlikely(status <= 0)) {
@@ -1147,8 +1149,6 @@ static int do_lookup(struct nameidata *nd, struct qstr *name,
goto unlazy;
}
}
- if (unlikely(d_need_lookup(dentry)))
- goto unlazy;
path->mnt = mnt;
path->dentry = dentry;
if (unlikely(!__follow_mount_rcu(nd, path, inode)))
--
1.7.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/6] vfs: don't revalidate just looked up dentry
2012-03-26 10:54 [PATCH 0/6] vfs: path lookup fixes and cleanups Miklos Szeredi
2012-03-26 10:54 ` [PATCH 1/6] vfs: fix d_need_lookup/d_revalidate order in do_lookup Miklos Szeredi
@ 2012-03-26 10:54 ` Miklos Szeredi
2012-03-26 10:54 ` [PATCH 3/6] vfs: move MAY_EXEC check from __lookup_hash() Miklos Szeredi
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Miklos Szeredi @ 2012-03-26 10:54 UTC (permalink / raw)
To: viro; +Cc: linux-fsdevel, linux-kernel, hch, mszeredi
From: Miklos Szeredi <mszeredi@suse.cz>
__lookup_hash() calls ->lookup() if the dentry needs lookup and on success
revalidates the dentry (all under dir->i_mutex).
While this is harmless it doesn't make a lot of sense.
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
fs/namei.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 599e4a0..c93ea6d 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1746,9 +1746,7 @@ static struct dentry *__lookup_hash(struct qstr *name,
* __lookup_hash is called with the parent dir's i_mutex already
* held, so we are good to go here.
*/
- dentry = d_inode_lookup(base, dentry, nd);
- if (IS_ERR(dentry))
- return dentry;
+ return d_inode_lookup(base, dentry, nd);
}
if (dentry && (dentry->d_flags & DCACHE_OP_REVALIDATE)) {
--
1.7.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/6] vfs: move MAY_EXEC check from __lookup_hash()
2012-03-26 10:54 [PATCH 0/6] vfs: path lookup fixes and cleanups Miklos Szeredi
2012-03-26 10:54 ` [PATCH 1/6] vfs: fix d_need_lookup/d_revalidate order in do_lookup Miklos Szeredi
2012-03-26 10:54 ` [PATCH 2/6] vfs: don't revalidate just looked up dentry Miklos Szeredi
@ 2012-03-26 10:54 ` Miklos Szeredi
2012-03-26 10:54 ` [PATCH 4/6] vfs: set LOOKUP_JUMPED in follow_managed Miklos Szeredi
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Miklos Szeredi @ 2012-03-26 10:54 UTC (permalink / raw)
To: viro; +Cc: linux-fsdevel, linux-kernel, hch, mszeredi
From: Miklos Szeredi <mszeredi@suse.cz>
The only caller of __lookup_hash() that needs the exec permission check on
parent is lookup_one_len().
All lookup_hash() callers already checked permission in LOOKUP_PARENT walk.
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
fs/namei.c | 11 +++++------
1 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index c93ea6d..3e13412 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1726,13 +1726,7 @@ int vfs_path_lookup(struct dentry *dentry, struct vfsmount *mnt,
static struct dentry *__lookup_hash(struct qstr *name,
struct dentry *base, struct nameidata *nd)
{
- struct inode *inode = base->d_inode;
struct dentry *dentry;
- int err;
-
- err = inode_permission(inode, MAY_EXEC);
- if (err)
- return ERR_PTR(err);
/*
* Don't bother with __d_lookup: callers are for creat as
@@ -1799,6 +1793,7 @@ struct dentry *lookup_one_len(const char *name, struct dentry *base, int len)
{
struct qstr this;
unsigned int c;
+ int err;
WARN_ON_ONCE(!mutex_is_locked(&base->d_inode->i_mutex));
@@ -1823,6 +1818,10 @@ struct dentry *lookup_one_len(const char *name, struct dentry *base, int len)
return ERR_PTR(err);
}
+ err = inode_permission(base->d_inode, MAY_EXEC);
+ if (err)
+ return ERR_PTR(err);
+
return __lookup_hash(&this, base, NULL);
}
--
1.7.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/6] vfs: set LOOKUP_JUMPED in follow_managed
2012-03-26 10:54 [PATCH 0/6] vfs: path lookup fixes and cleanups Miklos Szeredi
` (2 preceding siblings ...)
2012-03-26 10:54 ` [PATCH 3/6] vfs: move MAY_EXEC check from __lookup_hash() Miklos Szeredi
@ 2012-03-26 10:54 ` Miklos Szeredi
2012-03-30 2:39 ` Al Viro
2012-03-26 10:54 ` [PATCH 5/6] vfs: reorganize do_lookup Miklos Szeredi
2012-03-26 10:54 ` [PATCH 6/6] vfs: split __lookup_hash Miklos Szeredi
5 siblings, 1 reply; 12+ messages in thread
From: Miklos Szeredi @ 2012-03-26 10:54 UTC (permalink / raw)
To: viro; +Cc: linux-fsdevel, linux-kernel, hch, mszeredi
From: Miklos Szeredi <mszeredi@suse.cz>
This is cleaner than if callers need to do it.
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
fs/namei.c | 27 +++++++++++++--------------
1 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 3e13412..482992e 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -794,8 +794,10 @@ static int follow_automount(struct path *path, unsigned flags,
* This may only be called in refwalk mode.
*
* Serialization is taken care of in namespace.c
+ *
+ * If a mount was followed, set LOOKUP_JUMPED in nd->flags
*/
-static int follow_managed(struct path *path, unsigned flags)
+static int follow_managed(struct path *path, struct nameidata *nd)
{
struct vfsmount *mnt = path->mnt; /* held by caller, must be left alone */
unsigned managed;
@@ -839,7 +841,7 @@ static int follow_managed(struct path *path, unsigned flags)
/* Handle an automount point */
if (managed & DCACHE_NEED_AUTOMOUNT) {
- ret = follow_automount(path, flags, &need_mntput);
+ ret = follow_automount(path, nd->flags, &need_mntput);
if (ret < 0)
break;
continue;
@@ -851,9 +853,11 @@ static int follow_managed(struct path *path, unsigned flags)
if (need_mntput && path->mnt == mnt)
mntput(path->mnt);
- if (ret == -EISDIR)
- ret = 0;
- return ret < 0 ? ret : need_mntput;
+ if (ret < 0 && ret != -EISDIR)
+ return ret;
+ if (need_mntput)
+ nd->flags |= LOOKUP_JUMPED;
+ return 0;
}
int follow_down_one(struct path *path)
@@ -1212,13 +1216,11 @@ retry:
path->mnt = mnt;
path->dentry = dentry;
- err = follow_managed(path, nd->flags);
- if (unlikely(err < 0)) {
+ err = follow_managed(path, nd);
+ if (unlikely(err)) {
path_put_conditional(path, nd);
return err;
}
- if (err)
- nd->flags |= LOOKUP_JUMPED;
*inode = path->dentry->d_inode;
return 0;
}
@@ -2239,12 +2241,9 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
if (open_flag & O_EXCL)
goto exit_dput;
- error = follow_managed(path, nd->flags);
- if (error < 0)
- goto exit_dput;
-
+ error = follow_managed(path, nd);
if (error)
- nd->flags |= LOOKUP_JUMPED;
+ goto exit_dput;
error = -ENOENT;
if (!path->dentry->d_inode)
--
1.7.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 5/6] vfs: reorganize do_lookup
2012-03-26 10:54 [PATCH 0/6] vfs: path lookup fixes and cleanups Miklos Szeredi
` (3 preceding siblings ...)
2012-03-26 10:54 ` [PATCH 4/6] vfs: set LOOKUP_JUMPED in follow_managed Miklos Szeredi
@ 2012-03-26 10:54 ` Miklos Szeredi
2012-03-30 19:05 ` Al Viro
2012-03-26 10:54 ` [PATCH 6/6] vfs: split __lookup_hash Miklos Szeredi
5 siblings, 1 reply; 12+ messages in thread
From: Miklos Szeredi @ 2012-03-26 10:54 UTC (permalink / raw)
To: viro; +Cc: linux-fsdevel, linux-kernel, hch, mszeredi
From: Miklos Szeredi <mszeredi@suse.cz>
do_lookup's uncached lookup part basically open codes __lookup_hash so use that
instead.
This also eliminates the weird retry loop, that could, in theory, retry the
cached lookup any number of times (very unlikely scenario: needs two parallel
do_lookups and d_revalidate always returning zero).
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
fs/namei.c | 136 +++++++++++++++++++++++++----------------------------------
1 files changed, 58 insertions(+), 78 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 482992e..d500cb0 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1112,6 +1112,46 @@ static struct dentry *d_inode_lookup(struct dentry *parent, struct dentry *dentr
return dentry;
}
+static struct dentry *__lookup_hash(struct qstr *name, struct dentry *base,
+ struct nameidata *nd)
+{
+ struct dentry *dentry;
+
+ dentry = d_lookup(base, name);
+
+ if (dentry && d_need_lookup(dentry)) {
+ /*
+ * __lookup_hash is called with the parent dir's i_mutex already
+ * held, so we are good to go here.
+ */
+ return d_inode_lookup(base, dentry, nd);
+ }
+
+ if (dentry && (dentry->d_flags & DCACHE_OP_REVALIDATE)) {
+ int status = d_revalidate(dentry, nd);
+ if (unlikely(status <= 0)) {
+ /*
+ * The dentry failed validation.
+ * If d_revalidate returned 0 attempt to invalidate
+ * the dentry otherwise d_revalidate is asking us
+ * to return a fail status.
+ */
+ if (status < 0) {
+ dput(dentry);
+ return ERR_PTR(status);
+ } else if (!d_invalidate(dentry)) {
+ dput(dentry);
+ dentry = NULL;
+ }
+ }
+ }
+
+ if (!dentry)
+ dentry = d_alloc_and_lookup(base, name, nd);
+
+ return dentry;
+}
+
/*
* It's more convoluted than I'd like it to be, but... it's still fairly
* small and for now I'd prefer to have fast path as straight as possible.
@@ -1167,37 +1207,12 @@ unlazy:
dentry = __d_lookup(parent, name);
}
- if (dentry && unlikely(d_need_lookup(dentry))) {
- dput(dentry);
- dentry = NULL;
- }
-retry:
- if (unlikely(!dentry)) {
- struct inode *dir = parent->d_inode;
- BUG_ON(nd->inode != dir);
+ if (unlikely(!dentry))
+ goto need_lookup;
- mutex_lock(&dir->i_mutex);
- dentry = d_lookup(parent, name);
- if (likely(!dentry)) {
- dentry = d_alloc_and_lookup(parent, name, nd);
- if (IS_ERR(dentry)) {
- mutex_unlock(&dir->i_mutex);
- return PTR_ERR(dentry);
- }
- /* known good */
- need_reval = 0;
- status = 1;
- } else if (unlikely(d_need_lookup(dentry))) {
- dentry = d_inode_lookup(parent, dentry, nd);
- if (IS_ERR(dentry)) {
- mutex_unlock(&dir->i_mutex);
- return PTR_ERR(dentry);
- }
- /* known good */
- need_reval = 0;
- status = 1;
- }
- mutex_unlock(&dir->i_mutex);
+ if (unlikely(d_need_lookup(dentry))) {
+ dput(dentry);
+ goto need_lookup;
}
if (unlikely(dentry->d_flags & DCACHE_OP_REVALIDATE) && need_reval)
status = d_revalidate(dentry, nd);
@@ -1208,12 +1223,11 @@ retry:
}
if (!d_invalidate(dentry)) {
dput(dentry);
- dentry = NULL;
- need_reval = 1;
- goto retry;
+ goto need_lookup;
}
}
+success:
path->mnt = mnt;
path->dentry = dentry;
err = follow_managed(path, nd);
@@ -1223,6 +1237,17 @@ retry:
}
*inode = path->dentry->d_inode;
return 0;
+
+need_lookup:
+ BUG_ON(nd->inode != parent->d_inode);
+
+ mutex_lock(&parent->d_inode->i_mutex);
+ dentry = __lookup_hash(name, parent, nd);
+ mutex_unlock(&parent->d_inode->i_mutex);
+ if (IS_ERR(dentry))
+ return PTR_ERR(dentry);
+
+ goto success;
}
static inline int may_lookup(struct nameidata *nd)
@@ -1725,51 +1750,6 @@ int vfs_path_lookup(struct dentry *dentry, struct vfsmount *mnt,
return err;
}
-static struct dentry *__lookup_hash(struct qstr *name,
- struct dentry *base, struct nameidata *nd)
-{
- struct dentry *dentry;
-
- /*
- * Don't bother with __d_lookup: callers are for creat as
- * well as unlink, so a lot of the time it would cost
- * a double lookup.
- */
- dentry = d_lookup(base, name);
-
- if (dentry && d_need_lookup(dentry)) {
- /*
- * __lookup_hash is called with the parent dir's i_mutex already
- * held, so we are good to go here.
- */
- return d_inode_lookup(base, dentry, nd);
- }
-
- if (dentry && (dentry->d_flags & DCACHE_OP_REVALIDATE)) {
- int status = d_revalidate(dentry, nd);
- if (unlikely(status <= 0)) {
- /*
- * The dentry failed validation.
- * If d_revalidate returned 0 attempt to invalidate
- * the dentry otherwise d_revalidate is asking us
- * to return a fail status.
- */
- if (status < 0) {
- dput(dentry);
- return ERR_PTR(status);
- } else if (!d_invalidate(dentry)) {
- dput(dentry);
- dentry = NULL;
- }
- }
- }
-
- if (!dentry)
- dentry = d_alloc_and_lookup(base, name, nd);
-
- return dentry;
-}
-
/*
* Restricted form of lookup. Doesn't follow links, single-component only,
* needs parent already locked. Doesn't follow mounts.
--
1.7.7
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 5/6] vfs: reorganize do_lookup
2012-03-26 10:54 ` [PATCH 5/6] vfs: reorganize do_lookup Miklos Szeredi
@ 2012-03-30 19:05 ` Al Viro
2012-04-03 8:13 ` Miklos Szeredi
0 siblings, 1 reply; 12+ messages in thread
From: Al Viro @ 2012-03-30 19:05 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: linux-fsdevel, linux-kernel, hch, mszeredi
On Mon, Mar 26, 2012 at 12:54:23PM +0200, Miklos Szeredi wrote:
> This also eliminates the weird retry loop, that could, in theory, retry the
> cached lookup any number of times (very unlikely scenario: needs two parallel
> do_lookups and d_revalidate always returning zero).
That really needs to be carved into much smaller pieces - the sucker is
convoluted as hell and there's a lot of codepaths in there with nearly
zero test coverage. I've split it up into provably equivalent
transformations, leading more or less to the state where yours ends up.
I _think_ I've reconstructed the sequence of changes more or less
close to what you were doing there, but the next time you have to do
something of that kind, do not collapse that into a single patch. It's
really easier to review step by step...
Anyway, I'd put the whole thing into vfs.git#for-linus; see if you have
any problems with it.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 5/6] vfs: reorganize do_lookup
2012-03-30 19:05 ` Al Viro
@ 2012-04-03 8:13 ` Miklos Szeredi
2012-04-04 3:16 ` Al Viro
0 siblings, 1 reply; 12+ messages in thread
From: Miklos Szeredi @ 2012-04-03 8:13 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, linux-kernel, hch
Al Viro <viro@ZenIV.linux.org.uk> writes:
> On Mon, Mar 26, 2012 at 12:54:23PM +0200, Miklos Szeredi wrote:
>> This also eliminates the weird retry loop, that could, in theory, retry the
>> cached lookup any number of times (very unlikely scenario: needs two parallel
>> do_lookups and d_revalidate always returning zero).
>
> That really needs to be carved into much smaller pieces - the sucker is
> convoluted as hell and there's a lot of codepaths in there with nearly
> zero test coverage. I've split it up into provably equivalent
> transformations, leading more or less to the state where yours ends up.
> I _think_ I've reconstructed the sequence of changes more or less
> close to what you were doing there, but the next time you have to do
> something of that kind, do not collapse that into a single patch. It's
> really easier to review step by step...
Okay, but actually what I was doing there is looking at what the code
actually does and realizing that it's equivalent to __lookup_hash(), so
for me it was a single (albeit complex) single step.
But I'll try to keep reviewability in mind.
The do_last() reorganization in the atomic-open series needs to be split
up, I realize. Do you have any other high level comments about that
series?
Thanks,
Miklos
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 5/6] vfs: reorganize do_lookup
2012-04-03 8:13 ` Miklos Szeredi
@ 2012-04-04 3:16 ` Al Viro
2012-04-05 14:44 ` Miklos Szeredi
0 siblings, 1 reply; 12+ messages in thread
From: Al Viro @ 2012-04-04 3:16 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: linux-fsdevel, linux-kernel, hch
On Tue, Apr 03, 2012 at 10:13:18AM +0200, Miklos Szeredi wrote:
> The do_last() reorganization in the atomic-open series needs to be split
> up, I realize. Do you have any other high level comments about that
> series?
Yes.
a) Please, pull removal of open-from-d_revalidate() as far in front
of the queue as possible, *along* *with* -EOPENSTALE stuff. Without the
latter the former is simply broken - we might have hit -ESTALE before and
LOOKUP_REVAL might have already been set, so just failing ->open() with
ESTALE may end up not repeating it.
TBH, had that thing been in front of the queue, I would've put
it into the last pull request; that particular idiocy (NFS4 doing open
from all methods involved, except for ->open()) had been a serious source
of annoyance for a long time. It really needs killing...
b) opendata is simply bogus. You need to pass caller-allocated
struct file in any case, right? So why not use it to pass what you
need to pass? We want to be able to tell "it's a symlink, here's the
vfsmount/dentry, now sod off and handle it yourself"? Sure, but struct
file *already* contains struct path and that's not something that might
disappear on future kernel changes ;-)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 5/6] vfs: reorganize do_lookup
2012-04-04 3:16 ` Al Viro
@ 2012-04-05 14:44 ` Miklos Szeredi
0 siblings, 0 replies; 12+ messages in thread
From: Miklos Szeredi @ 2012-04-05 14:44 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, linux-kernel, hch
Al Viro <viro@ZenIV.linux.org.uk> writes:
> On Tue, Apr 03, 2012 at 10:13:18AM +0200, Miklos Szeredi wrote:
>
>> The do_last() reorganization in the atomic-open series needs to be split
>> up, I realize. Do you have any other high level comments about that
>> series?
>
> Yes.
> a) Please, pull removal of open-from-d_revalidate() as far in front
> of the queue as possible, *along* *with* -EOPENSTALE stuff. Without the
> latter the former is simply broken - we might have hit -ESTALE before and
> LOOKUP_REVAL might have already been set, so just failing ->open() with
> ESTALE may end up not repeating it.
> TBH, had that thing been in front of the queue, I would've put
> it into the last pull request; that particular idiocy (NFS4 doing open
> from all methods involved, except for ->open()) had been a serious source
> of annoyance for a long time. It really needs killing...
Will post updated series in a minute.
>
> b) opendata is simply bogus. You need to pass caller-allocated
> struct file in any case, right? So why not use it to pass what you
> need to pass? We want to be able to tell "it's a symlink, here's the
> vfsmount/dentry, now sod off and handle it yourself"?
My concern is about the visibility of a half cooked file pointer to the
filesystem. The compiler won't notice anything wrong with the
following, neither will casual testing, but it will go boom at
inopportune times:
foo_atomic_open(..., struct file *filp, ...)
{
/* ... */
if (unlikely error)
goto cleanup;
filp = finish_open(filp, dentry, NULL);
/* ... */
if (some other error)
goto cleanup;
return filp;
cleanup:
fput(filp);
return ERR_PTR(error);
}
Thanks,
Miklos
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 6/6] vfs: split __lookup_hash
2012-03-26 10:54 [PATCH 0/6] vfs: path lookup fixes and cleanups Miklos Szeredi
` (4 preceding siblings ...)
2012-03-26 10:54 ` [PATCH 5/6] vfs: reorganize do_lookup Miklos Szeredi
@ 2012-03-26 10:54 ` Miklos Szeredi
5 siblings, 0 replies; 12+ messages in thread
From: Miklos Szeredi @ 2012-03-26 10:54 UTC (permalink / raw)
To: viro; +Cc: linux-fsdevel, linux-kernel, hch, mszeredi
From: Miklos Szeredi <mszeredi@suse.cz>
Split __lookup_hash into two component functions:
lookup_dcache - tries cached lookup, returns whether real lookup is needed
lookup_real - calls i_op->lookup
This eliminates code duplication between d_alloc_and_lookup() and
d_inode_lookup().
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---
fs/namei.c | 103 +++++++++++++++++++++++++----------------------------------
1 files changed, 44 insertions(+), 59 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index d500cb0..d15d0ba 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1058,53 +1058,65 @@ static void follow_dotdot(struct nameidata *nd)
}
/*
- * Allocate a dentry with name and parent, and perform a parent
- * directory ->lookup on it. Returns the new dentry, or ERR_PTR
- * on error. parent->d_inode->i_mutex must be held. d_lookup must
- * have verified that no child exists while under i_mutex.
+ * This looks up the name in dcache, possibly revalidates the old dentry and
+ * allocates a new one if not found or not valid. In the need_lookup argument
+ * returns whether i_op->lookup is necessary.
+ *
+ * dir->d_inode->i_mutex must be held
*/
-static struct dentry *d_alloc_and_lookup(struct dentry *parent,
- struct qstr *name, struct nameidata *nd)
+static struct dentry *lookup_dcache(struct qstr *name, struct dentry *dir,
+ struct nameidata *nd, bool *need_lookup)
{
- struct inode *inode = parent->d_inode;
struct dentry *dentry;
- struct dentry *old;
+ int error;
- /* Don't create child dentry for a dead directory. */
- if (unlikely(IS_DEADDIR(inode)))
- return ERR_PTR(-ENOENT);
+ *need_lookup = false;
+ dentry = d_lookup(dir, name);
+ if (dentry) {
+ if (d_need_lookup(dentry)) {
+ *need_lookup = true;
+ } else if (dentry->d_flags & DCACHE_OP_REVALIDATE) {
+ error = d_revalidate(dentry, nd);
+ if (unlikely(error <= 0)) {
+ if (error < 0) {
+ dput(dentry);
+ return ERR_PTR(error);
+ } else if (!d_invalidate(dentry)) {
+ dput(dentry);
+ dentry = NULL;
+ }
+ }
+ }
+ }
- dentry = d_alloc(parent, name);
- if (unlikely(!dentry))
- return ERR_PTR(-ENOMEM);
+ if (!dentry) {
+ dentry = d_alloc(dir, name);
+ if (unlikely(!dentry))
+ return ERR_PTR(-ENOMEM);
- old = inode->i_op->lookup(inode, dentry, nd);
- if (unlikely(old)) {
- dput(dentry);
- dentry = old;
+ *need_lookup = true;
}
return dentry;
}
/*
- * We already have a dentry, but require a lookup to be performed on the parent
- * directory to fill in d_inode. Returns the new dentry, or ERR_PTR on error.
- * parent->d_inode->i_mutex must be held. d_lookup must have verified that no
- * child exists while under i_mutex.
+ * Call i_op->lookup on the dentry. The dentry must be negative but may be
+ * hashed if it was pouplated with DCACHE_NEED_LOOKUP.
+ *
+ * dir->d_inode->i_mutex must be held
*/
-static struct dentry *d_inode_lookup(struct dentry *parent, struct dentry *dentry,
- struct nameidata *nd)
+static struct dentry *lookup_real(struct inode *dir, struct dentry *dentry,
+ struct nameidata *nd)
{
- struct inode *inode = parent->d_inode;
struct dentry *old;
/* Don't create child dentry for a dead directory. */
- if (unlikely(IS_DEADDIR(inode))) {
+ if (unlikely(IS_DEADDIR(dir))) {
dput(dentry);
return ERR_PTR(-ENOENT);
}
- old = inode->i_op->lookup(inode, dentry, nd);
+ old = dir->i_op->lookup(dir, dentry, nd);
if (unlikely(old)) {
dput(dentry);
dentry = old;
@@ -1115,41 +1127,14 @@ static struct dentry *d_inode_lookup(struct dentry *parent, struct dentry *dentr
static struct dentry *__lookup_hash(struct qstr *name, struct dentry *base,
struct nameidata *nd)
{
+ bool need_lookup;
struct dentry *dentry;
- dentry = d_lookup(base, name);
-
- if (dentry && d_need_lookup(dentry)) {
- /*
- * __lookup_hash is called with the parent dir's i_mutex already
- * held, so we are good to go here.
- */
- return d_inode_lookup(base, dentry, nd);
- }
+ dentry = lookup_dcache(name, base, nd, &need_lookup);
+ if (!need_lookup)
+ return dentry;
- if (dentry && (dentry->d_flags & DCACHE_OP_REVALIDATE)) {
- int status = d_revalidate(dentry, nd);
- if (unlikely(status <= 0)) {
- /*
- * The dentry failed validation.
- * If d_revalidate returned 0 attempt to invalidate
- * the dentry otherwise d_revalidate is asking us
- * to return a fail status.
- */
- if (status < 0) {
- dput(dentry);
- return ERR_PTR(status);
- } else if (!d_invalidate(dentry)) {
- dput(dentry);
- dentry = NULL;
- }
- }
- }
-
- if (!dentry)
- dentry = d_alloc_and_lookup(base, name, nd);
-
- return dentry;
+ return lookup_real(base->d_inode, dentry, nd);
}
/*
--
1.7.7
^ permalink raw reply related [flat|nested] 12+ messages in thread