Re: [PATCH 2/5] xfs: use a cursor for bulk AIL insertion

[Christoph Hellwig <hch@infradead.org>]

> +	do {
> +		/* no placeholder, so get our insert location */
> +		if (!lip)
> +			lip = __xfs_trans_ail_cursor_last(ailp, cur,
> +								lsn, false);
> +
> +		if (!lip) {
> +			/*
> +			 * The list is empty, so just splice and return.  Our
> +			 * cursor is already guaranteed to be up to date, so we
> +			 * don't need to touch it here.
> +			 */
> +			list_splice(list, &ailp->xa_ail);
> +			return;
> +		}
> +
> +		/* The placeholder was invalidated, need to get a new cursor */
> +		if ((__psint_t)lip & 1)
> +			lip = NULL;
> +
> +	} while (lip == NULL);

Why do we even need a loop here?  Given that we're under xa_lock
no new cursor will get invalidated.  Isn't the simple code below
equivalent?

	/* no valid placeholder, get us a useful one */
	if (!lip || (__psint_t)lip & 1))
		lip = __xfs_trans_ail_cursor_last(ailp, cur, lsn, false);

	if (!lip) {
		/*
		 * The list is empty, so just splice and return.  Our
		 * cursor is already guaranteed to be up to date, so we
		 * don't need to touch it here.
		 */
		list_splice(list, &ailp->xa_ail);
		return;
	}

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs