[Ocfs2-devel] [PATCH] ocfs2: Fix wrong search logic in __ocfs2_resv_find_window
Joseph Qi
joseph.qi at linux.alibaba.com
Fri Apr 21 07:35:01 UTC 2023
Hi,
Could you please share a reproducer?
Thanks,
Joseph
On 4/6/23 11:40 PM, 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.
>
> ** analysis **
>
> __ocfs2_resv_find_window should use resv node from rb-tree to do linear
> search. But current code logic uses un-managered area between two resvs.
> The searching logic deviates from this func job, which should only focus
> on reservation areas (when rb_root is non NULL). Another issue of
> __ocfs2_resv_find_window is more inclined to generate inner fragment.
> It doesn't search & consume existing reservations but be apt to create
> new reservation item.
>
> This patch pulls this func (__ocfs2_resv_find_window) to do what it
> should do: search & consume resev. if this func fails to get suitable
> bitmap area, caller ocfs2_resv_find_window fallbacks to use oldest
> resv from LRU to do the final search.
>
> [1]: https://bugzilla.suse.com/show_bug.cgi?id=1131931
>
> Signed-off-by: Heming Zhao <heming.zhao at suse.com>
> ---
> fs/ocfs2/reservations.c | 48 +++++++++++++++++++++++++++++------------
> 1 file changed, 34 insertions(+), 14 deletions(-)
>
> diff --git a/fs/ocfs2/reservations.c b/fs/ocfs2/reservations.c
> index a9d1296d736d..eda672622d1d 100644
> --- a/fs/ocfs2/reservations.c
> +++ b/fs/ocfs2/reservations.c
> @@ -458,10 +458,11 @@ static void __ocfs2_resv_find_window(struct ocfs2_reservation_map *resmap,
> {
> struct rb_root *root = &resmap->m_reservations;
> unsigned int gap_start, gap_end, gap_len;
> - struct ocfs2_alloc_reservation *prev_resv, *next_resv;
> + struct ocfs2_alloc_reservation *prev_resv, *next_resv, *best_resv;
> struct rb_node *prev, *next;
> unsigned int cstart, clen;
> unsigned int best_start = 0, best_len = 0;
> + int create_new = 0;
>
> /*
> * Nasty cases to consider:
> @@ -540,8 +541,9 @@ static void __ocfs2_resv_find_window(struct ocfs2_reservation_map *resmap,
> if (clen) {
> best_len = clen;
> best_start = cstart;
> + create_new = 1;
> if (best_len == wanted)
> - goto out_insert;
> + goto out_create;
> }
>
> prev_resv = next_resv;
> @@ -557,13 +559,9 @@ static void __ocfs2_resv_find_window(struct ocfs2_reservation_map *resmap,
> while (1) {
> next = rb_next(prev);
> if (next) {
> - next_resv = rb_entry(next,
> - struct ocfs2_alloc_reservation,
> - r_node);
> -
> - gap_start = ocfs2_resv_end(prev_resv) + 1;
> - gap_end = next_resv->r_start - 1;
> - gap_len = gap_end - gap_start + 1;
> + gap_start = prev_resv->r_start;
> + gap_end = prev_resv->r_start + prev_resv->r_len - 1;
> + gap_len = prev_resv->r_len;
> } else {
> /*
> * We're at the rightmost edge of the
> @@ -575,8 +573,8 @@ static void __ocfs2_resv_find_window(struct ocfs2_reservation_map *resmap,
> gap_end = resmap->m_bitmap_len - 1;
> }
>
> - trace_ocfs2_resv_find_window_next(next ? next_resv->r_start: -1,
> - next ? ocfs2_resv_end(next_resv) : -1);
> + trace_ocfs2_resv_find_window_next(next ? prev_resv->r_start : -1,
> + next ? ocfs2_resv_end(prev_resv) : -1);
> /*
> * No need to check this gap if we have already found
> * a larger region of free bits.
> @@ -589,10 +587,16 @@ static void __ocfs2_resv_find_window(struct ocfs2_reservation_map *resmap,
> if (clen == wanted) {
> best_len = clen;
> best_start = cstart;
> - goto out_insert;
> + best_resv = prev_resv;
> + if (!next)
> + goto out_create;
> + else
> + goto out_insert;
> } else if (clen > best_len) {
> best_len = clen;
> best_start = cstart;
> + best_resv = prev_resv;
> + create_new = 0;
> }
>
> next_resv:
> @@ -604,12 +608,28 @@ static void __ocfs2_resv_find_window(struct ocfs2_reservation_map *resmap,
> r_node);
> }
>
> -out_insert:
> - if (best_len) {
> + if (!best_len)
> + return;
> +
> + if (create_new) {
> +out_create:
> resv->r_start = best_start;
> resv->r_len = best_len;
> ocfs2_resv_insert(resmap, resv);
> + return;
> }
> +
> +out_insert:
> + if (best_resv->r_len <= wanted) {
> + resv->r_start = best_start;
> + resv->r_len = best_len;
> + __ocfs2_resv_discard(resmap, best_resv);
> + } else {
> + best_resv->r_len -= best_len;
> + resv->r_start = ocfs2_resv_end(best_resv) + 1;
> + resv->r_len = best_len;
> + }
> + ocfs2_resv_insert(resmap, resv);
> }
>
> static void ocfs2_cannibalize_resv(struct ocfs2_reservation_map *resmap,
More information about the Ocfs2-devel
mailing list