1. Patchew
  2. linux
  3. View series

[PATCH] lockdep: avoid struct return in lock_stats()

Arnd Bergmann posted 1 patch 4 months ago 
include/linux/lockdep_types.h |  2 +-
kernel/locking/lockdep.c      | 27 ++++++++++++---------------
kernel/locking/lockdep_proc.c |  2 +-
3 files changed, 14 insertions(+), 17 deletions(-)
[PATCH] lockdep: avoid struct return in lock_stats()
Posted by Arnd Bergmann 4 months ago 
From: Arnd Bergmann <arnd@arndb.de>

Returning a large structure from the lock_stats() function causes clang
to have multiple copies of it on the stack and copy between them, which
can end up exceeding the frame size warning limit:

kernel/locking/lockdep.c:300:25: error: stack frame size (1464) exceeds limit (1280) in 'lock_stats' [-Werror,-Wframe-larger-than]
  300 | struct lock_class_stats lock_stats(struct lock_class *class)

Change the calling conventions to directly operate on the caller's copy,
which apparently is what gcc does already.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 include/linux/lockdep_types.h |  2 +-
 kernel/locking/lockdep.c      | 27 ++++++++++++---------------
 kernel/locking/lockdep_proc.c |  2 +-
 3 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/include/linux/lockdep_types.h b/include/linux/lockdep_types.h
index 9f361d3ab9d9..eae115a26488 100644
--- a/include/linux/lockdep_types.h
+++ b/include/linux/lockdep_types.h
@@ -175,7 +175,7 @@ struct lock_class_stats {
 	unsigned long			bounces[nr_bounce_types];
 };
 
-struct lock_class_stats lock_stats(struct lock_class *class);
+void lock_stats(struct lock_class *class, struct lock_class_stats *stats);
 void clear_lock_stats(struct lock_class *class);
 #endif
 
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index dd2bbf73718b..0c941418a215 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -297,33 +297,30 @@ static inline void lock_time_add(struct lock_time *src, struct lock_time *dst)
 	dst->nr += src->nr;
 }
 
-struct lock_class_stats lock_stats(struct lock_class *class)
+void lock_stats(struct lock_class *class, struct lock_class_stats *stats)
 {
-	struct lock_class_stats stats;
 	int cpu, i;
 
-	memset(&stats, 0, sizeof(struct lock_class_stats));
+	memset(stats, 0, sizeof(struct lock_class_stats));
 	for_each_possible_cpu(cpu) {
 		struct lock_class_stats *pcs =
 			&per_cpu(cpu_lock_stats, cpu)[class - lock_classes];
 
-		for (i = 0; i < ARRAY_SIZE(stats.contention_point); i++)
-			stats.contention_point[i] += pcs->contention_point[i];
+		for (i = 0; i < ARRAY_SIZE(stats->contention_point); i++)
+			stats->contention_point[i] += pcs->contention_point[i];
 
-		for (i = 0; i < ARRAY_SIZE(stats.contending_point); i++)
-			stats.contending_point[i] += pcs->contending_point[i];
+		for (i = 0; i < ARRAY_SIZE(stats->contending_point); i++)
+			stats->contending_point[i] += pcs->contending_point[i];
 
-		lock_time_add(&pcs->read_waittime, &stats.read_waittime);
-		lock_time_add(&pcs->write_waittime, &stats.write_waittime);
+		lock_time_add(&pcs->read_waittime, &stats->read_waittime);
+		lock_time_add(&pcs->write_waittime, &stats->write_waittime);
 
-		lock_time_add(&pcs->read_holdtime, &stats.read_holdtime);
-		lock_time_add(&pcs->write_holdtime, &stats.write_holdtime);
+		lock_time_add(&pcs->read_holdtime, &stats->read_holdtime);
+		lock_time_add(&pcs->write_holdtime, &stats->write_holdtime);
 
-		for (i = 0; i < ARRAY_SIZE(stats.bounces); i++)
-			stats.bounces[i] += pcs->bounces[i];
+		for (i = 0; i < ARRAY_SIZE(stats->bounces); i++)
+			stats->bounces[i] += pcs->bounces[i];
 	}
-
-	return stats;
 }
 
 void clear_lock_stats(struct lock_class *class)
diff --git a/kernel/locking/lockdep_proc.c b/kernel/locking/lockdep_proc.c
index b52c07c4707c..1916db9aa46b 100644
--- a/kernel/locking/lockdep_proc.c
+++ b/kernel/locking/lockdep_proc.c
@@ -657,7 +657,7 @@ static int lock_stat_open(struct inode *inode, struct file *file)
 			if (!test_bit(idx, lock_classes_in_use))
 				continue;
 			iter->class = class;
-			iter->stats = lock_stats(class);
+			lock_stats(class, &iter->stats);
 			iter++;
 		}
 
-- 
2.39.5
Re: [PATCH] lockdep: avoid struct return in lock_stats()
Posted by Boqun Feng 3 months, 3 weeks ago 
On Tue, Jun 10, 2025 at 11:29:21AM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Returning a large structure from the lock_stats() function causes clang
> to have multiple copies of it on the stack and copy between them, which
> can end up exceeding the frame size warning limit:
> 
> kernel/locking/lockdep.c:300:25: error: stack frame size (1464) exceeds limit (1280) in 'lock_stats' [-Werror,-Wframe-larger-than]
>   300 | struct lock_class_stats lock_stats(struct lock_class *class)
> 
> Change the calling conventions to directly operate on the caller's copy,
> which apparently is what gcc does already.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Queued for more tests and reviews, thanks!

Regards,
Boqun
Re: [PATCH] lockdep: avoid struct return in lock_stats()
Posted by Andy Shevchenko 3 months, 3 weeks ago 
On Tue, Jun 17, 2025 at 10:17:32PM -0700, Boqun Feng wrote:
> On Tue, Jun 10, 2025 at 11:29:21AM +0200, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> > 
> > Returning a large structure from the lock_stats() function causes clang
> > to have multiple copies of it on the stack and copy between them, which
> > can end up exceeding the frame size warning limit:
> > 
> > kernel/locking/lockdep.c:300:25: error: stack frame size (1464) exceeds limit (1280) in 'lock_stats' [-Werror,-Wframe-larger-than]
> >   300 | struct lock_class_stats lock_stats(struct lock_class *class)
> > 
> > Change the calling conventions to directly operate on the caller's copy,
> > which apparently is what gcc does already.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Queued for more tests and reviews, thanks!

What about this one:
    lockdep: change 'static const' variables to enum values

?

