Re: [PATCH 05/13] xfs: add unlink list pointers to xfs_inode

[Dave Chinner <david@fromorbit.com>]

On Sat, Aug 22, 2020 at 10:03:39AM +0100, Christoph Hellwig wrote:
> On Wed, Aug 12, 2020 at 07:25:48PM +1000, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > To move away from using the on disk inode buffers to track and log
> > unlinked inodes, we need pointers to track them in memory. Because
> > we have arbitrary remove order from the list, it needs to be a
> > double linked list.
> > 
> > We start by noting that inodes are always in memory when they are
> > active on the unlinked list, and hence we can track these inodes
> > without needing to take references to the inodes or store them in
> > the list. We cannot, however, use inode locks to protect the inodes
> > on the list - the list needs an external lock to serialise all
> > inserts and removals. We can use the existing AGI buffer lock for
> > this right now as that already serialises all unlinked list
> > traversals and modifications.
> > 
> > Hence we can convert the in-memory unlinked list to a simple
> > list_head list in the perag. We can use list_empty() to detect an
> > empty unlinked list, likewise we can detect the end of the list when
> > the inode next pointer points back to the perag list_head. This
> > makes insert, remove and traversal.
> > 
> > The only complication here is log recovery of old filesystems that
> > have multiple lists. These always remove from the head of the list,
> > so we can easily construct just enough of the unlinked list for
> > recovery from any list to work correctly.
> 
> I'd much prefer not bloating the inode for the relative rate case of
> inode unlinked while still open.  Can't we just allocate a temporary
> structure with the list_head and inode pointer instead?

That's precisely the complexity this code gets rid of. i.e. the
complex reverse pointer mapping rhashtable that had to be able to
handle rhashtable memory allocation failures and so required
fallbacks to straight buffer based unlink list walking. I much
prefer that we burn a little bit more memory for *much* simpler,
faster and more flexible code....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com