mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Steve Wahl <steve.wahl@hpe.com>
Cc: Justin Ernst <justin.ernst@hpe.com>,
	Kyle Meyer <kyle.meyer@hpe.com>,
	Dimitri Sivanich <dimitri.sivanich@hpe.com>,
	Russ Anderson <russ.anderson@hpe.com>,
	Darren Hart <dvhart@infradead.org>,
	Andy Shevchenko <andy@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	"H . Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	Justin Stitt <justinstitt@google.com>
Subject: Re: [PATCH] x86/platform/uv: Use sysfs_match_string() for string parsing in param_set_action()
Date: Wed, 13 Sep 2023 19:01:31 +0200	[thread overview]
Message-ID: <4f41e78e-5e47-0726-c64a-82559d1f799b@redhat.com> (raw)
In-Reply-To: <ZQHpC/oKLwfJuvRu@swahl-linux>

Hi Steve,

On 9/13/23 18:56, Steve Wahl wrote:
> On Wed, Sep 13, 2023 at 05:16:56PM +0200, Hans de Goede wrote:
>> Remove the custom, hard to read code to:
>>
>> 1. Make a copy of "val" with any potential '\n' at the end stripped
>> 2. Compare the copy against an array of allowed string values
>>
>> Linux has the sysfs_match_string() helper exactly for cases like this,
>> switch to this.
> 
> Hans,
> 
> I like this patch, compiling and testing now.
> 
> I was wondering, as long as we're in the neighborhood, how you feel
> about changing the stored variable uv_nmi_action to an int or enum
> rather than a string, since it can only be one of 6 values, and the
> string compare while processing an NMI strikes me as inefficent.
> 
> It could extend this patch, or be done as a follow on.  And I'm
> willing to supply the effort if you want me to.

I must admit I did not look at the code consuming uv_nmi_action
and I did wonder why this was not an enum from day 1.

I'll prepare a v2 of this patch which changes uv_nmi_action
to an enum.

Note I can compile test this only, so I gope you will be able to
test the v2 a bit more thoroughly :)

Regards,

Hans



  reply	other threads:[~2023-09-13 17:02 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-13 15:16 Hans de Goede
2023-09-13 16:56 ` Steve Wahl
2023-09-13 17:01   ` Hans de Goede [this message]
2023-09-13 17:03     ` Steve Wahl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4f41e78e-5e47-0726-c64a-82559d1f799b@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=andy@kernel.org \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=dimitri.sivanich@hpe.com \
    --cc=dvhart@infradead.org \
    --cc=hpa@zytor.com \
    --cc=justin.ernst@hpe.com \
    --cc=justinstitt@google.com \
    --cc=kyle.meyer@hpe.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=russ.anderson@hpe.com \
    --cc=steve.wahl@hpe.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®