Re: [PATCH 1/4] mm/filemap: return early if failed to allocate memory for split

[Matthew Wilcox]

On Tue, Mar 19, 2024 at 05:27:30PM +0800, Kairui Song wrote:
> From: Kairui Song <kasong@xxxxxxxxxxx>
> 
> xas_split_alloc could fail with NOMEM, and in such case, it should abort
> early instead of keep going and fail the xas_split below.
> 
> Signed-off-by: Kairui Song <kasong@xxxxxxxxxxx>

The usual way of programming with an xa_state is to not do any error
handling; the xas_ functions check for error and become no-ops, so we
only have to check at the end.

I think this case is worth making an exception for because we avoid
acquiring/releasing the lock.  Not that we should be getting ENOMEM
very often; we only allocate about 8 nodes usually and we get 7
nodes/page.

Acked-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>

>  mm/filemap.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 7437b2bd75c1..f07ea0b97698 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -880,9 +880,12 @@ noinline int __filemap_add_folio(struct address_space *mapping,
>  		unsigned int order = xa_get_order(xas.xa, xas.xa_index);
>  		void *entry, *old = NULL;
>  
> -		if (order > folio_order(folio))
> +		if (order > folio_order(folio)) {
>  			xas_split_alloc(&xas, xa_load(xas.xa, xas.xa_index),
>  					order, gfp);
> +			if (xas_error(&xas))
> +				goto error;
> +		}
>  		xas_lock_irq(&xas);
>  		xas_for_each_conflict(&xas, entry) {
>  			old = entry;
> -- 
> 2.43.0
>