1. Patchew
  2. linux
  3. View series

[PATCH] jbd2: enforce power-of-two default revoke hash size at compile time

Milos Nikic posted 1 patch 2 months, 1 week ago 
fs/jbd2/journal.c | 1 +
1 file changed, 1 insertion(+)
[PATCH] jbd2: enforce power-of-two default revoke hash size at compile time
Posted by Milos Nikic 2 months, 1 week ago 
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
Re: [PATCH] jbd2: enforce power-of-two default revoke hash size at compile time
Posted by Jan Kara 2 months, 1 week ago 
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
Re: [PATCH] jbd2: enforce power-of-two default revoke hash size at compile time
Posted by Andreas Dilger 2 months ago 
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
Re: [PATCH] jbd2: enforce power-of-two default revoke hash size at compile time
Posted by Jan Kara 2 months ago 
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
Re: [PATCH] jbd2: enforce power-of-two default revoke hash size at compile time
Posted by Theodore Tso 2 months ago 
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

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.