public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
From: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH 1/2] ltp-aiodio/dio_sparse: Fix write in dio_sparse()
Date: Thu, 9 Feb 2017 16:56:05 +0800	[thread overview]
Message-ID: <589C2EA5.6040707@cn.fujitsu.com> (raw)
In-Reply-To: <58882AA5.90906@cn.fujitsu.com>

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



  reply	other threads:[~2017-02-09  8:56 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-19  9:32 [LTP] [PATCH 1/2] ltp-aiodio/dio_sparse: Fix write in dio_sparse() Guangwen Feng
2017-01-19  9:32 ` [LTP] [PATCH 2/2] ltp-aiodio/dio_sparse: Fix usleep in read_sparse() Guangwen Feng
2017-01-24 13:43   ` Cyril Hrubis
2017-01-25  4:02     ` Guangwen Feng
2017-02-09  7:23       ` [LTP] [PATCH v2] " Guangwen Feng
2017-02-09  9:35         ` Cyril Hrubis
2017-02-14  3:20           ` Guangwen Feng
2017-03-21  8:08             ` [LTP] [PATCH] ltp-aiodio: Create the file before fork Guangwen Feng
2017-03-22 15:33               ` Cyril Hrubis
2017-01-24 13:40 ` [LTP] [PATCH 1/2] ltp-aiodio/dio_sparse: Fix write in dio_sparse() Cyril Hrubis
2017-01-25  4:33   ` Guangwen Feng
2017-02-09  8:56     ` Guangwen Feng [this message]
2017-02-09  9:31       ` Cyril Hrubis
2017-02-14  8:53         ` Guangwen Feng
2017-02-16 13:12           ` Cyril Hrubis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=589C2EA5.6040707@cn.fujitsu.com \
    --to=fenggw-fnst@cn.fujitsu.com \
    --cc=ltp@lists.linux.it \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox