[PATCH] ext4: isolate s_orphan_lock from read-mostly fields

[PATCH] ext4: isolate s_orphan_lock from read-mostly fields

[JonasZhou-oc]

From: Jonas Zhou <jonaszhou@zhaoxin.com>

s_orphan_lock is modified by orphan-list operations, but currently shares
a cache line with s_journal and s_ext4_flags.  The latter fields are read
from common ext4 paths, so unrelated accesses can cause the line containing
the lock to bounce between CPUs.

On a two-socket Intel Xeon Silver 4208 system with 16 workers, throughput
improved by 4.6% for stress-ng.dnotify and 1.7% for stress-ng.unlink.  Perf
c2c showed that HITM events on the affected fields decreased from 3,699 to
3,125 (-15.5%) for stress-ng.dnotify and from 1,368 to 571 (-58.3%) for
stress-ng.unlink.  Remote HITM events decreased by 45.1% and 65.5%,
respectively.  The journal/flags line had no remote HITM events after the
change; the remaining remote HITM events were on the orphan-lock line.

Signed-off-by: Jonas Zhou <jonaszhou@zhaoxin.com>
---
 fs/ext4/ext4.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 724a27e8be61..a3f16a981664 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1644,7 +1644,7 @@ struct ext4_sb_info {
 	/* Journaling */
 	struct journal_s *s_journal;
 	unsigned long s_ext4_flags;		/* Ext4 superblock flags */
-	struct mutex s_orphan_lock;	/* Protects on disk list changes */
+	struct mutex s_orphan_lock ____cacheline_aligned_in_smp;
 	struct list_head s_orphan;	/* List of orphaned inodes in on disk
 					   list */
 	struct ext4_orphan_info s_orphan_info;
-- 
2.25.1

Re: [PATCH] ext4: isolate s_orphan_lock from read-mostly fields

[sashiko-bot]

> From: Jonas Zhou <jonaszhou@zhaoxin.com>
> 
> s_orphan_lock is modified by orphan-list operations, but currently shares
> a cache line with s_journal and s_ext4_flags.  The latter fields are read
> from common ext4 paths, so unrelated accesses can cause the line containing
> the lock to bounce between CPUs.
> 
> On a two-socket Intel Xeon Silver 4208 system with 16 workers, throughput
> improved by 4.6% for stress-ng.dnotify and 1.7% for stress-ng.unlink.  Perf
> c2c showed that HITM events on the affected fields decreased from 3,699 to
> 3,125 (-15.5%) for stress-ng.dnotify and from 1,368 to 571 (-58.3%) for
> stress-ng.unlink.  Remote HITM events decreased by 45.1% and 65.5%,
> respectively.  The journal/flags line had no remote HITM events after the
> change; the remaining remote HITM events were on the orphan-lock line.
> 
> Signed-off-by: Jonas Zhou <jonaszhou@zhaoxin.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903021836.962624-1-jonaszhou-oc@zhaoxin.com?part=1

Re: [PATCH] ext4: isolate s_orphan_lock from read-mostly fields

[Jan Kara]

On Thu 03-09-26 10:18:35, JonasZhou-oc wrote:
> From: Jonas Zhou <jonaszhou@zhaoxin.com>
> 
> s_orphan_lock is modified by orphan-list operations, but currently shares
> a cache line with s_journal and s_ext4_flags.  The latter fields are read
> from common ext4 paths, so unrelated accesses can cause the line containing
> the lock to bounce between CPUs.
> 
> On a two-socket Intel Xeon Silver 4208 system with 16 workers, throughput
> improved by 4.6% for stress-ng.dnotify and 1.7% for stress-ng.unlink.  Perf
> c2c showed that HITM events on the affected fields decreased from 3,699 to
> 3,125 (-15.5%) for stress-ng.dnotify and from 1,368 to 571 (-58.3%) for
> stress-ng.unlink.  Remote HITM events decreased by 45.1% and 65.5%,
> respectively.  The journal/flags line had no remote HITM events after the
> change; the remaining remote HITM events were on the orphan-lock line.
> 
> Signed-off-by: Jonas Zhou <jonaszhou@zhaoxin.com>

Ok, that makes some sense. But I have some comments:

1) If you used orphan_file filesystem feature, you'd see much larger wins
(as the orphan lock would get completely out of the picture).

