* [PATCH AUTOSEL 4.4 12/37] jbd2: Fix statistics for the number of logged blocks [not found] <20191211153813.24126-1-sashal@kernel.org> @ 2019-12-11 15:37 ` Sasha Levin 2019-12-11 15:38 ` [PATCH AUTOSEL 4.4 27/37] ext4: work around deleting a file with i_nlink == 0 safely Sasha Levin 1 sibling, 0 replies; 7+ messages in thread From: Sasha Levin @ 2019-12-11 15:37 UTC (permalink / raw) To: linux-kernel, stable; +Cc: Jan Kara, Theodore Ts'o, Sasha Levin, linux-ext4 From: Jan Kara <jack@suse.cz> [ Upstream commit 015c6033068208d6227612c878877919f3fcf6b6 ] jbd2 statistics counting number of blocks logged in a transaction was wrong. It didn't count the commit block and more importantly it didn't count revoke descriptor blocks. Make sure these get properly counted. Reviewed-by: Theodore Ts'o <tytso@mit.edu> Signed-off-by: Jan Kara <jack@suse.cz> Link: https://lore.kernel.org/r/20191105164437.32602-13-jack@suse.cz Signed-off-by: Theodore Ts'o <tytso@mit.edu> Signed-off-by: Sasha Levin <sashal@kernel.org> --- fs/jbd2/commit.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c index 2d964ce456060..ebbd7d054cabd 100644 --- a/fs/jbd2/commit.c +++ b/fs/jbd2/commit.c @@ -740,7 +740,6 @@ start_journal_io: submit_bh(WRITE_SYNC, bh); } cond_resched(); - stats.run.rs_blocks_logged += bufs; /* Force a new descriptor to be generated next time round the loop. */ @@ -827,6 +826,7 @@ start_journal_io: if (unlikely(!buffer_uptodate(bh))) err = -EIO; jbd2_unfile_log_bh(bh); + stats.run.rs_blocks_logged++; /* * The list contains temporary buffer heads created by @@ -872,6 +872,7 @@ start_journal_io: BUFFER_TRACE(bh, "ph5: control buffer writeout done: unfile"); clear_buffer_jwrite(bh); jbd2_unfile_log_bh(bh); + stats.run.rs_blocks_logged++; __brelse(bh); /* One for getblk */ /* AKPM: bforget here */ } @@ -893,6 +894,7 @@ start_journal_io: } if (cbh) err = journal_wait_on_commit_record(journal, cbh); + stats.run.rs_blocks_logged++; if (jbd2_has_feature_async_commit(journal) && journal->j_flags & JBD2_BARRIER) { blkdev_issue_flush(journal->j_dev, GFP_NOFS, NULL); -- 2.20.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH AUTOSEL 4.4 27/37] ext4: work around deleting a file with i_nlink == 0 safely [not found] <20191211153813.24126-1-sashal@kernel.org> 2019-12-11 15:37 ` [PATCH AUTOSEL 4.4 12/37] jbd2: Fix statistics for the number of logged blocks Sasha Levin @ 2019-12-11 15:38 ` Sasha Levin 2019-12-11 16:19 ` Theodore Y. Ts'o 1 sibling, 1 reply; 7+ messages in thread From: Sasha Levin @ 2019-12-11 15:38 UTC (permalink / raw) To: linux-kernel, stable Cc: Theodore Ts'o, stable, Andreas Dilger, Sasha Levin, linux-ext4 From: Theodore Ts'o <tytso@mit.edu> [ Upstream commit c7df4a1ecb8579838ec8c56b2bb6a6716e974f37 ] If the file system is corrupted such that a file's i_links_count is too small, then it's possible that when unlinking that file, i_nlink will already be zero. Previously we were working around this kind of corruption by forcing i_nlink to one; but we were doing this before trying to delete the directory entry --- and if the file system is corrupted enough that ext4_delete_entry() fails, then we exit with i_nlink elevated, and this causes the orphan inode list handling to be FUBAR'ed, such that when we unmount the file system, the orphan inode list can get corrupted. A better way to fix this is to simply skip trying to call drop_nlink() if i_nlink is already zero, thus moving the check to the place where it makes the most sense. https://bugzilla.kernel.org/show_bug.cgi?id=205433 Link: https://lore.kernel.org/r/20191112032903.8828-1-tytso@mit.edu Signed-off-by: Theodore Ts'o <tytso@mit.edu> Cc: stable@kernel.org Reviewed-by: Andreas Dilger <adilger@dilger.ca> Signed-off-by: Sasha Levin <sashal@kernel.org> --- fs/ext4/namei.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index aa08e129149dc..712bf332e3948 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -3040,18 +3040,17 @@ static int ext4_unlink(struct inode *dir, struct dentry *dentry) if (IS_DIRSYNC(dir)) ext4_handle_sync(handle); - if (inode->i_nlink == 0) { - ext4_warning_inode(inode, "Deleting file '%.*s' with no links", - dentry->d_name.len, dentry->d_name.name); - set_nlink(inode, 1); - } retval = ext4_delete_entry(handle, dir, de, bh); if (retval) goto end_unlink; dir->i_ctime = dir->i_mtime = ext4_current_time(dir); ext4_update_dx_flag(dir); ext4_mark_inode_dirty(handle, dir); - drop_nlink(inode); + if (inode->i_nlink == 0) + ext4_warning_inode(inode, "Deleting file '%.*s' with no links", + dentry->d_name.len, dentry->d_name.name); + else + drop_nlink(inode); if (!inode->i_nlink) ext4_orphan_add(handle, inode); inode->i_ctime = ext4_current_time(inode); -- 2.20.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH AUTOSEL 4.4 27/37] ext4: work around deleting a file with i_nlink == 0 safely 2019-12-11 15:38 ` [PATCH AUTOSEL 4.4 27/37] ext4: work around deleting a file with i_nlink == 0 safely Sasha Levin @ 2019-12-11 16:19 ` Theodore Y. Ts'o 2019-12-11 18:25 ` Greg KH 2019-12-11 20:04 ` Sasha Levin 0 siblings, 2 replies; 7+ messages in thread From: Theodore Y. Ts'o @ 2019-12-11 16:19 UTC (permalink / raw) To: Sasha Levin; +Cc: linux-kernel, stable, stable, Andreas Dilger, linux-ext4 On Wed, Dec 11, 2019 at 10:38:03AM -0500, Sasha Levin wrote: > From: Theodore Ts'o <tytso@mit.edu> > > [ Upstream commit c7df4a1ecb8579838ec8c56b2bb6a6716e974f37 ] > > If the file system is corrupted such that a file's i_links_count is > too small, then it's possible that when unlinking that file, i_nlink > will already be zero. Previously we were working around this kind of > corruption by forcing i_nlink to one; but we were doing this before > trying to delete the directory entry --- and if the file system is > corrupted enough that ext4_delete_entry() fails, then we exit with > i_nlink elevated, and this causes the orphan inode list handling to be > FUBAR'ed, such that when we unmount the file system, the orphan inode > list can get corrupted. > > A better way to fix this is to simply skip trying to call drop_nlink() > if i_nlink is already zero, thus moving the check to the place where > it makes the most sense. > > https://bugzilla.kernel.org/show_bug.cgi?id=205433 > > Link: https://lore.kernel.org/r/20191112032903.8828-1-tytso@mit.edu > Signed-off-by: Theodore Ts'o <tytso@mit.edu> > Cc: stable@kernel.org > Reviewed-by: Andreas Dilger <adilger@dilger.ca> > Signed-off-by: Sasha Levin <sashal@kernel.org> I'm confused; this was explicitly cc'ed to stable@kernel.org, so why is your AUTOSEL picking this up? I would have thought this would get picked up via the normal stable kernel processes. Thanks, - Ted ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH AUTOSEL 4.4 27/37] ext4: work around deleting a file with i_nlink == 0 safely 2019-12-11 16:19 ` Theodore Y. Ts'o @ 2019-12-11 18:25 ` Greg KH 2019-12-11 20:04 ` Sasha Levin 1 sibling, 0 replies; 7+ messages in thread From: Greg KH @ 2019-12-11 18:25 UTC (permalink / raw) To: Theodore Y. Ts'o Cc: Sasha Levin, linux-kernel, stable, stable, Andreas Dilger, linux-ext4 On Wed, Dec 11, 2019 at 11:19:59AM -0500, Theodore Y. Ts'o wrote: > On Wed, Dec 11, 2019 at 10:38:03AM -0500, Sasha Levin wrote: > > From: Theodore Ts'o <tytso@mit.edu> > > > > [ Upstream commit c7df4a1ecb8579838ec8c56b2bb6a6716e974f37 ] > > > > If the file system is corrupted such that a file's i_links_count is > > too small, then it's possible that when unlinking that file, i_nlink > > will already be zero. Previously we were working around this kind of > > corruption by forcing i_nlink to one; but we were doing this before > > trying to delete the directory entry --- and if the file system is > > corrupted enough that ext4_delete_entry() fails, then we exit with > > i_nlink elevated, and this causes the orphan inode list handling to be > > FUBAR'ed, such that when we unmount the file system, the orphan inode > > list can get corrupted. > > > > A better way to fix this is to simply skip trying to call drop_nlink() > > if i_nlink is already zero, thus moving the check to the place where > > it makes the most sense. > > > > https://bugzilla.kernel.org/show_bug.cgi?id=205433 > > > > Link: https://lore.kernel.org/r/20191112032903.8828-1-tytso@mit.edu > > Signed-off-by: Theodore Ts'o <tytso@mit.edu> > > Cc: stable@kernel.org > > Reviewed-by: Andreas Dilger <adilger@dilger.ca> > > Signed-off-by: Sasha Levin <sashal@kernel.org> > > I'm confused; this was explicitly cc'ed to stable@kernel.org, so why > is your AUTOSEL picking this up? I would have thought this would get > picked up via the normal stable kernel processes. Perhaps because it is still in my queue, I haven't gotten there yet, but will do so after this next round of -rcs are released. thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH AUTOSEL 4.4 27/37] ext4: work around deleting a file with i_nlink == 0 safely 2019-12-11 16:19 ` Theodore Y. Ts'o 2019-12-11 18:25 ` Greg KH @ 2019-12-11 20:04 ` Sasha Levin 2019-12-12 15:17 ` Theodore Y. Ts'o 1 sibling, 1 reply; 7+ messages in thread From: Sasha Levin @ 2019-12-11 20:04 UTC (permalink / raw) To: Theodore Y. Ts'o Cc: linux-kernel, stable, stable, Andreas Dilger, linux-ext4 On Wed, Dec 11, 2019 at 11:19:59AM -0500, Theodore Y. Ts'o wrote: >On Wed, Dec 11, 2019 at 10:38:03AM -0500, Sasha Levin wrote: >> From: Theodore Ts'o <tytso@mit.edu> >> >> [ Upstream commit c7df4a1ecb8579838ec8c56b2bb6a6716e974f37 ] >> >> If the file system is corrupted such that a file's i_links_count is >> too small, then it's possible that when unlinking that file, i_nlink >> will already be zero. Previously we were working around this kind of >> corruption by forcing i_nlink to one; but we were doing this before >> trying to delete the directory entry --- and if the file system is >> corrupted enough that ext4_delete_entry() fails, then we exit with >> i_nlink elevated, and this causes the orphan inode list handling to be >> FUBAR'ed, such that when we unmount the file system, the orphan inode >> list can get corrupted. >> >> A better way to fix this is to simply skip trying to call drop_nlink() >> if i_nlink is already zero, thus moving the check to the place where >> it makes the most sense. >> >> https://bugzilla.kernel.org/show_bug.cgi?id=205433 >> >> Link: https://lore.kernel.org/r/20191112032903.8828-1-tytso@mit.edu >> Signed-off-by: Theodore Ts'o <tytso@mit.edu> >> Cc: stable@kernel.org >> Reviewed-by: Andreas Dilger <adilger@dilger.ca> >> Signed-off-by: Sasha Levin <sashal@kernel.org> > >I'm confused; this was explicitly cc'ed to stable@kernel.org, so why >is your AUTOSEL picking this up? I would have thought this would get >picked up via the normal stable kernel processes. My mistake, appologies. -- Thanks, Sasha ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH AUTOSEL 4.4 27/37] ext4: work around deleting a file with i_nlink == 0 safely 2019-12-11 20:04 ` Sasha Levin @ 2019-12-12 15:17 ` Theodore Y. Ts'o 2019-12-13 0:54 ` Sasha Levin 0 siblings, 1 reply; 7+ messages in thread From: Theodore Y. Ts'o @ 2019-12-12 15:17 UTC (permalink / raw) To: Sasha Levin; +Cc: linux-kernel, stable, stable, Andreas Dilger, linux-ext4 On Wed, Dec 11, 2019 at 03:04:54PM -0500, Sasha Levin wrote: > > I'm confused; this was explicitly cc'ed to stable@kernel.org, so why > > is your AUTOSEL picking this up? I would have thought this would get > > picked up via the normal stable kernel processes. > > My mistake, appologies. No worries; the intent was that it be backported to stable, and I don't really care with path it takes. I just wanted to make sure there wouldn't be confusion if you backported it to stable, and then Greg tried and then got a merge conflict. (Or worse, if the patch was one of the ones where it can be successfully applied *twice* w/o a patch conflict; I'm not sure if git cherry-pick is smarter than patch in this regard, but I don't think it is?) - Ted ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH AUTOSEL 4.4 27/37] ext4: work around deleting a file with i_nlink == 0 safely 2019-12-12 15:17 ` Theodore Y. Ts'o @ 2019-12-13 0:54 ` Sasha Levin 0 siblings, 0 replies; 7+ messages in thread From: Sasha Levin @ 2019-12-13 0:54 UTC (permalink / raw) To: Theodore Y. Ts'o Cc: linux-kernel, stable, stable, Andreas Dilger, linux-ext4 On Thu, Dec 12, 2019 at 10:17:06AM -0500, Theodore Y. Ts'o wrote: >On Wed, Dec 11, 2019 at 03:04:54PM -0500, Sasha Levin wrote: >> > I'm confused; this was explicitly cc'ed to stable@kernel.org, so why >> > is your AUTOSEL picking this up? I would have thought this would get >> > picked up via the normal stable kernel processes. >> >> My mistake, appologies. > >No worries; the intent was that it be backported to stable, and I >don't really care with path it takes. > >I just wanted to make sure there wouldn't be confusion if you >backported it to stable, and then Greg tried and then got a merge >conflict. (Or worse, if the patch was one of the ones where it can be >successfully applied *twice* w/o a patch conflict; I'm not sure if git >cherry-pick is smarter than patch in this regard, but I don't think it >is?) This one was just due to me running a bit faster then usual. I generally don't filter out stable tagged commits and Greg just gets to them faster than me (the delay on AUTOSEL is bigger than stable tagged commits). -- Thanks, Sasha ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-12-13 0:54 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20191211153813.24126-1-sashal@kernel.org> 2019-12-11 15:37 ` [PATCH AUTOSEL 4.4 12/37] jbd2: Fix statistics for the number of logged blocks Sasha Levin 2019-12-11 15:38 ` [PATCH AUTOSEL 4.4 27/37] ext4: work around deleting a file with i_nlink == 0 safely Sasha Levin 2019-12-11 16:19 ` Theodore Y. Ts'o 2019-12-11 18:25 ` Greg KH 2019-12-11 20:04 ` Sasha Levin 2019-12-12 15:17 ` Theodore Y. Ts'o 2019-12-13 0:54 ` Sasha Levin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).