public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfs: re-organize XFS_ILOCK asserts in xfs_itruncate_extents()
@ 2013-01-28 16:48 Carlos Maiolino
  2013-01-28 16:52 ` Geoffrey Wehrman
  2013-01-28 16:53 ` Carlos Maiolino
  0 siblings, 2 replies; 4+ messages in thread
From: Carlos Maiolino @ 2013-01-28 16:48 UTC (permalink / raw)
  To: xfs

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@redhat.com>
---
 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

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH] xfs: re-organize XFS_ILOCK asserts in xfs_itruncate_extents()
  2013-01-28 16:48 [PATCH] xfs: re-organize XFS_ILOCK asserts in xfs_itruncate_extents() Carlos Maiolino
@ 2013-01-28 16:52 ` Geoffrey Wehrman
  2013-01-28 16:56   ` Carlos Maiolino
  2013-01-28 16:53 ` Carlos Maiolino
  1 sibling, 1 reply; 4+ messages in thread
From: Geoffrey Wehrman @ 2013-01-28 16:52 UTC (permalink / raw)
  To: Carlos Maiolino; +Cc: xfs

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@redhat.com>
| ---
|  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.


Geoffrey

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] xfs: re-organize XFS_ILOCK asserts in xfs_itruncate_extents()
  2013-01-28 16:48 [PATCH] xfs: re-organize XFS_ILOCK asserts in xfs_itruncate_extents() Carlos Maiolino
  2013-01-28 16:52 ` Geoffrey Wehrman
@ 2013-01-28 16:53 ` Carlos Maiolino
  1 sibling, 0 replies; 4+ messages in thread
From: Carlos Maiolino @ 2013-01-28 16:53 UTC (permalink / raw)
  To: xfs

Please discard this patch, I left a doubled assert by mistake, my apologies for
that

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@redhat.com>
> ---
>  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
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

-- 
Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] xfs: re-organize XFS_ILOCK asserts in xfs_itruncate_extents()
  2013-01-28 16:52 ` Geoffrey Wehrman
@ 2013-01-28 16:56   ` Carlos Maiolino
  0 siblings, 0 replies; 4+ messages in thread
From: Carlos Maiolino @ 2013-01-28 16:56 UTC (permalink / raw)
  To: Geoffrey Wehrman; +Cc: xfs

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@redhat.com>
> | ---
> |  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

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-01-28 16:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-28 16:48 [PATCH] xfs: re-organize XFS_ILOCK asserts in xfs_itruncate_extents() Carlos Maiolino
2013-01-28 16:52 ` Geoffrey Wehrman
2013-01-28 16:56   ` Carlos Maiolino
2013-01-28 16:53 ` Carlos Maiolino

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox