[LTP] [PATCH 1/2] ltp-aiodio/dio_sparse: Fix write in dio_sparse()

[LTP] [PATCH 1/2] ltp-aiodio/dio_sparse: Fix write in dio_sparse()

[Guangwen Feng]

Current code looks buggy because when the offset is greater than
or equal to the filesize, it will never happen to do the write
in the loop, as a result, ADSP080..ADSP087 do not work actually.
Fix it by making sure that we do write writesize bytes.

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 testcases/kernel/io/ltp-aiodio/dio_sparse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/io/ltp-aiodio/dio_sparse.c b/testcases/kernel/io/ltp-aiodio/dio_sparse.c
index 41b9929..3828ed7 100644
--- a/testcases/kernel/io/ltp-aiodio/dio_sparse.c
+++ b/testcases/kernel/io/ltp-aiodio/dio_sparse.c
@@ -78,7 +78,7 @@ int dio_sparse(char *filename, int align, int writesize, int filesize, int offse
 
 	memset(bufptr, 0, writesize);
 	lseek(fd, offset, SEEK_SET);
-	for (i = offset; i < filesize;) {
+	for (i = 0; i < filesize;) {
 		if ((w = write(fd, bufptr, writesize)) != writesize) {
 			tst_resm(TBROK | TERRNO, "write() returned %d", w);
 			close(fd);
-- 
1.8.4.2

[LTP] [PATCH 2/2] ltp-aiodio/dio_sparse: Fix usleep in read_sparse()

[Guangwen Feng]

usleep(100000) sometimes leads to child process being too late
to do the read before being killed by parent process, tune it
to usleep(10) to make sure we do the real test in time.

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 testcases/kernel/io/ltp-aiodio/common_sparse.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/io/ltp-aiodio/common_sparse.h b/testcases/kernel/io/ltp-aiodio/common_sparse.h
index f7f4ef4..a7f5035 100644
--- a/testcases/kernel/io/ltp-aiodio/common_sparse.h
+++ b/testcases/kernel/io/ltp-aiodio/common_sparse.h
@@ -123,7 +123,7 @@ static void read_sparse(char *filename, int filesize)
 			fprintf(stderr, "Child %i waits for '%s' to appear\n",
 			        getpid(), filename);
 
-		usleep(100000);
+		usleep(10);
 	}
 
 	if (fd == -1) {
-- 
1.8.4.2

[LTP] [PATCH 1/2] ltp-aiodio/dio_sparse: Fix write in dio_sparse()

[Cyril Hrubis]

Hi!
> Current code looks buggy because when the offset is greater than
> or equal to the filesize, it will never happen to do the write
> in the loop, as a result, ADSP080..ADSP087 do not work actually.
> Fix it by making sure that we do write writesize bytes.
> 
> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
> ---
>  testcases/kernel/io/ltp-aiodio/dio_sparse.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/testcases/kernel/io/ltp-aiodio/dio_sparse.c b/testcases/kernel/io/ltp-aiodio/dio_sparse.c
> index 41b9929..3828ed7 100644
> --- a/testcases/kernel/io/ltp-aiodio/dio_sparse.c
> +++ b/testcases/kernel/io/ltp-aiodio/dio_sparse.c
> @@ -78,7 +78,7 @@ int dio_sparse(char *filename, int align, int writesize, int filesize, int offse
>  
>  	memset(bufptr, 0, writesize);
>  	lseek(fd, offset, SEEK_SET);
> -	for (i = offset; i < filesize;) {
> +	for (i = 0; i < filesize;) {
>  		if ((w = write(fd, bufptr, writesize)) != writesize) {
>  			tst_resm(TBROK | TERRNO, "write() returned %d", w);
>  			close(fd);

Hmm, it looks to me like the code actually does what it should have.
Since we pass a filesize and offset so the test should actually write
filesize - offset bytes. As far as I can tell the bug here is that the
test is not checking that filesize > offset before it starts.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 2/2] ltp-aiodio/dio_sparse: Fix usleep in read_sparse()

[Cyril Hrubis]

Hi!
> diff --git a/testcases/kernel/io/ltp-aiodio/common_sparse.h b/testcases/kernel/io/ltp-aiodio/common_sparse.h
> index f7f4ef4..a7f5035 100644
> --- a/testcases/kernel/io/ltp-aiodio/common_sparse.h
> +++ b/testcases/kernel/io/ltp-aiodio/common_sparse.h
> @@ -123,7 +123,7 @@ static void read_sparse(char *filename, int filesize)
>  			fprintf(stderr, "Child %i waits for '%s' to appear\n",
>  			        getpid(), filename);
>  
> -		usleep(100000);
> +		usleep(10);

Isn't 10us too small? Also we really should increase the number of the
loop iterations if we reduce the sleep as much.

>  	}
>  
>  	if (fd == -1) {
> -- 
> 1.8.4.2
> 
> 
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH 2/2] ltp-aiodio/dio_sparse: Fix usleep in read_sparse()

[Guangwen Feng]

Hi Cyril,

Thanks for your review.

On 01/24/2017 09:43 PM, Cyril Hrubis wrote:
> Hi!
>> diff --git a/testcases/kernel/io/ltp-aiodio/common_sparse.h b/testcases/kernel/io/ltp-aiodio/common_sparse.h
>> index f7f4ef4..a7f5035 100644
>> --- a/testcases/kernel/io/ltp-aiodio/common_sparse.h
>> +++ b/testcases/kernel/io/ltp-aiodio/common_sparse.h
>> @@ -123,7 +123,7 @@ static void read_sparse(char *filename, int filesize)
>>  			fprintf(stderr, "Child %i waits for '%s' to appear\n",
>>  			        getpid(), filename);
>>  
>> -		usleep(100000);
>> +		usleep(10);
> 
> Isn't 10us too small? Also we really should increase the number of the
> loop iterations if we reduce the sleep as much.
> 

Most of the time child works well by usleep(100), but there is still
a small chance that it misses the test on my system...

OK, I will also increase the loop number and send a V2.
Thanks.

Best Regards,
Guangwen Feng

>>  	}
>>  
>>  	if (fd == -1) {
>> -- 
>> 1.8.4.2
>>
>>
>>
>>
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp
> 

[LTP] [PATCH 1/2] ltp-aiodio/dio_sparse: Fix write in dio_sparse()

[Guangwen Feng]

Hi Cyril,

Thanks for your review.

On 01/24/2017 09:40 PM, Cyril Hrubis wrote:
> Hi!
>> Current code looks buggy because when the offset is greater than
>> or equal to the filesize, it will never happen to do the write
>> in the loop, as a result, ADSP080..ADSP087 do not work actually.
>> Fix it by making sure that we do write writesize bytes.
>>
>> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
>> ---
>>  testcases/kernel/io/ltp-aiodio/dio_sparse.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/testcases/kernel/io/ltp-aiodio/dio_sparse.c b/testcases/kernel/io/ltp-aiodio/dio_sparse.c
>> index 41b9929..3828ed7 100644
>> --- a/testcases/kernel/io/ltp-aiodio/dio_sparse.c
>> +++ b/testcases/kernel/io/ltp-aiodio/dio_sparse.c
>> @@ -78,7 +78,7 @@ int dio_sparse(char *filename, int align, int writesize, int filesize, int offse
>>  
>>  	memset(bufptr, 0, writesize);
>>  	lseek(fd, offset, SEEK_SET);
>> -	for (i = offset; i < filesize;) {
>> +	for (i = 0; i < filesize;) {
>>  		if ((w = write(fd, bufptr, writesize)) != writesize) {
>>  			tst_resm(TBROK | TERRNO, "write() returned %d", w);
>>  			close(fd);
> 
> Hmm, it looks to me like the code actually does what it should have.
> Since we pass a filesize and offset so the test should actually write
> filesize - offset bytes. As far as I can tell the bug here is that the
> test is not checking that filesize > offset before it starts.

Sorry, but shouldn't we write the whole "writesize" from the "offset"?
ADSP080 ADSP081 ADSP082 ADSP083 did reproduce the BUG via above modification...

Best Regards,
Guangwen Feng

[LTP] [PATCH v2] ltp-aiodio/dio_sparse: Fix usleep in read_sparse()

[Guangwen Feng]

usleep(100000) sometimes leads to child process being too late
to do the read before being killed by parent process, tune it
to usleep(100) to make sure we do the real test in time.

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 testcases/kernel/io/ltp-aiodio/common_sparse.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/io/ltp-aiodio/common_sparse.h b/testcases/kernel/io/ltp-aiodio/common_sparse.h
index f7f4ef4..7297319 100644
--- a/testcases/kernel/io/ltp-aiodio/common_sparse.h
+++ b/testcases/kernel/io/ltp-aiodio/common_sparse.h
@@ -113,7 +113,7 @@ static void read_sparse(char *filename, int filesize)
 	/*
 	 * Wait for the file to appear.
 	 */
-	for (i = 0; i < 10000; i++) {
+	for (i = 0; i < 10000000; i++) {
 		fd = open(filename, O_RDONLY);
 
 		if (fd != -1)
@@ -123,7 +123,7 @@ static void read_sparse(char *filename, int filesize)
 			fprintf(stderr, "Child %i waits for '%s' to appear\n",
 			        getpid(), filename);
 
-		usleep(100000);
+		usleep(100);
 	}
 
 	if (fd == -1) {
-- 
1.8.4.2

[LTP] [PATCH 1/2] ltp-aiodio/dio_sparse: Fix write in dio_sparse()

[Guangwen Feng]

Hi!

On 01/25/2017 12:33 PM, Guangwen Feng wrote:
> Hi Cyril,
> 
> Thanks for your review.
> 
> On 01/24/2017 09:40 PM, Cyril Hrubis wrote:
>> Hi!
>>> Current code looks buggy because when the offset is greater than
>>> or equal to the filesize, it will never happen to do the write
>>> in the loop, as a result, ADSP080..ADSP087 do not work actually.
>>> Fix it by making sure that we do write writesize bytes.
>>>
>>> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
>>> ---
>>>  testcases/kernel/io/ltp-aiodio/dio_sparse.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/testcases/kernel/io/ltp-aiodio/dio_sparse.c b/testcases/kernel/io/ltp-aiodio/dio_sparse.c
>>> index 41b9929..3828ed7 100644
>>> --- a/testcases/kernel/io/ltp-aiodio/dio_sparse.c
>>> +++ b/testcases/kernel/io/ltp-aiodio/dio_sparse.c
>>> @@ -78,7 +78,7 @@ int dio_sparse(char *filename, int align, int writesize, int filesize, int offse
>>>  
>>>  	memset(bufptr, 0, writesize);
>>>  	lseek(fd, offset, SEEK_SET);
>>> -	for (i = offset; i < filesize;) {
>>> +	for (i = 0; i < filesize;) {
>>>  		if ((w = write(fd, bufptr, writesize)) != writesize) {
>>>  			tst_resm(TBROK | TERRNO, "write() returned %d", w);
>>>  			close(fd);
>>
>> Hmm, it looks to me like the code actually does what it should have.
>> Since we pass a filesize and offset so the test should actually write
>> filesize - offset bytes. As far as I can tell the bug here is that the
>> test is not checking that filesize > offset before it starts.
> 
> Sorry, but shouldn't we write the whole "writesize" from the "offset"?
> ADSP080 ADSP081 ADSP082 ADSP083 did reproduce the BUG via above modification...

I think we pass an "offset" is to make sure writing from the file offset,
but the actual write size should be just the "writesize" argument.

As the second case of kernel commit 9ecd10b says:
    2. Direct writes starting from or beyong i_size (not inside i_size)
       also could trigger block allocation and expose stale data.  For
       example, consider a sparse file with i_size of 2k, and a write to
       offset 2k or 3k into the file, with a filesystem block size of 4k.
       (Thanks to Jeff Moyer for pointing this case out in his review.)


Best Regards,
Guangwen Feng

> 
> Best Regards,
> Guangwen Feng

[LTP] [PATCH 1/2] ltp-aiodio/dio_sparse: Fix write in dio_sparse()

[Cyril Hrubis]

Hi!
> >>>  	memset(bufptr, 0, writesize);
> >>>  	lseek(fd, offset, SEEK_SET);
> >>> -	for (i = offset; i < filesize;) {
> >>> +	for (i = 0; i < filesize;) {
> >>>  		if ((w = write(fd, bufptr, writesize)) != writesize) {
> >>>  			tst_resm(TBROK | TERRNO, "write() returned %d", w);
> >>>  			close(fd);
> >>
> >> Hmm, it looks to me like the code actually does what it should have.
> >> Since we pass a filesize and offset so the test should actually write
> >> filesize - offset bytes. As far as I can tell the bug here is that the
> >> test is not checking that filesize > offset before it starts.
> > 
> > Sorry, but shouldn't we write the whole "writesize" from the "offset"?
> > ADSP080 ADSP081 ADSP082 ADSP083 did reproduce the BUG via above modification...
> 
> I think we pass an "offset" is to make sure writing from the file offset,
> but the actual write size should be just the "writesize" argument.
> 
> As the second case of kernel commit 9ecd10b says:
>     2. Direct writes starting from or beyong i_size (not inside i_size)
>        also could trigger block allocation and expose stale data.  For
>        example, consider a sparse file with i_size of 2k, and a write to
>        offset 2k or 3k into the file, with a filesystem block size of 4k.
>        (Thanks to Jeff Moyer for pointing this case out in his review.)

Ah, looking at the git log, the offset was just added recently, that's
why the code is confusing, previously the filesize was size of the
file...

So what about we rename the writesize to blocksize and filesize to
writesize so that it's clear what the function dio_sparse does.

int dio_sparse(char *filename, int align, int blocksize, int writesize, int offset)

Also shouldn't we truncate the file to offset + filesize rather than
just filesize? And then pass the size to the children as offset +
filesize as well?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2] ltp-aiodio/dio_sparse: Fix usleep in read_sparse()

[Cyril Hrubis]

Hi!
> usleep(100000) sometimes leads to child process being too late
> to do the read before being killed by parent process, tune it
> to usleep(100) to make sure we do the real test in time.
> 
> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
> ---
>  testcases/kernel/io/ltp-aiodio/common_sparse.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/testcases/kernel/io/ltp-aiodio/common_sparse.h b/testcases/kernel/io/ltp-aiodio/common_sparse.h
> index f7f4ef4..7297319 100644
> --- a/testcases/kernel/io/ltp-aiodio/common_sparse.h
> +++ b/testcases/kernel/io/ltp-aiodio/common_sparse.h
> @@ -113,7 +113,7 @@ static void read_sparse(char *filename, int filesize)
>  	/*
>  	 * Wait for the file to appear.
>  	 */
> -	for (i = 0; i < 10000; i++) {
> +	for (i = 0; i < 10000000; i++) {
>  		fd = open(filename, O_RDONLY);
>  
>  		if (fd != -1)
> @@ -123,7 +123,7 @@ static void read_sparse(char *filename, int filesize)
>  			fprintf(stderr, "Child %i waits for '%s' to appear\n",
>  			        getpid(), filename);
>  
> -		usleep(100000);
> +		usleep(100);
>  	}

Looking at the code again, I see no reason why we can't open and
truncate the file before we start the read_sparse() children, then we
could drop this loop and just pass file descriptor to this function and
to the dio_sparse() and aiodio_sparse() as well. Or did I miss
something?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2] ltp-aiodio/dio_sparse: Fix usleep in read_sparse()

[Guangwen Feng]

Hi!

On 02/09/2017 05:35 PM, Cyril Hrubis wrote:
> Hi!
>> usleep(100000) sometimes leads to child process being too late
>> to do the read before being killed by parent process, tune it
>> to usleep(100) to make sure we do the real test in time.
>>
>> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
>> ---
>>  testcases/kernel/io/ltp-aiodio/common_sparse.h | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/testcases/kernel/io/ltp-aiodio/common_sparse.h b/testcases/kernel/io/ltp-aiodio/common_sparse.h
>> index f7f4ef4..7297319 100644
>> --- a/testcases/kernel/io/ltp-aiodio/common_sparse.h
>> +++ b/testcases/kernel/io/ltp-aiodio/common_sparse.h
>> @@ -113,7 +113,7 @@ static void read_sparse(char *filename, int filesize)
>>  	/*
>>  	 * Wait for the file to appear.
>>  	 */
>> -	for (i = 0; i < 10000; i++) {
>> +	for (i = 0; i < 10000000; i++) {
>>  		fd = open(filename, O_RDONLY);
>>  
>>  		if (fd != -1)
>> @@ -123,7 +123,7 @@ static void read_sparse(char *filename, int filesize)
>>  			fprintf(stderr, "Child %i waits for '%s' to appear\n",
>>  			        getpid(), filename);
>>  
>> -		usleep(100000);
>> +		usleep(100);
>>  	}
> 
> Looking at the code again, I see no reason why we can't open and
> truncate the file before we start the read_sparse() children, then we
> could drop this loop and just pass file descriptor to this function and
> to the dio_sparse() and aiodio_sparse() as well. Or did I miss
> something?

Yes, you are right, it is a thoroghly solution to this issue,
I will rewrite this according to your suggestion, thanks.

Best Regards,
Guangwen Feng 

[LTP] [PATCH 1/2] ltp-aiodio/dio_sparse: Fix write in dio_sparse()

[Guangwen Feng]

Hi!

On 02/09/2017 05:31 PM, Cyril Hrubis wrote:
> Hi!
>>>>>  	memset(bufptr, 0, writesize);
>>>>>  	lseek(fd, offset, SEEK_SET);
>>>>> -	for (i = offset; i < filesize;) {
>>>>> +	for (i = 0; i < filesize;) {
>>>>>  		if ((w = write(fd, bufptr, writesize)) != writesize) {
>>>>>  			tst_resm(TBROK | TERRNO, "write() returned %d", w);
>>>>>  			close(fd);
>>>>
>>>> Hmm, it looks to me like the code actually does what it should have.
>>>> Since we pass a filesize and offset so the test should actually write
>>>> filesize - offset bytes. As far as I can tell the bug here is that the
>>>> test is not checking that filesize > offset before it starts.
>>>
>>> Sorry, but shouldn't we write the whole "writesize" from the "offset"?
>>> ADSP080 ADSP081 ADSP082 ADSP083 did reproduce the BUG via above modification...
>>
>> I think we pass an "offset" is to make sure writing from the file offset,
>> but the actual write size should be just the "writesize" argument.
>>
>> As the second case of kernel commit 9ecd10b says:
>>     2. Direct writes starting from or beyong i_size (not inside i_size)
>>        also could trigger block allocation and expose stale data.  For
>>        example, consider a sparse file with i_size of 2k, and a write to
>>        offset 2k or 3k into the file, with a filesystem block size of 4k.
>>        (Thanks to Jeff Moyer for pointing this case out in his review.)
> 
> Ah, looking at the git log, the offset was just added recently, that's
> why the code is confusing, previously the filesize was size of the
> file...

Yes, it seems that the code get confusing after the offset was added,
and yes, previously the filesize was the size of the file, but sorry,
according to my understanding it still represents the size of the file now.

> 
> So what about we rename the writesize to blocksize and filesize to
> writesize so that it's clear what the function dio_sparse does.
> 
> int dio_sparse(char *filename, int align, int blocksize, int writesize, int offset)
> 
> Also shouldn't we truncate the file to offset + filesize rather than
> just filesize? And then pass the size to the children as offset +
> filesize as well?

In my humble opinion, there was no problem at all in the code before
the offset was added, and look at the commit message of adding offset,
the aim is to achieve the second case of kernel commit 9ecd10b above,
make it to direct write from or beyond the end of the file.

So, I think the writesize, filesize and offset should be independent of
each other, the "offset" here is not filesize's offset (i.e. filesize
does not calculate with offset), but only to decide where we write start from.

Best Regards,
Guangwen Feng 

[LTP] [PATCH 1/2] ltp-aiodio/dio_sparse: Fix write in dio_sparse()

[Cyril Hrubis]

Hi!
> >> As the second case of kernel commit 9ecd10b says:
> >>     2. Direct writes starting from or beyong i_size (not inside i_size)
> >>        also could trigger block allocation and expose stale data.  For
> >>        example, consider a sparse file with i_size of 2k, and a write to
> >>        offset 2k or 3k into the file, with a filesystem block size of 4k.
> >>        (Thanks to Jeff Moyer for pointing this case out in his review.)
> > 
> > Ah, looking at the git log, the offset was just added recently, that's
> > why the code is confusing, previously the filesize was size of the
> > file...
> 
> Yes, it seems that the code get confusing after the offset was added,
> and yes, previously the filesize was the size of the file, but sorry,
> according to my understanding it still represents the size of the file now.

Ok we are mixing two things together. One is how much size the file
actually takes on a filesystem and second one is the file length.

> > 
> > So what about we rename the writesize to blocksize and filesize to
> > writesize so that it's clear what the function dio_sparse does.
> > 
> > int dio_sparse(char *filename, int align, int blocksize, int writesize, int offset)
> > 
> > Also shouldn't we truncate the file to offset + filesize rather than
> > just filesize? And then pass the size to the children as offset +
> > filesize as well?
> 
> In my humble opinion, there was no problem at all in the code before
> the offset was added, and look at the commit message of adding offset,
> the aim is to achieve the second case of kernel commit 9ecd10b above,
> make it to direct write from or beyond the end of the file.

The patch is correct, there is no doubt about that.

> So, I think the writesize, filesize and offset should be independent of
> each other, the "offset" here is not filesize's offset (i.e. filesize
> does not calculate with offset), but only to decide where we write start from.

Okay, but still the writesize should be rename to something as
block_size.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] ltp-aiodio: Create the file before fork

[Guangwen Feng]

Currently, child's waiting for the file to appear in a loop with
usleep(100000) sometimes is too late to do the read before child
process is killed by parent process, which lead to child misses
the whole test.  Create and truncate the file before fork, then
drop the loop of waiting in read_sparse().

Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
---
 testcases/kernel/io/ltp-aiodio/aiodio_sparse.c | 32 +++++++++-----------------
 testcases/kernel/io/ltp-aiodio/common_sparse.h | 17 +-------------
 testcases/kernel/io/ltp-aiodio/dio_sparse.c    | 28 +++++++++-------------
 3 files changed, 23 insertions(+), 54 deletions(-)

diff --git a/testcases/kernel/io/ltp-aiodio/aiodio_sparse.c b/testcases/kernel/io/ltp-aiodio/aiodio_sparse.c
index 49a0915..d40e45b 100644
--- a/testcases/kernel/io/ltp-aiodio/aiodio_sparse.c
+++ b/testcases/kernel/io/ltp-aiodio/aiodio_sparse.c
@@ -43,6 +43,7 @@
 #define NUM_CHILDREN 1000
 
 int debug;
+int fd;
 
 static void setup(void);
 static void cleanup(void);
@@ -56,10 +57,8 @@ int TST_TOTAL = 1;
 /*
  * do async DIO writes to a sparse file
  */
-int aiodio_sparse(char *filename, int align, int writesize, int filesize,
-		  int num_aio)
+int aiodio_sparse(int fd, int align, int writesize, int filesize, int num_aio)
 {
-	int fd;
 	int i, w;
 	struct iocb **iocbs;
 	off_t offset;
@@ -81,15 +80,6 @@ int aiodio_sparse(char *filename, int align, int writesize, int filesize,
 		}
 	}
 
-	fd = open(filename, O_DIRECT | O_WRONLY | O_CREAT | O_EXCL, 0600);
-
-	if (fd < 0) {
-		tst_resm(TBROK | TERRNO, "open()");
-		return 1;
-	}
-
-	SAFE_FTRUNCATE(cleanup, fd, filesize);
-
 	/*
 	 * allocate the iocbs array and iocbs with buffers
 	 */
@@ -100,8 +90,6 @@ int aiodio_sparse(char *filename, int align, int writesize, int filesize,
 		TEST(posix_memalign(&bufptr, align, writesize));
 		if (TEST_RETURN) {
 			tst_resm(TBROK | TRERRNO, "cannot allocate aligned memory");
-			close(fd);
-			unlink(filename);
 			return 1;
 		}
 		memset(bufptr, 0, writesize);
@@ -114,8 +102,6 @@ int aiodio_sparse(char *filename, int align, int writesize, int filesize,
 	 */
 	if ((w = io_submit(myctx, num_aio, iocbs)) < 0) {
 		tst_resm(TBROK, "io_submit() returned %i", w);
-		close(fd);
-		unlink(filename);
 		return 1;
 	}
 
@@ -205,9 +191,6 @@ int aiodio_sparse(char *filename, int align, int writesize, int filesize,
 		}
 	}
 
-	close(fd);
-	unlink(filename);
-
 	return 0;
 }
 
@@ -275,11 +258,16 @@ int main(int argc, char **argv)
 	tst_resm(TINFO, "Dirtying free blocks");
 	dirty_freeblocks(filesize);
 
+	fd = SAFE_OPEN(cleanup, filename,
+		O_DIRECT | O_WRONLY | O_CREAT | O_EXCL, 0600);
+	SAFE_FTRUNCATE(cleanup, fd, filesize);
+
 	tst_resm(TINFO, "Starting I/O tests");
 	signal(SIGTERM, SIG_DFL);
 	for (i = 0; i < num_children; i++) {
 		switch (pid[i] = fork()) {
 		case 0:
+			SAFE_CLOSE(NULL, fd);
 			read_sparse(filename, filesize);
 			break;
 		case -1:
@@ -293,7 +281,7 @@ int main(int argc, char **argv)
 	}
 	tst_sig(FORK, DEF_HANDLER, cleanup);
 
-	ret = aiodio_sparse(filename, alignment, writesize, filesize, num_aio);
+	ret = aiodio_sparse(fd, alignment, writesize, filesize, num_aio);
 
 	tst_resm(TINFO, "Killing childrens(s)");
 
@@ -332,6 +320,8 @@ static void setup(void)
 
 static void cleanup(void)
 {
+	if (fd > 0 && close(fd))
+		tst_resm(TWARN | TERRNO, "Failed to close file");
+
 	tst_rmdir();
-	tst_exit();
 }
diff --git a/testcases/kernel/io/ltp-aiodio/common_sparse.h b/testcases/kernel/io/ltp-aiodio/common_sparse.h
index f7f4ef4..e8b906e 100644
--- a/testcases/kernel/io/ltp-aiodio/common_sparse.h
+++ b/testcases/kernel/io/ltp-aiodio/common_sparse.h
@@ -110,22 +110,7 @@ static void read_sparse(char *filename, int filesize)
 	int  i, j, r;
 	char buf[4096];
 
-	/*
-	 * Wait for the file to appear.
-	 */
-	for (i = 0; i < 10000; i++) {
-		fd = open(filename, O_RDONLY);
-
-		if (fd != -1)
-			break;
-
-		if (debug)
-			fprintf(stderr, "Child %i waits for '%s' to appear\n",
-			        getpid(), filename);
-
-		usleep(100000);
-	}
-
+	fd = open(filename, O_RDONLY);
 	if (fd == -1) {
 		if (debug)
 			fprintf(stderr, "Child %i failed to open '%s'\n",
diff --git a/testcases/kernel/io/ltp-aiodio/dio_sparse.c b/testcases/kernel/io/ltp-aiodio/dio_sparse.c
index 41b9929..67b338b 100644
--- a/testcases/kernel/io/ltp-aiodio/dio_sparse.c
+++ b/testcases/kernel/io/ltp-aiodio/dio_sparse.c
@@ -45,6 +45,7 @@ static void setup(void);
 static void cleanup(void);
 static void usage(void);
 static int debug = 0;
+static int fd;
 
 char *TCID = "dio_sparse";
 int TST_TOTAL = 1;
@@ -54,25 +55,14 @@ int TST_TOTAL = 1;
 /*
  * Write zeroes using O_DIRECT into sparse file.
  */
-int dio_sparse(char *filename, int align, int writesize, int filesize, int offset)
+int dio_sparse(int fd, int align, int writesize, int filesize, int offset)
 {
-	int fd;
 	void *bufptr;
 	int i, w;
 
-	fd = open(filename, O_DIRECT | O_WRONLY | O_CREAT | O_EXCL, 0600);
-
-	if (fd < 0) {
-		tst_resm(TBROK | TERRNO, "open()");
-		return 1;
-	}
-
-	SAFE_FTRUNCATE(cleanup, fd, filesize);
-
 	TEST(posix_memalign(&bufptr, align, writesize));
 	if (TEST_RETURN) {
 		tst_resm(TBROK | TRERRNO, "cannot allocate aligned memory");
-		close(fd);
 		return 1;
 	}
 
@@ -81,16 +71,12 @@ int dio_sparse(char *filename, int align, int writesize, int filesize, int offse
 	for (i = offset; i < filesize;) {
 		if ((w = write(fd, bufptr, writesize)) != writesize) {
 			tst_resm(TBROK | TERRNO, "write() returned %d", w);
-			close(fd);
 			return 1;
 		}
 
 		i += w;
 	}
 
-	close(fd);
-	unlink(filename);
-
 	return 0;
 }
 
@@ -156,11 +142,16 @@ int main(int argc, char **argv)
 	tst_resm(TINFO, "Dirtying free blocks");
 	dirty_freeblocks(filesize);
 
+	fd = SAFE_OPEN(cleanup, filename,
+		O_DIRECT | O_WRONLY | O_CREAT | O_EXCL, 0600);
+	SAFE_FTRUNCATE(cleanup, fd, filesize);
+
 	tst_resm(TINFO, "Starting I/O tests");
 	signal(SIGTERM, SIG_DFL);
 	for (i = 0; i < num_children; i++) {
 		switch (pid[i] = fork()) {
 		case 0:
+			SAFE_CLOSE(NULL, fd);
 			read_sparse(filename, filesize);
 			break;
 		case -1:
@@ -174,7 +165,7 @@ int main(int argc, char **argv)
 	}
 	tst_sig(FORK, DEF_HANDLER, cleanup);
 
-	ret = dio_sparse(filename, alignment, writesize, filesize, offset);
+	ret = dio_sparse(fd, alignment, writesize, filesize, offset);
 
 	tst_resm(TINFO, "Killing childrens(s)");
 
@@ -213,5 +204,8 @@ static void setup(void)
 
 static void cleanup(void)
 {
+	if (fd > 0 && close(fd))
+		tst_resm(TWARN | TERRNO, "Failed to close file");
+
 	tst_rmdir();
 }
-- 
1.8.4.2

[LTP] [PATCH] ltp-aiodio: Create the file before fork

[Cyril Hrubis]

Hi!
Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz