[PATCH] include/linux: Fix outdated comment on entry-common.h

[PATCH] include/linux: Fix outdated comment on entry-common.h

[Khalid Ali]

From: Khalid Ali <khaliidcaliy@gmail.com>

On most calls to this function, syscall_enter_from_user_mode_prepare()
never get called as the comment indicates.

Privious kernel version i used to see things happen as the function documentation
indicated (syscall_enter_from_user_mode_prepare() called before
syscall_enter_from_user_mode_work), however it seems now some things
have changed which makes that point irrevelant. Most preparations that
function does is handled manually by enter_from_user_mode() and
some other places. So this makes it misleading.

The point is remove strict function call indication on documentation as might be outdated
one day in the future. There multiple places currently called
syscall_enter_from_user_mode_work() without
syscall_enter_from_user_mode_prepare() get called so that will make it
condition not met.

Signed-off-by: Khalid Ali <khaliidcaliy@gmail.com>
---
 include/linux/entry-common.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h
index f94f3fdf15fc..8b52551600cf 100644
--- a/include/linux/entry-common.h
+++ b/include/linux/entry-common.h
@@ -146,8 +146,7 @@ long syscall_trace_enter(struct pt_regs *regs, long syscall,
  * @syscall:	The syscall number
  *
  * Invoked from architecture specific syscall entry code with interrupts
- * enabled after invoking syscall_enter_from_user_mode_prepare() and extra
- * architecture specific work.
+ * enabled and extra architecture specific work.
  *
  * Returns: The original or a modified syscall number
  *
-- 
2.49.0

Re: [PATCH] include/linux: Fix outdated comment on entry-common.h

[Thomas Gleixner]

On Sun, Jun 08 2025 at 19:48, Khalid Ali wrote:

'include/linux:' is not a valid prefix. See the documentation I pointed
you to in the other reply.

> From: Khalid Ali <khaliidcaliy@gmail.com>
>
> On most calls to this function, syscall_enter_from_user_mode_prepare()
> never get called as the comment indicates.
>
> Privious kernel version i used to see things happen as the function documentation
> indicated (syscall_enter_from_user_mode_prepare() called before
> syscall_enter_from_user_mode_work), however it seems now some things
> have changed which makes that point irrevelant. Most preparations that
> function does is handled manually by enter_from_user_mode() and
> some other places. So this makes it misleading.
>
> The point is remove strict function call indication on documentation as might be outdated
> one day in the future. There multiple places currently called
> syscall_enter_from_user_mode_work() without
> syscall_enter_from_user_mode_prepare() get called so that will make it
> condition not met.

That's again incomprehensible word salad.

You are right that the comment is not longer accurate, but your fixup
makes it even worse.

The real condition for calling this function is:

    1) enter_from_user_mode() has been invoked

    2) interrupts have been enabled

    3) Architecture specific work has been done

#1 must be the first thing. #2 and #3 have no ordering requirements.

syscall_enter_from_user_mode_prepare() does #1 and #2 together and in
the original implementation this was used in more places. So yes, the
comment is outdated, but it needs to describe the above requirements and
not something pulled out of thin air.

Thanks,

        tglx

Re: [PATCH] include/linux: Fix outdated comment on entry-common.h

[Khalid Ali]

> 'include/linux:' is not a valid prefix. See the documentation I pointed
> you to in the other reply.
>
> > From: Khalid Ali <khaliidcaliy@gmail.com>
> >
> > On most calls to this function, syscall_enter_from_user_mode_prepare()
> > never get called as the comment indicates.
> >
> > Privious kernel version i used to see things happen as the function documentation
> > indicated (syscall_enter_from_user_mode_prepare() called before
> > syscall_enter_from_user_mode_work), however it seems now some things
> > have changed which makes that point irrevelant. Most preparations that
> > function does is handled manually by enter_from_user_mode() and
> > some other places. So this makes it misleading.
> >
> > The point is remove strict function call indication on documentation as might be outdated
> > one day in the future. There multiple places currently called
> > syscall_enter_from_user_mode_work() without
> > syscall_enter_from_user_mode_prepare() get called so that will make it
> > condition not met.
>
> That's again incomprehensible word salad.
>
> You are right that the comment is not longer accurate, but your fixup
> makes it even worse.
>
> The real condition for calling this function is:
>
>    1) enter_from_user_mode() has been invoked
>
>    2) interrupts have been enabled
>
>    3) Architecture specific work has been done
>
> #1 must be the first thing. #2 and #3 have no ordering requirements.
>
> syscall_enter_from_user_mode_prepare() does #1 and #2 together and in
> the original implementation this was used in more places. So yes, the
> comment is outdated, but it needs to describe the above requirements and
> not something pulled out of thin air.
>
> Thanks,
>
>       tglx

