[EMAIL PROTECTED] wrote: > Hi, > > Jeff Mahoney: >> Is it because the permission check is unwanted in lookup_hash (well, >> technically, lookup_one_len)? >> >> What conditions make fput unusable? > > The permission check is unrelated. Where VFS execute the check, I don't > want to skip it. If you read lookup_one_len(), you would know nameidata > passed __lookup_hash() is always NULL. And this parameter surely make > NFS branch crash. > Essentially what aufs needs is lookup_one_len() only. But it cannot be > used for NFS branch. Finally I have to call __lookup_hash() directly. > > put_filp() is necessary to support 'atomic open' in NFSv4. > When an error happened after the file is opened internally, aufs has to > close it. If aufs called fput() instead of put_filp(), many unnecessary > and unwanted functions would be called. > > Do I make it clear?
Ok, yeah. fput() expects the file to have an instantiated dentry, which
may not exist. I've switched the code to use release_open_intent instead
and exported that in our kernel.
As for the lookup_one_len vs __lookup_hash issue, have you run into
problems with NFS2/3 as well as NFSv4? nfs_lookup looks like it can deal
with a NULL nameidata. NFSv4's nfs_atomic_lookup most definitely can't.
I wonder because while I'm keen to support NFS with aufs on openSUSE,
but NFS4 isn't as much of a concern on LiveCDs.
In my tree, I use the following check to enable NFS access in general,
but disable NFSv4. I also use lookup_one_len instead of __lookup_hash if
__lookup_hash isn't available. NFS4 isn't enabled without __lookup_hash.
static inline int au_test_unsupported_nfs(struct inode *h_inode)
{
#ifdef CONFIG_AUFS_BR_NFS4
return 0;
#else
return h_inode->i_sb->s_magic == NFS_SUPER_MAGIC &&
NFS_PROTO(h_inode)->version == 4;
#endif
}
I've attached the entire patch set:
- factor out the open-coded __lookup_one_len
- push the lookups down, so that we can pass name/len pairs to
lookup_one_len without duplicating qstr processing
- use release_open_intent instead of put_filp
- handle nfs4 separate from nfs2/3
I've done some light testing with NFSv3 and ensured it refuses an NFSv4
branch, but haven't tested otherwise with NFSv4.
-Jeff
--
Jeff Mahoney
SUSE Labs
From: Jeff Mahoney <[EMAIL PROTECTED]>
Subject: aufs: Factor out __lookup_one_len
The existing code acknowledges that it's just open coding __lookup_one_len,
so why not just copy the function entirely and call it. If the code is
ever merged with mainline, it will be an obvious thing to rename
and export rather than having two copies of it.
Signed-off-by: Jeff Mahoney <[EMAIL PROTECTED]>
---
fs/aufs25/br_nfs.c | 43 +++++++++++++++++++++++++------------------
1 file changed, 25 insertions(+), 18 deletions(-)
--- a/fs/aufs25/br_nfs.c
+++ b/fs/aufs25/br_nfs.c
@@ -231,6 +231,28 @@ int au_hin_after_reval(struct nameidata
return err;
}
+static int __lookup_one_len(const char *name, struct qstr *this,
+ struct dentry *base, int len)
+{
+ unsigned long hash;
+ unsigned int c;
+
+ this->name = name;
+ this->len = len;
+ if (!len)
+ return -EACCES;
+
+ hash = init_name_hash();
+ while (len--) {
+ c = *(const unsigned char *)name++;
+ if (c == '/' || c == '\0')
+ return -EACCES;
+ hash = partial_name_hash(c, hash);
+ }
+ this->hash = end_name_hash(hash);
+ return 0;
+}
+
#ifdef CONFIG_AUFS_DLGT
struct au_lookup_hash_args {
struct dentry **errp;
@@ -285,32 +307,17 @@ struct dentry *au_lkup_hash(const char *
int len, struct au_ndx *ndx)
{
struct dentry *dentry;
- char *p;
- unsigned long hash;
struct qstr this;
- unsigned int c;
struct nameidata tmp_nd, *ndo;
int err;
LKTRTrace("%.*s/%.*s\n", AuDLNPair(parent), len, name);
- /* todo: export and call __lookup_one_len() in fs/namei.c? */
- dentry = ERR_PTR(-EACCES);
- this.name = name;
- this.len = len;
- if (unlikely(!len))
+ err = __lookup_one_len(name, &this, parent, len);
+ dentry = ERR_PTR(err);
+ if (err)
goto out;
- p = (void *)name;
- hash = init_name_hash();
- while (len--) {
- c = *p++;
- if (unlikely(c == '/' || c == '\0'))
- goto out;
- hash = partial_name_hash(c, hash);
- }
- this.hash = end_name_hash(hash);
-
ndo = ndx->nd;
if (ndo) {
tmp_nd = *ndo;
From: Jeff Mahoney <[EMAIL PROTECTED]>
Subject: aufs: Handle NFSv4 enablement separately from NFSv2/3
NFSv4's atomic opens require that a nameidata be passed down
into __lookup_hash, which isn't exported. However, NFSv2/3
can be supported with just lookup_one_len.
This patch handles NFSv4 enablement separately and will
enable NFSv2/3 support if __lookup_hash isn't exported, and
addtionally enable NFSv4 if it is.
Signed-off-by: Jeff Mahoney <[EMAIL PROTECTED]>
---
fs/aufs25/branch.c | 2 +-
fs/aufs25/branch.h | 15 +++++++++++----
fs/aufs25/vfsub.c | 10 ++++++----
fs/aufs25/vfsub.h | 2 --
local.mk | 6 +++++-
patch/release_open_intent.patch | 14 ++++++++++++++
6 files changed, 37 insertions(+), 12 deletions(-)
--- a/fs/aufs25/branch.c
+++ b/fs/aufs25/branch.c
@@ -393,7 +393,7 @@ static int test_add(struct super_block *
goto out;
}
- if (unlikely(au_test_unsupported_nfs(inode->i_sb))) {
+ if (unlikely(au_test_unsupported_nfs(inode))) {
AuErr(AuNoNfsBranchMsg " %s\n", add->path);
goto out;
}
--- a/fs/aufs25/branch.h
+++ b/fs/aufs25/branch.h
@@ -28,6 +28,7 @@
#ifdef __KERNEL__
#include <linux/fs.h>
+#include <linux/nfs_fs.h>
#include <linux/mount.h>
#include <linux/sysfs.h>
#include <linux/aufs_type.h>
@@ -350,9 +351,14 @@ static inline void au_br_nfs_lockdep_on(
}
#ifdef CONFIG_AUFS_BR_NFS
-static inline int au_test_unsupported_nfs(struct super_block *h_sb)
+static inline int au_test_unsupported_nfs(struct inode *h_inode)
{
+#ifdef CONFIG_AUFS_BR_NFS4
return 0;
+#else
+ return h_inode->i_sb->s_magic == NFS_SUPER_MAGIC &&
+ NFS_PROTO(h_inode)->version == 4;
+#endif
}
/* it doesn't mntget() */
@@ -362,12 +368,13 @@ struct vfsmount *au_nfsmnt(struct super_
return au_do_nfsmnt(au_sbr_mnt(sb, bindex));
}
-#define AuNoNfsBranchMsg "dummy"
+#define AuNoNfsBranchMsg "NFSv4 branch is not supported" \
+ ", try some configurations and patches included in aufs source CVS."
#else
-static inline int au_test_unsupported_nfs(struct super_block *h_sb)
+static inline int au_test_unsupported_nfs(struct inode *h_inode)
{
- return h_sb->s_magic == NFS_SUPER_MAGIC;
+ return h_inode->i_sb->s_magic == NFS_SUPER_MAGIC;
}
static inline
--- a/fs/aufs25/vfsub.c
+++ b/fs/aufs25/vfsub.c
@@ -84,7 +84,6 @@ struct dentry *vfsub_lookup_one_len(cons
return d;
}
-#ifdef CONFIG_AUFS_LHASH_PATCH
struct dentry *vfsub__lookup_hash(struct qstr *name, struct dentry *parent,
struct nameidata *nd)
{
@@ -92,16 +91,19 @@ struct dentry *vfsub__lookup_hash(struct
LKTRTrace("%.*s/%.*s, nd %d\n",
AuDLNPair(parent), AuLNPair(name), !!nd);
- if (nd)
- LKTRTrace("nd{0x%x}\n", nd->flags);
+#ifdef CONFIG_AUFS_LHASH_PATCH
+ BUG_ON(!nd);
+ LKTRTrace("nd{0x%x}\n", nd->flags);
IMustLock(parent->d_inode);
d = __lookup_hash(name, parent, nd);
if (!IS_ERR(d))
au_update_fuse_h_inode(NULL, d); /*ignore*/
+#else
+ d = vfsub_lookup_one_len(name->name, parent, name->len);
+#endif
return d;
}
-#endif
/* ---------------------------------------------------------------------- */
--- a/fs/aufs25/vfsub.h
+++ b/fs/aufs25/vfsub.h
@@ -250,10 +250,8 @@ int vfsub_path_lookup(const char *name,
struct dentry *vfsub_lookup_one_len(const char *name, struct dentry *parent,
int len);
-#ifdef CONFIG_AUFS_LHASH_PATCH
struct dentry *vfsub__lookup_hash(struct qstr *name, struct dentry *parent,
struct nameidata *nd);
-#endif
/* ---------------------------------------------------------------------- */
--- a/local.mk
+++ b/local.mk
@@ -61,8 +61,12 @@ AUFS_DEF_CONFIG =
# automatic configurations
export CONFIG_AUFS_BR_NFS =
+export CONFIG_AUFS_BR_NFS4 =
ifdef CONFIG_NFS_FS
CONFIG_AUFS_BR_NFS = y
+ifdef CONFIG_NFS_V4
+CONFIG_AUFS_BR_NFS4 = y
+endif
ifeq "t" "$(shell test ${SUBLEVEL} -lt 16 && echo t)"
CONFIG_AUFS_BR_NFS =
else ifeq "t" "$(shell test ${SUBLEVEL} -ge 19 \
@@ -71,7 +75,7 @@ else ifeq "t" "$(shell test ${SUBLEVEL}
-o x${CONFIG_AUFS_PUT_FILP_PATCH} = xy \) \
\) \
&& echo t)"
-CONFIG_AUFS_BR_NFS =
+CONFIG_AUFS_BR_NFS4 =
endif
endif
--- /dev/null
+++ b/patch/release_open_intent.patch
@@ -0,0 +1,14 @@
+---
+ fs/namei.c | 1 +
+ 1 file changed, 1 insertion(+)
+
+--- a/fs/namei.c
++++ b/fs/namei.c
+@@ -410,6 +410,7 @@ void release_open_intent(struct nameidat
+ else
+ fput(nd->intent.open.file);
+ }
++EXPORT_SYMBOL_GPL(release_open_intent);
+
+ static inline struct dentry *
+ do_revalidate(struct dentry *dentry, struct nameidata *nd)
From: Jeff Mahoney <[EMAIL PROTECTED]>
Subject: aufs: Push qstr processing down to au_lkup_hash_dlgt
This patch pushes the qstr processing down into au_lkup_hash_dlgt so
that lookup_one_len doesn't end up duplicating the qstr setup. This
is used in later patches.
Signed-off-by: Jeff Mahoney <[EMAIL PROTECTED]>
---
fs/aufs25/br_nfs.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
--- a/fs/aufs25/br_nfs.c
+++ b/fs/aufs25/br_nfs.c
@@ -268,20 +268,24 @@ static void au_call_lookup_hash(void *ar
}
static struct dentry *
-au_lkup_hash_dlgt(struct qstr *this, struct dentry *parent,
+au_lkup_hash_dlgt(const char *name, int len, struct dentry *parent,
struct nameidata *nd, unsigned int flags)
{
struct dentry *dentry;
int dirperm1;
+ struct qstr this;
+ int err = __lookup_one_len(name, &this, parent, len);
+ if (err)
+ return ERR_PTR(err);
dirperm1 = au_ftest_ndx(flags, DIRPERM1);
if (!dirperm1 && !au_ftest_ndx(flags, DLGT))
- dentry = vfsub__lookup_hash(this, parent, nd);
+ dentry = vfsub__lookup_hash(&this, parent, nd);
else {
int wkq_err;
struct au_lookup_hash_args args = {
.errp = &dentry,
- .name = this,
+ .name = &this,
.base = parent,
.nd = nd
};
@@ -296,10 +300,14 @@ au_lkup_hash_dlgt(struct qstr *this, str
}
#else
static struct dentry *
-au_lkup_hash_dlgt(struct qstr *this, struct dentry *parent,
+au_lkup_hash_dlgt(const char *name, int len, struct dentry *parent,
struct nameidata *nd, unsigned int flags)
{
- return vfsub__lookup_hash(this, parent, nd);
+ struct qstr this;
+ int err = __lookup_one_len(name, &this, parent, len);
+ if (err)
+ return ERR_PTR(err);
+ return vfsub__lookup_hash(&this, parent, nd);
}
#endif /* CONFIG_AUFS_DLGT */
@@ -307,17 +315,11 @@ struct dentry *au_lkup_hash(const char *
int len, struct au_ndx *ndx)
{
struct dentry *dentry;
- struct qstr this;
struct nameidata tmp_nd, *ndo;
int err;
LKTRTrace("%.*s/%.*s\n", AuDLNPair(parent), len, name);
- err = __lookup_one_len(name, &this, parent, len);
- dentry = ERR_PTR(err);
- if (err)
- goto out;
-
ndo = ndx->nd;
if (ndo) {
tmp_nd = *ndo;
@@ -331,7 +333,7 @@ struct dentry *au_lkup_hash(const char *
tmp_nd.path.dentry = parent;
tmp_nd.path.mnt = ndx->nfsmnt;
path_get(&tmp_nd.path);
- dentry = au_lkup_hash_dlgt(&this, parent, &tmp_nd, ndx->flags);
+ dentry = au_lkup_hash_dlgt(name, len, parent, &tmp_nd, ndx->flags);
if (!IS_ERR(dentry)) {
/* why negative dentry for a new dir was unhashed? */
if (unlikely(d_unhashed(dentry)))
@@ -345,10 +347,10 @@ struct dentry *au_lkup_hash(const char *
}
path_put(&tmp_nd.path);
- out_intent:
+out_intent:
if (tmp_nd.intent.open.file)
put_filp(tmp_nd.intent.open.file);
- out:
+
AuTraceErrPtr(dentry);
return dentry;
}
From: Jeff Mahoney <[EMAIL PROTECTED]>
Subject: aufs: Use release_open_intent instead of put_filp
release_open_intent is specifically for releasing open intents. Use
that instead of the internal put_filp. release_open_intent is internal, but
the case for it to be exported is much more easily made.
Signed-off-by: Jeff Mahoney <[EMAIL PROTECTED]>
---
fs/aufs25/br_nfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/fs/aufs25/br_nfs.c
+++ b/fs/aufs25/br_nfs.c
@@ -225,7 +225,7 @@ int au_hin_after_reval(struct nameidata
nd->intent.open.file = NULL;
}
if (unlikely(nd->intent.open.file))
- put_filp(nd->intent.open.file);
+ release_open_intent(nd);
}
return err;
@@ -349,7 +349,7 @@ struct dentry *au_lkup_hash(const char *
out_intent:
if (tmp_nd.intent.open.file)
- put_filp(tmp_nd.intent.open.file);
+ release_open_intent(&tmp_nd);
AuTraceErrPtr(dentry);
return dentry;
aufs-factor-out-__lookup_one_len aufs-push-lookup-down aufs-use-release_open_intent aufs-nfs4-is-special
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------- This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/
