Messages in this thread | | | Date | Fri, 12 Feb 2021 19:19:21 +0800 | From | Shuo A Liu <> | Subject | Re: [PATCH] virt: acrn: Fix vCPU removing code build error |
| |
On Fri 12.Feb'21 at 12:02:18 +0100, Greg Kroah-Hartman wrote: >On Fri, Feb 12, 2021 at 06:58:53PM +0800, Shuo A Liu wrote: >> Hi Greg, >> >> On Fri 12.Feb'21 at 8:52:33 +0100, Greg Kroah-Hartman wrote: >> > On Fri, Feb 12, 2021 at 12:57:24PM +0800, shuo.a.liu@intel.com wrote: >> > > From: Shuo Liu <shuo.a.liu@intel.com> >> > > >> > > vCPU removing code depends on CONFIG_HOTPLUG_CPU as it uses remove_cpu() >> > > and add_cpu(). Make the vCPU removing interface building with >> > > CONFIG_HOTPLUG_CPU. >> > > >> > > ../drivers/virt/acrn/hsm.c: In function ‘remove_cpu_store’: >> > > ../drivers/virt/acrn/hsm.c:389:3: error: implicit declaration of function ‘remove_cpu’; [-Werror=implicit-function-declaration] >> > > remove_cpu(cpu); >> > > >> > > ../drivers/virt/acrn/hsm.c:402:2: error: implicit declaration of function ‘add_cpu’; [-Werror=implicit-function-declaration] >> > > add_cpu(cpu); >> > > >> > > Fixes: 279dcf693ac7 ("virt: acrn: Introduce an interface for Service VM to control vCPU") >> > > Reported-by: Randy Dunlap <rdunlap@infradead.org> >> > > Acked-by: Randy Dunlap <rdunlap@infradead.org> # build-tested >> > > Signed-off-by: Shuo Liu <shuo.a.liu@intel.com> >> > > --- >> > > drivers/virt/acrn/hsm.c | 4 ++++ >> > > 1 file changed, 4 insertions(+) >> > > >> > > diff --git a/drivers/virt/acrn/hsm.c b/drivers/virt/acrn/hsm.c >> > > index 1f6b7c54a1a4..e340788aacdf 100644 >> > > --- a/drivers/virt/acrn/hsm.c >> > > +++ b/drivers/virt/acrn/hsm.c >> > > @@ -372,6 +372,7 @@ static int acrn_dev_release(struct inode *inode, struct file *filp) >> > > return 0; >> > > } >> > > >> > > +#ifdef CONFIG_HOTPLUG_CPU >> > > static ssize_t remove_cpu_store(struct device *dev, >> > > struct device_attribute *attr, >> > > const char *buf, size_t count) >> > > @@ -403,9 +404,12 @@ static ssize_t remove_cpu_store(struct device *dev, >> > > return ret; >> > > } >> > > static DEVICE_ATTR_WO(remove_cpu); >> > > +#endif >> > > >> > > static struct attribute *acrn_attrs[] = { >> > > +#ifdef CONFIG_HOTPLUG_CPU >> > > &dev_attr_remove_cpu.attr, >> > > +#endif >> > > NULL >> > > }; >> > > >> > > >> > >> > Shouldn't the real solution for this be that remove_cpu() and add_cpu() >> > have function prototypes for when this is not enabled in the kernel >> > build? >> >> Something like this in linux/cpu.h? >> >> diff --git a/include/linux/cpu.h b/include/linux/cpu.h >> index 3aaa0687e8df..94a578a96202 100644 >> --- a/include/linux/cpu.h >> +++ b/include/linux/cpu.h >> @@ -108,6 +108,8 @@ static inline void cpu_maps_update_done(void) >> { >> } >> >> +static inline int add_cpu(unsigned int cpu) { return 0;} >> + >> #endif /* CONFIG_SMP */ >> extern struct bus_type cpu_subsys; >> >> @@ -137,6 +139,7 @@ static inline int cpus_read_trylock(void) { return >> true; } >> static inline void lockdep_assert_cpus_held(void) { } >> static inline void cpu_hotplug_disable(void) { } >> static inline void cpu_hotplug_enable(void) { } >> +static inline int remove_cpu(unsigned int cpu) { return -EPERM; } >> static inline void smp_shutdown_nonboot_cpus(unsigned int primary_cpu) >> { } >> #endif /* !CONFIG_HOTPLUG_CPU */ >> >> >> > >> > Putting #ifdef in .c files like this is not a good idea at all. >> > >> > Then, at runtime, you can determine if you need to create this sysfs >> > file or not, as you do not want to expose it to userspace if the kernel >> > can not handle it, right? >> >> Right. I don't want to expose the sysfs to userspace if the kernel built >> w/o CONFIG_HOTPLUG_CPU. But how to implement that if #ifdef is not used? >> misc_register() creates sysfs with .groups (acrn_attr_groups) >> unconditionally, then userspace can see the interface even it doesn't >> work. > >Use the is_visible() callback for your attribute group.
Thanks for the guide, is_visible() is good. Seems good i can provide the callback with return IS_ENABLED(CONFIG_HOTPLUG_CPU);
Will update in v2.
Thanks shuo
| |