xfs
[Top] [All Lists]

Re: [PATCH 3/8] xfs: bulk AIL insertion during transaction commit

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 3/8] xfs: bulk AIL insertion during transaction commit
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Tue, 30 Nov 2010 17:40:57 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1290993152-20999-4-git-send-email-david@xxxxxxxxxxxxx>
References: <1290993152-20999-1-git-send-email-david@xxxxxxxxxxxxx> <1290993152-20999-4-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
Generally looks good, a few minor comments below:

> +void
> +xfs_trans_committed_bulk(
> +     struct xfs_ail          *ailp,
> +     struct xfs_log_vec      *log_vector,
> +     xfs_lsn_t               commit_lsn,
> +     int                     aborted)
> +{
> +#define LGIA_SIZE    32
> +     struct xfs_log_item     *lgia[LGIA_SIZE];

The lgia name seems a bit to obscure to me.  What about just log_items
as variable name, and LOG_ITEM_BATCH_SIZE or similar for the size?

> +                     spin_lock(&ailp->xa_lock);
> +                     xfs_trans_ail_update_bulk(ailp, lgia, LGIA_SIZE,
> +                                                     commit_lsn);
> +                     for (i = 0; i < LGIA_SIZE; i++)
> +                             IOP_UNPIN(lgia[i], 0);

> +             spin_lock(&ailp->xa_lock);
> +             xfs_trans_ail_update_bulk(ailp, lgia, i, commit_lsn);
> +             for (j = 0; j < i; j++)
> +                     IOP_UNPIN(lgia[j], 0);

It might be worth to factor these two little sniplets into a little
helper.

> + * Bulk update version of xfs_trans_ail_update.

That won't be a very useful comment anymore once xfs_trans_ail_update is
implemented as a wrapper around this function..

> +     struct xfs_log_item     **lgia,

Same naming comment here.

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