* [PATCH 01/10] [PATCH] fs: take dcache_lock inside __d_path
2010-10-10 9:36 [PATCH 00/10] dcache cleanups Christoph Hellwig
@ 2010-10-10 9:36 ` Christoph Hellwig
2010-10-10 9:36 ` [PATCH 02/10] fs: simplify __d_free Christoph Hellwig
` (10 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2010-10-10 9:36 UTC (permalink / raw)
To: viro; +Cc: eparis, linux-fsdevel
[-- Attachment #1: take-dcache_lock-in-__d_path --]
[-- Type: text/plain, Size: 2855 bytes --]
All callers take dcache_lock just around the call to __d_path, so
take the lock into it in preparation of getting rid of dcache_lock.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/fs/dcache.c
===================================================================
--- linux-2.6.orig/fs/dcache.c 2010-10-09 22:59:55.482254091 +0200
+++ linux-2.6/fs/dcache.c 2010-10-10 09:36:07.338004055 +0200
@@ -1994,7 +1994,7 @@ global_root:
* Returns a pointer into the buffer or an error code if the
* path was too long.
*
- * "buflen" should be positive. Caller holds the dcache_lock.
+ * "buflen" should be positive.
*
* If path is not reachable from the supplied root, then the value of
* root is changed (without modifying refcounts).
@@ -2006,10 +2006,12 @@ char *__d_path(const struct path *path,
int error;
prepend(&res, &buflen, "\0", 1);
+ spin_lock(&dcache_lock);
error = prepend_path(path, root, &res, &buflen);
+ spin_unlock(&dcache_lock);
+
if (error)
return ERR_PTR(error);
-
return res;
}
Index: linux-2.6/fs/seq_file.c
===================================================================
--- linux-2.6.orig/fs/seq_file.c 2010-10-09 22:59:55.490010272 +0200
+++ linux-2.6/fs/seq_file.c 2010-10-09 23:00:38.952004057 +0200
@@ -462,9 +462,7 @@ int seq_path_root(struct seq_file *m, st
if (size) {
char *p;
- spin_lock(&dcache_lock);
p = __d_path(path, root, buf, size);
- spin_unlock(&dcache_lock);
res = PTR_ERR(p);
if (!IS_ERR(p)) {
char *end = mangle_path(buf, p, esc);
Index: linux-2.6/security/apparmor/path.c
===================================================================
--- linux-2.6.orig/security/apparmor/path.c 2010-10-09 22:59:55.500012996 +0200
+++ linux-2.6/security/apparmor/path.c 2010-10-09 23:00:38.958004057 +0200
@@ -72,10 +72,8 @@ static int d_namespace_path(struct path
path_get(&root);
}
- spin_lock(&dcache_lock);
tmp = root;
res = __d_path(path, &tmp, buf, buflen);
- spin_unlock(&dcache_lock);
*name = res;
/* handle error conditions - and still allow a partial path to
Index: linux-2.6/security/tomoyo/realpath.c
===================================================================
--- linux-2.6.orig/security/tomoyo/realpath.c 2010-10-09 22:59:55.518025777 +0200
+++ linux-2.6/security/tomoyo/realpath.c 2010-10-09 23:00:38.961023403 +0200
@@ -127,10 +127,8 @@ char *tomoyo_realpath_from_path(struct p
/* If we don't have a vfsmount, we can't calculate. */
if (!path->mnt)
break;
- spin_lock(&dcache_lock);
/* go to whatever namespace root we are under */
pos = __d_path(path, &ns_root, buf, buf_len);
- spin_unlock(&dcache_lock);
/* Prepend "/proc" prefix if using internal proc vfs mount. */
if (!IS_ERR(pos) && (path->mnt->mnt_flags & MNT_INTERNAL) &&
(path->mnt->mnt_sb->s_magic == PROC_SUPER_MAGIC)) {
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 02/10] fs: simplify __d_free
2010-10-10 9:36 [PATCH 00/10] dcache cleanups Christoph Hellwig
2010-10-10 9:36 ` [PATCH 01/10] [PATCH] fs: take dcache_lock inside __d_path Christoph Hellwig
@ 2010-10-10 9:36 ` Christoph Hellwig
2010-10-10 9:36 ` [PATCH 03/10] fs: use percpu counter for nr_dentry and nr_dentry_unused Christoph Hellwig
` (9 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2010-10-10 9:36 UTC (permalink / raw)
To: viro; +Cc: eparis, linux-fsdevel
[-- Attachment #1: cleanup-d_free --]
[-- Type: text/plain, Size: 1348 bytes --]
Remove d_callback and always call __d_free with a RCU head.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/fs/dcache.c
===================================================================
--- linux-2.6.orig/fs/dcache.c 2010-10-09 10:20:46.650004474 +0200
+++ linux-2.6/fs/dcache.c 2010-10-09 10:24:36.806004683 +0200
@@ -67,20 +67,16 @@ struct dentry_stat_t dentry_stat = {
.age_limit = 45,
};
-static void __d_free(struct dentry *dentry)
+static void __d_free(struct rcu_head *head)
{
+ struct dentry *dentry = container_of(head, struct dentry, d_u.d_rcu);
+
WARN_ON(!list_empty(&dentry->d_alias));
if (dname_external(dentry))
kfree(dentry->d_name.name);
kmem_cache_free(dentry_cache, dentry);
}
-static void d_callback(struct rcu_head *head)
-{
- struct dentry * dentry = container_of(head, struct dentry, d_u.d_rcu);
- __d_free(dentry);
-}
-
/*
* no dcache_lock, please. The caller must decrement dentry_stat.nr_dentry
* inside dcache_lock.
@@ -91,9 +87,9 @@ static void d_free(struct dentry *dentry
dentry->d_op->d_release(dentry);
/* if dentry was never inserted into hash, immediate free is OK */
if (hlist_unhashed(&dentry->d_hash))
- __d_free(dentry);
+ __d_free(&dentry->d_u.d_rcu);
else
- call_rcu(&dentry->d_u.d_rcu, d_callback);
+ call_rcu(&dentry->d_u.d_rcu, __d_free);
}
/*
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 03/10] fs: use percpu counter for nr_dentry and nr_dentry_unused
2010-10-10 9:36 [PATCH 00/10] dcache cleanups Christoph Hellwig
2010-10-10 9:36 ` [PATCH 01/10] [PATCH] fs: take dcache_lock inside __d_path Christoph Hellwig
2010-10-10 9:36 ` [PATCH 02/10] fs: simplify __d_free Christoph Hellwig
@ 2010-10-10 9:36 ` Christoph Hellwig
2010-10-10 9:36 ` [PATCH 04/10] fs: improve DCACHE_REFERENCED usage Christoph Hellwig
` (8 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2010-10-10 9:36 UTC (permalink / raw)
To: viro; +Cc: eparis, linux-fsdevel
[-- Attachment #1: nr_dentries-percpu --]
[-- Type: text/plain, Size: 6489 bytes --]
The nr_dentry stat is a globally touched cacheline and atomic operation
twice over the lifetime of a dentry. It is used for the benfit of userspace
only. Turn it into a per-cpu counter and always decrement it in d_free instead
of doing various batching operations to reduce lock hold times in the callers.
Based on an earlier patch from Nick Piggin <npiggin@suse.de>.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/fs/dcache.c
===================================================================
--- linux-2.6.orig/fs/dcache.c 2010-10-10 09:36:28.918003777 +0200
+++ linux-2.6/fs/dcache.c 2010-10-10 09:36:31.325003987 +0200
@@ -67,6 +67,19 @@ struct dentry_stat_t dentry_stat = {
.age_limit = 45,
};
+static struct percpu_counter nr_dentry __cacheline_aligned_in_smp;
+static struct percpu_counter nr_dentry_unused __cacheline_aligned_in_smp;
+
+#if defined(CONFIG_SYSCTL) && defined(CONFIG_PROC_FS)
+int proc_nr_dentry(ctl_table *table, int write, void __user *buffer,
+ size_t *lenp, loff_t *ppos)
+{
+ dentry_stat.nr_dentry = percpu_counter_sum_positive(&nr_dentry);
+ dentry_stat.nr_unused = percpu_counter_sum_positive(&nr_dentry_unused);
+ return proc_dointvec(table, write, buffer, lenp, ppos);
+}
+#endif
+
static void __d_free(struct rcu_head *head)
{
struct dentry *dentry = container_of(head, struct dentry, d_u.d_rcu);
@@ -78,13 +91,14 @@ static void __d_free(struct rcu_head *he
}
/*
- * no dcache_lock, please. The caller must decrement dentry_stat.nr_dentry
- * inside dcache_lock.
+ * no dcache_lock, please.
*/
static void d_free(struct dentry *dentry)
{
+ percpu_counter_dec(&nr_dentry);
if (dentry->d_op && dentry->d_op->d_release)
dentry->d_op->d_release(dentry);
+
/* if dentry was never inserted into hash, immediate free is OK */
if (hlist_unhashed(&dentry->d_hash))
__d_free(&dentry->d_u.d_rcu);
@@ -125,14 +139,14 @@ static void dentry_lru_add(struct dentry
{
list_add(&dentry->d_lru, &dentry->d_sb->s_dentry_lru);
dentry->d_sb->s_nr_dentry_unused++;
- dentry_stat.nr_unused++;
+ percpu_counter_inc(&nr_dentry_unused);
}
static void dentry_lru_add_tail(struct dentry *dentry)
{
list_add_tail(&dentry->d_lru, &dentry->d_sb->s_dentry_lru);
dentry->d_sb->s_nr_dentry_unused++;
- dentry_stat.nr_unused++;
+ percpu_counter_inc(&nr_dentry_unused);
}
static void dentry_lru_del(struct dentry *dentry)
@@ -140,7 +154,7 @@ static void dentry_lru_del(struct dentry
if (!list_empty(&dentry->d_lru)) {
list_del(&dentry->d_lru);
dentry->d_sb->s_nr_dentry_unused--;
- dentry_stat.nr_unused--;
+ percpu_counter_dec(&nr_dentry_unused);
}
}
@@ -149,7 +163,7 @@ static void dentry_lru_del_init(struct d
if (likely(!list_empty(&dentry->d_lru))) {
list_del_init(&dentry->d_lru);
dentry->d_sb->s_nr_dentry_unused--;
- dentry_stat.nr_unused--;
+ percpu_counter_dec(&nr_dentry_unused);
}
}
@@ -168,7 +182,6 @@ static struct dentry *d_kill(struct dent
struct dentry *parent;
list_del(&dentry->d_u.d_child);
- dentry_stat.nr_dentry--; /* For d_free, below */
/*drops the locks, at that point nobody can reach this dentry */
dentry_iput(dentry);
if (IS_ROOT(dentry))
@@ -314,7 +327,6 @@ int d_invalidate(struct dentry * dentry)
EXPORT_SYMBOL(d_invalidate);
/* This should be called _only_ with dcache_lock held */
-
static inline struct dentry * __dget_locked(struct dentry *dentry)
{
atomic_inc(&dentry->d_count);
@@ -534,7 +546,7 @@ static void prune_dcache(int count)
{
struct super_block *sb, *p = NULL;
int w_count;
- int unused = dentry_stat.nr_unused;
+ int unused = percpu_counter_sum_positive(&nr_dentry_unused);
int prune_ratio;
int pruned;
@@ -699,20 +711,13 @@ static void shrink_dcache_for_umount_sub
* otherwise we ascend to the parent and move to the
* next sibling if there is one */
if (!parent)
- goto out;
-
+ return;
dentry = parent;
-
} while (list_empty(&dentry->d_subdirs));
dentry = list_entry(dentry->d_subdirs.next,
struct dentry, d_u.d_child);
}
-out:
- /* several dentries were freed, need to correct nr_dentry */
- spin_lock(&dcache_lock);
- dentry_stat.nr_dentry -= detached;
- spin_unlock(&dcache_lock);
}
/*
@@ -896,12 +901,16 @@ EXPORT_SYMBOL(shrink_dcache_parent);
*/
static int shrink_dcache_memory(struct shrinker *shrink, int nr, gfp_t gfp_mask)
{
+ int nr_unused;
+
if (nr) {
if (!(gfp_mask & __GFP_FS))
return -1;
prune_dcache(nr);
}
- return (dentry_stat.nr_unused / 100) * sysctl_vfs_cache_pressure;
+
+ nr_unused = percpu_counter_sum_positive(&nr_dentry_unused);
+ return (nr_unused / 100) * sysctl_vfs_cache_pressure;
}
static struct shrinker dcache_shrinker = {
@@ -968,9 +977,10 @@ struct dentry *d_alloc(struct dentry * p
spin_lock(&dcache_lock);
if (parent)
list_add(&dentry->d_u.d_child, &parent->d_subdirs);
- dentry_stat.nr_dentry++;
spin_unlock(&dcache_lock);
+ percpu_counter_inc(&nr_dentry);
+
return dentry;
}
EXPORT_SYMBOL(d_alloc);
@@ -2417,6 +2427,9 @@ static void __init dcache_init(void)
{
int loop;
+ percpu_counter_init(&nr_dentry, 0);
+ percpu_counter_init(&nr_dentry_unused, 0);
+
/*
* A constructor could be added for stable state like the lists,
* but it is probably not worth it because of the cache nature
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h 2010-10-10 09:36:05.400003777 +0200
+++ linux-2.6/include/linux/fs.h 2010-10-10 09:36:31.326004057 +0200
@@ -2483,6 +2483,8 @@ ssize_t simple_attr_write(struct file *f
struct ctl_table;
int proc_nr_files(struct ctl_table *table, int write,
void __user *buffer, size_t *lenp, loff_t *ppos);
+int proc_nr_dentry(struct ctl_table *table, int write,
+ void __user *buffer, size_t *lenp, loff_t *ppos);
int proc_nr_inodes(struct ctl_table *table, int write,
void __user *buffer, size_t *lenp, loff_t *ppos);
int __init get_filesystem_list(char *buf);
Index: linux-2.6/kernel/sysctl.c
===================================================================
--- linux-2.6.orig/kernel/sysctl.c 2010-10-10 09:36:05.412003707 +0200
+++ linux-2.6/kernel/sysctl.c 2010-10-10 09:36:31.331004057 +0200
@@ -1377,7 +1377,7 @@ static struct ctl_table fs_table[] = {
.data = &dentry_stat,
.maxlen = 6*sizeof(int),
.mode = 0444,
- .proc_handler = proc_dointvec,
+ .proc_handler = proc_nr_dentry,
},
{
.procname = "overflowuid",
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 04/10] fs: improve DCACHE_REFERENCED usage
2010-10-10 9:36 [PATCH 00/10] dcache cleanups Christoph Hellwig
` (2 preceding siblings ...)
2010-10-10 9:36 ` [PATCH 03/10] fs: use percpu counter for nr_dentry and nr_dentry_unused Christoph Hellwig
@ 2010-10-10 9:36 ` Christoph Hellwig
2010-10-10 9:36 ` [PATCH 05/10] fs: split __shrink_dcache_sb Christoph Hellwig
` (7 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2010-10-10 9:36 UTC (permalink / raw)
To: viro; +Cc: eparis, linux-fsdevel, Nick Piggin
[-- Attachment #1: dcache-improve-DCACHE_REFERENCE --]
[-- Type: text/plain, Size: 1075 bytes --]
From: Nick Piggin <npiggin@suse.de>
dentry referenced bit is only set when installing the dentry back
onto the LRU. However with lazy LRU, the dentry can already be on
the LRU list at dput time, thus missing out on setting the referenced
bit. Fix this.
Signed-off-by: Nick Piggin <npiggin@suse.de>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/fs/dcache.c
===================================================================
--- linux-2.6.orig/fs/dcache.c 2010-10-09 10:26:11.000000000 +0200
+++ linux-2.6/fs/dcache.c 2010-10-09 10:29:04.611003707 +0200
@@ -246,13 +246,16 @@ repeat:
if (dentry->d_op->d_delete(dentry))
goto unhash_it;
}
+
/* Unreachable? Get rid of it */
if (d_unhashed(dentry))
goto kill_it;
- if (list_empty(&dentry->d_lru)) {
- dentry->d_flags |= DCACHE_REFERENCED;
+
+ /* Otherwise leave it cached and ensure it's on the LRU */
+ dentry->d_flags |= DCACHE_REFERENCED;
+ if (list_empty(&dentry->d_lru))
dentry_lru_add(dentry);
- }
+
spin_unlock(&dentry->d_lock);
spin_unlock(&dcache_lock);
return;
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 05/10] fs: split __shrink_dcache_sb
2010-10-10 9:36 [PATCH 00/10] dcache cleanups Christoph Hellwig
` (3 preceding siblings ...)
2010-10-10 9:36 ` [PATCH 04/10] fs: improve DCACHE_REFERENCED usage Christoph Hellwig
@ 2010-10-10 9:36 ` Christoph Hellwig
2010-10-10 9:36 ` [PATCH 06/10] fs: clean up dentry lru modification Christoph Hellwig
` (6 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2010-10-10 9:36 UTC (permalink / raw)
To: viro; +Cc: eparis, linux-fsdevel
[-- Attachment #1: dcache-split-__shrink_dcache_sb --]
[-- Type: text/plain, Size: 5342 bytes --]
Currently __shrink_dcache_sb has an extremly awkward calling convention
because it tries to please very different callers. Split out the
main loop into a shrink_dentry_list helper, which gets called directly
from shrink_dcache_sb for the cases where all dentries need to be pruned,
or from __shrink_dcache_sb for pruning only a certain number of dentries.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/fs/dcache.c
===================================================================
--- linux-2.6.orig/fs/dcache.c 2010-10-09 23:00:40.729003636 +0200
+++ linux-2.6/fs/dcache.c 2010-10-09 23:02:16.223005663 +0200
@@ -459,66 +459,20 @@ static void prune_one_dentry(struct dent
}
}
-/*
- * Shrink the dentry LRU on a given superblock.
- * @sb : superblock to shrink dentry LRU.
- * @count: If count is NULL, we prune all dentries on superblock.
- * @flags: If flags is non-zero, we need to do special processing based on
- * which flags are set. This means we don't need to maintain multiple
- * similar copies of this loop.
- */
-static void __shrink_dcache_sb(struct super_block *sb, int *count, int flags)
+static void shrink_dentry_list(struct list_head *list)
{
- LIST_HEAD(referenced);
- LIST_HEAD(tmp);
struct dentry *dentry;
- int cnt = 0;
-
- BUG_ON(!sb);
- BUG_ON((flags & DCACHE_REFERENCED) && count == NULL);
- spin_lock(&dcache_lock);
- if (count != NULL)
- /* called from prune_dcache() and shrink_dcache_parent() */
- cnt = *count;
-restart:
- if (count == NULL)
- list_splice_init(&sb->s_dentry_lru, &tmp);
- else {
- while (!list_empty(&sb->s_dentry_lru)) {
- dentry = list_entry(sb->s_dentry_lru.prev,
- struct dentry, d_lru);
- BUG_ON(dentry->d_sb != sb);
- spin_lock(&dentry->d_lock);
- /*
- * If we are honouring the DCACHE_REFERENCED flag and
- * the dentry has this flag set, don't free it. Clear
- * the flag and put it back on the LRU.
- */
- if ((flags & DCACHE_REFERENCED)
- && (dentry->d_flags & DCACHE_REFERENCED)) {
- dentry->d_flags &= ~DCACHE_REFERENCED;
- list_move(&dentry->d_lru, &referenced);
- spin_unlock(&dentry->d_lock);
- } else {
- list_move_tail(&dentry->d_lru, &tmp);
- spin_unlock(&dentry->d_lock);
- cnt--;
- if (!cnt)
- break;
- }
- cond_resched_lock(&dcache_lock);
- }
- }
- while (!list_empty(&tmp)) {
- dentry = list_entry(tmp.prev, struct dentry, d_lru);
+ while (!list_empty(list)) {
+ dentry = list_entry(list->prev, struct dentry, d_lru);
dentry_lru_del_init(dentry);
- spin_lock(&dentry->d_lock);
+
/*
* We found an inuse dentry which was not removed from
* the LRU because of laziness during lookup. Do not free
* it - just keep it off the LRU list.
*/
+ spin_lock(&dentry->d_lock);
if (atomic_read(&dentry->d_count)) {
spin_unlock(&dentry->d_lock);
continue;
@@ -527,13 +481,60 @@ restart:
/* dentry->d_lock was dropped in prune_one_dentry() */
cond_resched_lock(&dcache_lock);
}
- if (count == NULL && !list_empty(&sb->s_dentry_lru))
- goto restart;
- if (count != NULL)
- *count = cnt;
+}
+
+/**
+ * __shrink_dcache_sb - shrink the dentry LRU on a given superblock
+ * @sb: superblock to shrink dentry LRU.
+ * @count: number of entries to prune
+ * @flags: flags to control the dentry processing
+ *
+ * If flags contains DCACHE_REFERENCED reference dentries will not be pruned.
+ */
+static void __shrink_dcache_sb(struct super_block *sb, int *count, int flags)
+{
+ /* called from prune_dcache() and shrink_dcache_parent() */
+ struct dentry *dentry;
+ LIST_HEAD(referenced);
+ LIST_HEAD(tmp);
+ int cnt = *count;
+
+ spin_lock(&dcache_lock);
+ while (!list_empty(&sb->s_dentry_lru)) {
+ dentry = list_entry(sb->s_dentry_lru.prev,
+ struct dentry, d_lru);
+ BUG_ON(dentry->d_sb != sb);
+
+ /*
+ * If we are honouring the DCACHE_REFERENCED flag and the
+ * dentry has this flag set, don't free it. Clear the flag
+ * and put it back on the LRU.
+ */
+ if (flags & DCACHE_REFERENCED) {
+ spin_lock(&dentry->d_lock);
+ if (dentry->d_flags & DCACHE_REFERENCED) {
+ dentry->d_flags &= ~DCACHE_REFERENCED;
+ list_move(&dentry->d_lru, &referenced);
+ spin_unlock(&dentry->d_lock);
+ cond_resched_lock(&dcache_lock);
+ continue;
+ }
+ spin_unlock(&dentry->d_lock);
+ }
+
+ list_move_tail(&dentry->d_lru, &tmp);
+ if (!--cnt)
+ break;
+ cond_resched_lock(&dcache_lock);
+ }
+
+ *count = cnt;
+ shrink_dentry_list(&tmp);
+
if (!list_empty(&referenced))
list_splice(&referenced, &sb->s_dentry_lru);
spin_unlock(&dcache_lock);
+
}
/**
@@ -619,13 +620,19 @@ static void prune_dcache(int count)
* shrink_dcache_sb - shrink dcache for a superblock
* @sb: superblock
*
- * Shrink the dcache for the specified super block. This
- * is used to free the dcache before unmounting a file
- * system
+ * Shrink the dcache for the specified super block. This is used to free
+ * the dcache before unmounting a file system.
*/
-void shrink_dcache_sb(struct super_block * sb)
+void shrink_dcache_sb(struct super_block *sb)
{
- __shrink_dcache_sb(sb, NULL, 0);
+ LIST_HEAD(tmp);
+
+ spin_lock(&dcache_lock);
+ while (!list_empty(&sb->s_dentry_lru)) {
+ list_splice_init(&sb->s_dentry_lru, &tmp);
+ shrink_dentry_list(&tmp);
+ }
+ spin_unlock(&dcache_lock);
}
EXPORT_SYMBOL(shrink_dcache_sb);
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 06/10] fs: clean up dentry lru modification
2010-10-10 9:36 [PATCH 00/10] dcache cleanups Christoph Hellwig
` (4 preceding siblings ...)
2010-10-10 9:36 ` [PATCH 05/10] fs: split __shrink_dcache_sb Christoph Hellwig
@ 2010-10-10 9:36 ` Christoph Hellwig
2010-10-10 9:36 ` [PATCH 07/10] fs: use RCU read side protection in d_validate Christoph Hellwig
` (5 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2010-10-10 9:36 UTC (permalink / raw)
To: viro; +Cc: eparis, linux-fsdevel
[-- Attachment #1: dcache-simplify-lru --]
[-- Type: text/plain, Size: 4379 bytes --]
Always do a list_del_init on the LRU to make sure the list_empty invariant for
not beeing on the LRU always holds true, and fold dentry_lru_del_init into
dentry_lru_del. Replace the dentry_lru_add_tail primitive with a
dentry_lru_move_tail operations that simpler when the dentry already is one
the list, which is always is. Move the list_empty into dentry_lru_add to
fit the scheme of the other lru helpers, and simplify locking once we
move to a separate LRU lock.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/fs/dcache.c
===================================================================
--- linux-2.6.orig/fs/dcache.c 2010-10-09 23:02:16.223005663 +0200
+++ linux-2.6/fs/dcache.c 2010-10-10 09:35:59.292254021 +0200
@@ -133,37 +133,34 @@ static void dentry_iput(struct dentry *
}
/*
- * dentry_lru_(add|add_tail|del|del_init) must be called with dcache_lock held.
+ * dentry_lru_(add|del|move_tail) must be called with dcache_lock held.
*/
static void dentry_lru_add(struct dentry *dentry)
{
- list_add(&dentry->d_lru, &dentry->d_sb->s_dentry_lru);
- dentry->d_sb->s_nr_dentry_unused++;
- percpu_counter_inc(&nr_dentry_unused);
-}
-
-static void dentry_lru_add_tail(struct dentry *dentry)
-{
- list_add_tail(&dentry->d_lru, &dentry->d_sb->s_dentry_lru);
- dentry->d_sb->s_nr_dentry_unused++;
- percpu_counter_inc(&nr_dentry_unused);
+ if (list_empty(&dentry->d_lru)) {
+ list_add(&dentry->d_lru, &dentry->d_sb->s_dentry_lru);
+ dentry->d_sb->s_nr_dentry_unused++;
+ percpu_counter_inc(&nr_dentry_unused);
+ }
}
static void dentry_lru_del(struct dentry *dentry)
{
if (!list_empty(&dentry->d_lru)) {
- list_del(&dentry->d_lru);
+ list_del_init(&dentry->d_lru);
dentry->d_sb->s_nr_dentry_unused--;
percpu_counter_dec(&nr_dentry_unused);
}
}
-static void dentry_lru_del_init(struct dentry *dentry)
+static void dentry_lru_move_tail(struct dentry *dentry)
{
- if (likely(!list_empty(&dentry->d_lru))) {
- list_del_init(&dentry->d_lru);
- dentry->d_sb->s_nr_dentry_unused--;
- percpu_counter_dec(&nr_dentry_unused);
+ if (list_empty(&dentry->d_lru)) {
+ list_add_tail(&dentry->d_lru, &dentry->d_sb->s_dentry_lru);
+ dentry->d_sb->s_nr_dentry_unused++;
+ percpu_counter_inc(&nr_dentry_unused);
+ } else {
+ list_move_tail(&dentry->d_lru, &dentry->d_sb->s_dentry_lru);
}
}
@@ -253,8 +250,7 @@ repeat:
/* Otherwise leave it cached and ensure it's on the LRU */
dentry->d_flags |= DCACHE_REFERENCED;
- if (list_empty(&dentry->d_lru))
- dentry_lru_add(dentry);
+ dentry_lru_add(dentry);
spin_unlock(&dentry->d_lock);
spin_unlock(&dcache_lock);
@@ -333,7 +329,7 @@ EXPORT_SYMBOL(d_invalidate);
static inline struct dentry * __dget_locked(struct dentry *dentry)
{
atomic_inc(&dentry->d_count);
- dentry_lru_del_init(dentry);
+ dentry_lru_del(dentry);
return dentry;
}
@@ -452,7 +448,7 @@ static void prune_one_dentry(struct dent
if (dentry->d_op && dentry->d_op->d_delete)
dentry->d_op->d_delete(dentry);
- dentry_lru_del_init(dentry);
+ dentry_lru_del(dentry);
__d_drop(dentry);
dentry = d_kill(dentry);
spin_lock(&dcache_lock);
@@ -465,7 +461,7 @@ static void shrink_dentry_list(struct li
while (!list_empty(list)) {
dentry = list_entry(list->prev, struct dentry, d_lru);
- dentry_lru_del_init(dentry);
+ dentry_lru_del(dentry);
/*
* We found an inuse dentry which was not removed from
@@ -650,7 +646,7 @@ static void shrink_dcache_for_umount_sub
/* detach this root from the system */
spin_lock(&dcache_lock);
- dentry_lru_del_init(dentry);
+ dentry_lru_del(dentry);
__d_drop(dentry);
spin_unlock(&dcache_lock);
@@ -664,7 +660,7 @@ static void shrink_dcache_for_umount_sub
spin_lock(&dcache_lock);
list_for_each_entry(loop, &dentry->d_subdirs,
d_u.d_child) {
- dentry_lru_del_init(loop);
+ dentry_lru_del(loop);
__d_drop(loop);
cond_resched_lock(&dcache_lock);
}
@@ -841,14 +837,15 @@ resume:
struct dentry *dentry = list_entry(tmp, struct dentry, d_u.d_child);
next = tmp->next;
- dentry_lru_del_init(dentry);
/*
* move only zero ref count dentries to the end
* of the unused list for prune_dcache
*/
if (!atomic_read(&dentry->d_count)) {
- dentry_lru_add_tail(dentry);
+ dentry_lru_move_tail(dentry);
found++;
+ } else {
+ dentry_lru_del(dentry);
}
/*
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 07/10] fs: use RCU read side protection in d_validate
2010-10-10 9:36 [PATCH 00/10] dcache cleanups Christoph Hellwig
` (5 preceding siblings ...)
2010-10-10 9:36 ` [PATCH 06/10] fs: clean up dentry lru modification Christoph Hellwig
@ 2010-10-10 9:36 ` Christoph Hellwig
2010-10-10 9:36 ` [PATCH 08/10] exportfs: use dget_parent Christoph Hellwig
` (4 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2010-10-10 9:36 UTC (permalink / raw)
To: viro; +Cc: eparis, linux-fsdevel
[-- Attachment #1: dcache-d_validate-use-rcu --]
[-- Type: text/plain, Size: 1678 bytes --]
d_validate does a purely read lookup in the dentry hash, so use RCU read side
locking instead of dcache_lock. Split out from a larget patch by
Nick Piggin <npiggin@suse.de>.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/fs/dcache.c
===================================================================
--- linux-2.6.orig/fs/dcache.c 2010-10-09 23:02:58.688004125 +0200
+++ linux-2.6/fs/dcache.c 2010-10-09 23:05:04.988254510 +0200
@@ -1491,33 +1491,26 @@ out:
* This is used by ncpfs in its readdir implementation.
* Zero is returned in the dentry is invalid.
*/
-
-int d_validate(struct dentry *dentry, struct dentry *dparent)
+int d_validate(struct dentry *dentry, struct dentry *parent)
{
- struct hlist_head *base;
- struct hlist_node *lhp;
+ struct hlist_head *head = d_hash(parent, dentry->d_name.hash);
+ struct hlist_node *node;
+ struct dentry *d;
/* Check whether the ptr might be valid at all.. */
if (!kmem_ptr_validate(dentry_cache, dentry))
- goto out;
+ return 0;
+ if (dentry->d_parent != parent)
+ return 0;
- if (dentry->d_parent != dparent)
- goto out;
-
- spin_lock(&dcache_lock);
- base = d_hash(dparent, dentry->d_name.hash);
- hlist_for_each(lhp,base) {
- /* hlist_for_each_entry_rcu() not required for d_hash list
- * as it is parsed under dcache_lock
- */
- if (dentry == hlist_entry(lhp, struct dentry, d_hash)) {
- __dget_locked(dentry);
- spin_unlock(&dcache_lock);
+ rcu_read_lock();
+ hlist_for_each_entry_rcu(d, node, head, d_hash) {
+ if (d == dentry) {
+ dget(dentry);
return 1;
}
}
- spin_unlock(&dcache_lock);
-out:
+ rcu_read_unlock();
return 0;
}
EXPORT_SYMBOL(d_validate);
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 08/10] exportfs: use dget_parent
2010-10-10 9:36 [PATCH 00/10] dcache cleanups Christoph Hellwig
` (6 preceding siblings ...)
2010-10-10 9:36 ` [PATCH 07/10] fs: use RCU read side protection in d_validate Christoph Hellwig
@ 2010-10-10 9:36 ` Christoph Hellwig
2010-10-10 9:36 ` [PATCH 09/10] smbfs: " Christoph Hellwig
` (3 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2010-10-10 9:36 UTC (permalink / raw)
To: viro; +Cc: eparis, linux-fsdevel
[-- Attachment #1: exportfs-use-dget_parent --]
[-- Type: text/plain, Size: 1178 bytes --]
Use dget_parent instead of opencoding it. This simplifies the code, but
more importanly prepares for the more complicated locking for a parent
dget in the dcache scale patch series.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/fs/exportfs/expfs.c
===================================================================
--- linux-2.6.orig/fs/exportfs/expfs.c 2010-10-10 09:51:24.580004056 +0200
+++ linux-2.6/fs/exportfs/expfs.c 2010-10-10 09:52:34.490016907 +0200
@@ -74,21 +74,20 @@ static struct dentry *
find_disconnected_root(struct dentry *dentry)
{
dget(dentry);
- spin_lock(&dentry->d_lock);
- while (!IS_ROOT(dentry) &&
- (dentry->d_parent->d_flags & DCACHE_DISCONNECTED)) {
- struct dentry *parent = dentry->d_parent;
- dget(parent);
- spin_unlock(&dentry->d_lock);
+ while (!IS_ROOT(dentry)) {
+ struct dentry *parent = dget_parent(dentry);
+
+ if (parent->d_flags & DCACHE_DISCONNECTED) {
+ dput(parent);
+ break;
+ }
+
dput(dentry);
dentry = parent;
- spin_lock(&dentry->d_lock);
}
- spin_unlock(&dentry->d_lock);
return dentry;
}
-
/*
* Make sure target_dir is fully connected to the dentry tree.
*
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 09/10] smbfs: use dget_parent
2010-10-10 9:36 [PATCH 00/10] dcache cleanups Christoph Hellwig
` (7 preceding siblings ...)
2010-10-10 9:36 ` [PATCH 08/10] exportfs: use dget_parent Christoph Hellwig
@ 2010-10-10 9:36 ` Christoph Hellwig
2010-10-10 9:36 ` [PATCH 10/10] fsnotify: " Christoph Hellwig
` (2 subsequent siblings)
11 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2010-10-10 9:36 UTC (permalink / raw)
To: viro; +Cc: eparis, linux-fsdevel
[-- Attachment #1: smbfs-use-dget_parent --]
[-- Type: text/plain, Size: 2323 bytes --]
Use dget_parent instead of opencoding it. This simplifies the code, but
more importanly prepares for the more complicated locking for a parent
dget in the dcache scale patch series.
Note that the d_time assignment in smb_renew_times moves out of d_lock,
but it's a single atomic 32-bit value, and that's what other sites
setting it do already.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/fs/smbfs/dir.c
===================================================================
--- linux-2.6.orig/fs/smbfs/dir.c 2010-10-09 12:58:31.000263030 +0200
+++ linux-2.6/fs/smbfs/dir.c 2010-10-09 13:03:42.478255626 +0200
@@ -406,21 +406,15 @@ void
smb_renew_times(struct dentry * dentry)
{
dget(dentry);
- spin_lock(&dentry->d_lock);
- for (;;) {
- struct dentry *parent;
+ dentry->d_time = jiffies;
- dentry->d_time = jiffies;
- if (IS_ROOT(dentry))
- break;
- parent = dentry->d_parent;
- dget(parent);
- spin_unlock(&dentry->d_lock);
+ while (!IS_ROOT(dentry)) {
+ struct dentry *parent = dget_parent(dentry);
dput(dentry);
dentry = parent;
- spin_lock(&dentry->d_lock);
+
+ dentry->d_time = jiffies;
}
- spin_unlock(&dentry->d_lock);
dput(dentry);
}
Index: linux-2.6/fs/smbfs/proc.c
===================================================================
--- linux-2.6.orig/fs/smbfs/proc.c 2010-10-09 12:58:31.008254859 +0200
+++ linux-2.6/fs/smbfs/proc.c 2010-10-09 13:02:14.917005452 +0200
@@ -332,16 +332,15 @@ static int smb_build_path(struct smb_sb_
* and store it in reversed order [see reverse_string()]
*/
dget(entry);
- spin_lock(&entry->d_lock);
while (!IS_ROOT(entry)) {
struct dentry *parent;
if (maxlen < (3<<unicode)) {
- spin_unlock(&entry->d_lock);
dput(entry);
return -ENAMETOOLONG;
}
+ spin_lock(&entry->d_lock);
len = server->ops->convert(path, maxlen-2,
entry->d_name.name, entry->d_name.len,
server->local_nls, server->remote_nls);
@@ -359,15 +358,12 @@ static int smb_build_path(struct smb_sb_
}
*path++ = '\\';
maxlen -= len+1;
-
- parent = entry->d_parent;
- dget(parent);
spin_unlock(&entry->d_lock);
+
+ parent = dget_parent(entry);
dput(entry);
entry = parent;
- spin_lock(&entry->d_lock);
}
- spin_unlock(&entry->d_lock);
dput(entry);
reverse_string(buf, path-buf);
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 10/10] fsnotify: use dget_parent
2010-10-10 9:36 [PATCH 00/10] dcache cleanups Christoph Hellwig
` (8 preceding siblings ...)
2010-10-10 9:36 ` [PATCH 09/10] smbfs: " Christoph Hellwig
@ 2010-10-10 9:36 ` Christoph Hellwig
2010-10-11 13:59 ` Eric Paris
2010-10-13 1:04 ` [PATCH 00/10] dcache cleanups Dave Chinner
2010-10-13 1:46 ` Dave Chinner
11 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2010-10-10 9:36 UTC (permalink / raw)
To: viro; +Cc: eparis, linux-fsdevel
[-- Attachment #1: fsnotify-use-dget_parent --]
[-- Type: text/plain, Size: 2375 bytes --]
Use dget_parent instead of opencoding it. This simplifies the code, but
more importanly prepares for the more complicated locking for a parent
dget in the dcache scale patch series.
It means we do grab a reference to the parent now if need to be watched,
but not with the specified mask. If this turns out to be a problem
we'll have to revisit it, but for now let's keep as much as possible
dcache internals inside dcache.[ch].
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/fs/notify/fsnotify.c
===================================================================
--- linux-2.6.orig/fs/notify/fsnotify.c 2010-10-10 09:35:58.488004195 +0200
+++ linux-2.6/fs/notify/fsnotify.c 2010-10-10 09:53:53.103010272 +0200
@@ -88,8 +88,6 @@ void __fsnotify_parent(struct path *path
{
struct dentry *parent;
struct inode *p_inode;
- bool send = false;
- bool should_update_children = false;
if (!dentry)
dentry = path->dentry;
@@ -97,29 +95,12 @@ void __fsnotify_parent(struct path *path
if (!(dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED))
return;
- spin_lock(&dentry->d_lock);
- parent = dentry->d_parent;
+ parent = dget_parent(dentry);
p_inode = parent->d_inode;
- if (fsnotify_inode_watches_children(p_inode)) {
- if (p_inode->i_fsnotify_mask & mask) {
- dget(parent);
- send = true;
- }
- } else {
- /*
- * The parent doesn't care about events on it's children but
- * at least one child thought it did. We need to run all the
- * children and update their d_flags to let them know p_inode
- * doesn't care about them any more.
- */
- dget(parent);
- should_update_children = true;
- }
-
- spin_unlock(&dentry->d_lock);
-
- if (send) {
+ if (unlikely(!fsnotify_inode_watches_children(p_inode)))
+ __fsnotify_update_child_dentry_flags(p_inode);
+ else if (p_inode->i_fsnotify_mask & mask) {
/* we are notifying a parent so come up with the new mask which
* specifies these are events which came from a child. */
mask |= FS_EVENT_ON_CHILD;
@@ -130,13 +111,9 @@ void __fsnotify_parent(struct path *path
else
fsnotify(p_inode, mask, dentry->d_inode, FSNOTIFY_EVENT_INODE,
dentry->d_name.name, 0);
- dput(parent);
}
- if (unlikely(should_update_children)) {
- __fsnotify_update_child_dentry_flags(p_inode);
- dput(parent);
- }
+ dput(parent);
}
EXPORT_SYMBOL_GPL(__fsnotify_parent);
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 10/10] fsnotify: use dget_parent
2010-10-10 9:36 ` [PATCH 10/10] fsnotify: " Christoph Hellwig
@ 2010-10-11 13:59 ` Eric Paris
0 siblings, 0 replies; 18+ messages in thread
From: Eric Paris @ 2010-10-11 13:59 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: viro, linux-fsdevel
On Sun, 2010-10-10 at 05:36 -0400, Christoph Hellwig wrote:
> plain text document attachment (fsnotify-use-dget_parent)
> Use dget_parent instead of opencoding it. This simplifies the code, but
> more importanly prepares for the more complicated locking for a parent
> dget in the dcache scale patch series.
>
> It means we do grab a reference to the parent now if need to be watched,
> but not with the specified mask. If this turns out to be a problem
> we'll have to revisit it, but for now let's keep as much as possible
> dcache internals inside dcache.[ch].
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Nope this is fine, we were doing lots of extra work for little benefit
before.
Acked-by: Eric Paris <eparis@redhat.com>
> Index: linux-2.6/fs/notify/fsnotify.c
> ===================================================================
> --- linux-2.6.orig/fs/notify/fsnotify.c 2010-10-10 09:35:58.488004195 +0200
> +++ linux-2.6/fs/notify/fsnotify.c 2010-10-10 09:53:53.103010272 +0200
> @@ -88,8 +88,6 @@ void __fsnotify_parent(struct path *path
> {
> struct dentry *parent;
> struct inode *p_inode;
> - bool send = false;
> - bool should_update_children = false;
>
> if (!dentry)
> dentry = path->dentry;
> @@ -97,29 +95,12 @@ void __fsnotify_parent(struct path *path
> if (!(dentry->d_flags & DCACHE_FSNOTIFY_PARENT_WATCHED))
> return;
>
> - spin_lock(&dentry->d_lock);
> - parent = dentry->d_parent;
> + parent = dget_parent(dentry);
> p_inode = parent->d_inode;
>
> - if (fsnotify_inode_watches_children(p_inode)) {
> - if (p_inode->i_fsnotify_mask & mask) {
> - dget(parent);
> - send = true;
> - }
> - } else {
> - /*
> - * The parent doesn't care about events on it's children but
> - * at least one child thought it did. We need to run all the
> - * children and update their d_flags to let them know p_inode
> - * doesn't care about them any more.
> - */
> - dget(parent);
> - should_update_children = true;
> - }
> -
> - spin_unlock(&dentry->d_lock);
> -
> - if (send) {
> + if (unlikely(!fsnotify_inode_watches_children(p_inode)))
> + __fsnotify_update_child_dentry_flags(p_inode);
> + else if (p_inode->i_fsnotify_mask & mask) {
> /* we are notifying a parent so come up with the new mask which
> * specifies these are events which came from a child. */
> mask |= FS_EVENT_ON_CHILD;
> @@ -130,13 +111,9 @@ void __fsnotify_parent(struct path *path
> else
> fsnotify(p_inode, mask, dentry->d_inode, FSNOTIFY_EVENT_INODE,
> dentry->d_name.name, 0);
> - dput(parent);
> }
>
> - if (unlikely(should_update_children)) {
> - __fsnotify_update_child_dentry_flags(p_inode);
> - dput(parent);
> - }
> + dput(parent);
> }
> EXPORT_SYMBOL_GPL(__fsnotify_parent);
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 00/10] dcache cleanups
2010-10-10 9:36 [PATCH 00/10] dcache cleanups Christoph Hellwig
` (9 preceding siblings ...)
2010-10-10 9:36 ` [PATCH 10/10] fsnotify: " Christoph Hellwig
@ 2010-10-13 1:04 ` Dave Chinner
2010-10-13 11:21 ` Christoph Hellwig
2010-10-13 1:46 ` Dave Chinner
11 siblings, 1 reply; 18+ messages in thread
From: Dave Chinner @ 2010-10-13 1:04 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: viro, eparis, linux-fsdevel
On Sun, Oct 10, 2010 at 05:36:20AM -0400, Christoph Hellwig wrote:
> A couple of dcache cleanups in preparation of the dcache_lock splitup.
> Note that I already sent the first patch individually, but this series
> contains a fixed version.
This applies on top of my inode-scale branch, right?
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 00/10] dcache cleanups
2010-10-13 1:04 ` [PATCH 00/10] dcache cleanups Dave Chinner
@ 2010-10-13 11:21 ` Christoph Hellwig
0 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2010-10-13 11:21 UTC (permalink / raw)
To: Dave Chinner; +Cc: Christoph Hellwig, viro, eparis, linux-fsdevel
On Wed, Oct 13, 2010 at 12:04:59PM +1100, Dave Chinner wrote:
> On Sun, Oct 10, 2010 at 05:36:20AM -0400, Christoph Hellwig wrote:
> > A couple of dcache cleanups in preparation of the dcache_lock splitup.
> > Note that I already sent the first patch individually, but this series
> > contains a fixed version.
>
> This applies on top of my inode-scale branch, right?
Yes.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 00/10] dcache cleanups
2010-10-10 9:36 [PATCH 00/10] dcache cleanups Christoph Hellwig
` (10 preceding siblings ...)
2010-10-13 1:04 ` [PATCH 00/10] dcache cleanups Dave Chinner
@ 2010-10-13 1:46 ` Dave Chinner
2010-10-13 11:22 ` Christoph Hellwig
11 siblings, 1 reply; 18+ messages in thread
From: Dave Chinner @ 2010-10-13 1:46 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: viro, eparis, linux-fsdevel
On Sun, Oct 10, 2010 at 05:36:20AM -0400, Christoph Hellwig wrote:
> A couple of dcache cleanups in preparation of the dcache_lock splitup.
> Note that I already sent the first patch individually, but this series
> contains a fixed version.
Something in this series causes xfsdump to get really unhappy
(xfstests 026) - probably the get_parent changes. This message is
just continually spewed and the xfsdump process cannot be killed:
[ 884.392813] export: Eeek filesystem root is not connected, impossible
[ 884.393873] export: Eeek filesystem root is not connected, impossible
[ 884.394669] export: Eeek filesystem root is not connected, impossible
[ 884.395648] export: Eeek filesystem root is not connected, impossible
[ 884.396482] export: Eeek filesystem root is not connected, impossible
[ 884.397487] export: Eeek filesystem root is not connected, impossible
[ 884.398241] export: Eeek filesystem root is not connected, impossible
[ 884.399036] export: Eeek filesystem root is not connected, impossible
[ 884.400148] export: Eeek filesystem root is not connected, impossible
[ 884.401186] export: Eeek filesystem root is not connected, impossible
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [PATCH 00/10] dcache cleanups
2010-10-13 1:46 ` Dave Chinner
@ 2010-10-13 11:22 ` Christoph Hellwig
2010-10-13 15:56 ` Christoph Hellwig
0 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2010-10-13 11:22 UTC (permalink / raw)
To: Dave Chinner; +Cc: Christoph Hellwig, viro, eparis, linux-fsdevel
On Wed, Oct 13, 2010 at 12:46:02PM +1100, Dave Chinner wrote:
> On Sun, Oct 10, 2010 at 05:36:20AM -0400, Christoph Hellwig wrote:
> > A couple of dcache cleanups in preparation of the dcache_lock splitup.
> > Note that I already sent the first patch individually, but this series
> > contains a fixed version.
>
> Something in this series causes xfsdump to get really unhappy
> (xfstests 026) - probably the get_parent changes. This message is
> just continually spewed and the xfsdump process cannot be killed:
Hmm, I did a couple of xfstests run both with XFS and ext3 and even did
some nfs testing. Do you still see it with the exportfs patch removed?
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 00/10] dcache cleanups
2010-10-13 11:22 ` Christoph Hellwig
@ 2010-10-13 15:56 ` Christoph Hellwig
2010-10-16 8:17 ` Nick Piggin
0 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2010-10-13 15:56 UTC (permalink / raw)
To: Dave Chinner; +Cc: Christoph Hellwig, viro, eparis, linux-fsdevel
Found the bug. I did a small cleanup after testing the patch
and inverted a condition. The correct one is below:
---
From: Christoph Hellwig <hch@lst.de>
Subject: exportfs: use dget_parent
Use dget_parent instead of opencoding it. This simplifies the code, but
more importanly prepares for the more complicated locking for a parent
dget in the dcache scale patch series.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: linux-2.6/fs/exportfs/expfs.c
===================================================================
--- linux-2.6.orig/fs/exportfs/expfs.c 2010-10-11 16:58:59.533088586 -0400
+++ linux-2.6/fs/exportfs/expfs.c 2010-10-13 09:54:41.070259168 -0400
@@ -74,21 +74,20 @@ static struct dentry *
find_disconnected_root(struct dentry *dentry)
{
dget(dentry);
- spin_lock(&dentry->d_lock);
- while (!IS_ROOT(dentry) &&
- (dentry->d_parent->d_flags & DCACHE_DISCONNECTED)) {
- struct dentry *parent = dentry->d_parent;
- dget(parent);
- spin_unlock(&dentry->d_lock);
+ while (!IS_ROOT(dentry)) {
+ struct dentry *parent = dget_parent(dentry);
+
+ if (!(parent->d_flags & DCACHE_DISCONNECTED)) {
+ dput(parent);
+ break;
+ }
+
dput(dentry);
dentry = parent;
- spin_lock(&dentry->d_lock);
}
- spin_unlock(&dentry->d_lock);
return dentry;
}
-
/*
* Make sure target_dir is fully connected to the dentry tree.
*
^ permalink raw reply [flat|nested] 18+ messages in thread