[tip:x86/cache 3/3] arch/x86/kernel/cpu/resctrl/rdtgroup.c:1621:5-8: Unneeded variable: "ret". Return " 0" on line 1655

[tip:x86/cache 3/3] arch/x86/kernel/cpu/resctrl/rdtgroup.c:1621:5-8: Unneeded variable: "ret". Return " 0" on line 1655

[kernel test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/cache
head:   54e35eb8611cce5550d3d7689679b1a91c864f28
commit: 54e35eb8611cce5550d3d7689679b1a91c864f28 [3/3] x86/resctrl: Read supported bandwidth sources from CPUID
config: x86_64-randconfig-102-20240124 (https://download.01.org/0day-ci/archive/20240124/202401241810.jbd8Ipa1-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202401241810.jbd8Ipa1-lkp@intel.com/

cocci warnings: (new ones prefixed by >>)
>> arch/x86/kernel/cpu/resctrl/rdtgroup.c:1621:5-8: Unneeded variable: "ret". Return "  0" on line 1655

vim +1621 arch/x86/kernel/cpu/resctrl/rdtgroup.c

92bd5a1390335b Babu Moger 2023-01-13  1616  
92bd5a1390335b Babu Moger 2023-01-13  1617  static int mbm_config_write_domain(struct rdt_resource *r,
92bd5a1390335b Babu Moger 2023-01-13  1618  				   struct rdt_domain *d, u32 evtid, u32 val)
92bd5a1390335b Babu Moger 2023-01-13  1619  {
92bd5a1390335b Babu Moger 2023-01-13  1620  	struct mon_config_info mon_info = {0};
92bd5a1390335b Babu Moger 2023-01-13 @1621  	int ret = 0;
92bd5a1390335b Babu Moger 2023-01-13  1622  
92bd5a1390335b Babu Moger 2023-01-13  1623  	/*
92bd5a1390335b Babu Moger 2023-01-13  1624  	 * Read the current config value first. If both are the same then
92bd5a1390335b Babu Moger 2023-01-13  1625  	 * no need to write it again.
92bd5a1390335b Babu Moger 2023-01-13  1626  	 */
92bd5a1390335b Babu Moger 2023-01-13  1627  	mon_info.evtid = evtid;
92bd5a1390335b Babu Moger 2023-01-13  1628  	mondata_config_read(d, &mon_info);
92bd5a1390335b Babu Moger 2023-01-13  1629  	if (mon_info.mon_config == val)
92bd5a1390335b Babu Moger 2023-01-13  1630  		goto out;
92bd5a1390335b Babu Moger 2023-01-13  1631  
92bd5a1390335b Babu Moger 2023-01-13  1632  	mon_info.mon_config = val;
92bd5a1390335b Babu Moger 2023-01-13  1633  
92bd5a1390335b Babu Moger 2023-01-13  1634  	/*
92bd5a1390335b Babu Moger 2023-01-13  1635  	 * Update MSR_IA32_EVT_CFG_BASE MSR on one of the CPUs in the
92bd5a1390335b Babu Moger 2023-01-13  1636  	 * domain. The MSRs offset from MSR MSR_IA32_EVT_CFG_BASE
92bd5a1390335b Babu Moger 2023-01-13  1637  	 * are scoped at the domain level. Writing any of these MSRs
92bd5a1390335b Babu Moger 2023-01-13  1638  	 * on one CPU is observed by all the CPUs in the domain.
92bd5a1390335b Babu Moger 2023-01-13  1639  	 */
92bd5a1390335b Babu Moger 2023-01-13  1640  	smp_call_function_any(&d->cpu_mask, mon_event_config_write,
92bd5a1390335b Babu Moger 2023-01-13  1641  			      &mon_info, 1);
92bd5a1390335b Babu Moger 2023-01-13  1642  
92bd5a1390335b Babu Moger 2023-01-13  1643  	/*
92bd5a1390335b Babu Moger 2023-01-13  1644  	 * When an Event Configuration is changed, the bandwidth counters
92bd5a1390335b Babu Moger 2023-01-13  1645  	 * for all RMIDs and Events will be cleared by the hardware. The
92bd5a1390335b Babu Moger 2023-01-13  1646  	 * hardware also sets MSR_IA32_QM_CTR.Unavailable (bit 62) for
92bd5a1390335b Babu Moger 2023-01-13  1647  	 * every RMID on the next read to any event for every RMID.
92bd5a1390335b Babu Moger 2023-01-13  1648  	 * Subsequent reads will have MSR_IA32_QM_CTR.Unavailable (bit 62)
92bd5a1390335b Babu Moger 2023-01-13  1649  	 * cleared while it is tracked by the hardware. Clear the
92bd5a1390335b Babu Moger 2023-01-13  1650  	 * mbm_local and mbm_total counts for all the RMIDs.
92bd5a1390335b Babu Moger 2023-01-13  1651  	 */
92bd5a1390335b Babu Moger 2023-01-13  1652  	resctrl_arch_reset_rmid_all(r, d);
92bd5a1390335b Babu Moger 2023-01-13  1653  
92bd5a1390335b Babu Moger 2023-01-13  1654  out:
92bd5a1390335b Babu Moger 2023-01-13 @1655  	return ret;
92bd5a1390335b Babu Moger 2023-01-13  1656  }
92bd5a1390335b Babu Moger 2023-01-13  1657  

:::::: The code at line 1621 was first introduced by commit
:::::: 92bd5a1390335bb3cc76bdf1b4356edbc94d408d x86/resctrl: Add interface to write mbm_total_bytes_config

:::::: TO: Babu Moger <babu.moger@amd.com>
:::::: CC: Borislav Petkov (AMD) <bp@alien8.de>

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [tip:x86/cache 3/3] arch/x86/kernel/cpu/resctrl/rdtgroup.c:1621:5-8: Unneeded variable: "ret". Return " 0" on line 1655

[Borislav Petkov]

On Wed, Jan 24, 2024 at 06:31:41PM +0800, kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/cache
> head:   54e35eb8611cce5550d3d7689679b1a91c864f28
> commit: 54e35eb8611cce5550d3d7689679b1a91c864f28 [3/3] x86/resctrl: Read supported bandwidth sources from CPUID
> config: x86_64-randconfig-102-20240124 (https://download.01.org/0day-ci/archive/20240124/202401241810.jbd8Ipa1-lkp@intel.com/config)
> compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202401241810.jbd8Ipa1-lkp@intel.com/
> 
> cocci warnings: (new ones prefixed by >>)
> >> arch/x86/kernel/cpu/resctrl/rdtgroup.c:1621:5-8: Unneeded variable: "ret". Return "  0" on line 1655

Well, AFAICT, even with the tree checked out at

92bd5a139033 ("x86/resctrl: Add interface to write mbm_total_bytes_config")

which is the first patch that added this function, ret is unneeded.

But scripts/coccinelle/misc/returnvar.cocci doesn't warn about it then,
only now that that hunk with the "return -EINVAL;" is removed in the
patch you're reporting this against.

Lemme add some cocci people to Cc for comment and leave the rest for
reference.

Thx.

> vim +1621 arch/x86/kernel/cpu/resctrl/rdtgroup.c
> 
> 92bd5a1390335b Babu Moger 2023-01-13  1616  
> 92bd5a1390335b Babu Moger 2023-01-13  1617  static int mbm_config_write_domain(struct rdt_resource *r,
> 92bd5a1390335b Babu Moger 2023-01-13  1618  				   struct rdt_domain *d, u32 evtid, u32 val)
> 92bd5a1390335b Babu Moger 2023-01-13  1619  {
> 92bd5a1390335b Babu Moger 2023-01-13  1620  	struct mon_config_info mon_info = {0};
> 92bd5a1390335b Babu Moger 2023-01-13 @1621  	int ret = 0;
> 92bd5a1390335b Babu Moger 2023-01-13  1622  
> 92bd5a1390335b Babu Moger 2023-01-13  1623  	/*
> 92bd5a1390335b Babu Moger 2023-01-13  1624  	 * Read the current config value first. If both are the same then
> 92bd5a1390335b Babu Moger 2023-01-13  1625  	 * no need to write it again.
> 92bd5a1390335b Babu Moger 2023-01-13  1626  	 */
> 92bd5a1390335b Babu Moger 2023-01-13  1627  	mon_info.evtid = evtid;
> 92bd5a1390335b Babu Moger 2023-01-13  1628  	mondata_config_read(d, &mon_info);
> 92bd5a1390335b Babu Moger 2023-01-13  1629  	if (mon_info.mon_config == val)
> 92bd5a1390335b Babu Moger 2023-01-13  1630  		goto out;
> 92bd5a1390335b Babu Moger 2023-01-13  1631  
> 92bd5a1390335b Babu Moger 2023-01-13  1632  	mon_info.mon_config = val;
> 92bd5a1390335b Babu Moger 2023-01-13  1633  
> 92bd5a1390335b Babu Moger 2023-01-13  1634  	/*
> 92bd5a1390335b Babu Moger 2023-01-13  1635  	 * Update MSR_IA32_EVT_CFG_BASE MSR on one of the CPUs in the
> 92bd5a1390335b Babu Moger 2023-01-13  1636  	 * domain. The MSRs offset from MSR MSR_IA32_EVT_CFG_BASE
> 92bd5a1390335b Babu Moger 2023-01-13  1637  	 * are scoped at the domain level. Writing any of these MSRs
> 92bd5a1390335b Babu Moger 2023-01-13  1638  	 * on one CPU is observed by all the CPUs in the domain.
> 92bd5a1390335b Babu Moger 2023-01-13  1639  	 */
> 92bd5a1390335b Babu Moger 2023-01-13  1640  	smp_call_function_any(&d->cpu_mask, mon_event_config_write,
> 92bd5a1390335b Babu Moger 2023-01-13  1641  			      &mon_info, 1);
> 92bd5a1390335b Babu Moger 2023-01-13  1642  
> 92bd5a1390335b Babu Moger 2023-01-13  1643  	/*
> 92bd5a1390335b Babu Moger 2023-01-13  1644  	 * When an Event Configuration is changed, the bandwidth counters
> 92bd5a1390335b Babu Moger 2023-01-13  1645  	 * for all RMIDs and Events will be cleared by the hardware. The
> 92bd5a1390335b Babu Moger 2023-01-13  1646  	 * hardware also sets MSR_IA32_QM_CTR.Unavailable (bit 62) for
> 92bd5a1390335b Babu Moger 2023-01-13  1647  	 * every RMID on the next read to any event for every RMID.
> 92bd5a1390335b Babu Moger 2023-01-13  1648  	 * Subsequent reads will have MSR_IA32_QM_CTR.Unavailable (bit 62)
> 92bd5a1390335b Babu Moger 2023-01-13  1649  	 * cleared while it is tracked by the hardware. Clear the
> 92bd5a1390335b Babu Moger 2023-01-13  1650  	 * mbm_local and mbm_total counts for all the RMIDs.
> 92bd5a1390335b Babu Moger 2023-01-13  1651  	 */
> 92bd5a1390335b Babu Moger 2023-01-13  1652  	resctrl_arch_reset_rmid_all(r, d);
> 92bd5a1390335b Babu Moger 2023-01-13  1653  
> 92bd5a1390335b Babu Moger 2023-01-13  1654  out:
> 92bd5a1390335b Babu Moger 2023-01-13 @1655  	return ret;
> 92bd5a1390335b Babu Moger 2023-01-13  1656  }
> 92bd5a1390335b Babu Moger 2023-01-13  1657  
> 
> :::::: The code at line 1621 was first introduced by commit
> :::::: 92bd5a1390335bb3cc76bdf1b4356edbc94d408d x86/resctrl: Add interface to write mbm_total_bytes_config
> 
> :::::: TO: Babu Moger <babu.moger@amd.com>
> :::::: CC: Borislav Petkov (AMD) <bp@alien8.de>
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki

-- 
Regards/Gruss,
    Boris.

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

Re: [tip:x86/cache 3/3] arch/x86/kernel/cpu/resctrl/rdtgroup.c:1621:5-8: Unneeded variable: "ret". Return " 0" on line 1655

[Moger, Babu]

Hi Boris,

On 1/24/24 04:55, Borislav Petkov wrote:
> On Wed, Jan 24, 2024 at 06:31:41PM +0800, kernel test robot wrote:
>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/cache
>> head:   54e35eb8611cce5550d3d7689679b1a91c864f28
>> commit: 54e35eb8611cce5550d3d7689679b1a91c864f28 [3/3] x86/resctrl: Read supported bandwidth sources from CPUID
>> config: x86_64-randconfig-102-20240124 (https://download.01.org/0day-ci/archive/20240124/202401241810.jbd8Ipa1-lkp@intel.com/config)
>> compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
>>
>> If you fix the issue in a separate patch/commit (i.e. not just a new version of
>> the same patch/commit), kindly add following tags
>> | Reported-by: kernel test robot <lkp@intel.com>
>> | Closes: https://lore.kernel.org/oe-kbuild-all/202401241810.jbd8Ipa1-lkp@intel.com/
>>
>> cocci warnings: (new ones prefixed by >>)
>>>> arch/x86/kernel/cpu/resctrl/rdtgroup.c:1621:5-8: Unneeded variable: "ret". Return "  0" on line 1655

Hmm Interesting..

Looking at the code, I see the ret variable is really not required.

The following patch should fix the problem. Let me know if you want me to
send the patch officially. Thanks

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 2b69e560b05f..6057f96df73f 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1618,7 +1618,6 @@ static int mbm_config_write_domain(struct
rdt_resource *r,
                                   struct rdt_domain *d, u32 evtid, u32 val)
 {
        struct mon_config_info mon_info = {0};
-       int ret = 0;

        /*
         * Read the current config value first. If both are the same then
@@ -1652,7 +1651,7 @@ static int mbm_config_write_domain(struct
rdt_resource *r,
        resctrl_arch_reset_rmid_all(r, d);

 out:
-       return ret;
+       return 0;
 }

 static int mon_config_write(struct rdt_resource *r, char *tok, u32 evtid)

> 
> Well, AFAICT, even with the tree checked out at
> 
> 92bd5a139033 ("x86/resctrl: Add interface to write mbm_total_bytes_config")
> 
> which is the first patch that added this function, ret is unneeded.
> 
> But scripts/coccinelle/misc/returnvar.cocci doesn't warn about it then,
> only now that that hunk with the "return -EINVAL;" is removed in the
> patch you're reporting this against.
> 
> Lemme add some cocci people to Cc for comment and leave the rest for
> reference.
> 
> Thx.
> 
>> vim +1621 arch/x86/kernel/cpu/resctrl/rdtgroup.c
>>
>> 92bd5a1390335b Babu Moger 2023-01-13  1616  
>> 92bd5a1390335b Babu Moger 2023-01-13  1617  static int mbm_config_write_domain(struct rdt_resource *r,
>> 92bd5a1390335b Babu Moger 2023-01-13  1618  				   struct rdt_domain *d, u32 evtid, u32 val)
>> 92bd5a1390335b Babu Moger 2023-01-13  1619  {
>> 92bd5a1390335b Babu Moger 2023-01-13  1620  	struct mon_config_info mon_info = {0};
>> 92bd5a1390335b Babu Moger 2023-01-13 @1621  	int ret = 0;
>> 92bd5a1390335b Babu Moger 2023-01-13  1622  
>> 92bd5a1390335b Babu Moger 2023-01-13  1623  	/*
>> 92bd5a1390335b Babu Moger 2023-01-13  1624  	 * Read the current config value first. If both are the same then
>> 92bd5a1390335b Babu Moger 2023-01-13  1625  	 * no need to write it again.
>> 92bd5a1390335b Babu Moger 2023-01-13  1626  	 */
>> 92bd5a1390335b Babu Moger 2023-01-13  1627  	mon_info.evtid = evtid;
>> 92bd5a1390335b Babu Moger 2023-01-13  1628  	mondata_config_read(d, &mon_info);
>> 92bd5a1390335b Babu Moger 2023-01-13  1629  	if (mon_info.mon_config == val)
>> 92bd5a1390335b Babu Moger 2023-01-13  1630  		goto out;
>> 92bd5a1390335b Babu Moger 2023-01-13  1631  
>> 92bd5a1390335b Babu Moger 2023-01-13  1632  	mon_info.mon_config = val;
>> 92bd5a1390335b Babu Moger 2023-01-13  1633  
>> 92bd5a1390335b Babu Moger 2023-01-13  1634  	/*
>> 92bd5a1390335b Babu Moger 2023-01-13  1635  	 * Update MSR_IA32_EVT_CFG_BASE MSR on one of the CPUs in the
>> 92bd5a1390335b Babu Moger 2023-01-13  1636  	 * domain. The MSRs offset from MSR MSR_IA32_EVT_CFG_BASE
>> 92bd5a1390335b Babu Moger 2023-01-13  1637  	 * are scoped at the domain level. Writing any of these MSRs
>> 92bd5a1390335b Babu Moger 2023-01-13  1638  	 * on one CPU is observed by all the CPUs in the domain.
>> 92bd5a1390335b Babu Moger 2023-01-13  1639  	 */
>> 92bd5a1390335b Babu Moger 2023-01-13  1640  	smp_call_function_any(&d->cpu_mask, mon_event_config_write,
>> 92bd5a1390335b Babu Moger 2023-01-13  1641  			      &mon_info, 1);
>> 92bd5a1390335b Babu Moger 2023-01-13  1642  
>> 92bd5a1390335b Babu Moger 2023-01-13  1643  	/*
>> 92bd5a1390335b Babu Moger 2023-01-13  1644  	 * When an Event Configuration is changed, the bandwidth counters
>> 92bd5a1390335b Babu Moger 2023-01-13  1645  	 * for all RMIDs and Events will be cleared by the hardware. The
>> 92bd5a1390335b Babu Moger 2023-01-13  1646  	 * hardware also sets MSR_IA32_QM_CTR.Unavailable (bit 62) for
>> 92bd5a1390335b Babu Moger 2023-01-13  1647  	 * every RMID on the next read to any event for every RMID.
>> 92bd5a1390335b Babu Moger 2023-01-13  1648  	 * Subsequent reads will have MSR_IA32_QM_CTR.Unavailable (bit 62)
>> 92bd5a1390335b Babu Moger 2023-01-13  1649  	 * cleared while it is tracked by the hardware. Clear the
>> 92bd5a1390335b Babu Moger 2023-01-13  1650  	 * mbm_local and mbm_total counts for all the RMIDs.
>> 92bd5a1390335b Babu Moger 2023-01-13  1651  	 */
>> 92bd5a1390335b Babu Moger 2023-01-13  1652  	resctrl_arch_reset_rmid_all(r, d);
>> 92bd5a1390335b Babu Moger 2023-01-13  1653  
>> 92bd5a1390335b Babu Moger 2023-01-13  1654  out:
>> 92bd5a1390335b Babu Moger 2023-01-13 @1655  	return ret;
>> 92bd5a1390335b Babu Moger 2023-01-13  1656  }
>> 92bd5a1390335b Babu Moger 2023-01-13  1657  
>>
>> :::::: The code at line 1621 was first introduced by commit
>> :::::: 92bd5a1390335bb3cc76bdf1b4356edbc94d408d x86/resctrl: Add interface to write mbm_total_bytes_config
>>
>> :::::: TO: Babu Moger <babu.moger@amd.com>
>> :::::: CC: Borislav Petkov (AMD) <bp@alien8.de>
>>
>> -- 
>> 0-DAY CI Kernel Test Service
>> https://github.com/intel/lkp-tests/wiki
> 

-- 
Thanks
Babu Moger

Re: [tip:x86/cache 3/3] arch/x86/kernel/cpu/resctrl/rdtgroup.c:1621:5-8: Unneeded variable: "ret". Return " 0" on line 1655

[Borislav Petkov]

On Wed, Jan 24, 2024 at 08:58:27AM -0600, Moger, Babu wrote:
> The following patch should fix the problem.

Obviously.

> Let me know if you want me to send the patch officially.

Yes, please send a proper patch and add the Reported-by: and the Closes:
tags from the robot.

Thx.

-- 
Regards/Gruss,
    Boris.

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

[PATCH] x86/resctrl: Fix unneeded variable warning reported by kernel test robot

[Babu Moger]

kernel test robot reported the following warning after the commit
54e35eb8611c ("x86/resctrl: Read supported bandwidth sources from CPUID").

cocci warnings: (new ones prefixed by >>)
>> arch/x86/kernel/cpu/resctrl/rdtgroup.c:1621:5-8: Unneeded variable: "ret". Return "  0" on line 1655

Fix the warning by removing the variable "ret" and returning 0 directly.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202401241810.jbd8Ipa1-lkp@intel.com/
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 2b69e560b05f..6057f96df73f 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1618,7 +1618,6 @@ static int mbm_config_write_domain(struct rdt_resource *r,
 				   struct rdt_domain *d, u32 evtid, u32 val)
 {
 	struct mon_config_info mon_info = {0};
-	int ret = 0;
 
 	/*
 	 * Read the current config value first. If both are the same then
@@ -1652,7 +1651,7 @@ static int mbm_config_write_domain(struct rdt_resource *r,
 	resctrl_arch_reset_rmid_all(r, d);
 
 out:
-	return ret;
+	return 0;
 }
 
 static int mon_config_write(struct rdt_resource *r, char *tok, u32 evtid)
-- 
2.34.1

Re: [PATCH] x86/resctrl: Fix unneeded variable warning reported by kernel test robot

[Reinette Chatre]

Hi Babu,

Thank you for fixing this so promptly.

I think the subject can just be:
"x86/resctrl: Remove unneeded variable"

On 1/24/2024 9:52 AM, Babu Moger wrote:
> kernel test robot reported the following warning after the commit
> 54e35eb8611c ("x86/resctrl: Read supported bandwidth sources from CPUID").

This can be confusing since it implies that the patch you mention
introduces the issue but instead the variable has been unneeded since
the original:
92bd5a139033 ("x86/resctrl: Add interface to write mbm_total_bytes_config")

To help clarify you can mention this order of events and also add an
appropriate "Fixes:" tag.

> cocci warnings: (new ones prefixed by >>)
>>> arch/x86/kernel/cpu/resctrl/rdtgroup.c:1621:5-8: Unneeded variable: "ret". Return "  0" on line 1655
> 
> Fix the warning by removing the variable "ret" and returning 0 directly.

cocci warning was spot on*. This fix is not just a change to "make a
warning go away" but instead fixing an actual problem.
It can just be "Remove the unneeded variable and return 0 directly".

> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202401241810.jbd8Ipa1-lkp@intel.com/
> Signed-off-by: Babu Moger <babu.moger@amd.com>

Reinette

* I'll add a private setup with the goal to catch these earlier.

Re: [PATCH] x86/resctrl: Fix unneeded variable warning reported by kernel test robot

[Borislav Petkov]

On Wed, Jan 24, 2024 at 10:25:17AM -0800, Reinette Chatre wrote:
> This can be confusing since it implies that the patch you mention
> introduces the issue but instead the variable has been unneeded since
> the original:
> 92bd5a139033 ("x86/resctrl: Add interface to write mbm_total_bytes_config")

What I said. :)

> To help clarify you can mention this order of events and also add an
> appropriate "Fixes:" tag.
> 
> > cocci warnings: (new ones prefixed by >>)
> >>> arch/x86/kernel/cpu/resctrl/rdtgroup.c:1621:5-8: Unneeded variable: "ret". Return "  0" on line 1655
> > 
> > Fix the warning by removing the variable "ret" and returning 0 directly.
> 
> cocci warning was spot on*. This fix is not just a change to "make a
> warning go away" but instead fixing an actual problem.
> It can just be "Remove the unneeded variable and return 0 directly".

I'll fix all up before applying.

> * I'll add a private setup with the goal to catch these earlier.

Except that it doesn't fire with the patch that added the code. It looks
like the cocci script needs adjustment...

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH] x86/resctrl: Fix unneeded variable warning reported by kernel test robot

[Reinette Chatre]

Hi Boris,

On 1/24/2024 10:31 AM, Borislav Petkov wrote:
> On Wed, Jan 24, 2024 at 10:25:17AM -0800, Reinette Chatre wrote:
>> This can be confusing since it implies that the patch you mention
>> introduces the issue but instead the variable has been unneeded since
>> the original:
>> 92bd5a139033 ("x86/resctrl: Add interface to write mbm_total_bytes_config")
> 
> What I said. :)

Right from the start, yes.

> 
>> To help clarify you can mention this order of events and also add an
>> appropriate "Fixes:" tag.
>>
>>> cocci warnings: (new ones prefixed by >>)
>>>>> arch/x86/kernel/cpu/resctrl/rdtgroup.c:1621:5-8: Unneeded variable: "ret". Return "  0" on line 1655
>>>
>>> Fix the warning by removing the variable "ret" and returning 0 directly.
>>
>> cocci warning was spot on*. This fix is not just a change to "make a
>> warning go away" but instead fixing an actual problem.
>> It can just be "Remove the unneeded variable and return 0 directly".
> 
> I'll fix all up before applying.

Thank you very much. For what it is worth, I do agree with the actual fix
and you can add:
Acked-by: Reinette Chatre <reinette.chatre@intel.com>

> 
>> * I'll add a private setup with the goal to catch these earlier.
> 
> Except that it doesn't fire with the patch that added the code. It looks
> like the cocci script needs adjustment...

Reinette

Re: [PATCH] x86/resctrl: Fix unneeded variable warning reported by kernel test robot

[Borislav Petkov]

On Wed, Jan 24, 2024 at 10:51:49AM -0800, Reinette Chatre wrote:
> Thank you very much. For what it is worth, I do agree with the actual fix
> and you can add:
> Acked-by: Reinette Chatre <reinette.chatre@intel.com>

Ok, have a look at the below, pls, and lemme know if that's ok too.

mbm_config_write_domain() only returns 0 so it can be void. So the
callsite doesn't need to check retval either.

Thx.

---
From: Babu Moger <babu.moger@amd.com>
Date: Wed, 24 Jan 2024 11:52:56 -0600
Subject: [PATCH] x86/resctrl: Remove redundant variable in
 mbm_config_write_domain()

The kernel test robot reported the following warning after

  54e35eb8611c ("x86/resctrl: Read supported bandwidth sources from CPUID").

even though the issue is present even in the original patch which added
this function

  92bd5a139033 ("x86/resctrl: Add interface to write mbm_total_bytes_config")

  $ make C=1 CHECK=scripts/coccicheck arch/x86/kernel/cpu/resctrl/rdtgroup.o
  ...
  arch/x86/kernel/cpu/resctrl/rdtgroup.c:1621:5-8: Unneeded variable: "ret". Return "0" on line 1655

Remove the local variable 'ret'.

  [ bp: Massage commit message, make mbm_config_write_domain() void. ]

Fixes: 92bd5a139033 ("x86/resctrl: Add interface to write mbm_total_bytes_config")
Closes: https://lore.kernel.org/oe-kbuild-all/202401241810.jbd8Ipa1-lkp@intel.com/
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Babu Moger <babu.moger@amd.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Acked-by: Reinette Chatre <reinette.chatre@intel.com>
Link: https://lore.kernel.org/r/202401241810.jbd8Ipa1-lkp@intel.com
---
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 2b69e560b05f..c33eb77b6d70 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1614,11 +1614,10 @@ static void mon_event_config_write(void *info)
 	wrmsr(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config, 0);
 }
 
-static int mbm_config_write_domain(struct rdt_resource *r,
+static void mbm_config_write_domain(struct rdt_resource *r,
 				   struct rdt_domain *d, u32 evtid, u32 val)
 {
 	struct mon_config_info mon_info = {0};
-	int ret = 0;
 
 	/*
 	 * Read the current config value first. If both are the same then
@@ -1627,7 +1626,7 @@ static int mbm_config_write_domain(struct rdt_resource *r,
 	mon_info.evtid = evtid;
 	mondata_config_read(d, &mon_info);
 	if (mon_info.mon_config == val)
-		goto out;
+		return;
 
 	mon_info.mon_config = val;
 
@@ -1650,9 +1649,6 @@ static int mbm_config_write_domain(struct rdt_resource *r,
 	 * mbm_local and mbm_total counts for all the RMIDs.
 	 */
 	resctrl_arch_reset_rmid_all(r, d);
-
-out:
-	return ret;
 }
 
 static int mon_config_write(struct rdt_resource *r, char *tok, u32 evtid)
@@ -1661,7 +1657,6 @@ static int mon_config_write(struct rdt_resource *r, char *tok, u32 evtid)
 	char *dom_str = NULL, *id_str;
 	unsigned long dom_id, val;
 	struct rdt_domain *d;
-	int ret = 0;
 
 next:
 	if (!tok || tok[0] == '\0')
@@ -1690,9 +1685,7 @@ static int mon_config_write(struct rdt_resource *r, char *tok, u32 evtid)
 
 	list_for_each_entry(d, &r->domains, list) {
 		if (d->id == dom_id) {
-			ret = mbm_config_write_domain(r, d, evtid, val);
-			if (ret)
-				return -EINVAL;
+			mbm_config_write_domain(r, d, evtid, val);
 			goto next;
 		}
 	}
-- 
2.43.0

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH] x86/resctrl: Fix unneeded variable warning reported by kernel test robot

[Moger, Babu]

Hi Boris,

On 1/24/24 13:14, Borislav Petkov wrote:
> On Wed, Jan 24, 2024 at 10:51:49AM -0800, Reinette Chatre wrote:
>> Thank you very much. For what it is worth, I do agree with the actual fix
>> and you can add:
>> Acked-by: Reinette Chatre <reinette.chatre@intel.com>
> 
> Ok, have a look at the below, pls, and lemme know if that's ok too.
> 
> mbm_config_write_domain() only returns 0 so it can be void. So the
> callsite doesn't need to check retval either.

Yes. Looks good. Compile tested also. Thanks

> 
> Thx.
> 
> ---
> From: Babu Moger <babu.moger@amd.com>
> Date: Wed, 24 Jan 2024 11:52:56 -0600
> Subject: [PATCH] x86/resctrl: Remove redundant variable in
>  mbm_config_write_domain()
> 
> The kernel test robot reported the following warning after
> 
>   54e35eb8611c ("x86/resctrl: Read supported bandwidth sources from CPUID").
> 
> even though the issue is present even in the original patch which added
> this function
> 
>   92bd5a139033 ("x86/resctrl: Add interface to write mbm_total_bytes_config")
> 
>   $ make C=1 CHECK=scripts/coccicheck arch/x86/kernel/cpu/resctrl/rdtgroup.o
>   ...
>   arch/x86/kernel/cpu/resctrl/rdtgroup.c:1621:5-8: Unneeded variable: "ret". Return "0" on line 1655
> 
> Remove the local variable 'ret'.
> 
>   [ bp: Massage commit message, make mbm_config_write_domain() void. ]
> 
> Fixes: 92bd5a139033 ("x86/resctrl: Add interface to write mbm_total_bytes_config")
> Closes: https://lore.kernel.org/oe-kbuild-all/202401241810.jbd8Ipa1-lkp@intel.com/
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> Acked-by: Reinette Chatre <reinette.chatre@intel.com>
> Link: https://lore.kernel.org/r/202401241810.jbd8Ipa1-lkp@intel.com
> ---
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 2b69e560b05f..c33eb77b6d70 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -1614,11 +1614,10 @@ static void mon_event_config_write(void *info)
>  	wrmsr(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config, 0);
>  }
>  
> -static int mbm_config_write_domain(struct rdt_resource *r,
> +static void mbm_config_write_domain(struct rdt_resource *r,
>  				   struct rdt_domain *d, u32 evtid, u32 val)
>  {
>  	struct mon_config_info mon_info = {0};
> -	int ret = 0;
>  
>  	/*
>  	 * Read the current config value first. If both are the same then
> @@ -1627,7 +1626,7 @@ static int mbm_config_write_domain(struct rdt_resource *r,
>  	mon_info.evtid = evtid;
>  	mondata_config_read(d, &mon_info);
>  	if (mon_info.mon_config == val)
> -		goto out;
> +		return;
>  
>  	mon_info.mon_config = val;
>  
> @@ -1650,9 +1649,6 @@ static int mbm_config_write_domain(struct rdt_resource *r,
>  	 * mbm_local and mbm_total counts for all the RMIDs.
>  	 */
>  	resctrl_arch_reset_rmid_all(r, d);
> -
> -out:
> -	return ret;
>  }
>  
>  static int mon_config_write(struct rdt_resource *r, char *tok, u32 evtid)
> @@ -1661,7 +1657,6 @@ static int mon_config_write(struct rdt_resource *r, char *tok, u32 evtid)
>  	char *dom_str = NULL, *id_str;
>  	unsigned long dom_id, val;
>  	struct rdt_domain *d;
> -	int ret = 0;
>  
>  next:
>  	if (!tok || tok[0] == '\0')
> @@ -1690,9 +1685,7 @@ static int mon_config_write(struct rdt_resource *r, char *tok, u32 evtid)
>  
>  	list_for_each_entry(d, &r->domains, list) {
>  		if (d->id == dom_id) {
> -			ret = mbm_config_write_domain(r, d, evtid, val);
> -			if (ret)
> -				return -EINVAL;
> +			mbm_config_write_domain(r, d, evtid, val);
>  			goto next;
>  		}
>  	}

-- 
Thanks
Babu Moger

Re: [PATCH] x86/resctrl: Fix unneeded variable warning reported by kernel test robot

[Borislav Petkov]

On Wed, Jan 24, 2024 at 01:39:01PM -0600, Moger, Babu wrote:
> Yes. Looks good. Compile tested also. Thanks

Thanks.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH] x86/resctrl: Fix unneeded variable warning reported by kernel test robot

[Reinette Chatre]

Hi Boris,

On 1/24/2024 11:14 AM, Borislav Petkov wrote:
> On Wed, Jan 24, 2024 at 10:51:49AM -0800, Reinette Chatre wrote:
>> Thank you very much. For what it is worth, I do agree with the actual fix
>> and you can add:
>> Acked-by: Reinette Chatre <reinette.chatre@intel.com>
> 
> Ok, have a look at the below, pls, and lemme know if that's ok too.
> 
> mbm_config_write_domain() only returns 0 so it can be void. So the
> callsite doesn't need to check retval either.

Thank you very much for looking further and catching this.

You may already have taken care of this after sending this version, but I
just have a couple of nitpick comments intended to help this patch in case it
requires a clean slate from some checks. 

> 
> Thx.
> 
> ---
> From: Babu Moger <babu.moger@amd.com>
> Date: Wed, 24 Jan 2024 11:52:56 -0600
> Subject: [PATCH] x86/resctrl: Remove redundant variable in
>  mbm_config_write_domain()
> 
> The kernel test robot reported the following warning after
> 
>   54e35eb8611c ("x86/resctrl: Read supported bandwidth sources from CPUID").

I think a "commit" prefix is required here and below.

> 
> even though the issue is present even in the original patch which added
> this function
> 
>   92bd5a139033 ("x86/resctrl: Add interface to write mbm_total_bytes_config")
> 
>   $ make C=1 CHECK=scripts/coccicheck arch/x86/kernel/cpu/resctrl/rdtgroup.o
>   ...
>   arch/x86/kernel/cpu/resctrl/rdtgroup.c:1621:5-8: Unneeded variable: "ret". Return "0" on line 1655
> 
> Remove the local variable 'ret'.
> 
>   [ bp: Massage commit message, make mbm_config_write_domain() void. ]
> 
> Fixes: 92bd5a139033 ("x86/resctrl: Add interface to write mbm_total_bytes_config")
> Closes: https://lore.kernel.org/oe-kbuild-all/202401241810.jbd8Ipa1-lkp@intel.com/
> Reported-by: kernel test robot <lkp@intel.com>

I do not know if this is something tip prefers but the general patch checkers prefer
that "Reported-by:" is followed by "Closes:".

> Signed-off-by: Babu Moger <babu.moger@amd.com>
> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> Acked-by: Reinette Chatre <reinette.chatre@intel.com>
> Link: https://lore.kernel.org/r/202401241810.jbd8Ipa1-lkp@intel.com
> ---
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 2b69e560b05f..c33eb77b6d70 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -1614,11 +1614,10 @@ static void mon_event_config_write(void *info)
>  	wrmsr(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config, 0);
>  }
>  
> -static int mbm_config_write_domain(struct rdt_resource *r,
> +static void mbm_config_write_domain(struct rdt_resource *r,
>  				   struct rdt_domain *d, u32 evtid, u32 val)

This shifts the alignment slightly to no longer match the open parenthesis.

>  {
>  	struct mon_config_info mon_info = {0};
> -	int ret = 0;
>  
>  	/*
>  	 * Read the current config value first. If both are the same then
> @@ -1627,7 +1626,7 @@ static int mbm_config_write_domain(struct rdt_resource *r,
>  	mon_info.evtid = evtid;
>  	mondata_config_read(d, &mon_info);
>  	if (mon_info.mon_config == val)
> -		goto out;
> +		return;
>  
>  	mon_info.mon_config = val;
>  
> @@ -1650,9 +1649,6 @@ static int mbm_config_write_domain(struct rdt_resource *r,
>  	 * mbm_local and mbm_total counts for all the RMIDs.
>  	 */
>  	resctrl_arch_reset_rmid_all(r, d);
> -
> -out:
> -	return ret;
>  }
>  
>  static int mon_config_write(struct rdt_resource *r, char *tok, u32 evtid)
> @@ -1661,7 +1657,6 @@ static int mon_config_write(struct rdt_resource *r, char *tok, u32 evtid)
>  	char *dom_str = NULL, *id_str;
>  	unsigned long dom_id, val;
>  	struct rdt_domain *d;
> -	int ret = 0;
>  
>  next:
>  	if (!tok || tok[0] == '\0')
> @@ -1690,9 +1685,7 @@ static int mon_config_write(struct rdt_resource *r, char *tok, u32 evtid)
>  
>  	list_for_each_entry(d, &r->domains, list) {
>  		if (d->id == dom_id) {
> -			ret = mbm_config_write_domain(r, d, evtid, val);
> -			if (ret)
> -				return -EINVAL;
> +			mbm_config_write_domain(r, d, evtid, val);
>  			goto next;
>  		}
>  	}

The core changes look good to me. Thank you very much for creating this fix.

Reinette

Re: [PATCH] x86/resctrl: Fix unneeded variable warning reported by kernel test robot

[Borislav Petkov]

Hi,

On Wed, Jan 24, 2024 at 12:04:31PM -0800, Reinette Chatre wrote:
> >   54e35eb8611c ("x86/resctrl: Read supported bandwidth sources from CPUID").
> 
> I think a "commit" prefix is required here and below.

Yeah, but if you see a 12-char sha1 followed by a title in (" "), that
is a commit and nothing else, right?

If I say "commit" too it is kinda redundant.

> > Fixes: 92bd5a139033 ("x86/resctrl: Add interface to write mbm_total_bytes_config")
> > Closes: https://lore.kernel.org/oe-kbuild-all/202401241810.jbd8Ipa1-lkp@intel.com/
> > Reported-by: kernel test robot <lkp@intel.com>
> 
> I do not know if this is something tip prefers but the general patch checkers prefer
> that "Reported-by:" is followed by "Closes:".

Good question. Documentation/process/maintainer-tip.rst doesn't clarify
that, lemme send a patch for it and see what my brethren in arms think.
:-)

> This shifts the alignment slightly to no longer match the open parenthesis.

Fixed.

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH] x86/resctrl: Fix unneeded variable warning reported by kernel test robot

