Date:   Tue, 24 Jan 2023 14:05:37 +0100
From:   Borislav Petkov <bp@alien8.de>
To:     kernel test robot <lkp@intel.com>
Cc:     Babu Moger <babu.moger@amd.com>, llvm@lists.linux.dev,
        oe-kbuild-all@lists.linux.dev, linux-kernel@vger.kernel.org,
        x86@kernel.org, Reinette Chatre <reinette.chatre@intel.com>
Subject: Re: [tip:x86/cache 9/13]
 arch/x86/kernel/cpu/resctrl/rdtgroup.c:1456:6: warning: variable 'h' set but
 not used
Message-ID: <Y8/XoT23HVXHSY73@zn.tnic>
References: <202301242015.kbzkVteJ-lkp@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <202301242015.kbzkVteJ-lkp@intel.com>
Precedence: bulk

On Tue, Jan 24, 2023 at 08:37:14PM +0800, kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/cache
> head:   0a363fb23ee2f7beb08437ad7db86d195878d79f
> commit: dc2a3e857981f859889933cf66ded117d74edff1 [9/13] x86/resctrl: Add interface to read mbm_total_bytes_config

These patches have been around for a long while now. How come you test them just
now, after I take them?

> >> arch/x86/kernel/cpu/resctrl/rdtgroup.c:1456:6: warning: variable 'h' set but not used [-Wunused-but-set-variable]
>            u32 h;
>                ^

The fix is simple - use rsmsrl(), see below.

If it weren't that simple I'd simply ignore this warning altogether. Yes, it is
unused because the damn high MSR value needs to go somewhere, for chrissakes.
And if it didn't need that masking with MAX_EVT_CONFIG_BITS but the low 32-bit
MSR value would fit right into mon_info->mon_config, then I would've ignored
this one even faster.

And besides, I'm wondering if you're spending your 0day resources properly if
you're testing patches one-by-one for W=1 warnings?!

If it were me, I'd prefer if you build-test patches on lkml for build *errors*,
*before* they get picked up in some tree instead of testing the applied branches
already for some questionable warnings...

Thx.

---

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index cd4e668e5019..9bd0eb050e7a 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1453,17 +1453,17 @@ static void mon_event_config_read(void *info)
 {
 	struct mon_config_info *mon_info = info;
 	unsigned int index;
-	u32 h;
+	u64 msrval;
 
 	index = mon_event_config_index_get(mon_info->evtid);
 	if (index == INVALID_CONFIG_INDEX) {
 		pr_warn_once("Invalid event id %d\n", mon_info->evtid);
 		return;
 	}
-	rdmsr(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config, h);
+	rdmsrl(MSR_IA32_EVT_CFG_BASE + index, msrval);
 
 	/* Report only the valid event configuration bits */
-	mon_info->mon_config &= MAX_EVT_CONFIG_BITS;
+	mon_info->mon_config = msrval & MAX_EVT_CONFIG_BITS;
 }
 
 static void mondata_config_read(struct rdt_domain *d, struct mon_config_info *mon_info)


-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette