xfs
[Top] [All Lists]

Re: [PATCH 26/27] xfs: cleanup I/O-related buffer flags

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 26/27] xfs: cleanup I/O-related buffer flags
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Wed, 6 Jul 2011 13:54:11 +1000
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20110701094607.536262104@xxxxxxxxxxxxxxxxxxxxxx>
References: <20110701094321.936534538@xxxxxxxxxxxxxxxxxxxxxx> <20110701094607.536262104@xxxxxxxxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-06-14)
On Fri, Jul 01, 2011 at 05:43:47AM -0400, Christoph Hellwig wrote:
> Remove the unused and misnamed _XBF_RUN_QUEUES flag, rename XBF_LOG_BUFFER
> to the more fitting XBF_SYNCIO, and split XBF_ORDERED into XBF_FUA and
> XBF_FLUSH to allow more fine grained control over the bio flags.  Also
> cleanup processing of the flags in _xfs_buf_ioapply to make more sense,
> and renumber the sparse flag number space to group flags by purpose.
> 
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> 
> -             bp->b_flags &= ~_XBF_RUN_QUEUES;
> -             rw = (bp->b_flags & XBF_WRITE) ? WRITE_SYNC : READ_SYNC;
> -     } else if (bp->b_flags & _XBF_RUN_QUEUES) {
> -             ASSERT(!(bp->b_flags & XBF_READ_AHEAD));
> -             bp->b_flags &= ~_XBF_RUN_QUEUES;
> -             rw = (bp->b_flags & XBF_WRITE) ? WRITE_META : READ_META;
> +     if (bp->b_flags & XBF_WRITE) {
> +             if (bp->b_flags & XBF_SYNCIO)
> +                     rw = WRITE_SYNC;
> +             else
> +                     rw = WRITE;
> +             if (bp->b_flags & XBF_FUA)
> +                     rw |= REQ_FUA;
> +             if (bp->b_flags & XBF_FLUSH)
> +                     rw |= REQ_FLUSH;
> +     } else if (bp->b_flags & XBF_READ_AHEAD) {
> +             rw = READA;
>       } else {
> -             rw = (bp->b_flags & XBF_WRITE) ? WRITE :
> -                  (bp->b_flags & XBF_READ_AHEAD) ? READA : READ;
> +             rw = READ;
>       }

Is it worthwhile tagging all these as READ_META and WRITE_META?
Though that probably needs to be done as a separate commit...

>  /* flags used only internally */
> -#define _XBF_PAGES   (1 << 18)/* backed by refcounted pages */
> -#define      _XBF_RUN_QUEUES (1 << 19)/* run block device task queue */
> -#define      _XBF_KMEM       (1 << 20)/* backed by heap memory */
> -#define _XBF_DELWRI_Q        (1 << 21)/* buffer on delwri queue */
> +#define _XBF_PAGES   (1 << 20)/* backed by refcounted pages */
> +#define      _XBF_KMEM       (1 << 21)/* backed by heap memory */
> +#define _XBF_DELWRI_Q        (1 << 22)/* buffer on delwri queue */

Might be worthwhile cleaning up the stray tab before _XBF_KMEM
there.

Otherwise looks good.

Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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