[Reinette Chatre]

Hi Boris,

On 1/24/2024 12:45 PM, Borislav Petkov wrote:
> Hi,
> 
> On Wed, Jan 24, 2024 at 12:04:31PM -0800, Reinette Chatre wrote:
>>>   54e35eb8611c ("x86/resctrl: Read supported bandwidth sources from CPUID").
>>
>> I think a "commit" prefix is required here and below.
> 
> Yeah, but if you see a 12-char sha1 followed by a title in (" "), that
> is a commit and nothing else, right?
> 
> If I say "commit" too it is kinda redundant.

I do not know the motivation for that requirement. From what I can tell the
change [1] that added that check went in as first version without discussion.
[1] starts by saying that the format is "preferred" so I assume there is
some history that I am not familiar with.

Reinette

[1] https://lore.kernel.org/all/976c6cdd680db4b55ae31b5fc2d1779da5c0dc66.camel@perches.com/

Re: [PATCH] x86/resctrl: Fix unneeded variable warning reported by kernel test robot

[Borislav Petkov]

On Wed, Jan 24, 2024 at 01:31:01PM -0800, Reinette Chatre wrote:
> I do not know the motivation for that requirement. From what I can tell the
> change [1] that added that check went in as first version without discussion.
> [1] starts by saying that the format is "preferred" so I assume there is
> some history that I am not familiar with.

My main goal with commit messages, code comments and every other *text*
you have in the code is to be as succinct and understandable as possible
for time considerations, clarity, etc.

If I see a 12-char sha1 followed by a title, to me that is a commit. No
need to say "commit" too. But as already said, if you prefer to have
"commit" there, I'll add it - no biggie.

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH] x86/resctrl: Fix unneeded variable warning reported by kernel test robot

[Reinette Chatre]

Hi Boris,

On 1/24/2024 2:46 PM, Borislav Petkov wrote:
> On Wed, Jan 24, 2024 at 01:31:01PM -0800, Reinette Chatre wrote:
>> I do not know the motivation for that requirement. From what I can tell the
>> change [1] that added that check went in as first version without discussion.
>> [1] starts by saying that the format is "preferred" so I assume there is
>> some history that I am not familiar with.
> 
> My main goal with commit messages, code comments and every other *text*
> you have in the code is to be as succinct and understandable as possible
> for time considerations, clarity, etc.
> 
> If I see a 12-char sha1 followed by a title, to me that is a commit. No
> need to say "commit" too. But as already said, if you prefer to have
> "commit" there, I'll add it - no biggie.

I totally understand your sentiment. I am just the messenger here.

Without the "commit" this patch triggers a loud:

ERROR: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 92bd5a139033 ("x86/resctrl: Add interface to write mbm_total_bytes_config")'
#15:
  92bd5a139033 ("x86/resctrl: Add interface to write mbm_total_bytes_config")

