All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Boyd <sboyd@kernel.org>
To: Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
Subject: Re: [PATCH v2] clk: Evict unregistered clks from parent caches
Date: Tue, 17 Sep 2019 10:24:23 -0700	[thread overview]
Message-ID: <20190917172424.AAF5C214AF@mail.kernel.org> (raw)
In-Reply-To: <20190828181959.204401-1-sboyd@kernel.org>

Quoting Stephen Boyd (2019-08-28 11:19:59)
> We leave a dangling pointer in each clk_core::parents array that has an
> unregistered clk as a potential parent when that clk_core pointer is
> freed by clk{_hw}_unregister(). It is impossible for the true parent of
> a clk to be set with clk_set_parent() once the dangling pointer is left
> in the cache because we compare parent pointers in
> clk_fetch_parent_index() instead of checking for a matching clk name or
> clk_hw pointer.
> 
> Before commit ede77858473a ("clk: Remove global clk traversal on fetch
> parent index"), we would check clk_hw pointers, which has a higher
> chance of being the same between registration and unregistration, but it
> can still be allocated and freed by the clk provider. In fact, this has
> been a long standing problem since commit da0f0b2c3ad2 ("clk: Correct
> lookup logic in clk_fetch_parent_index()") where we stopped trying to
> compare clk names and skipped over entries in the cache that weren't
> NULL.
> 
> There are good (performance) reasons to not do the global tree lookup in
> cases where the cache holds dangling pointers to parents that have been
> unregistered. Let's take the performance hit on the uncommon
> registration path instead. Loop through all the clk_core::parents arrays
> when a clk is unregistered and set the entry to NULL when the parent
> cache entry and clk being unregistered are the same pointer. This will
> fix this problem and avoid the overhead for the "normal" case.
> 
> Based on a patch by Bjorn Andersson.
> 
> Fixes: da0f0b2c3ad2 ("clk: Correct lookup logic in clk_fetch_parent_index()")
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> Signed-off-by: Stephen Boyd <sboyd@kernel.org>
> ---

Applied to clk-next


      reply	other threads:[~2019-09-17 17:24 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-28 18:19 [PATCH v2] clk: Evict unregistered clks from parent caches Stephen Boyd
2019-09-17 17:24 ` Stephen Boyd [this message]

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=20190917172424.AAF5C214AF@mail.kernel.org \
    --to=sboyd@kernel.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=saiprakash.ranjan@codeaurora.org \
    /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.