Hi,

> diff --git a/fs/cachefiles/rdwr.c b/fs/cachefiles/rdwr.c
> index ad74a6a..ead4981 100644
> --- a/fs/cachefiles/rdwr.c
> +++ b/fs/cachefiles/rdwr.c
> @@ -523,7 +523,10 @@ static int cachefiles_read_backing_file(struct
> cachefiles_object *object,
>           netpage->index, cachefiles_gfp);
>    if (ret < 0) {
>     if (ret == -EEXIST) {
> + page_cache_release(backpage);
> + backpage = NULL;
>      page_cache_release(netpage);
> + netpage = NULL;
>      fscache_retrieval_complete(op, 1);
>      continue;
>     }
> @@ -596,7 +599,10 @@ static int cachefiles_read_backing_file(struct
> cachefiles_object *object,
>           netpage->index, cachefiles_gfp);
>    if (ret < 0) {
>     if (ret == -EEXIST) {
> + page_cache_release(backpage);
> + backpage = NULL;
>      page_cache_release(netpage);
> + netpage = NULL;
>      fscache_retrieval_complete(op, 1);
>      continue;
>     }

This doesn't directly apply to master any more but this does (but is untested):

Subject: [PATCH] cachefiles: page reference leak fix

Link: https://www.redhat.com/archives/linux-cachefs/2014-August/msg00017.html
Link: https://www.redhat.com/archives/linux-cachefs/2014-August/msg00021.html
Signed-off-by: Shantanu Goel <[email protected]>
[dja: forward ported to current upstream]
Signed-off-by: Daniel Axtens <[email protected]>
---
 fs/cachefiles/rdwr.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/cachefiles/rdwr.c b/fs/cachefiles/rdwr.c
index 40f7595aad10..db233588a69a 100644
--- a/fs/cachefiles/rdwr.c
+++ b/fs/cachefiles/rdwr.c
@@ -535,7 +535,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;
                        }
@@ -608,7 +611,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;
                        }
-- 
2.17.1


However, I am not sure about the first hunk.

For the second hunk - in backing_page_already_uptodate, we can see that
the backing page will be released if adding the netpage to the page
cache LRU succeeds, but will not be release in the -EEXIST case. This
change causes it to be release.

With the first hunk, we're in monitor_backing_page. It looks to me like
you could get to the release before the relevant get, so I'm worried it
will incorrectly change the refcount.  Am I understanding this
incorrectly?

Regards,
Daniel

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

Reply via email to