All of lore.kernel.org
 help / color / mirror / Atom feed
From: tingwei@codeaurora.org
To: Greg KH <gregkh@linuxfoundation.org>
Cc: tsoni@codeaurora.org, saiprakash.ranjan@codeaurora.org,
	kim.phillips@arm.com, mathieu.poirier@linaro.org,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	alexander.shishkin@linux.intel.com, coresight@lists.linaro.org,
	rdunlap@infradead.org, ykaukab@suse.de, linux@armlinux.org.uk,
	jinlmao@codeaurora.org, leo.yan@linaro.org,
	linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3 19/20] coresight: add try_get_module() in coresight_grab_device()
Date: Thu, 23 Jul 2020 08:35:16 +0800	[thread overview]
Message-ID: <49d35dc8429226e27d7638906cbc03ca@codeaurora.org> (raw)
In-Reply-To: <20200722104859.GA2827860@kroah.com>

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/20200701071427.10477-1-tingwei@codeaurora.org/

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
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2020-07-23  0:37 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-17  5:45 [PATCH v3 00/20] coresight: allow to build coresight as modules Tingwei Zhang
2020-07-17  5:45 ` [PATCH v3 01/20] coresight: cpu_debug: add module name in Kconfig Tingwei Zhang
2020-07-17  5:45 ` [PATCH v3 02/20] coresight: cpu_debug: define MODULE_DEVICE_TABLE Tingwei Zhang
2020-07-17  5:45 ` [PATCH v3 03/20] coresight: use IS_ENABLED for CONFIGs that may be modules Tingwei Zhang
2020-07-17  5:45 ` [PATCH v3 04/20] coresight: add coresight prefix to barrier_pkt Tingwei Zhang
2020-07-17  5:45 ` [PATCH v3 05/20] coresight: export global symbols Tingwei Zhang
2020-07-17  5:45 ` [PATCH v3 06/20] Allow to build coresight-stm as a module, for ease of development Tingwei Zhang
2020-07-17  5:45 ` [PATCH v3 07/20] coresight: allow etm3x to be built as a module Tingwei Zhang
2020-07-17  5:45 ` [PATCH v3 08/20] coresight: allow etm4x " Tingwei Zhang
2020-07-17 17:05   ` Mike Leach
2020-07-18  3:25     ` tingwei
2020-07-18 17:38       ` Mike Leach
2020-07-21 14:55         ` Mathieu Poirier
2020-07-21 23:22           ` tingwei
2020-07-20  6:58   ` Sai Prakash Ranjan
2020-07-21  7:03     ` Sai Prakash Ranjan
2020-07-17  5:45 ` [PATCH v3 09/20] coresight: allow etb " Tingwei Zhang
2020-07-17  5:45 ` [PATCH v3 10/20] coresight: allow tpiu " Tingwei Zhang
2020-07-17  5:45 ` [PATCH v3 11/20] coresight: allow tmc " Tingwei Zhang
2020-07-17  5:45 ` [PATCH v3 12/20] coresight: remove multiple init calls from funnel driver Tingwei Zhang
2020-07-17  5:45 ` [PATCH v3 13/20] coresight: remove multiple init calls from replicator driver Tingwei Zhang
2020-07-17  5:45 ` [PATCH v3 14/20] coresight: allow funnel and replicator drivers to be built as modules Tingwei Zhang
2020-07-17  5:45 ` [PATCH v3 15/20] coresight: cti: add function to register cti associate ops Tingwei Zhang
2020-07-17  5:45 ` [PATCH v3 16/20] coresight: allow cti to be built as a module Tingwei Zhang
2020-07-20 17:00   ` Mike Leach
2020-07-20 21:07     ` Mike Leach
2020-07-21 16:35       ` Mike Leach
2020-07-21 23:27         ` tingwei
2020-07-17  5:45 ` [PATCH v3 17/20] coresight: tmc-etr: add function to register catu ops Tingwei Zhang
2020-07-17  5:45 ` [PATCH v3 18/20] coresight: allow catu drivers to be built as modules Tingwei Zhang
2020-07-17  5:45 ` [PATCH v3 19/20] coresight: add try_get_module() in coresight_grab_device() Tingwei Zhang
2020-07-22 10:49   ` Suzuki K Poulose
2020-07-22 10:48     ` Greg KH
2020-07-22 11:26       ` Suzuki K Poulose
2020-07-23  0:18         ` tingwei
2020-07-23  0:35       ` tingwei [this message]
2020-07-22 10:51     ` Suzuki K Poulose
2020-07-23  0:19       ` tingwei
2020-07-23 19:36     ` Mathieu Poirier
2020-07-24  1:17       ` Tingwei Zhang
2020-07-17  5:45 ` [PATCH v3 20/20] coresight: allow the coresight core driver to be built as a module Tingwei Zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=49d35dc8429226e27d7638906cbc03ca@codeaurora.org \
    --to=tingwei@codeaurora.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=coresight@lists.linaro.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jinlmao@codeaurora.org \
    --cc=kim.phillips@arm.com \
    --cc=leo.yan@linaro.org \
    --cc=linux-arm-kernel-bounces@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux@armlinux.org.uk \
    --cc=mathieu.poirier@linaro.org \
    --cc=rdunlap@infradead.org \
    --cc=saiprakash.ranjan@codeaurora.org \
    --cc=suzuki.poulose@arm.com \
    --cc=tsoni@codeaurora.org \
    --cc=ykaukab@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.