Re: [PATCH 4/4] xfs: introduce inode unlink log item

[Dave Chinner <david@fromorbit.com>]

On Wed, Jul 01, 2020 at 06:31:59AM +0800, Gao Xiang wrote:
> On Tue, Jun 30, 2020 at 11:19:42AM -0700, Darrick J. Wong wrote:
> > On Tue, Jun 23, 2020 at 07:50:15PM +1000, Dave Chinner wrote:
> 
> ...
> 
> > > +
> > > +static uint64_t
> > > +xfs_iunlink_item_sort(
> > > +	struct xfs_log_item	*lip)
> > > +{
> > > +	return IUL_ITEM(lip)->iu_ino;
> > > +}
> > 
> > Oh, I see, ->iop_sort is supposed to return a sorting key for each log
> > item so that we can reorder the iunlink items to take locks in the
> > correct order.
> 
> Yes, so I'm not sure the naming of ->iop_sort... When I first saw the name,
> I thought it would be a compare function. (but after I read the code of
> xfs_trans_precommit_sort(), I found I'm wrong...)

Yeah, it's intended to return a sorting key, not do a sort. Naming
is hard, and this is an RFC so it's expected that the naming will
weird and need improvement. :)

As it is, I suspect that a generic filesystem object wide sorting
mechanism needs to be more complex than just returning a single 64
bit key. I think it will work generically if the key reference frame
is the same for all object - I kinda just chose the object's location
on disk for the reference frame and hence the use of the inode
number as the sort key value here.

However, I'm not 100% sure how that would end up working if we have
the object we are trying to sort doesn't have a defined physical
location.  That doesn't seem to be an issue right now - all of the
objects I want to sort have phsyical locations - so I've largely
ignored it this side of the problem. If we need to, we can change
how the sorting works when sorting by physical location is no longer
sufficient as this is purely an in-memory ordering issue...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com