Daniel Axtens <[email protected]> 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

--
Linux-cachefs mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-cachefs

Reply via email to