uapi headers should not use the kernel-internal CONFIG switches.
Palmer Dabbelt sent some patches to clean this up a couple of years
ago, but unfortunately some of those patches never got merged.
So here's a rebased version of those patches - since they are rather
trivial, I hope it's OK for everybody if they could go through Arnd's
"generic include/asm header files" branch.
v2:
- Added Reviewed-bys from v1
- Changed the CONFIG_CDROM_PKTCDVD_WCACHE patch according to Christoph's
suggestion
- Added final patch to clean the list in scripts/headers_install.sh
Palmer Dabbelt (3):
Move COMPAT_ATM_ADDPARTY to net/atm/svc.c
Move ep_take_care_of_epollwakeup() to fs/eventpoll.c
Move bp_type_idx to include/linux/hw_breakpoint.h
Thomas Huth (2):
pktcdvd: Remove CONFIG_CDROM_PKTCDVD_WCACHE from uapi header
scripts: Update the CONFIG_* ignore list in headers_install.sh
drivers/block/pktcdvd.c | 13 +++++++++----
fs/eventpoll.c | 13 +++++++++++++
include/linux/hw_breakpoint.h | 10 ++++++++++
include/uapi/linux/atmdev.h | 4 ----
include/uapi/linux/eventpoll.h | 12 ------------
include/uapi/linux/hw_breakpoint.h | 10 ----------
include/uapi/linux/pktcdvd.h | 11 -----------
net/atm/svc.c | 5 +++++
scripts/headers_install.sh | 4 ----
tools/include/uapi/linux/hw_breakpoint.h | 10 ----------
10 files changed, 37 insertions(+), 55 deletions(-)
--
2.31.1
From: Palmer Dabbelt <[email protected]>
This has a "#ifdef CONFIG_*" that used to be exposed to userspace.
The names in here are so generic that I don't think it's a good idea
to expose them to userspace (or even the rest of the kernel). There are
multiple in-kernel users, so it's been moved to a kernel header file.
Signed-off-by: Palmer Dabbelt <[email protected]>
Reviewed-by: Andrew Waterman <[email protected]>
Reviewed-by: Albert Ou <[email protected]>
Message-Id: <[email protected]>
[thuth: Remove it also from tools/include/uapi/linux/hw_breakpoint.h]
Reviewed-by: Arnd Bergmann <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Thomas Huth <[email protected]>
---
include/linux/hw_breakpoint.h | 10 ++++++++++
include/uapi/linux/hw_breakpoint.h | 10 ----------
tools/include/uapi/linux/hw_breakpoint.h | 10 ----------
3 files changed, 10 insertions(+), 20 deletions(-)
diff --git a/include/linux/hw_breakpoint.h b/include/linux/hw_breakpoint.h
index f319bd26b030..7fbb45911273 100644
--- a/include/linux/hw_breakpoint.h
+++ b/include/linux/hw_breakpoint.h
@@ -7,6 +7,16 @@
#ifdef CONFIG_HAVE_HW_BREAKPOINT
+enum bp_type_idx {
+ TYPE_INST = 0,
+#if defined(CONFIG_HAVE_MIXED_BREAKPOINTS_REGS)
+ TYPE_DATA = 0,
+#else
+ TYPE_DATA = 1,
+#endif
+ TYPE_MAX
+};
+
extern int __init init_hw_breakpoint(void);
static inline void hw_breakpoint_init(struct perf_event_attr *attr)
diff --git a/include/uapi/linux/hw_breakpoint.h b/include/uapi/linux/hw_breakpoint.h
index 965e4d8606d8..1575d3ca6f0d 100644
--- a/include/uapi/linux/hw_breakpoint.h
+++ b/include/uapi/linux/hw_breakpoint.h
@@ -22,14 +22,4 @@ enum {
HW_BREAKPOINT_INVALID = HW_BREAKPOINT_RW | HW_BREAKPOINT_X,
};
-enum bp_type_idx {
- TYPE_INST = 0,
-#ifdef CONFIG_HAVE_MIXED_BREAKPOINTS_REGS
- TYPE_DATA = 0,
-#else
- TYPE_DATA = 1,
-#endif
- TYPE_MAX
-};
-
#endif /* _UAPI_LINUX_HW_BREAKPOINT_H */
diff --git a/tools/include/uapi/linux/hw_breakpoint.h b/tools/include/uapi/linux/hw_breakpoint.h
index 965e4d8606d8..1575d3ca6f0d 100644
--- a/tools/include/uapi/linux/hw_breakpoint.h
+++ b/tools/include/uapi/linux/hw_breakpoint.h
@@ -22,14 +22,4 @@ enum {
HW_BREAKPOINT_INVALID = HW_BREAKPOINT_RW | HW_BREAKPOINT_X,
};
-enum bp_type_idx {
- TYPE_INST = 0,
-#ifdef CONFIG_HAVE_MIXED_BREAKPOINTS_REGS
- TYPE_DATA = 0,
-#else
- TYPE_DATA = 1,
-#endif
- TYPE_MAX
-};
-
#endif /* _UAPI_LINUX_HW_BREAKPOINT_H */
--
2.31.1
From: Palmer Dabbelt <[email protected]>
This used to be behind an #ifdef COMPAT_COMPAT, so most of userspace
wouldn't have seen the definition before. Unfortunately this header
file became visible to userspace, so the definition has instead been
moved to net/atm/svc.c (the only user).
Signed-off-by: Palmer Dabbelt <[email protected]>
Reviewed-by: Andrew Waterman <[email protected]>
Reviewed-by: Albert Ou <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Arnd Bergmann <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Thomas Huth <[email protected]>
---
include/uapi/linux/atmdev.h | 4 ----
net/atm/svc.c | 5 +++++
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/include/uapi/linux/atmdev.h b/include/uapi/linux/atmdev.h
index a5c15cf23bd7..20b0215084fc 100644
--- a/include/uapi/linux/atmdev.h
+++ b/include/uapi/linux/atmdev.h
@@ -101,10 +101,6 @@ struct atm_dev_stats {
/* use backend to make new if */
#define ATM_ADDPARTY _IOW('a', ATMIOC_SPECIAL+4,struct atm_iobuf)
/* add party to p2mp call */
-#ifdef CONFIG_COMPAT
-/* It actually takes struct sockaddr_atmsvc, not struct atm_iobuf */
-#define COMPAT_ATM_ADDPARTY _IOW('a', ATMIOC_SPECIAL+4,struct compat_atm_iobuf)
-#endif
#define ATM_DROPPARTY _IOW('a', ATMIOC_SPECIAL+5,int)
/* drop party from p2mp call */
diff --git a/net/atm/svc.c b/net/atm/svc.c
index 4a02bcaad279..d83556d8beb9 100644
--- a/net/atm/svc.c
+++ b/net/atm/svc.c
@@ -28,6 +28,11 @@
#include "signaling.h"
#include "addr.h"
+#ifdef CONFIG_COMPAT
+/* It actually takes struct sockaddr_atmsvc, not struct atm_iobuf */
+#define COMPAT_ATM_ADDPARTY _IOW('a', ATMIOC_SPECIAL + 4, struct compat_atm_iobuf)
+#endif
+
static int svc_create(struct net *net, struct socket *sock, int protocol,
int kern);
--
2.31.1
From: Palmer Dabbelt <[email protected]>
This doesn't make any sense to expose to userspace, so it's been moved
to the one user. This was introduced by commit 95f19f658ce1 ("epoll:
drop EPOLLWAKEUP if PM_SLEEP is disabled").
Signed-off-by: Palmer Dabbelt <[email protected]>
Reviewed-by: Andrew Waterman <[email protected]>
Reviewed-by: Albert Ou <[email protected]>
Message-Id: <[email protected]>
[thuth: Rebased to fix contextual conflicts]
Reviewed-by: Arnd Bergmann <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Thomas Huth <[email protected]>
---
fs/eventpoll.c | 13 +++++++++++++
include/uapi/linux/eventpoll.h | 12 ------------
2 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 64659b110973..e2a5d2cc9051 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -2042,6 +2042,19 @@ SYSCALL_DEFINE1(epoll_create, int, size)
return do_epoll_create(0);
}
+#ifdef CONFIG_PM_SLEEP
+static inline void ep_take_care_of_epollwakeup(struct epoll_event *epev)
+{
+ if ((epev->events & EPOLLWAKEUP) && !capable(CAP_BLOCK_SUSPEND))
+ epev->events &= ~EPOLLWAKEUP;
+}
+#else
+static inline void ep_take_care_of_epollwakeup(struct epoll_event *epev)
+{
+ epev->events &= ~EPOLLWAKEUP;
+}
+#endif
+
static inline int epoll_mutex_lock(struct mutex *mutex, int depth,
bool nonblock)
{
diff --git a/include/uapi/linux/eventpoll.h b/include/uapi/linux/eventpoll.h
index e687658843b1..cfbcc4cc49ac 100644
--- a/include/uapi/linux/eventpoll.h
+++ b/include/uapi/linux/eventpoll.h
@@ -85,16 +85,4 @@ struct epoll_event {
__u64 data;
} EPOLL_PACKED;
-#ifdef CONFIG_PM_SLEEP
-static inline void ep_take_care_of_epollwakeup(struct epoll_event *epev)
-{
- if ((epev->events & EPOLLWAKEUP) && !capable(CAP_BLOCK_SUSPEND))
- epev->events &= ~EPOLLWAKEUP;
-}
-#else
-static inline void ep_take_care_of_epollwakeup(struct epoll_event *epev)
-{
- epev->events &= ~EPOLLWAKEUP;
-}
-#endif
#endif /* _UAPI_LINUX_EVENTPOLL_H */
--
2.31.1
CONFIG_* switches should not be exposed in uapi headers, thus let's get
rid of the USE_WCACHING macro here (which was also named way to generic)
and integrate the logic directly in the only function that needs it.
Suggested-by: Christoph Hellwig <[email protected]>
Signed-off-by: Thomas Huth <[email protected]>
---
drivers/block/pktcdvd.c | 13 +++++++++----
include/uapi/linux/pktcdvd.h | 11 -----------
2 files changed, 9 insertions(+), 15 deletions(-)
diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 2f1a92509271..5ae2a80db2c3 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -1869,12 +1869,12 @@ static noinline_for_stack int pkt_probe_settings(struct pktcdvd_device *pd)
/*
* enable/disable write caching on drive
*/
-static noinline_for_stack int pkt_write_caching(struct pktcdvd_device *pd,
- int set)
+static noinline_for_stack int pkt_write_caching(struct pktcdvd_device *pd)
{
struct packet_command cgc;
struct scsi_sense_hdr sshdr;
unsigned char buf[64];
+ bool set = IS_ENABLED(CONFIG_CDROM_PKTCDVD_WCACHE);
int ret;
init_cdrom_command(&cgc, buf, sizeof(buf), CGC_DATA_READ);
@@ -1890,7 +1890,12 @@ static noinline_for_stack int pkt_write_caching(struct pktcdvd_device *pd,
if (ret)
return ret;
- buf[pd->mode_offset + 10] |= (!!set << 2);
+ /*
+ * use drive write caching -- we need deferred error handling to be
+ * able to successfully recover with this option (drive will return good
+ * status as soon as the cdb is validated).
+ */
+ buf[pd->mode_offset + 10] |= (set << 2);
cgc.buflen = cgc.cmd[8] = 2 + ((buf[0] << 8) | (buf[1] & 0xff));
ret = pkt_mode_select(pd, &cgc);
@@ -2085,7 +2090,7 @@ static int pkt_open_write(struct pktcdvd_device *pd)
return -EIO;
}
- pkt_write_caching(pd, USE_WCACHING);
+ pkt_write_caching(pd);
ret = pkt_get_max_speed(pd, &write_speed);
if (ret)
diff --git a/include/uapi/linux/pktcdvd.h b/include/uapi/linux/pktcdvd.h
index 9cbb55d21c94..6a5552dfd6af 100644
--- a/include/uapi/linux/pktcdvd.h
+++ b/include/uapi/linux/pktcdvd.h
@@ -29,17 +29,6 @@
*/
#define PACKET_WAIT_TIME (HZ * 5 / 1000)
-/*
- * use drive write caching -- we need deferred error handling to be
- * able to successfully recover with this option (drive will return good
- * status as soon as the cdb is validated).
- */
-#if defined(CONFIG_CDROM_PKTCDVD_WCACHE)
-#define USE_WCACHING 1
-#else
-#define USE_WCACHING 0
-#endif
-
/*
* No user-servicable parts beyond this point ->
*/
--
2.31.1
The file in include/uapi/linux/ have been cleaned in the previous patches,
so we can now remove these entries from the CONFIG_* ignore-list.
Signed-off-by: Thomas Huth <[email protected]>
---
scripts/headers_install.sh | 4 ----
1 file changed, 4 deletions(-)
diff --git a/scripts/headers_install.sh b/scripts/headers_install.sh
index 4041881746ad..36b56b746fce 100755
--- a/scripts/headers_install.sh
+++ b/scripts/headers_install.sh
@@ -83,10 +83,6 @@ arch/nios2/include/uapi/asm/swab.h:CONFIG_NIOS2_CI_SWAB_SUPPORT
arch/x86/include/uapi/asm/auxvec.h:CONFIG_IA32_EMULATION
arch/x86/include/uapi/asm/auxvec.h:CONFIG_X86_64
arch/x86/include/uapi/asm/mman.h:CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
-include/uapi/linux/atmdev.h:CONFIG_COMPAT
-include/uapi/linux/eventpoll.h:CONFIG_PM_SLEEP
-include/uapi/linux/hw_breakpoint.h:CONFIG_HAVE_MIXED_BREAKPOINTS_REGS
-include/uapi/linux/pktcdvd.h:CONFIG_CDROM_PKTCDVD_WCACHE
"
for c in $configs
--
2.31.1
On Fri, Mar 10, 2023, at 17:07, Thomas Huth wrote:
> uapi headers should not use the kernel-internal CONFIG switches.
> Palmer Dabbelt sent some patches to clean this up a couple of years
> ago, but unfortunately some of those patches never got merged.
> So here's a rebased version of those patches - since they are rather
> trivial, I hope it's OK for everybody if they could go through Arnd's
> "generic include/asm header files" branch.
>
> v2:
> - Added Reviewed-bys from v1
> - Changed the CONFIG_CDROM_PKTCDVD_WCACHE patch according to Christoph's
> suggestion
> - Added final patch to clean the list in scripts/headers_install.sh
Thanks for the rebase, applied to the asm-generic tree now, as I'm
fairly optimistic they are all good.
Arnd
Looks good:
Reviewed-by: Christoph Hellwig <[email protected]>