Skip to contentSkip to conversation

A few preparation patches for dcache further dcache shrinker rework

PATCH New Cyrill Gorcunov 1 participant 4 messages Jul 14, 2011

Subsystems (5)
VFS Staging NFS client Locking primitives +1 more subsystems
lore
Share & tools
lore

Cyrill Gorcunov

author
#0 lore raw

cover A few preparation patches for dcache further dcache shrinker rework

There were a thread at http://lwn.net/Articles/441164/ and while the restNIof dcache rework is still under review and discussion these three patchesN"might be considered independantly.

Please review, comments are highly appreciated!

n.b.: It's a first time I'm sending patches with additional From:
line via quilt tool, so if something get screwed (I hope not ;) in changelog, sorry please.

Cyrill

author
#1 lore raw

PATCH 1/3 vfs, dcache: Introduce lighten r/o rename_lock lockers

From: Pavel Emelyanov <[email protected]>
Subject: [patch 1/3] vfs, dcache: Introduce lighten r/o rename_lock lockers

There are some places, that need to _lock_ the rename_lock, but they do it for reading the dentry tree. When doing the write_seqlock they force the existing "willing to restart" readers do the restart, which is not actually required.

Introduce two more seqlock helpers, that lock the lock, but do not update the seq count.

Signed-off-by Pavel Emelyanov <[email protected]>
Signed-off-by Cyrill Gorcunov <[email protected]>
---
 fs/dcache.c             |   40 ++++++++++++++++++++--------------------
 include/linux/seqlock.h |   16 ++++++++++++++++
 2 files changed, 36 insertions(+), 20 deletions(-)
fs/dcache.c +20 -20
Index: linux-2.6.git/fs/dcache.c
===================================================================
@@ -1089,18 +1089,18 @@ resume:
 	if (!locked && read_seqretry(&rename_lock, seq))
 		goto rename_retry;
 	if (locked)
-		write_sequnlock(&rename_lock);
+		read_sequnlock(&rename_lock);
 	return 0; /* No mount points found in tree */
 positive:
 	if (!locked && read_seqretry(&rename_lock, seq))
 		goto rename_retry;
 	if (locked)
-		write_sequnlock(&rename_lock);
+		read_sequnlock(&rename_lock);
 	return 1;
 
 rename_retry:
 	locked = 1;
-	write_seqlock(&rename_lock);
+	read_seqlock(&rename_lock);
 	goto again;
 }
 EXPORT_SYMBOL(have_submounts);

@@ -1191,14 +1191,14 @@ out:
 	if (!locked && read_seqretry(&rename_lock, seq))
 		goto rename_retry;
 	if (locked)
-		write_sequnlock(&rename_lock);
+		read_sequnlock(&rename_lock);
 	return found;
 
 rename_retry:
 	if (found)
 		return found;
 	locked = 1;
-	write_seqlock(&rename_lock);
+	read_seqlock(&rename_lock);
 	goto again;
 }
 

@@ -2562,9 +2562,9 @@ char *__d_path(const struct path *path, 
 	int error;
 
 	prepend(&res, &buflen, "\0", 1);
-	write_seqlock(&rename_lock);
+	read_seqlock(&rename_lock);
 	error = prepend_path(path, root, &res, &buflen);
-	write_sequnlock(&rename_lock);
+	read_sequnlock(&rename_lock);
 
 	if (error)
 		return ERR_PTR(error);

@@ -2626,12 +2626,12 @@ char *d_path(const struct path *path, ch
 		return path->dentry->d_op->d_dname(path->dentry, buf, buflen);
 
 	get_fs_root(current->fs, &root);
-	write_seqlock(&rename_lock);
+	read_seqlock(&rename_lock);
 	tmp = root;
 	error = path_with_deleted(path, &tmp, &res, &buflen);
 	if (error)
 		res = ERR_PTR(error);
-	write_sequnlock(&rename_lock);
+	read_sequnlock(&rename_lock);
 	path_put(&root);
 	return res;
 }

@@ -2657,12 +2657,12 @@ char *d_path_with_unreachable(const stru
 		return path->dentry->d_op->d_dname(path->dentry, buf, buflen);
 
 	get_fs_root(current->fs, &root);
-	write_seqlock(&rename_lock);
+	read_seqlock(&rename_lock);
 	tmp = root;
 	error = path_with_deleted(path, &tmp, &res, &buflen);
 	if (!error && !path_equal(&tmp, &root))
 		error = prepend_unreachable(&res, &buflen);
-	write_sequnlock(&rename_lock);
+	read_sequnlock(&rename_lock);
 	path_put(&root);
 	if (error)
 		res =  ERR_PTR(error);

@@ -2729,9 +2729,9 @@ char *dentry_path_raw(struct dentry *den
 {
 	char *retval;
 
-	write_seqlock(&rename_lock);
+	read_seqlock(&rename_lock);
 	retval = __dentry_path(dentry, buf, buflen);
-	write_sequnlock(&rename_lock);
+	read_sequnlock(&rename_lock);
 
 	return retval;
 }

@@ -2742,7 +2742,7 @@ char *dentry_path(struct dentry *dentry,
 	char *p = NULL;
 	char *retval;
 
-	write_seqlock(&rename_lock);
+	read_seqlock(&rename_lock);
 	if (d_unlinked(dentry)) {
 		p = buf + buflen;
 		if (prepend(&p, &buflen, "//deleted", 10) != 0)

@@ -2750,7 +2750,7 @@ char *dentry_path(struct dentry *dentry,
 		buflen++;
 	}
 	retval = __dentry_path(dentry, buf, buflen);
-	write_sequnlock(&rename_lock);
+	read_sequnlock(&rename_lock);
 	if (!IS_ERR(retval) && p)
 		*p = '/';	/* restore '/' overriden with '\0' */
 	return retval;

@@ -2788,7 +2788,7 @@ SYSCALL_DEFINE2(getcwd, char __user *, b
 	get_fs_root_and_pwd(current->fs, &root, &pwd);
 
 	error = -ENOENT;
-	write_seqlock(&rename_lock);
+	read_seqlock(&rename_lock);
 	if (!d_unlinked(pwd.dentry)) {
 		unsigned long len;
 		struct path tmp = root;

@@ -2797,7 +2797,7 @@ SYSCALL_DEFINE2(getcwd, char __user *, b
 
 		prepend(&cwd, &buflen, "\0", 1);
 		error = prepend_path(&pwd, &tmp, &cwd, &buflen);
-		write_sequnlock(&rename_lock);
+		read_sequnlock(&rename_lock);
 
 		if (error)
 			goto out;

@@ -2817,7 +2817,7 @@ SYSCALL_DEFINE2(getcwd, char __user *, b
 				error = -EFAULT;
 		}
 	} else {
-		write_sequnlock(&rename_lock);
+		read_sequnlock(&rename_lock);
 	}
 
 out:

@@ -2947,12 +2947,12 @@ resume:
 	if (!locked && read_seqretry(&rename_lock, seq))
 		goto rename_retry;
 	if (locked)
-		write_sequnlock(&rename_lock);
+		read_sequnlock(&rename_lock);
 	return;
 
 rename_retry:
 	locked = 1;
-	write_seqlock(&rename_lock);
+	read_seqlock(&rename_lock);
 	goto again;
 }
include/linux/seqlock.h +16 -0
Index: linux-2.6.git/include/linux/seqlock.h
===================================================================
@@ -108,6 +108,22 @@ static __always_inline int read_seqretry
 	return unlikely(sl->sequence != start);
 }
 
+/*
+ * Lock/unlock the critical section against updates but
+ * no sequence counter altering, so the other lockless
+ * readers will not have to restart reading data if
+ * read-only-lock occured.
+ */
+
+static __always_inline void read_seqlock(seqlock_t *sl)
+{
+	spin_lock(&sl->lock);
+}
+
+static __always_inline void read_sequnlock(seqlock_t *sl)
+{
+	spin_unlock(&sl->lock);
+}
 
 /*
  * Version using sequence counter only.
author
#2 lore raw

PATCH 2/3 vfs, dcache: Factor out rename_lock locking

In reply to Cyrill Gorcunov

From: Pavel Emelyanov <[email protected]>
Subject: [patch 2/3] vfs, dcache: Factor out rename_lock locking

The set of routnes that try to get the dentry path with various flavours end up in two "core" routines -- the prepend_path and the __dentry_path.

Since the rename_lock only protects the path integrity, it's OK to move the rename_lock locking inside these two.

Signed-off-by Pavel Emelyanov <[email protected]>
Signed-off-by Cyrill Gorcunov <[email protected]>
---
 fs/dcache.c |   27 ++++++---------------------
 1 file changed, 6 insertions(+), 21 deletions(-)
fs/dcache.c +6 -21
Index: linux-2.6.git/fs/dcache.c
===================================================================
@@ -2489,6 +2489,7 @@ static int prepend_path(const struct pat
 	bool slash = false;
 	int error = 0;
 
+	read_seqlock(&rename_lock);
 	br_read_lock(vfsmount_lock);
 	while (dentry != root->dentry || vfsmnt != root->mnt) {
 		struct dentry * parent;

@@ -2521,6 +2522,7 @@ out:
 		error = prepend(buffer, buflen, "/", 1);
 
 	br_read_unlock(vfsmount_lock);
+	read_sequnlock(&rename_lock);
 	return error;
 
 global_root:

@@ -2562,10 +2564,7 @@ char *__d_path(const struct path *path, 
 	int error;
 
 	prepend(&res, &buflen, "\0", 1);
-	read_seqlock(&rename_lock);
 	error = prepend_path(path, root, &res, &buflen);
-	read_sequnlock(&rename_lock);
-
 	if (error)
 		return ERR_PTR(error);
 	return res;

@@ -2626,12 +2625,10 @@ char *d_path(const struct path *path, ch
 		return path->dentry->d_op->d_dname(path->dentry, buf, buflen);
 
 	get_fs_root(current->fs, &root);
-	read_seqlock(&rename_lock);
 	tmp = root;
 	error = path_with_deleted(path, &tmp, &res, &buflen);
 	if (error)
 		res = ERR_PTR(error);
-	read_sequnlock(&rename_lock);
 	path_put(&root);
 	return res;
 }

@@ -2657,12 +2654,10 @@ char *d_path_with_unreachable(const stru
 		return path->dentry->d_op->d_dname(path->dentry, buf, buflen);
 
 	get_fs_root(current->fs, &root);
-	read_seqlock(&rename_lock);
 	tmp = root;
 	error = path_with_deleted(path, &tmp, &res, &buflen);
 	if (!error && !path_equal(&tmp, &root))
 		error = prepend_unreachable(&res, &buflen);
-	read_sequnlock(&rename_lock);
 	path_put(&root);
 	if (error)
 		res =  ERR_PTR(error);

@@ -2706,6 +2701,7 @@ static char *__dentry_path(struct dentry
 	retval = end-1;
 	*retval = '/';
 
+	read_seqlock(&rename_lock);
 	while (!IS_ROOT(dentry)) {
 		struct dentry *parent = dentry->d_parent;
 		int error;

@@ -2720,20 +2716,16 @@ static char *__dentry_path(struct dentry
 		retval = end;
 		dentry = parent;
 	}
+	read_sequnlock(&rename_lock);
 	return retval;
 Elong:
+	read_sequnlock(&rename_lock);
 	return ERR_PTR(-ENAMETOOLONG);
 }
 
 char *dentry_path_raw(struct dentry *dentry, char *buf, int buflen)
 {
-	char *retval;
-
-	read_seqlock(&rename_lock);
-	retval = __dentry_path(dentry, buf, buflen);
-	read_sequnlock(&rename_lock);
-
-	return retval;
+	return __dentry_path(dentry, buf, buflen);
 }
 EXPORT_SYMBOL(dentry_path_raw);
 

@@ -2742,7 +2734,6 @@ char *dentry_path(struct dentry *dentry,
 	char *p = NULL;
 	char *retval;
 
-	read_seqlock(&rename_lock);
 	if (d_unlinked(dentry)) {
 		p = buf + buflen;
 		if (prepend(&p, &buflen, "//deleted", 10) != 0)

@@ -2750,7 +2741,6 @@ char *dentry_path(struct dentry *dentry,
 		buflen++;
 	}
 	retval = __dentry_path(dentry, buf, buflen);
-	read_sequnlock(&rename_lock);
 	if (!IS_ERR(retval) && p)
 		*p = '/';	/* restore '/' overriden with '\0' */
 	return retval;

@@ -2788,7 +2778,6 @@ SYSCALL_DEFINE2(getcwd, char __user *, b
 	get_fs_root_and_pwd(current->fs, &root, &pwd);
 
 	error = -ENOENT;
-	read_seqlock(&rename_lock);
 	if (!d_unlinked(pwd.dentry)) {
 		unsigned long len;
 		struct path tmp = root;

@@ -2797,7 +2786,6 @@ SYSCALL_DEFINE2(getcwd, char __user *, b
 
 		prepend(&cwd, &buflen, "\0", 1);
 		error = prepend_path(&pwd, &tmp, &cwd, &buflen);
-		read_sequnlock(&rename_lock);
 
 		if (error)
 			goto out;

@@ -2816,10 +2804,7 @@ SYSCALL_DEFINE2(getcwd, char __user *, b
 			if (copy_to_user(buf, cwd, len))
 				error = -EFAULT;
 		}
-	} else {
-		read_sequnlock(&rename_lock);
 	}
-
 out:
 	path_put(&pwd);
 	path_put(&root);
author
#3 lore raw

PATCH 3/3 vfs: Make the rename_lock per-sb

In reply to Cyrill Gorcunov

From: Pavel Emelyanov <[email protected]>
Subject: [patch 3/3] Make the rename_lock per-sb

There are two things I try to get with this patch.

The first one -- renames on different sb-s will not depend on each other. The second one -- I plan to factor out the dcache shrinking code (in further patches). In order to make the srink_dcache_for_umount work OK on a dying SB it's better to have this lock non-global.

There's some trickery in the prepend_path -- this one has to relock the rename lock as it ascends the vfsmount tree. This relock seems OK, as the dentry path of each mount renames solid, and even if we "race" with the rename on some of the super blocks, the overall path we get will be either the one we had before rename or them one we had after one, i.e. -- some existing one.

v2: Use a pointer s_rename_lock in nfs_path() instead of
dereferences in cycle.

Signed-off-by Pavel Emelyanov <[email protected]>
Signed-off-by Cyrill Gorcunov <[email protected]>
---
 drivers/staging/pohmelfs/path_entry.c |    4 +-
 fs/autofs4/waitq.c                    |    6 +--
 fs/dcache.c                           |   60 +++++++++++++++++-----------------
 fs/nfs/namespace.c                    |    7 ++-
 fs/super.c                            |    1 
 include/linux/dcache.h                |    2 -
 include/linux/fs.h                    |    2 +
 kernel/auditsc.c                      |    4 +-
 8 files changed, 44 insertions(+), 42 deletions(-)
drivers/staging/pohmelfs/path_entry.c +2 -2
Index: linux-2.6.git/drivers/staging/pohmelfs/path_entry.c
===================================================================
@@ -99,7 +99,7 @@ int pohmelfs_path_length(struct pohmelfs
 rename_retry:
 	len = 1; /* Root slash */
 	d = first;
-	seq = read_seqbegin(&rename_lock);
+	seq = read_seqbegin(&root->d_sb->s_rename_lock);
 	rcu_read_lock();
 
 	if (!IS_ROOT(d) && d_unhashed(d))

@@ -110,7 +110,7 @@ rename_retry:
 		d = d->d_parent;
 	}
 	rcu_read_unlock();
-	if (read_seqretry(&rename_lock, seq))
+	if (read_seqretry(&root->d_sb->s_rename_lock, seq))
 		goto rename_retry;
 
 	dput(root);
fs/autofs4/waitq.c +3 -3
Index: linux-2.6.git/fs/autofs4/waitq.c
===================================================================
@@ -195,7 +195,7 @@ rename_retry:
 	buf = *name;
 	len = 0;
 
-	seq = read_seqbegin(&rename_lock);
+	seq = read_seqbegin(&sbi->sb->s_rename_lock);
 	rcu_read_lock();
 	spin_lock(&sbi->fs_lock);
 	for (tmp = dentry ; tmp != root ; tmp = tmp->d_parent)

@@ -204,7 +204,7 @@ rename_retry:
 	if (!len || --len > NAME_MAX) {
 		spin_unlock(&sbi->fs_lock);
 		rcu_read_unlock();
-		if (read_seqretry(&rename_lock, seq))
+		if (read_seqretry(&sbi->sb->s_rename_lock, seq))
 			goto rename_retry;
 		return 0;
 	}

@@ -220,7 +220,7 @@ rename_retry:
 	}
 	spin_unlock(&sbi->fs_lock);
 	rcu_read_unlock();
-	if (read_seqretry(&rename_lock, seq))
+	if (read_seqretry(&sbi->sb->s_rename_lock, seq))
 		goto rename_retry;
 
 	return len;
fs/dcache.c +30 -30

207 lines not on this page · raw message

fs/nfs/namespace.c +4 -3
Index: linux-2.6.git/fs/nfs/namespace.c
===================================================================
@@ -47,6 +47,7 @@ static struct vfsmount *nfs_do_submount(
  */
 char *nfs_path(char **p, struct dentry *dentry, char *buffer, ssize_t buflen)
 {
+	seqlock_t *p_rename_lock = &dentry->d_sb->s_rename_lock;
 	char *end;
 	int namelen;
 	unsigned seq;

@@ -57,7 +58,7 @@ rename_retry:
 	*--end = '\0';
 	buflen--;
 
-	seq = read_seqbegin(&rename_lock);
+	seq = read_seqbegin(p_rename_lock);
 	rcu_read_lock();
 	while (1) {
 		spin_lock(&dentry->d_lock);

@@ -73,7 +74,7 @@ rename_retry:
 		spin_unlock(&dentry->d_lock);
 		dentry = dentry->d_parent;
 	}
-	if (read_seqretry(&rename_lock, seq)) {
+	if (read_seqretry(p_rename_lock, seq)) {
 		spin_unlock(&dentry->d_lock);
 		rcu_read_unlock();
 		goto rename_retry;

@@ -112,7 +113,7 @@ rename_retry:
 Elong_unlock:
 	spin_unlock(&dentry->d_lock);
 	rcu_read_unlock();
-	if (read_seqretry(&rename_lock, seq))
+	if (read_seqretry(p_rename_lock, seq))
 		goto rename_retry;
 Elong:
 	return ERR_PTR(-ENAMETOOLONG);
fs/super.c +1 -0
Index: linux-2.6.git/fs/super.c
===================================================================
@@ -105,6 +105,7 @@ static struct super_block *alloc_super(s
 		s->s_count = 1;
 		atomic_set(&s->s_active, 1);
 		mutex_init(&s->s_vfs_rename_mutex);
+		seqlock_init(&s->s_rename_lock);
 		lockdep_set_class(&s->s_vfs_rename_mutex, &type->s_vfs_rename_key);
 		mutex_init(&s->s_dquot.dqio_mutex);
 		mutex_init(&s->s_dquot.dqonoff_mutex);
include/linux/dcache.h +0 -2
Index: linux-2.6.git/include/linux/dcache.h
===================================================================
@@ -219,8 +219,6 @@ struct dentry_operations {
 #define DCACHE_MANAGED_DENTRY \
 	(DCACHE_MOUNTED|DCACHE_NEED_AUTOMOUNT|DCACHE_MANAGE_TRANSIT)
 
-extern seqlock_t rename_lock;
-
 static inline int dname_external(struct dentry *dentry)
 {
 	return dentry->d_name.name != dentry->d_iname;
include/linux/fs.h +2 -0
Index: linux-2.6.git/include/linux/fs.h
===================================================================
@@ -1421,6 +1421,8 @@ struct super_block {
 	 */
 	struct mutex s_vfs_rename_mutex;	/* Kludge */
 
+	seqlock_t s_rename_lock;
+
 	/*
 	 * Filesystem subtype.  If non-empty the filesystem type field
 	 * in /proc/mounts will be "type.subtype"
kernel/auditsc.c +2 -2
Index: linux-2.6.git/kernel/auditsc.c
===================================================================
@@ -1779,7 +1779,7 @@ retry:
 	drop = NULL;
 	d = dentry;
 	rcu_read_lock();
-	seq = read_seqbegin(&rename_lock);
+	seq = read_seqbegin(&dentry->d_sb->s_rename_lock);
 	for(;;) {
 		struct inode *inode = d->d_inode;
 		if (inode && unlikely(!hlist_empty(&inode->i_fsnotify_marks))) {

@@ -1797,7 +1797,7 @@ retry:
 			break;
 		d = parent;
 	}
-	if (unlikely(read_seqretry(&rename_lock, seq) || drop)) {  /* in this order */
+	if (unlikely(read_seqretry(&dentry->d_sb->s_rename_lock, seq) || drop)) {  /* in this order */
 		rcu_read_unlock();
 		if (!drop) {
 			/* just a race with rename */