On Wed, Sep 24, 2014 at 11:43:07AM -0400, Matthew Wilcox wrote:
> On Thu, Sep 11, 2014 at 01:09:26PM +1000, Dave Chinner wrote:
> > On Wed, Sep 10, 2014 at 11:23:37AM -0400, Matthew Wilcox wrote:
> > > On Wed, Sep 03, 2014 at 05:47:24PM +1000, Dave Chinner wrote:
> > > > > + error = get_block(inode, block, &bh, 0);
> > > > > + if (!error && (bh.b_size < PAGE_SIZE))
> > > > > + error = -EIO;
> > > > > + if (error)
> > > > > + goto unlock_page;
> > > >
> > > > page fault into unwritten region, returns buffer_unwritten(bh) ==
> > > > true. Hence buffer_written(bh) is false, and we take this branch:
> > > >
> > > > > + if (!buffer_written(&bh) && !vmf->cow_page) {
> > > > > + if (vmf->flags & FAULT_FLAG_WRITE) {
> > > > > + error = get_block(inode, block, &bh, 1);
> > > >
> > > > Exactly what are you expecting to happen here? We don't do
> > > > allocation because there are already unwritten blocks over this
> > > > extent, and so bh will be unchanged when returning. i.e. it will
> > > > still be mapping an unwritten extent.
> > >
> > > I was expecting calling get_block() on an unwritten extent to convert it
> > > to a written extent. Your suggestion below of using b_end_io() to do that
> > > is a better idea.
> > >
> > > So this should be:
> > >
> > > if (!buffer_mapped(&bh) && !vmf->cow_page) {
> > >
> > > ... right?
> >
> > Yes, that is the conclusion I reached as well. ;)
>
> Now I know why I was expecting get_block() on an unwritten extent to
> convert it to a written extent. That's the way ext4 behaves!
That seems wrong. Unwritten extent conversion should only occur
on IO completion...
>
> [ 236.660772] got bh ffffffffa06e3bd0 1000
> [ 236.660814] got bh for write ffffffffa06e3bd0 60
> [ 236.660821] calling end_io ffffffffa06e3bd0 60
>
> (1000 is BH_Unwritten, 60 is BH_Mapped | BH_New)
>
> The code producing this output:
>
> error = get_block(inode, block, &bh, 0);
> printk("got bh %p %lx\n", bh.b_end_io, bh.b_state);
> if (!error && (bh.b_size < PAGE_SIZE))
> error = -EIO;
> if (error)
> goto unlock_page;
>
> if (!buffer_mapped(&bh) && !vmf->cow_page) {
> if (vmf->flags & FAULT_FLAG_WRITE) {
> error = get_block(inode, block, &bh, 1);
> printk("got bh for write %p %lx\n", bh.b_end_io, bh.b_state);
%pF will do symbol decoding for you ;)
>
> # xfs_io -f -c "truncate 20k" -c "fiemap -v" -c "falloc 0 20k" -c "fiemap -v" -c "mmap -w 0 20k" -c "fiemap -v" -c "mwrite 4k 4k" -c "fiemap -v" /mnt/ram0/b
> /mnt/ram0/b:
> /mnt/ram0/b:
> EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS
> 0: [0..39]: 263176..263215 40 0x801
> /mnt/ram0/b:
> EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS
> 0: [0..39]: 263176..263215 40 0x801
> /mnt/ram0/b:
> EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS
> 0: [0..39]: 263176..263215 40 0x1
>
> Actually, this looks wrong ... ext4 should only have converted one block
> of the extent to written, not all of it. I think that means ext4 is
> exposing stale data :-( I'll keep digging.
Check to see if ext4 has zeroed the entire extent - it does some
convoluted "hole filling" in certain siutations where it extends the
range of allocation operations by writing zeros around the range that
it was asked to allocate.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>