xfs
[Top] [All Lists]

Re: [PATCH 1/5] xfs: clean up inode locking for RENAME_WHITEOUT

To: Eric Sandeen <sandeen@xxxxxxxxxxx>
Subject: Re: [PATCH 1/5] xfs: clean up inode locking for RENAME_WHITEOUT
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 25 Mar 2015 08:26:47 +1100
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <5511D2FC.8080907@xxxxxxxxxxx>
References: <1427194771-3105-1-git-send-email-david@xxxxxxxxxxxxx> <1427194771-3105-2-git-send-email-david@xxxxxxxxxxxxx> <5511D2FC.8080907@xxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Mar 24, 2015 at 04:11:24PM -0500, Eric Sandeen wrote:
> On 3/24/15 5:59 AM, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> > 
> > When doing RENAME_WHITEOUT, we now have to lock 5 inodes into the
> > rename transaction. This means we need to update
> > xfs_sort_for_rename() and xfs_lock_inodes() to handle up to 5
> > inodes. Because of the vagaries of rename, this means we could have
> > anywhere between 3 and 5 inodes locked into the transaction....
> > 
> > While xfs_lock_inodes() does not need anything other than an assert
> > telling us we are passing more inodes that we ever thought we should
> > see, it could do with a logic rework to remove all the indenting.
> > This is not a functional change - it just makes the code a lot
> > easier to read.
> > 
> > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> ...
> 
> > @@ -2681,19 +2668,22 @@ xfs_remove(
> >  /*
> >   * Enter all inodes for a rename transaction into a sorted array.
> >   */
> > +#define __XFS_SORT_INODES  5
> >  STATIC void
> >  xfs_sort_for_rename(
> > -   xfs_inode_t     *dp1,   /* in: old (source) directory inode */
> > -   xfs_inode_t     *dp2,   /* in: new (target) directory inode */
> > -   xfs_inode_t     *ip1,   /* in: inode of old entry */
> > -   xfs_inode_t     *ip2,   /* in: inode of new entry, if it
> > -                              already exists, NULL otherwise. */
> > -   xfs_inode_t     **i_tab,/* out: array of inode returned, sorted */
> > -   int             *num_inodes)  /* out: number of inodes in array */
> > +   struct xfs_inode        *dp1,   /* in: old (source) directory inode */
> > +   struct xfs_inode        *dp2,   /* in: new (target) directory inode */
> > +   struct xfs_inode        *ip1,   /* in: inode of old entry */
> > +   struct xfs_inode        *ip2,   /* in: inode of new entry */
> > +   struct xfs_inode        *wino,  /* in: whiteout inode */
> 
> I'm not 100% morally opposed, but you still have a wino lurking around here ;)

Ah, I missed that one when splitting the patch. Will fix.

-Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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