I usually (unless, for example, following checkpatch.pl advice causes a change
to fall out of place with surrounding code) try to format my patches to get a
clean slate from checkpatch.pl with the goal to eliminate obstacles to the
patch getting included.
Since you are the one that decides the rules for inclusion you can make this
check to be one where checkpatch.pl can be ignored. No objection from me if
you choose to do so (and I will note the precedent for future patches).

Reinette

Re: [PATCH] x86/resctrl: Fix unneeded variable warning reported by kernel test robot

[Borislav Petkov]

On Wed, Jan 24, 2024 at 03:03:15PM -0800, Reinette Chatre wrote:
> Since you are the one that decides the rules for inclusion you can make this
> check to be one where checkpatch.pl can be ignored. No objection from me if
> you choose to do so (and I will note the precedent for future patches).

Nah, that's not nearly as important for you to change your workflow.

What I'd suggest, though, is to sanity-check what checkpatch suggests and
ask yourself whether it always makes sense.

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH] x86/resctrl: Fix unneeded variable warning reported by kernel test robot

[Borislav Petkov]

On Thu, Jan 25, 2024 at 12:34:24AM +0100, Borislav Petkov wrote:
> On Wed, Jan 24, 2024 at 03:03:15PM -0800, Reinette Chatre wrote:
> > Since you are the one that decides the rules for inclusion you can make this
> > check to be one where checkpatch.pl can be ignored. No objection from me if
> > you choose to do so (and I will note the precedent for future patches).
> 
> Nah, that's not nearly as important for you to change your workflow.
> 
> What I'd suggest, though, is to sanity-check what checkpatch suggests and
> ask yourself whether it always makes sense.

