Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1422796AbbD2JuU (ORCPT ); Wed, 29 Apr 2015 05:50:20 -0400 Received: from mx5-phx2.redhat.com ([209.132.183.37]:38034 "EHLO mx5-phx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1422705AbbD2JuR (ORCPT ); Wed, 29 Apr 2015 05:50:17 -0400 Date: Wed, 29 Apr 2015 05:49:22 -0400 (EDT) From: Ulrich Obergfell To: Chris Metcalf Cc: Frederic Weisbecker , Don Zickus , Ingo Molnar , Andrew Morton , Andrew Jones , Fabian Frederick , Aaron Tomlin , Ben Zhang , Christoph Lameter , Gilad Ben-Yossef , Steven Rostedt , linux-kernel@vger.kernel.org, Jonathan Corbet , linux-doc@vger.kernel.org, Thomas Gleixner , Peter Zijlstra Message-ID: <1809970915.9080500.1430300962442.JavaMail.zimbra@redhat.com> In-Reply-To: <553FCC7F.9030603@ezchip.com> References: <20150416152808.GA16270@lerouge> <1429295838-6328-1-git-send-email-cmetcalf@ezchip.com> <1429295838-6328-2-git-send-email-cmetcalf@ezchip.com> <1539023152.4035475.1429619553058.JavaMail.zimbra@redhat.com> <553FCC7F.9030603@ezchip.com> Subject: Re: [PATCH v9 2/3] watchdog: add watchdog_cpumask sysctl to assist nohz MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [10.36.7.160] X-Mailer: Zimbra 8.0.6_GA_5922 (ZimbraWebClient - FF22 (Linux)/8.0.6_GA_5922) Thread-Topic: watchdog: add watchdog_cpumask sysctl to assist nohz Thread-Index: riXJaeXr+VsLuxeOv7bMuaUn3OWF2w== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2337 Lines: 60 ----- Original Message ----- From: "Chris Metcalf" [...] On 04/21/2015 08:32 AM, Ulrich Obergfell wrote: >> Chris, >> >> in v9, smpboot_update_cpumask_percpu_thread() allocates 'tmp' mask dynamically. >> This allocation can fail and thus the function can now return an error. However, >> this error is being ignored by proc_watchdog_cpumask(). > > Yes, I did that intentionally, because it seemed like a pretty extreme > corner case (not enough memory to allocate one cpumask), and a relatively > unproblematic outcome (we don't actually modify the running set of watchdog > threads the way the /proc knob requested). > > The problem with your proposal (to save the old cpumask and put it back on > failure) is that we will almost certainly not be able to do that either > if we can't successfully run smpboot_update_cpumask_percpu_thread(), > since that's exactly the allocation that we're presuming is going to fail > internally. > > I went down this rathole and decided it wasn't worth worrying about. > Let me know if you think we need to beat on it some more :-) Chris, the other handlers for the watchdog parameters in /proc restore the original value on failure, so I thought it would be nice to make the error handling consistent in that regard. However, on the other hand the 'watchdog_cpumask' parameter is kind of an exception in terms of when and how it should be used, and thus it's probably OK if this interface is less 'user-friendly'. As Don commented in https://lkml.org/lkml/2015/4/22/325 in reply to my suggestion to add a plausibility check for 'watchdog_cpumask': "I am not sure that is necessary. This was supposed to be a debugging interface for nohz (and possibly other technologies). ... Personally, I feel anyone who will use this sys interface will need to do so at their own risk." So I think we could apply the same rationale here and ignore a possible error returned by smpboot_update_cpumask_percpu_thread(). Perhaps you could add a few comment lines to the code. Don, please let us know what you think. Regards, Uli -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/