linux-kernel - Re: [PATCH RT v2] x86/mce: Defer mce wakeups to threads for PREEMPT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 02 May 2013 10:26:53 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	RT <linux-rt-users@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Clark Williams <clark@...hat.com>,
	John Kacur <jkacur@...hat.com>,
	Tony Luck <tony.luck@...el.com>,
	Borislav Petkov <bp@...en8.de>,
	Mauro Carvalho Chehab <mchehab@...hat.com>,
	Ingo Molnar <mingo@...nel.org>,
	"H. Peter Anvin" <hpa@...ux.intel.com>
Subject: Re: [PATCH RT v2] x86/mce: Defer mce wakeups to threads for
 PREEMPT_RT

Grumble, somehow these emails got lost in the crowd.

On Fri, 2013-04-26 at 10:24 +0200, Sebastian Andrzej Siewior wrote:
> * Steven Rostedt | 2013-04-11 14:33:34 [-0400]:
> 
> >diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
> >index e8d8ad0..060e473 100644
> >--- a/arch/x86/kernel/cpu/mcheck/mce.c
> >+++ b/arch/x86/kernel/cpu/mcheck/mce.c
> >@@ -1308,6 +1309,61 @@ static void mce_do_trigger(struct work_struct *work)
> > 
> > static DECLARE_WORK(mce_trigger_work, mce_do_trigger);
> > 
> >+static void __mce_notify_work(void)
> >+{
> >+	/* Not more than two messages every minute */
> >+	static DEFINE_RATELIMIT_STATE(ratelimit, 60*HZ, 2);
> >+
> >+	/* wake processes polling /dev/mcelog */
> >+	wake_up_interruptible(&mce_chrdev_wait);
> >+
> >+	/*
> >+	 * There is no risk of missing notifications because
> >+	 * work_pending is always cleared before the function is
> >+	 * executed.
> >+	 */
> >+	if (mce_helper[0] && !work_pending(&mce_trigger_work))
> >+		schedule_work(&mce_trigger_work);
> 
> Why is here this work_pending() check? You can't enqueue a work item
> twice.

Yep, that doesn't look needed. Looking at the current code we have this
commit:

commit 4d899be584d4b4c5d6b49d655176b25cebf6ff1a
Author: Tejun Heo <tj@...nel.org>
Date:   Fri Dec 21 17:57:05 2012 -0800

    x86/mce: don't use [delayed_]work_pending()
    
    There's no need to test whether a (delayed) work item in pending
    before queueing, flushing or cancelling it.  Most uses are unnecessary
    and quite a few of them are buggy.
    
    Remove unnecessary pending tests from x86/mce.  Only compile tested.
    
    v2: Local var work removed from mce_schedule_work() as suggested by
        Borislav.


-- Steve


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/