Re: [v3, 1/5] rfi-flush: Move the logic to avoid a redo into the debugfs code
On Wed, 2018-03-14 at 22:40:38 UTC, Mauricio Faria de Oliveira wrote: > From: Michael Ellerman> > rfi_flush_enable() includes a check to see if we're already > enabled (or disabled), and in that case does nothing. > > But that means calling setup_rfi_flush() a 2nd time doesn't actually > work, which is a bit confusing. > > Move that check into the debugfs code, where it really belongs. > > Signed-off-by: Michael Ellerman > Signed-off-by: Mauricio Faria de Oliveira Series applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/1e2a9fc7496955faacbbed49461d61 cheers
Re: [PATCH v3 1/5] rfi-flush: Move the logic to avoid a redo into the debugfs code
Hi Murilo and Michal, On 03/16/2018 05:52 AM, Michal Suchánek wrote: Do we need to take into account if no_rfi_flush == true? I think it makes sense you are able to override that using debugfs. It's interface used for diagnostics and testing. If this was in sysfs it would be a different story. Yes, I agree. The debugfs is way to override the cmdline option. Thanks for looking carefully at this :) cheers, Mauricio
Re: [PATCH v3 1/5] rfi-flush: Move the logic to avoid a redo into the debugfs code
On Thu, 15 Mar 2018 17:36:00 -0300 Murilo Opsfelder Araujowrote: > On 03/14/2018 07:40 PM, Mauricio Faria de Oliveira wrote: > > From: Michael Ellerman > > > > rfi_flush_enable() includes a check to see if we're already > > enabled (or disabled), and in that case does nothing. > > > > But that means calling setup_rfi_flush() a 2nd time doesn't actually > > work, which is a bit confusing. > > > > Move that check into the debugfs code, where it really belongs. > > > > Signed-off-by: Michael Ellerman > > Signed-off-by: Mauricio Faria de Oliveira > > --- > > arch/powerpc/kernel/setup_64.c | 13 - > > 1 file changed, 8 insertions(+), 5 deletions(-) > > > > diff --git a/arch/powerpc/kernel/setup_64.c > > b/arch/powerpc/kernel/setup_64.c index c388cc3..3efc01a 100644 > > --- a/arch/powerpc/kernel/setup_64.c > > +++ b/arch/powerpc/kernel/setup_64.c > > @@ -846,9 +846,6 @@ static void do_nothing(void *unused) > > > > void rfi_flush_enable(bool enable) > > { > > - if (rfi_flush == enable) > > - return; > > - > > if (enable) { > > do_rfi_flush_fixups(enabled_flush_types); > > on_each_cpu(do_nothing, NULL, 1); > > @@ -902,13 +899,19 @@ void __init setup_rfi_flush(enum > > l1d_flush_type types, bool enable) #ifdef CONFIG_DEBUG_FS > > static int rfi_flush_set(void *data, u64 val) > > { > > + bool enable; > > + > > if (val == 1) > > - rfi_flush_enable(true); > > + enable = true; > > else if (val == 0) > > - rfi_flush_enable(false); > > + enable = false; > > else > > return -EINVAL; > > > > + /* Only do anything if we're changing state */ > > + if (enable != rfi_flush) > > Hi, Mauricio. > > Do we need to take into account if no_rfi_flush == true? I think it makes sense you are able to override that using debugfs. It's interface used for diagnostics and testing. If this was in sysfs it would be a different story. Thanks Michal
Re: [PATCH v3 1/5] rfi-flush: Move the logic to avoid a redo into the debugfs code
On 03/14/2018 07:40 PM, Mauricio Faria de Oliveira wrote: > From: Michael Ellerman> > rfi_flush_enable() includes a check to see if we're already > enabled (or disabled), and in that case does nothing. > > But that means calling setup_rfi_flush() a 2nd time doesn't actually > work, which is a bit confusing. > > Move that check into the debugfs code, where it really belongs. > > Signed-off-by: Michael Ellerman > Signed-off-by: Mauricio Faria de Oliveira > --- > arch/powerpc/kernel/setup_64.c | 13 - > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c > index c388cc3..3efc01a 100644 > --- a/arch/powerpc/kernel/setup_64.c > +++ b/arch/powerpc/kernel/setup_64.c > @@ -846,9 +846,6 @@ static void do_nothing(void *unused) > > void rfi_flush_enable(bool enable) > { > - if (rfi_flush == enable) > - return; > - > if (enable) { > do_rfi_flush_fixups(enabled_flush_types); > on_each_cpu(do_nothing, NULL, 1); > @@ -902,13 +899,19 @@ void __init setup_rfi_flush(enum l1d_flush_type types, > bool enable) > #ifdef CONFIG_DEBUG_FS > static int rfi_flush_set(void *data, u64 val) > { > + bool enable; > + > if (val == 1) > - rfi_flush_enable(true); > + enable = true; > else if (val == 0) > - rfi_flush_enable(false); > + enable = false; > else > return -EINVAL; > > + /* Only do anything if we're changing state */ > + if (enable != rfi_flush) Hi, Mauricio. Do we need to take into account if no_rfi_flush == true? if ((enable != rfi_flush) && !no_rfi_flush) > + rfi_flush_enable(enable); > + > return 0; > } > Cheers Murilo
[PATCH v3 1/5] rfi-flush: Move the logic to avoid a redo into the debugfs code
From: Michael Ellermanrfi_flush_enable() includes a check to see if we're already enabled (or disabled), and in that case does nothing. But that means calling setup_rfi_flush() a 2nd time doesn't actually work, which is a bit confusing. Move that check into the debugfs code, where it really belongs. Signed-off-by: Michael Ellerman Signed-off-by: Mauricio Faria de Oliveira --- arch/powerpc/kernel/setup_64.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c index c388cc3..3efc01a 100644 --- a/arch/powerpc/kernel/setup_64.c +++ b/arch/powerpc/kernel/setup_64.c @@ -846,9 +846,6 @@ static void do_nothing(void *unused) void rfi_flush_enable(bool enable) { - if (rfi_flush == enable) - return; - if (enable) { do_rfi_flush_fixups(enabled_flush_types); on_each_cpu(do_nothing, NULL, 1); @@ -902,13 +899,19 @@ void __init setup_rfi_flush(enum l1d_flush_type types, bool enable) #ifdef CONFIG_DEBUG_FS static int rfi_flush_set(void *data, u64 val) { + bool enable; + if (val == 1) - rfi_flush_enable(true); + enable = true; else if (val == 0) - rfi_flush_enable(false); + enable = false; else return -EINVAL; + /* Only do anything if we're changing state */ + if (enable != rfi_flush) + rfi_flush_enable(enable); + return 0; } -- 2.7.4