Messages in this thread | | | Date | Tue, 11 Jan 2022 11:56:26 +0200 | Subject | Re: [PATCH v3 00/23] counter: cleanups and device lifetime fixes | From | Jarkko Nikula <> |
| |
Hi
On 1/6/22 17:13, Uwe Kleine-König wrote: > On Wed, Jan 05, 2022 at 09:26:58PM +0900, William Breathitt Gray wrote: >> On Wed, Dec 29, 2021 at 04:44:18PM +0100, Uwe Kleine-König wrote: >>> - I think intel-qep.c makes the counter unfunctional in >>> intel_qep_remove before the counter is unregistered. >> >> Hello Uwe, >> >> Would you elaborate some more on this? I think intel_qep_remove() is >> only called after the counter is unregistered because the struct >> counter_device parent is set to &pci->dev in intel_qep_probe(). Am I >> misunderstanding the removal path? > > If the counter device is unbound (e.g. via sysfs), the following calls > are made: > > intel_qep_remove() (stopping the hardware?) > devm_counter_release (devm callback of devm_counter_register or ..._add) > then the release callbacks of the earlier devm functions > > My concern is, that in the timeslot between intel_qep_remove() and > devm_counter_release() the device looks like a functional device and > might be queried/reconfigured/... while the hardware is already dead. > > It's probably not a big issue (unless for example reading the counter > this race window makes the hardware hang?), but it's at least ugly. > Maybe the worst effect is that a counter value is missed (which is OK at > unregister time). Still it would be nicer to first take down the counter > device and only then stop the hardware. > In HW point of view it should be safe. We do disable the HW in intel_qep_remove() but that doesn't render the HW unusable and registers are accessible.
Perhaps that line can go since I think it was put there just to stop the HW just in case after remove.
Jarkko
| |