Re: [PATCH v3] ext4: move orphan tracking away from journal state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Moving `s_orphan_lock` introduces new false sharing with the highly-contended `s_es_lock`.
--

commit 269f37a627203c8f7267166bbe89e9f2dfb71db7
Author: Jonas Zhou <jonaszhou@zhaoxin.com>

ext4: move orphan tracking away from journal state

Move s_orphan_lock, s_orphan and s_orphan_info together below
s_journal_triggers to separate orphan-list writes from journal
and flag readers.

Link: https://lore.kernel.org/all/575wyoks3ldokhrg6v43rxwsegaecod4bgrpbii6jeczy7czbi@a3yq26u52orp/

> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 724a27e8be613..1669109149731 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h

[ ... ]

> @@ -1786,6 +1782,12 @@ struct ext4_sb_info {
>  	/* Journal triggers for checksum computation */
>  	struct ext4_journal_trigger s_journal_triggers[EXT4_JOURNAL_TRIGGER_COUNT];
>  
> +	/* Orphan inode tracking */
> +	struct mutex s_orphan_lock;	/* Protects on disk list changes */

[Severity: Medium]
Does moving s_orphan_lock here introduce false sharing with s_es_lock?

On 64-bit architectures, s_es_lock (4 bytes), pointer alignment padding
(4 bytes), s_journal_triggers (24 bytes for a single 1-element array), and
s_orphan_lock (32 bytes) perfectly sum to 64 bytes. This places s_orphan_lock
exactly in the same 64-byte cacheline as s_es_lock.

This can cause cacheline bouncing during concurrent operations modifying extent
status trees and orphan lists, such as parallel unlinks.

> +	struct list_head s_orphan;	/* List of orphaned inodes in on disk
> +					   list */
> +	struct ext4_orphan_info s_orphan_info;
> +
>  	/* Ratelimit ext4 messages. */
>  	struct ratelimit_state s_err_ratelimit_state;
>  	struct ratelimit_state s_warning_ratelimit_state;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/67ae57ef2ee3ac27ab844c22594351c83142d86f.1789097450.git.jonaszhou@zhaoxin.com?part=1