Hello,

On Mon, May 17, 2021 at 02:39:21PM +0000, Dennis Zhou wrote:

On Mon, May 17, 2021 at 06:17:47AM -0700, Tom Rix wrote:

quoted

On 5/16/21 7:05 PM, Dennis Zhou wrote:

quoted

Hello,

On Sat, May 15, 2021 at 11:08:17AM -0700, trix@redhat.com wrote:

quoted

From: Tom Rix <trix@redhat.com>

Static analysis reports this problem
percpu.c:2945:6: warning: Assigned value is garbage or undefined
         upa = best_upa;
             ^ ~~~~~~~~
best_upa may not be set, so initialize it.

Signed-off-by: Tom Rix <trix@redhat.com>
---
  mm/percpu.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/mm/percpu.c b/mm/percpu.c
index a257c3efdf18b..6578b706fae81 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c

@@ -2916,6 +2916,7 @@ static struct pcpu_alloc_info * __init __flatten pcpu_build_alloc_info(
  	 * Related to atom_size, which could be much larger than the unit_size.
  	 */
  	last_allocs = INT_MAX;
+	best_upa = max_upa;
  	for (upa = max_upa; upa; upa--) {
  		int allocs = 0, wasted = 0;

-- 
2.26.3

I think the proper fix would be:

best_upa = 0;

I was looking for initializing with something that would work.

I think I prefer setting it to 0 because it forces the loop to have
succeeded vs being able to bypass it if the for loop logic was changed.

quoted

quoted

for (...) { }
BUG_ON(!best_upa);

WARN_ON instead?

This is initialization code. So if upa == 0, it really is a problem.
Having 0 units per allocation is bogus.

quoted

quoted

upa = best_upa;

If you're fine with this I'll make the changes and apply it to
for-5.13-fixes.

Can you also tell me what static analysis tool produced this? I'm just a
little curious because this code hasn't changed in several years so I'd
have expected some static analyzer to have caught this by now.

Clang 10

Tom

Thanks,
Dennis

Following up here. Are you find with me making the changes and
attributing it to you? Otherwise I can just spin another patch real
quick.

At this point I've already sent my PR for-5.13-fixes. So I'll queue some
fix for-5.14.
 
Thanks,
Dennis