2) You could just place orphan handling related entries to some other place
in the sb which is not so frequently read. That would reduce the contention
as well without wasting space for padding in struct ext4_sb_info.

3) Your patch drops a comment at s_orphan_lock.

In particular given point 1) above, I'm not sure this is really worth it -
if you care about orphan handling scalability, you should just enable
orphan_file feature.

								Honza

> ---
>  fs/ext4/ext4.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 724a27e8be61..a3f16a981664 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1644,7 +1644,7 @@ struct ext4_sb_info {
>  	/* Journaling */
>  	struct journal_s *s_journal;
>  	unsigned long s_ext4_flags;		/* Ext4 superblock flags */
> -	struct mutex s_orphan_lock;	/* Protects on disk list changes */
> +	struct mutex s_orphan_lock ____cacheline_aligned_in_smp;
>  	struct list_head s_orphan;	/* List of orphaned inodes in on disk
>  					   list */
>  	struct ext4_orphan_info s_orphan_info;
> -- 
> 2.25.1
> 
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] ext4: isolate s_orphan_lock from read-mostly fields

[JonasZhou-oc]

Hi Jan,

Thanks for the review. My original test filesystem did not have
orphan_file enabled. I agree that enabling it is the main scalability
improvement; this change only targets filesystems still using the
legacy list.

For v2, I exchanged s_journal and s_ext4_flags with s_err_report_sec
and s_li_request. Moving the readers keeps the orphan lock/list and
the intervening allocator fields at their original offsets. It adds
no padding and preserves the mutex comment.

sizeof(struct ext4_sb_info) remains 2496 bytes with my x86-64 config.
Compiled checks with quota disabled, lockdep, PREEMPT_RT and i386 also
show only these four offsets changing, with no structure-size increase.

Test results comparing v7.3-rc1 and v2 on two Xeon Silver 4208 sockets:

                      Mean throughput change    Paired 95% interval
legacy      dnotify                  +13.52%       [+4.83%, +22.92%]
legacy      unlink                   +31.70%      [+22.53%, +41.48%]
legacy      fallocate                 +0.25%        [-1.40%, +1.94%]
orphan_file dnotify                   -0.43%        [-5.44%, +4.96%]
orphan_file unlink                    -0.34%        [-2.76%, +2.13%]
orphan_file fallocate                 +0.29%        [-0.82%, +1.42%]

The intervals use four paired log ratios; the mean-change column uses
the ratio of arithmetic means. All four legacy dnotify/unlink pairs
improved. The orphan_file results show boot-to-boot variation, so I am
not claiming equivalence or a general filesystem speedup. These are
new measurements on memory-backed images, not the original v1 numbers.

Separate perf c2c dnotify captures on legacy filesystems show remote
HITM samples on the original journal/flags offsets in all four base
captures and none on their new cache line in the four v2 captures.

Thanks,
Jonas

[PATCH v2] ext4: move journal state away from orphan list updates

[JonasZhou-oc]

From: Jonas Zhou <jonaszhou@zhaoxin.com>

In the tested x86-64 layout, s_journal and s_ext4_flags share a cache
line with s_orphan_lock and s_orphan. Legacy orphan-list updates
invalidate the same line used by unrelated journal and inode paths.

Exchange s_journal and s_ext4_flags with s_err_report_sec and
s_li_request, which are used for error reporting and lazy-init
management. This separates the common journal and flag readers from
orphan-list writes without adding padding or shifting the intervening
allocator fields. Preserve the comment describing s_orphan_lock.

Only these four member offsets change, and sizeof(struct ext4_sb_info)
remains 2496 bytes with the tested x86-64 configuration. Compiled layout
checks with quota disabled, lockdep, PREEMPT_RT and i386 also preserve
the respective structure sizes and all other member offsets.

