From: Maulik Shah <email@example.com> To: Doug Anderson <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: Tue, 31 Mar 2020 14:27:41 +0530 [thread overview] Message-ID: <email@example.com> (raw) In-Reply-To: <CAD=FV=UZTyfVN=a35hiXxdNSvdhfK60HewP_p_VvH1KdoUa1ww@mail.gmail.com> Hi, On 3/28/2020 12:12 AM, Doug Anderson wrote: > 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? Ok, i will remove marking cache dirty here. > > >>> ...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. Agree but i would insist not remove locks in your cleanup/documentation series which are already there. These will be again need to be added. The locks don't cause any issue being there since only last cpu is invoking rpmh_flush() at present. Adding support for other RSCs is in my to do list, and when that is being done we can re-evaluate and remove if not required. > > 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. Sure. we can re-look all cases. > > > -Doug Thanks, Maulik -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
prev parent reply other threads:[~2020-03-31 8:57 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 2020-03-31 8:57 ` Maulik Shah [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 \ --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 \ --email@example.com \ --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.