From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753737AbbERLFn (ORCPT ); Mon, 18 May 2015 07:05:43 -0400 Received: from bombadil.infradead.org ([198.137.202.9]:35638 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752931AbbERLFe (ORCPT ); Mon, 18 May 2015 07:05:34 -0400 Date: Mon, 18 May 2015 13:05:13 +0200 From: Peter Zijlstra To: Ulrich Obergfell Cc: Michal Hocko , Linus Torvalds , Stephane Eranian , Don Zickus , Ingo Molnar , Andrew Morton , "Rafael J. Wysocki" , Kevin Hilman , Ulf Hansson , linux-pm@vger.kernel.org, LKML Subject: Re: suspend regression in 4.1-rc1 Message-ID: <20150518110513.GE21418@twins.programming.kicks-ass.net> References: <20150517185041.GA5897@dhcp22.suse.cz> <20150518073046.GO17717@twins.programming.kicks-ass.net> <20150518090336.GA6393@dhcp22.suse.cz> <20150518093150.GC21418@twins.programming.kicks-ass.net> <309071648.615900.1431946606778.JavaMail.zimbra@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <309071648.615900.1431946606778.JavaMail.zimbra@redhat.com> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, May 18, 2015 at 06:56:46AM -0400, Ulrich Obergfell wrote: > > Subject: watchdog: Fix merge 'conflict' > > > > Two watchdog changes that came through different trees had a non > > conflicting conflict, that is, one changed the semantics of a variable > > but no actual code conflict happened. So the merge appeared fine, but > > the resulting code did not behave as expected. > > > > Commit 195daf665a62 ("watchdog: enable the new user interface of the > > watchdog mechanism") changes the semantics of watchdog_user_enabled, > > which thereafter is only used by the functions introduced by > > b3738d293233 ("watchdog: Add watchdog enable/disable all functions"). > > Don and I already posted a patch in April to address this: > > https://lkml.org/lkml/2015/4/22/306 > http://ozlabs.org/~akpm/mmots/broken-out/watchdog-fix-watchdog_nmi_enable_all.patch Yeah, but it seems to have gotten lost on its way to Linus. > > There further appears to be a distinct lack of serialization between > > setting and using watchdog_enabled, so perhaps we should wrap the > > {en,dis}able_all() things in watchdog_proc_mutex. > > As I understand it, the {en,dis}able_all() functions are only called early > at kernel startup, so I do not see how they could be racing with watchdog > code that is executed in the context of write() system calls to parameters > in /proc/sys/kernel. Please see also my earlier reply to Michal for further > details: http://marc.info/?l=linux-pm&m=143194387208250&w=2 > > Do we really need synchronization here? Same argument as in my previous email; its best to implement exposed functions fully and correctly, irrespective of their usage sites. It costs little extra and might safe a few hairs down the lined. None of this is performance critical.