linux-kernel - [PATCH 40/52] fuse: Do not block on inode lock while freeing memory range

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20181210171318.16998-41-vgoyal@redhat.com>
Date:   Mon, 10 Dec 2018 12:13:06 -0500
From:   Vivek Goyal <vgoyal@...hat.com>
To:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org
Cc:     vgoyal@...hat.com, miklos@...redi.hu, stefanha@...hat.com,
        dgilbert@...hat.com, sweil@...hat.com, swhiteho@...hat.com
Subject: [PATCH 40/52] fuse: Do not block on inode lock while freeing memory range

Once we select a memory range to free, we currently block on inode
lock. Do not block and use trylock instead. And move on to next memory
range if trylock fails.

Reason being that in next few patches I want to enabling waiting for
memmory ranges to become free in fuse_iomap_begin(). So insted of
returning -EBUSY, a process will wait for a memory range to become
free.

We don't want to end up in a situation where process is sleeping in
iomap_begin() with inode lock held and worker is trying to free
memory from same inode, resulting in deadlock.

To avoid deadlock, use trylock instead.

Signed-off-by: Vivek Goyal <vgoyal@...hat.com>
---
 fs/fuse/file.c | 36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index d86f6e5c4daf..dbe3410a94d7 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -3891,7 +3891,12 @@ int fuse_dax_free_one_mapping(struct fuse_conn *fc, struct inode *inode,
 	int ret;
 	struct fuse_inode *fi = get_fuse_inode(inode);
 
-	inode_lock(inode);
+	/*
+	 * If process is blocked waiting for memory while holding inode
+	 * lock, we will deadlock. So continue to free next range.
+	 */
+	if (!inode_trylock(inode))
+		return -EAGAIN;
 	down_write(&fi->i_mmap_sem);
 	down_write(&fi->i_dmap_sem);
 	ret = fuse_dax_free_one_mapping_locked(fc, inode, dmap_start);
@@ -3903,19 +3908,22 @@ int fuse_dax_free_one_mapping(struct fuse_conn *fc, struct inode *inode,
 
 int fuse_dax_free_memory(struct fuse_conn *fc, unsigned long nr_to_free)
 {
-	struct fuse_dax_mapping *dmap, *pos;
-	int ret, i;
+	struct fuse_dax_mapping *dmap, *pos, *temp;
+	int ret, nr_freed = 0;
 	u64 dmap_start = 0, window_offset = 0;
 	struct inode *inode = NULL;
 
 	/* Pick first busy range and free it for now*/
-	for (i = 0; i < nr_to_free; i++) {
+	while(1) {
+		if (nr_freed >= nr_to_free)
+			break;
+
 		dmap = NULL;
 		spin_lock(&fc->lock);
 
-		list_for_each_entry(pos, &fc->busy_ranges, busy_list) {
-			dmap = pos;
-			inode = igrab(dmap->inode);
+		list_for_each_entry_safe(pos, temp, &fc->busy_ranges,
+						busy_list) {
+			inode = igrab(pos->inode);
 			/*
 			 * This inode is going away. That will free
 			 * up all the ranges anyway, continue to
@@ -3923,6 +3931,13 @@ int fuse_dax_free_memory(struct fuse_conn *fc, unsigned long nr_to_free)
 			 */
 			if (!inode)
 				continue;
+			/*
+			 * Take this element off list and add it tail. If
+			 * inode lock can't be obtained, this will help with
+			 * selecting new element
+			 */
+			dmap = pos;
+			list_move_tail(&dmap->busy_list, &fc->busy_ranges);
 			dmap_start = dmap->start;
 			window_offset = dmap->window_offset;
 			break;
@@ -3933,11 +3948,16 @@ int fuse_dax_free_memory(struct fuse_conn *fc, unsigned long nr_to_free)
 
 		ret = fuse_dax_free_one_mapping(fc, inode, dmap_start);
 		iput(inode);
-		if (ret) {
+		if (ret && ret != -EAGAIN) {
 			printk("%s(window_offset=0x%llx) failed. err=%d\n",
 				__func__, window_offset, ret);
 			return ret;
 		}
+
+		/* Could not get inode lock. Try next element */
+		if (ret == -EAGAIN)
+			continue;
+		nr_freed++;
 	}
 	return 0;
 }
-- 
2.13.6