[PATCH] maple_tree: reload mas before the second call for mas_empty_area

[Liam R. Howlett]

* Yang Erkun <yangerkun at huaweicloud.com> [241214 04:34]:
> From: Yang Erkun <yangerkun at huawei.com>
> 
> Change the LONG_MAX in simple_offset_add to 1024, and do latter:
> 
> [root at fedora ~]# mkdir /tmp/dir
> [root at fedora ~]# for i in {1..1024}; do touch /tmp/dir/$i; done
> touch: cannot touch '/tmp/dir/1024': Device or resource busy
> [root at fedora ~]# rm /tmp/dir/123
> [root at fedora ~]# touch /tmp/dir/1024
> [root at fedora ~]# rm /tmp/dir/100
> [root at fedora ~]# touch /tmp/dir/1025
> touch: cannot touch '/tmp/dir/1025': Device or resource busy
> 
> After we delete file 100, actually this is a empty entry, but the latter
> create failed unexpected.
> 
> mas_alloc_cyclic has two chance to find empty entry. First find the
> entry with range range_lo and range_hi, if no empty entry exist, and
> range_lo > min, retry find with range min and range_hi. However, the
> first call mas_empty_area may mark mas as EBUSY, and the second call for
> mas_empty_area will return false directly. Fix this by reload mas before
> second call for mas_empty_area.

I have test cases and would like to add this as one for the
mas_alloc_cyclic() call so that we don't re-introduce this issue later.

I think this will be different than the actual roll-over as the next
entry won't be 0 and set the correct flag to detect a wrapping event.

It will allow for testing of the issue you found, just not a true
wrapping around.  I'm working on reproducing this failure so I can have
a test case.  I think we probably need a true wrapping test case too.

> 
> Fixes: 9b6713cc7522 ("maple_tree: Add mtree_alloc_cyclic()")
> Signed-off-by: Yang Erkun <yangerkun at huawei.com>
> ---
>  lib/maple_tree.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> index d0ae808f3a14..1fd02f4a05e6 100644
> --- a/lib/maple_tree.c
> +++ b/lib/maple_tree.c
> @@ -4346,6 +4346,7 @@ int mas_alloc_cyclic(struct ma_state *mas, unsigned long *startp,
>  {
>  	unsigned long min = range_lo;
>  	int ret = 0;
> +	struct ma_state m = *mas;
>  
>  	range_lo = max(min, *next);
>  	ret = mas_empty_area(mas, range_lo, range_hi, 1);
> @@ -4354,6 +4355,7 @@ int mas_alloc_cyclic(struct ma_state *mas, unsigned long *startp,
>  		ret = 1;
>  	}
>  	if (ret < 0 && range_lo > min) {
> +		*mas = m;

You want mas_reset(mas);
The copy of the state is not necessary.

On inspection, the comment on this function seems a bit incorrect in
that it's saying -EBUSY is the only other potential return - but it
could also be -EINVAL (although I don't think the libfs code will ever
hit this).

I'm also not sure mas_destroy() is necessary.

>  		ret = mas_empty_area(mas, min, range_hi, 1);
>  		if (ret == 0)
>  			ret = 1;
> -- 
> 2.39.2
> 
> 
> -- 
> maple-tree mailing list
> maple-tree at lists.infradead.org
> https://lists.infradead.org/mailman/listinfo/maple-tree