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

Christoph Hellwig
2011-07-04T11:16:10Z

> + 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;
}