From: Doug Anderson <email@example.com> To: Maulik Shah <firstname.lastname@example.org> Cc: Stephen Boyd <email@example.com>, Evan Green <firstname.lastname@example.org>, Bjorn Andersson <email@example.com>, LKML <firstname.lastname@example.org>, linux-arm-msm <email@example.com>, Andy Gross <firstname.lastname@example.org>, Matthias Kaehlcke <email@example.com>, Rajendra Nayak <firstname.lastname@example.org>, Lina Iyer <email@example.com>, firstname.lastname@example.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: <email@example.com> Hi, On Fri, Mar 27, 2020 at 5:04 AM Maulik Shah <firstname.lastname@example.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 "email@example.com" 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
next prev parent 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] ` <firstname.lastname@example.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' \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.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.