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!
[ 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);
# 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
