From: David Howells <dhowells@redhat.com>
To: peterz@infradead.org
Cc: dhowells@redhat.com, aviro@redhat.com, hch@infradead.org,
hramrach@centrum.cz, ikent@redhat.com,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
jmoyer@redhat.com, miklos@szeredi.hu
Subject: [PATCH 2/2] union-mount: Duplicate the i_{, dir_}mutex lock classes and use for upper layer
Date: Mon, 11 Jul 2011 15:50:21 +0100 [thread overview]
Message-ID: <20110711145021.18050.76571.stgit@warthog.procyon.org.uk> (raw)
In-Reply-To: <1310392930.18678.63.camel@twins>
Duplicate the i_mutex and i_dir_mutex lock classes and use for unionmount upper
layer superblock instead of the normal lock classes. This solves some of the
lockdep noise when the VFS tries to hold locks on inodes in both layers at the
same time. Note these only occur if both layers are of the same filesystem
type.
As far as I can tell, most of the lockdep warnings are false positives since
the inodes being locked are part of different superblocks; however, because
lockdep works on lock *classes*, it can't determine this.
I suspect that giving each superblock its own lock class would overextend
lockdep.
---
fs/inode.c | 48 ++++++++++++++++++++++++++++++++++++------------
fs/namespace.c | 2 +-
fs/super.c | 8 ++++++++
include/linux/fs.h | 5 +++--
4 files changed, 48 insertions(+), 15 deletions(-)
diff --git a/fs/inode.c b/fs/inode.c
index 43566d1..95d076d 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -173,8 +173,14 @@ int inode_init_always(struct super_block *sb, struct inode *inode)
spin_lock_init(&inode->i_lock);
lockdep_set_class(&inode->i_lock, &sb->s_type->i_lock_key);
+ /* Duplicate the code with separate indices so that when lockdep print
+ * a warning, the numeric index is seen.
+ */
mutex_init(&inode->i_mutex);
- lockdep_set_class(&inode->i_mutex, &sb->s_type->i_mutex_key);
+ if (sb->s_lock_class == 0)
+ lockdep_set_class(&inode->i_mutex, &sb->s_type->i_mutex_key[0]);
+ else
+ lockdep_set_class(&inode->i_mutex, &sb->s_type->i_mutex_key[1]);
init_rwsem(&inode->i_alloc_sem);
lockdep_set_class(&inode->i_alloc_sem, &sb->s_type->i_alloc_sem_key);
@@ -882,18 +888,36 @@ void unlock_new_inode(struct inode *inode)
{
#ifdef CONFIG_DEBUG_LOCK_ALLOC
if (S_ISDIR(inode->i_mode)) {
- struct file_system_type *type = inode->i_sb->s_type;
+ struct super_block *sb = inode->i_sb;
+ struct file_system_type *type = sb->s_type;
- /* Set new key only if filesystem hasn't already changed it */
- if (!lockdep_match_class(&inode->i_mutex,
- &type->i_mutex_key)) {
- /*
- * ensure nobody is actually holding i_mutex
- */
- mutex_destroy(&inode->i_mutex);
- mutex_init(&inode->i_mutex);
- lockdep_set_class(&inode->i_mutex,
- &type->i_mutex_dir_key);
+ /* Set new key only if filesystem hasn't already changed it
+ *
+ * Duplicate the code with separate indices so that when
+ * lockdep print a warning, the numeric index is seen.
+ */
+ if (sb->s_lock_class == 0) {
+ if (!lockdep_match_class(&inode->i_mutex,
+ &type->i_mutex_key[0])) {
+ /*
+ * ensure nobody is actually holding i_mutex
+ */
+ mutex_destroy(&inode->i_mutex);
+ mutex_init(&inode->i_mutex);
+ lockdep_set_class(&inode->i_mutex,
+ &type->i_mutex_dir_key[0]);
+ }
+ } else {
+ if (!lockdep_match_class(&inode->i_mutex,
+ &type->i_mutex_key[1])) {
+ /*
+ * ensure nobody is actually holding i_mutex
+ */
+ mutex_destroy(&inode->i_mutex);
+ mutex_init(&inode->i_mutex);
+ lockdep_set_class(&inode->i_mutex,
+ &type->i_mutex_dir_key[1]);
+ }
}
}
#endif
diff --git a/fs/namespace.c b/fs/namespace.c
index 18958fd..59f0942 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2611,7 +2611,7 @@ long do_mount(char *dev_name, char *dir_name, char *type_page,
flags &= ~(MS_NOSUID | MS_NOEXEC | MS_NODEV | MS_ACTIVE | MS_BORN |
MS_NOATIME | MS_NODIRATIME | MS_RELATIME| MS_KERNMOUNT |
- MS_STRICTATIME | MS_UNION);
+ MS_STRICTATIME);
if (flags & MS_REMOUNT)
retval = do_remount(&path, flags & ~MS_REMOUNT, mnt_flags,
diff --git a/fs/super.c b/fs/super.c
index 95a2ebc..cd60a34 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -74,6 +74,7 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags)
INIT_LIST_HEAD(&s->s_files);
#endif
s->s_flags = flags;
+ s->s_lock_class = (flags & MS_UNION) ? 1 : 0;
s->s_bdi = &default_backing_dev_info;
INIT_LIST_HEAD(&s->s_instances);
INIT_HLIST_BL_HEAD(&s->s_anon);
@@ -346,6 +347,13 @@ retry:
deactivate_locked_super(old);
goto retry;
}
+#ifdef CONFIG_UNION_MOUNT
+ if (unlikely((old->s_flags | flags) & MS_UNION)) {
+ up_write(&old->s_umount);
+ deactivate_locked_super(old);
+ return ERR_PTR(-EINVAL);
+ }
+#endif
return old;
}
}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 4bf3903..2fd73a9 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1373,6 +1373,7 @@ struct super_block {
dev_t s_dev; /* search index; _not_ kdev_t */
unsigned char s_dirt;
unsigned char s_blocksize_bits;
+ u8 s_lock_class; /* Set of lock classes to use */
unsigned long s_blocksize;
loff_t s_maxbytes; /* Max file size */
struct file_system_type *s_type;
@@ -1842,8 +1843,8 @@ struct file_system_type {
struct lock_class_key s_vfs_rename_key;
struct lock_class_key i_lock_key;
- struct lock_class_key i_mutex_key;
- struct lock_class_key i_mutex_dir_key;
+ struct lock_class_key i_mutex_key[2];
+ struct lock_class_key i_mutex_dir_key[2];
struct lock_class_key i_alloc_sem_key;
};
prev parent reply other threads:[~2011-07-11 14:50 UTC|newest]
Thread overview: 73+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-12 15:00 Unionmount status? Michal Suchanek
2011-04-12 20:31 ` Ric Wheeler
2011-04-12 21:36 ` Michal Suchanek
2011-04-13 14:18 ` Jiri Kosina
2011-04-13 15:13 ` Michal Suchanek
2011-04-14 8:38 ` Miklos Szeredi
2011-04-14 9:48 ` Sedat Dilek
2011-04-14 9:58 ` Miklos Szeredi
2011-04-15 11:22 ` Michal Suchanek
2011-04-15 11:31 ` Miklos Szeredi
2011-04-15 11:51 ` Michal Suchanek
2011-04-15 12:29 ` Miklos Szeredi
2011-04-15 12:34 ` Michal Suchanek
2011-04-15 12:48 ` Miklos Szeredi
2011-04-15 21:48 ` Hugh Dickins
2011-04-15 22:18 ` Andreas Dilger
2011-04-18 13:31 ` Michal Suchanek
2011-04-19 20:04 ` [PATCH] tmpfs: implement generic xattr support Miklos Szeredi
2011-04-20 2:18 ` Phillip Lougher
2011-04-20 13:43 ` Miklos Szeredi
2011-04-21 6:59 ` Michal Suchanek
2011-04-21 9:08 ` Miklos Szeredi
2011-04-21 10:59 ` Michal Suchanek
2011-04-21 14:58 ` Jordi Pujol
2011-04-21 15:22 ` Michal Suchanek
2011-04-21 15:43 ` Michal Suchanek
2011-04-21 17:26 ` Miklos Szeredi
2011-04-21 19:17 ` Michal Suchanek
2011-04-20 16:00 ` Serge E. Hallyn
2011-05-12 4:20 ` Hugh Dickins
2011-05-12 7:52 ` Michal Suchanek
2011-05-12 12:27 ` Miklos Szeredi
2011-05-12 14:00 ` Miklos Szeredi
2011-05-12 16:52 ` Hugh Dickins
2011-04-18 13:34 ` Unionmount status? Michal Suchanek
2011-04-18 13:37 ` Michal Suchanek
2011-04-13 17:26 ` Ric Wheeler
2011-04-13 18:58 ` Michal Suchanek
2011-04-13 19:11 ` Ric Wheeler
2011-04-13 19:47 ` Michal Suchanek
2011-04-14 4:50 ` Ian Kent
2011-04-14 9:32 ` Michal Suchanek
2011-04-14 9:40 ` Miklos Szeredi
2011-04-14 13:21 ` Ric Wheeler
2011-04-14 14:54 ` Michal Suchanek
2011-04-15 16:31 ` Ric Wheeler
2011-04-14 19:14 ` David Howells
2011-06-29 9:39 ` Union mount and overlayfs bake off? Ric Wheeler
2011-06-29 11:40 ` Michal Suchanek
2011-06-29 10:17 ` David Howells
2011-06-30 12:44 ` Miklos Szeredi
2011-07-10 8:28 ` Union mount and lockdep design issues Ric Wheeler
2011-07-10 13:48 ` Peter Zijlstra
2011-07-11 8:35 ` Michal Suchanek
2011-07-11 11:01 ` David Howells
2011-07-11 12:00 ` Peter Zijlstra
2011-07-11 13:36 ` Michal Suchanek
2011-07-11 13:50 ` Ian Kent
2011-07-11 16:17 ` Michal Suchanek
2011-07-11 17:23 ` Ian Kent
2011-07-11 18:08 ` Michal Suchanek
2011-07-12 8:30 ` Miklos Szeredi
2011-07-12 9:58 ` Michal Suchanek
2011-07-12 11:45 ` Miklos Szeredi
2011-07-12 18:49 ` Michal Suchanek
2011-07-13 9:49 ` Miklos Szeredi
2011-07-13 12:02 ` David Howells
2011-07-13 13:20 ` Miklos Szeredi
2011-07-14 0:57 ` David Howells
2011-07-11 13:54 ` David Howells
2011-07-11 14:02 ` Peter Zijlstra
2011-07-11 14:50 ` [PATCH 1/2] VFS: Pass mount flags to sget() David Howells
2011-07-11 14:50 ` David Howells [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110711145021.18050.76571.stgit@warthog.procyon.org.uk \
--to=dhowells@redhat.com \
--cc=aviro@redhat.com \
--cc=hch@infradead.org \
--cc=hramrach@centrum.cz \
--cc=ikent@redhat.com \
--cc=jmoyer@redhat.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=miklos@szeredi.hu \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®