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@xxxxxxxxx wrote:
From: Shuo Liu <shuo.a.liu@xxxxxxxxx>
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@xxxxxxxxxxxxx>
Acked-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx> # build-tested
Signed-off-by: Shuo Liu <shuo.a.liu@xxxxxxxxx>
---
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
