Messages in this thread | | | Date | Tue, 24 Jan 2023 13:50:45 +0000 | From | Sudeep Holla <> | Subject | Re: [PATCH -next] cacheinfo: Correctly handle new acpi_get_cache_info() prototype |
| |
On Tue, Jan 24, 2023 at 01:31:06PM +0000, Conor Dooley wrote: > Hey! > > On Tue, Jan 24, 2023 at 01:34:46PM +0100, Pierre Gondois wrote: > > commit bd500361a937 ("ACPI: PPTT: Update acpi_find_last_cache_level() > > to acpi_get_cache_info()") > > updates the function acpi_get_cache_info(). > > > > If CONFIG_ACPI_PPTT is not defined, acpi_get_cache_info() doesn't > > update its *levels and *split_levels parameters and returns 0. > > This can lead to a faulty behaviour. > > > > Make acpi_get_cache_info() return an error code if CONFIG_ACPI_PPTT > > is not defined. Initialize levels and split_levels before passing > > their address to acpi_get_cache_info(). > > > > Also, in init_cache_level(): > > Hmm... > > > - commit e75d18cecbb3 ("arm64: cacheinfo: Fix incorrect > > assignment of signed error value to unsigned fw_level") > > checks the fw_level value in init_cache_level() in case > > the value is negative. Remove this check as the error code > > is not returned through fw_level anymore. > > - if no PPTT is present or CONFIG_ACPI_PPTT is not defined, > > it is still possible to use the cache information from clidr_el1. > > Instead of aborting if acpi_get_cache_info() returns an error > > code, just continue. > > To be honest, these feel like entirely separate things that should be > in different patches. You've got: > - Dan's smatch fixes > - a redundant check being removed > - a behaviour change for if acpi_get_cache_info() returns an error >
I am not too fussy about it, but for sure it would be cleaner for sure.
> > Reported-by: Dan Carpenter <error27@gmail.com> > > Reported-by: kernel test robot <lkp@intel.com> > > How about Link: to the LKP/Dan's report? > Link: https://lore.kernel.org/all/Y86iruJPuwNN7rZw@kili/ > > I did a quick check but didn't don't see the LKP report... >
Yes, LKP dropped all the cc when reported, even I saw after merging the changes. I think this is the one:
https://lore.kernel.org/all/202301052307.JYt1GWaJ-lkp@intel.com/
> Also a Fixes: tag too, no? >
+1, if you split make sure you tag fixes to the right one(mainly one that changes return from acpi_get_cache_info())
> Thanks, > Conor.
-- Regards, Sudeep
| |