Re: [PATCH 006/104] virtiofsd: Trim down imported files

[Dr. David Alan Gilbert]

* Xiao Yang (address@hidden) wrote:
> On 2019/12/13 0:37, Dr. David Alan Gilbert (git) wrote:
> > -   res = fuse_buf_copy(&pipe_buf, buf,
> > -                       FUSE_BUF_FORCE_SPLICE | FUSE_BUF_SPLICE_NONBLOCK);
> > -   if (res<  0) {
> > -           if (res == -EAGAIN || res == -EINVAL) {
> > -                   /*
> > -                    * Should only get EAGAIN on kernels with
> > -                    * broken SPLICE_F_NONBLOCK support (<=
> > -                    * 2.6.35) where this error or a short read is
> > -                    * returned even if the pipe itself is not
> > -                    * full
> > -                    *
> > -                    * EINVAL might mean that splice can't handle
> > -                    * this combination of input and output.
> > -                    */
> > -                   if (res == -EAGAIN)
> > -                           se->broken_splice_nonblock = 1;
> > -
> > -                   pthread_setspecific(se->pipe_key, NULL);
> > -                   fuse_ll_pipe_free(llp);
> > -                   goto fallback;
> > -           }
> > -           res = -res;
> > -           goto clear_pipe;
> > -   }
> > -
> > -   if (res != 0&&  res<  len) {
> > -           struct fuse_bufvec mem_buf = FUSE_BUFVEC_INIT(len);
> > -           void *mbuf;
> > -           size_t now_len = res;
> > -           /*
> > -            * For regular files a short count is either
> > -            *  1) due to EOF, or
> > -            *  2) because of broken SPLICE_F_NONBLOCK (see above)
> > -            *
> > -            * For other inputs it's possible that we overflowed
> > -            * the pipe because of small buffer fragments.
> > -            */
> > -
> > -           res = posix_memalign(&mbuf, pagesize, len);
> > -           if (res != 0)
> > -                   goto clear_pipe;
> > -
> > -           mem_buf.buf[0].mem = mbuf;
> > -           mem_buf.off = now_len;
> > -           res = fuse_buf_copy(&mem_buf, buf, 0);
> > -           if (res>  0) {
> > -                   char *tmpbuf;
> > -                   size_t extra_len = res;
> > -                   /*
> > -                    * Trickiest case: got more data.  Need to get
> > -                    * back the data from the pipe and then fall
> > -                    * back to regular write.
> > -                    */
> > -                   tmpbuf = malloc(headerlen);
> > -                   if (tmpbuf == NULL) {
> > -                           free(mbuf);
> > -                           res = ENOMEM;
> > -                           goto clear_pipe;
> > -                   }
> > -                   res = read_back(llp->pipe[0], tmpbuf, headerlen);
> > -                   free(tmpbuf);
> > -                   if (res != 0) {
> > -                           free(mbuf);
> > -                           goto clear_pipe;
> > -                   }
> > -                   res = read_back(llp->pipe[0], mbuf, now_len);
> > -                   if (res != 0) {
> > -                           free(mbuf);
> > -                           goto clear_pipe;
> > -                   }
> > -                   len = now_len + extra_len;
> > -                   iov[iov_count].iov_base = mbuf;
> > -                   iov[iov_count].iov_len = len;
> > -                   iov_count++;
> > -                   res = fuse_send_msg(se, ch, iov, iov_count);
> > -                   free(mbuf);
> > -                   return res;
> > -           }
> > -           free(mbuf);
> > -           res = now_len;
> > -   }
> > -   len = res;
> > -   out->len = headerlen + len;
> > -
> > -   if (se->debug) {
> > -           fuse_log(FUSE_LOG_DEBUG,
> > -                   "   unique: %llu, success, outsize: %i (splice)\n",
> > -                   (unsigned long long) out->unique, out->len);
> > -   }
> > -
> > -   splice_flags = 0;
> > -   if ((flags&  FUSE_BUF_SPLICE_MOVE)&&
> > -       (se->conn.want&  FUSE_CAP_SPLICE_MOVE))
> > -           splice_flags |= SPLICE_F_MOVE;
> > -
> > -   res = splice(llp->pipe[0], NULL, ch ? ch->fd : se->fd,
> > -                NULL, out->len, splice_flags);
> Hi,
> 
> 1) In buffer.c, fuse_buf_splice() uses splice(2) to copy/move data in some
> cases if the syscall is supported.
> 2) One pipe needs to be passed to splice(2) so splice(2) without one pipe
> fails and then go back to use other ways(e.g. use fuse_buf_fd_to_fd()).
> 3) fuse_buf_copy() calls fuse_buf_splice() indirectly and this patch has
> removed all pipes used by fuse_buf_copy().
> 
> Is it necessary to leave the code related to splice(2)?  Is it going to be
> used in future?
> We have to use splice(2) by the correct CONFIG_SPLICE macro If necessary.

Yes, I think we never set HAVE_SPLICE; so that code can go.
I'll change this patch to remove that as well.

Dave

> Best Regards,
> Xiao Yang
> 
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK