linux-kernel - Re: [RFC 1/2] watchdog: update watchdog attributes atomically

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]

Message-ID: <20130719163323.GB9759@dhcp22.suse.cz>
Date:	Fri, 19 Jul 2013 18:33:23 +0200
From:	Michal Hocko <mhocko@...e.cz>
To:	Don Zickus <dzickus@...hat.com>
Cc:	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...nel.org>
Subject: Re: [RFC 1/2] watchdog: update watchdog attributes atomically

On Fri 19-07-13 12:10:48, Don Zickus wrote:
> On Fri, Jul 19, 2013 at 11:04:58AM +0200, Michal Hocko wrote:
> > proc_dowatchdog doesn't synchronize multiple callers which
> > might lead to confusion when two parallel callers might confuse
> > watchdog_enable_all_cpus resp. watchdog_disable_all_cpus (e.g. watchdog
> > gets enabled even if watchdog_thresh was set to 0 already).
> > 
> > This patch adds a local mutex which synchronizes callers to the sysctl
> > handler.
> 
> Looks fine by me, except one little nitpick..
> 
> > 
> > Signed-off-by: Michal Hocko <mhocko@...e.cz>
> > ---
> >  kernel/watchdog.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/kernel/watchdog.c b/kernel/watchdog.c
> > index 1241d8c..2d64c02 100644
> > --- a/kernel/watchdog.c
> > +++ b/kernel/watchdog.c
> > @@ -520,13 +520,15 @@ int proc_dowatchdog(struct ctl_table *table, int write,
> >  		    void __user *buffer, size_t *lenp, loff_t *ppos)
> >  {
> >  	int err, old_thresh, old_enabled;
> > +	static DEFINE_MUTEX(watchdog_proc_mutex);
> 
> Should we just make this global instead of hiding it as a static inside a
> function.  I don't know the kernel rules for deciding which approach makes
> sense.  I know it is the same result in either case...

I've hidden it into the function to discourage from abusing it for
something else and because the usage is nicely focused in this
function. But I have no problem to pull it out.

-- 
Michal Hocko
SUSE Labs
--
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/