From: Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org> To: "J. Bruce Fields" <bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> Cc: Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>, linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Theodore Ts'o <tytso-3s7WtUTddSA@public.gmane.org>, Andreas Dilger <adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org> Subject: Re: [PATCH 01/12] vfs: pull ext4's double-i_mutex-locking into common code Date: Wed, 10 Jul 2013 08:04:11 +1000 [thread overview] Message-ID: <20130709220411.GK3438@dastard> (raw) In-Reply-To: <1372882356-14168-2-git-send-email-bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> On Wed, Jul 03, 2013 at 04:12:25PM -0400, J. Bruce Fields wrote: > From: "J. Bruce Fields" <bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > > We want to do this elsewhere as well. > > Cc: "Theodore Ts'o" <tytso-3s7WtUTddSA@public.gmane.org> > Cc: Andreas Dilger <adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org> > Signed-off-by: J. Bruce Fields <bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > --- > fs/ext4/ext4.h | 2 -- > fs/ext4/ioctl.c | 4 ++-- > fs/ext4/move_extent.c | 40 ++-------------------------------------- > fs/inode.c | 29 +++++++++++++++++++++++++++++ > include/linux/fs.h | 3 +++ > 5 files changed, 36 insertions(+), 42 deletions(-) > > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index 5aae3d1..3590abe 100644 Just to throw a spanner in the works - have you considered that other filesystems might have different inode lock ordering rules? For example, XFS locks multiple inodes in ascending inode number order, not ordered by pointer address. Hence we end up different inode lock ordering at different layers of the stack and I can't see that ending well.... > diff --git a/fs/inode.c b/fs/inode.c > index 00d5fc3..b8afbc7 100644 > --- a/fs/inode.c > +++ b/fs/inode.c > @@ -980,6 +980,35 @@ void unlock_new_inode(struct inode *inode) > EXPORT_SYMBOL(unlock_new_inode); > > /** > + * lock_two_nondirectories - take two i_mutexes on non-directory objects > + * @inode1: first inode to lock > + * @inode2: second inode to lock > + */ > +void lock_two_nondirectories(struct inode *inode1, struct inode *inode2) > +{ > + if (inode1 < inode2) { > + mutex_lock_nested(&inode1->i_mutex, I_MUTEX_PARENT); > + mutex_lock_nested(&inode2->i_mutex, I_MUTEX_CHILD); > + } else { > + mutex_lock_nested(&inode2->i_mutex, I_MUTEX_PARENT); > + mutex_lock_nested(&inode1->i_mutex, I_MUTEX_CHILD); > + } > +} > +EXPORT_SYMBOL(lock_two_nondirectories); What makes this specific to non-directories? If it's not to be used for directory inodes, then there should be WARN_ON_ONCE() guards in the code... Cheers, Dave. -- Dave Chinner david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html
WARNING: multiple messages have this Message-ID (diff)
From: Dave Chinner <david@fromorbit.com> To: "J. Bruce Fields" <bfields@redhat.com> Cc: Al Viro <viro@zeniv.linux.org.uk>, linux-nfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, "Theodore Ts'o" <tytso@mit.edu>, Andreas Dilger <adilger.kernel@dilger.ca> Subject: Re: [PATCH 01/12] vfs: pull ext4's double-i_mutex-locking into common code Date: Wed, 10 Jul 2013 08:04:11 +1000 [thread overview] Message-ID: <20130709220411.GK3438@dastard> (raw) In-Reply-To: <1372882356-14168-2-git-send-email-bfields@redhat.com> On Wed, Jul 03, 2013 at 04:12:25PM -0400, J. Bruce Fields wrote: > From: "J. Bruce Fields" <bfields@redhat.com> > > We want to do this elsewhere as well. > > Cc: "Theodore Ts'o" <tytso@mit.edu> > Cc: Andreas Dilger <adilger.kernel@dilger.ca> > Signed-off-by: J. Bruce Fields <bfields@redhat.com> > --- > fs/ext4/ext4.h | 2 -- > fs/ext4/ioctl.c | 4 ++-- > fs/ext4/move_extent.c | 40 ++-------------------------------------- > fs/inode.c | 29 +++++++++++++++++++++++++++++ > include/linux/fs.h | 3 +++ > 5 files changed, 36 insertions(+), 42 deletions(-) > > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index 5aae3d1..3590abe 100644 Just to throw a spanner in the works - have you considered that other filesystems might have different inode lock ordering rules? For example, XFS locks multiple inodes in ascending inode number order, not ordered by pointer address. Hence we end up different inode lock ordering at different layers of the stack and I can't see that ending well.... > diff --git a/fs/inode.c b/fs/inode.c > index 00d5fc3..b8afbc7 100644 > --- a/fs/inode.c > +++ b/fs/inode.c > @@ -980,6 +980,35 @@ void unlock_new_inode(struct inode *inode) > EXPORT_SYMBOL(unlock_new_inode); > > /** > + * lock_two_nondirectories - take two i_mutexes on non-directory objects > + * @inode1: first inode to lock > + * @inode2: second inode to lock > + */ > +void lock_two_nondirectories(struct inode *inode1, struct inode *inode2) > +{ > + if (inode1 < inode2) { > + mutex_lock_nested(&inode1->i_mutex, I_MUTEX_PARENT); > + mutex_lock_nested(&inode2->i_mutex, I_MUTEX_CHILD); > + } else { > + mutex_lock_nested(&inode2->i_mutex, I_MUTEX_PARENT); > + mutex_lock_nested(&inode1->i_mutex, I_MUTEX_CHILD); > + } > +} > +EXPORT_SYMBOL(lock_two_nondirectories); What makes this specific to non-directories? If it's not to be used for directory inodes, then there should be WARN_ON_ONCE() guards in the code... Cheers, Dave. -- Dave Chinner david@fromorbit.com
next prev parent reply other threads:[~2013-07-09 22:04 UTC|newest] Thread overview: 88+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-07-03 20:12 [PATCH 00/12] Implement NFSv4 delegations, take 8 J. Bruce Fields 2013-07-03 20:12 ` [PATCH 01/12] vfs: pull ext4's double-i_mutex-locking into common code J. Bruce Fields [not found] ` <1372882356-14168-2-git-send-email-bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2013-07-09 10:49 ` Jeff Layton 2013-07-09 10:49 ` Jeff Layton 2013-07-09 15:48 ` Theodore Ts'o 2013-07-09 22:04 ` Dave Chinner [this message] 2013-07-09 22:04 ` Dave Chinner 2013-07-10 0:21 ` J. Bruce Fields 2013-07-10 0:21 ` J. Bruce Fields [not found] ` <20130710002120.GM32574-spRCxval1Z7TsXDwO4sDpg@public.gmane.org> 2013-07-10 2:09 ` Dave Chinner 2013-07-10 2:09 ` Dave Chinner 2013-07-10 2:40 ` J. Bruce Fields [not found] ` <20130710024059.GN32574-spRCxval1Z7TsXDwO4sDpg@public.gmane.org> 2013-07-10 3:38 ` Dave Chinner 2013-07-10 3:38 ` Dave Chinner 2013-07-10 21:26 ` J. Bruce Fields 2013-07-10 21:26 ` J. Bruce Fields [not found] ` <20130710212620.GB24548-spRCxval1Z7TsXDwO4sDpg@public.gmane.org> 2013-07-11 14:04 ` Jeff Layton 2013-07-11 14:04 ` Jeff Layton [not found] ` <20130711100406.21b08420-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org> 2013-07-12 22:07 ` J. Bruce Fields 2013-07-12 22:07 ` J. Bruce Fields 2013-07-03 20:12 ` [PATCH 02/12] vfs: don't use PARENT/CHILD lock classes for non-directories J. Bruce Fields [not found] ` <1372882356-14168-3-git-send-email-bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2013-07-09 10:50 ` Jeff Layton 2013-07-09 10:50 ` Jeff Layton [not found] ` <1372882356-14168-1-git-send-email-bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2013-07-03 20:12 ` [PATCH 03/12] vfs: rename I_MUTEX_QUOTA now that it's not used for quotas J. Bruce Fields 2013-07-03 20:12 ` J. Bruce Fields [not found] ` <1372882356-14168-4-git-send-email-bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2013-07-09 10:54 ` Jeff Layton 2013-07-09 10:54 ` Jeff Layton 2013-07-09 14:26 ` J. Bruce Fields 2013-07-09 14:31 ` Jeff Layton 2013-07-03 20:12 ` [PATCH 04/12] vfs: take i_mutex on renamed file J. Bruce Fields 2013-07-03 20:12 ` J. Bruce Fields [not found] ` <1372882356-14168-5-git-send-email-bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2013-07-09 10:59 ` Jeff Layton 2013-07-09 10:59 ` Jeff Layton 2013-07-03 20:12 ` [PATCH 05/12] locks: introduce new FL_DELEG lock flag J. Bruce Fields 2013-07-03 20:12 ` J. Bruce Fields [not found] ` <1372882356-14168-6-git-send-email-bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2013-07-09 11:00 ` Jeff Layton 2013-07-09 11:00 ` Jeff Layton 2013-07-03 20:12 ` [PATCH 06/12] locks: implement delegations J. Bruce Fields [not found] ` <1372882356-14168-7-git-send-email-bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2013-07-09 12:23 ` Jeff Layton 2013-07-09 12:23 ` Jeff Layton [not found] ` <20130709082300.206bf176-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org> 2013-07-09 14:41 ` J. Bruce Fields 2013-07-09 14:41 ` J. Bruce Fields 2013-07-03 20:12 ` [PATCH 07/12] namei: minor vfs_unlink cleanup J. Bruce Fields [not found] ` <1372882356-14168-8-git-send-email-bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2013-07-09 12:50 ` Jeff Layton 2013-07-09 12:50 ` Jeff Layton 2013-07-03 20:12 ` [PATCH 08/12] locks: break delegations on unlink J. Bruce Fields [not found] ` <1372882356-14168-9-git-send-email-bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2013-07-09 13:05 ` Jeff Layton 2013-07-09 13:05 ` Jeff Layton [not found] ` <20130709090506.71c96841-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org> 2013-07-09 13:07 ` Jeff Layton 2013-07-09 13:07 ` Jeff Layton 2013-07-09 15:58 ` J. Bruce Fields 2013-07-09 15:58 ` J. Bruce Fields 2013-07-09 16:02 ` Jeff Layton 2013-07-09 19:29 ` J. Bruce Fields 2013-07-09 19:29 ` J. Bruce Fields 2013-07-03 20:12 ` [PATCH 09/12] locks: helper functions for delegation breaking J. Bruce Fields [not found] ` <1372882356-14168-10-git-send-email-bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2013-07-09 13:09 ` Jeff Layton 2013-07-09 13:09 ` Jeff Layton 2013-07-09 19:31 ` J. Bruce Fields 2013-07-09 19:37 ` Jeff Layton 2013-07-09 13:23 ` Jeff Layton 2013-07-09 19:38 ` J. Bruce Fields 2013-07-09 20:28 ` Jeff Layton 2013-07-03 20:12 ` [PATCH 10/12] locks: break delegations on rename J. Bruce Fields [not found] ` <1372882356-14168-11-git-send-email-bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2013-07-09 13:14 ` Jeff Layton 2013-07-09 13:14 ` Jeff Layton 2013-07-03 20:12 ` [PATCH 11/12] locks: break delegations on link J. Bruce Fields [not found] ` <1372882356-14168-12-git-send-email-bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2013-07-09 13:16 ` Jeff Layton 2013-07-09 13:16 ` Jeff Layton [not found] ` <20130709091617.1c175da4-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org> 2013-07-09 20:41 ` J. Bruce Fields 2013-07-09 20:41 ` J. Bruce Fields 2013-07-03 20:12 ` [PATCH 12/12] locks: break delegations on any attribute modification J. Bruce Fields 2013-07-09 13:30 ` Jeff Layton [not found] ` <20130709093047.0096f061-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org> 2013-07-09 20:51 ` J. Bruce Fields 2013-07-09 20:51 ` J. Bruce Fields [not found] ` <20130709205101.GK32574-spRCxval1Z7TsXDwO4sDpg@public.gmane.org> 2013-07-09 21:19 ` J. Bruce Fields 2013-07-09 21:19 ` J. Bruce Fields [not found] ` <20130709211911.GL32574-spRCxval1Z7TsXDwO4sDpg@public.gmane.org> 2013-07-10 1:26 ` Jeff Layton 2013-07-10 1:26 ` Jeff Layton [not found] ` <20130709212625.7fdfc6e1-4QP7MXygkU+dMjc06nkz3ljfA9RmPOcC@public.gmane.org> 2013-07-10 19:33 ` J. Bruce Fields 2013-07-10 19:33 ` J. Bruce Fields 2013-07-09 23:57 ` Jeff Layton 2013-07-09 23:57 ` Jeff Layton -- strict thread matches above, loose matches on Subject: below -- 2013-09-05 16:30 [PATCH 00/12] Implement NFSv4 delegations, take 10 J. Bruce Fields 2013-09-05 16:30 ` [PATCH 01/12] vfs: pull ext4's double-i_mutex-locking into common code J. Bruce Fields 2013-04-17 1:46 [PATCH 00/12] Implement NFSv4 delegations, take 7 J. Bruce Fields 2013-04-17 1:46 ` [PATCH 01/12] vfs: pull ext4's double-i_mutex-locking into common code J. Bruce Fields 2013-02-03 16:31 [PATCH 00/12] Implement NFSv4 delegations, take 6 J. Bruce Fields 2013-02-03 16:31 ` [PATCH 01/12] vfs: pull ext4's double-i_mutex-locking into common code J. Bruce Fields 2012-10-16 22:01 [PATCH 00/12] Implement NFSv4 delegations, take 5 J. Bruce Fields [not found] ` <1350424897-26894-1-git-send-email-bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2012-10-16 22:01 ` [PATCH 01/12] vfs: pull ext4's double-i_mutex-locking into common code J. Bruce Fields 2012-10-16 22:01 ` J. Bruce Fields
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=20130709220411.GK3438@dastard \ --to=david-fqsqvqoi3ljby3ivrkzq2a@public.gmane.org \ --cc=adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org \ --cc=bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \ --cc=linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \ --cc=tytso-3s7WtUTddSA@public.gmane.org \ --cc=viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.