Dammit, there's a reason I don't use this abomination:

$ cat /tmp/0001-x86-resctrl-Remove-redundant-variable-in-mbm_config_.patch | ./scripts/checkpatch.pl

WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#9: 
      commit 54e35eb8611c ("x86/resctrl: Read supported bandwidth sources from CPUID").


WTF?!

I have a line underneath which is even one char longer:

"  commit 92bd5a139033 ("x86/resctrl: Add interface to write mbm_total_bytes_config")"

but it doesn't complain about it.

What a bunch of crap.

I'm writing it as a maximally readable commit message and that's it.
Human-readable beats any script, any day of the week.

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH] x86/resctrl: Fix unneeded variable warning reported by kernel test robot

[Reinette Chatre]

Hi Boris,

On 1/24/2024 3:34 PM, Borislav Petkov wrote:
> On Wed, Jan 24, 2024 at 03:03:15PM -0800, Reinette Chatre wrote:
>> Since you are the one that decides the rules for inclusion you can make this
>> check to be one where checkpatch.pl can be ignored. No objection from me if
>> you choose to do so (and I will note the precedent for future patches).
> 
> Nah, that's not nearly as important for you to change your workflow.
> 
> What I'd suggest, though, is to sanity-check what checkpatch suggests and
> ask yourself whether it always makes sense.

