[LTP] [PATCH 1/1] aio-stress.c: Use static variable iterations

[LTP] [PATCH 1/1] aio-stress.c: Use static variable iterations

[Petr Vorel]

local iteration variable in worker() was probably left over from
previous versions rewrite (maybe -i was not used in previous versions).

Clang correctly reported:
aio-stress.c:1049:6: warning: variable 'iteration' set but not used [-Wunused-but-set-variable]
int iteration = 0;

Fixes: 054d45390 ("Rewrite aio-stress test using LTP API")

Reported-by: Richard Palethorpe <rpalethorpe@suse.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Hi,

reported by Richie [1], I got confused thus merge before this fix.

[1] https://lore.kernel.org/ltp/87h6yfifbr.fsf@suse.de/

 testcases/kernel/io/ltp-aiodio/aio-stress.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/testcases/kernel/io/ltp-aiodio/aio-stress.c b/testcases/kernel/io/ltp-aiodio/aio-stress.c
index 39db14d65..29e679087 100644
--- a/testcases/kernel/io/ltp-aiodio/aio-stress.c
+++ b/testcases/kernel/io/ltp-aiodio/aio-stress.c
@@ -1046,7 +1046,6 @@ static int *worker(struct thread_info *t)
 	char *this_stage = NULL;
 	struct timeval stage_time;
 	int status = 0;
-	int iteration = 0;
 	int cnt;
 
 	aio_setup(&t->io_ctx, 512);
@@ -1151,7 +1150,7 @@ restart:
 
 	/* someone got restarted, go back to the beginning */
 	if (t->active_opers && cnt < iterations) {
-		iteration++;
+		iterations++;
 		goto restart;
 	}
 
-- 
2.38.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] aio-stress.c: Use static variable iterations

[Richard Palethorpe]

Hello,

Petr Vorel <pvorel@suse.cz> writes:

> local iteration variable in worker() was probably left over from
> previous versions rewrite (maybe -i was not used in previous versions).
>
> Clang correctly reported:
> aio-stress.c:1049:6: warning: variable 'iteration' set but not used [-Wunused-but-set-variable]
> int iteration = 0;
>
> Fixes: 054d45390 ("Rewrite aio-stress test using LTP API")
>
> Reported-by: Richard Palethorpe <rpalethorpe@suse.com>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Hi,
>
> reported by Richie [1], I got confused thus merge before this fix.
>
> [1] https://lore.kernel.org/ltp/87h6yfifbr.fsf@suse.de/
>
>  testcases/kernel/io/ltp-aiodio/aio-stress.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/testcases/kernel/io/ltp-aiodio/aio-stress.c b/testcases/kernel/io/ltp-aiodio/aio-stress.c
> index 39db14d65..29e679087 100644
> --- a/testcases/kernel/io/ltp-aiodio/aio-stress.c
> +++ b/testcases/kernel/io/ltp-aiodio/aio-stress.c
> @@ -1046,7 +1046,6 @@ static int *worker(struct thread_info *t)
>  	char *this_stage = NULL;
>  	struct timeval stage_time;
>  	int status = 0;
> -	int iteration = 0;
>  	int cnt;
>  
>  	aio_setup(&t->io_ctx, 512);
> @@ -1151,7 +1150,7 @@ restart:
>  
>  	/* someone got restarted, go back to the beginning */
>  	if (t->active_opers && cnt < iterations) {
> -		iteration++;
> +		iterations++;

Why is this correct?

>  		goto restart;
>  	}


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] aio-stress.c: Use static variable iterations

[Petr Vorel]

Hi Richie,

> Hello,

> Petr Vorel <pvorel@suse.cz> writes:

> > local iteration variable in worker() was probably left over from
> > previous versions rewrite (maybe -i was not used in previous versions).

> > Clang correctly reported:
> > aio-stress.c:1049:6: warning: variable 'iteration' set but not used [-Wunused-but-set-variable]
> > int iteration = 0;

> > Fixes: 054d45390 ("Rewrite aio-stress test using LTP API")

> > Reported-by: Richard Palethorpe <rpalethorpe@suse.com>
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> > Hi,

