* [Virtio-fs] [PATCH-v3 0/1] virtiofs: FUSE_REMOVEMAPPING remove multiple entries in one call
@ 2019-06-05 3:02 Peng Tao
2019-06-05 3:02 ` [Virtio-fs] [PATCH] " Peng Tao
2019-06-05 20:30 ` [Virtio-fs] [PATCH-v3 0/1] " Vivek Goyal
0 siblings, 2 replies; 8+ messages in thread
From: Peng Tao @ 2019-06-05 3:02 UTC (permalink / raw)
To: virtio-fs; +Cc: Peng Tao, Vivek Goyal
Hi Vivek,
This works with corresponding qemu virtiofsd patch:
"[PATCH-v3] virtiofsd: make FUSE_REMOVEMAPPING support multiple entries".
v1-v2: make fuse_removemapping_in count fuse_removemapping_one
v2->v3:
1. fold in Vivek's cleanup and rename all forget_one to remove_one
2. assert dmap->list is not used by anyone when adding to temporary list
3. list_del() dmap->list when removing it from to_remove list
Please add your SOB line as I don't see it with your cleanup and thus
did not add it to the patch when folding your change.
Thanks,
Tao
Peng Tao (1):
virtiofs: FUSE_REMOVEMAPPING remove multiple entries in one call
fs/fuse/file.c | 205 +++++++++++++++++++++++++++-----------
fs/fuse/fuse_i.h | 6 +-
fs/fuse/inode.c | 2 +-
include/uapi/linux/fuse.h | 7 ++
4 files changed, 160 insertions(+), 60 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Virtio-fs] [PATCH] virtiofs: FUSE_REMOVEMAPPING remove multiple entries in one call
2019-06-05 3:02 [Virtio-fs] [PATCH-v3 0/1] virtiofs: FUSE_REMOVEMAPPING remove multiple entries in one call Peng Tao
@ 2019-06-05 3:02 ` Peng Tao
2019-06-05 18:06 ` Liu Bo
2019-06-05 20:30 ` [Virtio-fs] [PATCH-v3 0/1] " Vivek Goyal
1 sibling, 1 reply; 8+ messages in thread
From: Peng Tao @ 2019-06-05 3:02 UTC (permalink / raw)
To: virtio-fs; +Cc: Peng Tao, Vivek Goyal
Enhance FUSE_REMOVEMAPPING wire protocol to remove multiple mappings
in one call. So that fuse_removemapping() can send just one fuse request
for many dmap entries.
Signed-off-by: Peng Tao <tao.peng@linux.alibaba.com>
---
fs/fuse/file.c | 205 +++++++++++++++++++++++++++-----------
fs/fuse/fuse_i.h | 6 +-
fs/fuse/inode.c | 2 +-
include/uapi/linux/fuse.h | 7 ++
4 files changed, 160 insertions(+), 60 deletions(-)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index d0979dc32f08..5a0b80719815 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -27,6 +27,9 @@ INTERVAL_TREE_DEFINE(struct fuse_dax_mapping, rb, __u64, __subtree_last,
static struct fuse_dax_mapping *alloc_dax_mapping_reclaim(struct fuse_conn *fc,
struct inode *inode);
+static void fuse_dax_free_mappings_range(struct fuse_conn *fc,
+ struct inode *inode, loff_t start, loff_t end);
+
static int fuse_send_open(struct fuse_conn *fc, u64 nodeid, struct file *file,
int opcode, struct fuse_open_out *outargp)
{
@@ -319,75 +322,87 @@ static int fuse_setup_one_mapping(struct inode *inode,
return 0;
}
-static int fuse_removemapping_one(struct inode *inode,
- struct fuse_dax_mapping *dmap)
+static int
+fuse_send_removemapping(struct inode *inode,
+ struct fuse_removemapping_in *inargp,
+ struct fuse_removemapping_one *remove_one)
{
struct fuse_inode *fi = get_fuse_inode(inode);
struct fuse_conn *fc = get_fuse_conn(inode);
- struct fuse_removemapping_in inarg;
FUSE_ARGS(args);
- memset(&inarg, 0, sizeof(inarg));
- inarg.moffset = dmap->window_offset;
- inarg.len = dmap->length;
args.in.h.opcode = FUSE_REMOVEMAPPING;
args.in.h.nodeid = fi->nodeid;
- args.in.numargs = 1;
- args.in.args[0].size = sizeof(inarg);
- args.in.args[0].value = &inarg;
+ args.in.numargs = 2;
+ args.in.args[0].size = sizeof(*inargp);
+ args.in.args[0].value = inargp;
+ args.in.args[1].size = inargp->count * sizeof(*remove_one);
+ args.in.args[1].value = remove_one;
return fuse_simple_request(fc, &args);
}
-/*
- * It is called from evict_inode() and by that time inode is going away. So
- * this function does not take any locks like fi->i_dmap_sem for traversing
- * that fuse inode interval tree. If that lock is taken then lock validator
- * complains of deadlock situation w.r.t fs_reclaim lock.
- */
-void fuse_removemapping(struct inode *inode)
+static int dmap_list_send_removemappings(struct inode *inode, unsigned num,
+ struct list_head *to_remove)
{
- struct fuse_conn *fc = get_fuse_conn(inode);
- struct fuse_inode *fi = get_fuse_inode(inode);
- ssize_t err;
+ struct fuse_removemapping_one *remove_one, *ptr;
+ struct fuse_removemapping_in inarg;
struct fuse_dax_mapping *dmap;
+ int ret, i = 0, nr_alloc;
- /* Clear the mappings list */
- while (true) {
- WARN_ON(fi->nr_dmaps < 0);
+ nr_alloc = min_t(unsigned int, num, FUSE_REMOVEMAPPING_MAX_ENTRY);
+ remove_one = kmalloc_array(nr_alloc, sizeof(*remove_one), GFP_NOIO);
+ if (!remove_one)
+ return -ENOMEM;
- dmap = fuse_dax_interval_tree_iter_first(&fi->dmap_tree, 0,
- -1);
- if (dmap) {
- fuse_dax_interval_tree_remove(dmap, &fi->dmap_tree);
- fi->nr_dmaps--;
- dmap_remove_busy_list(fc, dmap);
+ ptr = remove_one;
+ list_for_each_entry(dmap, to_remove, list) {
+ ptr->moffset = dmap->window_offset;
+ ptr->len = dmap->length;
+ ptr++;
+ i++;
+ num--;
+ if (i >= nr_alloc || num == 0) {
+ memset(&inarg, 0, sizeof(inarg));
+ inarg.count = i;
+ ret = fuse_send_removemapping(inode, &inarg,
+ remove_one);
+ if (ret)
+ goto out;
+ ptr = remove_one;
+ i = 0;
}
+ }
+out:
+ kfree(remove_one);
+ return ret;
+}
- if (!dmap)
- break;
+static int fuse_removemapping_one(struct inode *inode,
+ struct fuse_dax_mapping *dmap)
+{
+ struct fuse_removemapping_in inarg;
+ struct fuse_removemapping_one remove_one;
- /*
- * During umount/shutdown, fuse connection is dropped first
- * and later evict_inode() is called later. That means any
- * removemapping messages are going to fail. Send messages
- * only if connection is up. Otherwise fuse daemon is
- * responsible for cleaning up any leftover references and
- * mappings.
- */
- if (fc->connected) {
- err = fuse_removemapping_one(inode, dmap);
- if (err) {
- pr_warn("Failed to removemapping. offset=0x%llx"
- " len=0x%llx\n", dmap->window_offset,
- dmap->length);
- }
- }
+ memset(&inarg, 0, sizeof(inarg));
+ /* TODO: fill in inarg.fh when available */
+ inarg.count = 1;
+ memset(&remove_one, 0, sizeof(remove_one));
+ remove_one.moffset = dmap->window_offset;
+ remove_one.len = dmap->length;
- dmap->inode = NULL;
+ return fuse_send_removemapping(inode, &inarg, &remove_one);
+}
- /* Add it back to free ranges list */
- free_dax_mapping(fc, dmap);
- }
+/*
+ * It is called from evict_inode() and by that time inode is going away. So
+ * this function does not take any locks like fi->i_dmap_sem for traversing
+ * that fuse inode interval tree. If that lock is taken then lock validator
+ * complains of deadlock situation w.r.t fs_reclaim lock.
+ */
+void fuse_cleanup_inode_mappings(struct inode *inode)
+{
+ struct fuse_conn *fc = get_fuse_conn(inode);
+ fuse_dax_free_mappings_range(fc, inode, 0, -1);
}
void fuse_finish_open(struct inode *inode, struct file *file)
@@ -3980,6 +3995,88 @@ static struct fuse_dax_mapping *alloc_dax_mapping_reclaim(struct fuse_conn *fc,
}
}
+/*
+ * Cleanup dmap entry and add back to free list. This should be called with
+ * fc->lock held.
+ */
+static void fuse_dax_do_free_mapping_locked(struct fuse_conn *fc,
+ struct fuse_dax_mapping *dmap)
+{
+ __dmap_remove_busy_list(fc, dmap);
+ dmap->inode = NULL;
+ dmap->start = dmap->end = 0;
+ __free_dax_mapping(fc, dmap);
+ pr_debug("fuse: freed memory range start=0x%llx end=0x%llx "
+ "window_offset=0x%llx length=0x%llx\n", dmap->start,
+ dmap->end, dmap->window_offset, dmap->length);
+}
+
+/*
+ * Free inode dmap entries whose range falls entirely inside [start, end].
+ * Does not take any locks. Caller must take care of any lock requirements.
+ * Lock ordering follows fuse_dax_free_one_mapping().
+ * inode->i_rwsem, fuse_inode->i_mmap_sem and fuse_inode->i_dmap_sem must be
+ * held exclusively, unless it is called from evict_inode() where no one else
+ * is accessing the inode.
+ */
+static void fuse_dax_free_mappings_range(struct fuse_conn *fc,
+ struct inode *inode, loff_t start,
+ loff_t end)
+{
+ struct fuse_inode *fi = get_fuse_inode(inode);
+ struct fuse_dax_mapping *dmap, *n;
+ int err, num = 0;
+ LIST_HEAD(to_remove);
+
+ pr_debug("fuse: %s: start=0x%llx, end=0x%llx\n", __func__, start, end);
+
+ /*
+ * Interval tree search matches intersecting entries. Adjust the range
+ * to avoid dropping partial valid entries.
+ */
+ start = ALIGN(start, FUSE_DAX_MEM_RANGE_SZ);
+ end = ALIGN_DOWN(end, FUSE_DAX_MEM_RANGE_SZ);
+
+ while (1) {
+ dmap = fuse_dax_interval_tree_iter_first(&fi->dmap_tree, start,
+ end);
+ if (!dmap)
+ break;
+ fuse_dax_interval_tree_remove(dmap, &fi->dmap_tree);
+ num++;
+ WARN_ON(!list_empty(&dmap->list));
+ list_add_tail(&dmap->list, &to_remove);
+ }
+
+ /* Nothing to remove */
+ if (list_empty(&to_remove))
+ return;
+
+ WARN_ON(fi->nr_dmaps < num);
+ fi->nr_dmaps -= num;
+ /*
+ * During umount/shutdown, fuse connection is dropped first
+ * and evict_inode() is called later. That means any
+ * removemapping messages are going to fail. Send messages
+ * only if connection is up. Otherwise fuse daemon is
+ * responsible for cleaning up any leftover references and
+ * mappings.
+ */
+ if (fc->connected) {
+ err = dmap_list_send_removemappings(inode, num, &to_remove);
+ if (err) {
+ pr_warn("Failed to removemappings. start=0x%llx"
+ " end=0x%llx\n", start, end);
+ }
+ }
+ spin_lock(&fc->lock);
+ list_for_each_entry_safe(dmap, n, &to_remove, list) {
+ list_del(&dmap->list);
+ fuse_dax_do_free_mapping_locked(fc, dmap);
+ }
+ spin_unlock(&fc->lock);
+}
+
int fuse_dax_free_one_mapping_locked(struct fuse_conn *fc, struct inode *inode,
u64 dmap_start)
{
@@ -4003,15 +4100,9 @@ int fuse_dax_free_one_mapping_locked(struct fuse_conn *fc, struct inode *inode,
/* Cleanup dmap entry and add back to free list */
spin_lock(&fc->lock);
- __dmap_remove_busy_list(fc, dmap);
- dmap->inode = NULL;
- dmap->start = dmap->end = 0;
- __free_dax_mapping(fc, dmap);
+ fuse_dax_do_free_mapping_locked(fc, dmap);
spin_unlock(&fc->lock);
- pr_debug("fuse: freed memory range window_offset=0x%llx,"
- " length=0x%llx\n", dmap->window_offset,
- dmap->length);
return ret;
}
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 7ac7f9a0b81b..17784dbd49a7 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -117,7 +117,9 @@ struct fuse_dax_mapping {
/* Pointer to inode where this memory range is mapped */
struct inode *inode;
- /* Will connect in fc->free_ranges to keep track of free memory */
+ /* Will connect in fc->free_ranges to keep track of free memory.
+ * When the mapping is in fc->busy_ranges, the field can also be
+ * used by temporary lists */
struct list_head list;
/* For interval tree in file/inode */
@@ -1286,6 +1288,6 @@ unsigned fuse_len_args(unsigned numargs, struct fuse_arg *args);
*/
u64 fuse_get_unique(struct fuse_iqueue *fiq);
void fuse_dax_free_mem_worker(struct work_struct *work);
-void fuse_removemapping(struct inode *inode);
+void fuse_cleanup_inode_mappings(struct inode *inode);
#endif /* _FS_FUSE_I_H */
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 302f7e04b645..4277324a7c3b 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -123,7 +123,7 @@ static void fuse_evict_inode(struct inode *inode)
struct fuse_conn *fc = get_fuse_conn(inode);
struct fuse_inode *fi = get_fuse_inode(inode);
if (IS_DAX(inode)) {
- fuse_removemapping(inode);
+ fuse_cleanup_inode_mappings(inode);
WARN_ON(fi->nr_dmaps);
}
fuse_queue_forget(fc, fi->forget, fi->nodeid, fi->nlookup);
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 5042e227e8a8..b588a028f099 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -850,10 +850,17 @@ struct fuse_setupmapping_out {
struct fuse_removemapping_in {
/* An already open handle */
uint64_t fh;
+ /* number of fuse_removemapping_one follows */
+ uint32_t count;
+};
+
+struct fuse_removemapping_one {
/* Offset into the dax window start the unmapping */
uint64_t moffset;
/* Length of mapping required */
uint64_t len;
};
+#define FUSE_REMOVEMAPPING_MAX_ENTRY \
+ (PAGE_SIZE / sizeof(struct fuse_removemapping_one))
#endif /* _LINUX_FUSE_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofs: FUSE_REMOVEMAPPING remove multiple entries in one call
2019-06-05 3:02 ` [Virtio-fs] [PATCH] " Peng Tao
@ 2019-06-05 18:06 ` Liu Bo
2019-06-05 18:35 ` Vivek Goyal
0 siblings, 1 reply; 8+ messages in thread
From: Liu Bo @ 2019-06-05 18:06 UTC (permalink / raw)
To: Peng Tao; +Cc: virtio-fs, Vivek Goyal
On Wed, Jun 05, 2019 at 11:02:33AM +0800, Peng Tao wrote:
> Enhance FUSE_REMOVEMAPPING wire protocol to remove multiple mappings
> in one call. So that fuse_removemapping() can send just one fuse request
> for many dmap entries.
>
> Signed-off-by: Peng Tao <tao.peng@linux.alibaba.com>
> ---
> fs/fuse/file.c | 205 +++++++++++++++++++++++++++-----------
> fs/fuse/fuse_i.h | 6 +-
> fs/fuse/inode.c | 2 +-
> include/uapi/linux/fuse.h | 7 ++
> 4 files changed, 160 insertions(+), 60 deletions(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index d0979dc32f08..5a0b80719815 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -27,6 +27,9 @@ INTERVAL_TREE_DEFINE(struct fuse_dax_mapping, rb, __u64, __subtree_last,
>
> static struct fuse_dax_mapping *alloc_dax_mapping_reclaim(struct fuse_conn *fc,
> struct inode *inode);
> +static void fuse_dax_free_mappings_range(struct fuse_conn *fc,
> + struct inode *inode, loff_t start, loff_t end);
> +
> static int fuse_send_open(struct fuse_conn *fc, u64 nodeid, struct file *file,
> int opcode, struct fuse_open_out *outargp)
> {
> @@ -319,75 +322,87 @@ static int fuse_setup_one_mapping(struct inode *inode,
> return 0;
> }
>
> -static int fuse_removemapping_one(struct inode *inode,
> - struct fuse_dax_mapping *dmap)
> +static int
> +fuse_send_removemapping(struct inode *inode,
> + struct fuse_removemapping_in *inargp,
> + struct fuse_removemapping_one *remove_one)
> {
> struct fuse_inode *fi = get_fuse_inode(inode);
> struct fuse_conn *fc = get_fuse_conn(inode);
> - struct fuse_removemapping_in inarg;
> FUSE_ARGS(args);
>
> - memset(&inarg, 0, sizeof(inarg));
> - inarg.moffset = dmap->window_offset;
> - inarg.len = dmap->length;
> args.in.h.opcode = FUSE_REMOVEMAPPING;
> args.in.h.nodeid = fi->nodeid;
> - args.in.numargs = 1;
> - args.in.args[0].size = sizeof(inarg);
> - args.in.args[0].value = &inarg;
> + args.in.numargs = 2;
> + args.in.args[0].size = sizeof(*inargp);
> + args.in.args[0].value = inargp;
> + args.in.args[1].size = inargp->count * sizeof(*remove_one);
> + args.in.args[1].value = remove_one;
> return fuse_simple_request(fc, &args);
> }
>
> -/*
> - * It is called from evict_inode() and by that time inode is going away. So
> - * this function does not take any locks like fi->i_dmap_sem for traversing
> - * that fuse inode interval tree. If that lock is taken then lock validator
> - * complains of deadlock situation w.r.t fs_reclaim lock.
> - */
> -void fuse_removemapping(struct inode *inode)
> +static int dmap_list_send_removemappings(struct inode *inode, unsigned num,
> + struct list_head *to_remove)
> {
> - struct fuse_conn *fc = get_fuse_conn(inode);
> - struct fuse_inode *fi = get_fuse_inode(inode);
> - ssize_t err;
> + struct fuse_removemapping_one *remove_one, *ptr;
> + struct fuse_removemapping_in inarg;
> struct fuse_dax_mapping *dmap;
> + int ret, i = 0, nr_alloc;
>
> - /* Clear the mappings list */
> - while (true) {
> - WARN_ON(fi->nr_dmaps < 0);
> + nr_alloc = min_t(unsigned int, num, FUSE_REMOVEMAPPING_MAX_ENTRY);
> + remove_one = kmalloc_array(nr_alloc, sizeof(*remove_one), GFP_NOIO);
GFP_NOIO's comments implies that memalloc_noio_{save,restore} are preferred,
also would GFP_NOFS be better?
> + if (!remove_one)
> + return -ENOMEM;
>
> - dmap = fuse_dax_interval_tree_iter_first(&fi->dmap_tree, 0,
> - -1);
> - if (dmap) {
> - fuse_dax_interval_tree_remove(dmap, &fi->dmap_tree);
> - fi->nr_dmaps--;
> - dmap_remove_busy_list(fc, dmap);
> + ptr = remove_one;
> + list_for_each_entry(dmap, to_remove, list) {
> + ptr->moffset = dmap->window_offset;
> + ptr->len = dmap->length;
> + ptr++;
> + i++;
> + num--;
> + if (i >= nr_alloc || num == 0) {
> + memset(&inarg, 0, sizeof(inarg));
> + inarg.count = i;
> + ret = fuse_send_removemapping(inode, &inarg,
> + remove_one);
> + if (ret)
> + goto out;
> + ptr = remove_one;
> + i = 0;
> }
> + }
> +out:
> + kfree(remove_one);
> + return ret;
> +}
>
> - if (!dmap)
> - break;
> +static int fuse_removemapping_one(struct inode *inode,
> + struct fuse_dax_mapping *dmap)
> +{
> + struct fuse_removemapping_in inarg;
> + struct fuse_removemapping_one remove_one;
>
> - /*
> - * During umount/shutdown, fuse connection is dropped first
> - * and later evict_inode() is called later. That means any
> - * removemapping messages are going to fail. Send messages
> - * only if connection is up. Otherwise fuse daemon is
> - * responsible for cleaning up any leftover references and
> - * mappings.
> - */
> - if (fc->connected) {
> - err = fuse_removemapping_one(inode, dmap);
> - if (err) {
> - pr_warn("Failed to removemapping. offset=0x%llx"
> - " len=0x%llx\n", dmap->window_offset,
> - dmap->length);
> - }
> - }
> + memset(&inarg, 0, sizeof(inarg));
> + /* TODO: fill in inarg.fh when available */
> + inarg.count = 1;
> + memset(&remove_one, 0, sizeof(remove_one));
> + remove_one.moffset = dmap->window_offset;
> + remove_one.len = dmap->length;
>
> - dmap->inode = NULL;
> + return fuse_send_removemapping(inode, &inarg, &remove_one);
> +}
>
> - /* Add it back to free ranges list */
> - free_dax_mapping(fc, dmap);
> - }
> +/*
> + * It is called from evict_inode() and by that time inode is going away. So
> + * this function does not take any locks like fi->i_dmap_sem for traversing
> + * that fuse inode interval tree. If that lock is taken then lock validator
> + * complains of deadlock situation w.r.t fs_reclaim lock.
> + */
> +void fuse_cleanup_inode_mappings(struct inode *inode)
> +{
> + struct fuse_conn *fc = get_fuse_conn(inode);
> + fuse_dax_free_mappings_range(fc, inode, 0, -1);
> }
>
> void fuse_finish_open(struct inode *inode, struct file *file)
> @@ -3980,6 +3995,88 @@ static struct fuse_dax_mapping *alloc_dax_mapping_reclaim(struct fuse_conn *fc,
> }
> }
>
> +/*
> + * Cleanup dmap entry and add back to free list. This should be called with
> + * fc->lock held.
> + */
> +static void fuse_dax_do_free_mapping_locked(struct fuse_conn *fc,
> + struct fuse_dax_mapping *dmap)
> +{
> + __dmap_remove_busy_list(fc, dmap);
> + dmap->inode = NULL;
> + dmap->start = dmap->end = 0;
> + __free_dax_mapping(fc, dmap);
> + pr_debug("fuse: freed memory range start=0x%llx end=0x%llx "
> + "window_offset=0x%llx length=0x%llx\n", dmap->start,
> + dmap->end, dmap->window_offset, dmap->length);
pr_debug() needs to be placed at the beginning as dmap->start & end have been
zero'd.
Other parts look good to me.
Reviewed-by: Liu Bo <bo.liu@linux.alibaba.com>
thanks,
-liubo
> +}
> +
> +/*
> + * Free inode dmap entries whose range falls entirely inside [start, end].
> + * Does not take any locks. Caller must take care of any lock requirements.
> + * Lock ordering follows fuse_dax_free_one_mapping().
> + * inode->i_rwsem, fuse_inode->i_mmap_sem and fuse_inode->i_dmap_sem must be
> + * held exclusively, unless it is called from evict_inode() where no one else
> + * is accessing the inode.
> + */
> +static void fuse_dax_free_mappings_range(struct fuse_conn *fc,
> + struct inode *inode, loff_t start,
> + loff_t end)
> +{
> + struct fuse_inode *fi = get_fuse_inode(inode);
> + struct fuse_dax_mapping *dmap, *n;
> + int err, num = 0;
> + LIST_HEAD(to_remove);
> +
> + pr_debug("fuse: %s: start=0x%llx, end=0x%llx\n", __func__, start, end);
> +
> + /*
> + * Interval tree search matches intersecting entries. Adjust the range
> + * to avoid dropping partial valid entries.
> + */
> + start = ALIGN(start, FUSE_DAX_MEM_RANGE_SZ);
> + end = ALIGN_DOWN(end, FUSE_DAX_MEM_RANGE_SZ);
> +
> + while (1) {
> + dmap = fuse_dax_interval_tree_iter_first(&fi->dmap_tree, start,
> + end);
> + if (!dmap)
> + break;
> + fuse_dax_interval_tree_remove(dmap, &fi->dmap_tree);
> + num++;
> + WARN_ON(!list_empty(&dmap->list));
> + list_add_tail(&dmap->list, &to_remove);
> + }
> +
> + /* Nothing to remove */
> + if (list_empty(&to_remove))
> + return;
> +
> + WARN_ON(fi->nr_dmaps < num);
> + fi->nr_dmaps -= num;
> + /*
> + * During umount/shutdown, fuse connection is dropped first
> + * and evict_inode() is called later. That means any
> + * removemapping messages are going to fail. Send messages
> + * only if connection is up. Otherwise fuse daemon is
> + * responsible for cleaning up any leftover references and
> + * mappings.
> + */
> + if (fc->connected) {
> + err = dmap_list_send_removemappings(inode, num, &to_remove);
> + if (err) {
> + pr_warn("Failed to removemappings. start=0x%llx"
> + " end=0x%llx\n", start, end);
> + }
> + }
> + spin_lock(&fc->lock);
> + list_for_each_entry_safe(dmap, n, &to_remove, list) {
> + list_del(&dmap->list);
> + fuse_dax_do_free_mapping_locked(fc, dmap);
> + }
> + spin_unlock(&fc->lock);
> +}
> +
> int fuse_dax_free_one_mapping_locked(struct fuse_conn *fc, struct inode *inode,
> u64 dmap_start)
> {
> @@ -4003,15 +4100,9 @@ int fuse_dax_free_one_mapping_locked(struct fuse_conn *fc, struct inode *inode,
>
> /* Cleanup dmap entry and add back to free list */
> spin_lock(&fc->lock);
> - __dmap_remove_busy_list(fc, dmap);
> - dmap->inode = NULL;
> - dmap->start = dmap->end = 0;
> - __free_dax_mapping(fc, dmap);
> + fuse_dax_do_free_mapping_locked(fc, dmap);
> spin_unlock(&fc->lock);
>
> - pr_debug("fuse: freed memory range window_offset=0x%llx,"
> - " length=0x%llx\n", dmap->window_offset,
> - dmap->length);
> return ret;
> }
>
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 7ac7f9a0b81b..17784dbd49a7 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -117,7 +117,9 @@ struct fuse_dax_mapping {
> /* Pointer to inode where this memory range is mapped */
> struct inode *inode;
>
> - /* Will connect in fc->free_ranges to keep track of free memory */
> + /* Will connect in fc->free_ranges to keep track of free memory.
> + * When the mapping is in fc->busy_ranges, the field can also be
> + * used by temporary lists */
> struct list_head list;
>
> /* For interval tree in file/inode */
> @@ -1286,6 +1288,6 @@ unsigned fuse_len_args(unsigned numargs, struct fuse_arg *args);
> */
> u64 fuse_get_unique(struct fuse_iqueue *fiq);
> void fuse_dax_free_mem_worker(struct work_struct *work);
> -void fuse_removemapping(struct inode *inode);
> +void fuse_cleanup_inode_mappings(struct inode *inode);
>
> #endif /* _FS_FUSE_I_H */
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 302f7e04b645..4277324a7c3b 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -123,7 +123,7 @@ static void fuse_evict_inode(struct inode *inode)
> struct fuse_conn *fc = get_fuse_conn(inode);
> struct fuse_inode *fi = get_fuse_inode(inode);
> if (IS_DAX(inode)) {
> - fuse_removemapping(inode);
> + fuse_cleanup_inode_mappings(inode);
> WARN_ON(fi->nr_dmaps);
> }
> fuse_queue_forget(fc, fi->forget, fi->nodeid, fi->nlookup);
> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> index 5042e227e8a8..b588a028f099 100644
> --- a/include/uapi/linux/fuse.h
> +++ b/include/uapi/linux/fuse.h
> @@ -850,10 +850,17 @@ struct fuse_setupmapping_out {
> struct fuse_removemapping_in {
> /* An already open handle */
> uint64_t fh;
> + /* number of fuse_removemapping_one follows */
> + uint32_t count;
> +};
> +
> +struct fuse_removemapping_one {
> /* Offset into the dax window start the unmapping */
> uint64_t moffset;
> /* Length of mapping required */
> uint64_t len;
> };
> +#define FUSE_REMOVEMAPPING_MAX_ENTRY \
> + (PAGE_SIZE / sizeof(struct fuse_removemapping_one))
>
> #endif /* _LINUX_FUSE_H */
> --
> 2.17.1
>
> _______________________________________________
> Virtio-fs mailing list
> Virtio-fs@redhat.com
> https://www.redhat.com/mailman/listinfo/virtio-fs
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofs: FUSE_REMOVEMAPPING remove multiple entries in one call
2019-06-05 18:06 ` Liu Bo
@ 2019-06-05 18:35 ` Vivek Goyal
2019-06-05 18:50 ` Liu Bo
0 siblings, 1 reply; 8+ messages in thread
From: Vivek Goyal @ 2019-06-05 18:35 UTC (permalink / raw)
To: Liu Bo; +Cc: virtio-fs, Peng Tao
On Wed, Jun 05, 2019 at 11:06:46AM -0700, Liu Bo wrote:
[..]
> > -/*
> > - * It is called from evict_inode() and by that time inode is going away. So
> > - * this function does not take any locks like fi->i_dmap_sem for traversing
> > - * that fuse inode interval tree. If that lock is taken then lock validator
> > - * complains of deadlock situation w.r.t fs_reclaim lock.
> > - */
> > -void fuse_removemapping(struct inode *inode)
> > +static int dmap_list_send_removemappings(struct inode *inode, unsigned num,
> > + struct list_head *to_remove)
> > {
> > - struct fuse_conn *fc = get_fuse_conn(inode);
> > - struct fuse_inode *fi = get_fuse_inode(inode);
> > - ssize_t err;
> > + struct fuse_removemapping_one *remove_one, *ptr;
> > + struct fuse_removemapping_in inarg;
> > struct fuse_dax_mapping *dmap;
> > + int ret, i = 0, nr_alloc;
> >
> > - /* Clear the mappings list */
> > - while (true) {
> > - WARN_ON(fi->nr_dmaps < 0);
> > + nr_alloc = min_t(unsigned int, num, FUSE_REMOVEMAPPING_MAX_ENTRY);
> > + remove_one = kmalloc_array(nr_alloc, sizeof(*remove_one), GFP_NOIO);
>
> GFP_NOIO's comments implies that memalloc_noio_{save,restore} are preferred,
> also would GFP_NOFS be better?
GFP_NOFS sounds reasonable. I am not sure how memalloc_noio_{save,restore}
API is better as opposed to using GFP_NOFS.
[..]
> > +/*
> > + * Cleanup dmap entry and add back to free list. This should be called with
> > + * fc->lock held.
> > + */
> > +static void fuse_dax_do_free_mapping_locked(struct fuse_conn *fc,
> > + struct fuse_dax_mapping *dmap)
> > +{
> > + __dmap_remove_busy_list(fc, dmap);
> > + dmap->inode = NULL;
> > + dmap->start = dmap->end = 0;
> > + __free_dax_mapping(fc, dmap);
> > + pr_debug("fuse: freed memory range start=0x%llx end=0x%llx "
> > + "window_offset=0x%llx length=0x%llx\n", dmap->start,
> > + dmap->end, dmap->window_offset, dmap->length);
>
> pr_debug() needs to be placed at the beginning as dmap->start & end have been
> zero'd.
>
Good point. Will fix.
Vivek
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofs: FUSE_REMOVEMAPPING remove multiple entries in one call
2019-06-05 18:35 ` Vivek Goyal
@ 2019-06-05 18:50 ` Liu Bo
0 siblings, 0 replies; 8+ messages in thread
From: Liu Bo @ 2019-06-05 18:50 UTC (permalink / raw)
To: Vivek Goyal; +Cc: virtio-fs, Peng Tao
On Wed, Jun 05, 2019 at 02:35:14PM -0400, Vivek Goyal wrote:
> On Wed, Jun 05, 2019 at 11:06:46AM -0700, Liu Bo wrote:
>
> [..]
> > > -/*
> > > - * It is called from evict_inode() and by that time inode is going away. So
> > > - * this function does not take any locks like fi->i_dmap_sem for traversing
> > > - * that fuse inode interval tree. If that lock is taken then lock validator
> > > - * complains of deadlock situation w.r.t fs_reclaim lock.
> > > - */
> > > -void fuse_removemapping(struct inode *inode)
> > > +static int dmap_list_send_removemappings(struct inode *inode, unsigned num,
> > > + struct list_head *to_remove)
> > > {
> > > - struct fuse_conn *fc = get_fuse_conn(inode);
> > > - struct fuse_inode *fi = get_fuse_inode(inode);
> > > - ssize_t err;
> > > + struct fuse_removemapping_one *remove_one, *ptr;
> > > + struct fuse_removemapping_in inarg;
> > > struct fuse_dax_mapping *dmap;
> > > + int ret, i = 0, nr_alloc;
> > >
> > > - /* Clear the mappings list */
> > > - while (true) {
> > > - WARN_ON(fi->nr_dmaps < 0);
> > > + nr_alloc = min_t(unsigned int, num, FUSE_REMOVEMAPPING_MAX_ENTRY);
> > > + remove_one = kmalloc_array(nr_alloc, sizeof(*remove_one), GFP_NOIO);
> >
> > GFP_NOIO's comments implies that memalloc_noio_{save,restore} are preferred,
> > also would GFP_NOFS be better?
>
> GFP_NOFS sounds reasonable. I am not sure how memalloc_noio_{save,restore}
> API is better as opposed to using GFP_NOFS.
>
mm devs need memalloc_nofs_{save,restore} to encourage fs to declare a
scope that all children callees inherit the nofs attribute.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7dea19f9ee636cb244109a4dba426bbb3e5304b7
thanks,
-liubo
> [..]
> > > +/*
> > > + * Cleanup dmap entry and add back to free list. This should be called with
> > > + * fc->lock held.
> > > + */
> > > +static void fuse_dax_do_free_mapping_locked(struct fuse_conn *fc,
> > > + struct fuse_dax_mapping *dmap)
> > > +{
> > > + __dmap_remove_busy_list(fc, dmap);
> > > + dmap->inode = NULL;
> > > + dmap->start = dmap->end = 0;
> > > + __free_dax_mapping(fc, dmap);
> > > + pr_debug("fuse: freed memory range start=0x%llx end=0x%llx "
> > > + "window_offset=0x%llx length=0x%llx\n", dmap->start,
> > > + dmap->end, dmap->window_offset, dmap->length);
> >
> > pr_debug() needs to be placed at the beginning as dmap->start & end have been
> > zero'd.
> >
>
> Good point. Will fix.
>
> Vivek
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Virtio-fs] [PATCH-v3 0/1] virtiofs: FUSE_REMOVEMAPPING remove multiple entries in one call
2019-06-05 3:02 [Virtio-fs] [PATCH-v3 0/1] virtiofs: FUSE_REMOVEMAPPING remove multiple entries in one call Peng Tao
2019-06-05 3:02 ` [Virtio-fs] [PATCH] " Peng Tao
@ 2019-06-05 20:30 ` Vivek Goyal
2019-06-06 6:51 ` Peng Tao
1 sibling, 1 reply; 8+ messages in thread
From: Vivek Goyal @ 2019-06-05 20:30 UTC (permalink / raw)
To: Peng Tao; +Cc: virtio-fs
Hi Peng Tao,
I have squashed this patch and forget request wait patches into existing
pathces and pushed latest patches here.
https://github.com/rhvgoyal/linux/commits/virtio-fs-dev-5.1
Please give it a try.
I have made more function name changes into existing patches. There were
so many functions with name fuse_dax_free* that I was finding it hard
to remember which function is doing what.
Thanks
Vivek
On Wed, Jun 05, 2019 at 11:02:32AM +0800, Peng Tao wrote:
> Hi Vivek,
>
> This works with corresponding qemu virtiofsd patch:
> "[PATCH-v3] virtiofsd: make FUSE_REMOVEMAPPING support multiple entries".
>
> v1-v2: make fuse_removemapping_in count fuse_removemapping_one
> v2->v3:
> 1. fold in Vivek's cleanup and rename all forget_one to remove_one
> 2. assert dmap->list is not used by anyone when adding to temporary list
> 3. list_del() dmap->list when removing it from to_remove list
>
> Please add your SOB line as I don't see it with your cleanup and thus
> did not add it to the patch when folding your change.
>
> Thanks,
> Tao
>
> Peng Tao (1):
> virtiofs: FUSE_REMOVEMAPPING remove multiple entries in one call
>
> fs/fuse/file.c | 205 +++++++++++++++++++++++++++-----------
> fs/fuse/fuse_i.h | 6 +-
> fs/fuse/inode.c | 2 +-
> include/uapi/linux/fuse.h | 7 ++
> 4 files changed, 160 insertions(+), 60 deletions(-)
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Virtio-fs] [PATCH-v3 0/1] virtiofs: FUSE_REMOVEMAPPING remove multiple entries in one call
2019-06-05 20:30 ` [Virtio-fs] [PATCH-v3 0/1] " Vivek Goyal
@ 2019-06-06 6:51 ` Peng Tao
0 siblings, 0 replies; 8+ messages in thread
From: Peng Tao @ 2019-06-06 6:51 UTC (permalink / raw)
To: Vivek Goyal; +Cc: virtio-fs
On 2019/6/6 04:30, Vivek Goyal wrote:
> Hi Peng Tao,
>
> I have squashed this patch and forget request wait patches into existing
> pathces and pushed latest patches here.
>
> https://github.com/rhvgoyal/linux/commits/virtio-fs-dev-5.1
>
> Please give it a try.
Both look good and work as expected. Thanks!
Cheers,
Tao
>
> I have made more function name changes into existing patches. There were
> so many functions with name fuse_dax_free* that I was finding it hard
> to remember which function is doing what.
>
> Thanks
> Vivek
>
> On Wed, Jun 05, 2019 at 11:02:32AM +0800, Peng Tao wrote:
>> Hi Vivek,
>>
>> This works with corresponding qemu virtiofsd patch:
>> "[PATCH-v3] virtiofsd: make FUSE_REMOVEMAPPING support multiple entries".
>>
>> v1-v2: make fuse_removemapping_in count fuse_removemapping_one
>> v2->v3:
>> 1. fold in Vivek's cleanup and rename all forget_one to remove_one
>> 2. assert dmap->list is not used by anyone when adding to temporary list
>> 3. list_del() dmap->list when removing it from to_remove list
>>
>> Please add your SOB line as I don't see it with your cleanup and thus
>> did not add it to the patch when folding your change.
>>
>> Thanks,
>> Tao
>>
>> Peng Tao (1):
>> virtiofs: FUSE_REMOVEMAPPING remove multiple entries in one call
>>
>> fs/fuse/file.c | 205 +++++++++++++++++++++++++++-----------
>> fs/fuse/fuse_i.h | 6 +-
>> fs/fuse/inode.c | 2 +-
>> include/uapi/linux/fuse.h | 7 ++
>> 4 files changed, 160 insertions(+), 60 deletions(-)
>>
>> --
>> 2.17.1
>>
--
Into something rich and strange.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Virtio-fs] [PATCH] virtiofs: FUSE_REMOVEMAPPING remove multiple entries in one call
@ 2019-05-29 4:30 Peng Tao
0 siblings, 0 replies; 8+ messages in thread
From: Peng Tao @ 2019-05-29 4:30 UTC (permalink / raw)
To: virtio-fs; +Cc: Peng Tao, Vivek Goyal
Enhance FUSE_REMOVEMAPPING wire protocol to remove multiple mappings
in one call. So that fuse_removemapping() does not need to send
one fuse request for each dmap entry.
Signed-off-by: Peng Tao <tao.peng@linux.alibaba.com>
---
This works with corresponding virtiofsd patch
"[PATCH-v2] virtiofsd: make FUSE_REMOVEMAPPING support multiple entries".
fs/fuse/file.c | 195 +++++++++++++++++++++++++++-----------
include/uapi/linux/fuse.h | 9 +-
2 files changed, 146 insertions(+), 58 deletions(-)
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index d0979dc32f08..a6a34249c598 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -27,6 +27,9 @@ INTERVAL_TREE_DEFINE(struct fuse_dax_mapping, rb, __u64, __subtree_last,
static struct fuse_dax_mapping *alloc_dax_mapping_reclaim(struct fuse_conn *fc,
struct inode *inode);
+static void __fuse_dax_free_mappings_range(struct fuse_conn *fc,
+ struct inode *inode, loff_t start, loff_t end);
+
static int fuse_send_open(struct fuse_conn *fc, u64 nodeid, struct file *file,
int opcode, struct fuse_open_out *outargp)
{
@@ -319,75 +322,90 @@ static int fuse_setup_one_mapping(struct inode *inode,
return 0;
}
-static int fuse_removemapping_one(struct inode *inode,
- struct fuse_dax_mapping *dmap)
+static int fuse_send_removemapping_request(struct inode *inode,
+ struct fuse_removemapping_in_header *header,
+ struct fuse_removemapping_in *inargp)
{
struct fuse_inode *fi = get_fuse_inode(inode);
struct fuse_conn *fc = get_fuse_conn(inode);
- struct fuse_removemapping_in inarg;
FUSE_ARGS(args);
- memset(&inarg, 0, sizeof(inarg));
- inarg.moffset = dmap->window_offset;
- inarg.len = dmap->length;
args.in.h.opcode = FUSE_REMOVEMAPPING;
args.in.h.nodeid = fi->nodeid;
- args.in.numargs = 1;
- args.in.args[0].size = sizeof(inarg);
- args.in.args[0].value = &inarg;
+ args.in.numargs = 2;
+ args.in.args[0].size = sizeof(*header);
+ args.in.args[0].value = header;
+ args.in.args[1].size = header->count * sizeof(*inargp);
+ args.in.args[1].value = inargp;
return fuse_simple_request(fc, &args);
}
-/*
- * It is called from evict_inode() and by that time inode is going away. So
- * this function does not take any locks like fi->i_dmap_sem for traversing
- * that fuse inode interval tree. If that lock is taken then lock validator
- * complains of deadlock situation w.r.t fs_reclaim lock.
- */
-void fuse_removemapping(struct inode *inode)
+static int fuse_do_removemappings(struct inode *inode, unsigned num,
+ struct list_head *to_remove)
{
- struct fuse_conn *fc = get_fuse_conn(inode);
- struct fuse_inode *fi = get_fuse_inode(inode);
- ssize_t err;
+ struct fuse_removemapping_in *inargp, *ptr;
+ struct fuse_removemapping_in_header header;
struct fuse_dax_mapping *dmap;
+ int ret, i = 0;
- /* Clear the mappings list */
- while (true) {
- WARN_ON(fi->nr_dmaps < 0);
+ if (num <= FUSE_REMOVEMAPPING_MAX_ENTRY) {
+ inargp = kmalloc_array(num, sizeof(*inargp), GFP_NOIO);
+ } else {
+ inargp = kmalloc_array(FUSE_REMOVEMAPPING_MAX_ENTRY,
+ sizeof(*inargp), GFP_NOIO);
+ }
+ if (inargp == NULL)
+ return -ENOMEM;
- dmap = fuse_dax_interval_tree_iter_first(&fi->dmap_tree, 0,
- -1);
- if (dmap) {
- fuse_dax_interval_tree_remove(dmap, &fi->dmap_tree);
- fi->nr_dmaps--;
- dmap_remove_busy_list(fc, dmap);
+ ptr = inargp;
+ list_for_each_entry(dmap, to_remove, list) {
+ ptr->moffset = dmap->window_offset;
+ ptr->len = dmap->length;
+ ptr++;
+ i++;
+ num--;
+ if (i >= FUSE_REMOVEMAPPING_MAX_ENTRY || num == 0) {
+ memset(&header, 0, sizeof(header));
+ header.count = i;
+ ret = fuse_send_removemapping_request(inode, &header, inargp);
+ if (ret)
+ goto out;
+ ptr = inargp;
+ i = 0;
}
+ }
- if (!dmap)
- break;
+out:
+ kfree(inargp);
+ return ret;
+}
- /*
- * During umount/shutdown, fuse connection is dropped first
- * and later evict_inode() is called later. That means any
- * removemapping messages are going to fail. Send messages
- * only if connection is up. Otherwise fuse daemon is
- * responsible for cleaning up any leftover references and
- * mappings.
- */
- if (fc->connected) {
- err = fuse_removemapping_one(inode, dmap);
- if (err) {
- pr_warn("Failed to removemapping. offset=0x%llx"
- " len=0x%llx\n", dmap->window_offset,
- dmap->length);
- }
- }
+static int fuse_removemapping_one(struct inode *inode,
+ struct fuse_dax_mapping *dmap)
+{
+ struct fuse_removemapping_in inarg;
+ struct fuse_removemapping_in_header header;
- dmap->inode = NULL;
+ memset(&header, 0, sizeof(header));
+ /* TODO: fill in header.fh when available */
+ header.count = 1;
+ memset(&inarg, 0, sizeof(inarg));
+ inarg.moffset = dmap->window_offset;
+ inarg.len = dmap->length;
- /* Add it back to free ranges list */
- free_dax_mapping(fc, dmap);
- }
+ return fuse_send_removemapping_request(inode, &header, &inarg);
+}
+
+/*
+ * It is called from evict_inode() and by that time inode is going away. So
+ * this function does not take any locks like fi->i_dmap_sem for traversing
+ * that fuse inode interval tree. If that lock is taken then lock validator
+ * complains of deadlock situation w.r.t fs_reclaim lock.
+ */
+void fuse_removemapping(struct inode *inode)
+{
+ struct fuse_conn *fc = get_fuse_conn(inode);
+ __fuse_dax_free_mappings_range(fc, inode, 0, -1);
}
void fuse_finish_open(struct inode *inode, struct file *file)
@@ -3980,6 +3998,75 @@ static struct fuse_dax_mapping *alloc_dax_mapping_reclaim(struct fuse_conn *fc,
}
}
+/* Cleanup dmap entry and add back to free list */
+static void fuse_dax_do_free_mapping_locked(struct fuse_conn *fc, struct fuse_dax_mapping *dmap)
+{
+ pr_debug("fuse: freed memory range start=0x%llx end=0x%llx "
+ "window_offset=0x%llx length=0x%llx\n", dmap->start,
+ dmap->end, dmap->window_offset, dmap->length);
+ __dmap_remove_busy_list(fc, dmap);
+ dmap->inode = NULL;
+ dmap->start = dmap->end = 0;
+ __free_dax_mapping(fc, dmap);
+}
+
+/*
+ * Free inode dmap entries whose range falls entirely inside [start, end].
+ * Does not take any locks. Caller must take care of any lock requirements.
+ * Lock ordering follows fuse_dax_free_one_mapping().
+ * inode->i_rwsem, fuse_inode->i_mmap_sem and fuse_inode->i_dmap_sem must be
+ * held unless it is called from evict_inode() where no one else is accessing
+ * the inode.
+ */
+static void __fuse_dax_free_mappings_range(struct fuse_conn *fc,
+ struct inode *inode, loff_t start, loff_t end)
+{
+ struct fuse_inode *fi = get_fuse_inode(inode);
+ struct fuse_dax_mapping *dmap, *n;
+ int err, num = 0;
+ LIST_HEAD(to_remove);
+
+ pr_debug("fuse: __fuse_dax_free_mappings_range "
+ "start=0x%llx, end=0x%llx\n", start, end);
+ /* interval tree search matches intersecting entries.
+ * Adjust the range to avoid dropping partial valid entries. */
+ start = ALIGN(start, FUSE_DAX_MEM_RANGE_SZ);
+ end = ALIGN_DOWN(end, FUSE_DAX_MEM_RANGE_SZ);
+
+ while ((dmap = fuse_dax_interval_tree_iter_first(&fi->dmap_tree, start, end))) {
+ fuse_dax_interval_tree_remove(dmap, &fi->dmap_tree);
+ num++;
+ list_add(&dmap->list, &to_remove);
+ }
+
+ /* Nothing to remove */
+ if (list_empty(&to_remove))
+ return;
+
+ WARN_ON(fi->nr_dmaps < num);
+ fi->nr_dmaps -= num;
+ /*
+ * During umount/shutdown, fuse connection is dropped first
+ * and later evict_inode() is called later. That means any
+ * removemapping messages are going to fail. Send messages
+ * only if connection is up. Otherwise fuse daemon is
+ * responsible for cleaning up any leftover references and
+ * mappings.
+ */
+ if (fc->connected) {
+ err = fuse_do_removemappings(inode, num, &to_remove);
+ if (err) {
+ pr_warn("Failed to removemappings. start=0x%llx"
+ " end=0x%llx\n", start, end);
+ }
+ }
+ spin_lock(&fc->lock);
+ list_for_each_entry_safe(dmap, n, &to_remove, list) {
+ fuse_dax_do_free_mapping_locked(fc, dmap);
+ }
+ spin_unlock(&fc->lock);
+}
+
int fuse_dax_free_one_mapping_locked(struct fuse_conn *fc, struct inode *inode,
u64 dmap_start)
{
@@ -4003,15 +4090,9 @@ int fuse_dax_free_one_mapping_locked(struct fuse_conn *fc, struct inode *inode,
/* Cleanup dmap entry and add back to free list */
spin_lock(&fc->lock);
- __dmap_remove_busy_list(fc, dmap);
- dmap->inode = NULL;
- dmap->start = dmap->end = 0;
- __free_dax_mapping(fc, dmap);
+ fuse_dax_do_free_mapping_locked(fc, dmap);
spin_unlock(&fc->lock);
- pr_debug("fuse: freed memory range window_offset=0x%llx,"
- " length=0x%llx\n", dmap->window_offset,
- dmap->length);
return ret;
}
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 5042e227e8a8..0dccfff50f91 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -847,13 +847,20 @@ struct fuse_setupmapping_out {
uint64_t len[FUSE_SETUPMAPPING_ENTRIES];
};
-struct fuse_removemapping_in {
+struct fuse_removemapping_in_header {
/* An already open handle */
uint64_t fh;
+ /* number of fuse_removemapping_in follows */
+ uint32_t count;
+};
+
+struct fuse_removemapping_in {
/* Offset into the dax window start the unmapping */
uint64_t moffset;
/* Length of mapping required */
uint64_t len;
};
+#define FUSE_REMOVEMAPPING_MAX_ENTRY \
+ (PAGE_SIZE / sizeof(struct fuse_removemapping_in))
#endif /* _LINUX_FUSE_H */
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-06-06 6:51 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-05 3:02 [Virtio-fs] [PATCH-v3 0/1] virtiofs: FUSE_REMOVEMAPPING remove multiple entries in one call Peng Tao
2019-06-05 3:02 ` [Virtio-fs] [PATCH] " Peng Tao
2019-06-05 18:06 ` Liu Bo
2019-06-05 18:35 ` Vivek Goyal
2019-06-05 18:50 ` Liu Bo
2019-06-05 20:30 ` [Virtio-fs] [PATCH-v3 0/1] " Vivek Goyal
2019-06-06 6:51 ` Peng Tao
-- strict thread matches above, loose matches on Subject: below --
2019-05-29 4:30 [Virtio-fs] [PATCH] " Peng Tao
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.