| Message ID | 20200319231021.18108-1-mdtipton@codeaurora.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9c0c54a8daf465e6e6454ffe6fc999e81a22dbf9 |
| Headers | show |
| Series | interconnect: qcom: Fix uninitialized tcs_cmd::wait | expand |
On Thu 19 Mar 16:10 PDT 2020, Mike Tipton wrote: > Currently, if tcs_cmd_gen is called with commit=false, then > tcs_cmd::wait is left uninitialized. Since the tcs_cmd structures passed > to this function aren't zero-initialized, then we're left with random > wait values. This results in waiting for completion for more commands > than is necessary, depending on what's on the stack at the time. > > Removing the unnecessary if-condition fixes this, but add an explicit > memset of the tcs_cmd structure as well to ensure predictable behavior > if more tcs_cmd members are added in the future. > > Fixes: 976daac4a1c5 ("interconnect: qcom: Consolidate interconnect RPMh support") > Signed-off-by: Mike Tipton <mdtipton@codeaurora.org> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> Regards, Bjorn > --- > drivers/interconnect/qcom/bcm-voter.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/interconnect/qcom/bcm-voter.c b/drivers/interconnect/qcom/bcm-voter.c > index 2adfde8cdf19..2a11a63e7217 100644 > --- a/drivers/interconnect/qcom/bcm-voter.c > +++ b/drivers/interconnect/qcom/bcm-voter.c > @@ -96,6 +96,8 @@ static inline void tcs_cmd_gen(struct tcs_cmd *cmd, u64 vote_x, u64 vote_y, > if (!cmd) > return; > > + memset(cmd, 0, sizeof(*cmd)); > + > if (vote_x == 0 && vote_y == 0) > valid = false; > > @@ -112,8 +114,7 @@ static inline void tcs_cmd_gen(struct tcs_cmd *cmd, u64 vote_x, u64 vote_y, > * Set the wait for completion flag on command that need to be completed > * before the next command. > */ > - if (commit) > - cmd->wait = true; > + cmd->wait = commit; > } > > static void tcs_list_gen(struct list_head *bcm_list, int bucket, > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project
diff --git a/drivers/interconnect/qcom/bcm-voter.c b/drivers/interconnect/qcom/bcm-voter.c index 2adfde8cdf19..2a11a63e7217 100644 --- a/drivers/interconnect/qcom/bcm-voter.c +++ b/drivers/interconnect/qcom/bcm-voter.c @@ -96,6 +96,8 @@ static inline void tcs_cmd_gen(struct tcs_cmd *cmd, u64 vote_x, u64 vote_y, if (!cmd) return; + memset(cmd, 0, sizeof(*cmd)); + if (vote_x == 0 && vote_y == 0) valid = false; @@ -112,8 +114,7 @@ static inline void tcs_cmd_gen(struct tcs_cmd *cmd, u64 vote_x, u64 vote_y, * Set the wait for completion flag on command that need to be completed * before the next command. */ - if (commit) - cmd->wait = true; + cmd->wait = commit; } static void tcs_list_gen(struct list_head *bcm_list, int bucket,
Currently, if tcs_cmd_gen is called with commit=false, then tcs_cmd::wait is left uninitialized. Since the tcs_cmd structures passed to this function aren't zero-initialized, then we're left with random wait values. This results in waiting for completion for more commands than is necessary, depending on what's on the stack at the time. Removing the unnecessary if-condition fixes this, but add an explicit memset of the tcs_cmd structure as well to ensure predictable behavior if more tcs_cmd members are added in the future. Fixes: 976daac4a1c5 ("interconnect: qcom: Consolidate interconnect RPMh support") Signed-off-by: Mike Tipton <mdtipton@codeaurora.org> --- drivers/interconnect/qcom/bcm-voter.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)