> > reported by Richie [1], I got confused thus merge before this fix.

> > [1] https://lore.kernel.org/ltp/87h6yfifbr.fsf@suse.de/

> >  testcases/kernel/io/ltp-aiodio/aio-stress.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)

> > diff --git a/testcases/kernel/io/ltp-aiodio/aio-stress.c b/testcases/kernel/io/ltp-aiodio/aio-stress.c
> > index 39db14d65..29e679087 100644
> > --- a/testcases/kernel/io/ltp-aiodio/aio-stress.c
> > +++ b/testcases/kernel/io/ltp-aiodio/aio-stress.c
> > @@ -1046,7 +1046,6 @@ static int *worker(struct thread_info *t)
> >  	char *this_stage = NULL;
> >  	struct timeval stage_time;
> >  	int status = 0;
> > -	int iteration = 0;
> >  	int cnt;

> >  	aio_setup(&t->io_ctx, 512);
> > @@ -1151,7 +1150,7 @@ restart:

> >  	/* someone got restarted, go back to the beginning */
> >  	if (t->active_opers && cnt < iterations) {
> > -		iteration++;
> > +		iterations++;

> Why is this correct?
OK, this is obviously wrong, we don't want to increase global iterations (-i).
iteration appeared in v6. In v8, the default changed to 500 and RUN_FOREVER -1
was not used any more.
The warning was on legacy version as well, IMHO code is dead and whole restart
label and iteration variable should go away. I'll send v2.
Thanks for catching my obvious error.

Kind regards,
Petr

> >  		goto restart;
> >  	}

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/1] aio-stress.c: Use static variable iterations

[Petr Vorel]

> Hi Richie,

> > Hello,

> > Petr Vorel <pvorel@suse.cz> writes:

> > > local iteration variable in worker() was probably left over from
> > > previous versions rewrite (maybe -i was not used in previous versions).

> > > Clang correctly reported:
> > > aio-stress.c:1049:6: warning: variable 'iteration' set but not used [-Wunused-but-set-variable]
> > > int iteration = 0;

> > > Fixes: 054d45390 ("Rewrite aio-stress test using LTP API")

> > > Reported-by: Richard Palethorpe <rpalethorpe@suse.com>
> > > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > > ---
> > > Hi,

> > > reported by Richie [1], I got confused thus merge before this fix.

> > > [1] https://lore.kernel.org/ltp/87h6yfifbr.fsf@suse.de/

> > >  testcases/kernel/io/ltp-aiodio/aio-stress.c | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)

> > > diff --git a/testcases/kernel/io/ltp-aiodio/aio-stress.c b/testcases/kernel/io/ltp-aiodio/aio-stress.c
> > > index 39db14d65..29e679087 100644
> > > --- a/testcases/kernel/io/ltp-aiodio/aio-stress.c
> > > +++ b/testcases/kernel/io/ltp-aiodio/aio-stress.c
> > > @@ -1046,7 +1046,6 @@ static int *worker(struct thread_info *t)
> > >  	char *this_stage = NULL;
> > >  	struct timeval stage_time;
> > >  	int status = 0;
> > > -	int iteration = 0;
> > >  	int cnt;

> > >  	aio_setup(&t->io_ctx, 512);
> > > @@ -1151,7 +1150,7 @@ restart:

> > >  	/* someone got restarted, go back to the beginning */
> > >  	if (t->active_opers && cnt < iterations) {
> > > -		iteration++;
> > > +		iterations++;

> > Why is this correct?
> OK, this is obviously wrong, we don't want to increase global iterations (-i).
> iteration appeared in v6. In v8, the default changed to 500 and RUN_FOREVER -1
> was not used any more.
> The warning was on legacy version as well, IMHO code is dead and whole restart
> label and iteration variable should go away. I'll send v2.
> Thanks for catching my obvious error.

Actually, thinking about it twice, I'm not sure if restart should be really
handled and thus only iteration variable should be removed (but restart label
kept) or it's so unlikely that it happen so that also restart label should be
deleted.

Kind regards,
Petr

> Kind regards,
> Petr

> > >  		goto restart;
> > >  	}

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp