|
Luís Henriques |
bf50b5 |
From: Kiran Kumar Modukuri <kiran.modukuri@gmail.com>
|
|
Luís Henriques |
bf50b5 |
Date: Mon, 24 Sep 2018 12:02:39 +1000
|
|
Luís Henriques |
bf50b5 |
Subject: cachefiles: Fix page leak in cachefiles_read_backing_file while
|
|
Luís Henriques |
bf50b5 |
vmscan is active
|
|
Luís Henriques |
bf50b5 |
Git-commit: 9a24ce5b66f9c8190d63b15f4473600db4935f1f
|
|
Luís Henriques |
bf50b5 |
Patch-mainline: v4.20-rc5
|
|
Luís Henriques |
bf50b5 |
References: bsc#1210430
|
|
Luís Henriques |
bf50b5 |
|
|
Luís Henriques |
bf50b5 |
[Description]
|
|
Luís Henriques |
bf50b5 |
|
|
Luís Henriques |
bf50b5 |
In a heavily loaded system where the system pagecache is nearing memory
|
|
Luís Henriques |
bf50b5 |
limits and fscache is enabled, pages can be leaked by fscache while trying
|
|
Luís Henriques |
bf50b5 |
read pages from cachefiles backend. This can happen because two
|
|
Luís Henriques |
bf50b5 |
applications can be reading same page from a single mount, two threads can
|
|
Luís Henriques |
bf50b5 |
be trying to read the backing page at same time. This results in one of
|
|
Luís Henriques |
bf50b5 |
the threads finding that a page for the backing file or netfs file is
|
|
Luís Henriques |
bf50b5 |
already in the radix tree. During the error handling cachefiles does not
|
|
Luís Henriques |
bf50b5 |
clean up the reference on backing page, leading to page leak.
|
|
Luís Henriques |
bf50b5 |
|
|
Luís Henriques |
bf50b5 |
[Fix]
|
|
Luís Henriques |
bf50b5 |
The fix is straightforward, to decrement the reference when error is
|
|
Luís Henriques |
bf50b5 |
encountered.
|
|
Luís Henriques |
bf50b5 |
|
|
Luís Henriques |
bf50b5 |
[dhowells: Note that I've removed the clearance and put of newpage as
|
|
Luís Henriques |
bf50b5 |
they aren't attested in the commit message and don't appear to actually
|
|
Luís Henriques |
bf50b5 |
achieve anything since a new page is only allocated is newpage!=NULL and
|
|
Luís Henriques |
bf50b5 |
any residual new page is cleared before returning.]
|
|
Luís Henriques |
bf50b5 |
|
|
Luís Henriques |
bf50b5 |
[Testing]
|
|
Luís Henriques |
bf50b5 |
I have tested the fix using following method for 12+ hrs.
|
|
Luís Henriques |
bf50b5 |
|
|
Luís Henriques |
bf50b5 |
1) mkdir -p /mnt/nfs ; mount -o vers=3,fsc <server_ip>:/export /mnt/nfs
|
|
Luís Henriques |
bf50b5 |
2) create 10000 files of 2.8MB in a NFS mount.
|
|
Luís Henriques |
bf50b5 |
3) start a thread to simulate heavy VM presssure
|
|
Luís Henriques |
bf50b5 |
(while true ; do echo 3 > /proc/sys/vm/drop_caches ; sleep 1 ; done)&
|
|
Luís Henriques |
bf50b5 |
4) start multiple parallel reader for data set at same time
|
|
Luís Henriques |
bf50b5 |
find /mnt/nfs -type f | xargs -P 80 cat > /dev/null &
|
|
Luís Henriques |
bf50b5 |
find /mnt/nfs -type f | xargs -P 80 cat > /dev/null &
|
|
Luís Henriques |
bf50b5 |
find /mnt/nfs -type f | xargs -P 80 cat > /dev/null &
|
|
Luís Henriques |
bf50b5 |
..
|
|
Luís Henriques |
bf50b5 |
..
|
|
Luís Henriques |
bf50b5 |
find /mnt/nfs -type f | xargs -P 80 cat > /dev/null &
|
|
Luís Henriques |
bf50b5 |
find /mnt/nfs -type f | xargs -P 80 cat > /dev/null &
|
|
Luís Henriques |
bf50b5 |
5) finally check using cat /proc/fs/fscache/stats | grep -i pages ;
|
|
Luís Henriques |
bf50b5 |
free -h , cat /proc/meminfo and page-types -r -b lru
|
|
Luís Henriques |
bf50b5 |
to ensure all pages are freed.
|
|
Luís Henriques |
bf50b5 |
|
|
Luís Henriques |
bf50b5 |
Reviewed-by: Daniel Axtens <dja@axtens.net>
|
|
Luís Henriques |
bf50b5 |
Signed-off-by: Shantanu Goel <sgoel01@yahoo.com>
|
|
Luís Henriques |
bf50b5 |
Signed-off-by: Kiran Kumar Modukuri <kiran.modukuri@gmail.com>
|
|
Luís Henriques |
bf50b5 |
[dja: forward ported to current upstream]
|
|
Luís Henriques |
bf50b5 |
Signed-off-by: Daniel Axtens <dja@axtens.net>
|
|
Luís Henriques |
bf50b5 |
Signed-off-by: David Howells <dhowells@redhat.com>
|
|
Luís Henriques |
bf50b5 |
Acked-by: Luís Henriques <lhenriques@suse.de>
|
|
Luís Henriques |
bf50b5 |
|
|
Luís Henriques |
bf50b5 |
---
|
|
Luís Henriques |
bf50b5 |
fs/cachefiles/rdwr.c | 6 ++++++
|
|
Luís Henriques |
bf50b5 |
1 file changed, 6 insertions(+)
|
|
Luís Henriques |
bf50b5 |
|
|
Luís Henriques |
bf50b5 |
diff --git a/fs/cachefiles/rdwr.c b/fs/cachefiles/rdwr.c
|
|
Luís Henriques |
bf50b5 |
index 40f7595aad10..db233588a69a 100644
|
|
Luís Henriques |
bf50b5 |
--- a/fs/cachefiles/rdwr.c
|
|
Luís Henriques |
bf50b5 |
+++ b/fs/cachefiles/rdwr.c
|
|
Luís Henriques |
bf50b5 |
@@ -535,7 +535,10 @@ static int cachefiles_read_backing_file(struct cachefiles_object *object,
|
|
Luís Henriques |
bf50b5 |
netpage->index, cachefiles_gfp);
|
|
Luís Henriques |
bf50b5 |
if (ret < 0) {
|
|
Luís Henriques |
bf50b5 |
if (ret == -EEXIST) {
|
|
Luís Henriques |
bf50b5 |
+ put_page(backpage);
|
|
Luís Henriques |
bf50b5 |
+ backpage = NULL;
|
|
Luís Henriques |
bf50b5 |
put_page(netpage);
|
|
Luís Henriques |
bf50b5 |
+ netpage = NULL;
|
|
Luís Henriques |
bf50b5 |
fscache_retrieval_complete(op, 1);
|
|
Luís Henriques |
bf50b5 |
continue;
|
|
Luís Henriques |
bf50b5 |
}
|
|
Luís Henriques |
bf50b5 |
@@ -608,7 +611,10 @@ static int cachefiles_read_backing_file(struct cachefiles_object *object,
|
|
Luís Henriques |
bf50b5 |
netpage->index, cachefiles_gfp);
|
|
Luís Henriques |
bf50b5 |
if (ret < 0) {
|
|
Luís Henriques |
bf50b5 |
if (ret == -EEXIST) {
|
|
Luís Henriques |
bf50b5 |
+ put_page(backpage);
|
|
Luís Henriques |
bf50b5 |
+ backpage = NULL;
|
|
Luís Henriques |
bf50b5 |
put_page(netpage);
|
|
Luís Henriques |
bf50b5 |
+ netpage = NULL;
|
|
Luís Henriques |
bf50b5 |
fscache_retrieval_complete(op, 1);
|
|
Luís Henriques |
bf50b5 |
continue;
|
|
Luís Henriques |
bf50b5 |
}
|
|
Luís Henriques |
bf50b5 |
|