On a system with two Xeon Silver 4208 processors and a filesystem
without orphan_file enabled, mean throughput improved by 13.52% for
stress-ng dnotify and 31.70% for unlink.

This targets users of the legacy orphan list. The orphan_file feature
avoids this lock in normal operation and remains the main scalability
improvement. Tests with orphan_file enabled show no clear throughput
change within the observed boot-to-boot variation.

Results from repeated tests comparing v7.3-rc1 and v2:

                       Mean throughput       Mean change
Feature      Test          base         v2           (%)
legacy       dnotify    19562.38   22206.66        +13.52
legacy       unlink        38.82      51.13        +31.70
legacy       fallocate    233.55     234.14         +0.25
orphan_file  dnotify    27702.16   27582.55         -0.43
orphan_file  unlink       116.93     116.52         -0.34
orphan_file  fallocate    235.20     235.89         +0.29

Descriptive 95% t intervals from the four paired log ratios (df=3),
expressed as percentage changes:
  legacy:      dnotify [+4.83, +22.92], unlink [+22.53, +41.48],
               fallocate [-1.40, +1.94]
  orphan_file: dnotify [-5.44, +4.96], unlink [-2.76, +2.13],
               fallocate [-0.82, +1.42]
These are intervals for the paired geometric ratio, not the arithmetic
mean ratio above. Four pairs do not establish performance equivalence.

For legacy dnotify, perf c2c shows remote HITM samples on the original
journal/flags offsets, but none on their new cache line after the
change. Local HITM samples and orphan-lock contention remain. The
combined HITM sample density of the orphan and journal lines is lower
than that of the original shared line.

Signed-off-by: Jonas Zhou <jonaszhou@zhaoxin.com>
---
Changes since v1:
- Replace explicit cache-line alignment with a four-member exchange.
- Keep the mutex comment and all other member offsets unchanged.
- Retest this layout against v7.3-rc1, including orphan_file controls.

