Messages in this thread | ![/](/images/icornerl.gif) | | From | "Rafael J. Wysocki" <> | Date | Wed, 3 Jan 2024 19:21:10 +0100 | Subject | Re: [PATCH v2 4/4] thermal: intel: hfi: Add a suspend notifier |
| |
On Wed, Jan 3, 2024 at 6:26 PM Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com> wrote: > > On Wed, Jan 03, 2024 at 02:34:26PM +0100, Rafael J. Wysocki wrote: > > > +static int hfi_pm_notify(struct notifier_block *nb, > > > + unsigned long mode, void *unused) > > > +{ > > > + struct hfi_cpu_info *info = &per_cpu(hfi_cpu_info, 0); > > > + struct hfi_instance *hfi = info->hfi_instance; > > > + int ret = 0; > > > + > > > + /* HFI may not be in use. */ > > > + if (!hfi) > > > + return ret; > > > + > > > + mutex_lock(&hfi_instance_lock); > > > + /* > > > + * Only handle the HFI instance of the package of the boot CPU. The > > > + * instances of other packages are handled in the CPU hotplug callbacks. > > > + */ > > > + switch (mode) { > > > + case PM_HIBERNATION_PREPARE: > > > + case PM_SUSPEND_PREPARE: > > > + case PM_RESTORE_PREPARE: > > > + ret = smp_call_function_single(0, hfi_do_disable, NULL, true); > > > + break; > > > + > > > + case PM_POST_RESTORE: > > > + case PM_POST_HIBERNATION: > > > + case PM_POST_SUSPEND: > > > + ret = smp_call_function_single(0, hfi_do_enable, hfi, true); > > > + break; > > > > Because this handles the boot CPU only, one has to wonder if it should > > be a syscore op rather than a PM notifier. > > > > It does not sleep AFAICS, so it can run in that context, and it is > > guaranteed to run on the boot CPU then, so it is not necessary to > > force that. Moreover, syscore ops are guaranteed to be > > non-concurrent, so locking is not needed. > > There are below warnings in smp_call_function_single() : > > /* > * Can deadlock when called with interrupts disabled. > * We allow cpu's that are not yet online though, as no one else can > * send smp call function interrupt to this cpu and as such deadlocks > * can't happen. > */ > WARN_ON_ONCE(cpu_online(this_cpu) && irqs_disabled() > && !oops_in_progress); > > /* > * When @wait we can deadlock when we interrupt between llist_add() and > * arch_send_call_function_ipi*(); when !@wait we can deadlock due to > * csd_lock() on because the interrupt context uses the same csd > * storage. > */ > WARN_ON_ONCE(!in_task()); > > And this one in syscore_suspend(): > > WARN_ONCE(!irqs_disabled(), > "Interrupts enabled before system core suspend.\n"); > > So seems they are not compatible.
But smp_call_function_single() need not be used in syscore ops at all, because they always run on the boot CPU.
| ![\](/images/icornerr.gif) |