From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753590AbdHQXTf (ORCPT ); Thu, 17 Aug 2017 19:19:35 -0400 Received: from mail-pg0-f48.google.com ([74.125.83.48]:32951 "EHLO mail-pg0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753271AbdHQXTe (ORCPT ); Thu, 17 Aug 2017 19:19:34 -0400 Date: Fri, 18 Aug 2017 08:19:50 +0900 From: Sergey Senozhatsky To: "Rafael J. Wysocki" Cc: Sergey Senozhatsky , Sergey Senozhatsky , Petr Mladek , Steven Rostedt , Jan Kara , Andrew Morton , Peter Zijlstra , Eric Biederman , Greg Kroah-Hartman , Jiri Slaby , Pavel Machek , Andreas Mohr , Tetsuo Handa , linux-kernel@vger.kernel.org Subject: Re: [RFC][PATCHv5 06/13] printk: register PM notifier Message-ID: <20170817231950.GA575@jagdpanzerIV.localdomain> References: <20170815025625.1977-1-sergey.senozhatsky@gmail.com> <1933345.ycXYzBI3bM@aspire.rjw.lan> <20170817055558.GC20907@jagdpanzerIV.localdomain> <2847932.h4qZLRXnUZ@aspire.rjw.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2847932.h4qZLRXnUZ@aspire.rjw.lan> User-Agent: Mutt/1.8.3 (2017-05-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, On (08/17/17 17:43), Rafael J. Wysocki wrote: [..] > > > In that case I would call printk_emergency_begin_sync() from > > > dpm_prepare() and printk_emergency_end_sync() from dpm_complete(). > > > > hm, isn't it the case that dpm_prepare/dpm_complete are invoked only > > by hibernate path? or does suspend path (s2ram, etc.) also calls > > dpm_prepare/dpm_complete? > > Yes, it does. oh, good. > > the 3 things we need to have (in PM context) for offloading: > > - unparked printk kthread > > - running scheduler > > - online non-boot CPUs (on a UP system, or with non-boot CPUs disabled, > > offloading is a bit questionable) > > > > - hm, may be something else... > > All of that is there during the entire device suspend/resume including > dpm_suspend/resume_noirq(). > > But probably dpm_prepare/complete() are better places for the hooks at > least for now. ok, thanks. > > > I just don't see much point in using the notifier thing if you can > > > achieve basically the same without using it. :-) > > > > sure, I just didn't want to mix printk internals with PM internals. > > that would put us in position of verifying future PM changes from > > printk-kthread point of view as well; and it can be quite complex, > > because printk offloading brings in big guns like scheduler and > > timekeeping. so the notifiers interface looks like a good > > alternative, besides those notifications happen early (and late) > > enough to keep us on the safe side. > > > > well, I may be wrong. > > I prefer direct invocations, becasue they generally allow to figure out > what's going on by simply following the code instead of having to > track all of the users of the notifiers to see what they may have > registered. I see the point: notifiers, in some sense, help us to alter sub-systems without ever touching them or even explaining anything to the maintainers. we just register a notifier and all of a sudden sub-system FOO begins to execute our code at some point. all done entirely with in the printk.c file. there is some power/flexibility and, perhaps, beauty in it, but there is also some potential for abuse/wrongdoing in it. if direct calls work better for you (and PM), then no objections from my side :) > Moreover, the ordering of what happens is clear then, whereas with notifiers it > depends on the registration ordering and the entry and exit path orderings of > notifiers are the same which may be problematic sometimes. > > In fact, the PM notifiers are mostly for stuff that cannot be done with frozen > user space and surely not for core subsystems. > > Let alone that adding two lines of code is better than adding 50 lines for the > same purpose IMO ... ok. I can rework the PM patch and get rid of notifiers for the next submission (unless there will be objections from others). will take a look. thanks! -ss