From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:50522 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728396AbeLJRNg (ORCPT ); Mon, 10 Dec 2018 12:13:36 -0500 From: Vivek Goyal To: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org Cc: vgoyal@redhat.com, miklos@szeredi.hu, stefanha@redhat.com, dgilbert@redhat.com, sweil@redhat.com, swhiteho@redhat.com Subject: [PATCH 40/52] fuse: Do not block on inode lock while freeing memory range Date: Mon, 10 Dec 2018 12:13:06 -0500 Message-Id: <20181210171318.16998-41-vgoyal@redhat.com> In-Reply-To: <20181210171318.16998-1-vgoyal@redhat.com> References: <20181210171318.16998-1-vgoyal@redhat.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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 --- 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