Re: [PATCH 4/9] xfs: introduce xfs_iunlink_lookup

From: Christoph Hellwig <hch@infradead.org>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 4/9] xfs: introduce xfs_iunlink_lookup
Date: Wed, 29 Jun 2022 00:17:16 -0700	[thread overview]
Message-ID: <Yrv8fKfV4pxA7ZxR@infradead.org> (raw)
In-Reply-To: <20220627004336.217366-5-david@fromorbit.com>

On Mon, Jun 27, 2022 at 10:43:31AM +1000, Dave Chinner wrote:
> When an inode is on an unlinked list during normal operation, it is
> guaranteed to be pinned in memory as it is either referenced by the
> current unlink operation or it has a open file descriptor that
> references it and has it pinned in memory. Hence to look up an inode
> on the unlinked list, we can do a direct inode cache lookup and
> always expect the lookup to succeed.

> +	rcu_read_lock();
> +	ip = radix_tree_lookup(&pag->pag_ici_root, agino);
> +
> +	/* Inode not in memory, nothing to do */
> +	if (!ip) {
> +		rcu_read_unlock();
> +		return NULL;
> +	}

If the above commit log is true (which I think it is), the comment here
is wrong, and the check should probably grow a WARN_ON as well.

> +	spin_lock(&ip->i_flags_lock);
> +	if (ip->i_ino != XFS_AGINO_TO_INO(mp, pag->pag_agno, agino) ||
> +	    (ip->i_flags & (XFS_IRECLAIMABLE | XFS_IRECLAIM))) {
> +		/* Uh-oh! */
> +		ip = NULL;
> +	}

And this should not happen either and could use an assert.