| Message ID | 20170217063936.13208-5-haozhong.zhang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 17.02.17 at 07:39, <haozhong.zhang@intel.com> wrote: > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> Not sure here, I can't imagine the function being here without reason, even if the plans perhaps never got carried out. Could you please identify in the commit message here whether there ever were callers (and if so, when the last one went away) or what commit introduced the function without any users? > --- a/xen/arch/x86/cpu/mcheck/mce.h > +++ b/xen/arch/x86/cpu/mcheck/mce.h > @@ -146,7 +146,6 @@ typedef struct mcinfo_extended *(*x86_mce_callback_t) > (struct mc_info *, uint16_t, uint64_t); > extern void x86_mce_callback_register(x86_mce_callback_t); > > -void *x86_mcinfo_add(struct mc_info *mi, void *mcinfo); > void *x86_mcinfo_reserve(struct mc_info *mi, int size); > void x86_mcinfo_dump(struct mc_info *mi); There's a comment earlier in this file which would need adjustment too. Maybe that also helps understand if the function would better be left in place ... Jan
On 02/17/17 02:55 -0700, Jan Beulich wrote: > >>> On 17.02.17 at 07:39, <haozhong.zhang@intel.com> wrote: > > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > > Not sure here, I can't imagine the function being here without > reason, even if the plans perhaps never got carried out. Could > you please identify in the commit message here whether there > ever were callers (and if so, when the last one went away) or > what commit introduced the function without any users? > The commit that removed the last uses of x86_mcinfo_add() is commit 9d13fd9fd320a7740c6446c048ff6a2990095966 Author: Keir Fraser <keir.fraser@citrix.com> Date: Mon Jun 7 15:46:48 2010 +0100 x86 mce: Change the method to get the extended MCA information. Several changes to get the extended MCA information: a) Use the x86_mcinfo_reserve in mcinfo_extended to reserve buffer from mc_info, instead of using the stack b) For intel's extended MSR, we don't need write them one by one as the MSR are continous c) We don't need enum mca_extinfo, since we can consider the extended MSR as either per bank, or global. Currently we add a hook in global data collection, and didn't call register intel_get_extended_msrs as callback. Later that hook can be replaced by cleaner way Signed-off-by: Jiang, Yunhong <yunhong.jiang@inte.com> Specially, it's a) in that commit that intended to directly update the buffer reserved by x86_mcinfo_reserve(), instead of updating another buffer and using x86_mcinfo_add() to copy it to the buffer reserved by x86_mcinfo_reserve(). It looks that x86_mcinfo_add() was deprecated by that commit, so it's reasonable to remove it. > > --- a/xen/arch/x86/cpu/mcheck/mce.h > > +++ b/xen/arch/x86/cpu/mcheck/mce.h > > @@ -146,7 +146,6 @@ typedef struct mcinfo_extended *(*x86_mce_callback_t) > > (struct mc_info *, uint16_t, uint64_t); > > extern void x86_mce_callback_register(x86_mce_callback_t); > > > > -void *x86_mcinfo_add(struct mc_info *mi, void *mcinfo); > > void *x86_mcinfo_reserve(struct mc_info *mi, int size); > > void x86_mcinfo_dump(struct mc_info *mi); > > There's a comment earlier in this file which would need adjustment > too. Maybe that also helps understand if the function would better > be left in place ... > I'll update the commit if we decide to remove this function. Thanks, Haozhong
>>> On 20.02.17 at 02:52, <haozhong.zhang@intel.com> wrote: > On 02/17/17 02:55 -0700, Jan Beulich wrote: >> >>> On 17.02.17 at 07:39, <haozhong.zhang@intel.com> wrote: >> > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> >> >> Not sure here, I can't imagine the function being here without >> reason, even if the plans perhaps never got carried out. Could >> you please identify in the commit message here whether there >> ever were callers (and if so, when the last one went away) or >> what commit introduced the function without any users? >> > > The commit that removed the last uses of x86_mcinfo_add() is > > commit 9d13fd9fd320a7740c6446c048ff6a2990095966 > Author: Keir Fraser <keir.fraser@citrix.com> > Date: Mon Jun 7 15:46:48 2010 +0100 > > x86 mce: Change the method to get the extended MCA information. > > Several changes to get the extended MCA information: > a) Use the x86_mcinfo_reserve in mcinfo_extended to reserve buffer > from > mc_info, instead of using the stack > b) For intel's extended MSR, we don't need write them one > by one as the MSR are continous > c) We don't need enum mca_extinfo, since we can consider > the extended MSR as either per bank, or global. Currently > we add a hook in global data collection, and didn't call > register intel_get_extended_msrs as callback. Later that > hook can be replaced by cleaner way > > Signed-off-by: Jiang, Yunhong <yunhong.jiang@inte.com> > > Specially, it's a) in that commit that intended to directly update the > buffer reserved by x86_mcinfo_reserve(), instead of updating another > buffer and using x86_mcinfo_add() to copy it to the buffer reserved by > x86_mcinfo_reserve(). It looks that x86_mcinfo_add() was deprecated by > that commit, so it's reasonable to remove it. Okay, so please add a reference to that commit to your patch description. Jan
On 02/20/17 02:00 -0700, Jan Beulich wrote: > >>> On 20.02.17 at 02:52, <haozhong.zhang@intel.com> wrote: > > On 02/17/17 02:55 -0700, Jan Beulich wrote: > >> >>> On 17.02.17 at 07:39, <haozhong.zhang@intel.com> wrote: > >> > Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> > >> > >> Not sure here, I can't imagine the function being here without > >> reason, even if the plans perhaps never got carried out. Could > >> you please identify in the commit message here whether there > >> ever were callers (and if so, when the last one went away) or > >> what commit introduced the function without any users? > >> > > > > The commit that removed the last uses of x86_mcinfo_add() is > > > > commit 9d13fd9fd320a7740c6446c048ff6a2990095966 > > Author: Keir Fraser <keir.fraser@citrix.com> > > Date: Mon Jun 7 15:46:48 2010 +0100 > > > > x86 mce: Change the method to get the extended MCA information. > > > > Several changes to get the extended MCA information: > > a) Use the x86_mcinfo_reserve in mcinfo_extended to reserve buffer > > from > > mc_info, instead of using the stack > > b) For intel's extended MSR, we don't need write them one > > by one as the MSR are continous > > c) We don't need enum mca_extinfo, since we can consider > > the extended MSR as either per bank, or global. Currently > > we add a hook in global data collection, and didn't call > > register intel_get_extended_msrs as callback. Later that > > hook can be replaced by cleaner way > > > > Signed-off-by: Jiang, Yunhong <yunhong.jiang@inte.com> > > > > Specially, it's a) in that commit that intended to directly update the > > buffer reserved by x86_mcinfo_reserve(), instead of updating another > > buffer and using x86_mcinfo_add() to copy it to the buffer reserved by > > x86_mcinfo_reserve(). It looks that x86_mcinfo_add() was deprecated by > > that commit, so it's reasonable to remove it. > > Okay, so please add a reference to that commit to your patch > description. > Sure, will add. Thanks, Haozhong
diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c index e327eab..f682520 100644 --- a/xen/arch/x86/cpu/mcheck/mce.c +++ b/xen/arch/x86/cpu/mcheck/mce.c @@ -834,22 +834,6 @@ void *x86_mcinfo_reserve(struct mc_info *mi, int size) return memset(mic_index, 0, size); } -void *x86_mcinfo_add(struct mc_info *mi, void *mcinfo) -{ - struct mcinfo_common *mic, *buf; - - mic = (struct mcinfo_common *)mcinfo; - buf = x86_mcinfo_reserve(mi, mic->size); - - if ( !buf ) - mce_printk(MCE_CRITICAL, - "mcinfo_add: No space left in mc_info\n"); - else - memcpy(buf, mic, mic->size); - - return buf; -} - static void x86_mcinfo_apei_save( struct mcinfo_global *mc_global, struct mcinfo_bank *mc_bank) { diff --git a/xen/arch/x86/cpu/mcheck/mce.h b/xen/arch/x86/cpu/mcheck/mce.h index e697780..56877c1 100644 --- a/xen/arch/x86/cpu/mcheck/mce.h +++ b/xen/arch/x86/cpu/mcheck/mce.h @@ -146,7 +146,6 @@ typedef struct mcinfo_extended *(*x86_mce_callback_t) (struct mc_info *, uint16_t, uint64_t); extern void x86_mce_callback_register(x86_mce_callback_t); -void *x86_mcinfo_add(struct mc_info *mi, void *mcinfo); void *x86_mcinfo_reserve(struct mc_info *mi, int size); void x86_mcinfo_dump(struct mc_info *mi);
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com> --- Cc: Christoph Egger <chegger@amazon.de> Cc: Liu Jinsong <jinsong.liu@alibaba-inc.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> --- xen/arch/x86/cpu/mcheck/mce.c | 16 ---------------- xen/arch/x86/cpu/mcheck/mce.h | 1 - 2 files changed, 17 deletions(-)