question about buffer_busy check

question about buffer_busy check

[Dmitry Monakhov]

[-- Attachment #1: Type: text/plain, Size: 840 bytes --]


Hi,
Can anyone please justify me the logic of fs/bufferc.c:buffer_busy()
How can we perform bit-wise operation for ->b_count and ->b_state?
 static inline int buffer_busy(struct buffer_head *bh)
 {
       return atomic_read(&bh->b_count) |
                (bh->b_state & ((1 << BH_Dirty) | (1 << BH_Lock)));
 }

I try to digg inside git/cvs history and it is appeared that 2.4 was
also implemented like this. At least it was so in 2000'th
http://lkml.indiana.edu/hypermail/linux/kernel/0006.0/0412.html
Also I've found similar complain
http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg24377.html
But seems nobody care about it. What's the point?
The only guess I have is that this is a miss typo  because buffer
is busy if some one hold an reference (bh->b_count !=0 )  ||
it is (dirty | locked). So following patch should fix 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-buffer-fix-miss-typo.patch --]
[-- Type: text/x-patch, Size: 698 bytes --]

>From dc45e525b647ed11f26781b80eed3894cc3ba325 Mon Sep 17 00:00:00 2001
From: Dmitry Monakhov <dmonakhov@openvz.org>
Date: Tue, 23 Apr 2013 15:24:24 +0400
Subject: [PATCH] buffer: fix miss typo


Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/buffer.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/buffer.c b/fs/buffer.c
index b4dcb34..4ffa6c9 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -3119,7 +3119,7 @@ EXPORT_SYMBOL(sync_dirty_buffer);
  */
 static inline int buffer_busy(struct buffer_head *bh)
 {
-	return atomic_read(&bh->b_count) |
+	return atomic_read(&bh->b_count) ||
 		(bh->b_state & ((1 << BH_Dirty) | (1 << BH_Lock)));
 }
 
-- 
1.7.1

Re: question about buffer_busy check

[Al Viro]

On Tue, Apr 23, 2013 at 03:41:18PM +0400, Dmitry Monakhov wrote:
> The only guess I have is that this is a miss typo  because buffer
> is busy if some one hold an reference (bh->b_count !=0 )  ||
> it is (dirty | locked). So following patch

... is pointless.  All callers only care about the return value being
zero or non-zero and not about specific values.  Granted, doing it that
way is a micro-optimization, but it's still a valid one.