[PATCH] wait_on_buffer refcounting checks

It is generally illegal to wait on an unpinned buffer - another CPU could
free it up even before __wait_on_buffer() has taken a ref against the buffer.

Maybe external locking rules will prevent this in specific cases, but that is
really subtle and fragile as locking rules are evolved.

The patch detects people calling wait_on_buffer() against an unpinned buffer
and issues a diagnostic.

Also remove the get_bh() from __wait_on_buffer().  It is too late.

fs/buffer.c

@@ -123,7 +123,9 @@ void __wait_on_buffer(struct buffer_head * bh)
 	wait_queue_head_t *wqh = bh_waitq_head(bh);
 	DEFINE_WAIT(wait);
 
-	get_bh(bh);
+	if (atomic_read(&bh->b_count) == 0)
+		buffer_error();
+
 	do {
 		prepare_to_wait(wqh, &wait, TASK_UNINTERRUPTIBLE);
 		if (buffer_locked(bh)) {
@@ -131,7 +133,6 @@ void __wait_on_buffer(struct buffer_head * bh)
 			io_schedule();
 		}
 	} while (buffer_locked(bh));
-	put_bh(bh);
 	finish_wait(wqh, &wait);
 }
 

include/linux/buffer_head.h

@@ -60,6 +60,12 @@ struct buffer_head {
 	struct list_head b_assoc_buffers; /* associated with another mapping */
 };
 
+/*
+ * Debug
+ */
+
+void __buffer_error(char *file, int line);
+#define buffer_error() __buffer_error(__FILE__, __LINE__)
 
 /*
  * macro tricks to expand the set_buffer_foo(), clear_buffer_foo()
@@ -183,7 +189,8 @@ extern int buffer_heads_over_limit;
  */
 int try_to_release_page(struct page * page, int gfp_mask);
 int block_invalidatepage(struct page *page, unsigned long offset);
-int block_write_full_page(struct page *page, get_block_t *get_block, struct writeback_control *wbc);
+int block_write_full_page(struct page *page, get_block_t *get_block,
+				struct writeback_control *wbc);
 int block_read_full_page(struct page*, get_block_t*);
 int block_prepare_write(struct page*, unsigned, unsigned, get_block_t*);
 int cont_prepare_write(struct page*, unsigned, unsigned, get_block_t*,
@@ -232,12 +239,14 @@ static inline void bforget(struct buffer_head *bh)
 		__bforget(bh);
 }
 
-static inline struct buffer_head *sb_bread(struct super_block *sb, sector_t block)
+static inline struct buffer_head *
+sb_bread(struct super_block *sb, sector_t block)
 {
 	return __bread(sb->s_bdev, block, sb->s_blocksize);
 }
 
-static inline struct buffer_head *sb_getblk(struct super_block *sb, sector_t block)
+static inline struct buffer_head *
+sb_getblk(struct super_block *sb, sector_t block)
 {
 	return __getblk(sb->s_bdev, block, sb->s_blocksize);
 }
@@ -256,9 +265,14 @@ map_bh(struct buffer_head *bh, struct super_block *sb, sector_t block)
 	bh->b_blocknr = block;
 }
 
+/*
+ * Calling wait_on_buffer() for a zero-ref buffer is illegal, so we call into
+ * __wait_on_buffer() just to trip a debug check.  Because debug code in inline
+ * functions is bloaty.
+ */
 static inline void wait_on_buffer(struct buffer_head *bh)
 {
-	if (buffer_locked(bh))
+	if (buffer_locked(bh) || atomic_read(&bh->b_count) == 0)
 		__wait_on_buffer(bh);
 }
 
@@ -268,11 +282,4 @@ static inline void lock_buffer(struct buffer_head *bh)
 		__wait_on_buffer(bh);
 }
 
-/*
- * Debug
- */
-
-void __buffer_error(char *file, int line);
-#define buffer_error() __buffer_error(__FILE__, __LINE__)
-
 #endif /* _LINUX_BUFFER_HEAD_H */