Re: [PATCH] btrfs: fix return value check of btrfs_start_transaction()

[Tsutomu Itoh <t-itoh@jp.fujitsu.com>]

(2011/01/21 8:47), Tsutomu Itoh wrote:
> (2011/01/21 1:09), Josef Bacik wrote:
>> I'd rather we go through and have these things return an error than do a
>> BUG_ON().  We're moving towards a more stable BTRFS, not one that panics more
>> often :).
> 
> Yes, I also think so.
> This patch is my first step.
> 
> My modification policy is as follows:
>
> 1. short term
>  - To more stable BTRFS, the part that should be corrected is clarified. 
>  - The panic is not done by the NULL pointer reference etc.
This means, even if temporary increase BUG_ON()...

In addition, I think that an important memory allocation should retry several times. 
So, I propose the following patches as this sample.

> 
> 2. long term
>  - BUG_ON() is decreased by using the forced-readonly framework(already posted by Liu Bo),
>    etc. 


This patch retries kmem_cache_alloc() in start_transaction() several times. 

Signed-off-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com>
---
 fs/btrfs/transaction.c |   22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff -urNp linux-2.6.38-rc1/fs/btrfs/transaction.c linux-2.6.38-rc1.new/fs/btrfs/transaction.c
--- linux-2.6.38-rc1/fs/btrfs/transaction.c	2011-01-19 08:14:02.000000000 +0900
+++ linux-2.6.38-rc1.new/fs/btrfs/transaction.c	2011-01-21 14:08:02.000000000 +0900
@@ -22,6 +22,7 @@
 #include <linux/writeback.h>
 #include <linux/pagemap.h>
 #include <linux/blkdev.h>
+#include <linux/ratelimit.h>
 #include "ctree.h"
 #include "disk-io.h"
 #include "transaction.h"
@@ -175,6 +176,25 @@ static int may_wait_transaction(struct b
 	return 0;
 }
 
+#define MAX_ITERATIONS 10
+
+static struct btrfs_trans_handle *alloc_trans_handle(void)
+{
+	struct btrfs_trans_handle *ret;
+	int i = 0;
+
+	ret = kmem_cache_alloc(btrfs_trans_handle_cachep, GFP_NOFS);
+	if (!ret) {
+		pr_notice_ratelimited("ENOMEM in %s, retrying.\n", __func__);
+		do {
+			yield();
+			ret = kmem_cache_alloc(btrfs_trans_handle_cachep,
+						GFP_NOFS);
+		} while (!ret && i++ < MAX_ITERATIONS);
+	}
+	return ret;
+}
+
 static struct btrfs_trans_handle *start_transaction(struct btrfs_root *root,
 						    u64 num_items, int type)
 {
@@ -185,7 +205,7 @@ static struct btrfs_trans_handle *start_
 	if (root->fs_info->fs_state & BTRFS_SUPER_FLAG_ERROR)
 		return ERR_PTR(-EROFS);
 again:
-	h = kmem_cache_alloc(btrfs_trans_handle_cachep, GFP_NOFS);
+	h = alloc_trans_handle();
 	if (!h)
 		return ERR_PTR(-ENOMEM);