On Mon, Dec 03, 2018 at 06:05:57PM +0000, James Morse wrote:
> Refactor the estatus queue's pool notification routine from
> NOTIFY_NMI's handlers. This will allow another notification
> method to use the estatus queue without duplicating this code.
>
> This patch adds rcu_read_lock()/rcu_read_unlock() around the list
s/This patch adds/Add/
> list_for_each_entry_rcu() walker. These aren't strictly necessary as
> the whole nmi_enter/nmi_exit() window is a spooky RCU read-side
> critical section.
>
> _in_nmi_notify_one() is separate from the rcu-list walker for a later
> caller that doesn't need to walk a list.
>
> Signed-off-by: James Morse <james.morse@xxxxxxx>
> Reviewed-by: Punit Agrawal <punit.agrawal@xxxxxxx>
> Tested-by: Tyler Baicar <tbaicar@xxxxxxxxxxxxxx>
>
> ---
> Changes since v6:
> * Removed pool grow/remove code as this is no longer necessary.
>
> Changes since v3:
> * Removed duplicate or redundant paragraphs in commit message.
> * Fixed the style of a zero check.
> Changes since v1:
> * Tidied up _in_nmi_notify_one().
> ---
> drivers/acpi/apei/ghes.c | 63 ++++++++++++++++++++++++++--------------
> 1 file changed, 41 insertions(+), 22 deletions(-)
...
> +static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
> +{
> + int ret = NMI_DONE;
> +
> + if (!atomic_add_unless(&ghes_in_nmi, 1, 1))
> + return ret;
> +
> + if (!ghes_estatus_queue_notified(&ghes_nmi))
> + ret = NMI_HANDLED;
So this reads kinda the other way around, at least to me:
"if the queue was *not* notified, the NMI was handled."
Maybe rename to this:
err = process_queue(&ghes_nmi);
if (!err)
ret = NMI_HANDLED;
to make it clearer...
And yeah, all those static functions having "ghes_" prefix is just
encumbering readability for no good reason.
Thx.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
