[Linux-cachefs] [PATCH v3] cachefiles: Page leaking in cachefiles_read_backing_file while vmscan is active

[David Howells]

Daniel Axtens <dja at axtens.net> wrote:

> @@ -511,6 +511,8 @@ static int cachefiles_read_backing_file(struct cachefiles_object *object,
>  				goto installed_new_backing_page;
>  			if (ret != -EEXIST)
>  				goto nomem;
> +			put_page(newpage);
> +			newpage = NULL;
>  		}

Ummm...  How does this hunk help?  If we have a hanging new page, it should
get freed between the "out:" label and the only return-statement.  We also
don't allocate another page if newpage != NULL.

> @@ -535,7 +537,10 @@ static int cachefiles_read_backing_file(struct cachefiles_object *object,
>  					    netpage->index, cachefiles_gfp);
>  		if (ret < 0) {
>  			if (ret == -EEXIST) {
> +				put_page(backpage);
> +				backpage = NULL;

This looks like a genuine bug fix.

>  				put_page(netpage);
> +				netpage = NULL;

Clearing netpage should be redundant since the continue-statement should cause
list_for_each_entry_safe() to overwrite netpage anyway.

>  				fscache_retrieval_complete(op, 1);
>  				continue;
>  			}
> @@ -608,7 +613,10 @@ static int cachefiles_read_backing_file(struct cachefiles_object *object,
>  					    netpage->index, cachefiles_gfp);
>  		if (ret < 0) {
>  			if (ret == -EEXIST) {
> +				put_page(backpage);
> +				backpage = NULL;
>  				put_page(netpage);
> +				netpage = NULL;
>  				fscache_retrieval_complete(op, 1);
>  				continue;
>  			}

Ditto for these changes: put/clear backpage, yes; clear netpage, redundant.

David