On Mon, Jun 03, 2019 at 10:46:51AM +0800, Peng Tao wrote:
[..]
> +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);
Hmm..... So normally dmap->list is used for putting an element in free list.
Given this element is on busy list, it must not be on free list hence you
are reusing dmap->list to put it temporarily on to_remove list.
> + }
> +
> + /* 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);
What removes dmap from to_remove list. I think it gets added to free
list and that overrides it? So an element is already part of to_remove
list and then we are calling list_add_tail(&dmap->list, &fc->free_ranges)
on it without removing it. That sounds wrong.
Vivek
