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

[Christoph Hellwig <hch@xxxxxxxxxxxxx>]

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