public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] audit: Quit audit_free_names() early if name list empty
@ 2026-01-21  2:35 Waiman Long
  2026-01-21  2:35 ` [PATCH 2/2] audit: Call path_{put,get}() in audit_alloc_name()/audit_free_names() only when necessary Waiman Long
  2026-01-21  3:11 ` [PATCH 1/2] audit: Quit audit_free_names() early if name list empty Al Viro
  0 siblings, 2 replies; 6+ messages in thread
From: Waiman Long @ 2026-01-21  2:35 UTC (permalink / raw)
  To: Paul Moore, Eric Paris, Christian Brauner, Al Viro
  Cc: linux-kernel, audit, Richard Guy Briggs, Ricardo Robaina,
	Waiman Long

Optimize audit_free_names() by quitting early if the name list is empty.
This eliminates the need to acquire and release the fs_struct spinlock
in path_put().

Signed-off-by: Waiman Long <longman@redhat.com>
---
 kernel/auditsc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index dd0563a8e0be..824b6fd98561 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -931,6 +931,9 @@ static inline void audit_free_names(struct audit_context *context)
 {
 	struct audit_names *n, *next;
 
+	if (list_empty(&context->names_list))
+		return;	/* audit_alloc_name() has not been called */
+
 	list_for_each_entry_safe(n, next, &context->names_list, list) {
 		list_del(&n->list);
 		if (n->name)
-- 
2.52.0


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

* [PATCH 2/2] audit: Call path_{put,get}() in audit_alloc_name()/audit_free_names() only when necessary
  2026-01-21  2:35 [PATCH 1/2] audit: Quit audit_free_names() early if name list empty Waiman Long
@ 2026-01-21  2:35 ` Waiman Long
  2026-01-21  3:11 ` [PATCH 1/2] audit: Quit audit_free_names() early if name list empty Al Viro
  1 sibling, 0 replies; 6+ messages in thread
From: Waiman Long @ 2026-01-21  2:35 UTC (permalink / raw)
  To: Paul Moore, Eric Paris, Christian Brauner, Al Viro
  Cc: linux-kernel, audit, Richard Guy Briggs, Ricardo Robaina,
	Waiman Long

It is found that on large SMP systems with a large number of CPUs and
auditing enabled, workloads that generate a massive amount of syscalls
(like open/close) in parallel on the same working directory can cause
significant spinlock contention of the lockref.lock of the working
directory's dentry.

One possible way to reduce such spinlock contention scenario is to
keep the pwd references in audit_free_names() and reuse it in the next
audit_alloc_name() call if there is no change in working directory.

A new pwd_reset field is added to audit_context to indicate, if set,
that the pwd has been reset but still hold mount and dentry references.
Another get_cond_fs_pwd() helper is added to fs_struct.h to conditionally
put the old references back and get the new ones if fs->pwd has been
changed.

By adding test code to count the number of effective audit_free_names()
and audit_alloc_name() calls and the number of relevant path_put() and
path_get() calls after rebooting a patched kernel with auditing enabled
on a 2-socket 96-CPU system, there were about 30k path_get()/path_put()
out of a total of about 1 million audit_free_names()/audit_alloc_name()
calls. It is about 3% of those before the patch for this particular case.

After resetting the counters and running a parallel kernel build,
the new figures were about 202k path_get()/path_put() out of about 56M
audit_free_names()/audit_alloc_name(). That is about 0.4%.

As auditing is increasingly used in production systems due to various
legal and commercial compliance requirements, it is important that we
should try to minimize performance overhead when auditing is enabled.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 include/linux/fs_struct.h | 14 ++++++++++++++
 kernel/audit.h            |  7 +++++++
 kernel/auditsc.c          | 17 +++++++++++------
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/include/linux/fs_struct.h b/include/linux/fs_struct.h
index 0070764b790a..f173b49ad47e 100644
--- a/include/linux/fs_struct.h
+++ b/include/linux/fs_struct.h
@@ -40,6 +40,20 @@ static inline void get_fs_pwd(struct fs_struct *fs, struct path *pwd)
 	read_sequnlock_excl(&fs->seq);
 }
 
+/*
+ * Conditionally get the current fs->pwd if it differs from the given pwd
+ */
+static inline void get_cond_fs_pwd(struct fs_struct *fs, struct path *pwd)
+{
+	read_seqlock_excl(&fs->seq);
+	if ((fs->pwd.dentry != pwd->dentry) || (fs->pwd.mnt != pwd->mnt)) {
+		path_put(pwd);
+		*pwd = fs->pwd;
+		path_get(pwd);
+	}
+	read_sequnlock_excl(&fs->seq);
+}
+
 extern bool current_chrooted(void);
 
 static inline int current_umask(void)
diff --git a/kernel/audit.h b/kernel/audit.h
index 7c401729e21b..03f3539b10e7 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -133,6 +133,13 @@ struct audit_context {
 	int		    name_count; /* total records in names_list */
 	struct list_head    names_list;	/* struct audit_names->list anchor */
 	char		    *filterkey;	/* key for rule that triggered record */
+	/*
+	 * pwd_reset is set if audit_free_names() has been called from
+	 * audit_reset_context() to reset pwd, but pwd is still holding dentry
+	 * and mount references to be used in later audit action without
+	 * the need to reacqure the references again.
+	 */
+	int		    pwd_reset;
 	struct path	    pwd;
 	struct audit_aux_data *aux;
 	struct audit_aux_data *aux_pids;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 824b6fd98561..30d931d9b9a4 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -942,9 +942,7 @@ static inline void audit_free_names(struct audit_context *context)
 			kfree(n);
 	}
 	context->name_count = 0;
-	path_put(&context->pwd);
-	context->pwd.dentry = NULL;
-	context->pwd.mnt = NULL;
+	context->pwd_reset = true;
 }
 
 static inline void audit_free_aux(struct audit_context *context)
@@ -1091,6 +1089,8 @@ static inline void audit_free_context(struct audit_context *context)
 	audit_reset_context(context);
 	audit_proctitle_free(context);
 	free_tree_refs(context);
+	if (context->pwd_reset)
+		path_put(&context->pwd);
 	kfree(context->filterkey);
 	kfree(context);
 }
@@ -1522,7 +1522,8 @@ static void audit_log_name(struct audit_context *context, struct audit_names *n,
 			/* name was specified as a relative path and the
 			 * directory component is the cwd
 			 */
-			if (context->pwd.dentry && context->pwd.mnt)
+			if (context->pwd.dentry && context->pwd.mnt &&
+			    !context->pwd_reset)
 				audit_log_d_path(ab, " name=", &context->pwd);
 			else
 				audit_log_format(ab, " name=(null)");
@@ -1770,7 +1771,7 @@ static void audit_log_exit(void)
 				  context->target_comm))
 		call_panic = 1;
 
-	if (context->pwd.dentry && context->pwd.mnt) {
+	if (context->pwd.dentry && context->pwd.mnt && !context->pwd_reset) {
 		ab = audit_log_start(context, GFP_KERNEL, AUDIT_CWD);
 		if (ab) {
 			audit_log_d_path(ab, "cwd=", &context->pwd);
@@ -2167,8 +2168,12 @@ static struct audit_names *audit_alloc_name(struct audit_context *context,
 	list_add_tail(&aname->list, &context->names_list);
 
 	context->name_count++;
-	if (!context->pwd.dentry)
+	if (context->pwd_reset) {
+		get_cond_fs_pwd(current->fs, &context->pwd);
+		context->pwd_reset = false;
+	} else if (!context->pwd.dentry) {
 		get_fs_pwd(current->fs, &context->pwd);
+	}
 	return aname;
 }
 
-- 
2.52.0


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

* Re: [PATCH 1/2] audit: Quit audit_free_names() early if name list empty
  2026-01-21  2:35 [PATCH 1/2] audit: Quit audit_free_names() early if name list empty Waiman Long
  2026-01-21  2:35 ` [PATCH 2/2] audit: Call path_{put,get}() in audit_alloc_name()/audit_free_names() only when necessary Waiman Long
@ 2026-01-21  3:11 ` Al Viro
  2026-01-21  4:08   ` Waiman Long
  1 sibling, 1 reply; 6+ messages in thread
From: Al Viro @ 2026-01-21  3:11 UTC (permalink / raw)
  To: Waiman Long
  Cc: Paul Moore, Eric Paris, Christian Brauner, linux-kernel, audit,
	Richard Guy Briggs, Ricardo Robaina

On Tue, Jan 20, 2026 at 09:35:08PM -0500, Waiman Long wrote:
> Optimize audit_free_names() by quitting early if the name list is empty.
> This eliminates the need to acquire and release the fs_struct spinlock
> in path_put().

Why would path_put() go anywhere need fs_struct spinlock???

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

* Re: [PATCH 1/2] audit: Quit audit_free_names() early if name list empty
  2026-01-21  3:11 ` [PATCH 1/2] audit: Quit audit_free_names() early if name list empty Al Viro
@ 2026-01-21  4:08   ` Waiman Long
  2026-01-21  5:26     ` Al Viro
  0 siblings, 1 reply; 6+ messages in thread
From: Waiman Long @ 2026-01-21  4:08 UTC (permalink / raw)
  To: Al Viro
  Cc: Paul Moore, Eric Paris, Christian Brauner, linux-kernel, audit,
	Richard Guy Briggs, Ricardo Robaina


On 1/20/26 10:11 PM, Al Viro wrote:
> On Tue, Jan 20, 2026 at 09:35:08PM -0500, Waiman Long wrote:
>> Optimize audit_free_names() by quitting early if the name list is empty.
>> This eliminates the need to acquire and release the fs_struct spinlock
>> in path_put().
> Why would path_put() go anywhere need fs_struct spinlock???
>
path_put() is defined in include/linux/fs_struct.h. It calls 
read_seq{un}lock_excl(&fs->seq) which, in turn, acquires the releases 
the spinlock underneath the seqlock_t.

Cheers,
Longman


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

* Re: [PATCH 1/2] audit: Quit audit_free_names() early if name list empty
  2026-01-21  4:08   ` Waiman Long
@ 2026-01-21  5:26     ` Al Viro
  2026-01-21 15:09       ` Waiman Long
  0 siblings, 1 reply; 6+ messages in thread
From: Al Viro @ 2026-01-21  5:26 UTC (permalink / raw)
  To: Waiman Long
  Cc: Paul Moore, Eric Paris, Christian Brauner, linux-kernel, audit,
	Richard Guy Briggs, Ricardo Robaina

On Tue, Jan 20, 2026 at 11:08:45PM -0500, Waiman Long wrote:
> 
> On 1/20/26 10:11 PM, Al Viro wrote:
> > On Tue, Jan 20, 2026 at 09:35:08PM -0500, Waiman Long wrote:
> > > Optimize audit_free_names() by quitting early if the name list is empty.
> > > This eliminates the need to acquire and release the fs_struct spinlock
> > > in path_put().
> > Why would path_put() go anywhere need fs_struct spinlock???
> > 
> path_put() is defined in include/linux/fs_struct.h. It calls
> read_seq{un}lock_excl(&fs->seq) which, in turn, acquires the releases the
> spinlock underneath the seqlock_t.

Really?  Which kernel would that be?  On mainline we have

; git grep -n -w path_put include/linux/fs_struct.h
; $ git grep -C 6 'void path_put\>' fs/namei.c
fs/namei.c-/**
fs/namei.c- * path_put - put a reference to a path
fs/namei.c- * @path: path to put the reference to
fs/namei.c- *
fs/namei.c- * Given a path decrement the reference count to the dentry and the vfsmount.
fs/namei.c- */
fs/namei.c:void path_put(const struct path *path)
fs/namei.c-{
fs/namei.c-     dput(path->dentry);
fs/namei.c-     mntput(path->mnt);
fs/namei.c-}
fs/namei.c-EXPORT_SYMBOL(path_put);
fs/namei.c-
;

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

* Re: [PATCH 1/2] audit: Quit audit_free_names() early if name list empty
  2026-01-21  5:26     ` Al Viro
@ 2026-01-21 15:09       ` Waiman Long
  0 siblings, 0 replies; 6+ messages in thread
From: Waiman Long @ 2026-01-21 15:09 UTC (permalink / raw)
  To: Al Viro, Waiman Long
  Cc: Paul Moore, Eric Paris, Christian Brauner, linux-kernel, audit,
	Richard Guy Briggs, Ricardo Robaina

On 1/21/26 12:26 AM, Al Viro wrote:
> On Tue, Jan 20, 2026 at 11:08:45PM -0500, Waiman Long wrote:
>> On 1/20/26 10:11 PM, Al Viro wrote:
>>> On Tue, Jan 20, 2026 at 09:35:08PM -0500, Waiman Long wrote:
>>>> Optimize audit_free_names() by quitting early if the name list is empty.
>>>> This eliminates the need to acquire and release the fs_struct spinlock
>>>> in path_put().
>>> Why would path_put() go anywhere need fs_struct spinlock???
>>>
>> path_put() is defined in include/linux/fs_struct.h. It calls
>> read_seq{un}lock_excl(&fs->seq) which, in turn, acquires the releases the
>> spinlock underneath the seqlock_t.
> Really?  Which kernel would that be?  On mainline we have
>
> ; git grep -n -w path_put include/linux/fs_struct.h
> ; $ git grep -C 6 'void path_put\>' fs/namei.c
> fs/namei.c-/**
> fs/namei.c- * path_put - put a reference to a path
> fs/namei.c- * @path: path to put the reference to
> fs/namei.c- *
> fs/namei.c- * Given a path decrement the reference count to the dentry and the vfsmount.
> fs/namei.c- */
> fs/namei.c:void path_put(const struct path *path)
> fs/namei.c-{
> fs/namei.c-     dput(path->dentry);
> fs/namei.c-     mntput(path->mnt);
> fs/namei.c-}
> fs/namei.c-EXPORT_SYMBOL(path_put);
> fs/namei.c-
> ;
>
Sorry, my mistake. I mixed it up with get_fs_pwd(). Will fix the patch.

Cheers,
Longman


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

end of thread, other threads:[~2026-01-21 15:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-21  2:35 [PATCH 1/2] audit: Quit audit_free_names() early if name list empty Waiman Long
2026-01-21  2:35 ` [PATCH 2/2] audit: Call path_{put,get}() in audit_alloc_name()/audit_free_names() only when necessary Waiman Long
2026-01-21  3:11 ` [PATCH 1/2] audit: Quit audit_free_names() early if name list empty Al Viro
2026-01-21  4:08   ` Waiman Long
2026-01-21  5:26     ` Al Viro
2026-01-21 15:09       ` Waiman Long

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