Re: [PATCH RFC v2 11/19] fuse: Add support to copy from/to the ring buffer

[Bernd Schubert <bernd.schubert@fastmail.fm>]

On 5/30/24 21:59, Josef Bacik wrote:
> On Wed, May 29, 2024 at 08:00:46PM +0200, Bernd Schubert wrote:
>> This adds support to existing fuse copy code to copy
>> from/to the ring buffer. The ring buffer is here mmaped
>> shared between kernel and userspace.
>>
>> This also fuse_ prefixes the copy_out_args function
>>
>> Signed-off-by: Bernd Schubert <bschubert@ddn.com>
>> ---
>>  fs/fuse/dev.c        | 60 ++++++++++++++++++++++++++++++----------------------
>>  fs/fuse/fuse_dev_i.h | 38 +++++++++++++++++++++++++++++++++
>>  2 files changed, 73 insertions(+), 25 deletions(-)
>>
>> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
>> index 05a87731b5c3..a7d26440de39 100644
>> --- a/fs/fuse/dev.c
>> +++ b/fs/fuse/dev.c
>> @@ -637,21 +637,7 @@ static int unlock_request(struct fuse_req *req)
>>  	return err;
>>  }
>>  
>> -struct fuse_copy_state {
>> -	int write;
>> -	struct fuse_req *req;
>> -	struct iov_iter *iter;
>> -	struct pipe_buffer *pipebufs;
>> -	struct pipe_buffer *currbuf;
>> -	struct pipe_inode_info *pipe;
>> -	unsigned long nr_segs;
>> -	struct page *pg;
>> -	unsigned len;
>> -	unsigned offset;
>> -	unsigned move_pages:1;
>> -};
>> -
>> -static void fuse_copy_init(struct fuse_copy_state *cs, int write,
>> +void fuse_copy_init(struct fuse_copy_state *cs, int write,
>>  			   struct iov_iter *iter)
>>  {
>>  	memset(cs, 0, sizeof(*cs));
>> @@ -662,6 +648,7 @@ static void fuse_copy_init(struct fuse_copy_state *cs, int write,
>>  /* Unmap and put previous page of userspace buffer */
>>  static void fuse_copy_finish(struct fuse_copy_state *cs)
>>  {
>> +
> 
> Extraneous newline.
> 
>>  	if (cs->currbuf) {
>>  		struct pipe_buffer *buf = cs->currbuf;
>>  
>> @@ -726,6 +713,10 @@ static int fuse_copy_fill(struct fuse_copy_state *cs)
>>  			cs->pipebufs++;
>>  			cs->nr_segs++;
>>  		}
>> +	} else if (cs->is_uring) {
>> +		if (cs->ring.offset > cs->ring.buf_sz)
>> +			return -ERANGE;
>> +		cs->len = cs->ring.buf_sz - cs->ring.offset;
>>  	} else {
>>  		size_t off;
>>  		err = iov_iter_get_pages2(cs->iter, &page, PAGE_SIZE, 1, &off);
>> @@ -744,21 +735,35 @@ static int fuse_copy_fill(struct fuse_copy_state *cs)
>>  static int fuse_copy_do(struct fuse_copy_state *cs, void **val, unsigned *size)
>>  {
>>  	unsigned ncpy = min(*size, cs->len);
>> +
>>  	if (val) {
>> -		void *pgaddr = kmap_local_page(cs->pg);
>> -		void *buf = pgaddr + cs->offset;
>> +
>> +		void *pgaddr = NULL;
>> +		void *buf;
>> +
>> +		if (cs->is_uring) {
>> +			buf = cs->ring.buf + cs->ring.offset;
>> +			cs->ring.offset += ncpy;
>> +
>> +		} else {
>> +			pgaddr = kmap_local_page(cs->pg);
>> +			buf = pgaddr + cs->offset;
>> +		}
>>  
>>  		if (cs->write)
>>  			memcpy(buf, *val, ncpy);
>>  		else
>>  			memcpy(*val, buf, ncpy);
>>  
>> -		kunmap_local(pgaddr);
>> +		if (pgaddr)
>> +			kunmap_local(pgaddr);
>> +
>>  		*val += ncpy;
>>  	}
>>  	*size -= ncpy;
>>  	cs->len -= ncpy;
>>  	cs->offset += ncpy;
>> +
> 
> Extraneous newline.
> 
> Once those nits are fixed you can add
> 
> Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks again for your reviews! I won't add this for now, as there are
too many changes after removing mmap.


Thanks,
Bernd