Tejun Heo wrote:
This patch kills the following request flag testing macros.
blk_noretry_request()
blk_rq_started()
blk_pm_suspend_request()
blk_pm_resume_request()
blk_sorted_rq()
blk_barrier_rq()
blk_fua_rq()
All above macros test for single request flag and not used widely and
seem to contribute more to obscurity than readability.
Signed-off-by: Tejun Heo <htejun@xxxxxxxxx>
heh, I guess that's a manner of opinion :)
To me, your patch makes things less readable. Example:
- int is_barrier = blk_fs_request(rq) && blk_barrier_rq(rq);
+ int is_barrier = blk_fs_request(rq) && rq->flags & REQ_HARDBARRIER;
After your change is applied, the above statement is no longer visually consistent with itself. The reader must decode two different styles of test in his brain, as opposed to one.
I'm applying this under protest.
The code may be right, but it visually makes _zero_ sense.
The fact that "blk_barrier_rq()" only triggers on hard barriers means that it does what you describe in the changelog, but read the code and see how little sense it makes when you _read_ it. You just checked that the rq has a barrier (either a soft or a hard one), and now you check "again" whether it has a barrier.
That's what it looks like from reading the code. Confusing. And thus prone to bugs.