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

[Tingwei Zhang]

On Fri, Jul 24, 2020 at 03:36:01AM +0800, Mathieu Poirier wrote:
> Hi Suzuki,
> 
> 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
> 
> If I understand correctly you are worried about cases where drivers would
> be
> removed but not the reference of the devices using those drivers in the
> coresight port connections?  If so you are very right.  Otherwise please
> expand
> on the scenario you have in mind.
>
Hi Mathieu and Suzuki,

I didn't add get/put_device() here since coresight doesn't support
dymanically add/remove device until this series. This series export one
possiblity to add/remove device via load/unload module. Since coresight
framework holds reference count for module when there's one active
session, device won't be removed when it's used by one active session.
Module unload routine checks module reference count before doing anything.

With a second thought, I think it may be better to add get/put_device()
with try_get_module()/put_module here to indicate we need protect both
code and data when there's on active session.
 
> The first version of this set was doing all that cleanup...  I haven't
> looked at
> this set yet but from what I've seen the cleanup code is not present in
> any
> of the sets after V1.  Tingwei, is the work done in
> coresight_disable_match()
> part of the later revisions?

I didn't include cleanup patch in later revision. Module reference count
is hold by coresight framework when there's one active session, so the
cleanup code won't get chance to be executed.

Thanks, Tingwei
> 
> > could imply that we have stale pointers in the _path_, which could
> > lead to corruption elsewhere. Should we do a get/put_device() instead ?
> > 
> > Cheers
> > Suzuki
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel