On Tue, Nov 30, 2010 at 05:40:57PM -0500, Christoph Hellwig wrote:
> 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.
lgia == "log item array"
> What about just log_items
> as variable name, and LOG_ITEM_BATCH_SIZE or similar for the size?
Ok, that's easy to do.
> > + 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.
I thought about it, but didn't because there was more code in
writing a helper. I'll do it anyway...
>
> > + * 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..
Yup, but when the wrapper is introduced the comment is updated
appropriately.
>
> > + struct xfs_log_item **lgia,
>
> Same naming comment here.
Will fix.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
