Messages in this thread | | | Date | Mon, 27 Jun 2005 22:24:44 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH]MTRR suspend/resume cleanup |
| |
Shaohua Li <shaohua.li@intel.com> wrote: > > Hi, > There has been some discuss about solving the SMP MTRR suspend/resume > breakage, but I didn't find a patch for it. This is an intent for it. > The basic idea is moving mtrr initializing into cpu_identify for all APs > (so it works for cpu hotplug). For BP, restore_processor_state is > responsible for restoring MTRR.
> @@ -332,6 +332,7 @@ int mtrr_add_page(unsigned long base, un > > error = -EINVAL; > > + lock_cpu_hotplug(); > /* Search for existing MTRR */ > down(&main_lock); > for (i = 0; i < num_var_ranges; ++i) {
Is this new locking? If so, please describe it.
> + > +static int __init mtrr_init_finialize(void) > +{ > + if (!mtrr_if) > + return 0; > + if (use_intel()) > + mtrr_state_warn(); > + else > + /* The CPUs haven't MTRR and seemes not support SMP. They have specific > + * drivers, we use a tricky method to support suspend/resume for them > + * TBD: is there any system with such CPU which supports suspend/resume? > + * if no, we should remove the code. > + */ > + sysdev_driver_register(&cpu_sysdev_class, > + &mtrr_sysdev_driver); > + return 0; > +}
That's pretty hard to read. Braces will improve it:
if (use_intel()) { mtrr_state_warn(); } else { /* * The CPUs haven't MTRR and seemes not support SMP. They have * specific drivers, we use a tricky method to support * suspend/resume for them. * TBD: is there any system with such CPU which supports * suspend/resume? If no, we should remove the code. */ sysdev_driver_register(&cpu_sysdev_class, &mtrr_sysdev_driver); }
> +#ifdef CONFIG_MTRR > +extern int mtrr_ap_init(void); > +extern int mtrr_bp_init(void); > +#else > +#define mtrr_ap_init() > +#define mtrr_bp_init() > +#endif
If this works, it means we're always ignoring the return values and these things should return void.
Otherwise the !CONFIG_MTRR stubs should be returning an integer. So we need either:
#ifdef CONFIG_MTRR extern void mtrr_ap_init(void); extern void mtrr_bp_init(void); #else #define mtrr_ap_init() do {} while (0) (or static inlines) #define mtrr_bp_init() do {} while (0) #endif
or
#ifdef CONFIG_MTRR extern int mtrr_ap_init(void); extern int mtrr_bp_init(void); #else static inline int mtrr_ap_init(void) { return 0; } static inline int mtrr_bp_init(void) { return 0; } #endif
> +#ifdef CONFIG_MTRR > +extern int mtrr_ap_init(void); > +extern int mtrr_bp_init(void); > +#else > +#define mtrr_ap_init() > +#define mtrr_bp_init(); > +#endif
Ditto.
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |