* [PATCH] virt: acrn: Fix vCPU removing code build error @ 2021-02-12 4:57 shuo.a.liu 2021-02-12 7:52 ` Greg Kroah-Hartman 0 siblings, 1 reply; 5+ messages in thread From: shuo.a.liu @ 2021-02-12 4:57 UTC (permalink / raw) To: linux-next Cc: Stephen Rothwell, Randy Dunlap, Greg Kroah-Hartman, linux-kernel, Shuo Liu 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 }; base-commit: 671176b0016c80b3943cb5387312c886aba3308d -- 2.28.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] virt: acrn: Fix vCPU removing code build error 2021-02-12 4:57 [PATCH] virt: acrn: Fix vCPU removing code build error shuo.a.liu @ 2021-02-12 7:52 ` Greg Kroah-Hartman 2021-02-12 10:58 ` Shuo A Liu 0 siblings, 1 reply; 5+ messages in thread From: Greg Kroah-Hartman @ 2021-02-12 7:52 UTC (permalink / raw) To: shuo.a.liu; +Cc: linux-next, Stephen Rothwell, Randy Dunlap, linux-kernel 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? 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? thanks, greg k-h ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] virt: acrn: Fix vCPU removing code build error 2021-02-12 7:52 ` Greg Kroah-Hartman @ 2021-02-12 10:58 ` Shuo A Liu 2021-02-12 11:02 ` Greg Kroah-Hartman 0 siblings, 1 reply; 5+ messages in thread From: Shuo A Liu @ 2021-02-12 10:58 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-next, Stephen Rothwell, Randy Dunlap, linux-kernel 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. Thanks shuo ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] virt: acrn: Fix vCPU removing code build error 2021-02-12 10:58 ` Shuo A Liu @ 2021-02-12 11:02 ` Greg Kroah-Hartman 2021-02-12 11:19 ` Shuo A Liu 0 siblings, 1 reply; 5+ messages in thread From: Greg Kroah-Hartman @ 2021-02-12 11:02 UTC (permalink / raw) To: Shuo A Liu; +Cc: linux-next, Stephen Rothwell, Randy Dunlap, linux-kernel 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, greg k-h ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] virt: acrn: Fix vCPU removing code build error 2021-02-12 11:02 ` Greg Kroah-Hartman @ 2021-02-12 11:19 ` Shuo A Liu 0 siblings, 0 replies; 5+ messages in thread From: Shuo A Liu @ 2021-02-12 11:19 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: linux-next, Stephen Rothwell, Randy Dunlap, linux-kernel 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-02-12 11:21 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-12 4:57 [PATCH] virt: acrn: Fix vCPU removing code build error shuo.a.liu 2021-02-12 7:52 ` Greg Kroah-Hartman 2021-02-12 10:58 ` Shuo A Liu 2021-02-12 11:02 ` Greg Kroah-Hartman 2021-02-12 11:19 ` Shuo A Liu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).