Re: [RFC][PATCHv5 06/13] printk: register PM notifier

[Sergey Senozhatsky]

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