Thanks and noted, however just asking why syscall_enter_from_user_mode() isn't calling
syscall_enter_from_user_mode_prepare() i don't get it why #1 and #2 is redone, is it because
of instrumentations since syscall_enter_from_user_mode_prepare() is calling instrumentation_end()?

Maybe we need to determine if instrumentation_end() is neccessary for syscall_enter_from_user_mode_prepare().
As i know the only place where syscall_enter_from_user_mode_prepare() is called is arch/x86/entry/syscall_32.c,
on that source when the function returns they begin the instrumentation again using instrumentation_begin(). So i think
with little adjusment of that source file and removing instrumentation_end() on syscall_enter_from_user_mode_prepare()
then we can use syscall_enter_from_user_mode_prepare() as we did. Yet don't know the reason, however suspect some duplication.

Another thing i should indicate if you don't agree with me is, can we change the comment the function to enter_from_user_mode(), 
and with little adjusment make steps as you mentioned.

Let me know if these ideas above right and whether i should keep going on updating the comment.
Thanks, Khalid ALi

Re: [PATCH] include/linux: Fix outdated comment on entry-common.h

[Thomas Gleixner]

On Fri, Jun 13 2025 at 20:58, Khalid Ali wrote:
> Thanks and noted, however just asking why syscall_enter_from_user_mode() isn't calling
> syscall_enter_from_user_mode_prepare() i don't get it why #1 and #2 is redone, is it because
> of instrumentations since syscall_enter_from_user_mode_prepare() is calling instrumentation_end()?

Nothing is redone. All call sites of syscall_enter_from_user_mode() have
to:

    1) invoke enter_from_user_mode()

    2) enable interrupts

in that very order.

syscall_enter_from_user_mode_prepare() is a helper function which
combines both. It was more widely used in the early implementations of
this infrastructure, but it's usage got reduced to one call site.

All other call sites invoke enter_from_user_mode() and then enable
interrupts before calling syscall_enter_from_user_mode().

That has nothing to do with instrumentation_end(). See
Documentation/core-api/entry.rst for an explanation of noinstr and
instrumentation_begin/end().

> Maybe we need to determine if instrumentation_end() is neccessary for
> syscall_enter_from_user_mode_prepare().

It's already determined. See documentation...

> As i know the only place where syscall_enter_from_user_mode_prepare()
> is called is arch/x86/entry/syscall_32.c, on that source when the
> function returns they begin the instrumentation again using
> instrumentation_begin(). So i think with little adjusment of that
> source file and removing instrumentation_end() on
> syscall_enter_from_user_mode_prepare() then we can use
> syscall_enter_from_user_mode_prepare() as we did. Yet don't know the
> reason, however suspect some duplication.

Can you please stop making uninformed assumptions? It's documented how
this works and there is neither duplication nor anything you can remove.

> Another thing i should indicate if you don't agree with me is, can we
> change the comment the function to enter_from_user_mode(), and with
> little adjusment make steps as you mentioned.

Did you read what I wrote:

>> So yes, the comment is outdated, but it needs to describe the above
>> requirements and not something pulled out of thin air.

?

Thanks,

        tglx

Re: [PATCH] include/linux: Fix outdated comment on entry-common.h

[Khalid Ali]

> Nothing is redone. All call sites of syscall_enter_from_user_mode() have
> to:
>
>    1) invoke enter_from_user_mode()
>
>    2) enable interrupts
>
> in that very order.
>
> syscall_enter_from_user_mode_prepare() is a helper function which
> combines both. It was more widely used in the early implementations of
> this infrastructure, but it's usage got reduced to one call site.

Please try to understand me before you make malformed responses too.

My point is simple and it is, why?

Currently neither you nor the documentation explained the technical reasons why 
syscall_enter_from_user_mode_prepare() was ommited and i was trying my best to understand
why?
If syscall_enter_from_user_mode() is doing #1 #2 when syscall_enter_from_user_mode_prepare()
already doing it then why it is not calling it. This is why i said it was redone.

> All other call sites invoke enter_from_user_mode() and then enable
> interrupts before calling syscall_enter_from_user_mode().

Honestly, my patch and my discussion doesn't involve about it's call sites.
My discussion is simple, either we update the comment if you disagree with me and the reason,
or we can simply use syscall_enter_from_user_mode_prepare() on syscall_enter_from_user_mode()
if you don't have strong reason to avoid it, then nothing happens because syscall_enter_from_user_mode_prepare()
internally calls enter_from_user_mode() and enables interrupts. syscall_enter_from_user_mode() also calls
enter_from_user_mode() directly also enables interrups, so if you don't know please check on your side also.

> That has nothing to do with instrumentation_end(). See
> Documentation/core-api/entry.rst for an explanation of noinstr and
> instrumentation_begin/end().

This one i was wrong, and i understand now. So forget about it.

My point was clear, the two steps already syscall_enter_from_user_mode_prepare() did and 
unless you tell me a clear reason instead of "we ommited for a reason" which you repeatly telling me.

> Did you read what I wrote: 
Again, my point is clear so try to understand as it is, then refuse or accept with clear reasons.

> ?

I don't know if what i asked you is hard and shouldn't be asked.

You must understand i didn't come here to argue with you or to annoy you. If you disagree with me these points i 
proposed to you and also disagree with the doc patch i said to you change enter_from_user_mode() instead 
of which makes up to date as i think because you didn't tell me why neither the doc you provided did. I think maybe 
about the doc just good English is enough instead of putting unused functions on the doc.

what are you proposing then, maybe you already have some way better than mine? OR
Tell me what are you expecting from me then?
Do i leave it like that, outdated and let mislead document readers?

Thanks,
Khalid Ali

Re: [PATCH] kernel/entry: Remove some redundancy checks on syscall works

[Thomas Gleixner]

Can you please reply to the mail you received, so that there are proper
In-Reply-To and References tags in the mail, which are required for mail
threading?

I almost missed your replies because they ended up as single mail
threads without reference somewhere in my endless mail pile.

On Fri, Jun 13 2025 at 20:28, Khalid Ali wrote:

> First if we are talking about performance then we may need likely() on
> SYSCALL_WORK_ENTER since the probability of condition evaluating as
> true is very high.

That depends on the system configuration scenario and the likely() has
been omitted on purpose.

> Second syscall_enter_audit() missing SYSCALL_WORK_SYSCALL_AUDIT
> evaluation, aren't we supposed to call it only if
> SYSCALL_WORK_SYSCALL_AUDIT is set?

That's redundant as syscall_enter_audit() checks for a valid audit
context already. Both are valid indicators and go in lockstep. So it
might be arguable that evaluating the work bit is cheaper than the
context check, but I doubt it's measurable.

Thanks,

        tglx

Re: [PATCH] include/linux: Fix outdated comment on entry-common.h

[Khalid Ali]

> That's redundant as syscall_enter_audit() checks for a valid audit
> context already. Both are valid indicators and go in lockstep. So it
> might be arguable that evaluating the work bit is cheaper than the
> context check, but I doubt it's measurable.

This will also increase readability and make things more consistent. So i think 
we should use  SYSCALL_WORK_SYSCALL_AUDIT bit to check if we need to do auditing,
instead of checking audit context, on both entry and exit.

So should i do with another patch, what i am verifying is if we really agree with one point.

Thanks
Khalid Ali