public inbox for linux-ext4@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] jbd2: enforce power-of-two default revoke hash size at compile time
@ 2026-04-13 21:27 Milos Nikic
  2026-04-14 12:59 ` Jan Kara
  0 siblings, 1 reply; 5+ messages in thread
From: Milos Nikic @ 2026-04-13 21:27 UTC (permalink / raw)
  To: jack, tytso, linux-ext4; +Cc: linux-kernel, Milos Nikic

The jbd2 revoke table relies on bitwise AND operations for fast hash
indexing, which requires the hash table size to be a strict power of two.

Currently, this requirement is only enforced at runtime via a J_ASSERT
in jbd2_journal_init_revoke(). While this successfully catches invalid
dynamic allocations, it means a developer accidentally modifying the
hardcoded JOURNAL_REVOKE_DEFAULT_HASH macro will experience a system
panic upon mounting the filesystem during testing.

Add a BUILD_BUG_ON() in journal_init_common() to validate the default
macro at compile time. This acts as an immediate, zero-overhead
safeguard, preventing compilation entirely if the default hash size is
mathematically invalid.

Signed-off-by: Milos Nikic <nikic.milos@gmail.com>
---
 fs/jbd2/journal.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index 4f397fcdb13c..62b36a2fc4e2 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1565,6 +1565,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
 	/* The journal is marked for error until we succeed with recovery! */
 	journal->j_flags = JBD2_ABORT;
 
+	BUILD_BUG_ON(!is_power_of_2(JOURNAL_REVOKE_DEFAULT_HASH));
 	/* Set up a default-sized revoke table for the new mount. */
 	err = jbd2_journal_init_revoke(journal, JOURNAL_REVOKE_DEFAULT_HASH);
 	if (err)
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] jbd2: enforce power-of-two default revoke hash size at compile time
  2026-04-13 21:27 [PATCH] jbd2: enforce power-of-two default revoke hash size at compile time Milos Nikic
@ 2026-04-14 12:59 ` Jan Kara
  2026-04-16  1:35   ` Andreas Dilger
  0 siblings, 1 reply; 5+ messages in thread
From: Jan Kara @ 2026-04-14 12:59 UTC (permalink / raw)
  To: Milos Nikic; +Cc: jack, tytso, linux-ext4, linux-kernel

On Mon 13-04-26 14:27:24, Milos Nikic wrote:
> The jbd2 revoke table relies on bitwise AND operations for fast hash
> indexing, which requires the hash table size to be a strict power of two.
> 
> Currently, this requirement is only enforced at runtime via a J_ASSERT
> in jbd2_journal_init_revoke(). While this successfully catches invalid
> dynamic allocations, it means a developer accidentally modifying the
> hardcoded JOURNAL_REVOKE_DEFAULT_HASH macro will experience a system
> panic upon mounting the filesystem during testing.
> 
> Add a BUILD_BUG_ON() in journal_init_common() to validate the default
> macro at compile time. This acts as an immediate, zero-overhead
> safeguard, preventing compilation entirely if the default hash size is
> mathematically invalid.
> 
> Signed-off-by: Milos Nikic <nikic.milos@gmail.com>

Eh, if you modify JOURNAL_REVOKE_DEFAULT_HASH you should better know what
you are doing and if you mess up, then the kernel failing with assertion
isn't that difficult to diagnose. So sorry I don't think this "cleanup" is
useful either.

								Honza

> ---
>  fs/jbd2/journal.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index 4f397fcdb13c..62b36a2fc4e2 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -1565,6 +1565,7 @@ static journal_t *journal_init_common(struct block_device *bdev,
>  	/* The journal is marked for error until we succeed with recovery! */
>  	journal->j_flags = JBD2_ABORT;
>  
> +	BUILD_BUG_ON(!is_power_of_2(JOURNAL_REVOKE_DEFAULT_HASH));
>  	/* Set up a default-sized revoke table for the new mount. */
>  	err = jbd2_journal_init_revoke(journal, JOURNAL_REVOKE_DEFAULT_HASH);
>  	if (err)
> -- 
> 2.53.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] jbd2: enforce power-of-two default revoke hash size at compile time
  2026-04-14 12:59 ` Jan Kara
@ 2026-04-16  1:35   ` Andreas Dilger
  2026-04-16 10:16     ` Jan Kara
  0 siblings, 1 reply; 5+ messages in thread
From: Andreas Dilger @ 2026-04-16  1:35 UTC (permalink / raw)
  To: Jan Kara; +Cc: Milos Nikic, tytso, linux-ext4, linux-kernel

On Apr 14, 2026, at 06:59, Jan Kara <jack@suse.cz> wrote:
> 
> On Mon 13-04-26 14:27:24, Milos Nikic wrote:
>> The jbd2 revoke table relies on bitwise AND operations for fast hash
>> indexing, which requires the hash table size to be a strict power of two.
>> 
>> Currently, this requirement is only enforced at runtime via a J_ASSERT
>> in jbd2_journal_init_revoke(). While this successfully catches invalid
>> dynamic allocations, it means a developer accidentally modifying the
>> hardcoded JOURNAL_REVOKE_DEFAULT_HASH macro will experience a system
>> panic upon mounting the filesystem during testing.
>> 
>> Add a BUILD_BUG_ON() in journal_init_common() to validate the default
>> macro at compile time. This acts as an immediate, zero-overhead
>> safeguard, preventing compilation entirely if the default hash size is
>> mathematically invalid.
>> 
>> Signed-off-by: Milos Nikic <nikic.milos@gmail.com>
> 
> Eh, if you modify JOURNAL_REVOKE_DEFAULT_HASH you should better know what
> you are doing and if you mess up, then the kernel failing with assertion
> isn't that difficult to diagnose. So sorry I don't think this "cleanup" is
> useful either.

