Re: [RFC][PATCH 8/14] Union-mount lookup
On Tue, May 15, 2007 at 10:00:45AM -0400, Trond Myklebust wrote:
> On Mon, 2007-05-14 at 15:12 +0530, Bharata B Rao wrote:
> > From: Jan Blunck <[EMAIL PROTECTED]>
> > Subject: Union-mount lookup
> >
> > Modifies the vfs lookup routines to work with union mounted directories.
> >
> > The existing lookup routines generally lookup for a pathname only in the
> > topmost or given directory. The changed versions of the lookup routines
> > search for the pathname in the entire union mounted stack. Also they have
> > been
> > modified to setup the union stack during lookup from dcache cache and from
> > real_lookup().
> >
> > Signed-off-by: Jan Blunck <[EMAIL PROTECTED]>
> > Signed-off-by: Bharata B Rao <[EMAIL PROTECTED]>
> > ---
> > fs/dcache.c| 16 +
> > fs/namei.c | 78 +-
> > fs/namespace.c | 35 ++
> > fs/union.c | 598
> > +
> > include/linux/dcache.h | 17 +
> > include/linux/namei.h |4
> > include/linux/union.h | 49
> > 7 files changed, 786 insertions(+), 11 deletions(-)
> >
> > --- a/fs/dcache.c
> > +++ b/fs/dcache.c
> > @@ -1286,7 +1286,7 @@ struct dentry * d_lookup(struct dentry *
> > return dentry;
> > }
> >
> > -struct dentry * __d_lookup(struct dentry * parent, struct qstr * name)
> > +struct dentry * __d_lookup_single(struct dentry *parent, struct qstr *name)
> > {
> > unsigned int len = name->len;
> > unsigned int hash = name->hash;
> > @@ -1371,6 +1371,20 @@ out:
> > return dentry;
> > }
> >
> > +struct dentry * d_lookup_single(struct dentry *parent, struct qstr *name)
> > +{
> > + struct dentry *dentry;
> > + unsigned long seq;
> > +
> > +do {
> > +seq = read_seqbegin(&rename_lock);
> > +dentry = __d_lookup_single(parent, name);
> > +if (dentry)
> > + break;
> > + } while (read_seqretry(&rename_lock, seq));
> > + return dentry;
> > +}
> > +
> > /**
> > * d_validate - verify dentry provided from insecure source
> > * @dentry: The dentry alleged to be valid child of @dparent
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > @@ -374,6 +374,33 @@ void release_open_intent(struct nameidat
> > }
> >
> > static inline struct dentry *
> > +do_revalidate_single(struct dentry *dentry, struct nameidata *nd)
> > +{
> > + int status = dentry->d_op->d_revalidate(dentry, nd);
> > + if (unlikely(status <= 0)) {
>
> d_revalidate() returns a 0 or 1 result, not an error.
Doesn't look like (see the comment below) because this is copied
as-is from do_revalidate().
>
> > + /*
> > +* 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) {
> > + if (!d_invalidate(dentry)) {
> > + __dput_single(dentry);
> > + dentry = NULL;
> > + }
> > + } else {
> > + __dput_single(dentry);
> > + dentry = ERR_PTR(status);
Regards,
Bharata.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH 8/14] Union-mount lookup
Quoting Jan Engelhardt ([EMAIL PROTECTED]):
>
> On May 16 2007 10:38, Bharata B Rao wrote:
> >>
> >> >+lookup_union:
> >> >+ do {
> >> >+ struct vfsmount *mnt = find_mnt(topmost);
> >> >+ UM_DEBUG_DCACHE("name=\"%s\", inode=%p, device=%s\n",
> >> >+ topmost->d_name.name, topmost->d_inode,
> >> >+ mnt->mnt_devname);
> >> >+ mntput(mnt);
> >> >+ } while (0);
> >>
> >> Why the extra do{}while? [elsewhere too]
> >
> >Not sure, may be to get a scope to define 'mnt' here. Jan ?
>
> What I was implicitly suggesting that mnt could be moved into the
> normal 'function scope'.
>
>
> Jan
This code can't stay anyway so it's kind of moot. find_mnt() is bogus,
and the topmost and overlaid mappings need to be changed from
dentry->dentry to (vfsmnt,dentry)->(vfsmnt,dentry) in order to cope with
bind mounts and mount namespaces.
-serge
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH 8/14] Union-mount lookup
On May 16 2007 10:38, Bharata B Rao wrote:
>>
>> >+lookup_union:
>> >+ do {
>> >+ struct vfsmount *mnt = find_mnt(topmost);
>> >+ UM_DEBUG_DCACHE("name=\"%s\", inode=%p, device=%s\n",
>> >+ topmost->d_name.name, topmost->d_inode,
>> >+ mnt->mnt_devname);
>> >+ mntput(mnt);
>> >+ } while (0);
>>
>> Why the extra do{}while? [elsewhere too]
>
>Not sure, may be to get a scope to define 'mnt' here. Jan ?
What I was implicitly suggesting that mnt could be moved into the
normal 'function scope'.
Jan
--
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH 8/14] Union-mount lookup
On Tue, May 15, 2007 at 09:57:24AM +0200, Jan Engelhardt wrote:
>
> On May 14 2007 15:12, Bharata B Rao wrote:
> >
> >+struct dentry * d_lookup_single(struct dentry *parent, struct qstr *name)
> >+{
> >+struct dentry *dentry;
> >+unsigned long seq;
> >+
> >+do {
> >+seq = read_seqbegin(&rename_lock);
> >+dentry = __d_lookup_single(parent, name);
> >+if (dentry)
> >+break;
> >+} while (read_seqretry(&rename_lock, seq));
> >+return dentry;
> >+}
>
> Replace with tabs.
This is copied from fs/dcache.c:d_lookup() and the whitespaces came from there.
But that is not an excuse, will fix.
>
> >+lookup_union:
> >+do {
> >+struct vfsmount *mnt = find_mnt(topmost);
> >+UM_DEBUG_DCACHE("name=\"%s\", inode=%p, device=%s\n",
> >+topmost->d_name.name, topmost->d_inode,
> >+mnt->mnt_devname);
> >+mntput(mnt);
> >+} while (0);
>
> Why the extra do{}while? [elsewhere too]
Not sure, may be to get a scope to define 'mnt' here. Jan ?
>
> >+if (topmost->d_union) {
> >+union_lock_spinlock(topmost, &topmost->d_lock);
> >+}
>
> Extra {} could go [elsewhere too].
>
> >+if (last->d_overlaid
> >+&& (last->d_overlaid != dentry)) {
>
> As can these extra () [elsewhere too].
>
Sure, will fix all these.
> >+static inline struct dentry * __lookup_hash_single(struct qstr *name,
> >struct dentry *base, struct nameidata *nd)
> >+{
> >+struct dentry *dentry;
> >+struct inode *inode;
> >+int err;
> >+
> >+inode = base->d_inode;
> >+
> >+err = permission(inode, MAY_EXEC, nd);
> >+dentry = ERR_PTR(err);
> >+if (err)
> >+goto out;
> >+
> >+dentry = __lookup_hash_kern_single(name, base, nd);
> >+out:
> >+return dentry;
> >+}
>
> This looks a little big for being inline.
ok.
Regards,
Bharata.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH 8/14] Union-mount lookup
On Mon, 2007-05-14 at 15:12 +0530, Bharata B Rao wrote:
> From: Jan Blunck <[EMAIL PROTECTED]>
> Subject: Union-mount lookup
>
> Modifies the vfs lookup routines to work with union mounted directories.
>
> The existing lookup routines generally lookup for a pathname only in the
> topmost or given directory. The changed versions of the lookup routines
> search for the pathname in the entire union mounted stack. Also they have been
> modified to setup the union stack during lookup from dcache cache and from
> real_lookup().
>
> Signed-off-by: Jan Blunck <[EMAIL PROTECTED]>
> Signed-off-by: Bharata B Rao <[EMAIL PROTECTED]>
> ---
> fs/dcache.c| 16 +
> fs/namei.c | 78 +-
> fs/namespace.c | 35 ++
> fs/union.c | 598
> +
> include/linux/dcache.h | 17 +
> include/linux/namei.h |4
> include/linux/union.h | 49
> 7 files changed, 786 insertions(+), 11 deletions(-)
>
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -1286,7 +1286,7 @@ struct dentry * d_lookup(struct dentry *
> return dentry;
> }
>
> -struct dentry * __d_lookup(struct dentry * parent, struct qstr * name)
> +struct dentry * __d_lookup_single(struct dentry *parent, struct qstr *name)
> {
> unsigned int len = name->len;
> unsigned int hash = name->hash;
> @@ -1371,6 +1371,20 @@ out:
> return dentry;
> }
>
> +struct dentry * d_lookup_single(struct dentry *parent, struct qstr *name)
> +{
> + struct dentry *dentry;
> + unsigned long seq;
> +
> +do {
> +seq = read_seqbegin(&rename_lock);
> +dentry = __d_lookup_single(parent, name);
> +if (dentry)
> + break;
> + } while (read_seqretry(&rename_lock, seq));
> + return dentry;
> +}
> +
> /**
> * d_validate - verify dentry provided from insecure source
> * @dentry: The dentry alleged to be valid child of @dparent
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -374,6 +374,33 @@ void release_open_intent(struct nameidat
> }
>
> static inline struct dentry *
> +do_revalidate_single(struct dentry *dentry, struct nameidata *nd)
> +{
> + int status = dentry->d_op->d_revalidate(dentry, nd);
> + if (unlikely(status <= 0)) {
d_revalidate() returns a 0 or 1 result, not an error.
> + /*
> + * 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) {
> + if (!d_invalidate(dentry)) {
> + __dput_single(dentry);
> + dentry = NULL;
> + }
> + } else {
> + __dput_single(dentry);
> + dentry = ERR_PTR(status);
See above
> + }
> + }
> + return dentry;
> +}
> +
> +/*
> + * FIXME: We need a union aware revalidate here!
> + */
> +static inline struct dentry *
> do_revalidate(struct dentry *dentry, struct nameidata *nd)
> {
> int status = dentry->d_op->d_revalidate(dentry, nd);
> @@ -403,16 +430,16 @@ do_revalidate(struct dentry *dentry, str
> */
> static struct dentry * cached_lookup(struct dentry * parent, struct qstr *
> name, struct nameidata *nd)
> {
> - struct dentry * dentry = __d_lookup(parent, name);
> + struct dentry *dentry = __d_lookup_single(parent, name);
>
> /* lockess __d_lookup may fail due to concurrent d_move()
>* in some unrelated directory, so try with d_lookup
>*/
> if (!dentry)
> - dentry = d_lookup(parent, name);
> + dentry = d_lookup_single(parent, name);
>
> if (dentry && dentry->d_op && dentry->d_op->d_revalidate)
> - dentry = do_revalidate(dentry, nd);
> + dentry = do_revalidate_single(dentry, nd);
>
> return dentry;
> }
> @@ -465,7 +492,7 @@ ok:
> * make sure that nobody added the entry to the dcache in the meantime..
> * SMP-safe
> */
> -static struct dentry * real_lookup(struct dentry * parent, struct qstr *
> name, struct nameidata *nd)
> +struct dentry * real_lookup_single(struct dentry *parent, struct qstr *name,
> struct nameidata *nd)
> {
> struct dentry * result;
> struct inode *dir = parent->d_inode;
> @@ -485,7 +512,7 @@ static struct dentry * real_lookup(struc
>*
>* so doing d_lookup() (with seqlock), instead of lockfree __d_lookup
>*/
> - result = d_lookup(parent, name);
> + result = d_lookup_single(parent, name);
> if (!result) {
> struct dentry * dentry = d_alloc(parent, name);
> result = ERR_PTR(-ENOMEM);
> @@ -506,7 +533,7 @@ static struct dentry * real_lookup(struc
>*/
> mutex_unlock(&dir->i_mutex);
> if (result->d_op && resul
Re: [RFC][PATCH 8/14] Union-mount lookup
On May 14 2007 15:12, Bharata B Rao wrote:
>
>+struct dentry * d_lookup_single(struct dentry *parent, struct qstr *name)
>+{
>+ struct dentry *dentry;
>+ unsigned long seq;
>+
>+do {
>+seq = read_seqbegin(&rename_lock);
>+dentry = __d_lookup_single(parent, name);
>+if (dentry)
>+ break;
>+ } while (read_seqretry(&rename_lock, seq));
>+ return dentry;
>+}
Replace with tabs.
>+lookup_union:
>+ do {
>+ struct vfsmount *mnt = find_mnt(topmost);
>+ UM_DEBUG_DCACHE("name=\"%s\", inode=%p, device=%s\n",
>+ topmost->d_name.name, topmost->d_inode,
>+ mnt->mnt_devname);
>+ mntput(mnt);
>+ } while (0);
Why the extra do{}while? [elsewhere too]
>+ if (topmost->d_union) {
>+ union_lock_spinlock(topmost, &topmost->d_lock);
>+ }
Extra {} could go [elsewhere too].
>+ if (last->d_overlaid
>+ && (last->d_overlaid != dentry)) {
As can these extra () [elsewhere too].
>+static inline struct dentry * __lookup_hash_single(struct qstr *name, struct
>dentry *base, struct nameidata *nd)
>+{
>+ struct dentry *dentry;
>+ struct inode *inode;
>+ int err;
>+
>+ inode = base->d_inode;
>+
>+ err = permission(inode, MAY_EXEC, nd);
>+ dentry = ERR_PTR(err);
>+ if (err)
>+ goto out;
>+
>+ dentry = __lookup_hash_kern_single(name, base, nd);
>+out:
>+ return dentry;
>+}
This looks a little big for being inline.
Jan
--
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RFC][PATCH 8/14] Union-mount lookup
From: Jan Blunck <[EMAIL PROTECTED]>
Subject: Union-mount lookup
Modifies the vfs lookup routines to work with union mounted directories.
The existing lookup routines generally lookup for a pathname only in the
topmost or given directory. The changed versions of the lookup routines
search for the pathname in the entire union mounted stack. Also they have been
modified to setup the union stack during lookup from dcache cache and from
real_lookup().
Signed-off-by: Jan Blunck <[EMAIL PROTECTED]>
Signed-off-by: Bharata B Rao <[EMAIL PROTECTED]>
---
fs/dcache.c| 16 +
fs/namei.c | 78 +-
fs/namespace.c | 35 ++
fs/union.c | 598 +
include/linux/dcache.h | 17 +
include/linux/namei.h |4
include/linux/union.h | 49
7 files changed, 786 insertions(+), 11 deletions(-)
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1286,7 +1286,7 @@ struct dentry * d_lookup(struct dentry *
return dentry;
}
-struct dentry * __d_lookup(struct dentry * parent, struct qstr * name)
+struct dentry * __d_lookup_single(struct dentry *parent, struct qstr *name)
{
unsigned int len = name->len;
unsigned int hash = name->hash;
@@ -1371,6 +1371,20 @@ out:
return dentry;
}
+struct dentry * d_lookup_single(struct dentry *parent, struct qstr *name)
+{
+ struct dentry *dentry;
+ unsigned long seq;
+
+do {
+seq = read_seqbegin(&rename_lock);
+dentry = __d_lookup_single(parent, name);
+if (dentry)
+ break;
+ } while (read_seqretry(&rename_lock, seq));
+ return dentry;
+}
+
/**
* d_validate - verify dentry provided from insecure source
* @dentry: The dentry alleged to be valid child of @dparent
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -374,6 +374,33 @@ void release_open_intent(struct nameidat
}
static inline struct dentry *
+do_revalidate_single(struct dentry *dentry, struct nameidata *nd)
+{
+ int status = dentry->d_op->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) {
+ if (!d_invalidate(dentry)) {
+ __dput_single(dentry);
+ dentry = NULL;
+ }
+ } else {
+ __dput_single(dentry);
+ dentry = ERR_PTR(status);
+ }
+ }
+ return dentry;
+}
+
+/*
+ * FIXME: We need a union aware revalidate here!
+ */
+static inline struct dentry *
do_revalidate(struct dentry *dentry, struct nameidata *nd)
{
int status = dentry->d_op->d_revalidate(dentry, nd);
@@ -403,16 +430,16 @@ do_revalidate(struct dentry *dentry, str
*/
static struct dentry * cached_lookup(struct dentry * parent, struct qstr *
name, struct nameidata *nd)
{
- struct dentry * dentry = __d_lookup(parent, name);
+ struct dentry *dentry = __d_lookup_single(parent, name);
/* lockess __d_lookup may fail due to concurrent d_move()
* in some unrelated directory, so try with d_lookup
*/
if (!dentry)
- dentry = d_lookup(parent, name);
+ dentry = d_lookup_single(parent, name);
if (dentry && dentry->d_op && dentry->d_op->d_revalidate)
- dentry = do_revalidate(dentry, nd);
+ dentry = do_revalidate_single(dentry, nd);
return dentry;
}
@@ -465,7 +492,7 @@ ok:
* make sure that nobody added the entry to the dcache in the meantime..
* SMP-safe
*/
-static struct dentry * real_lookup(struct dentry * parent, struct qstr * name,
struct nameidata *nd)
+struct dentry * real_lookup_single(struct dentry *parent, struct qstr *name,
struct nameidata *nd)
{
struct dentry * result;
struct inode *dir = parent->d_inode;
@@ -485,7 +512,7 @@ static struct dentry * real_lookup(struc
*
* so doing d_lookup() (with seqlock), instead of lockfree __d_lookup
*/
- result = d_lookup(parent, name);
+ result = d_lookup_single(parent, name);
if (!result) {
struct dentry * dentry = d_alloc(parent, name);
result = ERR_PTR(-ENOMEM);
@@ -506,7 +533,7 @@ static struct dentry * real_lookup(struc
*/
mutex_unlock(&dir->i_mutex);
if (result->d_op && result->d_op->d_revalidate) {
- result = do_revalidate(result, nd);
+ result = do_revalidate_single(result, nd);
if (!result)
result = ERR_PTR(-ENOENT);
}
@@ -699,7 +726,7 @@ static int __follow_mount(struct path *p
