Re: [PATCH] block: allow for_each_bvec to support zero len bvec

[Ming Lei <ming.lei@redhat.com>]

On Tue, Aug 11, 2020 at 12:23:31AM +0800, Ming Lei wrote:
> On Mon, Aug 10, 2020 at 04:52:17PM +0900, Tetsuo Handa wrote:
> > On 2020/08/10 12:19, Ming Lei wrote:
> > > Block layer usually doesn't support or allow zero-length bvec. Since
> > > commit 1bdc76aea115 ("iov_iter: use bvec iterator to implement
> > > iterate_bvec()"), iterate_bvec() switches to bvec iterator. However,
> > > Al mentioned that 'Zero-length segments are not disallowed' in iov_iter.
> > > 
> > > Fixes for_each_bvec() so that it can move on after seeing one zero
> > > length bvec.
> > > 
> > > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > > Link: https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2262077.html
> > > Fixes: 1bdc76aea115 ("iov_iter: use bvec iterator to implement iterate_bvec()")
> > 
> > Is this Fixes: correct? That commit should be in RHEL8's 4.18 kernel but that kernel
> > does not hit this bug.
> 
> Yeah, it is correct, see the following link:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.8&id=1bdc76aea1159a750846c2fc98e404403eb7d51c
> 
> Commit 1bdc76aea115 was merged to v4.8, so it is definitely in both RHEL8's
> 4.18 based kernel and upstream kernel.
> 
> > 
> > Moreover, maybe nobody cares, but behavior of splice() differs when there are only 
> > zero-length pages. With this fix, splice() returns 0 despite there is still pipe writers.
> 
> It is another new issue, which isn't related with Commit 1bdc76aea115,
> see below.
> 
> > Man page seems to say that splice() returns 0 when there is no pipe writers...
> > 
> >     A return value of 0 means end of input.  If fd_in refers to a pipe,
> >     then this means that there was no data to transfer, and it would not
> >     make sense to block because there are no writers connected to the
> >     write end of the pipe.
> > 
> > ----- test case -----
> > #define _GNU_SOURCE
> > #include <stdio.h>
> > #include <sys/types.h>
> > #include <sys/stat.h>
> > #include <fcntl.h>
> > #include <unistd.h>
> > #include <string.h>
> > 
> > int main(int argc, char *argv[])
> > {
> >         static char buffer[4096];
> >         const int fd = open("/tmp/testfile", O_WRONLY | O_CREAT, 0600);
> >         int pipe_fd[2] = { EOF, EOF };
> >         pipe(pipe_fd);
> >         write(pipe_fd[1], NULL, sizeof(buffer));
> >         write(pipe_fd[1], NULL, sizeof(buffer));
> >         memset(buffer, 'a', sizeof(buffer));
> >         //write(pipe_fd[1], buffer, sizeof(buffer));
> >         write(pipe_fd[1], NULL, sizeof(buffer));
> >         write(pipe_fd[1], NULL, sizeof(buffer));
> >         memset(buffer, 'b', sizeof(buffer));
> >         //write(pipe_fd[1], buffer, sizeof(buffer));
> >         write(pipe_fd[1], NULL, sizeof(buffer));
> >         write(pipe_fd[1], NULL, sizeof(buffer));
> >         memset(buffer, 'c', sizeof(buffer));
> >         //write(pipe_fd[1], buffer, sizeof(buffer));
> >         write(pipe_fd[1], NULL, sizeof(buffer));
> >         write(pipe_fd[1], NULL, sizeof(buffer));
> >         memset(buffer, 'd', sizeof(buffer));
> >         //write(pipe_fd[1], buffer, sizeof(buffer));
> >         write(pipe_fd[1], NULL, sizeof(buffer));
> >         write(pipe_fd[1], NULL, sizeof(buffer));
> >         splice(pipe_fd[0], NULL, fd, NULL, 65536, 0);
> >         return 0;
> > }
> 
> The above test doesn't trigger the reported lockup issue, so this patch
> isn't related with the new issue you described.

BTW, for_each_bvec won't be called in the above splice test code.


Thanks,
Ming