Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
 kernel/exit.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/exit.c b/kernel/exit.c
index 1eb2e7d36ce4..257dd8ed45ea 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -248,9 +248,10 @@ void release_task(struct task_struct *p)
 
 	cgroup_release(p);
 
+	thread_pid = get_pid(p->thread_pid);
+
 	write_lock_irq(&tasklist_lock);
 	ptrace_release_task(p);
-	thread_pid = get_pid(p->thread_pid);
 	__exit_signal(p);
 
 	/*
-- 
2.43.0

* Mateusz Guzik <mjguzik@gmail.com> [250201 11:31]:
> Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>

Change log is a bit sparse?  I get that the subject spells out what is
done, but anything to say here at all?  Reduces lock contention by
reducing lock time or something?  Maybe even some numbers you have in
the cover letter?

> ---
>  kernel/exit.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/exit.c b/kernel/exit.c
> index 1eb2e7d36ce4..257dd8ed45ea 100644
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -248,9 +248,10 @@ void release_task(struct task_struct *p)
>  
>  	cgroup_release(p);
>  
> +	thread_pid = get_pid(p->thread_pid);
> +
>  	write_lock_irq(&tasklist_lock);
>  	ptrace_release_task(p);
> -	thread_pid = get_pid(p->thread_pid);
>  	__exit_signal(p);
>  
>  	/*
> -- 
> 2.43.0
> 
>

On Mon, Feb 3, 2025 at 8:27 PM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
>
> * Mateusz Guzik <mjguzik@gmail.com> [250201 11:31]:
> > Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
>
> Change log is a bit sparse?  I get that the subject spells out what is
> done, but anything to say here at all?  Reduces lock contention by
> reducing lock time or something?  Maybe even some numbers you have in
> the cover letter?
>

I did not measure this bit *specifically*.

As one can expect get_pid issues an atomic and that's slow. And since
it can happen *prior* to taking the global lock it seems like an
obvious little nit to sort out.

I would argue the change is self-explanatory given the cover-letter.

-- 
Mateusz Guzik <mjguzik gmail.com>

* Mateusz Guzik <mjguzik@gmail.com> [250203 14:36]:
> On Mon, Feb 3, 2025 at 8:27 PM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> >
> > * Mateusz Guzik <mjguzik@gmail.com> [250201 11:31]:
> > > Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
> >
> > Change log is a bit sparse?  I get that the subject spells out what is
> > done, but anything to say here at all?  Reduces lock contention by
> > reducing lock time or something?  Maybe even some numbers you have in
> > the cover letter?
> >
> 
> I did not measure this bit *specifically*.
> 
> As one can expect get_pid issues an atomic and that's slow. And since
> it can happen *prior* to taking the global lock it seems like an
> obvious little nit to sort out.
> 
> I would argue the change is self-explanatory given the cover-letter.

But when you git blame on the file, you will not see that cover letter.

> 
> -- 
> Mateusz Guzik <mjguzik gmail.com>

On Mon, Feb 3, 2025 at 9:14 PM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
>
> * Mateusz Guzik <mjguzik@gmail.com> [250203 14:36]:
> > On Mon, Feb 3, 2025 at 8:27 PM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> > >
> > > * Mateusz Guzik <mjguzik@gmail.com> [250201 11:31]:
> > > > Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
> > >
> > > Change log is a bit sparse?  I get that the subject spells out what is
> > > done, but anything to say here at all?  Reduces lock contention by
> > > reducing lock time or something?  Maybe even some numbers you have in
> > > the cover letter?
> > >
> >
> > I did not measure this bit *specifically*.
> >
> > As one can expect get_pid issues an atomic and that's slow. And since
> > it can happen *prior* to taking the global lock it seems like an
> > obvious little nit to sort out.
> >
> > I would argue the change is self-explanatory given the cover-letter.
>
> But when you git blame on the file, you will not see that cover letter.

if this lands, I presume it is going to go through Andrew who uses
tooling pulling in the cover letter for each commit

but i'm not going to argue this bit, just provide with a commit
message which you think works and I'll use it

-- 
Mateusz Guzik <mjguzik gmail.com>

On Mon, 3 Feb 2025 21:22:31 +0100 Mateusz Guzik <mjguzik@gmail.com> wrote:

> On Mon, Feb 3, 2025 at 9:14 PM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> >
> > * Mateusz Guzik <mjguzik@gmail.com> [250203 14:36]:
> > > On Mon, Feb 3, 2025 at 8:27 PM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> > > >
> > > > * Mateusz Guzik <mjguzik@gmail.com> [250201 11:31]:
> > > > > Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
> > > >
> > > > Change log is a bit sparse?  I get that the subject spells out what is
> > > > done, but anything to say here at all?  Reduces lock contention by
> > > > reducing lock time or something?  Maybe even some numbers you have in
> > > > the cover letter?
> > > >
> > >
> > > I did not measure this bit *specifically*.
> > >
> > > As one can expect get_pid issues an atomic and that's slow. And since
> > > it can happen *prior* to taking the global lock it seems like an
> > > obvious little nit to sort out.
> > >
> > > I would argue the change is self-explanatory given the cover-letter.
> >
> > But when you git blame on the file, you will not see that cover letter.
> 
> if this lands, I presume it is going to go through Andrew who uses
> tooling pulling in the cover letter for each commit

No, I copy the [0/n] description into the [1/n] changelog only.

> but i'm not going to argue this bit, just provide with a commit
> message which you think works and I'll use it

It's rather irregular to ask Liam to explain your patch!

The Subject: describes what the patch does (which was obvious from the
code anyway) but the changelog fails to explain *why* the change was
made.  You've explained "why" adequately within this discussion so I
suggest you condense those words into this patch's changelog.


General muse: if a reviewer asks questions regarding a patch then we
should treat those questions as bug reports against the changelogs and
code comments: required information is missing.  So please let's go
through reviewer questions as we prepare the next revision of a
patchset and make sure that all those questions are fully answered,

* Mateusz Guzik <mjguzik@gmail.com> [250203 15:22]:
> On Mon, Feb 3, 2025 at 9:14 PM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> >
> > * Mateusz Guzik <mjguzik@gmail.com> [250203 14:36]:
> > > On Mon, Feb 3, 2025 at 8:27 PM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> > > >
> > > > * Mateusz Guzik <mjguzik@gmail.com> [250201 11:31]:
> > > > > Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
> > > >
> > > > Change log is a bit sparse?  I get that the subject spells out what is
> > > > done, but anything to say here at all?  Reduces lock contention by
> > > > reducing lock time or something?  Maybe even some numbers you have in
> > > > the cover letter?
> > > >
> > >
> > > I did not measure this bit *specifically*.
> > >
> > > As one can expect get_pid issues an atomic and that's slow. And since
> > > it can happen *prior* to taking the global lock it seems like an
> > > obvious little nit to sort out.
> > >
> > > I would argue the change is self-explanatory given the cover-letter.
> >
> > But when you git blame on the file, you will not see that cover letter.
> 
> if this lands, I presume it is going to go through Andrew who uses
> tooling pulling in the cover letter for each commit
> 
> but i'm not going to argue this bit, just provide with a commit
> message which you think works and I'll use it

Your comment above states why this is beneficial.  Why not use a
variation of your statement of get_pid issuing an atomic which can be
removed from the locking area?  This seems like an important detail to
capture.

> 
> -- 
> Mateusz Guzik <mjguzik gmail.com>