[PATCH AUTOSEL 5.10 9/9] ext4: enable the lazy init thread when remounting read/write

[PATCH AUTOSEL 5.10 9/9] ext4: enable the lazy init thread when remounting read/write

[Sasha Levin]

From: Theodore Ts'o <tytso@mit.edu>

[ Upstream commit eb1f822c76beeaa76ab8b6737ab9dc9f9798408c ]

In commit a44be64bbecb ("ext4: don't clear SB_RDONLY when remounting
r/w until quota is re-enabled") we defer clearing tyhe SB_RDONLY flag
in struct super.  However, we didn't defer when we checked sb_rdonly()
to determine the lazy itable init thread should be enabled, with the
next result that the lazy inode table initialization would not be
properly started.  This can cause generic/231 to fail in ext4's
nojournal mode.

Fix this by moving when we decide to start or stop the lazy itable
init thread to after we clear the SB_RDONLY flag when we are
remounting the file system read/write.

Fixes a44be64bbecb ("ext4: don't clear SB_RDONLY when remounting r/w until...")

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Link: https://lore.kernel.org/r/20230527035729.1001605-1-tytso@mit.edu
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 fs/ext4/super.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index d89750e90bc4b..f72896384dbc9 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -6005,18 +6005,6 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
 		}
 	}
 
-	/*
-	 * Reinitialize lazy itable initialization thread based on
-	 * current settings
-	 */
-	if (sb_rdonly(sb) || !test_opt(sb, INIT_INODE_TABLE))
-		ext4_unregister_li_request(sb);
-	else {
-		ext4_group_t first_not_zeroed;
-		first_not_zeroed = ext4_has_uninit_itable(sb);
-		ext4_register_li_request(sb, first_not_zeroed);
-	}
-
 	/*
 	 * Handle creation of system zone data early because it can fail.
 	 * Releasing of existing data is done when we are sure remount will
@@ -6054,6 +6042,18 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
 	if (enable_rw)
 		sb->s_flags &= ~SB_RDONLY;
 
+	/*
+	 * Reinitialize lazy itable initialization thread based on
+	 * current settings
+	 */
+	if (sb_rdonly(sb) || !test_opt(sb, INIT_INODE_TABLE))
+		ext4_unregister_li_request(sb);
+	else {
+		ext4_group_t first_not_zeroed;
+		first_not_zeroed = ext4_has_uninit_itable(sb);
+		ext4_register_li_request(sb, first_not_zeroed);
+	}
+
 	if (!ext4_has_feature_mmp(sb) || sb_rdonly(sb))
 		ext4_stop_mmpd(sbi);
 
-- 
2.39.2

Re: [PATCH AUTOSEL 5.10 9/9] ext4: enable the lazy init thread when remounting read/write

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1337 bytes --]

Hi!

> [ Upstream commit eb1f822c76beeaa76ab8b6737ab9dc9f9798408c ]
> 
> In commit a44be64bbecb ("ext4: don't clear SB_RDONLY when remounting
> r/w until quota is re-enabled") we defer clearing tyhe SB_RDONLY flag
> in struct super.  However, we didn't defer when we checked sb_rdonly()
> to determine the lazy itable init thread should be enabled, with the
> next result that the lazy inode table initialization would not be
> properly started.  This can cause generic/231 to fail in ext4's
> nojournal mode.
> 
> Fix this by moving when we decide to start or stop the lazy itable
> init thread to after we clear the SB_RDONLY flag when we are
> remounting the file system read/write.
> 
> Fixes a44be64bbecb ("ext4: don't clear SB_RDONLY when remounting r/w until...")
> 
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> Link: https://lore.kernel.org/r/20230527035729.1001605-1-tytso@mit.edu
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> Signed-off-by: Sasha Levin <sashal@kernel.org>

Normally "Fixes" would be "Fixes:" and in the signed-off block. Plus,
two consecutive sign-offs from tytso are probably wrong, too.

Best regards,
								Pavel
								
-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH AUTOSEL 5.10 9/9] ext4: enable the lazy init thread when remounting read/write

[Sasha Levin]

On Fri, Jun 16, 2023 at 09:41:04PM +0200, Pavel Machek wrote:
>Hi!
>
>> [ Upstream commit eb1f822c76beeaa76ab8b6737ab9dc9f9798408c ]
>>
>> In commit a44be64bbecb ("ext4: don't clear SB_RDONLY when remounting
>> r/w until quota is re-enabled") we defer clearing tyhe SB_RDONLY flag
>> in struct super.  However, we didn't defer when we checked sb_rdonly()
>> to determine the lazy itable init thread should be enabled, with the
>> next result that the lazy inode table initialization would not be
>> properly started.  This can cause generic/231 to fail in ext4's
>> nojournal mode.
>>
>> Fix this by moving when we decide to start or stop the lazy itable
>> init thread to after we clear the SB_RDONLY flag when we are
>> remounting the file system read/write.
>>
>> Fixes a44be64bbecb ("ext4: don't clear SB_RDONLY when remounting r/w until...")
>>
>> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
>> Link: https://lore.kernel.org/r/20230527035729.1001605-1-tytso@mit.edu
>> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>
>Normally "Fixes" would be "Fixes:" and in the signed-off block. Plus,
>two consecutive sign-offs from tytso are probably wrong, too.

I'm really not sure what you want us to do here, or in other places
where you've commented about issues with the upstream patch...

-- 
Thanks,
Sasha

Re: [PATCH AUTOSEL 5.10 9/9] ext4: enable the lazy init thread when remounting read/write

[Theodore Ts'o]

On Tue, Jun 20, 2023 at 10:43:39AM -0400, Sasha Levin wrote:
> > Normally "Fixes" would be "Fixes:" and in the signed-off block. Plus,
> > two consecutive sign-offs from tytso are probably wrong, too.
> 
> I'm really not sure what you want us to do here, or in other places
> where you've commented about issues with the upstream patch...

Yeah, those were issues with the upstream commit.  Sorry about that.

I personally don't care about whether you "clean up" the commit before
having it land in the LTS kernels or leave it as it as originally
commited to upstream, but then again, I'm not the kind of person who
asks whether there's a hyphen in "anal-retentive".  (I'm given to
understand it depends on whether the phrase is being used as an
attributive or predicative adjective.  :-)

						- Ted