On Sat, Feb 16, 2019 at 03:25:40PM +0800, Zorro Lang wrote:
> copyrange_f and splice_f functions use a while loop to read a file,
> it's fine if there's only one fsstress process(and its children),
> but if some third part testing processes remove the file in the
> middle phase of copyrange_f running, copyrange_f maybe always return
> 0, and the while loop can't be end. As below:
>
> root 47184 xxxxxx S+ ./fsstress -R -d /mnt/scratch -n 10000 -p 20 -v
> root 47187 xxxxxx R+ ./fsstress -d /mnt/scratch -n 10000 -p 20 -v
> root 47199 xxxxxx R+ ./fsstress -d /mnt/scratch -n 10000 -p 20 -v
> root 47314 xxxxxx S+ grep --color=auto fsstress
> ...
> ...
> copy_file_range(3, [372258], 4, [2658770], 71179, 0) = 0
> copy_file_range(3, [372258], 4, [2658770], 71179, 0) = 0
> copy_file_range(3, [372258], 4, [2658770], 71179, 0) = 0
> copy_file_range(3, [372258], 4, [2658770], 71179, 0) = 0
> ...
> ...
> lr-x------. 1 root root 64 Jan 28 11:34 /proc/47187/fd/3 -> '/mnt/scratch/p2/f2 (deleted)'
>
> Signed-off-by: Zorro Lang <zlang@xxxxxxxxxx>
> ---
>
> V2 remove 300 times loop, just return if get ret == 0
>
> Thanks,
> Zorro
>
> ltp/fsstress.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/ltp/fsstress.c b/ltp/fsstress.c
> index 25e0c3e2..138f7ecf 100644
> --- a/ltp/fsstress.c
> +++ b/ltp/fsstress.c
> @@ -2363,7 +2363,7 @@ copyrange_f(
> int v2;
> int fd1;
> int fd2;
> - size_t ret;
> + size_t ret = 0;
> int e;
What's this fix?
>
> /* Load paths */
> @@ -2452,7 +2452,7 @@ copyrange_f(
> if (ret < 0) {
> if (errno != EAGAIN || tries++ >= 300)
> break;
> - } else if (ret > len)
> + } else if (ret > len || ret == 0)
Shoul dbe changing the first line, not this one.
- if (ret < 0) {
+ if (ret <= 0) {
> break;
> else if (ret > 0)
> len -= ret;
> @@ -2908,6 +2908,9 @@ splice_f(int opno, long r)
>
> len -= ret1;
> total += ret1;
> + if (ret1 == 0) {
> + break;
> + }
Same thing as above. The checks after the splice() calls should
be changed from "if (ret < 0)" to "if (ret <= 0)". And there's two
splice calls with different error return values that need to be
checked, not one. And there's two checks for ret2, not one. :P
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
