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@xxxxxxxxxx>
> > > Link: https://www.mail-archive.com/linux-kernel@xxxxxxxxxxxxxxx/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
