xfs
[Top] [All Lists]

Re: [PATCH] XFS: random cleanups of xfs_swap_extents

To: "Josef 'Jeff' Sipek" <jeffpc@xxxxxxxxxxxxxx>
Subject: Re: [PATCH] XFS: random cleanups of xfs_swap_extents
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Wed, 12 Nov 2008 05:09:58 -0500
Cc: XFS Mailing List <xfs@xxxxxxxxxxx>
In-reply-to: <20081107230054.GH26208@xxxxxxxxxxxxxx>
References: <20081107230054.GH26208@xxxxxxxxxxxxxx>
User-agent: Mutt/1.5.18 (2008-05-17)
On Fri, Nov 07, 2008 at 06:00:54PM -0500, Josef 'Jeff' Sipek wrote:
> XFS: random cleanups of xfs_swap_extents
> 
> From: Josef 'Jeff' Sipek <jeffpc@xxxxxxxxxxxxxx>
> 
> 1) remove lock_flags var since it's never modified and only obfuscates the
> code
> 
> 2) calling kfree/vfree on a NULL is valid

Looks good to me, but I'd go even further and kill the stupid locked
variable by having the exit path structured as:

 out_trans_cancel:
        xfs_trans_cancel(tp, 0);
 out_unlock:
        xfs_iunlock(ip,  XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
        xfs_iunlock(tip, XFS_ILOCK_EXCL | XFS_IOLOCK_EXCL);
 out_free:
        kmem_free(tempifp);
 out:
        return error;
}

With the one case that does a trans_cancel with partially unlocked
inodes handcoding most of it.

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