* [PATCH 2/9] dcache: close d_move race in d_splice_alias
2014-02-18 20:28 ` [PATCH 1/9] dcache: move d_splice_alias J. Bruce Fields
@ 2014-02-18 20:28 ` J. Bruce Fields
2014-02-21 1:43 ` Christoph Hellwig
2014-02-18 20:28 ` [PATCH 3/9] dcache: d_splice_alias mustn't create directory aliases J. Bruce Fields
` (7 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: J. Bruce Fields @ 2014-02-18 20:28 UTC (permalink / raw)
To: linux-fsdevel
Cc: linux-btrfs, Josef Bacik, Eric W. Biederman, J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
d_splice_alias will d_move an IS_ROOT() directory dentry into place if
one exists. This should be safe as long as the dentry remains IS_ROOT,
but I can't see what guarantees that: once we drop the i_lock all we
hold here is the i_mutex on an unrelated parent directory.
Instead copy the logic of d_materialise_unique.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/dcache.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/fs/dcache.c b/fs/dcache.c
index 332b58c..fd50e52 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -2695,9 +2695,14 @@ struct dentry *d_splice_alias(struct inode *inode, struct dentry *dentry)
new = __d_find_alias(inode, 1);
if (new) {
BUG_ON(!(new->d_flags & DCACHE_DISCONNECTED));
+ write_seqlock(&rename_lock);
+ __d_materialise_dentry(dentry, new);
+ write_sequnlock(&rename_lock);
+ __d_drop(new);
+ _d_rehash(new);
+ spin_unlock(&new->d_lock);
spin_unlock(&inode->i_lock);
security_d_instantiate(new, inode);
- d_move(new, dentry);
iput(inode);
} else {
/* already taking inode->i_lock, so d_add() by hand */
--
1.8.5.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 2/9] dcache: close d_move race in d_splice_alias
2014-02-18 20:28 ` [PATCH 2/9] dcache: close d_move race in d_splice_alias J. Bruce Fields
@ 2014-02-21 1:43 ` Christoph Hellwig
0 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2014-02-21 1:43 UTC (permalink / raw)
To: J. Bruce Fields
Cc: linux-fsdevel, linux-btrfs, Josef Bacik, Eric W. Biederman
On Tue, Feb 18, 2014 at 03:28:58PM -0500, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
>
> d_splice_alias will d_move an IS_ROOT() directory dentry into place if
> one exists. This should be safe as long as the dentry remains IS_ROOT,
> but I can't see what guarantees that: once we drop the i_lock all we
> hold here is the i_mutex on an unrelated parent directory.
>
> Instead copy the logic of d_materialise_unique.
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/9] dcache: d_splice_alias mustn't create directory aliases
2014-02-18 20:28 ` [PATCH 1/9] dcache: move d_splice_alias J. Bruce Fields
2014-02-18 20:28 ` [PATCH 2/9] dcache: close d_move race in d_splice_alias J. Bruce Fields
@ 2014-02-18 20:28 ` J. Bruce Fields
2014-02-18 20:29 ` [PATCH 4/9] dcache: d_splice_alias should ignore DCACHE_DISCONNECTED J. Bruce Fields
` (6 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: J. Bruce Fields @ 2014-02-18 20:28 UTC (permalink / raw)
To: linux-fsdevel
Cc: linux-btrfs, Josef Bacik, Eric W. Biederman, J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
Currently if d_splice_alias finds a directory with an alias that is not
IS_ROOT or not DCACHE_DISCONNECTED, it creates a duplicate directory.
Duplicate directory dentries are unacceptable; it is better just to
error out.
(In the case of a local filesystem the most likely case is filesystem
corruption: for example, perhaps two directories point to the same child
directory, and the other parent has already been found and cached.)
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/dcache.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/fs/dcache.c b/fs/dcache.c
index fd50e52..4550227 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -2672,6 +2672,9 @@ static void __d_materialise_dentry(struct dentry *dentry, struct dentry *anon)
* DCACHE_DISCONNECTED), then d_move that in place of the given dentry
* and return it, else simply d_add the inode to the dentry and return NULL.
*
+ * If a non-IS_ROOT directory is found, the filesystem is corrupt, and
+ * we should error out: directories can't have multiple aliases.
+ *
* This is needed in the lookup routine of any filesystem that is exportable
* (via knfsd) so that we can build dcache paths to directories effectively.
*
@@ -2692,9 +2695,13 @@ struct dentry *d_splice_alias(struct inode *inode, struct dentry *dentry)
if (inode && S_ISDIR(inode->i_mode)) {
spin_lock(&inode->i_lock);
- new = __d_find_alias(inode, 1);
+ new = __d_find_any_alias(inode);
if (new) {
- BUG_ON(!(new->d_flags & DCACHE_DISCONNECTED));
+ if (!IS_ROOT(new) || !(new->d_flags & DCACHE_DISCONNECTED)) {
+ spin_unlock(&inode->i_lock);
+ dput(new);
+ return ERR_PTR(-EIO);
+ }
write_seqlock(&rename_lock);
__d_materialise_dentry(dentry, new);
write_sequnlock(&rename_lock);
--
1.8.5.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 4/9] dcache: d_splice_alias should ignore DCACHE_DISCONNECTED
2014-02-18 20:28 ` [PATCH 1/9] dcache: move d_splice_alias J. Bruce Fields
2014-02-18 20:28 ` [PATCH 2/9] dcache: close d_move race in d_splice_alias J. Bruce Fields
2014-02-18 20:28 ` [PATCH 3/9] dcache: d_splice_alias mustn't create directory aliases J. Bruce Fields
@ 2014-02-18 20:29 ` J. Bruce Fields
2014-02-18 20:29 ` [PATCH 5/9] dcache: d_obtain_alias callers don't all want DISCONNECTED J. Bruce Fields
` (5 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: J. Bruce Fields @ 2014-02-18 20:29 UTC (permalink / raw)
To: linux-fsdevel
Cc: linux-btrfs, Josef Bacik, Eric W. Biederman, J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
Any IS_ROOT() alias should be safe to use; there's nothing special about
DCACHE_DISCONNECTED dentries.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/dcache.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/dcache.c b/fs/dcache.c
index 4550227..e3e4618 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -2668,9 +2668,9 @@ static void __d_materialise_dentry(struct dentry *dentry, struct dentry *anon)
* @inode: the inode which may have a disconnected dentry
* @dentry: a negative dentry which we want to point to the inode.
*
- * If inode is a directory and has a 'disconnected' dentry (i.e. IS_ROOT and
- * DCACHE_DISCONNECTED), then d_move that in place of the given dentry
- * and return it, else simply d_add the inode to the dentry and return NULL.
+ * If inode is a directory and has an IS_ROOT alias, then d_move that in
+ * place of the given dentry and return it, else simply d_add the inode
+ * to the dentry and return NULL.
*
* If a non-IS_ROOT directory is found, the filesystem is corrupt, and
* we should error out: directories can't have multiple aliases.
@@ -2697,7 +2697,7 @@ struct dentry *d_splice_alias(struct inode *inode, struct dentry *dentry)
spin_lock(&inode->i_lock);
new = __d_find_any_alias(inode);
if (new) {
- if (!IS_ROOT(new) || !(new->d_flags & DCACHE_DISCONNECTED)) {
+ if (!IS_ROOT(new)) {
spin_unlock(&inode->i_lock);
dput(new);
return ERR_PTR(-EIO);
--
1.8.5.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 5/9] dcache: d_obtain_alias callers don't all want DISCONNECTED
2014-02-18 20:28 ` [PATCH 1/9] dcache: move d_splice_alias J. Bruce Fields
` (2 preceding siblings ...)
2014-02-18 20:29 ` [PATCH 4/9] dcache: d_splice_alias should ignore DCACHE_DISCONNECTED J. Bruce Fields
@ 2014-02-18 20:29 ` J. Bruce Fields
2014-02-21 1:44 ` Christoph Hellwig
2014-02-18 20:29 ` [PATCH 6/9] dcache: remove unused d_find_alias parameter J. Bruce Fields
` (4 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: J. Bruce Fields @ 2014-02-18 20:29 UTC (permalink / raw)
To: linux-fsdevel
Cc: linux-btrfs, Josef Bacik, Eric W. Biederman, J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
There are a few d_obtain_alias callers that are using it to get the
root of a filesystem which may already have an alias somewhere else.
This is not the same as the filehandle-lookup case, and none of them
actually need DCACHE_DISCONNECTED set.
In the btrfs case this was causing a spurious printk from
nfsd/nfsfh.c:fh_verify when it found an unexpected DCACHE_DISCONNECTED
dentry.
Which isn't really a serious problem, but it would really be clearer if
we reserved DCACHE_DISCONNECTED for those cases where it's actually
needed.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/btrfs/super.c | 2 +-
fs/ceph/super.c | 2 +-
fs/dcache.c | 69 +++++++++++++++++++++++++++++++++++---------------
fs/nfs/getroot.c | 2 +-
fs/nilfs2/super.c | 2 +-
include/linux/dcache.h | 1 +
6 files changed, 54 insertions(+), 24 deletions(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 97cc241..07ce96b 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -925,7 +925,7 @@ setup_root:
return dget(sb->s_root);
}
- return d_obtain_alias(inode);
+ return d_obtain_alias_root(inode);
}
static int btrfs_fill_super(struct super_block *sb,
diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 2df963f..9666950 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -732,7 +732,7 @@ static struct dentry *open_root_dentry(struct ceph_fs_client *fsc,
goto out;
}
} else {
- root = d_obtain_alias(inode);
+ root = d_obtain_alias_root(inode);
}
ceph_init_dentry(root);
dout("open_root_inode success, root dentry is %p\n", root);
diff --git a/fs/dcache.c b/fs/dcache.c
index e3e4618..3a1057a 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1832,25 +1832,7 @@ struct dentry *d_find_any_alias(struct inode *inode)
}
EXPORT_SYMBOL(d_find_any_alias);
-/**
- * d_obtain_alias - find or allocate a dentry for a given inode
- * @inode: inode to allocate the dentry for
- *
- * Obtain a dentry for an inode resulting from NFS filehandle conversion or
- * similar open by handle operations. The returned dentry may be anonymous,
- * or may have a full name (if the inode was already in the cache).
- *
- * When called on a directory inode, we must ensure that the inode only ever
- * has one dentry. If a dentry is found, that is returned instead of
- * allocating a new one.
- *
- * On successful return, the reference to the inode has been transferred
- * to the dentry. In case of an error the reference on the inode is released.
- * To make it easier to use in export operations a %NULL or IS_ERR inode may
- * be passed in and will be the error will be propagate to the return value,
- * with a %NULL @inode replaced by ERR_PTR(-ESTALE).
- */
-struct dentry *d_obtain_alias(struct inode *inode)
+struct dentry *__d_obtain_alias(struct inode *inode, int disconnected)
{
static const struct qstr anonstring = QSTR_INIT("/", 1);
struct dentry *tmp;
@@ -1881,7 +1863,10 @@ struct dentry *d_obtain_alias(struct inode *inode)
}
/* attach a disconnected dentry */
- add_flags = d_flags_for_inode(inode) | DCACHE_DISCONNECTED;
+ add_flags = d_flags_for_inode(inode);
+
+ if (disconnected)
+ add_flags |= DCACHE_DISCONNECTED;
spin_lock(&tmp->d_lock);
tmp->d_inode = inode;
@@ -1902,9 +1887,53 @@ struct dentry *d_obtain_alias(struct inode *inode)
iput(inode);
return res;
}
+
+/**
+ * d_obtain_alias - find or allocate a DISCONNECTED dentry for a given inode
+ * @inode: inode to allocate the dentry for
+ *
+ * Obtain a dentry for an inode resulting from NFS filehandle conversion or
+ * similar open by handle operations. The returned dentry may be anonymous,
+ * or may have a full name (if the inode was already in the cache).
+ *
+ * When called on a directory inode, we must ensure that the inode only ever
+ * has one dentry. If a dentry is found, that is returned instead of
+ * allocating a new one.
+ *
+ * On successful return, the reference to the inode has been transferred
+ * to the dentry. In case of an error the reference on the inode is released.
+ * To make it easier to use in export operations a %NULL or IS_ERR inode may
+ * be passed in and the error will be propagated to the return value,
+ * with a %NULL @inode replaced by ERR_PTR(-ESTALE).
+ */
+struct dentry *d_obtain_alias(struct inode *inode)
+{
+ return __d_obtain_alias(inode, 1);
+}
EXPORT_SYMBOL(d_obtain_alias);
/**
+ * d_obtain_alias_root - find or allocate a dentry for a given inode
+ * @inode: inode to allocate the dentry for
+ *
+ * Obtain an IS_ROOT dentry for the root of a filesystem.
+ *
+ * We must ensure that directory inodes only ever have one dentry. If a
+ * dentry is found, that is returned instead of allocating a new one.
+ *
+ * On successful return, the reference to the inode has been transferred
+ * to the dentry. In case of an error the reference on the inode is
+ * released. A %NULL or IS_ERR inode may be passed in and will be the
+ * error will be propagate to the return value, with a %NULL @inode
+ * replaced by ERR_PTR(-ESTALE).
+ */
+struct dentry *d_obtain_alias_root(struct inode *inode)
+{
+ return __d_obtain_alias(inode, 0);
+}
+EXPORT_SYMBOL(d_obtain_alias_root);
+
+/**
* d_add_ci - lookup or allocate new dentry with case-exact name
* @inode: the inode case-insensitive lookup has found
* @dentry: the negative dentry that was passed to the parent's lookup func
diff --git a/fs/nfs/getroot.c b/fs/nfs/getroot.c
index 66984a9..7b32522 100644
--- a/fs/nfs/getroot.c
+++ b/fs/nfs/getroot.c
@@ -112,7 +112,7 @@ struct dentry *nfs_get_root(struct super_block *sb, struct nfs_fh *mntfh,
* if the dentry tree reaches them; however if the dentry already
* exists, we'll pick it up at this point and use it as the root
*/
- ret = d_obtain_alias(inode);
+ ret = d_obtain_alias_root(inode);
if (IS_ERR(ret)) {
dprintk("nfs_get_root: get root dentry failed\n");
goto out;
diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
index 7ac2a12..a518a73 100644
--- a/fs/nilfs2/super.c
+++ b/fs/nilfs2/super.c
@@ -942,7 +942,7 @@ static int nilfs_get_root_dentry(struct super_block *sb,
iput(inode);
}
} else {
- dentry = d_obtain_alias(inode);
+ dentry = d_obtain_alias_root(inode);
if (IS_ERR(dentry)) {
ret = PTR_ERR(dentry);
goto failed_dentry;
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index bf72e9a..2973aab 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -247,6 +247,7 @@ extern struct dentry * d_splice_alias(struct inode *, struct dentry *);
extern struct dentry * d_add_ci(struct dentry *, struct inode *, struct qstr *);
extern struct dentry *d_find_any_alias(struct inode *inode);
extern struct dentry * d_obtain_alias(struct inode *);
+extern struct dentry * d_obtain_alias_root(struct inode *);
extern void shrink_dcache_sb(struct super_block *);
extern void shrink_dcache_parent(struct dentry *);
extern void shrink_dcache_for_umount(struct super_block *);
--
1.8.5.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 5/9] dcache: d_obtain_alias callers don't all want DISCONNECTED
2014-02-18 20:29 ` [PATCH 5/9] dcache: d_obtain_alias callers don't all want DISCONNECTED J. Bruce Fields
@ 2014-02-21 1:44 ` Christoph Hellwig
2014-02-24 20:23 ` J. Bruce Fields
0 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2014-02-21 1:44 UTC (permalink / raw)
To: J. Bruce Fields
Cc: linux-fsdevel, linux-btrfs, Josef Bacik, Eric W. Biederman
>
> - return d_obtain_alias(inode);
> + return d_obtain_alias_root(inode);
Can we call this d_obtain_root or similar, please?
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 5/9] dcache: d_obtain_alias callers don't all want DISCONNECTED
2014-02-21 1:44 ` Christoph Hellwig
@ 2014-02-24 20:23 ` J. Bruce Fields
0 siblings, 0 replies; 15+ messages in thread
From: J. Bruce Fields @ 2014-02-24 20:23 UTC (permalink / raw)
To: Christoph Hellwig
Cc: J. Bruce Fields, linux-fsdevel, linux-btrfs, Josef Bacik,
Eric W. Biederman
On Thu, Feb 20, 2014 at 05:44:14PM -0800, Christoph Hellwig wrote:
> >
> > - return d_obtain_alias(inode);
> > + return d_obtain_alias_root(inode);
>
> Can we call this d_obtain_root or similar, please?
Yes, I like d_obtain_root better, done.
I'll send out the updated series sometime.
--b.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 6/9] dcache: remove unused d_find_alias parameter
2014-02-18 20:28 ` [PATCH 1/9] dcache: move d_splice_alias J. Bruce Fields
` (3 preceding siblings ...)
2014-02-18 20:29 ` [PATCH 5/9] dcache: d_obtain_alias callers don't all want DISCONNECTED J. Bruce Fields
@ 2014-02-18 20:29 ` J. Bruce Fields
2014-02-18 20:29 ` [PATCH 7/9] dcache: d_find_alias needn't recheck IS_ROOT && DCACHE_DISCONNECTED J. Bruce Fields
` (3 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: J. Bruce Fields @ 2014-02-18 20:29 UTC (permalink / raw)
To: linux-fsdevel
Cc: linux-btrfs, Josef Bacik, Eric W. Biederman, J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/dcache.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/fs/dcache.c b/fs/dcache.c
index 3a1057a..efe3d3b 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -719,8 +719,6 @@ EXPORT_SYMBOL(dget_parent);
/**
* d_find_alias - grab a hashed alias of inode
* @inode: inode in question
- * @want_discon: flag, used by d_splice_alias, to request
- * that only a DISCONNECTED alias be returned.
*
* If inode has a hashed alias, or is a directory and has any alias,
* acquire the reference to alias and return it. Otherwise return NULL.
@@ -729,10 +727,9 @@ EXPORT_SYMBOL(dget_parent);
* of a filesystem.
*
* If the inode has an IS_ROOT, DCACHE_DISCONNECTED alias, then prefer
- * any other hashed alias over that one unless @want_discon is set,
- * in which case only return an IS_ROOT, DCACHE_DISCONNECTED alias.
+ * any other hashed alias over that one.
*/
-static struct dentry *__d_find_alias(struct inode *inode, int want_discon)
+static struct dentry *__d_find_alias(struct inode *inode)
{
struct dentry *alias, *discon_alias;
@@ -744,7 +741,7 @@ again:
if (IS_ROOT(alias) &&
(alias->d_flags & DCACHE_DISCONNECTED)) {
discon_alias = alias;
- } else if (!want_discon) {
+ } else {
__dget_dlock(alias);
spin_unlock(&alias->d_lock);
return alias;
@@ -775,7 +772,7 @@ struct dentry *d_find_alias(struct inode *inode)
if (!hlist_empty(&inode->i_dentry)) {
spin_lock(&inode->i_lock);
- de = __d_find_alias(inode, 0);
+ de = __d_find_alias(inode);
spin_unlock(&inode->i_lock);
}
return de;
@@ -2784,7 +2781,7 @@ struct dentry *d_materialise_unique(struct dentry *dentry, struct inode *inode)
struct dentry *alias;
/* Does an aliased dentry already exist? */
- alias = __d_find_alias(inode, 0);
+ alias = __d_find_alias(inode);
if (alias) {
actual = alias;
write_seqlock(&rename_lock);
--
1.8.5.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 7/9] dcache: d_find_alias needn't recheck IS_ROOT && DCACHE_DISCONNECTED
2014-02-18 20:28 ` [PATCH 1/9] dcache: move d_splice_alias J. Bruce Fields
` (4 preceding siblings ...)
2014-02-18 20:29 ` [PATCH 6/9] dcache: remove unused d_find_alias parameter J. Bruce Fields
@ 2014-02-18 20:29 ` J. Bruce Fields
2014-02-18 20:29 ` [PATCH 8/9] exportfs: update Exporting documentation J. Bruce Fields
` (2 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: J. Bruce Fields @ 2014-02-18 20:29 UTC (permalink / raw)
To: linux-fsdevel
Cc: linux-btrfs, Josef Bacik, Eric W. Biederman, J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
If we get to this point and discover the dentry is not a root dentry, or
not DCACHE_DISCONNECTED--great, we always prefer that anyway.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/dcache.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/fs/dcache.c b/fs/dcache.c
index efe3d3b..448ef98 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -753,12 +753,9 @@ again:
alias = discon_alias;
spin_lock(&alias->d_lock);
if (S_ISDIR(inode->i_mode) || !d_unhashed(alias)) {
- if (IS_ROOT(alias) &&
- (alias->d_flags & DCACHE_DISCONNECTED)) {
- __dget_dlock(alias);
- spin_unlock(&alias->d_lock);
- return alias;
- }
+ __dget_dlock(alias);
+ spin_unlock(&alias->d_lock);
+ return alias;
}
spin_unlock(&alias->d_lock);
goto again;
--
1.8.5.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 8/9] exportfs: update Exporting documentation
2014-02-18 20:28 ` [PATCH 1/9] dcache: move d_splice_alias J. Bruce Fields
` (5 preceding siblings ...)
2014-02-18 20:29 ` [PATCH 7/9] dcache: d_find_alias needn't recheck IS_ROOT && DCACHE_DISCONNECTED J. Bruce Fields
@ 2014-02-18 20:29 ` J. Bruce Fields
2014-02-18 20:29 ` [PATCH 9/9] dcache: rename DCACHE_DISCONNECTED -> DCACHE_CONNECTING J. Bruce Fields
2014-02-21 1:42 ` [PATCH 1/9] dcache: move d_splice_alias Christoph Hellwig
8 siblings, 0 replies; 15+ messages in thread
From: J. Bruce Fields @ 2014-02-18 20:29 UTC (permalink / raw)
To: linux-fsdevel
Cc: linux-btrfs, Josef Bacik, Eric W. Biederman, J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
Minor documentation updates:
- refer to d_obtain_alias rather than d_alloc_anon
- explain when to use d_splice_alias and when
d_materialise_unique.
- cut some details of d_splice_alias/d_materialise_unique
implementation.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
Documentation/filesystems/nfs/Exporting | 38 ++++++++++++++++++++-------------
1 file changed, 23 insertions(+), 15 deletions(-)
diff --git a/Documentation/filesystems/nfs/Exporting b/Documentation/filesystems/nfs/Exporting
index e543b1a..9b7de5b 100644
--- a/Documentation/filesystems/nfs/Exporting
+++ b/Documentation/filesystems/nfs/Exporting
@@ -66,23 +66,31 @@ b/ A per-superblock list "s_anon" of dentries which are the roots of
c/ Helper routines to allocate anonymous dentries, and to help attach
loose directory dentries at lookup time. They are:
- d_alloc_anon(inode) will return a dentry for the given inode.
+ d_obtain_alias(inode) will return a dentry for the given inode.
If the inode already has a dentry, one of those is returned.
If it doesn't, a new anonymous (IS_ROOT and
DCACHE_DISCONNECTED) dentry is allocated and attached.
In the case of a directory, care is taken that only one dentry
can ever be attached.
- d_splice_alias(inode, dentry) will make sure that there is a
- dentry with the same name and parent as the given dentry, and
- which refers to the given inode.
- If the inode is a directory and already has a dentry, then that
- dentry is d_moved over the given dentry.
- If the passed dentry gets attached, care is taken that this is
- mutually exclusive to a d_alloc_anon operation.
- If the passed dentry is used, NULL is returned, else the used
- dentry is returned. This corresponds to the calling pattern of
- ->lookup.
-
+ d_splice_alias(inode, dentry) or d_materialise_unique(dentry, inode)
+ will introduce a new dentry into the tree; either the passed-in
+ dentry or a preexisting alias for the given inode (such as an
+ anonymous one created by d_obtain_alias), if appropriate. The two
+ functions differ in their handling of directories with preexisting
+ aliases:
+ d_splice_alias will use any existing IS_ROOT dentry, but it will
+ return -EIO rather than try to move a dentry with a different
+ parent. This is appropriate for local filesystems, which
+ should never see such an alias unless the filesystem is
+ corrupted somehow (for example, if two on-disk directory
+ entries refer to the same directory.)
+ d_obtain_alias will attempt to move any dentry. This is
+ appropriate for distributed filesystems, where finding a
+ directory other than where we last cached it may be a normal
+ consequence of concurrent operations on other hosts.
+ Both functions return NULL when the passed-in dentry is used,
+ following the calling convention of ->lookup.
+
Filesystem Issues
-----------------
@@ -120,12 +128,12 @@ struct which has the following members:
fh_to_dentry (mandatory)
Given a filehandle fragment, this should find the implied object and
- create a dentry for it (possibly with d_alloc_anon).
+ create a dentry for it (possibly with d_obtain_alias).
fh_to_parent (optional but strongly recommended)
Given a filehandle fragment, this should find the parent of the
- implied object and create a dentry for it (possibly with d_alloc_anon).
- May fail if the filehandle fragment is too small.
+ implied object and create a dentry for it (possibly with
+ d_obtain_alias). May fail if the filehandle fragment is too small.
get_parent (optional but strongly recommended)
When given a dentry for a directory, this should return a dentry for
--
1.8.5.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 9/9] dcache: rename DCACHE_DISCONNECTED -> DCACHE_CONNECTING
2014-02-18 20:28 ` [PATCH 1/9] dcache: move d_splice_alias J. Bruce Fields
` (6 preceding siblings ...)
2014-02-18 20:29 ` [PATCH 8/9] exportfs: update Exporting documentation J. Bruce Fields
@ 2014-02-18 20:29 ` J. Bruce Fields
2014-02-21 1:42 ` [PATCH 1/9] dcache: move d_splice_alias Christoph Hellwig
8 siblings, 0 replies; 15+ messages in thread
From: J. Bruce Fields @ 2014-02-18 20:29 UTC (permalink / raw)
To: linux-fsdevel
Cc: linux-btrfs, Josef Bacik, Eric W. Biederman, J. Bruce Fields
From: "J. Bruce Fields" <bfields@redhat.com>
The DCACHE_DISCONNECTED flag was intended *only* to mark dentries which
were looked up by filehandle and are currently in the process of being
hooked up to the rest of dcache.
Over time people have also confused it with IS_ROOT, using it to mark
directories that are permanently "disconnected" just because for example
they're the root of some filesystem.
Perhaps a different name would have helped. (Thanks to Eric Biederman
for the suggestion of DCACHE_CONNECTING.)
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
Documentation/filesystems/nfs/Exporting | 4 ++--
drivers/staging/lustre/lustre/llite/namei.c | 4 ++--
fs/dcache.c | 8 ++++----
fs/exportfs/expfs.c | 16 ++++++++--------
fs/fat/namei_vfat.c | 4 ++--
fs/nfsd/nfsfh.c | 4 ++--
fs/ocfs2/dcache.c | 2 +-
fs/ocfs2/namei.c | 2 +-
include/linux/dcache.h | 4 ++--
9 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/Documentation/filesystems/nfs/Exporting b/Documentation/filesystems/nfs/Exporting
index 9b7de5b..6d3b272 100644
--- a/Documentation/filesystems/nfs/Exporting
+++ b/Documentation/filesystems/nfs/Exporting
@@ -52,7 +52,7 @@ the dcache that are not needed for normal filesystem access.
To implement these features, the dcache has:
-a/ A dentry flag DCACHE_DISCONNECTED which is set on
+a/ A dentry flag DCACHE_CONNECTING which is set on
any dentry that might not be part of the proper prefix.
This is set when anonymous dentries are created, and cleared when a
dentry is noticed to be a child of a dentry which is in the proper
@@ -69,7 +69,7 @@ c/ Helper routines to allocate anonymous dentries, and to help attach
d_obtain_alias(inode) will return a dentry for the given inode.
If the inode already has a dentry, one of those is returned.
If it doesn't, a new anonymous (IS_ROOT and
- DCACHE_DISCONNECTED) dentry is allocated and attached.
+ DCACHE_CONNECTING) dentry is allocated and attached.
In the case of a directory, care is taken that only one dentry
can ever be attached.
d_splice_alias(inode, dentry) or d_materialise_unique(dentry, inode)
diff --git a/drivers/staging/lustre/lustre/llite/namei.c b/drivers/staging/lustre/lustre/llite/namei.c
index fc8d264..f271111 100644
--- a/drivers/staging/lustre/lustre/llite/namei.c
+++ b/drivers/staging/lustre/lustre/llite/namei.c
@@ -368,7 +368,7 @@ static struct dentry *ll_find_alias(struct inode *inode, struct dentry *dentry)
LASSERT(alias != dentry);
spin_lock(&alias->d_lock);
- if (alias->d_flags & DCACHE_DISCONNECTED)
+ if (alias->d_flags & DCACHE_CONNECTING)
/* LASSERT(last_discon == NULL); LU-405, bz 20055 */
discon_alias = alias;
else if (alias->d_parent == dentry->d_parent &&
@@ -395,7 +395,7 @@ static struct dentry *ll_find_alias(struct inode *inode, struct dentry *dentry)
/*
* Similar to d_splice_alias(), but lustre treats invalid alias
- * similar to DCACHE_DISCONNECTED, and tries to use it anyway.
+ * similar to DCACHE_CONNECTING, and tries to use it anyway.
*/
struct dentry *ll_splice_alias(struct inode *inode, struct dentry *de)
{
diff --git a/fs/dcache.c b/fs/dcache.c
index 448ef98..0d5e1a5 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -726,7 +726,7 @@ EXPORT_SYMBOL(dget_parent);
* it can be unhashed only if it has no children, or if it is the root
* of a filesystem.
*
- * If the inode has an IS_ROOT, DCACHE_DISCONNECTED alias, then prefer
+ * If the inode has an IS_ROOT, DCACHE_CONNECTING alias, then prefer
* any other hashed alias over that one.
*/
static struct dentry *__d_find_alias(struct inode *inode)
@@ -739,7 +739,7 @@ again:
spin_lock(&alias->d_lock);
if (S_ISDIR(inode->i_mode) || !d_unhashed(alias)) {
if (IS_ROOT(alias) &&
- (alias->d_flags & DCACHE_DISCONNECTED)) {
+ (alias->d_flags & DCACHE_CONNECTING)) {
discon_alias = alias;
} else {
__dget_dlock(alias);
@@ -1860,7 +1860,7 @@ struct dentry *__d_obtain_alias(struct inode *inode, int disconnected)
add_flags = d_flags_for_inode(inode);
if (disconnected)
- add_flags |= DCACHE_DISCONNECTED;
+ add_flags |= DCACHE_CONNECTING;
spin_lock(&tmp->d_lock);
tmp->d_inode = inode;
@@ -1883,7 +1883,7 @@ struct dentry *__d_obtain_alias(struct inode *inode, int disconnected)
}
/**
- * d_obtain_alias - find or allocate a DISCONNECTED dentry for a given inode
+ * d_obtain_alias - find or allocate a CONNECTING dentry for a given inode
* @inode: inode to allocate the dentry for
*
* Obtain a dentry for an inode resulting from NFS filehandle conversion or
diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index 48a359d..50c6973 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -72,7 +72,7 @@ find_acceptable_alias(struct dentry *result,
static bool dentry_connected(struct dentry *dentry)
{
dget(dentry);
- while (dentry->d_flags & DCACHE_DISCONNECTED) {
+ while (dentry->d_flags & DCACHE_CONNECTING) {
struct dentry *parent = dget_parent(dentry);
dput(dentry);
@@ -89,13 +89,13 @@ static bool dentry_connected(struct dentry *dentry)
static void clear_disconnected(struct dentry *dentry)
{
dget(dentry);
- while (dentry->d_flags & DCACHE_DISCONNECTED) {
+ while (dentry->d_flags & DCACHE_CONNECTING) {
struct dentry *parent = dget_parent(dentry);
WARN_ON_ONCE(IS_ROOT(dentry));
spin_lock(&dentry->d_lock);
- dentry->d_flags &= ~DCACHE_DISCONNECTED;
+ dentry->d_flags &= ~DCACHE_CONNECTING;
spin_unlock(&dentry->d_lock);
dput(dentry);
@@ -187,12 +187,12 @@ out_reconnected:
/*
* Make sure target_dir is fully connected to the dentry tree.
*
- * On successful return, DCACHE_DISCONNECTED will be cleared on
+ * On successful return, DCACHE_CONNECTING will be cleared on
* target_dir, and target_dir->d_parent->...->d_parent will reach the
* root of the filesystem.
*
- * Whenever DCACHE_DISCONNECTED is unset, target_dir is fully connected.
- * But the converse is not true: target_dir may have DCACHE_DISCONNECTED
+ * Whenever DCACHE_CONNECTING is unset, target_dir is fully connected.
+ * But the converse is not true: target_dir may have DCACHE_CONNECTING
* set but already be connected. In that case we'll verify the
* connection to root and then clear the flag.
*
@@ -208,7 +208,7 @@ reconnect_path(struct vfsmount *mnt, struct dentry *target_dir, char *nbuf)
dentry = dget(target_dir);
- while (dentry->d_flags & DCACHE_DISCONNECTED) {
+ while (dentry->d_flags & DCACHE_CONNECTING) {
BUG_ON(dentry == mnt->mnt_sb->s_root);
if (IS_ROOT(dentry))
@@ -437,7 +437,7 @@ struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
* to ensure our dentry is connected all the way up to the
* filesystem root.
*/
- if (result->d_flags & DCACHE_DISCONNECTED) {
+ if (result->d_flags & DCACHE_CONNECTING) {
err = reconnect_path(mnt, result, nbuf);
if (err)
goto err_result;
diff --git a/fs/fat/namei_vfat.c b/fs/fat/namei_vfat.c
index 6df8d3d..3e450fc 100644
--- a/fs/fat/namei_vfat.c
+++ b/fs/fat/namei_vfat.c
@@ -705,7 +705,7 @@ static int vfat_find(struct inode *dir, struct qstr *qname,
*/
static int vfat_d_anon_disconn(struct dentry *dentry)
{
- return IS_ROOT(dentry) && (dentry->d_flags & DCACHE_DISCONNECTED);
+ return IS_ROOT(dentry) && (dentry->d_flags & DCACHE_CONNECTING);
}
static struct dentry *vfat_lookup(struct inode *dir, struct dentry *dentry,
@@ -738,7 +738,7 @@ static struct dentry *vfat_lookup(struct inode *dir, struct dentry *dentry,
alias = d_find_alias(inode);
if (alias && !vfat_d_anon_disconn(alias)) {
/*
- * This inode has non anonymous-DCACHE_DISCONNECTED
+ * This inode has non anonymous-DCACHE_CONNECTING
* dentry. This means, the user did ->lookup() by an
* another name (longname vs 8.3 alias of it) in past.
*
diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index 3c37b16..0bc18ae 100644
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c
@@ -252,8 +252,8 @@ static __be32 nfsd_set_fh_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp)
}
if (S_ISDIR(dentry->d_inode->i_mode) &&
- (dentry->d_flags & DCACHE_DISCONNECTED)) {
- printk("nfsd: find_fh_dentry returned a DISCONNECTED directory: %pd2\n",
+ (dentry->d_flags & DCACHE_CONNECTING)) {
+ printk("nfsd: find_fh_dentry returned a CONNECTING directory: %pd2\n",
dentry);
}
diff --git a/fs/ocfs2/dcache.c b/fs/ocfs2/dcache.c
index 0d3a97d..3e954a1 100644
--- a/fs/ocfs2/dcache.c
+++ b/fs/ocfs2/dcache.c
@@ -455,7 +455,7 @@ static void ocfs2_dentry_iput(struct dentry *dentry, struct inode *inode)
* No dentry lock is ok if we're disconnected or
* unhashed.
*/
- if (!(dentry->d_flags & DCACHE_DISCONNECTED) &&
+ if (!(dentry->d_flags & DCACHE_CONNECTING) &&
!d_unhashed(dentry)) {
unsigned long long ino = 0ULL;
if (inode)
diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
index f4d609b..a753ae5 100644
--- a/fs/ocfs2/namei.c
+++ b/fs/ocfs2/namei.c
@@ -149,7 +149,7 @@ bail_add:
if (inode) {
/*
- * If d_splice_alias() finds a DCACHE_DISCONNECTED
+ * If d_splice_alias() finds a DCACHE_CONNECTING
* dentry, it will d_move() it on top of ourse. The
* return value will indicate this however, so in
* those cases, we switch them around for the locking
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index 2973aab..9f22329 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -177,14 +177,14 @@ struct dentry_operations {
#define DCACHE_OP_DELETE 0x00000008
#define DCACHE_OP_PRUNE 0x00000010
-#define DCACHE_DISCONNECTED 0x00000020
+#define DCACHE_CONNECTING 0x00000020
/* This dentry is possibly not currently connected to the dcache tree, in
* which case its parent will either be itself, or will have this flag as
* well. nfsd will not use a dentry with this bit set, but will first
* endeavour to clear the bit either by discovering that it is connected,
* or by performing lookup operations. Any filesystem which supports
* nfsd_operations MUST have a lookup function which, if it finds a
- * directory inode with a DCACHE_DISCONNECTED dentry, will d_move that
+ * directory inode with a DCACHE_CONNECTING dentry, will d_move that
* dentry into place and return that dentry rather than the passed one,
* typically using d_splice_alias. */
--
1.8.5.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 1/9] dcache: move d_splice_alias
2014-02-18 20:28 ` [PATCH 1/9] dcache: move d_splice_alias J. Bruce Fields
` (7 preceding siblings ...)
2014-02-18 20:29 ` [PATCH 9/9] dcache: rename DCACHE_DISCONNECTED -> DCACHE_CONNECTING J. Bruce Fields
@ 2014-02-21 1:42 ` Christoph Hellwig
8 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2014-02-21 1:42 UTC (permalink / raw)
To: J. Bruce Fields
Cc: linux-fsdevel, linux-btrfs, Josef Bacik, Eric W. Biederman
On Tue, Feb 18, 2014 at 03:28:57PM -0500, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
>
> Just a trivial move to locate it near (similar) d_materialise_unique
> code and save some forward references in a following patch.
>
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 15+ messages in thread