xfs
[Top] [All Lists]

Re: [PATCH 5/5] xfs: assert that we hold the ilock for extent map access

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 5/5] xfs: assert that we hold the ilock for extent map access
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 6 Dec 2013 08:40:29 +1100
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20131205212219.GA12602@xxxxxxxxxxxxx>
References: <20131205155830.620826868@xxxxxxxxxxxxxxxxxxxxxx> <20131205155951.874279041@xxxxxxxxxxxxxxxxxxxxxx> <20131205211047.GG29897@dastard> <20131205212219.GA12602@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Dec 05, 2013 at 01:22:19PM -0800, Christoph Hellwig wrote:
> On Fri, Dec 06, 2013 at 08:10:47AM +1100, Dave Chinner wrote:
> > Looks good, but can we add an assert to xfs_bunmapi() at the same
> > time just to cover all the public bmapi interfaces with locking
> > requirements?
> 
> Sure, will do.
> 
> Btw, I got another idea to sort this mess out a bit better:
> 
>  - add a new XFS_ILOCK_BMAP flag, and fold the bmap locking magic
>    into xfs_ilock.
>  - because the flag is now passed down we can assert that it is
>    passed in xfs_bmapi_read and friends even if the extent list
>    is already read in and thus improve coverage.

Hmmm - I'm not sure I can see how that would work - the checks on
lock mode look at the lock directly, not at some other flag register
to indicate the locking context....

Are you thinking of expanding the code in xfs_isilocked() to handle
this as well? 

And what do we do with the unlock case, as we can't tell after the
fact from the inode state whether we locked shared or excl because
the extent list has now been read in....

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>