Will do. Thank you for considering and discussing this detail. The final
patch you just merged looks good to me. Thank you very much for fixing this.

Reinette

[tip: x86/cache] x86/resctrl: Remove redundant variable in mbm_config_write_domain()

[tip-bot2 for Babu Moger]

The following commit has been merged into the x86/cache branch of tip:

Commit-ID:     fc747eebef734563cf68a512f57937c8f231834a
Gitweb:        https://git.kernel.org/tip/fc747eebef734563cf68a512f57937c8f231834a
Author:        Babu Moger <babu.moger@amd.com>
AuthorDate:    Wed, 24 Jan 2024 11:52:56 -06:00
Committer:     Borislav Petkov (AMD) <bp@alien8.de>
CommitterDate: Thu, 25 Jan 2024 00:41:59 +01:00

x86/resctrl: Remove redundant variable in mbm_config_write_domain()

The kernel test robot reported the following warning after commit

  54e35eb8611c ("x86/resctrl: Read supported bandwidth sources from CPUID").

even though the issue is present even in the original commit

  92bd5a139033 ("x86/resctrl: Add interface to write mbm_total_bytes_config")

which added this function. The reported warning is:

  $ make C=1 CHECK=scripts/coccicheck arch/x86/kernel/cpu/resctrl/rdtgroup.o
  ...
  arch/x86/kernel/cpu/resctrl/rdtgroup.c:1621:5-8: Unneeded variable: "ret". Return "0" on line 1655

