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?
Palmer Dabbelt (4):
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
Move USE_WCACHING to drivers/block/pktcdvd.c
drivers/block/pktcdvd.c | 11 +++++++++++
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 +++++
tools/include/uapi/linux/hw_breakpoint.h | 10 ----------
9 files changed, 39 insertions(+), 47 deletions(-)
--
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]>
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]
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
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]
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]>
I don't think this was ever intended to be exposed to userspace, but
it did require an "#ifdef CONFIG_*". Since the name is kind of
generic and was only used in one place, I've moved the definition to
the one user.
Signed-off-by: Palmer Dabbelt <[email protected]>
Reviewed-by: Andrew Waterman <[email protected]>
Reviewed-by: Albert Ou <[email protected]>
Message-Id: <[email protected]>
Signed-off-by: Thomas Huth <[email protected]>
---
drivers/block/pktcdvd.c | 11 +++++++++++
include/uapi/linux/pktcdvd.h | 11 -----------
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 2f1a92509271..1d5ec8d9d1a5 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -90,6 +90,17 @@ do { \
#define MAX_SPEED 0xffff
+/*
+ * 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
+
static DEFINE_MUTEX(pktcdvd_mutex);
static struct pktcdvd_device *pkt_devs[MAX_WRITERS];
static struct proc_dir_entry *pkt_proc;
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
On Fri, Feb 17, 2023, at 21:22, Thomas Huth wrote:
> 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]
> Signed-off-by: Thomas Huth <[email protected]>
Reviewed-by: Arnd Bergmann <[email protected]>
On Fri, Feb 17, 2023, at 21:23, Thomas Huth wrote:
> 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]
> Signed-off-by: Thomas Huth <[email protected]>
It took me a while to understand this code enough to be confident this
is the right solution. Note that CONFIG_HAVE_MIXED_BREAKPOINTS_REGS is
purely dependent on the architecture and could be replaced with something
that checks for x86||sh but it should be safe to assume that the
enum should never have been part of the uapi header.
Reviewed-by: Arnd Bergmann <[email protected]>
Arnd
On Fri, Feb 17, 2023, at 21:23, Thomas Huth wrote:
> From: Palmer Dabbelt <[email protected]>
>
> I don't think this was ever intended to be exposed to userspace, but
> it did require an "#ifdef CONFIG_*". Since the name is kind of
> generic and was only used in one place, I've moved the definition to
> the one user.
>
> Signed-off-by: Palmer Dabbelt <[email protected]>
> Reviewed-by: Andrew Waterman <[email protected]>
> Reviewed-by: Albert Ou <[email protected]>
> Message-Id: <[email protected]>
> Signed-off-by: Thomas Huth <[email protected]>
> ---
> drivers/block/pktcdvd.c | 11 +++++++++++
> include/uapi/linux/pktcdvd.h | 11 -----------
> 2 files changed, 11 insertions(+), 11 deletions(-)
I'm fairly sure there are more bits in uapi/linux/pktcdvd.h that should
be in drivers/block/pktcdvd.c instead, along with all of
include/linux/pktcdvd.h, but this change is obvious and safe by itself,
so
Reviewed-by: Arnd Bergmann <[email protected]>
On Fri, Feb 17, 2023, at 21:22, 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?
I just sent the pull request for the merge window an will be out
of office for most of next week. The patches all look good and
I provided a Reviewed-by: tag in case someone else wants to pick
up some or all of them. Otherwise I can apply them when I get
back.
Arnd
Looks good:
Reviewed-by: Christoph Hellwig <[email protected]>
Looks good:
Reviewed-by: Christoph Hellwig <[email protected]>
Looks good:
Reviewed-by: Christoph Hellwig <[email protected]>
On Fri, Feb 17, 2023 at 09:23:01PM +0100, Thomas Huth wrote:
> From: Palmer Dabbelt <[email protected]>
>
> I don't think this was ever intended to be exposed to userspace, but
> it did require an "#ifdef CONFIG_*". Since the name is kind of
> generic and was only used in one place, I've moved the definition to
> the one user.
I'd just remove USE_WCACHING entirel with something like:
diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 2f1a92509271c4..5ae2a80db2c341 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 9cbb55d21c94af..6a5552dfd6af4e 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 ->
*/