xfs
[Top] [All Lists]

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

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 2/5] xfs: use a cursor for bulk AIL insertion
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Mon, 4 Jul 2011 07:16:05 -0400
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1309757260-5484-3-git-send-email-david@xxxxxxxxxxxxx>
References: <1309757260-5484-1-git-send-email-david@xxxxxxxxxxxxx> <1309757260-5484-3-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
> +     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;
        }

<Prev in Thread] Current Thread [Next in Thread>