On 05.12.2025 21:36, Milan Djokic wrote:
Handling of unsupported sysctl commands currently diverges: some commands
return -EOPNOTSUPP, while others fall back to generic -ENOSYS.
Unify the behavior so that unsupported commands consistently return the
appropriate error code, allowing the control domain to correctly identify
unsupported operations.
Using IS_ENABLED() macro to identify disabled commands, allowing
dead code to be removed at compile time.
Signed-off-by: Milan Djokic <milan_djokic@xxxxxxxx>
Overall quite okay imo, yet there are a number of small issues.
@@ -170,19 +173,22 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t)
u_sysctl)
op->u.availheap.avail_bytes <<= PAGE_SHIFT;
break;
-#ifdef CONFIG_PM_STATS
case XEN_SYSCTL_get_pmstat:
- ret = do_get_pm_info(&op->u.get_pmstat);
+ if ( !IS_ENABLED(CONFIG_PM_STATS) )
+ ret = -EOPNOTSUPP;
+ else
+ ret = do_get_pm_info(&op->u.get_pmstat);
break;
-#endif
-#ifdef CONFIG_PM_OP
case XEN_SYSCTL_pm_op:
- ret = do_pm_op(&op->u.pm_op);
- if ( ret == -EAGAIN )
- copyback = 1;
+ if ( !IS_ENABLED(CONFIG_PM_OP) )
+ ret = -EOPNOTSUPP;
+ else {
Misplaced figure brace.
--- a/xen/include/xen/perfc.h
+++ b/xen/include/xen/perfc.h
@@ -92,7 +92,6 @@ DECLARE_PER_CPU(perfc_t[NUM_PERFCOUNTERS], perfcounters);
#endif
struct xen_sysctl_perfc_op;
-int perfc_control(struct xen_sysctl_perfc_op *pc);
extern void cf_check perfc_printall(unsigned char key);
extern void cf_check perfc_reset(unsigned char key);
@@ -114,4 +113,7 @@ extern void cf_check perfc_reset(unsigned char key);
#endif /* CONFIG_PERF_COUNTERS */
+struct xen_sysctl_perfc_op;
+extern int perfc_control(struct xen_sysctl_perfc_op *pc);
Why would you move the function decl, but duplicate the struct forward decl?
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -7,6 +7,7 @@
#include <asm/system.h>
#include <asm/spinlock.h>
+#include <public/sysctl.h>
Why would this be needed? It means effectively the whole codebase gains a
dependency on this header even when DEBUG_LOCK_PROFILE=n. Yes, you may
need ...
@@ -40,8 +41,6 @@ union lock_debug { };
#ifdef CONFIG_DEBUG_LOCK_PROFILE
-#include <public/sysctl.h>
-
/*
lock profiling on:
@@ -164,7 +163,6 @@ void _lock_profile_deregister_struct(int32_t type,
#define lock_profile_deregister_struct(type, ptr)
\
_lock_profile_deregister_struct(type, &((ptr)->profile_head))
-extern int spinlock_profile_control(struct xen_sysctl_lockprof_op *pc);
extern void cf_check spinlock_profile_printall(unsigned char key);
extern void cf_check spinlock_profile_reset(unsigned char key);
@@ -360,4 +358,6 @@ static always_inline void nrspin_lock_irq(rspinlock_t *l)
#define nrspin_unlock_irqrestore(l, f) _nrspin_unlock_irqrestore(l, f)
#define nrspin_unlock_irq(l) _nrspin_unlock_irq(l)
+extern int spinlock_profile_control(struct xen_sysctl_lockprof_op *pc);
+
#endif /* __SPINLOCK_H__ */
... a forward decl of struct xen_sysctl_lockprof_op; I don't see what's
wrong with doing just that.
Jan
