[Virtio-fs] [PATCH v2 2/2][RFC] use fuse_buf_writev to replace fuse_buf_write for better performance

[piaojun]

Hi Dave,

On 2019/8/7 23:37, Dr. David Alan Gilbert wrote:
> * piaojun (piaojun at huawei.com) wrote:
>> fuse_buf_writev() only handles the normal write in which src is buffer
>> and dest is fd. Specially if src buffer represents guest physical
>> address that can't be mapped by the daemon process, IO must be bounced
>> back to the VMM to do it by fuse_buf_copy().
>>
>> Signed-off-by: Jun Piao <piaojun at huawei.com>
>> Suggested-by: Dr. David Alan Gilbert <dgilbert at redhat.com>
>> ---
>>  contrib/virtiofsd/passthrough_ll.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/contrib/virtiofsd/passthrough_ll.c b/contrib/virtiofsd/passthrough_ll.c
>> index cc9c175..c1bbc53 100644
>> --- a/contrib/virtiofsd/passthrough_ll.c
>> +++ b/contrib/virtiofsd/passthrough_ll.c
>> @@ -2023,7 +2023,10 @@ static void lo_write_buf(fuse_req_t req, fuse_ino_t ino,
>>  		fuse_debug("lo_write_buf(ino=%" PRIu64 ", size=%zd, off=%lu)\n",
>>  			   ino, out_buf.buf[0].size, (unsigned long) off);
>>
>> -	res = fuse_buf_copy(req, &out_buf, in_buf, 0);
>> +	if (!(in_buf->buf[0].flags & FUSE_BUF_PHYS_ADDR))
> 
> I don't think you can assume that the flags in in_buf[0] represent the
> state of the entire vector; I'm pretty sure we had a case of an
> application that did a writev() in the guest where the first element was
> on the stack and the other was in an mmap, and it was only the 2nd one
> that had the flag.

Yes, this assumption is idealized, and perhaps I need check all the
flags of entire vector before doing writev, right?

Thanks,
Jun

> 
> But also you're not checking any of the other flags either.
> 
> Dave
> 
>> +		res = fuse_buf_writev(req, &out_buf, in_buf, out_buf.buf[0].flags);
>> +	else
>> +		res = fuse_buf_copy(req, &out_buf, in_buf, 0);
>>  	if(res < 0) {
>>  		fuse_reply_err(req, -res);
>>  	} else {
>> -- 
> --
> Dr. David Alan Gilbert / dgilbert at redhat.com / Manchester, UK
> .
>