| Message ID | 20200623040814.23791-5-mdtipton@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | interconnect: qcom: Misc bcm-voter changes and fixes | expand |
Hi Mike, On 6/23/20 07:08, Mike Tipton wrote: > Small BW votes that translate to less than a single BCM unit are > currently truncated to zero. Ensure that non-zero BW requests always > result in at least a vote of 1 to BCM. > > Fixes: 976daac4a1c5 ("interconnect: qcom: Consolidate interconnect RPMh support") > Signed-off-by: Mike Tipton <mdtipton@codeaurora.org> > --- > drivers/interconnect/qcom/bcm-voter.c | 27 +++++++++++++++++++-------- > 1 file changed, 19 insertions(+), 8 deletions(-) > > diff --git a/drivers/interconnect/qcom/bcm-voter.c b/drivers/interconnect/qcom/bcm-voter.c > index a68c858ca6b7..9e2612fe7fad 100644 > --- a/drivers/interconnect/qcom/bcm-voter.c > +++ b/drivers/interconnect/qcom/bcm-voter.c > @@ -54,8 +54,20 @@ static int cmp_vcd(void *priv, struct list_head *a, struct list_head *b) > return 1; > } > > +static u64 bcm_div(u64 num, u64 base) > +{ > + /* Ensure that small votes aren't lost. */ > + if (num && num < base) > + return 1; > + > + do_div(num, base); do_div() does a 64-by-32 division, which will truncate these to 32-bit. > + > + return num; > +} > + > static void bcm_aggregate(struct qcom_icc_bcm *bcm) > { > + struct qcom_icc_node *node; > size_t i, bucket; > u64 agg_avg[QCOM_ICC_NUM_BUCKETS] = {0}; > u64 agg_peak[QCOM_ICC_NUM_BUCKETS] = {0}; > @@ -63,22 +75,21 @@ static void bcm_aggregate(struct qcom_icc_bcm *bcm) > > for (bucket = 0; bucket < QCOM_ICC_NUM_BUCKETS; bucket++) { > for (i = 0; i < bcm->num_nodes; i++) { > - temp = bcm->nodes[i]->sum_avg[bucket] * bcm->aux_data.width; > - do_div(temp, bcm->nodes[i]->buswidth * bcm->nodes[i]->channels); > + node = bcm->nodes[i]; > + temp = bcm_div(node->sum_avg[bucket] * bcm->aux_data.width, > + node->buswidth * node->channels); > agg_avg[bucket] = max(agg_avg[bucket], temp); > > - temp = bcm->nodes[i]->max_peak[bucket] * bcm->aux_data.width; > - do_div(temp, bcm->nodes[i]->buswidth); > + temp = bcm_div(node->max_peak[bucket] * bcm->aux_data.width, > + node->buswidth); > agg_peak[bucket] = max(agg_peak[bucket], temp); > } > > temp = agg_avg[bucket] * bcm->vote_scale; > - do_div(temp, bcm->aux_data.unit); > - bcm->vote_x[bucket] = temp; > + bcm->vote_x[bucket] = bcm_div(temp, bcm->aux_data.unit); > > temp = agg_peak[bucket] * bcm->vote_scale; > - do_div(temp, bcm->aux_data.unit); > - bcm->vote_y[bucket] = temp; > + bcm->vote_y[bucket] = bcm_div(temp, bcm->aux_data.unit); > } > > if (bcm->keepalive && bcm->vote_x[QCOM_ICC_BUCKET_AMC] == 0 && > The rest looks good. Thanks, Georgi
On 7/2/2020 4:11 AM, Georgi Djakov wrote: > Hi Mike, > > On 6/23/20 07:08, Mike Tipton wrote: >> Small BW votes that translate to less than a single BCM unit are >> currently truncated to zero. Ensure that non-zero BW requests always >> result in at least a vote of 1 to BCM. >> >> Fixes: 976daac4a1c5 ("interconnect: qcom: Consolidate interconnect RPMh support") >> Signed-off-by: Mike Tipton <mdtipton@codeaurora.org> >> --- >> drivers/interconnect/qcom/bcm-voter.c | 27 +++++++++++++++++++-------- >> 1 file changed, 19 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/interconnect/qcom/bcm-voter.c b/drivers/interconnect/qcom/bcm-voter.c >> index a68c858ca6b7..9e2612fe7fad 100644 >> --- a/drivers/interconnect/qcom/bcm-voter.c >> +++ b/drivers/interconnect/qcom/bcm-voter.c >> @@ -54,8 +54,20 @@ static int cmp_vcd(void *priv, struct list_head *a, struct list_head *b) >> return 1; >> } >> >> +static u64 bcm_div(u64 num, u64 base) >> +{ >> + /* Ensure that small votes aren't lost. */ >> + if (num && num < base) >> + return 1; >> + >> + do_div(num, base); > > do_div() does a 64-by-32 division, which will truncate these to 32-bit. I can change base to a u32. It doesn't need anything more than that. > >> + >> + return num; >> +} >> + >> static void bcm_aggregate(struct qcom_icc_bcm *bcm) >> { >> + struct qcom_icc_node *node; >> size_t i, bucket; >> u64 agg_avg[QCOM_ICC_NUM_BUCKETS] = {0}; >> u64 agg_peak[QCOM_ICC_NUM_BUCKETS] = {0}; >> @@ -63,22 +75,21 @@ static void bcm_aggregate(struct qcom_icc_bcm *bcm) >> >> for (bucket = 0; bucket < QCOM_ICC_NUM_BUCKETS; bucket++) { >> for (i = 0; i < bcm->num_nodes; i++) { >> - temp = bcm->nodes[i]->sum_avg[bucket] * bcm->aux_data.width; >> - do_div(temp, bcm->nodes[i]->buswidth * bcm->nodes[i]->channels); >> + node = bcm->nodes[i]; >> + temp = bcm_div(node->sum_avg[bucket] * bcm->aux_data.width, >> + node->buswidth * node->channels); >> agg_avg[bucket] = max(agg_avg[bucket], temp); >> >> - temp = bcm->nodes[i]->max_peak[bucket] * bcm->aux_data.width; >> - do_div(temp, bcm->nodes[i]->buswidth); >> + temp = bcm_div(node->max_peak[bucket] * bcm->aux_data.width, >> + node->buswidth); >> agg_peak[bucket] = max(agg_peak[bucket], temp); >> } >> >> temp = agg_avg[bucket] * bcm->vote_scale; >> - do_div(temp, bcm->aux_data.unit); >> - bcm->vote_x[bucket] = temp; >> + bcm->vote_x[bucket] = bcm_div(temp, bcm->aux_data.unit); >> >> temp = agg_peak[bucket] * bcm->vote_scale; >> - do_div(temp, bcm->aux_data.unit); >> - bcm->vote_y[bucket] = temp; >> + bcm->vote_y[bucket] = bcm_div(temp, bcm->aux_data.unit); >> } >> >> if (bcm->keepalive && bcm->vote_x[QCOM_ICC_BUCKET_AMC] == 0 && >> > > The rest looks good. > > Thanks, > Georgi >
Hi Mike,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.8-rc4 next-20200706]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Mike-Tipton/interconnect-qcom-Misc-bcm-voter-changes-and-fixes/20200623-121301
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git dd0d718152e4c65b173070d48ea9dfc06894c3e5
config: arm64-randconfig-s032-20200706 (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.2-14-g8fce3d7a-dirty
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
drivers/interconnect/qcom/bcm-voter.c:79:77: sparse: sparse: restricted __le16 degrades to integer
drivers/interconnect/qcom/bcm-voter.c:83:78: sparse: sparse: restricted __le16 degrades to integer
>> drivers/interconnect/qcom/bcm-voter.c:89:66: sparse: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned long long [usertype] base @@ got restricted __le32 [usertype] unit @@
>> drivers/interconnect/qcom/bcm-voter.c:89:66: sparse: expected unsigned long long [usertype] base
drivers/interconnect/qcom/bcm-voter.c:89:66: sparse: got restricted __le32 [usertype] unit
drivers/interconnect/qcom/bcm-voter.c:92:66: sparse: sparse: incorrect type in argument 2 (different base types) @@ expected unsigned long long [usertype] base @@ got restricted __le32 [usertype] unit @@
drivers/interconnect/qcom/bcm-voter.c:92:66: sparse: expected unsigned long long [usertype] base
drivers/interconnect/qcom/bcm-voter.c:92:66: sparse: got restricted __le32 [usertype] unit
drivers/interconnect/qcom/bcm-voter.c:124:21: sparse: sparse: restricted __le32 degrades to integer
drivers/interconnect/qcom/bcm-voter.c:124:21: sparse: sparse: restricted __le32 degrades to integer
vim +89 drivers/interconnect/qcom/bcm-voter.c
67
68 static void bcm_aggregate(struct qcom_icc_bcm *bcm)
69 {
70 struct qcom_icc_node *node;
71 size_t i, bucket;
72 u64 agg_avg[QCOM_ICC_NUM_BUCKETS] = {0};
73 u64 agg_peak[QCOM_ICC_NUM_BUCKETS] = {0};
74 u64 temp;
75
76 for (bucket = 0; bucket < QCOM_ICC_NUM_BUCKETS; bucket++) {
77 for (i = 0; i < bcm->num_nodes; i++) {
78 node = bcm->nodes[i];
79 temp = bcm_div(node->sum_avg[bucket] * bcm->aux_data.width,
80 node->buswidth * node->channels);
81 agg_avg[bucket] = max(agg_avg[bucket], temp);
82
83 temp = bcm_div(node->max_peak[bucket] * bcm->aux_data.width,
84 node->buswidth);
85 agg_peak[bucket] = max(agg_peak[bucket], temp);
86 }
87
88 temp = agg_avg[bucket] * bcm->vote_scale;
> 89 bcm->vote_x[bucket] = bcm_div(temp, bcm->aux_data.unit);
90
91 temp = agg_peak[bucket] * bcm->vote_scale;
92 bcm->vote_y[bucket] = bcm_div(temp, bcm->aux_data.unit);
93 }
94
95 if (bcm->keepalive && bcm->vote_x[QCOM_ICC_BUCKET_AMC] == 0 &&
96 bcm->vote_y[QCOM_ICC_BUCKET_AMC] == 0) {
97 bcm->vote_x[QCOM_ICC_BUCKET_AMC] = 1;
98 bcm->vote_x[QCOM_ICC_BUCKET_WAKE] = 1;
99 bcm->vote_y[QCOM_ICC_BUCKET_AMC] = 1;
100 bcm->vote_y[QCOM_ICC_BUCKET_WAKE] = 1;
101 }
102 }
103
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/interconnect/qcom/bcm-voter.c b/drivers/interconnect/qcom/bcm-voter.c index a68c858ca6b7..9e2612fe7fad 100644 --- a/drivers/interconnect/qcom/bcm-voter.c +++ b/drivers/interconnect/qcom/bcm-voter.c @@ -54,8 +54,20 @@ static int cmp_vcd(void *priv, struct list_head *a, struct list_head *b) return 1; } +static u64 bcm_div(u64 num, u64 base) +{ + /* Ensure that small votes aren't lost. */ + if (num && num < base) + return 1; + + do_div(num, base); + + return num; +} + static void bcm_aggregate(struct qcom_icc_bcm *bcm) { + struct qcom_icc_node *node; size_t i, bucket; u64 agg_avg[QCOM_ICC_NUM_BUCKETS] = {0}; u64 agg_peak[QCOM_ICC_NUM_BUCKETS] = {0}; @@ -63,22 +75,21 @@ static void bcm_aggregate(struct qcom_icc_bcm *bcm) for (bucket = 0; bucket < QCOM_ICC_NUM_BUCKETS; bucket++) { for (i = 0; i < bcm->num_nodes; i++) { - temp = bcm->nodes[i]->sum_avg[bucket] * bcm->aux_data.width; - do_div(temp, bcm->nodes[i]->buswidth * bcm->nodes[i]->channels); + node = bcm->nodes[i]; + temp = bcm_div(node->sum_avg[bucket] * bcm->aux_data.width, + node->buswidth * node->channels); agg_avg[bucket] = max(agg_avg[bucket], temp); - temp = bcm->nodes[i]->max_peak[bucket] * bcm->aux_data.width; - do_div(temp, bcm->nodes[i]->buswidth); + temp = bcm_div(node->max_peak[bucket] * bcm->aux_data.width, + node->buswidth); agg_peak[bucket] = max(agg_peak[bucket], temp); } temp = agg_avg[bucket] * bcm->vote_scale; - do_div(temp, bcm->aux_data.unit); - bcm->vote_x[bucket] = temp; + bcm->vote_x[bucket] = bcm_div(temp, bcm->aux_data.unit); temp = agg_peak[bucket] * bcm->vote_scale; - do_div(temp, bcm->aux_data.unit); - bcm->vote_y[bucket] = temp; + bcm->vote_y[bucket] = bcm_div(temp, bcm->aux_data.unit); } if (bcm->keepalive && bcm->vote_x[QCOM_ICC_BUCKET_AMC] == 0 &&
Small BW votes that translate to less than a single BCM unit are currently truncated to zero. Ensure that non-zero BW requests always result in at least a vote of 1 to BCM. Fixes: 976daac4a1c5 ("interconnect: qcom: Consolidate interconnect RPMh support") Signed-off-by: Mike Tipton <mdtipton@codeaurora.org> --- drivers/interconnect/qcom/bcm-voter.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-)