xfs
[Top] [All Lists]

Re: [PATCH] xfs: re-organize XFS_ILOCK asserts in xfs_itruncate_extents(

To: Geoffrey Wehrman <gwehrman@xxxxxxx>
Subject: Re: [PATCH] xfs: re-organize XFS_ILOCK asserts in xfs_itruncate_extents()
From: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
Date: Mon, 28 Jan 2013 11:56:00 -0500
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20130128165257.GB7124@xxxxxxx>
References: <1359391710-3862-1-git-send-email-cmaiolino@xxxxxxxxxx> <20130128165257.GB7124@xxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Mon, Jan 28, 2013 at 10:52:57AM -0600, Geoffrey Wehrman wrote:
> On Mon, Jan 28, 2013 at 11:48:30AM -0500, Carlos Maiolino wrote:
> | An logically OR'red assert for check an inode locked in XFS_ILOCK_EXCL and
> | XFS_IOLOCK_EXCL looks better than the old way, avoiding possible mistakes 
> while
> | readin the code
> | 
> | Signed-off-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
> | ---
> |  fs/xfs/xfs_inode.c | 5 +++--
> |  1 file changed, 3 insertions(+), 2 deletions(-)
> | 
> | diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> | index 66282dc..f7efe77 100644
> | --- a/fs/xfs/xfs_inode.c
> | +++ b/fs/xfs/xfs_inode.c
> | @@ -1395,9 +1395,10 @@ xfs_itruncate_extents(
> |     int                     error = 0;
> |     int                     done = 0;
> |  
> | -   ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> | -   ASSERT(!atomic_read(&VFS_I(ip)->i_count) ||
> | +   ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL) ||
> |            xfs_isilocked(ip, XFS_IOLOCK_EXCL));
> | +   ASSERT(!atomic_read(&VFS_I(ip)->i_count));
> | +   ASSERT(!atomic_read(&VFS_I(ip)->i_count));
> |     ASSERT(new_size <= XFS_ISIZE(ip));
> |     ASSERT(tp->t_flags & XFS_TRANS_PERM_LOG_RES);
> |     ASSERT(ip->i_itemp != NULL);
> | -- 
> | 1.8.1
> 
> NACK.  You are changing the logic of the asserts.  The original first
> assert indicates that the ILOCK is always locked.  The modified asserts
> allow eith the ILOCK or the IOLOCK to be locked.  This is not correct.
> 
Yeah, makes sense, I thought about a possible logic change, but didn't realize
it was really true.

Thanks for feedback.

forget about this idea then, too much for little gain :)
> 
> Geoffrey

-- 
Carlos

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