| Message ID | 20210330134537.423447-1-joel@jms.id.au |
|---|---|
| State | New |
| Headers | show |
| Series | jffs2: Hook up splice_write callback | expand |
On Wed, Mar 31, 2021 at 12:15:37AM +1030, Joel Stanley wrote: > overlayfs using jffs2 as the upper filesystem would fail in some cases > since moving to v5.10. The test case used was to run 'touch' on a file > that exists in the lower fs, causing the modification time to be > updated. It returns EINVAL when the bug is triggered. > > A bisection showed this was introduced in v5.9-rc1, with commit > 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops"). > Reverting that commit restores the expected behaviour. > > Some digging showed that this was due to jffs2 lacking an implementation > of splice_write. (For unknown reasons the warn_unsupported that should > trigger was not displaying any output). > > Adding this patch resolved the issue and the test now passes. Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de> ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
On Tue, Mar 30, 2021 at 06:17:15PM +0200, Christoph Hellwig wrote: > On Wed, Mar 31, 2021 at 12:15:37AM +1030, Joel Stanley wrote: > > overlayfs using jffs2 as the upper filesystem would fail in some cases > > since moving to v5.10. The test case used was to run 'touch' on a file > > that exists in the lower fs, causing the modification time to be > > updated. It returns EINVAL when the bug is triggered. > > > > A bisection showed this was introduced in v5.9-rc1, with commit > > 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops"). > > Reverting that commit restores the expected behaviour. > > > > Some digging showed that this was due to jffs2 lacking an implementation > > of splice_write. (For unknown reasons the warn_unsupported that should > > trigger was not displaying any output). > > > > Adding this patch resolved the issue and the test now passes. > > Looks good: > > Reviewed-by: Christoph Hellwig <hch@lst.de> The same goes for quite a few other filesystems, actually - at least adfs, affs, bfs, hfs, hfsplus, hostfs, hpfs, minix, omfs, sysv, ufs and vboxsf are in the same boat, and I suspect that ecryptfs and ntfs might be too. Christoph, do you see any problems with doing the same thing for that bunch as well? ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
On Tue, Mar 30, 2021 at 06:31:00PM +0000, Al Viro wrote: > On Tue, Mar 30, 2021 at 06:17:15PM +0200, Christoph Hellwig wrote: > > On Wed, Mar 31, 2021 at 12:15:37AM +1030, Joel Stanley wrote: > > > overlayfs using jffs2 as the upper filesystem would fail in some cases > > > since moving to v5.10. The test case used was to run 'touch' on a file > > > that exists in the lower fs, causing the modification time to be > > > updated. It returns EINVAL when the bug is triggered. > > > > > > A bisection showed this was introduced in v5.9-rc1, with commit > > > 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops"). > > > Reverting that commit restores the expected behaviour. > > > > > > Some digging showed that this was due to jffs2 lacking an implementation > > > of splice_write. (For unknown reasons the warn_unsupported that should > > > trigger was not displaying any output). > > > > > > Adding this patch resolved the issue and the test now passes. > > > > Looks good: > > > > Reviewed-by: Christoph Hellwig <hch@lst.de> > > The same goes for quite a few other filesystems, actually - at least > adfs, affs, bfs, hfs, hfsplus, hostfs, hpfs, minix, omfs, sysv, ufs > and vboxsf are in the same boat, and I suspect that ecryptfs and ntfs > might be too. > > Christoph, do you see any problems with doing the same thing for that > bunch as well? coda and udf as well, by the look of it... ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
On Tue, Mar 30, 2021 at 9:45 PM Joel Stanley <joel@jms.id.au> wrote: > > overlayfs using jffs2 as the upper filesystem would fail in some cases > since moving to v5.10. The test case used was to run 'touch' on a file > that exists in the lower fs, causing the modification time to be > updated. It returns EINVAL when the bug is triggered. > > A bisection showed this was introduced in v5.9-rc1, with commit > 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops"). > Reverting that commit restores the expected behaviour. > > Some digging showed that this was due to jffs2 lacking an implementation > of splice_write. (For unknown reasons the warn_unsupported that should > trigger was not displaying any output). > > Adding this patch resolved the issue and the test now passes. > > Fixes: 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops") > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- > fs/jffs2/file.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c > index f8fb89b10227..4fc8cd698d1a 100644 > --- a/fs/jffs2/file.c > +++ b/fs/jffs2/file.c > @@ -57,6 +57,7 @@ const struct file_operations jffs2_file_operations = > .mmap = generic_file_readonly_mmap, > .fsync = jffs2_fsync, > .splice_read = generic_file_splice_read, > + .splice_write = iter_file_splice_write, > }; > > /* jffs2_file_inode_operations */ > -- > 2.30.2 > Verified on g220a openbmc system that the patch fixes the issue. Tested-by: Lei YU <yulei.sh@bytedance.com> ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
On Tue, Mar 30, 2021 at 06:31:00PM +0000, Al Viro wrote: > The same goes for quite a few other filesystems, actually - at least > adfs, affs, bfs, hfs, hfsplus, hostfs, hpfs, minix, omfs, sysv, ufs > and vboxsf are in the same boat, and I suspect that ecryptfs and ntfs > might be too. > > Christoph, do you see any problems with doing the same thing for that > bunch as well? Linus asked to only fix things up where we get reports, but I think his intent there was more random procfs and debugfs files rather than file systems. So just doing the sweep should be fine. ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c index f8fb89b10227..4fc8cd698d1a 100644 --- a/fs/jffs2/file.c +++ b/fs/jffs2/file.c @@ -57,6 +57,7 @@ const struct file_operations jffs2_file_operations = .mmap = generic_file_readonly_mmap, .fsync = jffs2_fsync, .splice_read = generic_file_splice_read, + .splice_write = iter_file_splice_write, }; /* jffs2_file_inode_operations */
overlayfs using jffs2 as the upper filesystem would fail in some cases since moving to v5.10. The test case used was to run 'touch' on a file that exists in the lower fs, causing the modification time to be updated. It returns EINVAL when the bug is triggered. A bisection showed this was introduced in v5.9-rc1, with commit 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops"). Reverting that commit restores the expected behaviour. Some digging showed that this was due to jffs2 lacking an implementation of splice_write. (For unknown reasons the warn_unsupported that should trigger was not displaying any output). Adding this patch resolved the issue and the test now passes. Fixes: 36e2c7421f02 ("fs: don't allow splice read/write without explicit ops") Signed-off-by: Joel Stanley <joel@jms.id.au> --- fs/jffs2/file.c | 1 + 1 file changed, 1 insertion(+) -- 2.30.2 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/