Re: [PATCH v4 0/6] arch_topology: Build cacheinfo from primary CPU

[Pierre Gondois]

On 1/18/23 13:07, Sudeep Holla wrote:
> On Wed, Jan 18, 2023 at 11:55:59AM +0000, Sudeep Holla wrote:
> > On Wed, 4 Jan 2023 19:30:23 +0100, Pierre Gondois wrote:
> > > v2:
> > >   - Applied renaming/formatting comments from v1.
> > >   - Check CACHE_TYPE_VALID flag in pppt.c.
> > > v3:
> > >   - Applied Sudeep's suggestions (for patch 5/5):
> > >     - Renaming allocate_cache_info() -> fecth_cache_info()
> > >     - Updated error message
> > >     - Extract an inline allocate_cache_info() function
> > >   - Re-run checkpatch with --strict option
> > > v4:
> > >   - Remove RISC-V's implementation of init_cache_level() as not
> > >     necessary.
> > >   - Add patch: 'cacheinfo: Check 'cache-unified' property to count
> > >     cache leaves' to increase the number of leaves at a cache level
> > >     when no cache-size property is found.
> > >   - In cacheinfo: Use RISC-V's init_cache_level() [...],
> > >     make 'levels', 'leaves' and 'level' unsigned int to match
> > >     of_property_read_u32()'s parameters signedness.
> > >
> > > [...]
> >
> > Applied to sudeep.holla/linux (for-next/cacheinfo), thanks!
>
> I pushed the changes and then noticed some build warning report by
> kbuild posted only to you and one list(missing this list). Please post the
> fix if required on top of my for-next/cacheinfo so that it can be added
> on the top. Sorry for missing that.

Hi Sudeep,
I think the reported issue can be ignored, the 'levels' and 'split_levels'
variables are initialized when used. If necessary, it is straightforward
to fix the warning.
Regards,
Pierre


The reported issue:
--- Start ---

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@xxxxxxxxx>

All warnings (new ones prefixed by >>):

   drivers/base/cacheinfo.c: In function 'fetch_cache_info':
> > drivers/base/cacheinfo.c:440:50: warning: 'levels' is used uninitialized [-Wuninitialized]
     440 |                 this_cpu_ci->num_leaves = levels + split_levels;
         |                                           ~~~~~~~^~~~~~~~~~~~~~
   drivers/base/cacheinfo.c:420:22: note: 'levels' was declared here
     420 |         unsigned int levels, split_levels;
         |                      ^~~~~~
> > drivers/base/cacheinfo.c:440:50: warning: 'split_levels' is used uninitialized [-Wuninitialized]
     440 |                 this_cpu_ci->num_leaves = levels + split_levels;
         |                                           ~~~~~~~^~~~~~~~~~~~~~
   drivers/base/cacheinfo.c:420:30: note: 'split_levels' was declared here
     420 |         unsigned int levels, split_levels;
         |                              ^~~~~~~~~~~~


vim +/levels +440 drivers/base/cacheinfo.c

   416	
   417	int fetch_cache_info(unsigned int cpu)
   418	{
   419		struct cpu_cacheinfo *this_cpu_ci;
   420		unsigned int levels, split_levels;
   421		int ret;
   422	
   423		if (acpi_disabled) {
   424			ret = init_of_cache_level(cpu);
   425			if (ret < 0)
   426				return ret;
   427		} else {
   428			ret = acpi_get_cache_info(cpu, &levels, &split_levels);
   429			if (ret < 0)
   430				return ret;
   431	
   432			this_cpu_ci = get_cpu_cacheinfo(cpu);
   433			this_cpu_ci->num_levels = levels;
   434			/*
   435			 * This assumes that:
   436			 * - there cannot be any split caches (data/instruction)
   437			 *   above a unified cache
   438			 * - data/instruction caches come by pair
   439			 */
 > 440			this_cpu_ci->num_leaves = levels + split_levels;
   441		}
   442		if (!cache_leaves(cpu))
   443			return -ENOENT;
   444	
   445		return allocate_cache_info(cpu);
   446	}
   447	

--- End ---