[LTP] [PATCH v3 1/2] syscalls/splice04: add test for splice() from pipe to pipe

[Jan Stancek]

----- Original Message -----
> From: Boyang Xue <bxue@redhat.com>
> 
> This test case covers pipe to pipe splice operation, which was introduced
> in kernel commit 7c77f0b3f920 ("splice: implement pipe to pipe splicing").
> 
> Signed-off-by: Boyang Xue <bxue@redhat.com>

Hi,

Many lines go over 80 characters.

> +static void setup(void)
> +{
> +	int i, fd, pipe_max_unpriv, pipe_limit;
> +	FILE *s;
> +
> +	fd = SAFE_OPEN("/proc/sys/fs/pipe-max-size", O_RDONLY);
> +	s = fdopen(fd, "r");
> +	if (s == NULL)
> +		tst_brk(TBROK, "fdopen() error");
> +	if (fscanf(s, "%d", &pipe_max_unpriv) != 1)
> +		tst_brk(TBROK, "Fail to get value of /proc/sys/fs/pipe-max-size");
> +	else {
> +		pipe_limit = pipe_max_unpriv < num_len_data ? pipe_max_unpriv :
> num_len_data;
> +		num_len_data = pipe_limit;
> +	}
> +	if (fclose(s) == EOF)
> +		tst_brk(TBROK, "Fail to close file");

You can use helper macro: SAFE_FILE_SCANF

> +
> +	if (tst_parse_int(str_len_data, &num_len_data, 1, pipe_limit))
> +		tst_brk(TBROK, "Invalid length of data: '%s', valid value: [1, %d]",
> str_len_data, pipe_limit);
> +	tst_res(TINFO, "splice size = %d", num_len_data);
> +	arr_in = SAFE_MALLOC(num_len_data);
> +	arr_out = SAFE_MALLOC(num_len_data);
> +	for (i = 0; i < num_len_data; i++)
> +		arr_in[i] = i & 0xff;
> +}
...
> +static void pipe_pipe(void)
> +{
> +	int pp1[2], pp2[2], i, ret;
> +
> +	SAFE_PIPE(pp1);
> +	SAFE_PIPE(pp2);
> +	SAFE_WRITE(1, pp1[1], arr_in, num_len_data);
> +	do {
> +		ret = splice(pp1[0], NULL, pp2[1], NULL, num_len_data, SPLICE_F_MOVE);

You always try to splice num_len_data bytes, which can lead to hang
if this loop runs multiple times, because there aren't enough bytes
in pipe after first splice.

For example, on 4.10:
[pid  2987] 11:55:49.090001 splice(3, NULL, 6, NULL, 65536, SPLICE_F_MOVE) = 32768
[pid  2987] 11:55:49.090073 splice(3, NULL, 6, NULL, 65536, SPLICE_F_MOVE<hangs>

We have 65536 bytes in pipe, first splice transfered half of that,
second splice is again trying to transfer 65536 bytes.

> +		if (ret == 0)
> +			break;

from man2 splice:
"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 trans‐
fer, and it would not make sense to block because there are no writers connected to the write end of the pipe."

Since you still have write end opened it can block rather than return 0.
Also, it makes sense only if fd_in is pipe, which is not the case of
splice05 testcase.

> +		else if (ret == -1) {
> +			tst_res(TFAIL | TERRNO, "splice error");
> +			goto exit;
> +		}
> +	} while (num_len_data - ret);

Unless first splice transfers all, this condition is always true.

Regards,
Jan