Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 31 May 2023 16:03:44 -0400
From:   Vivek Goyal <vgoyal@redhat.com>
To:     Jingbo Xu <jefflexu@linux.alibaba.com>
Cc:     miklos@szeredi.hu, linux-fsdevel@vger.kernel.org,
        gerry@linux.alibaba.com, linux-kernel@vger.kernel.org,
        German Maglione <gmaglione@redhat.com>
Subject: Re: [PATCH] fuse: fix return value of inode_inline_reclaim_one_dmap
 in error path
Message-ID: <ZHeoIFrp303f0E8d@redhat.com>
References: <20230424123250.125404-1-jefflexu@linux.alibaba.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230424123250.125404-1-jefflexu@linux.alibaba.com>
Precedence: bulk

On Mon, Apr 24, 2023 at 08:32:50PM +0800, Jingbo Xu wrote:
> When range already got reclaimed by somebody else, return NULL so that
> the caller could retry to allocate or reclaim another range, instead of
> mistakenly returning the range already got reclaimed and reused by
> others.
> 
> Reported-by: Liu Jiang <gerry@linux.alibaba.com>
> Fixes: 9a752d18c85a ("virtiofs: add logic to free up a memory range")
> Signed-off-by: Jingbo Xu <jefflexu@linux.alibaba.com>

Hi Jingbo,

This patch looks correct to me.

Are you able to reproduce the problem? Or you are fixing it based on
code inspection?

How are you testing this? We don't have virtiofsd DAX implementation yet
in rust virtiofsd yet. 

I am not sure how to test this chagne now. We had out of tree patches
in qemu and now qemu has gotten rid of C version of virtiofsd so these
patches might not even work now.

Thanks
Vivek
> ---
>  fs/fuse/dax.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/fuse/dax.c b/fs/fuse/dax.c
> index 8e74f278a3f6..59aadfd89ee5 100644
> --- a/fs/fuse/dax.c
> +++ b/fs/fuse/dax.c
> @@ -985,6 +985,7 @@ inode_inline_reclaim_one_dmap(struct fuse_conn_dax *fcd, struct inode *inode,
>  	node = interval_tree_iter_first(&fi->dax->tree, start_idx, start_idx);
>  	/* Range already got reclaimed by somebody else */
>  	if (!node) {
> +		dmap = NULL;
>  		if (retry)
>  			*retry = true;
>  		goto out_write_dmap_sem;
> -- 
> 2.19.1.6.gb485710b
>