linux-kernel - Re: [2.6.27-rc4] XFS i_lock vs i

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 25 Aug 2008 08:59:33 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Dave Chinner <david@...morbit.com>
Cc:	Lachlan McIlroy <lachlan@....com>,
	Daniel J Blueman <daniel.blueman@...il.com>,
	Linux Kernel <linux-kernel@...r.kernel.org>, xfs@....sgi.com,
	hch@....de
Subject: Re: [2.6.27-rc4] XFS i_lock vs i_iolock...

On Mon, 2008-08-25 at 13:55 +1000, Dave Chinner wrote:
> On Mon, Aug 25, 2008 at 12:12:23PM +1000, Lachlan McIlroy wrote:
> > Dave Chinner wrote:
> >> On Fri, Aug 22, 2008 at 10:12:59PM +0100, Daniel J Blueman wrote:
> >>> On 2.6.27-rc4 with various debug options enabled, lockdep claims lock
> >>> ordering issues with XFS [1] - easiest reproducer is just running
> >>> xfs_fsr. Mount options I was using were
> >>> 'nobarrier,noatime,nodiratime'.
> >>>
> >>> Thanks,
> >>>   Daniel
> >>>
> >>> --- [1]
> >>>
> >>> =======================================================
> >>> [ INFO: possible circular locking dependency detected ]
> >>> 2.6.27-rc4-224c #1
> >>> -------------------------------------------------------
> >>> xfs_fsr/5763 is trying to acquire lock:
> >>>  (&(&ip->i_lock)->mr_lock/2){--..}, at: [<ffffffff803ad8fc>] xfs_ilock+0x8c/0xb0
> >>>
> >>> but task is already holding lock:
> >>>  (&(&ip->i_iolock)->mr_lock/3){--..}, at: [<ffffffff803ad915>]
> >>> xfs_ilock+0xa5/0xb0
> >>
> >> False positive. We do:
> >>
> >> 	xfs_lock_two_inodes(ip, tip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL);
> >
> > Why not just change the above line to two lines:
> > 	xfs_lock_two_inodes(ip, tip, XFS_IOLOCK_EXCL);
> > 	xfs_lock_two_inodes(ip, tip, XFS_ILOCK_EXCL);
> 
> Yeah, that'd work, but it implllies that we no longer allow
> xfs_lock_two_inodes() to take both inode locks at once. 

How can you take two locks in one go? It seems to me you always need to
take them one after another, and as soon as you do that, you have
ordering constraints.

Of course it could be that doesn't matter, because there is another
serializing lock, but that isn't clear from this context.

> It
> would need a comment blaming^Wexplaining why lockdep requires
> us to do this, and then debug code in xfs_lock_two_inodes() to
> catch this when someone makes this mistake again in the future.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/