All of lore.kernel.org
 help / color / mirror / Atom feed
From: Doug Anderson <dianders@chromium.org>
To: Maulik Shah <mkshah@codeaurora.org>
Cc: Stephen Boyd <swboyd@chromium.org>,
	Evan Green <evgreen@chromium.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	LKML <linux-kernel@vger.kernel.org>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	Andy Gross <agross@kernel.org>,
	Matthias Kaehlcke <mka@chromium.org>,
	Rajendra Nayak <rnayak@codeaurora.org>,
	Lina Iyer <ilina@codeaurora.org>,
	lsrao@codeaurora.org
Subject: Re: [PATCH v14 6/6] soc: qcom: rpmh-rsc: Allow using free WAKE TCS for active request
Date: Fri, 27 Mar 2020 11:42:03 -0700	[thread overview]
Message-ID: <CAD=FV=UZTyfVN=a35hiXxdNSvdhfK60HewP_p_VvH1KdoUa1ww@mail.gmail.com> (raw)
In-Reply-To: <8d19958d-7334-ca4e-d7ba-f5919a56b279@codeaurora.org>

Hi,

On Fri, Mar 27, 2020 at 5:04 AM Maulik Shah <mkshah@codeaurora.org> wrote:
>
> > Why can't rpmh_write()
> > / rpmh_write_async() / rpmh_write_batch() just always unconditionally
> > mark the cache dirty?  Are there really lots of cases when those calls
> > are made and they do nothing?
>
> At rpmh.c, it doesn't know that rpmh-rsc.c worked on borrowed TCS to finish the request.
>
> We should not blindly mark caches dirty everytime.

In message ID "5a5274ac-41f4-b06d-ff49-c00cef67aa7f@codeaurora.org"
which seems to be missing from the archives, you said:

> yes we should trust callers not to send duplicate data

...you can see some reference to it in my reply:

https://lore.kernel.org/r/CAD=FV=VPSahhK71k_D+nfL1=5QE5sKMQT=6zzyEF7+JWMcTxsg@mail.gmail.com/

If callers are trusted to never send duplicate data then ever call to
rpmh_write() will make a change.  ...and thus the cache should always
be marked dirty, no?  Also note that since rpmh_write() to "active"
also counts as a write to "wake" even those will dirty the cache.

Which case are you expecting a rpmh_write() call to not dirty the cache?


> > ...interestingly after your patch I guess now I guess tcs_invalidate()
> > no longer needs spinlocks since it's only ever called from PM code on
> > the last CPU.  ...if you agree, I can always do it in my cleanup
> > series.  See:
> >
> > https://lore.kernel.org/r/CAD=FV=Xp1o68HnC2-hMnffDDsi+jjgc9pNrdNuypjQZbS5K4nQ@mail.gmail.com
> >
> > -Doug
>
> There are other RSCs which use same driver, so lets keep spinlock.

It is really hard to try to write keeping in mind these "other RSCs"
for which there is no code upstream.  IMO we should write the code
keeping in mind what is supported upstream and then when those "other
RSCs" get added we can evaluate their needs.

Specifically when reasoning about rpmh.c and rpmh-rsc.c I can only
look at the code that's there now and decide whether it is race free
or there are races.  Back when I was analyzing the proposal to do
rpmh_flush() all the time (not from PM code) it felt like there were a
bunch of races, especially in the zero-active-TCS case.  Most of the
races go away when you assume that rpmh_flush() is only ever called
from the PM code when nobody could be in the middle of an active
transfer.

If we are ever planning to call rpmh_flush() from another place we
need to re-look at all those races.


-Doug

  reply	other threads:[~2020-03-27 18:42 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-26 17:37 [PATCH v14 0/6] Invoke rpmh_flush for non OSI targets Maulik Shah
2020-03-26 17:37 ` [PATCH v14 1/6] arm64: dts: qcom: sc7180: Add cpuidle low power states Maulik Shah
2020-03-26 17:37 ` [PATCH v14 2/6] soc: qcom: rpmh: Update dirty flag only when data changes Maulik Shah
2020-03-26 17:37 ` [PATCH v14 3/6] soc: qcom: rpmh: Invalidate SLEEP and WAKE TCSes before flushing new data Maulik Shah
2020-03-26 18:31   ` Doug Anderson
2020-03-27  8:23     ` Maulik Shah
2020-03-26 17:37 ` [PATCH v14 4/6] soc: qcom: rpmh: Invoke rpmh_flush() for dirty caches Maulik Shah
2020-03-26 21:18   ` Doug Anderson
     [not found]     ` <7bd2c923-4003-a1c4-610f-548e79a94d35@codeaurora.org>
2020-03-27 18:22       ` Doug Anderson
2020-03-31  8:26         ` Maulik Shah
2020-03-26 17:37 ` [PATCH v14 5/6] soc: qcom: rpmh-rsc: Clear active mode configuration for wake TCS Maulik Shah
2020-03-26 18:20   ` Doug Anderson
2020-03-26 17:37 ` [PATCH v14 6/6] soc: qcom: rpmh-rsc: Allow using free WAKE TCS for active request Maulik Shah
2020-03-26 21:46   ` Doug Anderson
2020-03-27 12:04     ` Maulik Shah
2020-03-27 18:42       ` Doug Anderson [this message]
2020-03-31  8:57         ` Maulik Shah

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='CAD=FV=UZTyfVN=a35hiXxdNSvdhfK60HewP_p_VvH1KdoUa1ww@mail.gmail.com' \
    --to=dianders@chromium.org \
    --cc=agross@kernel.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=evgreen@chromium.org \
    --cc=ilina@codeaurora.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lsrao@codeaurora.org \
    --cc=mka@chromium.org \
    --cc=mkshah@codeaurora.org \
    --cc=rnayak@codeaurora.org \
    --cc=swboyd@chromium.org \
    --subject='Re: [PATCH v14 6/6] soc: qcom: rpmh-rsc: Allow using free WAKE TCS for active request' \
    /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

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.