From mboxrd@z Thu Jan 1 00:00:00 1970 References: <1565634725-89726-1-git-send-email-bo.liu@linux.alibaba.com> <1565634725-89726-2-git-send-email-bo.liu@linux.alibaba.com> <5D5214FC.4060804@huawei.com> <20190813182945.w3of5ftnho2k7t5l@US-160370MP2.local> From: piaojun Message-ID: <5D53598C.8050202@huawei.com> Date: Wed, 14 Aug 2019 08:45:00 +0800 MIME-Version: 1.0 In-Reply-To: <20190813182945.w3of5ftnho2k7t5l@US-160370MP2.local> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Subject: Re: [Virtio-fs] [PATCH 2/2] virtio-fs: do not removemapping if dmap will be used immediately List-Id: Development discussions about virtio-fs List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: bo.liu@linux.alibaba.com Cc: virtio-fs@redhat.com On 2019/8/14 2:29, Liu Bo wrote: > On Tue, Aug 13, 2019 at 09:40:12AM +0800, piaojun wrote: >> Hi Bo, >> >> On 2019/8/13 2:32, Liu Bo wrote: >>> According to how we implement REMOVEMAPPING in daemon, it's unnecessary to >>> involve a REMOVEMAPPING to reuse a dmap when doing inline reclaim because >>> dmaps got from inline reclaim get reused for another mapping without being >>> added back to 'free' dmap pool. >>> >>> This skips REMOVEMAPPING for inline reclaim only and we don't do >>> REMOVEMAPPING unless someone has raced in to add a dmap to the range. >>> >>> With the following two patches applied, >>> "virtio-fs: do not removemapping if dmap will be used immediately" >>> "virtio-fs: try hard to do inline reclaim", >>> >>> fio fio-tmp.job >>> read: IOPS=8055, BW=31.5MiB/s (32.0MB/s)(3776MiB/120001msec) >> >> Do you mean the before result is 31.5MB/s and after is 32MB/s? I wonder >> what is your backend storage? >> > > hmm...the 'w/o/ results are listed in the below 'w/o' section. "w/o=without" plays a joke with me, :) > > thanks, > -liubo > >> Jun >> >>> clat (usec): min=6, max=1842, avg=113.75, stdev=83.78 >>> clat percentiles (usec): >>> 95.00th=[ 202] >>> 99.00th=[ 221] >>> >>> w/o >>> read: IOPS=4312, BW=16.8MiB/s (17.7MB/s)(2021MiB/120001msec) >>> clat (usec): min=10, max=3415, avg=220.98, stdev=102.85 >>> clat percentiles (usec): >>> 95.00th=[ 359] >>> 99.00th=[ 392] >>> >>> [1]: >>> virtiofsd -o cache="always" >>> qemu -device vhost-user-fs-pci,queue-size=1024,chardev=char0,tag=myfs-1,cache-size=4G >>> >>> [2]: >>> cat fio-tmp.job >>> >>> ; fio-rand-read.job for fiotest >>> >>> [global] >>> name=fio-rand-read >>> filename=fio_file >>> rw=randread >>> bs=4K >>> direct=1 >>> numjobs=1 >>> time_based=1 >>> runtime=120 >>> directory=/mnt/test/ >>> >>> [file1] >>> size=10G >>> ioengine=psync >>> iodepth=1 >>> >>> Signed-off-by: Liu Bo >>> --- >>> fs/fuse/file.c | 18 +++++++++++++++--- >>> 1 file changed, 15 insertions(+), 3 deletions(-) >>> >>> diff --git a/fs/fuse/file.c b/fs/fuse/file.c >>> index 2ea670a..b5239b1 100644 >>> --- a/fs/fuse/file.c >>> +++ b/fs/fuse/file.c >>> @@ -1933,7 +1933,11 @@ static int iomap_begin_setup_new_mapping(struct inode *inode, loff_t pos, >>> if (ret < 0) { >>> printk("fuse_setup_one_mapping() failed. err=%d" >>> " pos=0x%llx, writable=%d\n", ret, pos, writable); >>> + >>> + /* liubo: ignore failure if removemapping fails. */ >>> + dmap_removemapping_one(inode, dmap); Why do we need remove mapping if the setup has not been done? Jun >>> dmap_add_to_free_pool(fc, alloc_dmap); >>> + >>> up_write(&fi->i_dmap_sem); >>> return ret; >>> } >>> @@ -3996,7 +4000,8 @@ static int dmap_writeback_invalidate(struct inode *inode, >>> } >>> >>> static int reclaim_one_dmap_locked(struct fuse_conn *fc, struct inode *inode, >>> - struct fuse_dax_mapping *dmap) >>> + struct fuse_dax_mapping *dmap, >>> + bool inline_reclaim) >>> { >>> int ret; >>> struct fuse_inode *fi = get_fuse_inode(inode); >>> @@ -4021,6 +4026,13 @@ static int reclaim_one_dmap_locked(struct fuse_conn *fc, struct inode *inode, >>> fuse_dax_interval_tree_remove(dmap, &fi->dmap_tree); >>> fi->nr_dmaps--; >>> >>> + /* >>> + * for inline reclaim, it's unnecessary to removing mapping since it'll >>> + * be used by another range immediately. >>> + */ >>> + if (inline_reclaim) >>> + return 0; >>> + >>> ret = dmap_removemapping_one(inode, dmap); >>> if (ret) { >>> pr_warn("Failed to remove mapping. offset=0x%llx len=0x%llx\n", >>> @@ -4051,7 +4063,7 @@ static int reclaim_one_dmap_locked(struct fuse_conn *fc, struct inode *inode, >>> if (!dmap) >>> return NULL; >>> >>> - ret = reclaim_one_dmap_locked(fc, inode, dmap); >>> + ret = reclaim_one_dmap_locked(fc, inode, dmap, true); >>> if (ret < 0) >>> return ERR_PTR(ret); >>> >>> @@ -4136,7 +4148,7 @@ int lookup_and_reclaim_dmap_locked(struct fuse_conn *fc, struct inode *inode, >>> if (refcount_read(&dmap->refcnt) > 1) >>> return 0; >>> >>> - ret = reclaim_one_dmap_locked(fc, inode, dmap); >>> + ret = reclaim_one_dmap_locked(fc, inode, dmap, false); >>> if (ret < 0) >>> return ret; >>> >>> > . >