All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Sandeen <sandeen@sandeen.net>
To: lachlan@sgi.com
Cc: xfs-oss <xfs@oss.sgi.com>
Subject: Re: [PATCH] fix corruption case for block size < page size
Date: Tue, 16 Dec 2008 00:21:46 -0600	[thread overview]
Message-ID: <494748FA.20404@sandeen.net> (raw)
In-Reply-To: <4947466D.7000705@sandeen.net>

Eric Sandeen wrote:
> Lachlan McIlroy wrote:
>> Eric Sandeen wrote:
>>> Actually; after the truncate down step (3) we should have:
>>>
>>>      |<--------trunc-----------------------
>>> 3: |11??|                                       trunc down to 1/2 block
>>>      ^
>>>      |
>>>     EOF
>>>
>>> Hm, but does the end of this block get zeroed now or only when we
>>> subsequently extend the size?  The latter I think...?
>> Only when extending the file size.
> 
> Right.
> 
>>> So I think in the next step:
>>>
>>>  trunc-->|
>>> 4: |1100|                                        trunc up to block+1byte
>>>       ^^
>>>   now || this part of the block gets zeroed, right, by xfs_zero_eof?
>> Yes (by xfs_zero_last_block()).
> 
> Right.  :)  But I *think* that after this step we are actually zeroing
> into block 1 (2nd block) and causing it to get zeroed/mapped.  Off by
> one maybe?
> 
>>>> Because of the truncate to 256 bytes
>>>> only the first block is allocated and everything beyond 512 bytes is
>>>> a hole.  
>>> Yep, up until the last write anyway.
>>>
>>>> More specifically there is a hole under the remainder of the
>>>> page so xfs_zero_eof() will skip that region and not zero anything.
>>> Well, the last write (step 5) is still completely within the page...
>>>
>>> Right, that's what it *should* be doing; but in page_state_convert (and
>>> I'll admit to not having this 100% nailed down) we write block 1 and map
>>> blocks 2 & 3 back into the file, and get:
>>>
>>> # |1100|0000|1111|1111|2222|----|----|----|
>>>              ^^^^ ^^^^
>>> where these  |||| |||| blocks are stale data, and block 1 is written
>>> (but at least zeroed).  How block 1 got zeroed I guess I'm not quite
>> I think block 1 got zeroed during the last write because the file size
>> was extended from 513 to 2048.  Byte 513 is just inside block 1.  But
>> that block should have been a hole and xfs_zero_last_block() should
>> have skipped it.
> 
> I think the 2nd extending write does skip it but from a bit more looking
> the first extending truncate might step into it by one... still looking
> into that.

Gah; or not.  what is going on here...  Doing just steps 1, 2, 3, 4
(ending on the extending truncate):

# xfs_io -c "pwrite -S 0x11 -b 4096 0 4096" -c "mmap -r 0 512" -c "mread
0 512" -c "munmap" -c "truncate 256" -c "truncate 514" -t -d -f
/mnt/scratch/testfile

# xfs_bmap -v /mnt/scratch/testfile
/mnt/scratch/testfile:
 EXT: FILE-OFFSET      BLOCK-RANGE      AG AG-OFFSET        TOTAL
   0: [0..0]:          63..63            0 (63..63)             1
   1: [1..1]:          hole                                     1

It looks like what I expect, at this point.  But then:

# sync
# xfs_bmap -v /mnt/scratch/testfile
/mnt/scratch/testfile:
 EXT: FILE-OFFSET      BLOCK-RANGE      AG AG-OFFSET        TOTAL
   0: [0..1]:          63..64            0 (63..64)             2

Um, why'd that last block get mapped in?  mmap vs. direct IO I'm
guessing... w/o the mmap read this does not happen.

-Eric (heading to bed now...)

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2008-12-16  6:21 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-13  7:07 [PATCH] fix corruption case for block size < page size Eric Sandeen
2008-12-13 17:48 ` Eric Sandeen
2008-12-13 18:20 ` Eric Sandeen
2008-12-16  5:00   ` Lachlan McIlroy
2008-12-16  5:40     ` Eric Sandeen
2008-12-16  6:05       ` Lachlan McIlroy
2008-12-16  6:10         ` Eric Sandeen
2008-12-16  6:21           ` Eric Sandeen [this message]
2008-12-16  6:51             ` Eric Sandeen
2009-01-07  5:23               ` Lachlan McIlroy
2009-01-07  5:53                 ` Eric Sandeen
2009-01-07  6:32                   ` Lachlan McIlroy
2009-01-07 21:42                     ` Dave Chinner
2009-01-09  0:18                       ` Lachlan McIlroy
2008-12-16  7:54           ` Lachlan McIlroy

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=494748FA.20404@sandeen.net \
    --to=sandeen@sandeen.net \
    --cc=lachlan@sgi.com \
    --cc=xfs@oss.sgi.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.