Re: [PATCH 3/5] xfs: add XFS_ITEM_UNSAFE for log item push return result

["Darrick J. Wong" <djwong@kernel.org>]

On Fri, Aug 23, 2024 at 07:04:37PM +0800, Long Li wrote:
> After pushing log items, the log item may have been freed, making it
> unsafe to access in tracepoints. This commit introduces XFS_ITEM_UNSAFE
> to indicate when an item might be freed during the item push operation.
> 
> Signed-off-by: Long Li <leo.lilong@huawei.com>
> ---
>  fs/xfs/xfs_stats.h     | 1 +
>  fs/xfs/xfs_trans.h     | 1 +
>  fs/xfs/xfs_trans_ail.c | 7 +++++++
>  3 files changed, 9 insertions(+)
> 
> diff --git a/fs/xfs/xfs_stats.h b/fs/xfs/xfs_stats.h
> index a61fb56ed2e6..9a7a020587cf 100644
> --- a/fs/xfs/xfs_stats.h
> +++ b/fs/xfs/xfs_stats.h
> @@ -86,6 +86,7 @@ struct __xfsstats {
>  	uint32_t		xs_push_ail_pushbuf;
>  	uint32_t		xs_push_ail_pinned;
>  	uint32_t		xs_push_ail_locked;
> +	uint32_t		xs_push_ail_unsafe;
>  	uint32_t		xs_push_ail_flushing;
>  	uint32_t		xs_push_ail_restarts;
>  	uint32_t		xs_push_ail_flush;
> diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
> index f06cc0f41665..fd4f04853fe2 100644
> --- a/fs/xfs/xfs_trans.h
> +++ b/fs/xfs/xfs_trans.h
> @@ -117,6 +117,7 @@ void	xfs_log_item_init(struct xfs_mount *mp, struct xfs_log_item *item,
>  #define XFS_ITEM_PINNED		1
>  #define XFS_ITEM_LOCKED		2
>  #define XFS_ITEM_FLUSHING	3
> +#define XFS_ITEM_UNSAFE		4
>  
>  /*
>   * This is the structure maintained for every active transaction.
> diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
> index 8ede9d099d1f..a5ab1ffb8937 100644
> --- a/fs/xfs/xfs_trans_ail.c
> +++ b/fs/xfs/xfs_trans_ail.c
> @@ -561,6 +561,13 @@ xfsaild_push(
>  
>  			stuck++;
>  			break;
> +		case XFS_ITEM_UNSAFE:
> +			/*
> +			 * The item may have been freed, so we can't access the
> +			 * log item here.
> +			 */
> +			XFS_STATS_INC(mp, xs_push_ail_unsafe);

Aha, so this is in reaction to Dave's comment "So, yeah, these failure
cases need to return something different to xfsaild_push() so it knows
that it is unsafe to reference the log item, even for tracing purposes."

What we're trying to communicate through xfsaild_push_item is that we've
cycled the AIL lock and possibly done something (e.g. deleting the log
item from the AIL) such that the lip reference might be stale.

Can we call this XFS_ITEM_STALEREF?  "Unsafe" is vague.

--D

> +			break;
>  		default:
>  			ASSERT(0);
>  			break;
> -- 
> 2.39.2
> 
>