(I can't quickly find the pointer right now, but I think in lore.kernel.org you can find it)

-- 
With Best Regards,
Andy Shevchenko
Re: [PATCH] lockdep: avoid struct return in lock_stats()
Posted by Boqun Feng 3 months, 3 weeks ago 
On Wed, Jun 18, 2025 at 08:51:07PM +0300, Andy Shevchenko wrote:
> On Tue, Jun 17, 2025 at 10:17:32PM -0700, Boqun Feng wrote:
> > On Tue, Jun 10, 2025 at 11:29:21AM +0200, Arnd Bergmann wrote:
> > > From: Arnd Bergmann <arnd@arndb.de>
> > > 
> > > Returning a large structure from the lock_stats() function causes clang
> > > to have multiple copies of it on the stack and copy between them, which
> > > can end up exceeding the frame size warning limit:
> > > 
> > > kernel/locking/lockdep.c:300:25: error: stack frame size (1464) exceeds limit (1280) in 'lock_stats' [-Werror,-Wframe-larger-than]
> > >   300 | struct lock_class_stats lock_stats(struct lock_class *class)
> > > 
> > > Change the calling conventions to directly operate on the caller's copy,
> > > which apparently is what gcc does already.
> > > 
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > 
> > Queued for more tests and reviews, thanks!
> 
> What about this one:
>     lockdep: change 'static const' variables to enum values
> 
> ?
> 
> (I can't quickly find the pointer right now, but I think in lore.kernel.org you can find it)
> 

This one:

	https://lore.kernel.org/lkml/20250409122314.2848028-6-arnd@kernel.org/

? I will take a look (give me some time to learn about that compiler
option), but seems trivial enough, should be in the next merge window if
no problem found. Thanks for bringing it up.

Regards,
Boqun

> -- 
> With Best Regards,
> Andy Shevchenko
> 
>
[tip: locking/core] locking/lockdep: Avoid struct return in lock_stats()
Posted by tip-bot2 for Arnd Bergmann 2 months, 3 weeks ago 
The following commit has been merged into the locking/core branch of tip:

Commit-ID:     d7c36d6350b5a4b27256eaeeea3b72621a819c9a
Gitweb:        https://git.kernel.org/tip/d7c36d6350b5a4b27256eaeeea3b72621a819c9a
Author:        Arnd Bergmann <arnd@arndb.de>
AuthorDate:    Tue, 10 Jun 2025 11:29:21 +02:00
Committer:     Boqun Feng <boqun.feng@gmail.com>
CommitterDate: Mon, 14 Jul 2025 21:57:20 -07:00

locking/lockdep: Avoid struct return in lock_stats()

Returning a large structure from the lock_stats() function causes clang
to have multiple copies of it on the stack and copy between them, which
can end up exceeding the frame size warning limit:

kernel/locking/lockdep.c:300:25: error: stack frame size (1464) exceeds limit (1280) in 'lock_stats' [-Werror,-Wframe-larger-than]
  300 | struct lock_class_stats lock_stats(struct lock_class *class)

Change the calling conventions to directly operate on the caller's copy,
which apparently is what gcc does already.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Boqun Feng <boqun.feng@gmail.com>
Link: https://lore.kernel.org/r/20250610092941.2642847-1-arnd@kernel.org
---
 include/linux/lockdep_types.h |  2 +-
 kernel/locking/lockdep.c      | 27 ++++++++++++---------------
 kernel/locking/lockdep_proc.c |  2 +-
 3 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/include/linux/lockdep_types.h b/include/linux/lockdep_types.h
index 9f361d3..eae115a 100644
--- a/include/linux/lockdep_types.h
+++ b/include/linux/lockdep_types.h
@@ -175,7 +175,7 @@ struct lock_class_stats {
 	unsigned long			bounces[nr_bounce_types];
 };
 
-struct lock_class_stats lock_stats(struct lock_class *class);
+void lock_stats(struct lock_class *class, struct lock_class_stats *stats);
 void clear_lock_stats(struct lock_class *class);
 #endif
 
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index dd2bbf7..0c94141 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -297,33 +297,30 @@ static inline void lock_time_add(struct lock_time *src, struct lock_time *dst)
 	dst->nr += src->nr;
 }
 
-struct lock_class_stats lock_stats(struct lock_class *class)
+void lock_stats(struct lock_class *class, struct lock_class_stats *stats)
 {
-	struct lock_class_stats stats;
 	int cpu, i;
 
-	memset(&stats, 0, sizeof(struct lock_class_stats));
+	memset(stats, 0, sizeof(struct lock_class_stats));
 	for_each_possible_cpu(cpu) {
 		struct lock_class_stats *pcs =
 			&per_cpu(cpu_lock_stats, cpu)[class - lock_classes];
 
-		for (i = 0; i < ARRAY_SIZE(stats.contention_point); i++)
-			stats.contention_point[i] += pcs->contention_point[i];
+		for (i = 0; i < ARRAY_SIZE(stats->contention_point); i++)
+			stats->contention_point[i] += pcs->contention_point[i];
 
-		for (i = 0; i < ARRAY_SIZE(stats.contending_point); i++)
-			stats.contending_point[i] += pcs->contending_point[i];
+		for (i = 0; i < ARRAY_SIZE(stats->contending_point); i++)
+			stats->contending_point[i] += pcs->contending_point[i];
 
-		lock_time_add(&pcs->read_waittime, &stats.read_waittime);
-		lock_time_add(&pcs->write_waittime, &stats.write_waittime);
+		lock_time_add(&pcs->read_waittime, &stats->read_waittime);
+		lock_time_add(&pcs->write_waittime, &stats->write_waittime);
 
-		lock_time_add(&pcs->read_holdtime, &stats.read_holdtime);
-		lock_time_add(&pcs->write_holdtime, &stats.write_holdtime);
+		lock_time_add(&pcs->read_holdtime, &stats->read_holdtime);
+		lock_time_add(&pcs->write_holdtime, &stats->write_holdtime);
 
-		for (i = 0; i < ARRAY_SIZE(stats.bounces); i++)
-			stats.bounces[i] += pcs->bounces[i];
+		for (i = 0; i < ARRAY_SIZE(stats->bounces); i++)
+			stats->bounces[i] += pcs->bounces[i];
 	}
-
-	return stats;
 }
 
 void clear_lock_stats(struct lock_class *class)
diff --git a/kernel/locking/lockdep_proc.c b/kernel/locking/lockdep_proc.c
index b52c07c..1916db9 100644
--- a/kernel/locking/lockdep_proc.c
+++ b/kernel/locking/lockdep_proc.c
@@ -657,7 +657,7 @@ static int lock_stat_open(struct inode *inode, struct file *file)
 			if (!test_bit(idx, lock_classes_in_use))
 				continue;
 			iter->class = class;
-			iter->stats = lock_stats(class);
+			lock_stats(class, &iter->stats);
 			iter++;
 		}

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.