Subject: Re: [PATCH v3 19/20] coresight: add try_get_module() in coresight_grab_device()

From: [email-masked]
Date: Thu, 23 Jul 2020 08:35:16 +0800

On 2020-07-22 18:48, Greg KH wrote:
> On Wed, Jul 22, 2020 at 11:49:48AM +0100, Suzuki K Poulose wrote:
>> Hi Tingwei,
>> 
>> On 07/17/2020 06:45 AM, Tingwei Zhang wrote:
>> > When coresight device is in an active session, driver module of
>> > that device should not be removed. Use try_get_module() in
>> > coresight_grab_device() to prevent module to be unloaded.
>> >
>> 
>> Is this really sufficient ? AFAIU, a device could be removed, but the
>> module may still be alive due to the refcount on the module. This
>> could imply that we have stale pointers in the _path_, which could
>> lead to corruption elsewhere. Should we do a get/put_device() instead 
>> ?
> 
> Remember there are two separate things here, code and data.  There are
> two different reference counts for them, do not confuse the two.
> 
> get/put is needed when you have a reference to the data, module stuff 
> is
> when you are calling into code.
> 
> But note that you do not always need to grab a reference count to the
> module, as long as the module can properly tear the data down when it 
> is
> asked to be removed.  Look at networking drivers as a great example of
> that.
> 
> thanks,
> 
> greg k-h
> 

Hi Greg,

Understand your point. I made the attempt to not hold the refcount of 
module
but stop/clean up active session in v1 of this series.  Link is as 
below.
https://lore.kernel.org/linux-arm-kernel/[email-masked]/

However, there's a lot of tricks to play to make it work especially on 
perf
trace when trace session is dynamically created/start/stop by perf 
framework.
It may have performance overhead as well.

With the suggestion from Mathieu, I'm wondering whether this complexity
introduced really worth. Remove a coresight module driver with active 
trace
session ongoing is a corner case. From v2, I turned to another 
alternative to
grad a reference count to module when the operation function in that 
module
could be called by coresight framework. That's a simpler and cleaner 
solution
in my opinion.

Thanks,
Tingwei

> _______________________________________________
> linux-arm-kernel mailing list
> [email-masked]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel