Daniel Axtens <dja@xxxxxxxxxx> 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
Linux-cachefs@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/linux-cachefs
