lkml.org 
[lkml]   [2022]   [Jan]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3 00/23] counter: cleanups and device lifetime fixes
From
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

\
 
 \ /
  Last update: 2022-01-11 10:56    [W:0.070 / U:0.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site