* [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).