v1:
https://lore.kernel.org/all/20260903021836.962624-1-jonaszhou-oc@zhaoxin.com/

 fs/ext4/ext4.h | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 724a27e8be61..16ef01078233 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1641,9 +1641,13 @@ struct ext4_sb_info {
 	struct super_block *s_sb;
 	struct buffer_head *s_mmp_bh;
 
+	/* timeout in seconds for s_err_report; 0 disables the timer. */
+	unsigned long s_err_report_sec;
+
+	/* Lazy inode table initialization info */
+	struct ext4_li_request *s_li_request;
+
 	/* Journaling */
-	struct journal_s *s_journal;
-	unsigned long s_ext4_flags;		/* Ext4 superblock flags */
 	struct mutex s_orphan_lock;	/* Protects on disk list changes */
 	struct list_head s_orphan;	/* List of orphaned inodes in on disk
 					   list */
@@ -1747,11 +1751,10 @@ struct ext4_sb_info {
 
 	/* timer for periodic error stats printing */
 	struct timer_list s_err_report;
-	/* timeout in seconds for s_err_report; 0 disables the timer. */
-	unsigned long s_err_report_sec;
+	/* Keep journal state away from orphan list updates. */
+	struct journal_s *s_journal;
+	unsigned long s_ext4_flags;		/* Ext4 superblock flags */
 
-	/* Lazy inode table initialization info */
-	struct ext4_li_request *s_li_request;
 	/* Wait multiplier for lazy initialization thread */
 	unsigned int s_li_wait_mult;
 

base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.43.0

Re: [PATCH v2] ext4: move journal state away from orphan list updates

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Concurrent sysfs writes to err_report_sec corrupt the timer list due to unnecessary timer_setup()
--

commit 5546001d405559aa5c5103006d23b436861ae874
Author: Jonas Zhou <jonaszhou@zhaoxin.com>
Subject: ext4: move journal state away from orphan list updates

This patch exchanges s_journal and s_ext4_flags with s_err_report_sec
and s_li_request in struct ext4_sb_info. This separates common journal
and flag readers from orphan-list writes to improve cache line usage
and avoid false sharing.

> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 724a27e8be613..16ef010782331 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1641,9 +1641,13 @@ struct ext4_sb_info {
>  	struct super_block *s_sb;
>  	struct buffer_head *s_mmp_bh;
>  
> +	/* timeout in seconds for s_err_report; 0 disables the timer. */
> +	unsigned long s_err_report_sec;
> +

[Severity: High]
This isn't a bug introduced by this patch, but while looking at the movement
of s_err_report_sec, I noticed a pre-existing issue in its sysfs store
handler.

Can concurrent writes to the err_report_sec sysfs file corrupt the timer list?

Looking at err_report_sec_store() in fs/ext4/sysfs.c, it does not hold any
locks. If two threads concurrently write non-zero values when the current
value is 0:

Thread A
  if (!sbi->s_err_report_sec && t)
      timer_setup(&sbi->s_err_report, print_daily_error_info, 0);
  ...
  mod_timer(&sbi->s_err_report, ...);

Thread B
  if (!sbi->s_err_report_sec && t)
      timer_setup(&sbi->s_err_report, print_daily_error_info, 0);
  ...
  mod_timer(&sbi->s_err_report, ...);

Thread A arms the timer and puts it in the active timer wheel. Thread B then
calls timer_setup() which zeroes timer->entry.pprev. This makes Thread B's
subsequent mod_timer() re-insert the timer without detaching it, corrupting
the timer wheel's doubly-linked hlist.

Since the timer is already unconditionally initialized during filesystem
mount, and timer_delete_sync() doesn't deinitialize it, is this timer_setup()
call in the sysfs handler unnecessary?

[ ... ]

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

Re: [PATCH v2] ext4: move journal state away from orphan list updates

[Jan Kara]

On Mon 07-09-26 19:12:42, JonasZhou-oc wrote:
> From: Jonas Zhou <jonaszhou@zhaoxin.com>
> 
> In the tested x86-64 layout, s_journal and s_ext4_flags share a cache
> line with s_orphan_lock and s_orphan. Legacy orphan-list updates
> invalidate the same line used by unrelated journal and inode paths.
> 
> Exchange s_journal and s_ext4_flags with s_err_report_sec and
> s_li_request, which are used for error reporting and lazy-init
> management. This separates the common journal and flag readers from
> orphan-list writes without adding padding or shifting the intervening
> allocator fields. Preserve the comment describing s_orphan_lock.
> 
> Only these four member offsets change, and sizeof(struct ext4_sb_info)
> remains 2496 bytes with the tested x86-64 configuration. Compiled layout
> checks with quota disabled, lockdep, PREEMPT_RT and i386 also preserve
> the respective structure sizes and all other member offsets.
> 
> On a system with two Xeon Silver 4208 processors and a filesystem
> without orphan_file enabled, mean throughput improved by 13.52% for
> stress-ng dnotify and 31.70% for unlink.
> 
> This targets users of the legacy orphan list. The orphan_file feature
> avoids this lock in normal operation and remains the main scalability
> improvement. Tests with orphan_file enabled show no clear throughput
> change within the observed boot-to-boot variation.
> 
> Results from repeated tests comparing v7.3-rc1 and v2:
> 
>                        Mean throughput       Mean change
> Feature      Test          base         v2           (%)
> legacy       dnotify    19562.38   22206.66        +13.52
> legacy       unlink        38.82      51.13        +31.70
> legacy       fallocate    233.55     234.14         +0.25
> orphan_file  dnotify    27702.16   27582.55         -0.43
> orphan_file  unlink       116.93     116.52         -0.34
> orphan_file  fallocate    235.20     235.89         +0.29
> 
> Descriptive 95% t intervals from the four paired log ratios (df=3),
> expressed as percentage changes:
>   legacy:      dnotify [+4.83, +22.92], unlink [+22.53, +41.48],
>                fallocate [-1.40, +1.94]
>   orphan_file: dnotify [-5.44, +4.96], unlink [-2.76, +2.13],
>                fallocate [-0.82, +1.42]
> These are intervals for the paired geometric ratio, not the arithmetic
> mean ratio above. Four pairs do not establish performance equivalence.
> 
> For legacy dnotify, perf c2c shows remote HITM samples on the original
> journal/flags offsets, but none on their new cache line after the
> change. Local HITM samples and orphan-lock contention remain. The
> combined HITM sample density of the orphan and journal lines is lower
> than that of the original shared line.
> 
> Signed-off-by: Jonas Zhou <jonaszhou@zhaoxin.com>

Sorry, but we should keep things logically belonging together close. I'd
keep s_journal & s_ext4_flags in the read-mostly part of the sb and perhaps
move orphan-related things (s_orphan_lock, s_orphan, s_orphan_info) later.
Perhaps below s_journal_triggers definition. And add a comment there like:

	/* Orphan inode tracking */

								Honza

> ---
> Changes since v1:
> - Replace explicit cache-line alignment with a four-member exchange.
> - Keep the mutex comment and all other member offsets unchanged.
> - Retest this layout against v7.3-rc1, including orphan_file controls.
> 
> v1:
> https://lore.kernel.org/all/20260903021836.962624-1-jonaszhou-oc@zhaoxin.com/
> 
>  fs/ext4/ext4.h | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 724a27e8be61..16ef01078233 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1641,9 +1641,13 @@ struct ext4_sb_info {
>  	struct super_block *s_sb;
>  	struct buffer_head *s_mmp_bh;
>  
> +	/* timeout in seconds for s_err_report; 0 disables the timer. */
> +	unsigned long s_err_report_sec;
> +
> +	/* Lazy inode table initialization info */
> +	struct ext4_li_request *s_li_request;
> +
>  	/* Journaling */
> -	struct journal_s *s_journal;
> -	unsigned long s_ext4_flags;		/* Ext4 superblock flags */
>  	struct mutex s_orphan_lock;	/* Protects on disk list changes */
>  	struct list_head s_orphan;	/* List of orphaned inodes in on disk
>  					   list */
> @@ -1747,11 +1751,10 @@ struct ext4_sb_info {
>  
>  	/* timer for periodic error stats printing */
>  	struct timer_list s_err_report;
> -	/* timeout in seconds for s_err_report; 0 disables the timer. */
> -	unsigned long s_err_report_sec;
> +	/* Keep journal state away from orphan list updates. */
> +	struct journal_s *s_journal;
> +	unsigned long s_ext4_flags;		/* Ext4 superblock flags */
>  
> -	/* Lazy inode table initialization info */
> -	struct ext4_li_request *s_li_request;
>  	/* Wait multiplier for lazy initialization thread */
>  	unsigned int s_li_wait_mult;
>  
> 
> base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
> -- 
> 2.43.0
> 
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

[JonasZhou-oc]

From: Jonas Zhou <jonaszhou@zhaoxin.com>

In the tested x86-64 layout, s_journal and s_ext4_flags share a cache
line with s_orphan_lock and s_orphan. Legacy orphan-list updates
invalidate the same line used by unrelated journal and inode paths.

Move s_orphan_lock, s_orphan and s_orphan_info together below
s_journal_triggers. This separates orphan-list writes from journal
and flag readers while keeping the related fields in logical groups.
Add a comment identifying the orphan tracking group and preserve the
existing member comments.

On a system with two Xeon Silver 4208 processors and a filesystem
without orphan_file enabled, repeated tests against v7.3-rc2 recorded
mean throughput increases of 11.06% for stress-ng dnotify and 35.93%
for unlink.

Tests with orphan_file enabled showed no clear throughput change
within the observed boot-to-boot variation.

perf c2c shows remote HITM samples on the original shared line, but
none on the separate journal line after the change. Local HITM samples
and orphan-lock contention remain.

Suggested-by: Jan Kara <jack@suse.cz>
Link: https://lore.kernel.org/all/575wyoks3ldokhrg6v43rxwsegaecod4bgrpbii6jeczy7czbi@a3yq26u52orp/
Signed-off-by: Jonas Zhou <jonaszhou@zhaoxin.com>
---
Changes since v2:
- Follow Jan's suggestion to move the orphan tracking fields together
  below s_journal_triggers, instead of exchanging journal state with
  the error-reporting and lazy-init fields.
- Add an Orphan inode tracking comment.
- Rebase and retest against v7.3-rc2.

v2:
https://lore.kernel.org/all/a33183c1c24ea47a98afdd4756b49365593a2762.1788779199.git.jonaszhou@zhaoxin.com/
v1:
https://lore.kernel.org/all/20260903021836.962624-1-jonaszhou-oc@zhaoxin.com/

 fs/ext4/ext4.h | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 724a27e8be61..166910914973 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -1644,10 +1644,6 @@ struct ext4_sb_info {
 	/* Journaling */
 	struct journal_s *s_journal;
 	unsigned long s_ext4_flags;		/* Ext4 superblock flags */
-	struct mutex s_orphan_lock;	/* Protects on disk list changes */
-	struct list_head s_orphan;	/* List of orphaned inodes in on disk
-					   list */
-	struct ext4_orphan_info s_orphan_info;
 	unsigned long s_commit_interval;
 	u32 s_max_batch_time;
 	u32 s_min_batch_time;
@@ -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 */
+	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;

base-commit: df2908090cda368b01ff43709f51890076c56157
-- 
2.43.0

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

[sashiko-bot]

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

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

[Jan Kara]

On Fri 11-09-26 11:31:13, JonasZhou-oc wrote:
> From: Jonas Zhou <jonaszhou@zhaoxin.com>
> 
> In the tested x86-64 layout, s_journal and s_ext4_flags share a cache
> line with s_orphan_lock and s_orphan. Legacy orphan-list updates
> invalidate the same line used by unrelated journal and inode paths.
> 
> Move s_orphan_lock, s_orphan and s_orphan_info together below
> s_journal_triggers. This separates orphan-list writes from journal
> and flag readers while keeping the related fields in logical groups.
> Add a comment identifying the orphan tracking group and preserve the
> existing member comments.
> 
> On a system with two Xeon Silver 4208 processors and a filesystem
> without orphan_file enabled, repeated tests against v7.3-rc2 recorded
> mean throughput increases of 11.06% for stress-ng dnotify and 35.93%
> for unlink.
> 
> Tests with orphan_file enabled showed no clear throughput change
> within the observed boot-to-boot variation.
> 
> perf c2c shows remote HITM samples on the original shared line, but
> none on the separate journal line after the change. Local HITM samples
> and orphan-lock contention remain.
> 
> Suggested-by: Jan Kara <jack@suse.cz>
> Link: https://lore.kernel.org/all/575wyoks3ldokhrg6v43rxwsegaecod4bgrpbii6jeczy7czbi@a3yq26u52orp/
> Signed-off-by: Jonas Zhou <jonaszhou@zhaoxin.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
> Changes since v2:
> - Follow Jan's suggestion to move the orphan tracking fields together
>   below s_journal_triggers, instead of exchanging journal state with
>   the error-reporting and lazy-init fields.
> - Add an Orphan inode tracking comment.
> - Rebase and retest against v7.3-rc2.
> 
> v2:
> https://lore.kernel.org/all/a33183c1c24ea47a98afdd4756b49365593a2762.1788779199.git.jonaszhou@zhaoxin.com/
> v1:
> https://lore.kernel.org/all/20260903021836.962624-1-jonaszhou-oc@zhaoxin.com/
> 
>  fs/ext4/ext4.h | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 724a27e8be61..166910914973 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -1644,10 +1644,6 @@ struct ext4_sb_info {
>  	/* Journaling */
>  	struct journal_s *s_journal;
>  	unsigned long s_ext4_flags;		/* Ext4 superblock flags */
> -	struct mutex s_orphan_lock;	/* Protects on disk list changes */
> -	struct list_head s_orphan;	/* List of orphaned inodes in on disk
> -					   list */
> -	struct ext4_orphan_info s_orphan_info;
>  	unsigned long s_commit_interval;
>  	u32 s_max_batch_time;
>  	u32 s_min_batch_time;
> @@ -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 */
> +	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;
> 
> base-commit: df2908090cda368b01ff43709f51890076c56157
> -- 
> 2.43.0
> 
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR