Messages in this thread | | | Date | Wed, 15 Jul 2009 18:27:57 +0900 | From | Tejun Heo <> | Subject | Re: [PATCH 2/4] block: use the same failfast bits for bio and request |
| |
Hello, Boaz.
Boaz Harrosh wrote: >>> flags as bit positions generally only make sense if you use >>> test/set/clear_bit, otherwise they just confuse things. > > first please make a distinction between test/set/clear_bit and > test/__set/__clear_bit the former is not an option since it's not what > we need.
Block flags look like the way they do today because a while back they were accessed with atomic bitops (the versions w/o the underbars). Now that they're all inside spinlocks, it all became moot.
> What can be more clear then rq_set_flags(req, QUEUE_FLAG_QUEUED) then > rq_clear_flags(req, QUEUE_FLAG_QUEUED) later.
req->cmd_flags |= QUEUE_FLAG_QUEUED / &= ~QUEUE_FLAG_QUEUED might not be as clear but should be sufficient, I suppose.
> That's why I suggested the set/clear_flags() variable size macro > which can set/clear multiple bit-flags at same cost of masks, only > that the compiler calculates the mask in compile time. > > This can also be good for the greps above. .eg: > test_flags(&rq->cmd_flags, REQ_FAILFAST_DEV, REQ_FAILFAST_TRANSPORT, REQ_FAILFAST_DRIVER); ... > As said, yes, the the lower-case accessors for upper-case bits does nothing, > but use __set/__clear/test is a different matter that can also replace the > sugary need of these.
Heh.. I don't know. What about things like flags & mask == mask2 test? The vararg macros would work for most cases and I wouldn't be violently against them if they were already in place but I don't see much benefit of all those when people are already very accustomed to using c bitops to handle flags.
Thanks.
-- tejun
| |