Remove the local variable 'ret'.

  [ bp: Massage commit message, make mbm_config_write_domain() void. ]

Fixes: 92bd5a139033 ("x86/resctrl: Add interface to write mbm_total_bytes_config")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202401241810.jbd8Ipa1-lkp@intel.com/
Signed-off-by: Babu Moger <babu.moger@amd.com>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Acked-by: Reinette Chatre <reinette.chatre@intel.com>
Link: https://lore.kernel.org/r/202401241810.jbd8Ipa1-lkp@intel.com
---
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 2b69e56..aa24343 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1614,11 +1614,10 @@ static void mon_event_config_write(void *info)
 	wrmsr(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config, 0);
 }
 
-static int mbm_config_write_domain(struct rdt_resource *r,
-				   struct rdt_domain *d, u32 evtid, u32 val)
+static void mbm_config_write_domain(struct rdt_resource *r,
+				    struct rdt_domain *d, u32 evtid, u32 val)
 {
 	struct mon_config_info mon_info = {0};
-	int ret = 0;
 
 	/*
 	 * Read the current config value first. If both are the same then
@@ -1627,7 +1626,7 @@ static int mbm_config_write_domain(struct rdt_resource *r,
 	mon_info.evtid = evtid;
 	mondata_config_read(d, &mon_info);
 	if (mon_info.mon_config == val)
-		goto out;
+		return;
 
 	mon_info.mon_config = val;
 
@@ -1650,9 +1649,6 @@ static int mbm_config_write_domain(struct rdt_resource *r,
 	 * mbm_local and mbm_total counts for all the RMIDs.
 	 */
 	resctrl_arch_reset_rmid_all(r, d);
-
-out:
-	return ret;
 }
 
 static int mon_config_write(struct rdt_resource *r, char *tok, u32 evtid)
@@ -1661,7 +1657,6 @@ static int mon_config_write(struct rdt_resource *r, char *tok, u32 evtid)
 	char *dom_str = NULL, *id_str;
 	unsigned long dom_id, val;
 	struct rdt_domain *d;
-	int ret = 0;
 
 next:
 	if (!tok || tok[0] == '\0')
@@ -1690,9 +1685,7 @@ next:
 
 	list_for_each_entry(d, &r->domains, list) {
 		if (d->id == dom_id) {
-			ret = mbm_config_write_domain(r, d, evtid, val);
-			if (ret)
-				return -EINVAL;
+			mbm_config_write_domain(r, d, evtid, val);
 			goto next;
 		}
 	}