Re: search_one_bitmap_forward oops

[Edward Shishkin <edward.shishkin@gmail.com>]

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

On 03/24/2011 03:33 PM, Bringfried Stecklum wrote:
> Edward Shishkin wrote:
>> On 03/24/2011 12:56 AM, Bringfried Stecklum wrote:
>>> Edward Shishkin wrote:
>>>> On 03/24/2011 12:34 AM, Bringfried Stecklum wrote:
>>>>> Edward Shishkin wrote:
>>>>>> On 03/23/2011 11:41 PM, Bringfried Stecklum wrote:
>>>>>>> Dear Edward,
>>>>>>>
>>>>>>> thanks for the reply. Since I contacted you again concerning this
>>>>>>> bug,
>>>>>>> my system is going mad because of it. I didn't change anything
>>>>>>> concerning the kernel but even just after login and idling, the bug
>>>>>>> happens. I'd like to stay with Reiser4. But right now it is getting
>>>>>>> really difficult.
>>>>>>
>>>>>> Do you use the mount option "dont_load_bitmap"?
>>>>>
>>>>> Yes, I used it so far for faster booting but switched it off now.
>>>>
>>>>
>>>> Please, don't use this mount option for now: it doesn't
>>>> work properly. I'll try to reproduce and fix it at leisure.
>>>
>>> Looks like, up'n running now for half an hour. Normally, I prefer to
>>> hibernate between sessions and thus boot rarely, the longer boot time is
>>> no obstacle. Good luck with fixing the issue.
>>
>>
>> It looks like missing bitmap is not uploaded somewhere..
>
> But it used to work quite some time...

It seems I have found the bug.

The problem arises when we get to the /* race: ...*/ point (see
the attached patch). This is not an error path, nevertheless we
release all bitmaps and set zeroes to bnode fields. Welcome to
the Oops...

Everyone, who experienced problems with on-demand bitmap loading
in reiser4 (the mount option "dont_load_bitmap"), please, try the 
attached patch.

Thanks,
Edward.

[-- Attachment #2: reiser4-fix-on-demand-bitmap-load.patch --]
[-- Type: text/plain, Size: 2378 bytes --]

Index: linux-2.6.38/fs/reiser4/plugin/space/bitmap.c
===================================================================
--- linux-2.6.38.orig/fs/reiser4/plugin/space/bitmap.c
+++ linux-2.6.38/fs/reiser4/plugin/space/bitmap.c
@@ -830,45 +830,43 @@ static int load_and_lock_bnode(struct bi
 	}
 
 	ret = prepare_bnode(bnode, &cjnode, &wjnode);
-	if (ret == 0) {
-		mutex_lock(&bnode->mutex);
+	if (ret)
+		return ret;
 
-		if (!atomic_read(&bnode->loaded)) {
-			assert("nikita-2822", cjnode != NULL);
-			assert("nikita-2823", wjnode != NULL);
-			assert("nikita-2824", jnode_is_loaded(cjnode));
-			assert("nikita-2825", jnode_is_loaded(wjnode));
-
-			bnode->wjnode = wjnode;
-			bnode->cjnode = cjnode;
-
-			ret = check_struct_bnode(bnode, current_blocksize);
-			if (!ret) {
-				cjnode = wjnode = NULL;
-				atomic_set(&bnode->loaded, 1);
-				/* working bitmap is initialized by on-disk
-				 * commit bitmap. This should be performed
-				 * under mutex. */
-				memcpy(bnode_working_data(bnode),
-				       bnode_commit_data(bnode),
-				       bmap_size(current_blocksize));
-			} else
-				mutex_unlock(&bnode->mutex);
-		} else
-			/* race: someone already loaded bitmap while we were
-			 * busy initializing data. */
-			check_bnode_loaded(bnode);
-	}
-
-	if (wjnode != NULL) {
-		release(wjnode);
-		bnode->wjnode = NULL;
-	}
-	if (cjnode != NULL) {
-		release(cjnode);
-		bnode->cjnode = NULL;
-	}
+	mutex_lock(&bnode->mutex);
 
+	if (!atomic_read(&bnode->loaded)) {
+		assert("nikita-2822", cjnode != NULL);
+		assert("nikita-2823", wjnode != NULL);
+		assert("nikita-2824", jnode_is_loaded(cjnode));
+		assert("nikita-2825", jnode_is_loaded(wjnode));
+
+		bnode->wjnode = wjnode;
+		bnode->cjnode = cjnode;
+
+		ret = check_struct_bnode(bnode, current_blocksize);
+		if (unlikely(ret != 0))
+			goto error;
+
+		atomic_set(&bnode->loaded, 1);
+		/* working bitmap is initialized by on-disk
+		 * commit bitmap. This should be performed
+		 * under mutex. */
+		memcpy(bnode_working_data(bnode),
+		       bnode_commit_data(bnode),
+		       bmap_size(current_blocksize));
+	} else
+		/* race: someone already loaded bitmap
+		 * while we were busy initializing data. */
+		check_bnode_loaded(bnode);
+	return 0;
+
+ error:
+	release(wjnode);
+	release(cjnode);
+	bnode->wjnode = NULL;
+	bnode->cjnode = NULL;
+	mutex_unlock(&bnode->mutex);
 	return ret;
 }