From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1166181AbdEZAjm (ORCPT ); Thu, 25 May 2017 20:39:42 -0400 Received: from mail-pf0-f196.google.com ([209.85.192.196]:33982 "EHLO mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1163357AbdEZAje (ORCPT ); Thu, 25 May 2017 20:39:34 -0400 Date: Fri, 26 May 2017 10:39:09 +1000 From: Nicholas Piggin To: Don Zickus Cc: linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org Subject: Re: [PATCH 4/4] watchdog: provide watchdog_reconfigure() for arch watchdogs Message-ID: <20170526103909.04f7710c@roar.ozlabs.ibm.com> In-Reply-To: <20170525140833.3qkuvoiavlf5ra6d@redhat.com> References: <20170525082856.21685-1-npiggin@gmail.com> <20170525082856.21685-5-npiggin@gmail.com> <20170525140833.3qkuvoiavlf5ra6d@redhat.com> Organization: IBM X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 25 May 2017 10:08:33 -0400 Don Zickus wrote: > On Thu, May 25, 2017 at 06:28:56PM +1000, Nicholas Piggin wrote: > > After reconfiguring watchdog sysctls etc., architecture specific > > watchdogs may not get all their parameters updated. > > > > watchdog_reconfigure() can be implemented to pull the new values > > in and set the arch NMI watchdog. > > I understand the reason for this patch and I don't have any real objection > on how it was implemented within the constraints of all the current logic. > > I just wonder if the current logic should be adjusted to make the hardlockup > detector, namely the perf implementation more separate so it can handle what > you would like more cleanly. > > The watchdog_nmi_reconfigure is sort of hackish, but it is hard to fault you > based on how things are designed. I am going to poke at it a little bit, > but I will probably not find time to do much and accept what you have for > now. I actually agree with you. These patches are basically an initial bridge to get us to decoupling hld-perf from hld-arch, but the code could definitely use several more passes to clean things up. One thing we want to be mindful of is some watchdogs are very light weight, minimal, and some may not even want to call C code (at least from the NMI and touch-watchdog paths). But having said that, it may not be a bad idea to have implementations provide a watchdog driver struct with some of the methods and reconfiguration they support. E.g., suspend/resume, stop/start on CPUs, adjust timeouts, etc.). I didn't want to go the whole hog and over-engineer something that doesn't work though, so I'm hoping we can get the powerpc watchdog in, and then keep working on the apis. Let me know what you think after you poke at it though. Thanks, Nick