* [PATCH 1/3] ext4: avoid kernel warning when writing the superblock to a dead device @ 2018-12-31 19:48 Theodore Ts'o 2018-12-31 19:48 ` [PATCH 2/3] ext4: use ext4_write_inode() when fsyncing w/o a journal Theodore Ts'o ` (2 more replies) 0 siblings, 3 replies; 5+ messages in thread From: Theodore Ts'o @ 2018-12-31 19:48 UTC (permalink / raw) To: Ext4 Developers List; +Cc: Theodore Ts'o, stable The xfstests generic/475 test switches the underlying device with dm-error while running a stress test. This results in a large number of file system errors, and since we can't lock the buffer head when marking the superblock dirty in the ext4_grp_locked_error() case, it's possible the superblock to be !buffer_uptodate() without buffer_write_io_error() being true. We need to set buffer_uptodate() before we call mark_buffer_dirty() or this will trigger a WARN_ON. It's safe to do this since the superblock must have been properly read into memory or the mount would have been successful. So if buffer_uptodate() is not set, we can safely assume that this happened due to a failed attempt to write the superblock. Signed-off-by: Theodore Ts'o <tytso@mit.edu> Cc: stable@vger.kernel.org --- fs/ext4/super.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/ext4/super.c b/fs/ext4/super.c index d6c142d73d99..fb12d3c17c1b 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -4902,7 +4902,7 @@ static int ext4_commit_super(struct super_block *sb, int sync) ext4_superblock_csum_set(sb); if (sync) lock_buffer(sbh); - if (buffer_write_io_error(sbh)) { + if (buffer_write_io_error(sbh) || !buffer_uptodate(sbh)) { /* * Oh, dear. A previous attempt to write the * superblock failed. This could happen because the -- 2.19.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] ext4: use ext4_write_inode() when fsyncing w/o a journal 2018-12-31 19:48 [PATCH 1/3] ext4: avoid kernel warning when writing the superblock to a dead device Theodore Ts'o @ 2018-12-31 19:48 ` Theodore Ts'o 2019-01-23 11:34 ` Jan Kara 2018-12-31 19:48 ` [PATCH 3/3] ext4: track writeback errors using the generic tracking infrastructure Theodore Ts'o 2019-01-02 14:27 ` [PATCH 1/3] ext4: avoid kernel warning when writing the superblock to a dead device Sasha Levin 2 siblings, 1 reply; 5+ messages in thread From: Theodore Ts'o @ 2018-12-31 19:48 UTC (permalink / raw) To: Ext4 Developers List; +Cc: Theodore Ts'o, stable In no-journal mode, we previously used __generic_file_fsync() in no-journal mode. This triggers a lockdep warning, and in addition, it's not safe to depend on the inode writeback mechanism in the case ext4. We can solve both problems by calling ext4_write_inode() directly. Signed-off-by: Theodore Ts'o <tytso@mit.edu> Cc: stable@kernel.org --- fs/ext4/fsync.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/fs/ext4/fsync.c b/fs/ext4/fsync.c index 26a7fe5c4fd3..87a7ff00ef62 100644 --- a/fs/ext4/fsync.c +++ b/fs/ext4/fsync.c @@ -116,8 +116,16 @@ int ext4_sync_file(struct file *file, loff_t start, loff_t end, int datasync) goto out; } + ret = file_write_and_wait_range(file, start, end); + if (ret) + return ret; + if (!journal) { - ret = __generic_file_fsync(file, start, end, datasync); + struct writeback_control wbc = { + .sync_mode = WB_SYNC_ALL + }; + + ret = ext4_write_inode(inode, &wbc); if (!ret) ret = ext4_sync_parent(inode); if (test_opt(inode->i_sb, BARRIER)) @@ -125,9 +133,6 @@ int ext4_sync_file(struct file *file, loff_t start, loff_t end, int datasync) goto out; } - ret = file_write_and_wait_range(file, start, end); - if (ret) - return ret; /* * data=writeback,ordered: * The caller's filemap_fdatawrite()/wait will sync the data. -- 2.19.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/3] ext4: use ext4_write_inode() when fsyncing w/o a journal 2018-12-31 19:48 ` [PATCH 2/3] ext4: use ext4_write_inode() when fsyncing w/o a journal Theodore Ts'o @ 2019-01-23 11:34 ` Jan Kara 0 siblings, 0 replies; 5+ messages in thread From: Jan Kara @ 2019-01-23 11:34 UTC (permalink / raw) To: Theodore Ts'o; +Cc: Ext4 Developers List, stable On Mon 31-12-18 14:48:35, Theodore Ts'o wrote: > In no-journal mode, we previously used __generic_file_fsync() in > no-journal mode. This triggers a lockdep warning, and in addition, > it's not safe to depend on the inode writeback mechanism in the case > ext4. We can solve both problems by calling ext4_write_inode() > directly. > > Signed-off-by: Theodore Ts'o <tytso@mit.edu> > Cc: stable@kernel.org > --- > fs/ext4/fsync.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/fs/ext4/fsync.c b/fs/ext4/fsync.c > index 26a7fe5c4fd3..87a7ff00ef62 100644 > --- a/fs/ext4/fsync.c > +++ b/fs/ext4/fsync.c > @@ -116,8 +116,16 @@ int ext4_sync_file(struct file *file, loff_t start, loff_t end, int datasync) > goto out; > } > > + ret = file_write_and_wait_range(file, start, end); > + if (ret) > + return ret; > + > if (!journal) { > - ret = __generic_file_fsync(file, start, end, datasync); > + struct writeback_control wbc = { > + .sync_mode = WB_SYNC_ALL > + }; > + > + ret = ext4_write_inode(inode, &wbc); Going through some older email... How come this is safe Ted? ext4_write_inode() will not write out metadata buffers associated with the inode (unlike __generic_file_fsync() which calls sync_mapping_buffers()). So you probably need to call sync_mapping_buffers() before calling ext4_write_inode() here? Honza > @@ -125,9 +133,6 @@ int ext4_sync_file(struct file *file, loff_t start, loff_t end, int datasync) > goto out; > } > > - ret = file_write_and_wait_range(file, start, end); > - if (ret) > - return ret; > /* > * data=writeback,ordered: > * The caller's filemap_fdatawrite()/wait will sync the data. > -- > 2.19.1 > -- Jan Kara <jack@suse.com> SUSE Labs, CR ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 3/3] ext4: track writeback errors using the generic tracking infrastructure 2018-12-31 19:48 [PATCH 1/3] ext4: avoid kernel warning when writing the superblock to a dead device Theodore Ts'o 2018-12-31 19:48 ` [PATCH 2/3] ext4: use ext4_write_inode() when fsyncing w/o a journal Theodore Ts'o @ 2018-12-31 19:48 ` Theodore Ts'o 2019-01-02 14:27 ` [PATCH 1/3] ext4: avoid kernel warning when writing the superblock to a dead device Sasha Levin 2 siblings, 0 replies; 5+ messages in thread From: Theodore Ts'o @ 2018-12-31 19:48 UTC (permalink / raw) To: Ext4 Developers List; +Cc: Theodore Ts'o, stable We already using mapping_set_error() in fs/ext4/page_io.c, so all we need to do is to use file_check_and_advance_wb_err() when handling fsync() requests in ext4_sync_file(). Signed-off-by: Theodore Ts'o <tytso@mit.edu> Cc: stable@kernel.org --- fs/ext4/fsync.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/ext4/fsync.c b/fs/ext4/fsync.c index 87a7ff00ef62..712f00995390 100644 --- a/fs/ext4/fsync.c +++ b/fs/ext4/fsync.c @@ -164,6 +164,9 @@ int ext4_sync_file(struct file *file, loff_t start, loff_t end, int datasync) ret = err; } out: + err = file_check_and_advance_wb_err(file); + if (ret == 0) + ret = err; trace_ext4_sync_file_exit(inode, ret); return ret; } -- 2.19.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] ext4: avoid kernel warning when writing the superblock to a dead device 2018-12-31 19:48 [PATCH 1/3] ext4: avoid kernel warning when writing the superblock to a dead device Theodore Ts'o 2018-12-31 19:48 ` [PATCH 2/3] ext4: use ext4_write_inode() when fsyncing w/o a journal Theodore Ts'o 2018-12-31 19:48 ` [PATCH 3/3] ext4: track writeback errors using the generic tracking infrastructure Theodore Ts'o @ 2019-01-02 14:27 ` Sasha Levin 2 siblings, 0 replies; 5+ messages in thread From: Sasha Levin @ 2019-01-02 14:27 UTC (permalink / raw) To: Sasha Levin, Theodore Ts'o, Ext4 Developers List Cc: Theodore Ts'o, stable, stable, stable Hi, [This is an automated email] This commit has been processed because it contains a -stable tag. The stable tag indicates that it's relevant for the following trees: all The bot has tested the following trees: v4.20.0, v4.19.13, v4.14.91, v4.9.148, v4.4.169, v3.18.131, v4.20.0: Build OK! v4.19.13: Build OK! v4.14.91: Build OK! v4.9.148: Build OK! v4.4.169: Failed to apply! Possible dependencies: 1566a48aaa10 ("ext4: don't lock buffer in ext4_commit_super if holding spinlock") 4743f8399061 ("ext4: Fix WARN_ON_ONCE in ext4_commit_super()") v3.18.131: Failed to apply! Possible dependencies: 1566a48aaa10 ("ext4: don't lock buffer in ext4_commit_super if holding spinlock") 4743f8399061 ("ext4: Fix WARN_ON_ONCE in ext4_commit_super()") 564bc402526e ("ext4, jbd2: add REQ_FUA flag when recording an error in the superblock") How should we proceed with this patch? -- Thanks, Sasha ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-01-23 11:35 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-31 19:48 [PATCH 1/3] ext4: avoid kernel warning when writing the superblock to a dead device Theodore Ts'o 2018-12-31 19:48 ` [PATCH 2/3] ext4: use ext4_write_inode() when fsyncing w/o a journal Theodore Ts'o 2019-01-23 11:34 ` Jan Kara 2018-12-31 19:48 ` [PATCH 3/3] ext4: track writeback errors using the generic tracking infrastructure Theodore Ts'o 2019-01-02 14:27 ` [PATCH 1/3] ext4: avoid kernel warning when writing the superblock to a dead device Sasha Levin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.