[Virtio-fs] [PATCH 1/2] virtio-fs: send setupmapping with write flag only if IOMAP_WRITE

[Liu Bo]

On Tue, May 07, 2019 at 02:29:43PM -0400, Vivek Goyal wrote:
> On Tue, May 07, 2019 at 01:58:10PM +0800, Liu Bo wrote:
> > This is to let virtiofs daemon to do inline fallocate to preallocate
> > space for this mapping.
> > 
> > Signed-off-by: Liu Bo <bo.liu at linux.alibaba.com>
> > ---
> >  fs/fuse/file.c | 21 ++++++++++++++-------
> >  1 file changed, 14 insertions(+), 7 deletions(-)
> > 
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index 488e932f..6f403c8 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -251,8 +251,8 @@ static void free_dax_mapping(struct fuse_conn *fc,
> >  
> >  /* offset passed in should be aligned to FUSE_DAX_MEM_RANGE_SZ */
> >  static int fuse_setup_one_mapping(struct inode *inode,
> > -				struct file *file, loff_t offset,
> > -				struct fuse_dax_mapping *dmap)
> > +				  struct file *file, loff_t offset, unsigned flags,
> > +				  struct fuse_dax_mapping *dmap)
> >  {
> >  	struct fuse_conn *fc = get_fuse_conn(inode);
> >  	struct fuse_inode *fi = get_fuse_inode(inode);
> > @@ -276,14 +276,21 @@ static int fuse_setup_one_mapping(struct inode *inode,
> >  		inarg.fh = -1;
> >  	inarg.moffset = dmap->window_offset;
> >  	inarg.len = FUSE_DAX_MEM_RANGE_SZ;
> > +
> > +       /*
> > +	* with IOMAP_WRITE, virtiofs daemon will make sure space is
> > +	* allocated for the range.
> > +	*/
> >  	if (file) {
> > -		inarg.flags |= (file->f_mode & FMODE_WRITE) ?
> > -				FUSE_SETUPMAPPING_FLAG_WRITE : 0;
> > +		if (file->f_mode & FMODE_WRITE &&
> > +		    flags & IOMAP_WRITE)
> > +			inarg.flags |= FUSE_SETUPMAPPING_FLAG_WRITE;
> >  		inarg.flags |= (file->f_mode & FMODE_READ) ?
> >  				FUSE_SETUPMAPPING_FLAG_READ : 0;
> >  	} else {
> >  		inarg.flags |= FUSE_SETUPMAPPING_FLAG_READ;
> > -		inarg.flags |= FUSE_SETUPMAPPING_FLAG_WRITE;
> > +		if (flags & IOMAP_WRITE)
> > +			inarg.flags |= FUSE_SETUPMAPPING_FLAG_WRITE;
> 
> Not sure we can afford to map a mapping read only. We don't have a
> mechanism yet to upgrade a mapping. IOW, what if one process opens
> file read only and does some read. This will lead to a mapping setup
> with FUSE_SETUPMAPPING_FLAG_READ which will be cached. Now another process
> might open file read/write and try to write to same mapping without
> having to go through setupmapping path again and will run into this
> -ENOSPC again.

You're right, patch 2 provides a "mkwrite" helper to map the mapping RW.

thanks,
-liubo

> 
> Vivek
> 
> >  	}
> >  	args.in.h.opcode = FUSE_SETUPMAPPING;
> >  	args.in.h.nodeid = fi->nodeid;
> > @@ -1847,8 +1854,8 @@ static int fuse_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
> >  
> >  		/* Setup one mapping */
> >  		ret = fuse_setup_one_mapping(inode, NULL,
> > -				ALIGN_DOWN(pos, FUSE_DAX_MEM_RANGE_SZ),
> > -				alloc_dmap);
> > +					ALIGN_DOWN(pos, FUSE_DAX_MEM_RANGE_SZ),
> > +					     flags, alloc_dmap);
> >  		if (ret < 0) {
> >  			printk("fuse_setup_one_mapping() failed. err=%d"
> >  				" pos=0x%llx\n", ret, pos);
> > -- 
> > 1.8.3.1
> > 
> > _______________________________________________
> > Virtio-fs mailing list
> > Virtio-fs at redhat.com
> > https://www.redhat.com/mailman/listinfo/virtio-fs