* [PATCH] xfs: replace bp->flags usage with predefined macros
@ 2011-06-29 0:53 Chandra Seetharaman
2011-06-29 4:46 ` Dave Chinner
0 siblings, 1 reply; 6+ messages in thread
From: Chandra Seetharaman @ 2011-06-29 0:53 UTC (permalink / raw)
To: XFS Mailing List; +Cc: Alex Elder
Cleanup: Replace bp->flags usage with predefined macros.
Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
---
diff --git a/fs/xfs/linux-2.6/xfs_buf.c b/fs/xfs/linux-2.6/xfs_buf.c
index 5e68099..8b24dc4 100644
--- a/fs/xfs/linux-2.6/xfs_buf.c
+++ b/fs/xfs/linux-2.6/xfs_buf.c
@@ -470,7 +470,7 @@ _xfs_buf_find(
* continue searching to the right for an exact match.
*/
if (bp->b_buffer_length != range_length) {
- ASSERT(bp->b_flags & XBF_STALE);
+ ASSERT(XFS_BUF_ISSTALE(bp));
rbp = &(*rbp)->rb_right;
continue;
}
@@ -516,7 +516,7 @@ found:
* it. We need to keep flags such as how we allocated the buffer memory
* intact here.
*/
- if (bp->b_flags & XBF_STALE) {
+ if (XFS_BUF_ISSTALE(bp)) {
ASSERT((bp->b_flags & _XBF_DELWRI_Q) == 0);
bp->b_flags &= XBF_MAPPED | _XBF_KMEM | _XBF_PAGES;
}
@@ -631,7 +631,7 @@ xfs_buf_read(
goto no_buffer;
} else {
/* We do not want read in the flags */
- bp->b_flags &= ~XBF_READ;
+ XFS_BUF_UNREAD(bp);
}
}
@@ -868,7 +868,7 @@ xfs_buf_rele(
ASSERT(atomic_read(&bp->b_hold) > 0);
if (atomic_dec_and_lock(&bp->b_hold, &pag->pag_buf_lock)) {
- if (!(bp->b_flags & XBF_STALE) &&
+ if (!XFS_BUF_ISSTALE(bp) &&
atomic_read(&bp->b_lru_ref)) {
xfs_buf_lru_add(bp);
spin_unlock(&pag->pag_buf_lock);
@@ -904,7 +904,7 @@ xfs_buf_cond_lock(
locked = down_trylock(&bp->b_sema) == 0;
if (locked)
XB_SET_OWNER(bp);
- else if (atomic_read(&bp->b_pin_count) && (bp->b_flags & XBF_STALE))
+ else if (atomic_read(&bp->b_pin_count) && XFS_BUF_ISSTALE(bp))
xfs_log_force(bp->b_target->bt_mount, 0);
trace_xfs_buf_cond_lock(bp, _RET_IP_);
@@ -933,7 +933,7 @@ xfs_buf_lock(
{
trace_xfs_buf_lock(bp, _RET_IP_);
- if (atomic_read(&bp->b_pin_count) && (bp->b_flags & XBF_STALE))
+ if (atomic_read(&bp->b_pin_count) && XFS_BUF_ISSTALE(bp))
xfs_log_force(bp->b_target->bt_mount, 0);
down(&bp->b_sema);
XB_SET_OWNER(bp);
@@ -954,7 +954,7 @@ xfs_buf_unlock(
{
if ((bp->b_flags & (XBF_DELWRI|_XBF_DELWRI_Q)) == XBF_DELWRI) {
atomic_inc(&bp->b_hold);
- bp->b_flags |= XBF_ASYNC;
+ XFS_BUF_ASYNC(bp);
xfs_buf_delwri_queue(bp, 0);
}
@@ -997,7 +997,7 @@ xfs_buf_iodone_work(
if (bp->b_iodone)
(*(bp->b_iodone))(bp);
- else if (bp->b_flags & XBF_ASYNC)
+ else if (XFS_BUF_ISASYNC(bp))
xfs_buf_relse(bp);
}
@@ -1010,9 +1010,9 @@ xfs_buf_ioend(
bp->b_flags &= ~(XBF_READ | XBF_WRITE | XBF_READ_AHEAD);
if (bp->b_error == 0)
- bp->b_flags |= XBF_DONE;
+ XFS_BUF_DONE(bp);
- if ((bp->b_iodone) || (bp->b_flags & XBF_ASYNC)) {
+ if ((bp->b_iodone) || XFS_BUF_ISASYNC(bp)) {
if (schedule) {
INIT_WORK(&bp->b_iodone_work, xfs_buf_iodone_work);
queue_work(xfslogd_workqueue, &bp->b_iodone_work);
@@ -1041,8 +1041,9 @@ xfs_bwrite(
{
int error;
- bp->b_flags |= XBF_WRITE;
- bp->b_flags &= ~(XBF_ASYNC | XBF_READ);
+ XFS_BUF_WRITE(bp);
+ XFS_BUF_UNASYNC(bp);
+ XFS_BUF_UNREAD(bp);
xfs_buf_delwri_dequeue(bp);
xfs_bdstrat_cb(bp);
@@ -1061,8 +1062,9 @@ xfs_bdwrite(
{
trace_xfs_buf_bdwrite(bp, _RET_IP_);
- bp->b_flags &= ~XBF_READ;
- bp->b_flags |= (XBF_DELWRI | XBF_ASYNC);
+ XFS_BUF_UNREAD(bp);
+ XFS_BUF_DELAYWRITE(bp);
+ XFS_BUF_ASYNC(bp);
xfs_buf_delwri_queue(bp, 1);
}
@@ -1108,7 +1110,7 @@ STATIC int
xfs_bioerror_relse(
struct xfs_buf *bp)
{
- int64_t fl = XFS_BUF_BFLAGS(bp);
+ int64_t async = XFS_BUF_ISASYNC(bp);
/*
* No need to wait until the buffer is unpinned.
* We aren't flushing it.
@@ -1122,7 +1124,7 @@ xfs_bioerror_relse(
XFS_BUF_DONE(bp);
XFS_BUF_STALE(bp);
XFS_BUF_CLR_IODONE_FUNC(bp);
- if (!(fl & XBF_ASYNC)) {
+ if (!async) {
/*
* Mark b_error and B_ERROR _both_.
* Lot's of chunkcache code assumes that.
@@ -1203,7 +1205,7 @@ xfs_buf_bio_end_io(
xfs_buf_ioerror(bp, -error);
- if (!error && xfs_buf_is_vmapped(bp) && (bp->b_flags & XBF_READ))
+ if (!error && xfs_buf_is_vmapped(bp) && (XFS_BUF_ISREAD(bp)))
invalidate_kernel_vmap_range(bp->b_addr, xfs_buf_vmap_len(bp));
_xfs_buf_ioend(bp, 1);
@@ -1223,19 +1225,19 @@ _xfs_buf_ioapply(
total_nr_pages = bp->b_page_count;
map_i = 0;
- if (bp->b_flags & XBF_ORDERED) {
- ASSERT(!(bp->b_flags & XBF_READ));
+ if (XFS_BUF_ISORDERED(bp)) {
+ ASSERT(!(XFS_BUF_ISREAD(bp)));
rw = WRITE_FLUSH_FUA;
} else if (bp->b_flags & XBF_LOG_BUFFER) {
ASSERT(!(bp->b_flags & XBF_READ_AHEAD));
bp->b_flags &= ~_XBF_RUN_QUEUES;
- rw = (bp->b_flags & XBF_WRITE) ? WRITE_SYNC : READ_SYNC;
+ rw = XFS_BUF_ISWRITE(bp) ? 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;
+ rw = XFS_BUF_ISWRITE(bp) ? WRITE_META : READ_META;
} else {
- rw = (bp->b_flags & XBF_WRITE) ? WRITE :
+ rw = XFS_BUF_ISWRITE(bp) ? WRITE :
(bp->b_flags & XBF_READ_AHEAD) ? READA : READ;
}
@@ -1289,14 +1291,13 @@ xfs_buf_iorequest(
{
trace_xfs_buf_iorequest(bp, _RET_IP_);
- if (bp->b_flags & XBF_DELWRI) {
+ if (XFS_BUF_ISDELAYWRITE(bp)) {
xfs_buf_delwri_queue(bp, 1);
return 0;
}
- if (bp->b_flags & XBF_WRITE) {
+ if (XFS_BUF_ISWRITE(bp))
xfs_buf_wait_unpin(bp);
- }
xfs_buf_hold(bp);
@@ -1622,7 +1623,7 @@ xfs_buf_delwri_dequeue(
int dequeued = 0;
spin_lock(dwlk);
- if ((bp->b_flags & XBF_DELWRI) && !list_empty(&bp->b_list)) {
+ if (XFS_BUF_ISDELAYWRITE(bp) && !list_empty(&bp->b_list)) {
ASSERT(bp->b_flags & _XBF_DELWRI_Q);
list_del_init(&bp->b_list);
dequeued = 1;
@@ -1650,7 +1651,7 @@ xfs_buf_delwri_promote(
struct xfs_buftarg *btp = bp->b_target;
long age = xfs_buf_age_centisecs * msecs_to_jiffies(10) + 1;
- ASSERT(bp->b_flags & XBF_DELWRI);
+ ASSERT(XFS_BUF_ISDELAYWRITE(bp));
ASSERT(bp->b_flags & _XBF_DELWRI_Q);
/*
@@ -1692,7 +1693,7 @@ xfs_buf_delwri_split(
INIT_LIST_HEAD(list);
spin_lock(dwlk);
list_for_each_entry_safe(bp, n, dwq, b_list) {
- ASSERT(bp->b_flags & XBF_DELWRI);
+ ASSERT(XFS_BUF_ISDELAYWRITE(bp));
if (!XFS_BUF_ISPINNED(bp) && !xfs_buf_cond_lock(bp)) {
if (!force &&
@@ -1703,7 +1704,7 @@ xfs_buf_delwri_split(
bp->b_flags &= ~(XBF_DELWRI|_XBF_DELWRI_Q|
_XBF_RUN_QUEUES);
- bp->b_flags |= XBF_WRITE;
+ XFS_BUF_WRITE(bp);
list_move_tail(&bp->b_list, list);
trace_xfs_buf_delwri_split(bp, _RET_IP_);
} else
@@ -1826,7 +1827,7 @@ xfs_flush_buftarg(
ASSERT(target == bp->b_target);
list_del_init(&bp->b_list);
if (wait) {
- bp->b_flags &= ~XBF_ASYNC;
+ XFS_BUF_UNASYNC(bp);
list_add(&bp->b_list, &wait_list);
}
xfs_bdstrat_cb(bp);
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] xfs: replace bp->flags usage with predefined macros
2011-06-29 0:53 [PATCH] xfs: replace bp->flags usage with predefined macros Chandra Seetharaman
@ 2011-06-29 4:46 ` Dave Chinner
2011-06-29 6:30 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: Dave Chinner @ 2011-06-29 4:46 UTC (permalink / raw)
To: Chandra Seetharaman; +Cc: Alex Elder, XFS Mailing List
On Tue, Jun 28, 2011 at 05:53:04PM -0700, Chandra Seetharaman wrote:
> Cleanup: Replace bp->flags usage with predefined macros.
>
> Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
Christoph can correct me if I'm wrong, but I'm pretty sure his long term
direction is to remove the XFS_BUF_* macros completely.
Even so, in any new code we add or modify in xfs_buf.c we tend to
open code the flags. Hence the only code there that still uses the
XFS_BUF_* macros for manipulating flags are those we haven't needed
to touch for a long time....
> ---
> diff --git a/fs/xfs/linux-2.6/xfs_buf.c b/fs/xfs/linux-2.6/xfs_buf.c
> index 5e68099..8b24dc4 100644
> --- a/fs/xfs/linux-2.6/xfs_buf.c
> +++ b/fs/xfs/linux-2.6/xfs_buf.c
> @@ -470,7 +470,7 @@ _xfs_buf_find(
> * continue searching to the right for an exact match.
> */
> if (bp->b_buffer_length != range_length) {
> - ASSERT(bp->b_flags & XBF_STALE);
> + ASSERT(XFS_BUF_ISSTALE(bp));
> rbp = &(*rbp)->rb_right;
> continue;
> }
> @@ -516,7 +516,7 @@ found:
> * it. We need to keep flags such as how we allocated the buffer memory
> * intact here.
> */
> - if (bp->b_flags & XBF_STALE) {
> + if (XFS_BUF_ISSTALE(bp)) {
> ASSERT((bp->b_flags & _XBF_DELWRI_Q) == 0);
> bp->b_flags &= XBF_MAPPED | _XBF_KMEM | _XBF_PAGES;
> }
And this code fragment is an example of why we are open coding the
flags checks: we do non-trivial open-coded flag manipulations
in many places, and hiding the fact that state is held in b_flags
makes it harder to read the code.
That is, the code as it stands in this case makes it obvious
that that if the buffer was stale, afterwards it is not stale
because the bit was cleared from b_flags. Using the macro hides the
fact that the stale state is held in b_flags and therefore cleared
by the conditional code and that the buffer is no longer stale...
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] xfs: replace bp->flags usage with predefined macros
2011-06-29 4:46 ` Dave Chinner
@ 2011-06-29 6:30 ` Christoph Hellwig
2011-06-29 20:33 ` Chandra Seetharaman
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2011-06-29 6:30 UTC (permalink / raw)
To: Dave Chinner; +Cc: XFS Mailing List, Chandra Seetharaman, Alex Elder
On Wed, Jun 29, 2011 at 02:46:16PM +1000, Dave Chinner wrote:
> On Tue, Jun 28, 2011 at 05:53:04PM -0700, Chandra Seetharaman wrote:
> > Cleanup: Replace bp->flags usage with predefined macros.
> >
> > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
>
> Christoph can correct me if I'm wrong, but I'm pretty sure his long term
> direction is to remove the XFS_BUF_* macros completely.
Yes, at least those that are simpler flags get/set/clear wrappers.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs: replace bp->flags usage with predefined macros
2011-06-29 6:30 ` Christoph Hellwig
@ 2011-06-29 20:33 ` Chandra Seetharaman
2011-06-29 21:58 ` Alex Elder
0 siblings, 1 reply; 6+ messages in thread
From: Chandra Seetharaman @ 2011-06-29 20:33 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: XFS Mailing List, Alex Elder
On Wed, 2011-06-29 at 02:30 -0400, Christoph Hellwig wrote:
> On Wed, Jun 29, 2011 at 02:46:16PM +1000, Dave Chinner wrote:
> > On Tue, Jun 28, 2011 at 05:53:04PM -0700, Chandra Seetharaman wrote:
> > > Cleanup: Replace bp->flags usage with predefined macros.
> > >
> > > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> >
> > Christoph can correct me if I'm wrong, but I'm pretty sure his long term
> > direction is to remove the XFS_BUF_* macros completely.
>
> Yes, at least those that are simpler flags get/set/clear wrappers.
So, the suggestion is to go the other way and remove all the wrappers
and use bp->flags directly ?
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs: replace bp->flags usage with predefined macros
2011-06-29 20:33 ` Chandra Seetharaman
@ 2011-06-29 21:58 ` Alex Elder
2011-06-29 22:02 ` Chandra Seetharaman
0 siblings, 1 reply; 6+ messages in thread
From: Alex Elder @ 2011-06-29 21:58 UTC (permalink / raw)
To: sekharan; +Cc: Christoph Hellwig, XFS Mailing List
On Wed, 2011-06-29 at 13:33 -0700, Chandra Seetharaman wrote:
> On Wed, 2011-06-29 at 02:30 -0400, Christoph Hellwig wrote:
> > On Wed, Jun 29, 2011 at 02:46:16PM +1000, Dave Chinner wrote:
> > > On Tue, Jun 28, 2011 at 05:53:04PM -0700, Chandra Seetharaman wrote:
> > > > Cleanup: Replace bp->flags usage with predefined macros.
> > > >
> > > > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> > >
> > > Christoph can correct me if I'm wrong, but I'm pretty sure his long term
> > > direction is to remove the XFS_BUF_* macros completely.
> >
> > Yes, at least those that are simpler flags get/set/clear wrappers.
>
> So, the suggestion is to go the other way and remove all the wrappers
> and use bp->flags directly ?
Yes, that's right.
Sorry about that. Chandra asked me for some things to do
on XFS so I fired a few small simple odds and ends I had
made note of along the way. This was one--I said that the
use of macros should be consistent. I said we should
eventually do away with them entirely, but I didn't recommend
he do that right now since this was being done by a new
contributor.
Chandra, I think if you're up to it, doing away with the
macros is the right way to go.
-Alex
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs: replace bp->flags usage with predefined macros
2011-06-29 21:58 ` Alex Elder
@ 2011-06-29 22:02 ` Chandra Seetharaman
0 siblings, 0 replies; 6+ messages in thread
From: Chandra Seetharaman @ 2011-06-29 22:02 UTC (permalink / raw)
To: aelder; +Cc: Christoph Hellwig, XFS Mailing List
On Wed, 2011-06-29 at 16:58 -0500, Alex Elder wrote:
> On Wed, 2011-06-29 at 13:33 -0700, Chandra Seetharaman wrote:
> > On Wed, 2011-06-29 at 02:30 -0400, Christoph Hellwig wrote:
> > > On Wed, Jun 29, 2011 at 02:46:16PM +1000, Dave Chinner wrote:
> > > > On Tue, Jun 28, 2011 at 05:53:04PM -0700, Chandra Seetharaman wrote:
> > > > > Cleanup: Replace bp->flags usage with predefined macros.
> > > > >
> > > > > Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
> > > >
> > > > Christoph can correct me if I'm wrong, but I'm pretty sure his long term
> > > > direction is to remove the XFS_BUF_* macros completely.
> > >
> > > Yes, at least those that are simpler flags get/set/clear wrappers.
> >
> > So, the suggestion is to go the other way and remove all the wrappers
> > and use bp->flags directly ?
>
> Yes, that's right.
>
> Sorry about that. Chandra asked me for some things to do
> on XFS so I fired a few small simple odds and ends I had
> made note of along the way. This was one--I said that the
> use of macros should be consistent. I said we should
> eventually do away with them entirely, but I didn't recommend
> he do that right now since this was being done by a new
> contributor.
>
> Chandra, I think if you're up to it, doing away with the
> macros is the right way to go.
Will do.
>
> -Alex
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-06-29 22:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-29 0:53 [PATCH] xfs: replace bp->flags usage with predefined macros Chandra Seetharaman
2011-06-29 4:46 ` Dave Chinner
2011-06-29 6:30 ` Christoph Hellwig
2011-06-29 20:33 ` Chandra Seetharaman
2011-06-29 21:58 ` Alex Elder
2011-06-29 22:02 ` Chandra Seetharaman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox