[RFC,06/86] Revert "entry: Fix compile error in dynamic_irqentry_exit_cond_resched()"

Commit Message

Ankur Arora Nov. 7, 2023, 9:56 p.m. UTC

This reverts commit 0a70045ed8516dfcff4b5728557e1ef3fd017c53.

Signed-off-by: Ankur Arora <[e-mail]>
---
 kernel/entry/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Greg KH Nov. 8, 2023, 7:47 a.m. UTC | #1

On Tue, Nov 07, 2023 at 01:56:52PM -0800, Ankur Arora wrote:
> This reverts commit 0a70045ed8516dfcff4b5728557e1ef3fd017c53.
> 

None of these reverts say "why" the revert is needed, or why you even
want to do this at all.  Reverting a compilation error feels like you
are going to be adding a compilation error to the build, which is
generally considered a bad thing :(

So, more information please.

thanks,

greg k-h

Ankur Arora Nov. 8, 2023, 9:09 a.m. UTC | #2

Greg KH <[e-mail]> writes:

> On Tue, Nov 07, 2023 at 01:56:52PM -0800, Ankur Arora wrote:
>> This reverts commit 0a70045ed8516dfcff4b5728557e1ef3fd017c53.
>>
>
> None of these reverts say "why" the revert is needed, or why you even
> want to do this at all.  Reverting a compilation error feels like you
> are going to be adding a compilation error to the build, which is
> generally considered a bad thing :(

Yeah, one of the many issues with this string of reverts.

I was concerned about repeating the same thing over and over enough
that I just put my explanation at the bottom of the cover-letter and
nowhere else.

The reasoning was this:

The PREEMPT_DYNAMIC code uses the static_calls to dynamically
switch between voluntary and full preemption.

Thomas had outlined an approach (see https://lore.kernel.org/lkml/87cyyfxd4k.ffs@tglx/)
(which this series implements) where instead of depending on
cond_resched(), a none/voluntary/full preemption model could be enforced
by the scheduler. And, this could be done without needing the cond_resched()
preemption points. And, thus also wouldn't need the PREEMPT_DYNAMIC logic.

But, as Steven Rostedt pointed out to me that reverting this code was
all wrong. Since, there's nothing wrong with the logic, it makes sense
to just extract out the bits that are incompatible instead of reverting
functioning code.

Will do that when I send out the next version.

Thanks

--
ankur

Greg KH Nov. 8, 2023, 10 a.m. UTC | #3

On Wed, Nov 08, 2023 at 01:09:10AM -0800, Ankur Arora wrote:
> 
> Greg KH <[e-mail]> writes:
> 
> > On Tue, Nov 07, 2023 at 01:56:52PM -0800, Ankur Arora wrote:
> >> This reverts commit 0a70045ed8516dfcff4b5728557e1ef3fd017c53.
> >>
> >
> > None of these reverts say "why" the revert is needed, or why you even
> > want to do this at all.  Reverting a compilation error feels like you
> > are going to be adding a compilation error to the build, which is
> > generally considered a bad thing :(
> 
> Yeah, one of the many issues with this string of reverts.
> 
> I was concerned about repeating the same thing over and over enough
> that I just put my explanation at the bottom of the cover-letter and
> nowhere else.

cover letters are not in the changelog when patches are committed :)

thanks,

greg k-h

Patch

diff --git a/kernel/entry/common.c b/kernel/entry/common.c
index d7ee4bc3f2ba..ba684e9853c1 100644
--- a/kernel/entry/common.c
+++ b/kernel/entry/common.c
@@ -396,7 +396,7 @@  DEFINE_STATIC_CALL(irqentry_exit_cond_resched, raw_irqentry_exit_cond_resched);
 DEFINE_STATIC_KEY_TRUE(sk_dynamic_irqentry_exit_cond_resched);
 void dynamic_irqentry_exit_cond_resched(void)
 {
-	if (!static_branch_unlikely(&sk_dynamic_irqentry_exit_cond_resched))
+	if (!static_key_unlikely(&sk_dynamic_irqentry_exit_cond_resched))
 		return;
 	raw_irqentry_exit_cond_resched();
 }