LKML Archive on lore.kernel.org help / color / mirror / Atom feed
From: Douglas Anderson <dianders@chromium.org> To: Andy Gross <agross@kernel.org>, Bjorn Andersson <bjorn.andersson@linaro.org>, Maulik Shah <mkshah@codeaurora.org> Cc: Rajendra Nayak <rnayak@codeaurora.org>, mka@chromium.org, evgreen@chromium.org, swboyd@chromium.org, Lina Iyer <ilina@codeaurora.org>, Douglas Anderson <dianders@chromium.org>, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [RFT PATCH 8/9] drivers: qcom: rpmh-rsc: spin_lock_irqsave() for tcs_invalidate() Date: Fri, 6 Mar 2020 15:59:50 -0800 [thread overview] Message-ID: <20200306155707.RFT.8.I07c1f70e0e8f2dc0004bd38970b4e258acdc773e@changeid> (raw) In-Reply-To: <20200306235951.214678-1-dianders@chromium.org> Auditing tcs_invalidate() made me worried. Specifically I saw that it used spin_lock(), not spin_lock_irqsave(). That always worries me. As I understand it, using spin_lock() is only valid in these situations: a) You know you are running in the interrupt handler (and all other users of the lock use the "irqsave" variant). b) You know that nobody using the lock is ever running in the interrupt handler. c) You know that someone else has always disabled interrupts before your code runs and thus the "irqsave" variant is pointless. From auditing the driver we look OK. ...except that there is one further corner case. If sometimes your code is called with IRQs disabled and sometimes it's not you will get in trouble if someone ever boots your board with "nosmp" (AKA in uniprocessor mode). In such a case if someone else has the lock (without disabling interrupts) and they get swapped out then your code (with interrupts disabled) might loop forever waiting for the spinlock. It's just safer to use the irqsave version, so let's do that. In future patches I believe tcs_invalidate() will always be called with interrupts off anyway. Signed-off-by: Douglas Anderson <dianders@chromium.org> --- drivers/soc/qcom/rpmh-rsc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c index 693873fce895..0297da5ceeaa 100644 --- a/drivers/soc/qcom/rpmh-rsc.c +++ b/drivers/soc/qcom/rpmh-rsc.c @@ -210,9 +210,10 @@ static bool tcs_is_free(struct rsc_drv *drv, int tcs_id) static int tcs_invalidate(struct rsc_drv *drv, int type) { int m; + unsigned long flags; struct tcs_group *tcs = &drv->tcs[type]; - spin_lock(&tcs->lock); + spin_lock_irqsave(&tcs->lock, flags); if (bitmap_empty(tcs->slots, MAX_TCS_SLOTS)) { spin_unlock(&tcs->lock); return 0; @@ -227,7 +228,7 @@ static int tcs_invalidate(struct rsc_drv *drv, int type) write_tcs_reg_sync(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, m, 0); } bitmap_zero(tcs->slots, MAX_TCS_SLOTS); - spin_unlock(&tcs->lock); + spin_unlock_irqrestore(&tcs->lock, flags); return 0; } -- 2.25.1.481.gfbce0eb801-goog
next prev parent reply other threads:[~2020-03-07 0:00 UTC|newest] Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-03-06 23:59 [RFT PATCH 0/9] drivers: qcom: rpmh-rsc: Cleanup / add lots of comments Douglas Anderson 2020-03-06 23:59 ` [RFT PATCH 1/9] drivers: qcom: rpmh-rsc: Clean code reading/writing regs/cmds Douglas Anderson 2020-03-11 8:47 ` Maulik Shah 2020-03-11 15:03 ` Doug Anderson 2020-03-11 16:17 ` Matthias Kaehlcke 2020-03-11 19:30 ` Stephen Boyd 2020-03-06 23:59 ` [RFT PATCH 2/9] drivers: qcom: rpmh-rsc: Document the register layout better Douglas Anderson 2020-03-11 9:35 ` Maulik Shah 2020-03-11 15:27 ` Doug Anderson 2020-03-11 18:49 ` Evan Green 2020-03-11 20:08 ` Stephen Boyd 2020-03-11 22:35 ` Doug Anderson 2020-03-06 23:59 ` [RFT PATCH 3/9] drivers: qcom: rpmh-rsc: Fold tcs_ctrl_write() into its single caller Douglas Anderson 2020-03-11 9:50 ` Maulik Shah 2020-03-06 23:59 ` [RFT PATCH 4/9] drivers: qcom: rpmh-rsc: Remove get_tcs_of_type() abstraction Douglas Anderson 2020-03-11 12:02 ` Maulik Shah 2020-03-06 23:59 ` [RFT PATCH 5/9] drivers: qcom: rpmh-rsc: A lot of comments Douglas Anderson 2020-03-06 23:59 ` [RFT PATCH 6/9] drivers: qcom: rpmh-rsc: Only use "tcs_in_use" for ACTIVE_ONLY Douglas Anderson 2020-03-11 0:33 ` Doug Anderson 2020-03-06 23:59 ` [RFT PATCH 7/9] drivers: qcom: rpmh-rsc: Warning if tcs_write() used for non-active Douglas Anderson 2020-03-06 23:59 ` Douglas Anderson [this message] 2020-03-06 23:59 ` [RFT PATCH 9/9] drivers: qcom: rpmh-rsc: Kill cmd_cache and find_match() with fire Douglas Anderson 2020-03-11 0:35 ` Doug Anderson 2020-03-11 9:48 ` [RFT PATCH 0/9] drivers: qcom: rpmh-rsc: Cleanup / add lots of comments 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=20200306155707.RFT.8.I07c1f70e0e8f2dc0004bd38970b4e258acdc773e@changeid \ --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=mka@chromium.org \ --cc=mkshah@codeaurora.org \ --cc=rnayak@codeaurora.org \ --cc=swboyd@chromium.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).