[Ocfs2-devel] [PATCH] ocfs2: Fix wrong search logic in __ocfs2_resv_find_window

[Heming Zhao]

Hello Joseph,

On Thu, Apr 06, 2023 at 11:40:52PM +0800, Heming Zhao wrote:
> ** problem **
> 
> Current code triggers a defragfs bug [1]:
> 
> ```
> tw-tst:~ # defragfs.ocfs2 /mnt/test_from_dd1
> defragfs.ocfs2 1.8.7
> [ERROR]Move extent failed:"/mnt/test_from_dd1" - No space left on device
> [ERROR]"/mnt/test_from_dd1":No space left on device
> ```
> 
> I added some debug messages in relevant functions. When above error
> messages appeared, the la-window still had enough continuous bitmap
> to use, the -ENOSPC is wrong.
> 

There is another problem with la-window alloc flow, and also should have a patch.
When reservatioins are disabled, ocfs2_local_alloc_find_clear_bits starts linear
search, and returns 'bitoff:-1' if 'numfound < *numbits'. In my view, this code
logic is wrong, in this case, it should return max suitable numfound.

The pseudo code:

```
diff -Nupr a/localalloc.c b/localalloc.c
--- a/localalloc.c	2023-04-07 23:33:42.077083405 +0800
+++ b/localalloc.c	2023-04-07 23:35:10.756344502 +0800
@@ -835,6 +835,7 @@ static int ocfs2_local_alloc_find_clear_
 				     struct ocfs2_alloc_reservation *resv)
 {
 	int numfound = 0, bitoff, left, startoff;
+	unsigned int best_start, best_len = 0;
 	int local_resv = 0;
 	struct ocfs2_alloc_reservation r;
 	void *bitmap = NULL;
@@ -940,6 +941,10 @@ static int ocfs2_local_alloc_find_clear_
 			numfound = 1;
 			startoff = bitoff+1;
 		}
+		if (numfound > best_len) {
+			best_len = numfound;
+			best_start = startoff - numfound;
+		}
 		/* we got everything we needed */
 		if (numfound == *numbits) {
 			/* mlog(0, "Found it all!\n"); */
@@ -949,10 +954,12 @@ static int ocfs2_local_alloc_find_clear_
 
 	trace_ocfs2_local_alloc_find_clear_bits_search_bitmap(bitoff, numfound);
 
-	if (numfound == *numbits)
-		bitoff = startoff - numfound;
-	else
+	if (best_len = 0) {
 		bitoff = -1;
+	} else {
+		bitoff = best_start;
+		*numbits = best_len;
+	}
 
 bail:
 	if (local_resv)
```

If you agree with my thinking, I will test & file a formal patch.

Thanks,
Heming