Jan,
this is a BUILD_BUG_ON() so it won't cause any runtime assertion.

Cheers, Andreas 

> Honza
> 
>> ---
>> fs/jbd2/journal.c | 1 +
>> 1 file changed, 1 insertion(+)
>> 
>> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
>> index 4f397fcdb13c..62b36a2fc4e2 100644
>> --- a/fs/jbd2/journal.c
>> +++ b/fs/jbd2/journal.c
>> @@ -1565,6 +1565,7 @@ static journal_t *journal_init_common(struct 
>>  	/* The journal is marked for error until we succeed with recovery! */
>>  	journal->j_flags = JBD2_ABORT;
>> 
>> +	BUILD_BUG_ON(!is_power_of_2(JOURNAL_REVOKE_DEFAULT_HASH));
>>  	/* Set up a default-sized revoke table for the new mount. */
>>  	err = jbd2_journal_init_revoke(journal, JOURNAL_REVOKE_DEFAULT_HASH);
>>  	if (err)
>> -- 
>> 2.53.0
>> 
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
> 


Cheers, Andreas






^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] jbd2: enforce power-of-two default revoke hash size at compile time
  2026-04-16  1:35   ` Andreas Dilger
@ 2026-04-16 10:16     ` Jan Kara
  2026-04-16 11:54       ` Theodore Tso
  0 siblings, 1 reply; 5+ messages in thread
From: Jan Kara @ 2026-04-16 10:16 UTC (permalink / raw)
  To: Andreas Dilger; +Cc: Jan Kara, Milos Nikic, tytso, linux-ext4, linux-kernel

Hi!

On Wed 15-04-26 19:35:21, Andreas Dilger wrote:
> On Apr 14, 2026, at 06:59, Jan Kara <jack@suse.cz> wrote:
> > On Mon 13-04-26 14:27:24, Milos Nikic wrote:
> >> The jbd2 revoke table relies on bitwise AND operations for fast hash
> >> indexing, which requires the hash table size to be a strict power of two.
> >> 
> >> Currently, this requirement is only enforced at runtime via a J_ASSERT
> >> in jbd2_journal_init_revoke(). While this successfully catches invalid
> >> dynamic allocations, it means a developer accidentally modifying the
> >> hardcoded JOURNAL_REVOKE_DEFAULT_HASH macro will experience a system
> >> panic upon mounting the filesystem during testing.
> >> 
> >> Add a BUILD_BUG_ON() in journal_init_common() to validate the default
> >> macro at compile time. This acts as an immediate, zero-overhead
> >> safeguard, preventing compilation entirely if the default hash size is
> >> mathematically invalid.
> >> 
> >> Signed-off-by: Milos Nikic <nikic.milos@gmail.com>
> > 
> > Eh, if you modify JOURNAL_REVOKE_DEFAULT_HASH you should better know what
> > you are doing and if you mess up, then the kernel failing with assertion
> > isn't that difficult to diagnose. So sorry I don't think this "cleanup" is
> > useful either.
> 
> Jan,
> this is a BUILD_BUG_ON() so it won't cause any runtime assertion.

Yes, I know. But there is already a runtime assertion in
jbd2_journal_init_revoke() making sure the passed value is a power of two
which is verifying also other callers that aren't using
JOURNAL_REVOKE_DEFAULT_HASH value. I don't see a value in the *additional*
BUILD_BUG_ON when we already have that runtime check.

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] jbd2: enforce power-of-two default revoke hash size at compile time
  2026-04-16 10:16     ` Jan Kara
@ 2026-04-16 11:54       ` Theodore Tso
  0 siblings, 0 replies; 5+ messages in thread
From: Theodore Tso @ 2026-04-16 11:54 UTC (permalink / raw)
  To: Jan Kara; +Cc: Andreas Dilger, Milos Nikic, linux-ext4, linux-kernel

On Thu, Apr 16, 2026 at 12:16:05PM +0200, Jan Kara wrote:
> 
> Yes, I know. But there is already a runtime assertion in
> jbd2_journal_init_revoke() making sure the passed value is a power of two
> which is verifying also other callers that aren't using
> JOURNAL_REVOKE_DEFAULT_HASH value. I don't see a value in the *additional*
> BUILD_BUG_ON when we already have that runtime check.

... and if a developer doesn't run regression tests before bothering
the list with a patch... that's not a developer we want to cater to.

Milos, my recommended to workflow is to use kvm-xfstests[1], and just do:

% install-kconfig
% kbuild
% kvm-xfstests smoke

[1] https://github.com/tytso/xfstests-bld/blob/master/Documentation/kvm-quickstart.md

For a successful run, "kvm-xfstests smoke" will run take 15-20
minutes.  In the case of screwing up the default power-of-two default
revoke hash size, "kvm-xfstests smoke" will report a failure in less
than a minute.

						- Ted

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-04-16 11:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-13 21:27 [PATCH] jbd2: enforce power-of-two default revoke hash size at compile time Milos Nikic
2026-04-14 12:59 ` Jan Kara
2026-04-16  1:35   ` Andreas Dilger
2026-04-16 10:16     ` Jan Kara
2026-04-16 11:54       ` Theodore Tso

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox