On Mon, Feb 27, 2017 at 11:09:01AM -0500, Brian Foster wrote:
> cc Christoph
>
> On Mon, Feb 27, 2017 at 12:22:20PM +0800, Xiong Zhou wrote:
> > Hi,
> >
> > These 2 tests PASS on Linus tree commit:
> > 37c8596 Merge tag 'tty-4.11-rc1' of git://git.kernel.org/pub/scm/linux...
> > FAIL on commit:
> > 60e8d3e Merge tag 'pci-v4.11-changes' of git://git.kernel.org/pub/scm/...
> >
> > LTP latest commit: c60d3ca move_pages12: include lapi/mmap.h
> >
> > Steps:
> >
> > sh-4.2# pwd
> > /root/ltp
> > sh-4.2# git log --oneline -1
> > c60d3ca move_pages12: include lapi/mmap.h
> > sh-4.2# uname -r
> > 4.10.0-master-60e8d3e+
> > sh-4.2# mount | grep test1
> > /dev/sda3 on /test1 type xfs (rw,relatime,seclabel,attr2,inode64,logbsize=256k,sunit=512,swidth=512,noquota)
> > sh-4.2# xfs_info /test1
> > meta-data=/dev/sda3 isize=512 agcount=16, agsize=245696 blks
> > = sectsz=512 attr=2, projid32bit=1
> > = crc=1 finobt=1 spinodes=0
> > data = bsize=4096 blocks=3931136, imaxpct=25
> > = sunit=64 swidth=64 blks
> > naming =version 2 bsize=4096 ascii-ci=0 ftype=1
> > log =internal bsize=4096 blocks=2560, version=2
> > = sectsz=512 sunit=64 blks, lazy-count=1
> > realtime =none extsz=4096 blocks=0, rtextents=0
> > sh-4.2#
> > sh-4.2# TMPDIR=/test1 ./testcases/kernel/syscalls/write/write03
> > write03 0 TINFO : Enter Block 1: test to check if write corrupts the file when write fails
> > write03 1 TFAIL : write03.c:125: failure of write(2) corrupted the file
> > write03 0 TINFO : Exit block 1
> > sh-4.2#
>
> On a quick test, both of these are reproduced after commit fa7f138ac4
> ("xfs: clear delalloc and cache on buffered write failure"). That patch
> fixed a problem where if the write allocates a block but fails to write
> anything (written == 0), we'd leave a delalloc block lingering in the
> inode.
>
> With that change, this test now fails because it sends two writes within
> a single block. The first allocates the block, writes 100 bytes and
> returns successfully. The next attempts to write the next 100 bytes,
> fails and triggers the cleanup of the block because we can't tell
> whether this write or the previous had allocated it.
>
> I'm not convinced the right solution is to just go back to the previous
> code. That obviously reintroduces the original problem, but we'd also
> still have a similar problem if the second (failed) write was a rewrite
> of the first. The error handling of the second write would kill off the
> blocks allocated and written to successfully by the first. I'm wondering
> if the right thing to do here is factor in i_size as it appears that's
> what this code did prior to the iomap transition. I'm not sure where
> that leaves us wrt to writes into sparse files, though. I may need to
> play with this a bit..
>
After playing around a bit, I don't think using i_size is the right
approach either. It just exacerbates the original problem on buffered
writes into sparse files. We can end up leaving around however many
delalloc blocks we've allocated.
I think we need a way to differentiate preexisting (previously written)
delalloc blocks from those allocated and unused by the current write. We
might be able to do that by looking at the pagecache, but I think that
means looking at the buffer state to make sure we handle sub-page block
sizes correctly. I.e., make *_iomap_end_delalloc() punch out all
delalloc blocks in the non-written range that are either not page backed
or not dirty+delalloc buffer backed. Hm?
Brian
> Christoph, any thoughts on this?
>
> Brian
>
> > sh-4.2# TMPDIR=/test1 ./testcases/kernel/syscalls/writev/writev07
> > tst_test.c:760: INFO: Timeout per run is 0h 05m 00s
> > writev07.c:60: INFO: starting test with initial file offset: 0
> > writev07.c:82: INFO: got EFAULT
> > writev07.c:87: FAIL: file was written to
> > writev07.c:93: PASS: offset stayed unchanged
> > writev07.c:60: INFO: starting test with initial file offset: 65
> > writev07.c:82: INFO: got EFAULT
> > writev07.c:89: PASS: file stayed untouched
> > writev07.c:93: PASS: offset stayed unchanged
> > writev07.c:60: INFO: starting test with initial file offset: 4096
> > writev07.c:82: INFO: got EFAULT
> > writev07.c:89: PASS: file stayed untouched
> > writev07.c:93: PASS: offset stayed unchanged
> > writev07.c:60: INFO: starting test with initial file offset: 4097
> > writev07.c:82: INFO: got EFAULT
> > writev07.c:89: PASS: file stayed untouched
> > writev07.c:93: PASS: offset stayed unchanged
> >
> > Summary:
> > passed 7
> > failed 1
> > skipped 0
> > warnings 0
> > sh-4.2#
> > sh-4.2# mkfs.xfs -V
> > mkfs.xfs version 4.7.0
> > sh-4.2# cd ../xfsprogs/
> > sh-4.2# git log --oneline -1
> > d7e1f5f xfsprogs: Release v4.7
> > sh-4.2#
> >
> > Thanks,
> > Xiong
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
