* [patch] cpu: remove some dead code in store_cache_disable() @ 2012-04-19 7:00 Dan Carpenter 2012-04-19 8:18 ` Srivatsa S. Bhat 2012-04-25 13:50 ` [tip:x86/urgent] x86, intel_cacheinfo: Fix error return code in amd_set_l3_disable_slot() tip-bot for Srivatsa S. Bhat 0 siblings, 2 replies; 10+ messages in thread From: Dan Carpenter @ 2012-04-19 7:00 UTC (permalink / raw) To: Thomas Gleixner Cc: Ingo Molnar, H. Peter Anvin, x86, Frank Arnold, Borislav Petkov, linux-kernel, kernel-janitors amd_set_l3_disable_slot() never returns -EEXIST, it only returns -EINVAL or zero. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> diff --git a/arch/x86/kernel/cpu/intel_cacheinfo.c b/arch/x86/kernel/cpu/intel_cacheinfo.c index 73d08ed..7b4e294 100644 --- a/arch/x86/kernel/cpu/intel_cacheinfo.c +++ b/arch/x86/kernel/cpu/intel_cacheinfo.c @@ -466,12 +466,9 @@ static ssize_t store_cache_disable(struct _cpuid4_info *this_leaf, return -EINVAL; err = amd_set_l3_disable_slot(this_leaf->base.nb, cpu, slot, val); - if (err) { - if (err == -EEXIST) - printk(KERN_WARNING "L3 disable slot %d in use!\n", - slot); + if (err) return err; - } + return count; } ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [patch] cpu: remove some dead code in store_cache_disable() 2012-04-19 7:00 [patch] cpu: remove some dead code in store_cache_disable() Dan Carpenter @ 2012-04-19 8:18 ` Srivatsa S. Bhat 2012-04-19 11:02 ` Borislav Petkov 2012-04-25 13:50 ` [tip:x86/urgent] x86, intel_cacheinfo: Fix error return code in amd_set_l3_disable_slot() tip-bot for Srivatsa S. Bhat 1 sibling, 1 reply; 10+ messages in thread From: Srivatsa S. Bhat @ 2012-04-19 8:18 UTC (permalink / raw) To: Dan Carpenter Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Frank Arnold, Borislav Petkov, linux-kernel, kernel-janitors On 04/19/2012 12:30 PM, Dan Carpenter wrote: > amd_set_l3_disable_slot() never returns -EEXIST, it only returns -EINVAL > or zero. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > diff --git a/arch/x86/kernel/cpu/intel_cacheinfo.c b/arch/x86/kernel/cpu/intel_cacheinfo.c > index 73d08ed..7b4e294 100644 > --- a/arch/x86/kernel/cpu/intel_cacheinfo.c > +++ b/arch/x86/kernel/cpu/intel_cacheinfo.c > @@ -466,12 +466,9 @@ static ssize_t store_cache_disable(struct _cpuid4_info *this_leaf, > return -EINVAL; > > err = amd_set_l3_disable_slot(this_leaf->base.nb, cpu, slot, val); > - if (err) { > - if (err == -EEXIST) > - printk(KERN_WARNING "L3 disable slot %d in use!\n", > - slot); > + if (err) > return err; > - } > + > return count; > } > Looking at the comments around the code and the print statement your patch is trying to remove, I wonder if it would be more appropriate to return -EEXIST in amd_set_l3_disable_slot(), like this: --- From: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> Subject: [PATCH] cpu: Fix error return code in amd_set_l3_disable_slot() If the L3 disable slot is already in use, return -EEXIST instead of -EINVAL. The caller, store_cache_disable(), checks this return value to print an appropriate warning. Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> --- arch/x86/kernel/cpu/intel_cacheinfo.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/x86/kernel/cpu/intel_cacheinfo.c b/arch/x86/kernel/cpu/intel_cacheinfo.c index 73d08ed..0b49e29 100644 --- a/arch/x86/kernel/cpu/intel_cacheinfo.c +++ b/arch/x86/kernel/cpu/intel_cacheinfo.c @@ -433,7 +433,7 @@ int amd_set_l3_disable_slot(struct amd_northbridge *nb, int cpu, unsigned slot, /* check if @slot is already used or the index is already disabled */ ret = amd_get_l3_disable_slot(nb, slot); if (ret >= 0) - return -EINVAL; + return -EEXIST; if (index > nb->l3_cache.indices) return -EINVAL; Regards, Srivatsa S. Bhat ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [patch] cpu: remove some dead code in store_cache_disable() 2012-04-19 8:18 ` Srivatsa S. Bhat @ 2012-04-19 11:02 ` Borislav Petkov 2012-04-19 11:14 ` Srivatsa S. Bhat 2012-04-19 11:44 ` Dan Carpenter 0 siblings, 2 replies; 10+ messages in thread From: Borislav Petkov @ 2012-04-19 11:02 UTC (permalink / raw) To: Srivatsa S. Bhat Cc: Dan Carpenter, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Frank Arnold, linux-kernel, kernel-janitors On Thu, Apr 19, 2012 at 01:48:02PM +0530, Srivatsa S. Bhat wrote: > On 04/19/2012 12:30 PM, Dan Carpenter wrote: > > > amd_set_l3_disable_slot() never returns -EEXIST, it only returns -EINVAL > > or zero. > > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > diff --git a/arch/x86/kernel/cpu/intel_cacheinfo.c b/arch/x86/kernel/cpu/intel_cacheinfo.c > > index 73d08ed..7b4e294 100644 > > --- a/arch/x86/kernel/cpu/intel_cacheinfo.c > > +++ b/arch/x86/kernel/cpu/intel_cacheinfo.c > > @@ -466,12 +466,9 @@ static ssize_t store_cache_disable(struct _cpuid4_info *this_leaf, > > return -EINVAL; > > > > err = amd_set_l3_disable_slot(this_leaf->base.nb, cpu, slot, val); > > - if (err) { > > - if (err == -EEXIST) > > - printk(KERN_WARNING "L3 disable slot %d in use!\n", > > - slot); > > + if (err) > > return err; > > - } > > + > > return count; > > } > > > > > Looking at the comments around the code and the print statement your patch > is trying to remove, I wonder if it would be more appropriate to return > -EEXIST in amd_set_l3_disable_slot(), like this: > > --- > From: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> > Subject: [PATCH] cpu: Fix error return code in amd_set_l3_disable_slot() > > If the L3 disable slot is already in use, return -EEXIST instead of -EINVAL. > The caller, store_cache_disable(), checks this return value to print an > appropriate warning. > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> > --- > > arch/x86/kernel/cpu/intel_cacheinfo.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/arch/x86/kernel/cpu/intel_cacheinfo.c b/arch/x86/kernel/cpu/intel_cacheinfo.c > index 73d08ed..0b49e29 100644 > --- a/arch/x86/kernel/cpu/intel_cacheinfo.c > +++ b/arch/x86/kernel/cpu/intel_cacheinfo.c > @@ -433,7 +433,7 @@ int amd_set_l3_disable_slot(struct amd_northbridge *nb, int cpu, unsigned slot, > /* check if @slot is already used or the index is already disabled */ > ret = amd_get_l3_disable_slot(nb, slot); > if (ret >= 0) > - return -EINVAL; > + return -EEXIST; > > if (index > nb->l3_cache.indices) > return -EINVAL; Well, let's see, there's 8cc1176e5de534d55cb26ff0cef3fd0d6ad8c3c0 which was intending at looking at -EEXIST when it gets returned but forgot to return it at the end. Crap. Somebody should b*tchslap its author... oh, that's me. Good catch guys, thanks. I'll take a bit enhanced version of Srivatsa's patch because it goes in the way of what was originally intended. The enhanced version returns -EEXIST for the other slot too when we're disabling the same index, see below: -- From: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com> Date: Thu, 19 Apr 2012 12:35:08 +0200 Subject: [PATCH] x86, intel_cacheinfo: Fix error return code in amd_set_l3_disable_slot() If the L3 disable slot is already in use, return -EEXIST instead of -EINVAL. The caller, store_cache_disable(), checks this return value to print an appropriate warning. Also, we want to signal with -EEXIST that the current index we're disabling has actually been already disabled on the node: $ echo 12 > /sys/devices/system/cpu/cpu3/cache/index3/cache_disable_0 $ echo 12 > /sys/devices/system/cpu/cpu3/cache/index3/cache_disable_0 -bash: echo: write error: File exists $ echo 12 > /sys/devices/system/cpu/cpu3/cache/index3/cache_disable_1 -bash: echo: write error: File exists $ echo 12 > /sys/devices/system/cpu/cpu5/cache/index3/cache_disable_1 -bash: echo: write error: File exists The old code would say -bash: echo: write error: Invalid argument for disable slot 1 when playing the example above with no output in dmesg, which is clearly misleading. Cc: <stable@vger.kernel.org> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> Link: http://lkml.kernel.org/r/20120419070053.GB16645@elgon.mountain [Boris: add testing for the other index too] Signed-off-by: Borislav Petkov <borislav.petkov@amd.com> --- arch/x86/kernel/cpu/intel_cacheinfo.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/x86/kernel/cpu/intel_cacheinfo.c b/arch/x86/kernel/cpu/intel_cacheinfo.c index 73d08ed98a64..b8f3653dddbc 100644 --- a/arch/x86/kernel/cpu/intel_cacheinfo.c +++ b/arch/x86/kernel/cpu/intel_cacheinfo.c @@ -433,14 +433,14 @@ int amd_set_l3_disable_slot(struct amd_northbridge *nb, int cpu, unsigned slot, /* check if @slot is already used or the index is already disabled */ ret = amd_get_l3_disable_slot(nb, slot); if (ret >= 0) - return -EINVAL; + return -EEXIST; if (index > nb->l3_cache.indices) return -EINVAL; /* check whether the other slot has disabled the same index already */ if (index == amd_get_l3_disable_slot(nb, !slot)) - return -EINVAL; + return -EEXIST; amd_l3_disable_index(nb, cpu, slot, index); @@ -468,8 +468,8 @@ static ssize_t store_cache_disable(struct _cpuid4_info *this_leaf, err = amd_set_l3_disable_slot(this_leaf->base.nb, cpu, slot, val); if (err) { if (err == -EEXIST) - printk(KERN_WARNING "L3 disable slot %d in use!\n", - slot); + pr_warning("L3 slot %d in use/index already disabled!\n", + slot); return err; } return count; -- 1.7.9.3.362.g71319 -- Regards/Gruss, Boris. Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach GM: Alberto Bozzo Reg: Dornach, Landkreis Muenchen HRB Nr. 43632 WEEE Registernr: 129 19551 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [patch] cpu: remove some dead code in store_cache_disable() 2012-04-19 11:02 ` Borislav Petkov @ 2012-04-19 11:14 ` Srivatsa S. Bhat 2012-04-19 11:44 ` Dan Carpenter 1 sibling, 0 replies; 10+ messages in thread From: Srivatsa S. Bhat @ 2012-04-19 11:14 UTC (permalink / raw) To: Borislav Petkov Cc: Dan Carpenter, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Frank Arnold, linux-kernel, kernel-janitors On 04/19/2012 04:32 PM, Borislav Petkov wrote: > On Thu, Apr 19, 2012 at 01:48:02PM +0530, Srivatsa S. Bhat wrote: >> On 04/19/2012 12:30 PM, Dan Carpenter wrote: >> >>> amd_set_l3_disable_slot() never returns -EEXIST, it only returns -EINVAL >>> or zero. >>> >>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> >>> >>> diff --git a/arch/x86/kernel/cpu/intel_cacheinfo.c b/arch/x86/kernel/cpu/intel_cacheinfo.c >>> index 73d08ed..7b4e294 100644 >>> --- a/arch/x86/kernel/cpu/intel_cacheinfo.c >>> +++ b/arch/x86/kernel/cpu/intel_cacheinfo.c >>> @@ -466,12 +466,9 @@ static ssize_t store_cache_disable(struct _cpuid4_info *this_leaf, >>> return -EINVAL; >>> >>> err = amd_set_l3_disable_slot(this_leaf->base.nb, cpu, slot, val); >>> - if (err) { >>> - if (err == -EEXIST) >>> - printk(KERN_WARNING "L3 disable slot %d in use!\n", >>> - slot); >>> + if (err) >>> return err; >>> - } >>> + >>> return count; >>> } >>> >> >> >> Looking at the comments around the code and the print statement your patch >> is trying to remove, I wonder if it would be more appropriate to return >> -EEXIST in amd_set_l3_disable_slot(), like this: >> >> --- >> From: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> >> Subject: [PATCH] cpu: Fix error return code in amd_set_l3_disable_slot() >> >> If the L3 disable slot is already in use, return -EEXIST instead of -EINVAL. >> The caller, store_cache_disable(), checks this return value to print an >> appropriate warning. >> >> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> >> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> >> --- >> >> arch/x86/kernel/cpu/intel_cacheinfo.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/arch/x86/kernel/cpu/intel_cacheinfo.c b/arch/x86/kernel/cpu/intel_cacheinfo.c >> index 73d08ed..0b49e29 100644 >> --- a/arch/x86/kernel/cpu/intel_cacheinfo.c >> +++ b/arch/x86/kernel/cpu/intel_cacheinfo.c >> @@ -433,7 +433,7 @@ int amd_set_l3_disable_slot(struct amd_northbridge *nb, int cpu, unsigned slot, >> /* check if @slot is already used or the index is already disabled */ >> ret = amd_get_l3_disable_slot(nb, slot); >> if (ret >= 0) >> - return -EINVAL; >> + return -EEXIST; >> >> if (index > nb->l3_cache.indices) >> return -EINVAL; > > Well, let's see, there's 8cc1176e5de534d55cb26ff0cef3fd0d6ad8c3c0 which > was intending at looking at -EEXIST when it gets returned but forgot to > return it at the end. Crap. Somebody should b*tchslap its author... oh, > that's me. > > Good catch guys, thanks. I'll take a bit enhanced version of Srivatsa's > patch because it goes in the way of what was originally intended. The > enhanced version returns -EEXIST for the other slot too when we're > disabling the same index, see below: > > -- > From: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com> > Date: Thu, 19 Apr 2012 12:35:08 +0200 > Subject: [PATCH] x86, intel_cacheinfo: Fix error return code in amd_set_l3_disable_slot() > > If the L3 disable slot is already in use, return -EEXIST instead of > -EINVAL. The caller, store_cache_disable(), checks this return value to > print an appropriate warning. > > Also, we want to signal with -EEXIST that the current index we're > disabling has actually been already disabled on the node: > > $ echo 12 > /sys/devices/system/cpu/cpu3/cache/index3/cache_disable_0 > $ echo 12 > /sys/devices/system/cpu/cpu3/cache/index3/cache_disable_0 > -bash: echo: write error: File exists > $ echo 12 > /sys/devices/system/cpu/cpu3/cache/index3/cache_disable_1 > -bash: echo: write error: File exists > $ echo 12 > /sys/devices/system/cpu/cpu5/cache/index3/cache_disable_1 > -bash: echo: write error: File exists > > The old code would say > > -bash: echo: write error: Invalid argument > > for disable slot 1 when playing the example above with no output in > dmesg, which is clearly misleading. > > Cc: <stable@vger.kernel.org> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> > Link: http://lkml.kernel.org/r/20120419070053.GB16645@elgon.mountain > [Boris: add testing for the other index too] > Signed-off-by: Borislav Petkov <borislav.petkov@amd.com> > --- > arch/x86/kernel/cpu/intel_cacheinfo.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kernel/cpu/intel_cacheinfo.c b/arch/x86/kernel/cpu/intel_cacheinfo.c > index 73d08ed98a64..b8f3653dddbc 100644 > --- a/arch/x86/kernel/cpu/intel_cacheinfo.c > +++ b/arch/x86/kernel/cpu/intel_cacheinfo.c > @@ -433,14 +433,14 @@ int amd_set_l3_disable_slot(struct amd_northbridge *nb, int cpu, unsigned slot, > /* check if @slot is already used or the index is already disabled */ > ret = amd_get_l3_disable_slot(nb, slot); > if (ret >= 0) > - return -EINVAL; > + return -EEXIST; > > if (index > nb->l3_cache.indices) > return -EINVAL; > > /* check whether the other slot has disabled the same index already */ > if (index == amd_get_l3_disable_slot(nb, !slot)) > - return -EINVAL; > + return -EEXIST; > > amd_l3_disable_index(nb, cpu, slot, index); > > @@ -468,8 +468,8 @@ static ssize_t store_cache_disable(struct _cpuid4_info *this_leaf, > err = amd_set_l3_disable_slot(this_leaf->base.nb, cpu, slot, val); > if (err) { > if (err == -EEXIST) > - printk(KERN_WARNING "L3 disable slot %d in use!\n", > - slot); > + pr_warning("L3 slot %d in use/index already disabled!\n", > + slot); > return err; > } > return count; Wow, that's even better! Thanks :-) Regards, Srivatsa S. Bhat ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [patch] cpu: remove some dead code in store_cache_disable() 2012-04-19 11:02 ` Borislav Petkov 2012-04-19 11:14 ` Srivatsa S. Bhat @ 2012-04-19 11:44 ` Dan Carpenter 2012-04-19 16:53 ` [GIT PULL] L3 CID fix for 3.5 Borislav Petkov 1 sibling, 1 reply; 10+ messages in thread From: Dan Carpenter @ 2012-04-19 11:44 UTC (permalink / raw) To: Borislav Petkov Cc: Srivatsa S. Bhat, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, Frank Arnold, linux-kernel, kernel-janitors Great. Thanks. regards, dan carpenter ^ permalink raw reply [flat|nested] 10+ messages in thread
* [GIT PULL] L3 CID fix for 3.5 2012-04-19 11:44 ` Dan Carpenter @ 2012-04-19 16:53 ` Borislav Petkov 2012-04-25 9:30 ` Borislav Petkov 0 siblings, 1 reply; 10+ messages in thread From: Borislav Petkov @ 2012-04-19 16:53 UTC (permalink / raw) To: Ingo Molnar, H. Peter Anvin Cc: Dan Carpenter, Borislav Petkov, Srivatsa S. Bhat, Thomas Gleixner, x86, Frank Arnold, linux-kernel, kernel-janitors Hi guys, please pull the following fix for L3 cache index disable into tip. Since there's no real breakage but simply a correctness issue, it can wait for 3.5. I've also removed the stable tag I had in the previous version. Thanks. -- The following changes since commit e816b57a337ea3b755de72bec38c10c864f23015: Linux 3.4-rc3 (2012-04-15 18:28:29 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git tags/l3-fix-for-3.5 for you to fetch changes up to a720b2dd2470a52345df11dca8d6c1466599f812: x86, intel_cacheinfo: Fix error return code in amd_set_l3_disable_slot() (2012-04-19 18:30:28 +0200) ---------------------------------------------------------------- A small L3 cache index disable fix from Srivatsa Bhat which unifies the way the code checks for already disabled indices. ---------------------------------------------------------------- Srivatsa S. Bhat (1): x86, intel_cacheinfo: Fix error return code in amd_set_l3_disable_slot() arch/x86/kernel/cpu/intel_cacheinfo.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) Appending patch here too: -- commit a720b2dd2470a52345df11dca8d6c1466599f812 Author: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> Date: Thu Apr 19 12:35:08 2012 +0200 x86, intel_cacheinfo: Fix error return code in amd_set_l3_disable_slot() If the L3 disable slot is already in use, return -EEXIST instead of -EINVAL. The caller, store_cache_disable(), checks this return value to print an appropriate warning. Also, we want to signal with -EEXIST that the current index we're disabling has actually been already disabled on the node: $ echo 12 > /sys/devices/system/cpu/cpu3/cache/index3/cache_disable_0 $ echo 12 > /sys/devices/system/cpu/cpu3/cache/index3/cache_disable_0 -bash: echo: write error: File exists $ echo 12 > /sys/devices/system/cpu/cpu3/cache/index3/cache_disable_1 -bash: echo: write error: File exists $ echo 12 > /sys/devices/system/cpu/cpu5/cache/index3/cache_disable_1 -bash: echo: write error: File exists The old code would say -bash: echo: write error: Invalid argument for disable slot 1 when playing the example above with no output in dmesg, which is clearly misleading. Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> Link: http://lkml.kernel.org/r/20120419070053.GB16645@elgon.mountain [Boris: add testing for the other index too] Signed-off-by: Borislav Petkov <borislav.petkov@amd.com> diff --git a/arch/x86/kernel/cpu/intel_cacheinfo.c b/arch/x86/kernel/cpu/intel_cacheinfo.c index 73d08ed98a64..b8f3653dddbc 100644 --- a/arch/x86/kernel/cpu/intel_cacheinfo.c +++ b/arch/x86/kernel/cpu/intel_cacheinfo.c @@ -433,14 +433,14 @@ int amd_set_l3_disable_slot(struct amd_northbridge *nb, int cpu, unsigned slot, /* check if @slot is already used or the index is already disabled */ ret = amd_get_l3_disable_slot(nb, slot); if (ret >= 0) - return -EINVAL; + return -EEXIST; if (index > nb->l3_cache.indices) return -EINVAL; /* check whether the other slot has disabled the same index already */ if (index == amd_get_l3_disable_slot(nb, !slot)) - return -EINVAL; + return -EEXIST; amd_l3_disable_index(nb, cpu, slot, index); @@ -468,8 +468,8 @@ static ssize_t store_cache_disable(struct _cpuid4_info *this_leaf, err = amd_set_l3_disable_slot(this_leaf->base.nb, cpu, slot, val); if (err) { if (err == -EEXIST) - printk(KERN_WARNING "L3 disable slot %d in use!\n", - slot); + pr_warning("L3 slot %d in use/index already disabled!\n", + slot); return err; } return count; -- Regards/Gruss, Boris. Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach GM: Alberto Bozzo Reg: Dornach, Landkreis Muenchen HRB Nr. 43632 WEEE Registernr: 129 19551 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [GIT PULL] L3 CID fix for 3.5 2012-04-19 16:53 ` [GIT PULL] L3 CID fix for 3.5 Borislav Petkov @ 2012-04-25 9:30 ` Borislav Petkov 2012-04-25 10:23 ` Ingo Molnar 0 siblings, 1 reply; 10+ messages in thread From: Borislav Petkov @ 2012-04-25 9:30 UTC (permalink / raw) To: Ingo Molnar, H. Peter Anvin Cc: x86, kernel-janitors, linux-kernel, Srivatsa S. Bhat, Frank Arnold, Thomas Gleixner, Dan Carpenter Ping? Guys, any issues with this that I should know of? Thanks. On Thu, Apr 19, 2012 at 06:53:53PM +0200, Borislav Petkov wrote: > Hi guys, > > please pull the following fix for L3 cache index disable into tip. Since > there's no real breakage but simply a correctness issue, it can wait for > 3.5. I've also removed the stable tag I had in the previous version. > > Thanks. > > -- > The following changes since commit e816b57a337ea3b755de72bec38c10c864f23015: > > Linux 3.4-rc3 (2012-04-15 18:28:29 -0700) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git tags/l3-fix-for-3.5 > > for you to fetch changes up to a720b2dd2470a52345df11dca8d6c1466599f812: > > x86, intel_cacheinfo: Fix error return code in amd_set_l3_disable_slot() (2012-04-19 18:30:28 +0200) > > ---------------------------------------------------------------- > A small L3 cache index disable fix from Srivatsa Bhat which unifies the > way the code checks for already disabled indices. > > ---------------------------------------------------------------- > Srivatsa S. Bhat (1): > x86, intel_cacheinfo: Fix error return code in amd_set_l3_disable_slot() > > arch/x86/kernel/cpu/intel_cacheinfo.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > Appending patch here too: > > -- > commit a720b2dd2470a52345df11dca8d6c1466599f812 > Author: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> > Date: Thu Apr 19 12:35:08 2012 +0200 > > x86, intel_cacheinfo: Fix error return code in amd_set_l3_disable_slot() > > If the L3 disable slot is already in use, return -EEXIST instead of > -EINVAL. The caller, store_cache_disable(), checks this return value to > print an appropriate warning. > > Also, we want to signal with -EEXIST that the current index we're > disabling has actually been already disabled on the node: > > $ echo 12 > /sys/devices/system/cpu/cpu3/cache/index3/cache_disable_0 > $ echo 12 > /sys/devices/system/cpu/cpu3/cache/index3/cache_disable_0 > -bash: echo: write error: File exists > $ echo 12 > /sys/devices/system/cpu/cpu3/cache/index3/cache_disable_1 > -bash: echo: write error: File exists > $ echo 12 > /sys/devices/system/cpu/cpu5/cache/index3/cache_disable_1 > -bash: echo: write error: File exists > > The old code would say > > -bash: echo: write error: Invalid argument > > for disable slot 1 when playing the example above with no output in > dmesg, which is clearly misleading. > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> > Link: http://lkml.kernel.org/r/20120419070053.GB16645@elgon.mountain > [Boris: add testing for the other index too] > Signed-off-by: Borislav Petkov <borislav.petkov@amd.com> > > diff --git a/arch/x86/kernel/cpu/intel_cacheinfo.c b/arch/x86/kernel/cpu/intel_cacheinfo.c > index 73d08ed98a64..b8f3653dddbc 100644 > --- a/arch/x86/kernel/cpu/intel_cacheinfo.c > +++ b/arch/x86/kernel/cpu/intel_cacheinfo.c > @@ -433,14 +433,14 @@ int amd_set_l3_disable_slot(struct amd_northbridge *nb, int cpu, unsigned slot, > /* check if @slot is already used or the index is already disabled */ > ret = amd_get_l3_disable_slot(nb, slot); > if (ret >= 0) > - return -EINVAL; > + return -EEXIST; > > if (index > nb->l3_cache.indices) > return -EINVAL; > > /* check whether the other slot has disabled the same index already */ > if (index == amd_get_l3_disable_slot(nb, !slot)) > - return -EINVAL; > + return -EEXIST; > > amd_l3_disable_index(nb, cpu, slot, index); > > @@ -468,8 +468,8 @@ static ssize_t store_cache_disable(struct _cpuid4_info *this_leaf, > err = amd_set_l3_disable_slot(this_leaf->base.nb, cpu, slot, val); > if (err) { > if (err == -EEXIST) > - printk(KERN_WARNING "L3 disable slot %d in use!\n", > - slot); > + pr_warning("L3 slot %d in use/index already disabled!\n", > + slot); > return err; > } > return count; > > -- > Regards/Gruss, > Boris. > > Advanced Micro Devices GmbH > Einsteinring 24, 85609 Dornach > GM: Alberto Bozzo > Reg: Dornach, Landkreis Muenchen > HRB Nr. 43632 WEEE Registernr: 129 19551 > _______________________________________________ > osrc-patches mailing list > osrc-patches@elbe.amd.com > https://elbe.amd.com/mailman/listinfo/osrc-patches -- Regards/Gruss, Boris. Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach GM: Alberto Bozzo Reg: Dornach, Landkreis Muenchen HRB Nr. 43632 WEEE Registernr: 129 19551 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GIT PULL] L3 CID fix for 3.5 2012-04-25 9:30 ` Borislav Petkov @ 2012-04-25 10:23 ` Ingo Molnar 2012-04-25 10:35 ` Borislav Petkov 0 siblings, 1 reply; 10+ messages in thread From: Ingo Molnar @ 2012-04-25 10:23 UTC (permalink / raw) To: Borislav Petkov Cc: Ingo Molnar, H. Peter Anvin, x86, kernel-janitors, linux-kernel, Srivatsa S. Bhat, Frank Arnold, Thomas Gleixner, Dan Carpenter * Borislav Petkov <borislav.petkov@amd.com> wrote: > Ping? > > Guys, any issues with this that I should know of? Looks good to me. You marked it v3.5, should I pull it into v3.4? Looks small enough to me and we better keep such user facing interfaces correct, even if there's no direct regression. Thanks, Ingo ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GIT PULL] L3 CID fix for 3.5 2012-04-25 10:23 ` Ingo Molnar @ 2012-04-25 10:35 ` Borislav Petkov 0 siblings, 0 replies; 10+ messages in thread From: Borislav Petkov @ 2012-04-25 10:35 UTC (permalink / raw) To: Ingo Molnar Cc: Ingo Molnar, H. Peter Anvin, x86, kernel-janitors, linux-kernel, Srivatsa S. Bhat, Frank Arnold, Thomas Gleixner, Dan Carpenter On Wed, Apr 25, 2012 at 12:23:58PM +0200, Ingo Molnar wrote: > Looks good to me. You marked it v3.5, should I pull it into v3.4? > Looks small enough to me and we better keep such user facing > interfaces correct, even if there's no direct regression. I'm fine with whatever you decide, thanks. -- Regards/Gruss, Boris. Advanced Micro Devices GmbH Einsteinring 24, 85609 Dornach GM: Alberto Bozzo Reg: Dornach, Landkreis Muenchen HRB Nr. 43632 WEEE Registernr: 129 19551 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [tip:x86/urgent] x86, intel_cacheinfo: Fix error return code in amd_set_l3_disable_slot() 2012-04-19 7:00 [patch] cpu: remove some dead code in store_cache_disable() Dan Carpenter 2012-04-19 8:18 ` Srivatsa S. Bhat @ 2012-04-25 13:50 ` tip-bot for Srivatsa S. Bhat 1 sibling, 0 replies; 10+ messages in thread From: tip-bot for Srivatsa S. Bhat @ 2012-04-25 13:50 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, hpa, mingo, srivatsa.bhat, tglx, borislav.petkov, dan.carpenter Commit-ID: a720b2dd2470a52345df11dca8d6c1466599f812 Gitweb: http://git.kernel.org/tip/a720b2dd2470a52345df11dca8d6c1466599f812 Author: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> AuthorDate: Thu, 19 Apr 2012 12:35:08 +0200 Committer: Borislav Petkov <borislav.petkov@amd.com> CommitDate: Thu, 19 Apr 2012 18:30:28 +0200 x86, intel_cacheinfo: Fix error return code in amd_set_l3_disable_slot() If the L3 disable slot is already in use, return -EEXIST instead of -EINVAL. The caller, store_cache_disable(), checks this return value to print an appropriate warning. Also, we want to signal with -EEXIST that the current index we're disabling has actually been already disabled on the node: $ echo 12 > /sys/devices/system/cpu/cpu3/cache/index3/cache_disable_0 $ echo 12 > /sys/devices/system/cpu/cpu3/cache/index3/cache_disable_0 -bash: echo: write error: File exists $ echo 12 > /sys/devices/system/cpu/cpu3/cache/index3/cache_disable_1 -bash: echo: write error: File exists $ echo 12 > /sys/devices/system/cpu/cpu5/cache/index3/cache_disable_1 -bash: echo: write error: File exists The old code would say -bash: echo: write error: Invalid argument for disable slot 1 when playing the example above with no output in dmesg, which is clearly misleading. Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> Link: http://lkml.kernel.org/r/20120419070053.GB16645@elgon.mountain [Boris: add testing for the other index too] Signed-off-by: Borislav Petkov <borislav.petkov@amd.com> --- arch/x86/kernel/cpu/intel_cacheinfo.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/x86/kernel/cpu/intel_cacheinfo.c b/arch/x86/kernel/cpu/intel_cacheinfo.c index 73d08ed..b8f3653 100644 --- a/arch/x86/kernel/cpu/intel_cacheinfo.c +++ b/arch/x86/kernel/cpu/intel_cacheinfo.c @@ -433,14 +433,14 @@ int amd_set_l3_disable_slot(struct amd_northbridge *nb, int cpu, unsigned slot, /* check if @slot is already used or the index is already disabled */ ret = amd_get_l3_disable_slot(nb, slot); if (ret >= 0) - return -EINVAL; + return -EEXIST; if (index > nb->l3_cache.indices) return -EINVAL; /* check whether the other slot has disabled the same index already */ if (index == amd_get_l3_disable_slot(nb, !slot)) - return -EINVAL; + return -EEXIST; amd_l3_disable_index(nb, cpu, slot, index); @@ -468,8 +468,8 @@ static ssize_t store_cache_disable(struct _cpuid4_info *this_leaf, err = amd_set_l3_disable_slot(this_leaf->base.nb, cpu, slot, val); if (err) { if (err == -EEXIST) - printk(KERN_WARNING "L3 disable slot %d in use!\n", - slot); + pr_warning("L3 slot %d in use/index already disabled!\n", + slot); return err; } return count; ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-04-25 13:51 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-04-19 7:00 [patch] cpu: remove some dead code in store_cache_disable() Dan Carpenter 2012-04-19 8:18 ` Srivatsa S. Bhat 2012-04-19 11:02 ` Borislav Petkov 2012-04-19 11:14 ` Srivatsa S. Bhat 2012-04-19 11:44 ` Dan Carpenter 2012-04-19 16:53 ` [GIT PULL] L3 CID fix for 3.5 Borislav Petkov 2012-04-25 9:30 ` Borislav Petkov 2012-04-25 10:23 ` Ingo Molnar 2012-04-25 10:35 ` Borislav Petkov 2012-04-25 13:50 ` [tip:x86/urgent] x86, intel_cacheinfo: Fix error return code in amd_set_l3_disable_slot() tip-bot for Srivatsa S. Bhat
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox