| Message ID | 20181208163639.GR24487@magnolia (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [GIT,PULL] xfs: fixes for v4.20-rc6 | expand |
On Sat, Dec 8, 2018 at 8:36 AM Darrick J. Wong <darrick.wong@oracle.com> wrote: > > Finally, the most important fix is to the pipe splicing code (aka the > generic copy_file_range fallback) to avoid pointless short directio > reads by only asking the filesystem for as much data as there are > available pages in the pipe buffer. Our previous fix (simulated short > directio reads because the number of pages didn't match the length of > the read requested) caused subtle problems on overlayfs, so that part is > reverted. Honestly, I really wish you simply wouldn't send me "xfs" fixes that aren't really xfs-specific at all. All the splice patches (and honestly, I feel some of the iomap ones too) that have come in through the xfs tree should have been handled separately as actual VFS patches. Or at least had acks from Al or something. I'm looking at that splice patch, and my initial reaction was "Hmm. but that breaks 0 vs -EAGAIN". But then I realized that that's why you're validating pipe->nrbufs, because the special temporary per-thread pipe is always supposed to be empty on entry, so a zero length can't happen. But just the fact that I felt like I had to go and look at one of these commits makes me go "this is not an XFS fix at all, it shouldn't have been treated as an XFS patch, and the original commit that *caused* the problem shouldn't have been treated as one either". So please. I want to feel like when I get a XFS pull from the XFS maintainer, I don't need to worry about it, and I can just do the git pull without having to check details. But that means that when you do changes outside of XFS code, those changes need to be handled _differently_. And they shouldn't be bypassing Al etc. And even if you can't get an Ack from Al, send them separately, so that I can check them without there being any XFS issues that are mixed up with the pull. So the patch looks good, and I'm merging this, but I really really don't have to feel like I need to look at xfs pulls this way. Linus
The pull request you sent on Sat, 8 Dec 2018 08:36:39 -0800:
> git://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git tags/xfs-4.20-fixes-3
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/f896adc42d5399eb68c9900bd4fd471ccea895e4
Thank you!
On Sat, Dec 08, 2018 at 11:24:48AM -0800, Linus Torvalds wrote: > Honestly, I really wish you simply wouldn't send me "xfs" fixes that > aren't really xfs-specific at all. > > All the splice patches (and honestly, I feel some of the iomap ones > too) that have come in through the xfs tree should have been handled > separately as actual VFS patches. Or at least had acks from Al or > something. iomap came in through the XFS tree originally, and has been very much maintained in combo, even though we now have another user with gfs. Maybe we just need to make that official in MAINTAINERS?
On Sat, Dec 08, 2018 at 11:24:48AM -0800, Linus Torvalds wrote: > On Sat, Dec 8, 2018 at 8:36 AM Darrick J. Wong <darrick.wong@oracle.com> wrote: > > > > Finally, the most important fix is to the pipe splicing code (aka the > > generic copy_file_range fallback) to avoid pointless short directio > > reads by only asking the filesystem for as much data as there are > > available pages in the pipe buffer. Our previous fix (simulated short > > directio reads because the number of pages didn't match the length of > > the read requested) caused subtle problems on overlayfs, so that part is > > reverted. > > Honestly, I really wish you simply wouldn't send me "xfs" fixes that > aren't really xfs-specific at all. > > All the splice patches (and honestly, I feel some of the iomap ones > too) that have come in through the xfs tree should have been handled > separately as actual VFS patches. Or at least had acks from Al or > something. > > I'm looking at that splice patch, and my initial reaction was "Hmm. > but that breaks 0 vs -EAGAIN". But then I realized that that's why > you're validating pipe->nrbufs, because the special temporary > per-thread pipe is always supposed to be empty on entry, so a zero > length can't happen. > > But just the fact that I felt like I had to go and look at one of > these commits makes me go "this is not an XFS fix at all, it shouldn't > have been treated as an XFS patch, and the original commit that > *caused* the problem shouldn't have been treated as one either". > > So please. I want to feel like when I get a XFS pull from the XFS > maintainer, I don't need to worry about it, and I can just do the git > pull without having to check details. > > But that means that when you do changes outside of XFS code, those > changes need to be handled _differently_. And they shouldn't be > bypassing Al etc. And even if you can't get an Ack from Al, send them > separately, so that I can check them without there being any XFS > issues that are mixed up with the pull. Ok, I'll put xfs and vfs changesets in separate pull requests in the future. Thanks for the feedback. --D > So the patch looks good, and I'm merging this, but I really really > don't have to feel like I need to look at xfs pulls this way. > > Linus
On Tue, Dec 11, 2018 at 05:58:18AM -0800, Christoph Hellwig wrote: > On Sat, Dec 08, 2018 at 11:24:48AM -0800, Linus Torvalds wrote: > > Honestly, I really wish you simply wouldn't send me "xfs" fixes that > > aren't really xfs-specific at all. > > > > All the splice patches (and honestly, I feel some of the iomap ones > > too) that have come in through the xfs tree should have been handled > > separately as actual VFS patches. Or at least had acks from Al or > > something. > > iomap came in through the XFS tree originally, and has been very much > maintained in combo, even though we now have another user with gfs. > > Maybe we just need to make that official in MAINTAINERS? Yes, though who should be the official maintainer? You wrote most of the iomap code; do you want to maintain it yourself? Or have people funnel them wherever XFS patches go? :) --D
On Wed, Dec 12, 2018 at 10:16:58AM -0800, Darrick J. Wong wrote: > On Tue, Dec 11, 2018 at 05:58:18AM -0800, Christoph Hellwig wrote: > > On Sat, Dec 08, 2018 at 11:24:48AM -0800, Linus Torvalds wrote: > > > Honestly, I really wish you simply wouldn't send me "xfs" fixes that > > > aren't really xfs-specific at all. > > > > > > All the splice patches (and honestly, I feel some of the iomap ones > > > too) that have come in through the xfs tree should have been handled > > > separately as actual VFS patches. Or at least had acks from Al or > > > something. > > > > iomap came in through the XFS tree originally, and has been very much > > maintained in combo, even though we now have another user with gfs. > > > > Maybe we just need to make that official in MAINTAINERS? > > Yes, though who should be the official maintainer? You wrote most of > the iomap code; do you want to maintain it yourself? Or have people > funnel them wherever XFS patches go? :) Well, the original version is from Dave, although I'm not sure if much of that code is left. I think just maintaining it with XFS is a good idea. But if we need an explicit maintainer I can do it.
Hi Linus, Here are hopefully the last set of fixes for 4.20. There's a fix for a longstanding statfs reporting problem with project quotas, a correction for page cache invalidation behaviors when fallocating near EOF, and a fix for a broken metadata verifier return code. Finally, the most important fix is to the pipe splicing code (aka the generic copy_file_range fallback) to avoid pointless short directio reads by only asking the filesystem for as much data as there are available pages in the pipe buffer. Our previous fix (simulated short directio reads because the number of pages didn't match the length of the read requested) caused subtle problems on overlayfs, so that part is reverted. Anyhow, this series passes fstests -g all on xfs and overlay+xfs, and has passed 17 billion fsx operations problem-free since I started testing; and merges cleanly with master as of last night, so please let me know if you encounter problems. --D The following changes since commit 8c110d43c6bca4b24dd13272a9d4e0ba6f2ec957: iomap: readpages doesn't zero page tail beyond EOF (2018-11-21 10:10:54 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git tags/xfs-4.20-fixes-3 for you to fetch changes up to 8f67b5adc030553fbc877124306f3f3bdab89aa8: iomap: partially revert 4721a601099 (simulated directio short read on EFAULT) (2018-12-04 09:40:02 -0800) ---------------------------------------------------------------- Changes since last update: - Fix broken project quota inode counts - Fix incorrect PAGE_MASK/PAGE_SIZE usage - Fix incorrect return value in btree verifier - Fix WARN_ON remap flags false positive - Fix splice read overflows ---------------------------------------------------------------- Darrick J. Wong (4): xfs: fix PAGE_MASK usage in xfs_free_file_space vfs: allow some remap flags to be passed to vfs_clone_file_range splice: don't read more than available pipe space iomap: partially revert 4721a601099 (simulated directio short read on EFAULT) Eric Sandeen (1): xfs: fix inverted return from xfs_btree_sblock_verify_crc Ye Yin (1): fs/xfs: fix f_ffree value for statfs when project quota is set fs/iomap.c | 9 --------- fs/read_write.c | 2 +- fs/splice.c | 7 ++++++- fs/xfs/libxfs/xfs_btree.c | 2 +- fs/xfs/xfs_bmap_util.c | 4 ++-- fs/xfs/xfs_qm_bhv.c | 2 +- 6 files changed, 11 insertions(+), 15 deletions(-)