Re: [PATCH v6] close_range.2: new page documenting close_range(2)
On Sun, 21 Mar 2021 16:38:59 +0100, "Michael Kerrisk (man-pages)" wrote: > On 3/9/21 8:53 PM, Stephen Kitt wrote: > > On Thu, 28 Jan 2021 21:50:23 +0100, "Michael Kerrisk (man-pages)" > > wrote: > >> Thanks for your patch revision. I've merged it, and have > >> done some light editing, but I still have a question: > > > > Does this need anything more? I don’t see it in the man-pages repo. > > Sorry, Stephen. It's just me being slow. I've made a few edits, > replaced the example program with another that more clearly allows > the user to see what's going on, and pushed to Git. Thanks, your example program is indeed much better! Regards, Stephen pgpCskU70zyVN.pgp Description: OpenPGP digital signature
Re: [PATCH v6] close_range.2: new page documenting close_range(2)
Hi Michael, On Thu, 28 Jan 2021 21:50:23 +0100, "Michael Kerrisk (man-pages)" wrote: > Thanks for your patch revision. I've merged it, and have > done some light editing, but I still have a question: Does this need anything more? I don’t see it in the man-pages repo. Regards, Stephen pgp6aXXmSEblE.pgp Description: OpenPGP digital signature
[PATCH] vgacon: drop unused vga_init_done
Commit 973c096f6a85 ("vgacon: remove software scrollback support")
removed all uses of vga_init_done, so let's get rid of it entirely.
Signed-off-by: Stephen Kitt
---
drivers/video/console/vgacon.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index 17876f0179b5..36bded9c9876 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -79,7 +79,6 @@ static struct uni_pagedir *vgacon_uni_pagedir;
static int vgacon_refcount;
/* Description of the hardware situation */
-static boolvga_init_done;
static unsigned long vga_vram_base __read_mostly; /* Base of
video memory */
static unsigned long vga_vram_end__read_mostly; /* End of video
memory */
static unsigned intvga_vram_size __read_mostly; /* Size of
video memory */
@@ -360,8 +359,6 @@ static const char *vgacon_startup(void)
vgacon_xres = screen_info.orig_video_cols * VGA_FONTWIDTH;
vgacon_yres = vga_scan_lines;
- vga_init_done = true;
-
return display_desc;
}
base-commit: f40ddce88593482919761f74910f42f4b84c004b
--
2.29.2
Re: [PATCH v6] close_range.2: new page documenting close_range(2)
Hello Michael, On Thu, 28 Jan 2021 21:50:23 +0100, "Michael Kerrisk (man-pages)" wrote: > Thanks for your patch revision. I've merged it, and have > done some light editing, but I still have a question: > > On 1/23/21 5:11 PM, Stephen Kitt wrote: > > [...] > > > +.SH ERRORS > > > +.TP > > +.B EMFILE > > +The per-process limit on the number of open file descriptors has been > > reached +(see the description of > > +.B RLIMIT_NOFILE > > +in > > +.BR getrlimit (2)). > > I think there was already a question about this error, but > I still have a doubt. > > A glance at the code tells me that indeed EMFILE can occur. > But how can the reason be because the limit on the number > of open file descriptors has been reached? I mean: no new > FDs are being opened, so how can we go over the limit. I think > the cause of this error is something else, but what is it? Here’s how I understand the code that can lead to EMFILE: * in __close_range(), if CLOSE_RANGE_UNSHARE is set, call unshare_fd() with CLONE_FILES to clone the fd table * unshare_fd() calls dup_fd() * dup_fd() allocates a new fdtable, and if the resulting fdtable ends up being too small to hold the number of fds calculated by sane_fdtable_size(), fails with EMFILE I suspect that, given that we’re starting with a valid fdtable, the only way this can happen is if there’s a race with sysctl_nr_open being reduced. Incidentally, isn’t this comment in file.c somewhat misleading? /* * If the requested range is greater than the current maximum, * we're closing everything so only copy all file descriptors * beneath the lowest file descriptor. */ As I understand it, dup_fd() will always copy any open file descriptor anyway, it won’t stop at max_unshare_fds if that’s lower than the number of open fds (thanks to save_fdtable_size())... Regards, Stephen pgpWHg2yLvrAW.pgp Description: OpenPGP digital signature
[PATCH v6] close_range.2: new page documenting close_range(2)
This documents close_range(2) based on information in 278a5fbaed89dacd04e9d052f4594ffd0e0585de, 60997c3d45d9a67daf01c56d805ae4fec37e0bd8, and 582f1fb6b721facf04848d2ca57f34468da1813e. Signed-off-by: Stephen Kitt --- V6: bit mask, close-on-exec flag language improvements another close(2) reference only include one example program ensure the example code doesn't wrap V5: clarification of the open/close_range/execve sequence V4: sort flags alphabetically move commit references inside the corresponding section more semantic newlines unformat numeric constants more formatting for function references escape C backslashes C99 loop indices V3: fix synopsis overflow copy notes from membarrier.2 re the lack of wrapper semantic newlines drop non-standard "USE CASES" section heading add code example V2: unsigned int to match the kernel declarations groff and grammar tweaks CLOSE_RANGE_UNSHARE unshares *and* closes Explain that EMFILE and ENOMEM can occur with C_R_U "Conforming to" phrasing Detailed explanation of CLOSE_RANGE_UNSHARE Reading /proc isn't common man2/close_range.2 | 236 + 1 file changed, 236 insertions(+) create mode 100644 man2/close_range.2 diff --git a/man2/close_range.2 b/man2/close_range.2 new file mode 100644 index 0..5abb73990 --- /dev/null +++ b/man2/close_range.2 @@ -0,0 +1,236 @@ +.\" Copyright (c) 2020 Stephen Kitt +.\" +.\" %%%LICENSE_START(VERBATIM) +.\" Permission is granted to make and distribute verbatim copies of this +.\" manual provided the copyright notice and this permission notice are +.\" preserved on all copies. +.\" +.\" Permission is granted to copy and distribute modified versions of this +.\" manual under the conditions for verbatim copying, provided that the +.\" entire resulting derived work is distributed under the terms of a +.\" permission notice identical to this one. +.\" +.\" Since the Linux kernel and libraries are constantly changing, this +.\" manual page may be incorrect or out-of-date. The author(s) assume no +.\" responsibility for errors or omissions, or for damages resulting from +.\" the use of the information contained herein. The author(s) may not +.\" have taken the same level of care in the production of this manual, +.\" which is licensed free of charge, as they might when working +.\" professionally. +.\" +.\" Formatted or processed versions of this manual, if unaccompanied by +.\" the source, must acknowledge the copyright and authors of this work. +.\" %%%LICENSE_END +.\" +.TH CLOSE_RANGE 2 2020-12-08 "Linux" "Linux Programmer's Manual" +.SH NAME +close_range \- close all file descriptors in a given range +.SH SYNOPSIS +.nf +.B #include +.PP +.BI "int close_range(unsigned int " first ", unsigned int " last , +.BI "unsigned int " flags ); +.fi +.PP +.IR Note : +There is no glibc wrapper for this system call; see NOTES. +.SH DESCRIPTION +The +.BR close_range () +system call closes all open file descriptors from +.I first +to +.I last +(included). +.PP +Errors closing a given file descriptor are currently ignored. +.PP +.I flags +is a bit mask containing 0 or more of the following: +.TP +.BR CLOSE_RANGE_CLOEXEC " (since Linux 5.11)" +sets the file descriptor's close-on-exec flag instead of +immediately closing the file descriptors. +.TP +.B CLOSE_RANGE_UNSHARE +unshares the range of file descriptors from any other processes, +before closing them, +avoiding races with other threads sharing the file descriptor table. +.SH RETURN VALUE +On success, +.BR close_range () +returns 0. +On error, \-1 is returned and +.I errno +is set to indicate the cause of the error. +.SH ERRORS +.TP +.B EINVAL +.I flags +is not valid, or +.I first +is greater than +.IR last . +.PP +The following can occur with +.B CLOSE_RANGE_UNSHARE +(when constructing the new descriptor table): +.TP +.B EMFILE +The per-process limit on the number of open file descriptors has been reached +(see the description of +.B RLIMIT_NOFILE +in +.BR getrlimit (2)). +.TP +.B ENOMEM +Insufficient kernel memory was available. +.SH VERSIONS +.BR close_range () +first appeared in Linux 5.9. +.SH CONFORMING TO +.BR close_range () +is a nonstandard function that is also present on FreeBSD. +.SH NOTES +Glibc does not provide a wrapper for this system call; call it using +.BR syscall (2). +.SS Closing all open file descriptors +.\" 278a5fbaed89dacd04e9d052f4594ffd0e0585de +To avoid blindly closing file descriptors +in the range of possible file descriptors, +this is sometimes implemented (on Linux) +by listing open file descriptors in +.I /proc/self/fd/ +and calling +.BR close (2) +on each one. +.BR close_range () +can take care of this without requirin
Re: [PATCH v5] close_range.2: new page documenting close_range(2)
Hi Michael, On Tue, 22 Dec 2020 21:36:28 +0100, "Michael Kerrisk (man-pages)" wrote: > On 12/21/20 8:46 PM, Stephen Kitt wrote: [...] > > +Errors closing a given file descriptor are currently ignored. > > +.PP > > +.I flags > > +can be 0 or set to one or both of the following: > > Better, I think: > "flags is a bit mask containing 0 or more of the following:" Indeed, thanks! > > +.TP > > +.BR CLOSE_RANGE_CLOEXEC " (since Linux 5.10)" > > s/5.10/5.11/ ? Oops, yes, 5.11. > > +sets the close-on-exec bit instead of > > s/close-on-exec bit/file descriptor's close-on-exec flag/ Noted. > > +immediately closing the file descriptors. > > +.TP > > +.B CLOSE_RANGE_UNSHARE > > +unshares the range of file descriptors from any other processes, > > +before closing them, > > +avoiding races with other threads sharing the file descriptor table. > > +.SH RETURN VALUE > > +On success, > > +.BR close_range () > > +returns 0. > > +On error, \-1 is returned and > > +.I errno > > +is set to indicate the cause of the error. > > +.SH ERRORS > > +.TP > > +.B EINVAL > > +.I flags > > +is not valid, or > > +.I first > > +is greater than > > +.IR last . > > +.PP > > +The following can occur with > > +.B CLOSE_RANGE_UNSHARE > > +(when constructing the new descriptor table): > > +.TP > > +.B EMFILE > > +The per-process limit on the number of open file descriptors has been > > reached +(see the description of > > +.B RLIMIT_NOFILE > > +in > > +.BR getrlimit (2)). > > +.TP > > +.B ENOMEM > > +Insufficient kernel memory was available. > > +.SH VERSIONS > > +.BR close_range () > > +first appeared in Linux 5.9. > > +.SH CONFORMING TO > > +.BR close_range () > > +is a nonstandard function that is also present on FreeBSD. > > +.SH NOTES > > +Glibc does not provide a wrapper for this system call; call it using > > +.BR syscall (2). > > +.SS Closing all open file descriptors > > +.\" 278a5fbaed89dacd04e9d052f4594ffd0e0585de > > +To avoid blindly closing file descriptors > > +in the range of possible file descriptors, > > +this is sometimes implemented (on Linux) > > +by listing open file descriptors in > > +.I /proc/self/fd/ > > +and calling > > +.BR close (2) > > +on each one. > > +.BR close_range () > > +can take care of this without requiring > > +.I /proc > > +and within a single system call, > > +which provides significant performance benefits. > > +.SS Closing file descriptors before exec > > +.\" 60997c3d45d9a67daf01c56d805ae4fec37e0bd8 > > +File descriptors can be closed safely using > > +.PP > > +.in +4n > > +.EX > > +/* we don't want anything past stderr here */ > > +close_range(3, ~0U, CLOSE_RANGE_UNSHARE); > > +execve(); > > +.EE > > +.in > > +.PP > > +.B CLOSE_RANGE_UNSHARE > > +is conceptually equivalent to > > +.PP > > +.in +4n > > +.EX > > +unshare(CLONE_FILES); > > +close_range(first, last, 0); > > +.EE > > +.in > > +.PP > > +but can be more efficient: > > +if the unshared range extends past > > +the current maximum number of file descriptors allocated > > +in the caller's file descriptor table > > +(the common case when > > +.I last > > +is ~0U), > > +the kernel will unshare a new file descriptor table for the caller up to > > +.IR first . > > +This avoids subsequent close calls entirely; > > s/close/.BR close (2)/ Noted. > > +the whole operation is complete once the table is unshared. > > +.SS Closing files on \fBexec\fP > > +.\" 582f1fb6b721facf04848d2ca57f34468da1813e > > +This is particularly useful in cases where multiple > > +.RB pre- exec > > +setup steps risk conflicting with each other. > > +For example, setting up a > > +.BR seccomp (2) > > +profile can conflict with a > > +.BR close_range () > > +call: > > +if the file descriptors are closed before the > > +.BR seccomp (2) > > +profile is set up, > > +the profile setup can't use them itself, > > +or control their closure; > > +if the file descriptors are closed afterwards, > > +the seccomp profile can't block the > > +.BR close_range () > > +call or any fallbacks. > > +Using > > +.B CLOSE_RANGE_CLOEXEC > > +avoids this: > > +the descriptors can be marked before the > > +.BR seccomp (2) > > +profile is set up, >
Re: [PATCH v3] close_range.2: new page documenting close_range(2)
Hi Alex, On Mon, 21 Dec 2020 20:33:06 +0100, "Alejandro Colomar (man-pages)" wrote: > On 12/21/20 8:24 PM, Stephen Kitt wrote: > > On Sat, 19 Dec 2020 15:00:00 +0100, "Alejandro Colomar (man-pages)" > > wrote: > >> On 12/18/20 5:58 PM, Stephen Kitt wrote: > > [...] > >>> +This program executes the command given on its command-line after > >>> +opening the files listed after the command, > >>> +and then using > >> > >> s/using/uses/ > > > > It’s the same form as “opening”: “after opening ... and then using”. The > > overall sequence is “open”, “close_range”, “execve”. > > Ahhh. Then I think the comma is misleading. > What about the following?: > > > On 12/18/20 5:58 PM, Stephen Kitt wrote: > > +.PP > > +This program executes the command given on its command-line after > > +opening the files listed after the command, > > +and then using > > +.B close_range > > +to close them: > > This program executes the command given on its command line, > after opening the files listed after the command > and then using *close_range()* to close them: Yes, that works better. I’ll follow up with a v5 with just that change. Regards, Stephen pgpP80blqpEv0.pgp Description: OpenPGP digital signature
[PATCH v5] close_range.2: new page documenting close_range(2)
This documents close_range(2) based on information in 278a5fbaed89dacd04e9d052f4594ffd0e0585de, 60997c3d45d9a67daf01c56d805ae4fec37e0bd8, and 582f1fb6b721facf04848d2ca57f34468da1813e. Signed-off-by: Stephen Kitt --- V5: clarification of the open/close_range/execve sequence V4: sort flags alphabetically move commit references inside the corresponding section more semantic newlines unformat numeric constants more formatting for function references escape C backslashes C99 loop indices V3: fix synopsis overflow copy notes from membarrier.2 re the lack of wrapper semantic newlines drop non-standard "USE CASES" section heading add code example V2: unsigned int to match the kernel declarations groff and grammar tweaks CLOSE_RANGE_UNSHARE unshares *and* closes Explain that EMFILE and ENOMEM can occur with C_R_U "Conforming to" phrasing Detailed explanation of CLOSE_RANGE_UNSHARE Reading /proc isn't common man2/close_range.2 | 267 + 1 file changed, 267 insertions(+) create mode 100644 man2/close_range.2 diff --git a/man2/close_range.2 b/man2/close_range.2 new file mode 100644 index 0..0677a9bf9 --- /dev/null +++ b/man2/close_range.2 @@ -0,0 +1,267 @@ +.\" Copyright (c) 2020 Stephen Kitt +.\" +.\" %%%LICENSE_START(VERBATIM) +.\" Permission is granted to make and distribute verbatim copies of this +.\" manual provided the copyright notice and this permission notice are +.\" preserved on all copies. +.\" +.\" Permission is granted to copy and distribute modified versions of this +.\" manual under the conditions for verbatim copying, provided that the +.\" entire resulting derived work is distributed under the terms of a +.\" permission notice identical to this one. +.\" +.\" Since the Linux kernel and libraries are constantly changing, this +.\" manual page may be incorrect or out-of-date. The author(s) assume no +.\" responsibility for errors or omissions, or for damages resulting from +.\" the use of the information contained herein. The author(s) may not +.\" have taken the same level of care in the production of this manual, +.\" which is licensed free of charge, as they might when working +.\" professionally. +.\" +.\" Formatted or processed versions of this manual, if unaccompanied by +.\" the source, must acknowledge the copyright and authors of this work. +.\" %%%LICENSE_END +.\" +.TH CLOSE_RANGE 2 2020-12-08 "Linux" "Linux Programmer's Manual" +.SH NAME +close_range \- close all file descriptors in a given range +.SH SYNOPSIS +.nf +.B #include +.PP +.BI "int close_range(unsigned int " first ", unsigned int " last , +.BI "unsigned int " flags ); +.fi +.PP +.IR Note : +There is no glibc wrapper for this system call; see NOTES. +.SH DESCRIPTION +The +.BR close_range () +system call closes all open file descriptors from +.I first +to +.I last +(included). +.PP +Errors closing a given file descriptor are currently ignored. +.PP +.I flags +can be 0 or set to one or both of the following: +.TP +.BR CLOSE_RANGE_CLOEXEC " (since Linux 5.10)" +sets the close-on-exec bit instead of +immediately closing the file descriptors. +.TP +.B CLOSE_RANGE_UNSHARE +unshares the range of file descriptors from any other processes, +before closing them, +avoiding races with other threads sharing the file descriptor table. +.SH RETURN VALUE +On success, +.BR close_range () +returns 0. +On error, \-1 is returned and +.I errno +is set to indicate the cause of the error. +.SH ERRORS +.TP +.B EINVAL +.I flags +is not valid, or +.I first +is greater than +.IR last . +.PP +The following can occur with +.B CLOSE_RANGE_UNSHARE +(when constructing the new descriptor table): +.TP +.B EMFILE +The per-process limit on the number of open file descriptors has been reached +(see the description of +.B RLIMIT_NOFILE +in +.BR getrlimit (2)). +.TP +.B ENOMEM +Insufficient kernel memory was available. +.SH VERSIONS +.BR close_range () +first appeared in Linux 5.9. +.SH CONFORMING TO +.BR close_range () +is a nonstandard function that is also present on FreeBSD. +.SH NOTES +Glibc does not provide a wrapper for this system call; call it using +.BR syscall (2). +.SS Closing all open file descriptors +.\" 278a5fbaed89dacd04e9d052f4594ffd0e0585de +To avoid blindly closing file descriptors +in the range of possible file descriptors, +this is sometimes implemented (on Linux) +by listing open file descriptors in +.I /proc/self/fd/ +and calling +.BR close (2) +on each one. +.BR close_range () +can take care of this without requiring +.I /proc +and within a single system call, +which provides significant performance benefits. +.SS Closing file descriptors before exec +.\" 60997c3d45d9a67daf01c56d805ae4fec37e0b
[PATCH v4] close_range.2: new page documenting close_range(2)
This documents close_range(2) based on information in 278a5fbaed89dacd04e9d052f4594ffd0e0585de, 60997c3d45d9a67daf01c56d805ae4fec37e0bd8, and 582f1fb6b721facf04848d2ca57f34468da1813e. Signed-off-by: Stephen Kitt --- V4: sort flags alphabetically move commit references inside the corresponding section more semantic newlines unformat numeric constants more formatting for function references escape C backslashes C99 loop indices V3: fix synopsis overflow copy notes from membarrier.2 re the lack of wrapper semantic newlines drop non-standard "USE CASES" section heading add code example V2: unsigned int to match the kernel declarations groff and grammar tweaks CLOSE_RANGE_UNSHARE unshares *and* closes Explain that EMFILE and ENOMEM can occur with C_R_U "Conforming to" phrasing Detailed explanation of CLOSE_RANGE_UNSHARE Reading /proc isn't common man2/close_range.2 | 267 + 1 file changed, 267 insertions(+) create mode 100644 man2/close_range.2 diff --git a/man2/close_range.2 b/man2/close_range.2 new file mode 100644 index 0..a8590902b --- /dev/null +++ b/man2/close_range.2 @@ -0,0 +1,267 @@ +.\" Copyright (c) 2020 Stephen Kitt +.\" +.\" %%%LICENSE_START(VERBATIM) +.\" Permission is granted to make and distribute verbatim copies of this +.\" manual provided the copyright notice and this permission notice are +.\" preserved on all copies. +.\" +.\" Permission is granted to copy and distribute modified versions of this +.\" manual under the conditions for verbatim copying, provided that the +.\" entire resulting derived work is distributed under the terms of a +.\" permission notice identical to this one. +.\" +.\" Since the Linux kernel and libraries are constantly changing, this +.\" manual page may be incorrect or out-of-date. The author(s) assume no +.\" responsibility for errors or omissions, or for damages resulting from +.\" the use of the information contained herein. The author(s) may not +.\" have taken the same level of care in the production of this manual, +.\" which is licensed free of charge, as they might when working +.\" professionally. +.\" +.\" Formatted or processed versions of this manual, if unaccompanied by +.\" the source, must acknowledge the copyright and authors of this work. +.\" %%%LICENSE_END +.\" +.TH CLOSE_RANGE 2 2020-12-08 "Linux" "Linux Programmer's Manual" +.SH NAME +close_range \- close all file descriptors in a given range +.SH SYNOPSIS +.nf +.B #include +.PP +.BI "int close_range(unsigned int " first ", unsigned int " last , +.BI "unsigned int " flags ); +.fi +.PP +.IR Note : +There is no glibc wrapper for this system call; see NOTES. +.SH DESCRIPTION +The +.BR close_range () +system call closes all open file descriptors from +.I first +to +.I last +(included). +.PP +Errors closing a given file descriptor are currently ignored. +.PP +.I flags +can be 0 or set to one or both of the following: +.TP +.BR CLOSE_RANGE_CLOEXEC " (since Linux 5.10)" +sets the close-on-exec bit instead of +immediately closing the file descriptors. +.TP +.B CLOSE_RANGE_UNSHARE +unshares the range of file descriptors from any other processes, +before closing them, +avoiding races with other threads sharing the file descriptor table. +.SH RETURN VALUE +On success, +.BR close_range () +returns 0. +On error, \-1 is returned and +.I errno +is set to indicate the cause of the error. +.SH ERRORS +.TP +.B EINVAL +.I flags +is not valid, or +.I first +is greater than +.IR last . +.PP +The following can occur with +.B CLOSE_RANGE_UNSHARE +(when constructing the new descriptor table): +.TP +.B EMFILE +The per-process limit on the number of open file descriptors has been reached +(see the description of +.B RLIMIT_NOFILE +in +.BR getrlimit (2)). +.TP +.B ENOMEM +Insufficient kernel memory was available. +.SH VERSIONS +.BR close_range () +first appeared in Linux 5.9. +.SH CONFORMING TO +.BR close_range () +is a nonstandard function that is also present on FreeBSD. +.SH NOTES +Glibc does not provide a wrapper for this system call; call it using +.BR syscall (2). +.SS Closing all open file descriptors +.\" 278a5fbaed89dacd04e9d052f4594ffd0e0585de +To avoid blindly closing file descriptors +in the range of possible file descriptors, +this is sometimes implemented (on Linux) +by listing open file descriptors in +.I /proc/self/fd/ +and calling +.BR close (2) +on each one. +.BR close_range () +can take care of this without requiring +.I /proc +and within a single system call, +which provides significant performance benefits. +.SS Closing file descriptors before exec +.\" 60997c3d45d9a67daf01c56d805ae4fec37e0bd8 +File descriptors can be closed safely using +.PP +.in +
Re: [PATCH v3] close_range.2: new page documenting close_range(2)
Hi Alex, On Sat, 19 Dec 2020 15:00:00 +0100, "Alejandro Colomar (man-pages)" wrote: > On 12/18/20 5:58 PM, Stephen Kitt wrote: [...] > > +This program executes the command given on its command-line after > > +opening the files listed after the command, > > +and then using > > s/using/uses/ It’s the same form as “opening”: “after opening ... and then using”. The overall sequence is “open”, “close_range”, “execve”. Regards, Stephen pgpemG_x_d2wU.pgp Description: OpenPGP digital signature
Re: [PATCH v3] close_range.2: new page documenting close_range(2)
Hi Alex, On Sat, 19 Dec 2020 15:00:00 +0100, "Alejandro Colomar (man-pages)" wrote: > Please see some comments below. > It's looking good ;) Thanks for your review and patience! > On 12/18/20 5:58 PM, Stephen Kitt wrote: > > This documents close_range(2) based on information in > > 278a5fbaed89dacd04e9d052f4594ffd0e0585de, > > 60997c3d45d9a67daf01c56d805ae4fec37e0bd8, and > > 582f1fb6b721facf04848d2ca57f34468da1813e. > > > > Signed-off-by: Stephen Kitt > > --- > > V3: fix synopsis overflow > > copy notes from membarrier.2 re the lack of wrapper > > semantic newlines > > drop non-standard "USE CASES" section heading > > add code example > > > > V2: unsigned int to match the kernel declarations > > groff and grammar tweaks > > CLOSE_RANGE_UNSHARE unshares *and* closes > > Explain that EMFILE and ENOMEM can occur with C_R_U > > "Conforming to" phrasing > > Detailed explanation of CLOSE_RANGE_UNSHARE > > Reading /proc isn't common > > > > man2/close_range.2 | 266 + > > 1 file changed, 266 insertions(+) > > create mode 100644 man2/close_range.2 > > > > diff --git a/man2/close_range.2 b/man2/close_range.2 > > new file mode 100644 > > index 0..f8f2053ac > > --- /dev/null > > +++ b/man2/close_range.2 > > @@ -0,0 +1,266 @@ > > +.\" Copyright (c) 2020 Stephen Kitt > > +.\" > > +.\" %%%LICENSE_START(VERBATIM) > > +.\" Permission is granted to make and distribute verbatim copies of this > > +.\" manual provided the copyright notice and this permission notice are > > +.\" preserved on all copies. > > +.\" > > +.\" Permission is granted to copy and distribute modified versions of > > this +.\" manual under the conditions for verbatim copying, provided that > > the +.\" entire resulting derived work is distributed under the terms of a > > +.\" permission notice identical to this one. > > +.\" > > +.\" Since the Linux kernel and libraries are constantly changing, this > > +.\" manual page may be incorrect or out-of-date. The author(s) assume no > > +.\" responsibility for errors or omissions, or for damages resulting from > > +.\" the use of the information contained herein. The author(s) may not > > +.\" have taken the same level of care in the production of this manual, > > +.\" which is licensed free of charge, as they might when working > > +.\" professionally. > > +.\" > > +.\" Formatted or processed versions of this manual, if unaccompanied by > > +.\" the source, must acknowledge the copyright and authors of this work. > > +.\" %%%LICENSE_END > > +.\" > > +.TH CLOSE_RANGE 2 2020-12-08 "Linux" "Linux Programmer's Manual" > > +.SH NAME > > +close_range \- close all file descriptors in a given range > > +.SH SYNOPSIS > > +.nf > > +.B #include > > +.PP > > +.BI "int close_range(unsigned int " first ", unsigned int " last , > > +.BI "unsigned int " flags ); > > +.fi > > +.PP > > +.IR Note : > > +There is no glibc wrapper for this system call; see NOTES. > > +.SH DESCRIPTION > > +The > > +.BR close_range () > > +system call closes all open file descriptors from > > +.I first > > +to > > +.I last > > +(included). > > +.PP > > +Errors closing a given file descriptor are currently ignored. > > +.PP > > +.I flags > > +can be 0 or set to one or both of the following: > > +.TP > > +.B CLOSE_RANGE_UNSHARE > > +unshares the range of file descriptors from any other processes, > > +before closing them, > > +avoiding races with other threads sharing the file descriptor table. > > +.TP > > +.BR CLOSE_RANGE_CLOEXEC " (since Linux 5.10)" > > |sort > > I prefer alphabetic order rather than adding new items at the bottom. > When lists grow, it becomes difficult to find what you're looking for. > > CLOEXEC should go before UNSHARE. That makes sense. > > +sets the close-on-exec bit instead of immediately closing the file > > +descriptors. > > [ > sets the close-on-exec bit instead of > immediately closing the file descriptors. > ] Is this for semantic reasons, or to balance the lines and make them easier to read in the roff source? > > +.SH RETURN VALUE > > +On success, > > +.BR close_range () > > +r
[PATCH v3] close_range.2: new page documenting close_range(2)
This documents close_range(2) based on information in 278a5fbaed89dacd04e9d052f4594ffd0e0585de, 60997c3d45d9a67daf01c56d805ae4fec37e0bd8, and 582f1fb6b721facf04848d2ca57f34468da1813e. Signed-off-by: Stephen Kitt --- V3: fix synopsis overflow copy notes from membarrier.2 re the lack of wrapper semantic newlines drop non-standard "USE CASES" section heading add code example V2: unsigned int to match the kernel declarations groff and grammar tweaks CLOSE_RANGE_UNSHARE unshares *and* closes Explain that EMFILE and ENOMEM can occur with C_R_U "Conforming to" phrasing Detailed explanation of CLOSE_RANGE_UNSHARE Reading /proc isn't common man2/close_range.2 | 266 + 1 file changed, 266 insertions(+) create mode 100644 man2/close_range.2 diff --git a/man2/close_range.2 b/man2/close_range.2 new file mode 100644 index 0..f8f2053ac --- /dev/null +++ b/man2/close_range.2 @@ -0,0 +1,266 @@ +.\" Copyright (c) 2020 Stephen Kitt +.\" +.\" %%%LICENSE_START(VERBATIM) +.\" Permission is granted to make and distribute verbatim copies of this +.\" manual provided the copyright notice and this permission notice are +.\" preserved on all copies. +.\" +.\" Permission is granted to copy and distribute modified versions of this +.\" manual under the conditions for verbatim copying, provided that the +.\" entire resulting derived work is distributed under the terms of a +.\" permission notice identical to this one. +.\" +.\" Since the Linux kernel and libraries are constantly changing, this +.\" manual page may be incorrect or out-of-date. The author(s) assume no +.\" responsibility for errors or omissions, or for damages resulting from +.\" the use of the information contained herein. The author(s) may not +.\" have taken the same level of care in the production of this manual, +.\" which is licensed free of charge, as they might when working +.\" professionally. +.\" +.\" Formatted or processed versions of this manual, if unaccompanied by +.\" the source, must acknowledge the copyright and authors of this work. +.\" %%%LICENSE_END +.\" +.TH CLOSE_RANGE 2 2020-12-08 "Linux" "Linux Programmer's Manual" +.SH NAME +close_range \- close all file descriptors in a given range +.SH SYNOPSIS +.nf +.B #include +.PP +.BI "int close_range(unsigned int " first ", unsigned int " last , +.BI "unsigned int " flags ); +.fi +.PP +.IR Note : +There is no glibc wrapper for this system call; see NOTES. +.SH DESCRIPTION +The +.BR close_range () +system call closes all open file descriptors from +.I first +to +.I last +(included). +.PP +Errors closing a given file descriptor are currently ignored. +.PP +.I flags +can be 0 or set to one or both of the following: +.TP +.B CLOSE_RANGE_UNSHARE +unshares the range of file descriptors from any other processes, +before closing them, +avoiding races with other threads sharing the file descriptor table. +.TP +.BR CLOSE_RANGE_CLOEXEC " (since Linux 5.10)" +sets the close-on-exec bit instead of immediately closing the file +descriptors. +.SH RETURN VALUE +On success, +.BR close_range () +returns 0. +On error, \-1 is returned and +.I errno +is set to indicate the cause of the error. +.SH ERRORS +.TP +.B EINVAL +.I flags +is not valid, or +.I first +is greater than +.IR last . +.PP +The following can occur with +.B CLOSE_RANGE_UNSHARE +(when constructing the new descriptor table): +.TP +.B EMFILE +The per-process limit on the number of open file descriptors has been reached +(see the description of +.B RLIMIT_NOFILE +in +.BR getrlimit (2)). +.TP +.B ENOMEM +Insufficient kernel memory was available. +.SH VERSIONS +.BR close_range () +first appeared in Linux 5.9. +.SH CONFORMING TO +.BR close_range () +is a nonstandard function that is also present on FreeBSD. +.SH NOTES +Glibc does not provide a wrapper for this system call; call it using +.BR syscall (2). +.\" 278a5fbaed89dacd04e9d052f4594ffd0e0585de +.SS Closing all open file descriptors +To avoid blindly closing file descriptors in the range of possible +file descriptors, +this is sometimes implemented (on Linux) by listing open file +descriptors in +.I /proc/self/fd/ +and calling +.BR close (2) +on each one. +.BR close_range () +can take care of this without requiring +.I /proc +and with a single system call, +which provides significant performance benefits. +.\" 60997c3d45d9a67daf01c56d805ae4fec37e0bd8 +.SS Closing file descriptors before exec +File descriptors can be closed safely using +.PP +.in +4n +.EX +/* we don't want anything past stderr here */ +close_range(3, ~0U, CLOSE_RANGE_UNSHARE); +execve(); +.EE +.in +.PP +.B CLOSE_RANGE_UNSHARE +is conceptually equivalent to +.PP +.in +4n +.EX +unshare(CLONE_FILES); +close_range(firs
Re: Ping: [patch] close_range.2: new page documenting close_range(2)
Hi Alex, Le 18/12/2020 11:12, Alejandro Colomar (man-pages) a écrit : Linux 5.10 has been recently released. Do you have any updates for this patch? Yes, I have a v3 in preparation, with _CLOEXEC and a code example. I'll wrap it up today. Regards, Stephen
Re: [PATCH v2] close_range.2: new page documenting close_range(2)
Hi Alejandro, On Thu, 10 Dec 2020 01:24:28 +0100, "Alejandro Colomar (man-pages)" wrote: > A few more comments below. Thanks for all the detailed reviews! (Same goes to everyone reviewing this yesterday, thanks for helping turn my not-so-great attempt into something really useful.) > Michael, please have a look at them too. > > Christian, do you have any program that you used to test the syscall > that could be added as an example program to the page? I have some example code that I could turn into an example program, but I imagine Christian has better code than me ;-). > > +.BI "int close_range(unsigned int " first ", unsigned int " last ", > > unsigned int " flags ); > > This line overflows an 80-col terminal. Fix: > > .BI "int close_range(unsigned int " first ", unsigned int " last , > .BI "unsigned int " flags ); Noted, thanks. > > +.fi > > Please, add a note here that there is no wrapper for this syscall, > as in other syscalls without wrapper (see membarrier(2) as an example). > That way it's easier to grep, if all pages have the same notice. Ah, right, I used pidfd_open(2) as an example, where it's mentioned later. > Please use semantic newlines. > See man-pages(7)::STYLE GUIDE::Use semantic newlines Noted, thanks. > > +.SH NOTES > > +Currently, there is no glibc wrapper for this system call; call it using > > +.BR syscall (2). > > I can see that this notice is also present on a few pages, > but the one in membarrier(2) is more extended. > > Please, copy the notices from membarrier(2). > There's one in SYNOPSIS, and one in NOTES. Will do! > > +.SH USE CASES > > This section is unconventional. Please move that text to one of the > traditional sections. I think DESCRIPTION would be the best place for this. > > For a list of the traditional sections, > see man-pages(7)::DESCRIPTION::Sections within a manual page Indeed; it grew out of pidfd_open's "Use cases" section but making all this part of the description would be much better. I'll wait and see if there are other comments, and send a v3 addressing all the above. Regards, Stephen pgpg1XLDNOYXS.pgp Description: OpenPGP digital signature
[PATCH v2] close_range.2: new page documenting close_range(2)
This documents close_range(2) based on information in 278a5fbaed89dacd04e9d052f4594ffd0e0585de and 60997c3d45d9a67daf01c56d805ae4fec37e0bd8. Signed-off-by: Stephen Kitt --- V2: unsigned int to match the kernel declarations groff and grammar tweaks CLOSE_RANGE_UNSHARE unshares *and* closes Explain that EMFILE and ENOMEM can occur with C_R_U "Conforming to" phrasing Detailed explanation of CLOSE_RANGE_UNSHARE Reading /proc isn't common man2/close_range.2 | 138 + 1 file changed, 138 insertions(+) create mode 100644 man2/close_range.2 diff --git a/man2/close_range.2 b/man2/close_range.2 new file mode 100644 index 0..403142b33 --- /dev/null +++ b/man2/close_range.2 @@ -0,0 +1,138 @@ +.\" Copyright (c) 2020 Stephen Kitt +.\" +.\" %%%LICENSE_START(VERBATIM) +.\" Permission is granted to make and distribute verbatim copies of this +.\" manual provided the copyright notice and this permission notice are +.\" preserved on all copies. +.\" +.\" Permission is granted to copy and distribute modified versions of this +.\" manual under the conditions for verbatim copying, provided that the +.\" entire resulting derived work is distributed under the terms of a +.\" permission notice identical to this one. +.\" +.\" Since the Linux kernel and libraries are constantly changing, this +.\" manual page may be incorrect or out-of-date. The author(s) assume no +.\" responsibility for errors or omissions, or for damages resulting from +.\" the use of the information contained herein. The author(s) may not +.\" have taken the same level of care in the production of this manual, +.\" which is licensed free of charge, as they might when working +.\" professionally. +.\" +.\" Formatted or processed versions of this manual, if unaccompanied by +.\" the source, must acknowledge the copyright and authors of this work. +.\" %%%LICENSE_END +.\" +.TH CLOSE_RANGE 2 2020-12-08 "Linux" "Linux Programmer's Manual" +.SH NAME +close_range \- close all file descriptors in a given range +.SH SYNOPSIS +.nf +.B #include +.PP +.BI "int close_range(unsigned int " first ", unsigned int " last ", unsigned int " flags ); +.fi +.SH DESCRIPTION +The +.BR close_range () +system call closes all open file descriptors from +.I first +to +.I last +(included). +.PP +Errors closing a given file descriptor are currently ignored. +.PP +.I flags +can be set to +.B CLOSE_RANGE_UNSHARE +to unshare the range of file descriptors from any other processes, +before closing them, avoiding races with other threads sharing the +file descriptor table. +.SH RETURN VALUE +On success, +.BR close_range () +returns 0. +On error, \-1 is returned and +.I errno +is set to indicate the cause of the error. +.SH ERRORS +.TP +.B EINVAL +.I flags +is not valid, or +.I first +is greater than +.IR last . +.PP +The following can occur with +.B CLOSE_RANGE_UNSHARE +(when constructing the new descriptor table): +.TP +.B EMFILE +The per-process limit on the number of open file descriptors has been reached +(see the description of +.B RLIMIT_NOFILE +in +.BR getrlimit (2)). +.TP +.B ENOMEM +Insufficient kernel memory was available. +.SH VERSIONS +.BR close_range () +first appeared in Linux 5.9. +.SH CONFORMING TO +.BR close_range () +is a nonstandard function that is also present on FreeBSD. +.SH NOTES +Currently, there is no glibc wrapper for this system call; call it using +.BR syscall (2). +.PP +.B CLOSE_RANGE_UNSHARE +is conceptually equivalent to +.PP +.in +4n +.EX +unshare(CLONE_FILES); +close_range(first, last, 0); +.EE +.in +.PP +but can be more efficient: if the unshared range extends past the +current maximum number of file descriptors allocated in the caller's +file descriptor table (the common case when +.I last +is +.BR ~0U ), +the kernel will unshare a new file descriptor +table for the caller up to +.IR first . +This avoids subsequent close calls entirely; the whole operation is +complete once the table is unshared. +.SH USE CASES +.\" 278a5fbaed89dacd04e9d052f4594ffd0e0585de +.\" 60997c3d45d9a67daf01c56d805ae4fec37e0bd8 +.SS Closing file descriptors before exec +File descriptors can be closed safely using +.PP +.in +4n +.EX +/* we don't want anything past stderr here */ +close_range(3, ~0U, CLOSE_RANGE_UNSHARE); +execve(); +.EE +.in +.SS Closing all open file descriptors +To avoid blindly closing file descriptors in the range of possible +file descriptors, this is sometimes implemented (on Linux) by listing +open file descriptors in +.I /proc/self/fd/ +and calling +.BR close (2) +on each one. +.BR close_range () +can take care of this without requiring +.I /proc +and with a single system call, which provides significant performance +benefits. +.SH SEE ALSO +.BR close (2) base-commit: b5dae3959625f5ff378e9edf9139057d1c06bb55 -- 2.20.1
Re: [patch] close_range.2: new page documenting close_range(2)
Le 09/12/2020 10:40, Christian Brauner a écrit : On Wed, Dec 09, 2020 at 09:50:38AM +0100, Michael Kerrisk (man-pages) wrote: > +.PP > +.I flags > +can be set to > +.B CLOSE_RANGE_UNSHARE > +to unshare the range of file descriptors from any other processes, > +.I instead > +of closing them. Really "instead of closing them"? I had supposed that rather that this should be "before closing them". That's also how the kernel code reads to me, from a quick glance. It's also mentioned in the commit message. Basically setting CLOSE_RANGE_UNSHARE is equivalent to: unshare(CLONE_FILES); close_range(, ); Yes, I got that mixed up, thanks for the clarification! I'll send a v2 addressing the review comments later today. Regards, Stephen
[patch] close_range.2: new page documenting close_range(2)
This documents close_range(2) based on information in 278a5fbaed89dacd04e9d052f4594ffd0e0585de and 60997c3d45d9a67daf01c56d805ae4fec37e0bd8. Signed-off-by: Stephen Kitt --- man2/close_range.2 | 112 + 1 file changed, 112 insertions(+) create mode 100644 man2/close_range.2 diff --git a/man2/close_range.2 b/man2/close_range.2 new file mode 100644 index 0..62167d9b0 --- /dev/null +++ b/man2/close_range.2 @@ -0,0 +1,112 @@ +.\" Copyright (c) 2020 Stephen Kitt +.\" +.\" %%%LICENSE_START(VERBATIM) +.\" Permission is granted to make and distribute verbatim copies of this +.\" manual provided the copyright notice and this permission notice are +.\" preserved on all copies. +.\" +.\" Permission is granted to copy and distribute modified versions of this +.\" manual under the conditions for verbatim copying, provided that the +.\" entire resulting derived work is distributed under the terms of a +.\" permission notice identical to this one. +.\" +.\" Since the Linux kernel and libraries are constantly changing, this +.\" manual page may be incorrect or out-of-date. The author(s) assume no +.\" responsibility for errors or omissions, or for damages resulting from +.\" the use of the information contained herein. The author(s) may not +.\" have taken the same level of care in the production of this manual, +.\" which is licensed free of charge, as they might when working +.\" professionally. +.\" +.\" Formatted or processed versions of this manual, if unaccompanied by +.\" the source, must acknowledge the copyright and authors of this work. +.\" %%%LICENSE_END +.\" +.TH CLOSE_RANGE 2 2020-12-08 "Linux" "Linux Programmer's Manual" +.SH NAME +close_range \- close all file descriptors in a given range +.SH SYNOPSIS +.nf +.B #include +.PP +.BI "int close_range(int " first ", int " last ", unsigned int " flags ); +.fi +.SH DESCRIPTION +The +.BR close_range () +system call closes all open file descriptors from +.I first +to +.IR last +(included). +.PP +Errors closing a given file descriptor are currently ignored. +.PP +.I flags +can be set to +.B CLOSE_RANGE_UNSHARE +to unshare the range of file descriptors from any other processes, +.I instead +of closing them. +.SH RETURN VALUE +On success, +.BR close_range () +return 0. +On error, \-1 is returned and +.I errno +is set to indicate the cause of the error. +.SH ERRORS +.TP +.B EINVAL +.I flags +is not valid, or +.I first +is greater than +.IR last . +.TP +.B EMFILE +The per-process limit on the number of open file descriptors has been reached +(see the description of +.BR RLIMIT_NOFILE +in +.BR getrlimit (2)). +.TP +.B ENOMEM +Insufficient kernel memory was available. +.SH VERSIONS +.BR close_range () +first appeared in Linux 5.9. +.SH CONFORMING TO +.BR close_range () +is available on Linux and FreeBSD. +.SH NOTES +Currently, there is no glibc wrapper for this system call; call it using +.BR syscall (2). +.SH USE CASES +.\" 278a5fbaed89dacd04e9d052f4594ffd0e0585de +.\" 60997c3d45d9a67daf01c56d805ae4fec37e0bd8 +.SS Closing file descriptors before exec +File descriptors can be closed safely using +.PP +.in +4n +.EX +/* we don't want anything past stderr here */ +close_range(3, ~0U, CLOSE_RANGE_UNSHARE); +execve(); +.EE +.in +.PP +.SS Closing all open file descriptors +This is commonly implemented (on Linux) by listing open file +descriptors in +.B /proc/self/fd/ +and calling +.BR close (2) +on each one. +.BR close_range () +can take care of this without requiring +.B /proc +and with a single system call, which provides significant performance +benefits. +.SH SEE ALSO +.BR close (2) base-commit: b5dae3959625f5ff378e9edf9139057d1c06bb55 -- 2.20.1
[PATCH] docs: clean up sysctl/kernel: titles, version
This cleans up a few titles with extra colons, and removes the reference to kernel 2.2. The docs don't yet cover *all* of 5.10 or 5.11, but I think they're close enough. Most entries are documented, and have been checked against current kernels. Signed-off-by: Stephen Kitt --- Documentation/admin-guide/sysctl/kernel.rst | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst index d4b32cc32bb7..7d53146798c0 100644 --- a/Documentation/admin-guide/sysctl/kernel.rst +++ b/Documentation/admin-guide/sysctl/kernel.rst @@ -14,7 +14,7 @@ For general info and legal blurb, please look in :doc:`index`. -- This file contains documentation for the sysctl files in -``/proc/sys/kernel/`` and is valid for Linux kernel version 2.2. +``/proc/sys/kernel/``. The files in this directory can be used to tune and monitor miscellaneous and general things in the operation of the Linux @@ -1095,8 +1095,8 @@ Enables/disables scheduler statistics. Enabling this feature incurs a small amount of overhead in the scheduler but is useful for debugging and performance tuning. -sched_util_clamp_min: -= +sched_util_clamp_min + Max allowed *minimum* utilization. @@ -1106,8 +1106,8 @@ It means that any requested uclamp.min value cannot be greater than sched_util_clamp_min, i.e., it is restricted to the range [0:sched_util_clamp_min]. -sched_util_clamp_max: -= +sched_util_clamp_max + Max allowed *maximum* utilization. @@ -1117,8 +1117,8 @@ It means that any requested uclamp.max value cannot be greater than sched_util_clamp_max, i.e., it is restricted to the range [0:sched_util_clamp_max]. -sched_util_clamp_min_rt_default: - +sched_util_clamp_min_rt_default +=== By default Linux is tuned for performance. Which means that RT tasks always run at the highest frequency and most capable (highest capacity) CPU (in base-commit: 0477e92881850d44910a7e94fc2c46f96faa131f -- 2.20.1
Re: [PATCH] docs: rewrite admin-guide/sysctl/abi.rst
On Wed, 16 Sep 2020 12:43:10 -0600, Jonathan Corbet wrote: > On Fri, 11 Sep 2020 21:01:52 +0200 > Stephen Kitt wrote: > > Following the structure used in sysctl/kernel.rst, this updates > > abi.rst to use ReStructured Text more fully and updates the entries to > > match current kernels: > > > > * the list of files is now the table of contents; > > * links are used to point to other documentation and other sections; > > * all the existing entries are no longer present, so this removes > > them; > > * document vsyscall32. > > > > Mentions of the kernel version are dropped. Since the document is > > entirely rewritten, I've replaced the copyright statement. > > > > Signed-off-by: Stephen Kitt > > Replacing a copyright makes me a little nervous, but I guess that is OK > here since everything else is replaced too. I hesitated too, but after checking carefully that the original contents were all gone, decided it sort of made sense... I could just drop that line too, I’m not sure there’s much point in documentation that’s supposed to be collectively maintained (and authorship is determined by the commits anyway). > Could I trouble you, though, for a version that adds an SPDX line at the > top while you're at it? No problem, v2 incoming. While I’m at it, might I ask if it would be possible to carry https://lkml.org/lkml/2020/8/12/181 (sort-of-re-submitted in https://lkml.org/lkml/2020/9/11/1079) in the docs tree? It fixes a docs commit... Regards, Stephen pgpQnzVuu9tcv.pgp Description: OpenPGP digital signature
[PATCH v2] docs: rewrite admin-guide/sysctl/abi.rst
Following the structure used in sysctl/kernel.rst, this updates abi.rst to use ReStructured Text more fully and updates the entries to match current kernels: * the list of files is now the table of contents; * links are used to point to other documentation and other sections; * all the existing entries are no longer present, so this removes them; * document vsyscall32. Mentions of the kernel version are dropped. Since the document is entirely rewritten, I've replaced the copyright statement. Signed-off-by: Stephen Kitt --- Changes since v1: - added SPDX-License-Identifier Documentation/admin-guide/sysctl/abi.rst | 73 +++- 1 file changed, 20 insertions(+), 53 deletions(-) diff --git a/Documentation/admin-guide/sysctl/abi.rst b/Documentation/admin-guide/sysctl/abi.rst index 599bcde7f0b7..ac87eafdb54f 100644 --- a/Documentation/admin-guide/sysctl/abi.rst +++ b/Documentation/admin-guide/sysctl/abi.rst @@ -1,67 +1,34 @@ +.. SPDX-License-Identifier: GPL-2.0+ + Documentation for /proc/sys/abi/ -kernel version 2.6.0.test2 +.. See scripts/check-sysctl-docs to keep this up to date: +.. scripts/check-sysctl-docs -vtable="abi" \ +.. Documentation/admin-guide/sysctl/abi.rst \ +.. $(git grep -l register_sysctl_) -Copyright (c) 2003, Fabian Frederick +Copyright (c) 2020, Stephen Kitt -For general info: index.rst. +For general info, see :doc:`index`. -- -This path is binary emulation relevant aka personality types aka abi. -When a process is executed, it's linked to an exec_domain whose -personality is defined using values available from /proc/sys/abi. -You can find further details about abi in include/linux/personality.h. - -Here are the files featuring in 2.6 kernel: - -- defhandler_coff -- defhandler_elf -- defhandler_lcall7 -- defhandler_libcso -- fake_utsname -- trace - -defhandler_coff - -defined value: - PER_SCOSVR3:: - - 0x0003 | STICKY_TIMEOUTS | WHOLE_SECONDS | SHORT_INODE - -defhandler_elf --- - -defined value: - PER_LINUX:: - - 0 - -defhandler_lcall7 -- - -defined value : - PER_SVR4:: - - 0x0001 | STICKY_TIMEOUTS | MMAP_PAGE_ZERO, - -defhandler_libsco -- - -defined value: - PER_SVR4:: +The files in ``/proc/sys/abi`` can be used to see and modify +ABI-related settings. - 0x0001 | STICKY_TIMEOUTS | MMAP_PAGE_ZERO, +Currently, these files might (depending on your configuration) +show up in ``/proc/sys/kernel``: -fake_utsname - +.. contents:: :local: -Unused +vsyscall32 (x86) + -trace -- +Determines whether the kernels maps a vDSO page into 32-bit processes; +can be set to 1 to enable, or 0 to disable. Defaults to enabled if +``CONFIG_COMPAT_VDSO`` is set, disabled otherwide. -Unused +This controls the same setting as the ``vdso32`` kernel boot +parameter. base-commit: a070991fe9d1b6d97eb3cf24a2a6ed5f8e7c0169 -- 2.20.1
Re: [PATCH] Fix references to nommu-mmap.rst
Hi,
This is still relevant as far as I can tell; master and docs-next still have
references to the old nommu-mmap.rst.
Regards,
Stephen
On Wed, 12 Aug 2020 11:22:30 +0200, Stephen Kitt wrote:
> nommu-mmap.rst was moved to Documentation/admin-guide/mm; this patch
> updates the remaining stale references to Documentation/mm.
>
> Fixes: 800c02f5d030 ("docs: move nommu-mmap.txt to admin-guide and rename to
> ReST")
> Signed-off-by: Stephen Kitt
> ---
> init/Kconfig | 2 +-
> mm/Kconfig | 2 +-
> mm/nommu.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 2dd5531dae98..8d5fefd1f229 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1957,7 +1957,7 @@ config MMAP_ALLOW_UNINITIALIZED
> userspace. Since that isn't generally a problem on no-MMU systems,
> it is normally safe to say Y here.
>
> - See Documentation/mm/nommu-mmap.rst for more information.
> + See Documentation/admin-guide/mm/nommu-mmap.rst for more information.
>
> config SYSTEM_DATA_VERIFICATION
> def_bool n
> diff --git a/mm/Kconfig b/mm/Kconfig
> index d41f3fa7e923..29e239497718 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -387,7 +387,7 @@ config NOMMU_INITIAL_TRIM_EXCESS
> This option specifies the initial value of this option. The default
> of 1 says that all excess pages should be trimmed.
>
> - See Documentation/mm/nommu-mmap.rst for more information.
> + See Documentation/admin-guide/mm/nommu-mmap.rst for more information.
>
> config TRANSPARENT_HUGEPAGE
> bool "Transparent Hugepage Support"
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 64539971188b..e8e2c5bb6f0a 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -5,7 +5,7 @@
> * Replacement code for mm functions to support CPU's that don't
> * have any form of memory management unit (thus no virtual memory).
> *
> - * See Documentation/mm/nommu-mmap.rst
> + * See Documentation/admin-guide/mm/nommu-mmap.rst
> *
> * Copyright (c) 2004-2008 David Howells
> * Copyright (c) 2000-2003 David McCullough
>
> base-commit: e176b7a3054eef44a22f6ca3d14168dcf9bad21e
> --
> 2.20.1
>
pgpdqpHX86BGS.pgp
Description: OpenPGP digital signature
[PATCH] docs: rewrite admin-guide/sysctl/abi.rst
Following the structure used in sysctl/kernel.rst, this updates abi.rst to use ReStructured Text more fully and updates the entries to match current kernels: * the list of files is now the table of contents; * links are used to point to other documentation and other sections; * all the existing entries are no longer present, so this removes them; * document vsyscall32. Mentions of the kernel version are dropped. Since the document is entirely rewritten, I've replaced the copyright statement. Signed-off-by: Stephen Kitt --- Documentation/admin-guide/sysctl/abi.rst | 71 ++-- 1 file changed, 18 insertions(+), 53 deletions(-) diff --git a/Documentation/admin-guide/sysctl/abi.rst b/Documentation/admin-guide/sysctl/abi.rst index 599bcde7f0b7..d88b48db8bf9 100644 --- a/Documentation/admin-guide/sysctl/abi.rst +++ b/Documentation/admin-guide/sysctl/abi.rst @@ -2,66 +2,31 @@ Documentation for /proc/sys/abi/ -kernel version 2.6.0.test2 +.. See scripts/check-sysctl-docs to keep this up to date: +.. scripts/check-sysctl-docs -vtable="abi" \ +.. Documentation/admin-guide/sysctl/abi.rst \ +.. $(git grep -l register_sysctl_) -Copyright (c) 2003, Fabian Frederick +Copyright (c) 2020, Stephen Kitt -For general info: index.rst. +For general info, see :doc:`index`. -- -This path is binary emulation relevant aka personality types aka abi. -When a process is executed, it's linked to an exec_domain whose -personality is defined using values available from /proc/sys/abi. -You can find further details about abi in include/linux/personality.h. +The files in ``/proc/sys/abi`` can be used to see and modify +ABI-related settings. -Here are the files featuring in 2.6 kernel: +Currently, these files might (depending on your configuration) +show up in ``/proc/sys/kernel``: -- defhandler_coff -- defhandler_elf -- defhandler_lcall7 -- defhandler_libcso -- fake_utsname -- trace +.. contents:: :local: -defhandler_coff +vsyscall32 (x86) + -defined value: - PER_SCOSVR3:: +Determines whether the kernels maps a vDSO page into 32-bit processes; +can be set to 1 to enable, or 0 to disable. Defaults to enabled if +``CONFIG_COMPAT_VDSO`` is set, disabled otherwide. - 0x0003 | STICKY_TIMEOUTS | WHOLE_SECONDS | SHORT_INODE - -defhandler_elf --- - -defined value: - PER_LINUX:: - - 0 - -defhandler_lcall7 -- - -defined value : - PER_SVR4:: - - 0x0001 | STICKY_TIMEOUTS | MMAP_PAGE_ZERO, - -defhandler_libsco -- - -defined value: - PER_SVR4:: - - 0x0001 | STICKY_TIMEOUTS | MMAP_PAGE_ZERO, - -fake_utsname - - -Unused - -trace -- - -Unused +This controls the same setting as the ``vdso32`` kernel boot +parameter. base-commit: 5ff4aa70bf347e13ec87697b1c732ce86060c47d -- 2.20.1
[PATCH] hwmon: use simple i2c probe function
Many hwmon drivers don't use the id information provided by the old
i2c probe function, and the remainder can easily be adapted to the new
form ("probe_new") by calling i2c_match_id explicitly.
This avoids scanning the identifier tables during probes.
Drivers which didn't use the id are converted as-is; drivers which did
are modified to call i2c_match_id() with the same level of
error-handling (if any) as before.
This patch wraps up the transition for hwmon, with four stragglers not
included in the previous large patch.
Signed-off-by: Stephen Kitt
---
drivers/hwmon/adc128d818.c | 5 ++---
drivers/hwmon/ads7828.c| 9 +
drivers/hwmon/lm87.c | 4 ++--
drivers/hwmon/w83795.c | 9 +
4 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/hwmon/adc128d818.c b/drivers/hwmon/adc128d818.c
index f9edec195c35..80511d316845 100644
--- a/drivers/hwmon/adc128d818.c
+++ b/drivers/hwmon/adc128d818.c
@@ -427,8 +427,7 @@ static int adc128_init_client(struct adc128_data *data)
return 0;
}
-static int adc128_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+static int adc128_probe(struct i2c_client *client)
{
struct device *dev = >dev;
struct regulator *regulator;
@@ -524,7 +523,7 @@ static struct i2c_driver adc128_driver = {
.name = "adc128d818",
.of_match_table = of_match_ptr(adc128_of_match),
},
- .probe = adc128_probe,
+ .probe_new = adc128_probe,
.remove = adc128_remove,
.id_table = adc128_id,
.detect = adc128_detect,
diff --git a/drivers/hwmon/ads7828.c b/drivers/hwmon/ads7828.c
index d895b73fde6f..7246198f0901 100644
--- a/drivers/hwmon/ads7828.c
+++ b/drivers/hwmon/ads7828.c
@@ -99,8 +99,9 @@ static const struct regmap_config ads2830_regmap_config = {
.val_bits = 8,
};
-static int ads7828_probe(struct i2c_client *client,
-const struct i2c_device_id *id)
+static const struct i2c_device_id ads7828_device_ids[];
+
+static int ads7828_probe(struct i2c_client *client)
{
struct device *dev = >dev;
struct ads7828_platform_data *pdata = dev_get_platdata(dev);
@@ -141,7 +142,7 @@ static int ads7828_probe(struct i2c_client *client,
chip = (enum ads7828_chips)
of_device_get_match_data(>dev);
else
- chip = id->driver_data;
+ chip = i2c_match_id(ads7828_device_ids, client)->driver_data;
/* Bound Vref with min/max values */
vref_mv = clamp_val(vref_mv, ADS7828_EXT_VREF_MV_MIN,
@@ -207,7 +208,7 @@ static struct i2c_driver ads7828_driver = {
},
.id_table = ads7828_device_ids,
- .probe = ads7828_probe,
+ .probe_new = ads7828_probe,
};
module_i2c_driver(ads7828_driver);
diff --git a/drivers/hwmon/lm87.c b/drivers/hwmon/lm87.c
index ad501ac4a594..24cb8a1c3381 100644
--- a/drivers/hwmon/lm87.c
+++ b/drivers/hwmon/lm87.c
@@ -912,7 +912,7 @@ static int lm87_init_client(struct i2c_client *client)
return 0;
}
-static int lm87_probe(struct i2c_client *client, const struct i2c_device_id
*id)
+static int lm87_probe(struct i2c_client *client)
{
struct lm87_data *data;
struct device *hwmon_dev;
@@ -994,7 +994,7 @@ static struct i2c_driver lm87_driver = {
.name = "lm87",
.of_match_table = lm87_of_match,
},
- .probe = lm87_probe,
+ .probe_new = lm87_probe,
.id_table = lm87_id,
.detect = lm87_detect,
.address_list = normal_i2c,
diff --git a/drivers/hwmon/w83795.c b/drivers/hwmon/w83795.c
index 44f68b965aec..9cff2e399f1d 100644
--- a/drivers/hwmon/w83795.c
+++ b/drivers/hwmon/w83795.c
@@ -2134,8 +2134,9 @@ static void w83795_apply_temp_config(struct w83795_data
*data, u8 config,
}
}
-static int w83795_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+static const struct i2c_device_id w83795_id[];
+
+static int w83795_probe(struct i2c_client *client)
{
int i;
u8 tmp;
@@ -2148,7 +2149,7 @@ static int w83795_probe(struct i2c_client *client,
return -ENOMEM;
i2c_set_clientdata(client, data);
- data->chip_type = id->driver_data;
+ data->chip_type = i2c_match_id(w83795_id, client)->driver_data;
data->bank = i2c_smbus_read_byte_data(client, W83795_REG_BANKSEL);
mutex_init(>update_lock);
@@ -2256,7 +2257,7 @@ static struct i2c_driver w83795_driver = {
.driver = {
.name = "w83795",
},
- .probe = w83795_probe,
+ .probe_new = w83795_probe,
.remove = w83795_remove,
.id_table = w83795_id,
--
2.25.4
[PATCH] hwmon: (f75375s) use simple i2c probe
As part of the ongoing i2c transition to the simple probe
("probe_new"), this patch uses i2c_match_id to retrieve the
driver_data for the probed device. The id parameter is thus no longer
necessary and the simple probe can be used instead.
Signed-off-by: Stephen Kitt
---
drivers/hwmon/f75375s.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c
index eb847a7d6b83..3e567be60fb1 100644
--- a/drivers/hwmon/f75375s.c
+++ b/drivers/hwmon/f75375s.c
@@ -113,8 +113,7 @@ struct f75375_data {
static int f75375_detect(struct i2c_client *client,
struct i2c_board_info *info);
-static int f75375_probe(struct i2c_client *client,
- const struct i2c_device_id *id);
+static int f75375_probe(struct i2c_client *client);
static int f75375_remove(struct i2c_client *client);
static const struct i2c_device_id f75375_id[] = {
@@ -130,7 +129,7 @@ static struct i2c_driver f75375_driver = {
.driver = {
.name = "f75375",
},
- .probe = f75375_probe,
+ .probe_new = f75375_probe,
.remove = f75375_remove,
.id_table = f75375_id,
.detect = f75375_detect,
@@ -814,8 +813,7 @@ static void f75375_init(struct i2c_client *client, struct
f75375_data *data,
}
-static int f75375_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+static int f75375_probe(struct i2c_client *client)
{
struct f75375_data *data;
struct f75375s_platform_data *f75375s_pdata =
@@ -832,7 +830,7 @@ static int f75375_probe(struct i2c_client *client,
i2c_set_clientdata(client, data);
mutex_init(>update_lock);
- data->kind = id->driver_data;
+ data->kind = i2c_match_id(f75375_id, client)->driver_data;
err = sysfs_create_group(>dev.kobj, _group);
if (err)
--
2.25.4
[PATCH] hwmon: (tmp513) use simple i2c probe
As part of the ongoing i2c transition to the simple probe
("probe_new"), this patch uses i2c_match_id to retrieve the
driver_data for the probed device. The id parameter is thus no longer
necessary and the simple probe can be used instead.
Signed-off-by: Stephen Kitt
---
drivers/hwmon/tmp513.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/hwmon/tmp513.c b/drivers/hwmon/tmp513.c
index df66e0bc1253..9f5885b0eb74 100644
--- a/drivers/hwmon/tmp513.c
+++ b/drivers/hwmon/tmp513.c
@@ -709,8 +709,7 @@ static int tmp51x_configure(struct device *dev, struct
tmp51x_data *data)
return 0;
}
-static int tmp51x_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+static int tmp51x_probe(struct i2c_client *client)
{
struct device *dev = >dev;
struct tmp51x_data *data;
@@ -724,7 +723,7 @@ static int tmp51x_probe(struct i2c_client *client,
if (client->dev.of_node)
data->id = (enum tmp51x_ids)device_get_match_data(>dev);
else
- data->id = id->driver_data;
+ data->id = i2c_match_id(tmp51x_id, client)->driver_data;
ret = tmp51x_configure(dev, data);
if (ret < 0) {
@@ -751,7 +750,7 @@ static int tmp51x_probe(struct i2c_client *client,
if (IS_ERR(hwmon_dev))
return PTR_ERR(hwmon_dev);
- dev_dbg(dev, "power monitor %s\n", id->name);
+ dev_dbg(dev, "power monitor %s\n", client->name);
return 0;
}
@@ -761,7 +760,7 @@ static struct i2c_driver tmp51x_driver = {
.name = "tmp51x",
.of_match_table = of_match_ptr(tmp51x_of_match),
},
- .probe = tmp51x_probe,
+ .probe_new = tmp51x_probe,
.id_table = tmp51x_id,
};
--
2.25.4
[PATCH] hwmon: (dme1737) use simple i2c probe
As part of the ongoing i2c transition to the simple probe
("probe_new"), this patch uses i2c_match_id to retrieve the
driver_data for the probed device. The id parameter is thus no longer
necessary and the simple probe can be used instead.
Signed-off-by: Stephen Kitt
---
drivers/hwmon/dme1737.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/dme1737.c b/drivers/hwmon/dme1737.c
index c3472b73fa79..c1e4cfb40c3d 100644
--- a/drivers/hwmon/dme1737.c
+++ b/drivers/hwmon/dme1737.c
@@ -2461,8 +2461,9 @@ static int dme1737_i2c_detect(struct i2c_client *client,
return 0;
}
-static int dme1737_i2c_probe(struct i2c_client *client,
-const struct i2c_device_id *id)
+static const struct i2c_device_id dme1737_id[];
+
+static int dme1737_i2c_probe(struct i2c_client *client)
{
struct dme1737_data *data;
struct device *dev = >dev;
@@ -2473,7 +2474,7 @@ static int dme1737_i2c_probe(struct i2c_client *client,
return -ENOMEM;
i2c_set_clientdata(client, data);
- data->type = id->driver_data;
+ data->type = i2c_match_id(dme1737_id, client)->driver_data;
data->client = client;
data->name = client->name;
mutex_init(>update_lock);
@@ -2529,7 +2530,7 @@ static struct i2c_driver dme1737_i2c_driver = {
.driver = {
.name = "dme1737",
},
- .probe = dme1737_i2c_probe,
+ .probe_new = dme1737_i2c_probe,
.remove = dme1737_i2c_remove,
.id_table = dme1737_id,
.detect = dme1737_i2c_detect,
--
2.25.4
Re: [PATCH] drivers/hwmon/adm1177.c: use simple i2c probe
On Fri, 14 Aug 2020 08:13:58 -0700, Guenter Roeck wrote:
> On Thu, Aug 13, 2020 at 06:09:58PM +0200, Stephen Kitt wrote:
> > This driver doesn't use the id information provided by the old i2c
> > probe function, so it can trivially be converted to the simple
> > ("probe_new") form.
> >
> > Signed-off-by: Stephen Kitt
>
> I'll apply the entire series, but please don't use entire path names
> as tag in the future but follow bubsystem rules.
Sorry about that, I’ll bear that in mind for the remaining hwmon patches (and
more generally).
Regards,
Stephen
pgpIjzcaAib9Y.pgp
Description: OpenPGP digital signature
[PATCH] drivers/hwmon/ltc2947-i2c.c: use simple i2c probe
This driver doesn't use the id information provided by the old i2c
probe function, so it can trivially be converted to the simple
("probe_new") form.
Signed-off-by: Stephen Kitt
---
drivers/hwmon/ltc2947-i2c.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/hwmon/ltc2947-i2c.c b/drivers/hwmon/ltc2947-i2c.c
index cf6074b110ae..ad0dfd3efbf8 100644
--- a/drivers/hwmon/ltc2947-i2c.c
+++ b/drivers/hwmon/ltc2947-i2c.c
@@ -15,8 +15,7 @@ static const struct regmap_config ltc2947_regmap_config = {
.val_bits = 8,
};
-static int ltc2947_probe(struct i2c_client *i2c,
-const struct i2c_device_id *id)
+static int ltc2947_probe(struct i2c_client *i2c)
{
struct regmap *map;
@@ -39,7 +38,7 @@ static struct i2c_driver ltc2947_driver = {
.of_match_table = ltc2947_of_match,
.pm = _pm_ops,
},
- .probe = ltc2947_probe,
+ .probe_new = ltc2947_probe,
.id_table = ltc2947_id,
};
module_i2c_driver(ltc2947_driver);
--
2.25.4
[PATCH] hwmon: use simple i2c probe function
Many hwmon drivers don't use the id information provided by the old
i2c probe function, and the remainder can easily be adapted to the new
form ("probe_new") by calling i2c_match_id explicitly.
This avoids scanning the identifier tables during probes.
Drivers which didn't use the id are converted as-is; drivers which did
are modified as follows:
* if the information in i2c_client is sufficient, that's used instead
(client->name);
* anything else is handled by calling i2c_match_id() with the same
level of error-handling (if any) as before.
A few drivers aren't included in this patch because they have a
different set of maintainers. They will be covered by other patches.
Signed-off-by: Stephen Kitt
---
drivers/hwmon/ad7414.c | 5 ++---
drivers/hwmon/ad7418.c | 9 +
drivers/hwmon/adm1021.c | 9 +
drivers/hwmon/adm1025.c | 5 ++---
drivers/hwmon/adm1026.c | 5 ++---
drivers/hwmon/adm1031.c | 9 +
drivers/hwmon/adm9240.c | 5 ++---
drivers/hwmon/adt7410.c | 5 ++---
drivers/hwmon/adt7411.c | 5 ++---
drivers/hwmon/adt7462.c | 5 ++---
drivers/hwmon/adt7470.c | 5 ++---
drivers/hwmon/adt7475.c | 6 +++---
drivers/hwmon/amc6821.c | 5 ++---
drivers/hwmon/asb100.c | 8 +++-
drivers/hwmon/atxp1.c | 5 ++---
drivers/hwmon/ds1621.c | 9 +
drivers/hwmon/ds620.c | 5 ++---
drivers/hwmon/emc1403.c | 8 +---
drivers/hwmon/emc6w201.c| 5 ++---
drivers/hwmon/fschmd.c | 10 --
drivers/hwmon/ftsteutates.c | 4 ++--
drivers/hwmon/g760a.c | 5 ++---
drivers/hwmon/g762.c| 4 ++--
drivers/hwmon/gl518sm.c | 5 ++---
drivers/hwmon/gl520sm.c | 5 ++---
drivers/hwmon/hih6130.c | 5 ++---
drivers/hwmon/ina209.c | 5 ++---
drivers/hwmon/ina2xx.c | 9 +
drivers/hwmon/ina3221.c | 5 ++---
drivers/hwmon/jc42.c| 4 ++--
drivers/hwmon/lineage-pem.c | 5 ++---
drivers/hwmon/lm63.c| 9 +
drivers/hwmon/lm75.c| 9 +
drivers/hwmon/lm77.c| 4 ++--
drivers/hwmon/lm78.c| 9 +
drivers/hwmon/lm80.c| 5 ++---
drivers/hwmon/lm83.c| 9 +
drivers/hwmon/lm85.c| 8 +---
drivers/hwmon/lm90.c| 7 +++
drivers/hwmon/lm92.c| 5 ++---
drivers/hwmon/lm93.c| 5 ++---
drivers/hwmon/lm95234.c | 9 +
drivers/hwmon/lm95241.c | 5 ++---
drivers/hwmon/lm95245.c | 5 ++---
drivers/hwmon/ltc2945.c | 5 ++---
drivers/hwmon/ltc2990.c | 5 ++---
drivers/hwmon/ltc4151.c | 5 ++---
drivers/hwmon/ltc4215.c | 5 ++---
drivers/hwmon/ltc4222.c | 5 ++---
drivers/hwmon/ltc4245.c | 5 ++---
drivers/hwmon/ltc4260.c | 5 ++---
drivers/hwmon/ltc4261.c | 5 ++---
drivers/hwmon/max16065.c| 8 +---
drivers/hwmon/max1619.c | 5 ++---
drivers/hwmon/max1668.c | 9 +
drivers/hwmon/max31730.c| 4 ++--
drivers/hwmon/max31790.c| 5 ++---
drivers/hwmon/max6621.c | 5 ++---
drivers/hwmon/max6639.c | 5 ++---
drivers/hwmon/max6642.c | 5 ++---
drivers/hwmon/max6650.c | 10 ++
drivers/hwmon/max6697.c | 9 +
drivers/hwmon/mcp3021.c | 9 +
drivers/hwmon/nct7802.c | 5 ++---
drivers/hwmon/nct7904.c | 5 ++---
drivers/hwmon/occ/p8_i2c.c | 5 ++---
drivers/hwmon/pcf8591.c | 5 ++---
drivers/hwmon/powr1220.c| 5 ++---
drivers/hwmon/sht21.c | 5 ++---
drivers/hwmon/sht3x.c | 9 +
drivers/hwmon/shtc1.c | 9 +
drivers/hwmon/smm665.c | 9 +
drivers/hwmon/smsc47m192.c | 5 ++---
drivers/hwmon/stts751.c | 5 ++---
drivers/hwmon/tc654.c | 5 ++---
drivers/hwmon/tc74.c| 5 ++---
drivers/hwmon/thmc50.c | 9 +
drivers/hwmon/tmp102.c | 5 ++---
drivers/hwmon/tmp103.c | 5 ++---
drivers/hwmon/tmp108.c | 5 ++---
drivers/hwmon/tmp401.c | 7 +++
drivers/hwmon/tmp421.c | 7 +++
drivers/hwmon/w83773g.c | 5 ++---
drivers/hwmon/w83781d.c | 9 +
drivers/hwmon/w83792d.c | 7 +++
drivers/hwmon/w83l785ts.c | 8 +++-
drivers/hwmon/w83l786ng.c | 4 ++--
87 files changed, 250 insertions(+), 285 deletions(-)
diff --git a/drivers/hwmon/ad7414.c b/drivers/hwmon/ad7414.c
index a529f2efc790..6a765755d061 100644
--- a/drivers/hwmon/ad7414.c
+++ b/drivers/hwmon/ad7414.c
@@ -169,8 +169,7 @@ static struct attribute *ad7414_attrs[] = {
ATTRIBUTE_GROUPS(ad7414);
-static int ad7414_probe(struct i2c_client *client,
- const struct i2c_device_id *dev_id)
+static int ad7414_probe(struct i2c_client *client)
{
struct device *dev = >dev;
struct ad7414_data *data;
@@ -222,7 +221,7 @@ static struct i2c_driver ad7414_driver = {
.name = "ad7414",
.of_
[PATCH] drivers/hwmon/asc7621.c: use simple i2c probe
This driver doesn't use the id information provided by the old i2c
probe function, so it can trivially be converted to the simple
("probe_new") form.
Signed-off-by: Stephen Kitt
---
drivers/hwmon/asc7621.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hwmon/asc7621.c b/drivers/hwmon/asc7621.c
index 9e14e2829ee9..600ffc7e1900 100644
--- a/drivers/hwmon/asc7621.c
+++ b/drivers/hwmon/asc7621.c
@@ -1087,7 +1087,7 @@ static void asc7621_init_client(struct i2c_client *client)
}
static int
-asc7621_probe(struct i2c_client *client, const struct i2c_device_id *id)
+asc7621_probe(struct i2c_client *client)
{
struct asc7621_data *data;
int i, err;
@@ -1193,7 +1193,7 @@ static struct i2c_driver asc7621_driver = {
.driver = {
.name = "asc7621",
},
- .probe = asc7621_probe,
+ .probe_new = asc7621_probe,
.remove = asc7621_remove,
.id_table = asc7621_id,
.detect = asc7621_detect,
--
2.25.4
[PATCH] drivers/hwmon/lm73.c: use simple i2c probe
This driver doesn't use the id information provided by the old i2c
probe function, so it can trivially be converted to the simple
("probe_new") form.
Signed-off-by: Stephen Kitt
---
drivers/hwmon/lm73.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hwmon/lm73.c b/drivers/hwmon/lm73.c
index 733c48bf6c98..beb0d61bcd82 100644
--- a/drivers/hwmon/lm73.c
+++ b/drivers/hwmon/lm73.c
@@ -190,7 +190,7 @@ ATTRIBUTE_GROUPS(lm73);
/* device probe and removal */
static int
-lm73_probe(struct i2c_client *client, const struct i2c_device_id *id)
+lm73_probe(struct i2c_client *client)
{
struct device *dev = >dev;
struct device *hwmon_dev;
@@ -277,7 +277,7 @@ static struct i2c_driver lm73_driver = {
.name = "lm73",
.of_match_table = lm73_of_match,
},
- .probe = lm73_probe,
+ .probe_new = lm73_probe,
.id_table = lm73_ids,
.detect = lm73_detect,
.address_list = normal_i2c,
--
2.25.4
[PATCH] drivers/hwmon/adm1177.c: use simple i2c probe
This driver doesn't use the id information provided by the old i2c
probe function, so it can trivially be converted to the simple
("probe_new") form.
Signed-off-by: Stephen Kitt
---
drivers/hwmon/adm1177.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/hwmon/adm1177.c b/drivers/hwmon/adm1177.c
index d314223a404a..6e8bb661894b 100644
--- a/drivers/hwmon/adm1177.c
+++ b/drivers/hwmon/adm1177.c
@@ -196,8 +196,7 @@ static void adm1177_remove(void *data)
regulator_disable(st->reg);
}
-static int adm1177_probe(struct i2c_client *client,
-const struct i2c_device_id *id)
+static int adm1177_probe(struct i2c_client *client)
{
struct device *dev = >dev;
struct device *hwmon_dev;
@@ -277,7 +276,7 @@ static struct i2c_driver adm1177_driver = {
.name = "adm1177",
.of_match_table = adm1177_dt_ids,
},
- .probe = adm1177_probe,
+ .probe_new = adm1177_probe,
.id_table = adm1177_id,
};
module_i2c_driver(adm1177_driver);
--
2.25.4
[PATCH] drivers/hwmon/emc2103.c: use simple i2c probe
This driver doesn't use the id information provided by the old i2c
probe function, so it can trivially be converted to the simple
("probe_new") form.
Signed-off-by: Stephen Kitt
---
drivers/hwmon/emc2103.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hwmon/emc2103.c b/drivers/hwmon/emc2103.c
index 924c02c1631d..e4c95ca9e19f 100644
--- a/drivers/hwmon/emc2103.c
+++ b/drivers/hwmon/emc2103.c
@@ -551,7 +551,7 @@ static const struct attribute_group emc2103_temp4_group = {
};
static int
-emc2103_probe(struct i2c_client *client, const struct i2c_device_id *id)
+emc2103_probe(struct i2c_client *client)
{
struct emc2103_data *data;
struct device *hwmon_dev;
@@ -653,7 +653,7 @@ static struct i2c_driver emc2103_driver = {
.driver = {
.name = "emc2103",
},
- .probe = emc2103_probe,
+ .probe_new = emc2103_probe,
.id_table = emc2103_ids,
.detect = emc2103_detect,
.address_list = normal_i2c,
--
2.25.4
[PATCH] drivers/hwmon/w83791d.c: use simple i2c probe
This driver doesn't use the id information provided by the old i2c
probe function, so it can trivially be converted to the simple
("probe_new") form.
Signed-off-by: Stephen Kitt
---
drivers/hwmon/w83791d.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/hwmon/w83791d.c b/drivers/hwmon/w83791d.c
index aad8d4da5802..37b25a1474c4 100644
--- a/drivers/hwmon/w83791d.c
+++ b/drivers/hwmon/w83791d.c
@@ -315,8 +315,7 @@ struct w83791d_data {
u8 vrm; /* hwmon-vid */
};
-static int w83791d_probe(struct i2c_client *client,
-const struct i2c_device_id *id);
+static int w83791d_probe(struct i2c_client *client);
static int w83791d_detect(struct i2c_client *client,
struct i2c_board_info *info);
static int w83791d_remove(struct i2c_client *client);
@@ -342,7 +341,7 @@ static struct i2c_driver w83791d_driver = {
.driver = {
.name = "w83791d",
},
- .probe = w83791d_probe,
+ .probe_new = w83791d_probe,
.remove = w83791d_remove,
.id_table = w83791d_id,
.detect = w83791d_detect,
@@ -1346,8 +1345,7 @@ static int w83791d_detect(struct i2c_client *client,
return 0;
}
-static int w83791d_probe(struct i2c_client *client,
-const struct i2c_device_id *id)
+static int w83791d_probe(struct i2c_client *client)
{
struct w83791d_data *data;
struct device *dev = >dev;
--
2.25.4
[PATCH] drivers/hwmon/w83793.c: use simple i2c probe
This driver doesn't use the id information provided by the old i2c
probe function, so it can trivially be converted to the simple
("probe_new") form.
Signed-off-by: Stephen Kitt
---
drivers/hwmon/w83793.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/hwmon/w83793.c b/drivers/hwmon/w83793.c
index 3f59f2a1a5e3..e7d0484eabe4 100644
--- a/drivers/hwmon/w83793.c
+++ b/drivers/hwmon/w83793.c
@@ -283,8 +283,7 @@ static void w83793_release_resources(struct kref *ref)
static u8 w83793_read_value(struct i2c_client *client, u16 reg);
static int w83793_write_value(struct i2c_client *client, u16 reg, u8 value);
-static int w83793_probe(struct i2c_client *client,
- const struct i2c_device_id *id);
+static int w83793_probe(struct i2c_client *client);
static int w83793_detect(struct i2c_client *client,
struct i2c_board_info *info);
static int w83793_remove(struct i2c_client *client);
@@ -303,7 +302,7 @@ static struct i2c_driver w83793_driver = {
.driver = {
.name = "w83793",
},
- .probe = w83793_probe,
+ .probe_new = w83793_probe,
.remove = w83793_remove,
.id_table = w83793_id,
.detect = w83793_detect,
@@ -1646,8 +1645,7 @@ static int w83793_detect(struct i2c_client *client,
return 0;
}
-static int w83793_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+static int w83793_probe(struct i2c_client *client)
{
struct device *dev = >dev;
static const int watchdog_minors[] = {
--
2.25.4
[PATCH] drivers/hwmon/adm1029.c: use simple i2c probe
This driver doesn't use the id information provided by the old i2c
probe function, so it can trivially be converted to the simple
("probe_new") form.
Signed-off-by: Stephen Kitt
---
drivers/hwmon/adm1029.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/hwmon/adm1029.c b/drivers/hwmon/adm1029.c
index f7752a5bef31..50b1df7b008c 100644
--- a/drivers/hwmon/adm1029.c
+++ b/drivers/hwmon/adm1029.c
@@ -352,8 +352,7 @@ static int adm1029_init_client(struct i2c_client *client)
return 1;
}
-static int adm1029_probe(struct i2c_client *client,
-const struct i2c_device_id *id)
+static int adm1029_probe(struct i2c_client *client)
{
struct device *dev = >dev;
struct adm1029_data *data;
@@ -390,7 +389,7 @@ static struct i2c_driver adm1029_driver = {
.driver = {
.name = "adm1029",
},
- .probe = adm1029_probe,
+ .probe_new = adm1029_probe,
.id_table = adm1029_id,
.detect = adm1029_detect,
.address_list = normal_i2c,
--
2.25.4
[PATCH] Fix references to nommu-mmap.rst
nommu-mmap.rst was moved to Documentation/admin-guide/mm; this patch
updates the remaining stale references to Documentation/mm.
Fixes: 800c02f5d030 ("docs: move nommu-mmap.txt to admin-guide and rename to
ReST")
Signed-off-by: Stephen Kitt
---
init/Kconfig | 2 +-
mm/Kconfig | 2 +-
mm/nommu.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/init/Kconfig b/init/Kconfig
index 2dd5531dae98..8d5fefd1f229 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1957,7 +1957,7 @@ config MMAP_ALLOW_UNINITIALIZED
userspace. Since that isn't generally a problem on no-MMU systems,
it is normally safe to say Y here.
- See Documentation/mm/nommu-mmap.rst for more information.
+ See Documentation/admin-guide/mm/nommu-mmap.rst for more information.
config SYSTEM_DATA_VERIFICATION
def_bool n
diff --git a/mm/Kconfig b/mm/Kconfig
index d41f3fa7e923..29e239497718 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -387,7 +387,7 @@ config NOMMU_INITIAL_TRIM_EXCESS
This option specifies the initial value of this option. The default
of 1 says that all excess pages should be trimmed.
- See Documentation/mm/nommu-mmap.rst for more information.
+ See Documentation/admin-guide/mm/nommu-mmap.rst for more information.
config TRANSPARENT_HUGEPAGE
bool "Transparent Hugepage Support"
diff --git a/mm/nommu.c b/mm/nommu.c
index 64539971188b..e8e2c5bb6f0a 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -5,7 +5,7 @@
* Replacement code for mm functions to support CPU's that don't
* have any form of memory management unit (thus no virtual memory).
*
- * See Documentation/mm/nommu-mmap.rst
+ * See Documentation/admin-guide/mm/nommu-mmap.rst
*
* Copyright (c) 2004-2008 David Howells
* Copyright (c) 2000-2003 David McCullough
base-commit: e176b7a3054eef44a22f6ca3d14168dcf9bad21e
--
2.20.1
[PATCH v2] ARM: davinci: use simple i2c probe function
The i2c probe functions here don't use the id information provided in
their second argument, so the single-parameter i2c probe function
("probe_new") can be used instead.
This avoids scanning the identifier tables during probes.
Signed-off-by: Stephen Kitt
---
Changes since v1:
- split into per-sub-architecture patches.
arch/arm/mach-davinci/board-dm644x-evm.c | 5 ++---
arch/arm/mach-davinci/board-dm646x-evm.c | 10 --
2 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/arch/arm/mach-davinci/board-dm644x-evm.c
b/arch/arm/mach-davinci/board-dm644x-evm.c
index a5d3708fedf6..d0dcf69cc76d 100644
--- a/arch/arm/mach-davinci/board-dm644x-evm.c
+++ b/arch/arm/mach-davinci/board-dm644x-evm.c
@@ -548,8 +548,7 @@ static const struct property_entry eeprom_properties[] = {
*/
static struct i2c_client *dm6446evm_msp;
-static int dm6446evm_msp_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+static int dm6446evm_msp_probe(struct i2c_client *client)
{
dm6446evm_msp = client;
return 0;
@@ -569,7 +568,7 @@ static const struct i2c_device_id dm6446evm_msp_ids[] = {
static struct i2c_driver dm6446evm_msp_driver = {
.driver.name= "dm6446evm_msp",
.id_table = dm6446evm_msp_ids,
- .probe = dm6446evm_msp_probe,
+ .probe_new = dm6446evm_msp_probe,
.remove = dm6446evm_msp_remove,
};
diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c
b/arch/arm/mach-davinci/board-dm646x-evm.c
index 4600b617f9b4..2dce16fff77e 100644
--- a/arch/arm/mach-davinci/board-dm646x-evm.c
+++ b/arch/arm/mach-davinci/board-dm646x-evm.c
@@ -160,8 +160,7 @@ static struct platform_device davinci_aemif_device = {
#define DM646X_EVM_ATA_PWD BIT(1)
/* CPLD Register 0 Client: used for I/O Control */
-static int cpld_reg0_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+static int cpld_reg0_probe(struct i2c_client *client)
{
if (HAS_ATA) {
u8 data;
@@ -197,7 +196,7 @@ static const struct i2c_device_id cpld_reg_ids[] = {
static struct i2c_driver dm6467evm_cpld_driver = {
.driver.name= "cpld_reg0",
.id_table = cpld_reg_ids,
- .probe = cpld_reg0_probe,
+ .probe_new = cpld_reg0_probe,
};
/* LEDS */
@@ -402,8 +401,7 @@ static struct snd_platform_data dm646x_evm_snd_data[] = {
#ifdef CONFIG_I2C
static struct i2c_client *cpld_client;
-static int cpld_video_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+static int cpld_video_probe(struct i2c_client *client)
{
cpld_client = client;
return 0;
@@ -424,7 +422,7 @@ static struct i2c_driver cpld_video_driver = {
.driver = {
.name = "cpld_video",
},
- .probe = cpld_video_probe,
+ .probe_new = cpld_video_probe,
.remove = cpld_video_remove,
.id_table = cpld_video_id,
};
base-commit: bcf876870b95592b52519ed4aafcf9d95999bc9c
--
2.20.1
Re: [PATCH] arch/arm: use simple i2c probe function
Hi,
On Sun, 9 Aug 2020 10:38:58 +0200, Krzysztof Kozlowski
wrote:
> On Fri, Aug 07, 2020 at 05:31:00PM +0200, Stephen Kitt wrote:
> > The i2c probe functions here don't use the id information provided in
> > their second argument, so the single-parameter i2c probe function
> > ("probe_new") can be used instead.
> >
> > This avoids scanning the identifier tables during probes.
> >
> > Signed-off-by: Stephen Kitt
> > ---
> > arch/arm/mach-davinci/board-dm644x-evm.c | 5 ++---
> > arch/arm/mach-davinci/board-dm646x-evm.c | 10 --
> > arch/arm/mach-s3c64xx/mach-crag6410-module.c | 5 ++---
> > 3 files changed, 8 insertions(+), 12 deletions(-)
>
> You need to split it per sub architecture maintainers. The subject
> prefix is then for example: "ARM: s3c64xx: ".
Thanks, will do!
Regards,
Stephen
pgpo4GpxBHN4X.pgp
Description: OpenPGP digital signature
[PATCH v2] ARM: s3c64xx: use simple i2c probe function
The i2c probe functions here don't use the id information provided in
their second argument, so the single-parameter i2c probe function
("probe_new") can be used instead.
This avoids scanning the identifier tables during probes.
Signed-off-by: Stephen Kitt
---
Changes since v1:
- split into per-sub-architecture patches.
arch/arm/mach-s3c64xx/mach-crag6410-module.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-s3c64xx/mach-crag6410-module.c
b/arch/arm/mach-s3c64xx/mach-crag6410-module.c
index 34f1baa10c54..43b587e79d21 100644
--- a/arch/arm/mach-s3c64xx/mach-crag6410-module.c
+++ b/arch/arm/mach-s3c64xx/mach-crag6410-module.c
@@ -378,8 +378,7 @@ static const struct {
.i2c_devs = wm2200_i2c, .num_i2c_devs = ARRAY_SIZE(wm2200_i2c) },
};
-static int wlf_gf_module_probe(struct i2c_client *i2c,
- const struct i2c_device_id *i2c_id)
+static int wlf_gf_module_probe(struct i2c_client *i2c)
{
int ret, i, j, id, rev;
@@ -432,7 +431,7 @@ static struct i2c_driver wlf_gf_module_driver = {
.driver = {
.name = "wlf-gf-module"
},
- .probe = wlf_gf_module_probe,
+ .probe_new = wlf_gf_module_probe,
.id_table = wlf_gf_module_id,
};
base-commit: bcf876870b95592b52519ed4aafcf9d95999bc9c
--
2.20.1
[PATCH v5] hwmon/pmbus: use simple i2c probe function
pmbus_do_probe doesn't use the id information provided in its second
argument, so this can be removed, which then allows using the
single-parameter i2c probe function ("probe_new") for probes.
This avoids scanning the identifier tables during probes.
Drivers which didn't use the id are converted as-is; drivers which did
are modified as follows:
* if the information in i2c_client is sufficient, that's used instead
(client->name);
* configured v. probed comparisons are performed by comparing the
configured name to the detected name, instead of the ids; this
involves strcmp but is still cheaper than comparing all the device
names when scanning the tables;
* anything else is handled by calling i2c_match_id() with the same
level of error-handling (if any) as before.
Additionally, the mismatch message in the ltc2978 driver is adjusted
so that it no longer assumes that the driver_data is an index into
ltc2978_id.
Signed-off-by: Stephen Kitt
---
Changes since v1:
- i2c_device_id declarations are left unchanged;
- all drivers are converted, using client info or i2c_match_id().
Changes since v2:
- updated the documentation;
- fixed unbalanced braces around the modified else in ibm-cffps.c.
Changes since v3:
- fix ltc2978 to avoid assuming names and driver_data are bijective,
and that driver_data are indexes into ltc2978_id.
Changes since v4:
- explicitly case id->driver_data to int in a printk call.
Documentation/hwmon/pmbus-core.rst | 3 +--
Documentation/hwmon/pmbus.rst | 7 +++
drivers/hwmon/pmbus/adm1275.c | 11 +--
drivers/hwmon/pmbus/bel-pfe.c | 11 ++-
drivers/hwmon/pmbus/ibm-cffps.c| 19 ---
drivers/hwmon/pmbus/inspur-ipsps.c | 7 +++
drivers/hwmon/pmbus/ir35221.c | 7 +++
drivers/hwmon/pmbus/ir38064.c | 7 +++
drivers/hwmon/pmbus/irps5401.c | 7 +++
drivers/hwmon/pmbus/isl68137.c | 11 ++-
drivers/hwmon/pmbus/lm25066.c | 11 ++-
drivers/hwmon/pmbus/ltc2978.c | 14 --
drivers/hwmon/pmbus/ltc3815.c | 7 +++
drivers/hwmon/pmbus/max16064.c | 7 +++
drivers/hwmon/pmbus/max16601.c | 7 +++
drivers/hwmon/pmbus/max20730.c | 11 ++-
drivers/hwmon/pmbus/max20751.c | 7 +++
drivers/hwmon/pmbus/max31785.c | 9 -
drivers/hwmon/pmbus/max34440.c | 13 +++--
drivers/hwmon/pmbus/max8688.c | 7 +++
drivers/hwmon/pmbus/pmbus.c| 11 ++-
drivers/hwmon/pmbus/pmbus.h| 3 +--
drivers/hwmon/pmbus/pmbus_core.c | 3 +--
drivers/hwmon/pmbus/pxe1610.c | 7 +++
drivers/hwmon/pmbus/tps40422.c | 7 +++
drivers/hwmon/pmbus/tps53679.c | 11 ++-
drivers/hwmon/pmbus/ucd9000.c | 13 ++---
drivers/hwmon/pmbus/ucd9200.c | 13 ++---
drivers/hwmon/pmbus/xdpe12284.c| 7 +++
drivers/hwmon/pmbus/zl6100.c | 11 +--
30 files changed, 131 insertions(+), 138 deletions(-)
diff --git a/Documentation/hwmon/pmbus-core.rst
b/Documentation/hwmon/pmbus-core.rst
index 501b37b0610d..e22c4f6808bc 100644
--- a/Documentation/hwmon/pmbus-core.rst
+++ b/Documentation/hwmon/pmbus-core.rst
@@ -270,8 +270,7 @@ obtain the chip status. Therefore, it must _not_ be called
from that function.
::
- int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id,
-struct pmbus_driver_info *info);
+ int pmbus_do_probe(struct i2c_client *client, struct pmbus_driver_info
*info);
Execute probe function. Similar to standard probe function for other drivers,
with the pointer to struct pmbus_driver_info as additional argument. Calls
diff --git a/Documentation/hwmon/pmbus.rst b/Documentation/hwmon/pmbus.rst
index 2658ddee70eb..1f31317fe186 100644
--- a/Documentation/hwmon/pmbus.rst
+++ b/Documentation/hwmon/pmbus.rst
@@ -143,10 +143,9 @@ Emerson DS1200 power modules might look as follows::
| PMBUS_HAVE_FAN12 | PMBUS_HAVE_STATUS_FAN12,
};
- static int ds1200_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+ static int ds1200_probe(struct i2c_client *client)
{
- return pmbus_do_probe(client, id, _info);
+ return pmbus_do_probe(client, _info);
}
static int ds1200_remove(struct i2c_client *client)
@@ -166,7 +165,7 @@ Emerson DS1200 power modules might look as follows::
.driver = {
.name = "ds1200",
},
- .probe = ds1200_probe,
+ .probe_new = ds1200_probe,
.remove = ds1200_remove,
.id_table = ds1200_id,
};
diff --git a/drivers/hwmon/pmbus/adm1275.c b/drivers/hwmon/pmbus/adm1275.c
index 19317575d1c6..7fc3a3b1b55c 100644
--- a/drivers/hwmon/pmbus/adm1275.c
+++ b/drivers/hwmon/pmbus/adm1275.c
@@ -462,8 +462,7 @@ static const struct i2c_device_id adm1275_id[] = {
};
MODULE_DEVICE_TABLE(i2c,
[PATCH v4] hwmon/pmbus: use simple i2c probe function
pmbus_do_probe doesn't use the id information provided in its second
argument, so this can be removed, which then allows using the
single-parameter i2c probe function ("probe_new") for probes.
This avoids scanning the identifier tables during probes.
Drivers which didn't use the id are converted as-is; drivers which did
are modified as follows:
* if the information in i2c_client is sufficient, that's used instead
(client->name);
* configured v. probed comparisons are performed by comparing the
configured name to the detected name, instead of the ids; this
involves strcmp but is still cheaper than comparing all the device
names when scanning the tables;
* anything else is handled by calling i2c_match_id() with the same
level of error-handling (if any) as before.
Additionally, the mismatch message in the ltc2978 driver is adjusted
so that it no longer assumes that the driver_data is an index into
ltc2978_id.
Signed-off-by: Stephen Kitt
---
Changes since v1:
- i2c_device_id declarations are left unchanged;
- all drivers are converted, using client info or i2c_match_id().
Changes since v2:
- updated the documentation;
- fixed unbalanced braces around the modified else in ibm-cffps.c.
Changes since v3:
- fix ltc2978 to avoid assuming names and driver_data are bijective,
and that driver_data are indexes into ltc2978_id.
Documentation/hwmon/pmbus-core.rst | 3 +--
Documentation/hwmon/pmbus.rst | 7 +++
drivers/hwmon/pmbus/adm1275.c | 11 +--
drivers/hwmon/pmbus/bel-pfe.c | 11 ++-
drivers/hwmon/pmbus/ibm-cffps.c| 19 ---
drivers/hwmon/pmbus/inspur-ipsps.c | 7 +++
drivers/hwmon/pmbus/ir35221.c | 7 +++
drivers/hwmon/pmbus/ir38064.c | 7 +++
drivers/hwmon/pmbus/irps5401.c | 7 +++
drivers/hwmon/pmbus/isl68137.c | 11 ++-
drivers/hwmon/pmbus/lm25066.c | 11 ++-
drivers/hwmon/pmbus/ltc2978.c | 14 --
drivers/hwmon/pmbus/ltc3815.c | 7 +++
drivers/hwmon/pmbus/max16064.c | 7 +++
drivers/hwmon/pmbus/max16601.c | 7 +++
drivers/hwmon/pmbus/max20730.c | 11 ++-
drivers/hwmon/pmbus/max20751.c | 7 +++
drivers/hwmon/pmbus/max31785.c | 9 -
drivers/hwmon/pmbus/max34440.c | 13 +++--
drivers/hwmon/pmbus/max8688.c | 7 +++
drivers/hwmon/pmbus/pmbus.c| 11 ++-
drivers/hwmon/pmbus/pmbus.h| 3 +--
drivers/hwmon/pmbus/pmbus_core.c | 3 +--
drivers/hwmon/pmbus/pxe1610.c | 7 +++
drivers/hwmon/pmbus/tps40422.c | 7 +++
drivers/hwmon/pmbus/tps53679.c | 11 ++-
drivers/hwmon/pmbus/ucd9000.c | 13 ++---
drivers/hwmon/pmbus/ucd9200.c | 13 ++---
drivers/hwmon/pmbus/xdpe12284.c| 7 +++
drivers/hwmon/pmbus/zl6100.c | 11 +--
30 files changed, 131 insertions(+), 138 deletions(-)
diff --git a/Documentation/hwmon/pmbus-core.rst
b/Documentation/hwmon/pmbus-core.rst
index 501b37b0610d..e22c4f6808bc 100644
--- a/Documentation/hwmon/pmbus-core.rst
+++ b/Documentation/hwmon/pmbus-core.rst
@@ -270,8 +270,7 @@ obtain the chip status. Therefore, it must _not_ be called
from that function.
::
- int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id,
-struct pmbus_driver_info *info);
+ int pmbus_do_probe(struct i2c_client *client, struct pmbus_driver_info
*info);
Execute probe function. Similar to standard probe function for other drivers,
with the pointer to struct pmbus_driver_info as additional argument. Calls
diff --git a/Documentation/hwmon/pmbus.rst b/Documentation/hwmon/pmbus.rst
index 2658ddee70eb..1f31317fe186 100644
--- a/Documentation/hwmon/pmbus.rst
+++ b/Documentation/hwmon/pmbus.rst
@@ -143,10 +143,9 @@ Emerson DS1200 power modules might look as follows::
| PMBUS_HAVE_FAN12 | PMBUS_HAVE_STATUS_FAN12,
};
- static int ds1200_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+ static int ds1200_probe(struct i2c_client *client)
{
- return pmbus_do_probe(client, id, _info);
+ return pmbus_do_probe(client, _info);
}
static int ds1200_remove(struct i2c_client *client)
@@ -166,7 +165,7 @@ Emerson DS1200 power modules might look as follows::
.driver = {
.name = "ds1200",
},
- .probe = ds1200_probe,
+ .probe_new = ds1200_probe,
.remove = ds1200_remove,
.id_table = ds1200_id,
};
diff --git a/drivers/hwmon/pmbus/adm1275.c b/drivers/hwmon/pmbus/adm1275.c
index 19317575d1c6..7fc3a3b1b55c 100644
--- a/drivers/hwmon/pmbus/adm1275.c
+++ b/drivers/hwmon/pmbus/adm1275.c
@@ -462,8 +462,7 @@ static const struct i2c_device_id adm1275_id[] = {
};
MODULE_DEVICE_TABLE(i2c, adm1275_id);
-static int adm1275_probe(struct i2c_client *client,
-
Re: [PATCH v3] hwmon/pmbus: use simple i2c probe function
On Fri, 7 Aug 2020 21:07:07 +0200, Stephen Kitt wrote:
> On Fri, 7 Aug 2020 10:32:31 -0700, Guenter Roeck wrote:
> > On Fri, Aug 07, 2020 at 06:28:01PM +0200, Stephen Kitt wrote:
> > >
> > > -static int ltc2978_probe(struct i2c_client *client,
> > > - const struct i2c_device_id *id)
> > > +static int ltc2978_probe(struct i2c_client *client)
> > > {
> > > int i, chip_id;
> > > struct ltc2978_data *data;
> > > @@ -670,10 +669,10 @@ static int ltc2978_probe(struct i2c_client
> > > *client, return chip_id;
> > >
> > > data->id = chip_id;
> > > - if (data->id != id->driver_data)
> > > + if (strcmp(client->name, ltc2978_id[data->id].name) != 0)
> >
> > I was about to apply this patch, but this is problematic: It assumes that
> > __stringify(id) == ltc2978_id[id].name and that ltc2978_id[id].driver_data
> > == id. While that is curently the case (as far as I can see), it is still
> > unsafe. I think it would be much safer to use i2c_match_id() here.
>
> I’m not following the __stringify assumption
[...]
I get it, the code assumes there’s a bijection between the set of names and
the set of driver_data values. So effectively we can’t log the detected name
based on the chip_id...
Regards,
Stephen
pgpm_wvfSlRNV.pgp
Description: OpenPGP digital signature
Re: [PATCH v3] hwmon/pmbus: use simple i2c probe function
On Fri, 7 Aug 2020 10:32:31 -0700, Guenter Roeck wrote:
> On Fri, Aug 07, 2020 at 06:28:01PM +0200, Stephen Kitt wrote:
> >
> > -static int ltc2978_probe(struct i2c_client *client,
> > -const struct i2c_device_id *id)
> > +static int ltc2978_probe(struct i2c_client *client)
> > {
> > int i, chip_id;
> > struct ltc2978_data *data;
> > @@ -670,10 +669,10 @@ static int ltc2978_probe(struct i2c_client *client,
> > return chip_id;
> >
> > data->id = chip_id;
> > - if (data->id != id->driver_data)
> > + if (strcmp(client->name, ltc2978_id[data->id].name) != 0)
>
> I was about to apply this patch, but this is problematic: It assumes that
> __stringify(id) == ltc2978_id[id].name and that ltc2978_id[id].driver_data
> == id. While that is curently the case (as far as I can see), it is still
> unsafe. I think it would be much safer to use i2c_match_id() here.
I’m not following the __stringify assumption, but I do get your point about
the driver_data being a valid index into the array; that was already baked
into the code, as
dev_warn(>dev,
"Device mismatch: Configured %s, detected %s\n",
client->name,
ltc2978_id[data->id].name);
but I’ll fix both.
Similar assumptions are present in other drivers here IIRC, I’ll fix those
too.
Regards,
Stephen
pgp12kL18kQdp.pgp
Description: OpenPGP digital signature
Re: [PATCH v2] hwmon/pmbus: use simple i2c probe function
On Fri, 7 Aug 2020 08:23:29 -0700, Guenter Roeck wrote:
> On 8/7/20 12:45 AM, Stephen Kitt wrote:
> > pmbus_do_probe doesn't use the id information provided in its second
> > argument, so this can be removed, which then allows using the
> > single-parameter i2c probe function ("probe_new") for probes.
> >
> > This avoids scanning the identifier tables during probes.
> >
> > Drivers which didn't use the id are converted as-is; drivers which did
> > are modified as follows:
> >
> > * if the information in i2c_client is sufficient, that's used instead
> > (client->name);
> > * configured v. probed comparisons are performed by comparing the
> > configured name to the detected name, instead of the ids; this
> > involves strcmp but is still cheaper than comparing all the device
> > names when scanning the tables;
> > * anything else is handled by calling i2c_match_id() with the same
> > level of error-handling (if any) as before.
> >
> > Signed-off-by: Stephen Kitt
>
> Please also update the documentation.
>
> Documentation/hwmon/pmbus-core.rst: int pmbus_do_probe(struct i2c_client
> *client, const struct i2c_device_id *id, Documentation/hwmon/pmbus.rst:
> return pmbus_do_probe(client, id, _info);
Aargh, I usually *start* from the documentation, but I didn’t think to check
in this case.
> Also, please fix the checkpatch issue reported by Wolfram.
Will do, v3 on its way.
Thanks for the reviews!
Regards,
Stephen
pgpDssvachgtn.pgp
Description: OpenPGP digital signature
[PATCH v3] hwmon/pmbus: use simple i2c probe function
pmbus_do_probe doesn't use the id information provided in its second
argument, so this can be removed, which then allows using the
single-parameter i2c probe function ("probe_new") for probes.
This avoids scanning the identifier tables during probes.
Drivers which didn't use the id are converted as-is; drivers which did
are modified as follows:
* if the information in i2c_client is sufficient, that's used instead
(client->name);
* configured v. probed comparisons are performed by comparing the
configured name to the detected name, instead of the ids; this
involves strcmp but is still cheaper than comparing all the device
names when scanning the tables;
* anything else is handled by calling i2c_match_id() with the same
level of error-handling (if any) as before.
Signed-off-by: Stephen Kitt
---
Changes since v1:
- i2c_device_id declarations are left unchanged;
- all drivers are converted, using client info or i2c_match_id().
Changes since v2:
- updated the documentation;
- fixed unbalanced braces around the modified else in ibm-cffps.c.
Documentation/hwmon/pmbus-core.rst | 3 +--
Documentation/hwmon/pmbus.rst | 7 +++
drivers/hwmon/pmbus/adm1275.c | 11 +--
drivers/hwmon/pmbus/bel-pfe.c | 11 ++-
drivers/hwmon/pmbus/ibm-cffps.c| 19 ---
drivers/hwmon/pmbus/inspur-ipsps.c | 7 +++
drivers/hwmon/pmbus/ir35221.c | 7 +++
drivers/hwmon/pmbus/ir38064.c | 7 +++
drivers/hwmon/pmbus/irps5401.c | 7 +++
drivers/hwmon/pmbus/isl68137.c | 11 ++-
drivers/hwmon/pmbus/lm25066.c | 11 ++-
drivers/hwmon/pmbus/ltc2978.c | 11 +--
drivers/hwmon/pmbus/ltc3815.c | 7 +++
drivers/hwmon/pmbus/max16064.c | 7 +++
drivers/hwmon/pmbus/max16601.c | 7 +++
drivers/hwmon/pmbus/max20730.c | 11 ++-
drivers/hwmon/pmbus/max20751.c | 7 +++
drivers/hwmon/pmbus/max31785.c | 9 -
drivers/hwmon/pmbus/max34440.c | 13 +++--
drivers/hwmon/pmbus/max8688.c | 7 +++
drivers/hwmon/pmbus/pmbus.c| 11 ++-
drivers/hwmon/pmbus/pmbus.h| 3 +--
drivers/hwmon/pmbus/pmbus_core.c | 3 +--
drivers/hwmon/pmbus/pxe1610.c | 7 +++
drivers/hwmon/pmbus/tps40422.c | 7 +++
drivers/hwmon/pmbus/tps53679.c | 11 ++-
drivers/hwmon/pmbus/ucd9000.c | 13 ++---
drivers/hwmon/pmbus/ucd9200.c | 13 ++---
drivers/hwmon/pmbus/xdpe12284.c| 7 +++
drivers/hwmon/pmbus/zl6100.c | 11 +--
30 files changed, 128 insertions(+), 138 deletions(-)
diff --git a/Documentation/hwmon/pmbus-core.rst
b/Documentation/hwmon/pmbus-core.rst
index 501b37b0610d..e22c4f6808bc 100644
--- a/Documentation/hwmon/pmbus-core.rst
+++ b/Documentation/hwmon/pmbus-core.rst
@@ -270,8 +270,7 @@ obtain the chip status. Therefore, it must _not_ be called
from that function.
::
- int pmbus_do_probe(struct i2c_client *client, const struct i2c_device_id *id,
-struct pmbus_driver_info *info);
+ int pmbus_do_probe(struct i2c_client *client, struct pmbus_driver_info
*info);
Execute probe function. Similar to standard probe function for other drivers,
with the pointer to struct pmbus_driver_info as additional argument. Calls
diff --git a/Documentation/hwmon/pmbus.rst b/Documentation/hwmon/pmbus.rst
index 2658ddee70eb..1f31317fe186 100644
--- a/Documentation/hwmon/pmbus.rst
+++ b/Documentation/hwmon/pmbus.rst
@@ -143,10 +143,9 @@ Emerson DS1200 power modules might look as follows::
| PMBUS_HAVE_FAN12 | PMBUS_HAVE_STATUS_FAN12,
};
- static int ds1200_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+ static int ds1200_probe(struct i2c_client *client)
{
- return pmbus_do_probe(client, id, _info);
+ return pmbus_do_probe(client, _info);
}
static int ds1200_remove(struct i2c_client *client)
@@ -166,7 +165,7 @@ Emerson DS1200 power modules might look as follows::
.driver = {
.name = "ds1200",
},
- .probe = ds1200_probe,
+ .probe_new = ds1200_probe,
.remove = ds1200_remove,
.id_table = ds1200_id,
};
diff --git a/drivers/hwmon/pmbus/adm1275.c b/drivers/hwmon/pmbus/adm1275.c
index 19317575d1c6..7fc3a3b1b55c 100644
--- a/drivers/hwmon/pmbus/adm1275.c
+++ b/drivers/hwmon/pmbus/adm1275.c
@@ -462,8 +462,7 @@ static const struct i2c_device_id adm1275_id[] = {
};
MODULE_DEVICE_TABLE(i2c, adm1275_id);
-static int adm1275_probe(struct i2c_client *client,
-const struct i2c_device_id *id)
+static int adm1275_probe(struct i2c_client *client)
{
s32 (*config_read_fn)(const struct i2c_client *client, u8 reg);
u8 block_buffer[I2C_SMBUS_BLOCK_MAX + 1];
@@ -506,10 +505,10 @@ static int adm1275_probe(struct i2c_client *client,
[PATCH] arch/arm: use simple i2c probe function
The i2c probe functions here don't use the id information provided in
their second argument, so the single-parameter i2c probe function
("probe_new") can be used instead.
This avoids scanning the identifier tables during probes.
Signed-off-by: Stephen Kitt
---
arch/arm/mach-davinci/board-dm644x-evm.c | 5 ++---
arch/arm/mach-davinci/board-dm646x-evm.c | 10 --
arch/arm/mach-s3c64xx/mach-crag6410-module.c | 5 ++---
3 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/arch/arm/mach-davinci/board-dm644x-evm.c
b/arch/arm/mach-davinci/board-dm644x-evm.c
index a5d3708fedf6..d0dcf69cc76d 100644
--- a/arch/arm/mach-davinci/board-dm644x-evm.c
+++ b/arch/arm/mach-davinci/board-dm644x-evm.c
@@ -548,8 +548,7 @@ static const struct property_entry eeprom_properties[] = {
*/
static struct i2c_client *dm6446evm_msp;
-static int dm6446evm_msp_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+static int dm6446evm_msp_probe(struct i2c_client *client)
{
dm6446evm_msp = client;
return 0;
@@ -569,7 +568,7 @@ static const struct i2c_device_id dm6446evm_msp_ids[] = {
static struct i2c_driver dm6446evm_msp_driver = {
.driver.name= "dm6446evm_msp",
.id_table = dm6446evm_msp_ids,
- .probe = dm6446evm_msp_probe,
+ .probe_new = dm6446evm_msp_probe,
.remove = dm6446evm_msp_remove,
};
diff --git a/arch/arm/mach-davinci/board-dm646x-evm.c
b/arch/arm/mach-davinci/board-dm646x-evm.c
index 4600b617f9b4..2dce16fff77e 100644
--- a/arch/arm/mach-davinci/board-dm646x-evm.c
+++ b/arch/arm/mach-davinci/board-dm646x-evm.c
@@ -160,8 +160,7 @@ static struct platform_device davinci_aemif_device = {
#define DM646X_EVM_ATA_PWD BIT(1)
/* CPLD Register 0 Client: used for I/O Control */
-static int cpld_reg0_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+static int cpld_reg0_probe(struct i2c_client *client)
{
if (HAS_ATA) {
u8 data;
@@ -197,7 +196,7 @@ static const struct i2c_device_id cpld_reg_ids[] = {
static struct i2c_driver dm6467evm_cpld_driver = {
.driver.name= "cpld_reg0",
.id_table = cpld_reg_ids,
- .probe = cpld_reg0_probe,
+ .probe_new = cpld_reg0_probe,
};
/* LEDS */
@@ -402,8 +401,7 @@ static struct snd_platform_data dm646x_evm_snd_data[] = {
#ifdef CONFIG_I2C
static struct i2c_client *cpld_client;
-static int cpld_video_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+static int cpld_video_probe(struct i2c_client *client)
{
cpld_client = client;
return 0;
@@ -424,7 +422,7 @@ static struct i2c_driver cpld_video_driver = {
.driver = {
.name = "cpld_video",
},
- .probe = cpld_video_probe,
+ .probe_new = cpld_video_probe,
.remove = cpld_video_remove,
.id_table = cpld_video_id,
};
diff --git a/arch/arm/mach-s3c64xx/mach-crag6410-module.c
b/arch/arm/mach-s3c64xx/mach-crag6410-module.c
index 34f1baa10c54..43b587e79d21 100644
--- a/arch/arm/mach-s3c64xx/mach-crag6410-module.c
+++ b/arch/arm/mach-s3c64xx/mach-crag6410-module.c
@@ -378,8 +378,7 @@ static const struct {
.i2c_devs = wm2200_i2c, .num_i2c_devs = ARRAY_SIZE(wm2200_i2c) },
};
-static int wlf_gf_module_probe(struct i2c_client *i2c,
- const struct i2c_device_id *i2c_id)
+static int wlf_gf_module_probe(struct i2c_client *i2c)
{
int ret, i, j, id, rev;
@@ -432,7 +431,7 @@ static struct i2c_driver wlf_gf_module_driver = {
.driver = {
.name = "wlf-gf-module"
},
- .probe = wlf_gf_module_probe,
+ .probe_new = wlf_gf_module_probe,
.id_table = wlf_gf_module_id,
};
--
2.25.4
[PATCH] arch/powerpc: use simple i2c probe function
The i2c probe functions here don't use the id information provided in
their second argument, so the single-parameter i2c probe function
("probe_new") can be used instead.
This avoids scanning the identifier tables during probes.
Signed-off-by: Stephen Kitt
---
arch/powerpc/platforms/44x/ppc476.c| 5 ++---
arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c | 4 ++--
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/platforms/44x/ppc476.c
b/arch/powerpc/platforms/44x/ppc476.c
index cba83eee685c..07f7e3ce67b5 100644
--- a/arch/powerpc/platforms/44x/ppc476.c
+++ b/arch/powerpc/platforms/44x/ppc476.c
@@ -86,8 +86,7 @@ static void __noreturn avr_reset_system(char *cmd)
avr_halt_system(AVR_PWRCTL_RESET);
}
-static int avr_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+static int avr_probe(struct i2c_client *client)
{
avr_i2c_client = client;
ppc_md.restart = avr_reset_system;
@@ -104,7 +103,7 @@ static struct i2c_driver avr_driver = {
.driver = {
.name = "akebono-avr",
},
- .probe = avr_probe,
+ .probe_new = avr_probe,
.id_table = avr_id,
};
diff --git a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
index 0967bdfb1691..409481016928 100644
--- a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
+++ b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
@@ -142,7 +142,7 @@ static int mcu_gpiochip_remove(struct mcu *mcu)
return 0;
}
-static int mcu_probe(struct i2c_client *client, const struct i2c_device_id *id)
+static int mcu_probe(struct i2c_client *client)
{
struct mcu *mcu;
int ret;
@@ -221,7 +221,7 @@ static struct i2c_driver mcu_driver = {
.name = "mcu-mpc8349emitx",
.of_match_table = mcu_of_match_table,
},
- .probe = mcu_probe,
+ .probe_new = mcu_probe,
.remove = mcu_remove,
.id_table = mcu_ids,
};
--
2.25.4
[PATCH v2] hwmon/pmbus: use simple i2c probe function
pmbus_do_probe doesn't use the id information provided in its second
argument, so this can be removed, which then allows using the
single-parameter i2c probe function ("probe_new") for probes.
This avoids scanning the identifier tables during probes.
Drivers which didn't use the id are converted as-is; drivers which did
are modified as follows:
* if the information in i2c_client is sufficient, that's used instead
(client->name);
* configured v. probed comparisons are performed by comparing the
configured name to the detected name, instead of the ids; this
involves strcmp but is still cheaper than comparing all the device
names when scanning the tables;
* anything else is handled by calling i2c_match_id() with the same
level of error-handling (if any) as before.
Signed-off-by: Stephen Kitt
---
Changes since v1:
- i2c_device_id declarations are left unchanged;
- all drivers are converted, using client info or i2c_match_id().
drivers/hwmon/pmbus/adm1275.c | 11 +--
drivers/hwmon/pmbus/bel-pfe.c | 11 ++-
drivers/hwmon/pmbus/ibm-cffps.c| 17 +++--
drivers/hwmon/pmbus/inspur-ipsps.c | 7 +++
drivers/hwmon/pmbus/ir35221.c | 7 +++
drivers/hwmon/pmbus/ir38064.c | 7 +++
drivers/hwmon/pmbus/irps5401.c | 7 +++
drivers/hwmon/pmbus/isl68137.c | 11 ++-
drivers/hwmon/pmbus/lm25066.c | 11 ++-
drivers/hwmon/pmbus/ltc2978.c | 11 +--
drivers/hwmon/pmbus/ltc3815.c | 7 +++
drivers/hwmon/pmbus/max16064.c | 7 +++
drivers/hwmon/pmbus/max16601.c | 7 +++
drivers/hwmon/pmbus/max20730.c | 11 ++-
drivers/hwmon/pmbus/max20751.c | 7 +++
drivers/hwmon/pmbus/max31785.c | 9 -
drivers/hwmon/pmbus/max34440.c | 13 +++--
drivers/hwmon/pmbus/max8688.c | 7 +++
drivers/hwmon/pmbus/pmbus.c| 11 ++-
drivers/hwmon/pmbus/pmbus.h| 3 +--
drivers/hwmon/pmbus/pmbus_core.c | 3 +--
drivers/hwmon/pmbus/pxe1610.c | 7 +++
drivers/hwmon/pmbus/tps40422.c | 7 +++
drivers/hwmon/pmbus/tps53679.c | 11 ++-
drivers/hwmon/pmbus/ucd9000.c | 13 ++---
drivers/hwmon/pmbus/ucd9200.c | 13 ++---
drivers/hwmon/pmbus/xdpe12284.c| 7 +++
drivers/hwmon/pmbus/zl6100.c | 11 +--
28 files changed, 123 insertions(+), 131 deletions(-)
diff --git a/drivers/hwmon/pmbus/adm1275.c b/drivers/hwmon/pmbus/adm1275.c
index 19317575d1c6..7fc3a3b1b55c 100644
--- a/drivers/hwmon/pmbus/adm1275.c
+++ b/drivers/hwmon/pmbus/adm1275.c
@@ -462,8 +462,7 @@ static const struct i2c_device_id adm1275_id[] = {
};
MODULE_DEVICE_TABLE(i2c, adm1275_id);
-static int adm1275_probe(struct i2c_client *client,
-const struct i2c_device_id *id)
+static int adm1275_probe(struct i2c_client *client)
{
s32 (*config_read_fn)(const struct i2c_client *client, u8 reg);
u8 block_buffer[I2C_SMBUS_BLOCK_MAX + 1];
@@ -506,10 +505,10 @@ static int adm1275_probe(struct i2c_client *client,
return -ENODEV;
}
- if (id->driver_data != mid->driver_data)
+ if (strcmp(client->name, mid->name) != 0)
dev_notice(>dev,
"Device mismatch: Configured %s, detected %s\n",
- id->name, mid->name);
+ client->name, mid->name);
if (mid->driver_data == adm1272 || mid->driver_data == adm1278 ||
mid->driver_data == adm1293 || mid->driver_data == adm1294)
@@ -791,14 +790,14 @@ static int adm1275_probe(struct i2c_client *client,
info->R[PSC_TEMPERATURE] = coefficients[tindex].R;
}
- return pmbus_do_probe(client, id, info);
+ return pmbus_do_probe(client, info);
}
static struct i2c_driver adm1275_driver = {
.driver = {
.name = "adm1275",
},
- .probe = adm1275_probe,
+ .probe_new = adm1275_probe,
.remove = pmbus_do_remove,
.id_table = adm1275_id,
};
diff --git a/drivers/hwmon/pmbus/bel-pfe.c b/drivers/hwmon/pmbus/bel-pfe.c
index f236e18f45a5..2c5b853d6c7f 100644
--- a/drivers/hwmon/pmbus/bel-pfe.c
+++ b/drivers/hwmon/pmbus/bel-pfe.c
@@ -87,12 +87,13 @@ static struct pmbus_driver_info pfe_driver_info[] = {
},
};
-static int pfe_pmbus_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+static const struct i2c_device_id pfe_device_id[];
+
+static int pfe_pmbus_probe(struct i2c_client *client)
{
int model;
- model = (int)id->driver_data;
+ model = (int)i2c_match_id(pfe_device_id, client)->driver_data;
/*
* PFE3000-12-069RA devices may not stay in page 0 during device
@@ -104,7 +105,7 @@ static int pfe_pmbus_probe(struct i2
Re: [PATCH] hwmon/pmbus: use simple i2c probe function
On Thu, 6 Aug 2020 14:48:58 -0700, Guenter Roeck wrote:
> On 8/6/20 1:12 PM, Stephen Kitt wrote:
> > On Thu, 6 Aug 2020 12:15:55 -0700, Guenter Roeck
> > wrote:
> >> On 8/6/20 9:16 AM, Stephen Kitt wrote:
[...]
> >> Also, I am not convinced that replacements such as
> >>
> >> - { "ipsps1", 0 },
> >> + { .name = "ipsps1" },
> >>
> >> are an improvement. I would suggest to leave that alone for
> >> consistency (and to make it easier to add more devices to the
> >> various drivers if that happens in the future).
> >
> > From reading through all the drivers using id_table, it seems to me that
> > we could do away with driver_data altogether and move all that to
> > driver-local structures, in many cases covering more than just an id. By
> > only initialising the elements of the structure that are really needed, I
> > was hoping to (a) make it more obvious that driver_data isn’t used, and
> > (b) allow removing it without touching all the code again.
> >
>
> I don't see it as an improvement to replace a common data structure with
> per-driver data structures. That sounds too much like "let's re-invent
> the wheel over and over again". If that is where things are going, I'd
> rather have it implemented everywhere else first. I am ok with the other
> changes, but not with this.
I agree, and I wasn’t intending on encouraging re-inventing the wheel in each
driver. Let’s focus on probe_new for now...
What did you mean by “to make it easier to add more devices to the various
drivers if that happens in the future”? There are already many drivers with
multiple devices but no driver_data, dropping the explicit driver_data
initialisation doesn’t necessarily make it harder to add devices, does it?
Regards,
Stephen
pgp4nCMrDbOak.pgp
Description: OpenPGP digital signature
Re: [PATCH] docs: remove the 2.6 "Upgrading I2C Drivers" guide
Hi Jon, You’ll see v2 of this patch show up soon, see the context below — this is a patch on top of Mauro’s conversion of the i2c docs to .rst. Regards, Stephen On Thu, 6 Aug 2020 10:33:56 +0200, Wolfram Sang wrote: > > > Maybe because I don't have the commit in my tree? Can you rebase on top > > > of 5.8? > > > > Ah, yes, the commit is on top of Linus’ current master, following the > > merge of docs-5.9 from Jon’s tree. In 5.8 the file is a .txt file, but > > Mauro converted it to .rst for 5.9, and this patch removes the latter > > file (to avoid a merge conflict later on...). If you prefer, I can submit > > it to the docs tree instead! > > I see. Thanks for the heads up! > > > > And please also remove the reference in Documentation/i2c/index.rst > > > > Oops, yes, I’ll do that in v2 once we decide where it should go. > > I am fine with either it going via the doc-tree or you sending me v2 > again after 5.9-rc1. For the first case: > > Acked-by: Wolfram Sang > pgpJYwHaUXQo9.pgp Description: OpenPGP digital signature
Re: [PATCH] hwmon/pmbus: use simple i2c probe function
On Thu, 6 Aug 2020 12:15:55 -0700, Guenter Roeck wrote:
> On 8/6/20 9:16 AM, Stephen Kitt wrote:
> > pmbus_do_probe doesn't use the id information provided in its second
> > argument, so this can be removed, which then allows using the
> > single-parameter i2c probe function ("probe_new") for probes which
> > don't use the id information either.
> >
> > This avoids scanning the identifier tables during probes.
> >
> > Additionally, in cases where the id information (driver_data) isn't
> > used, the corresponding declarations are removed from the id_table,
> > and .name is specified explicitly.
> >
>
> The ultimate idea seems to be to remove the "old" i2c probe function
> entirely. This means we'll have to touch the various drivers again
> to make that happen if they are not converted to probe_new.
>
> With that in mind, since we are at it, why not use probe_new() in
> every driver and call i2c_match_id() in cases where it is actually
> needed/used ?
Yes, I was planning on doing that in a second phase, but I can do it right
now (perhaps as a patch series) if that would be better.
> Also, I am not convinced that replacements such as
>
> - { "ipsps1", 0 },
> + { .name = "ipsps1" },
>
> are an improvement. I would suggest to leave that alone for
> consistency (and to make it easier to add more devices to the
> various drivers if that happens in the future).
From reading through all the drivers using id_table, it seems to me that we
could do away with driver_data altogether and move all that to driver-local
structures, in many cases covering more than just an id. By only initialising
the elements of the structure that are really needed, I was hoping to (a)
make it more obvious that driver_data isn’t used, and (b) allow removing it
without touching all the code again.
Regards,
Stephen
pgpzdUlCupZo4.pgp
Description: OpenPGP digital signature
[PATCH] hwmon/pmbus: use simple i2c probe function
pmbus_do_probe doesn't use the id information provided in its second
argument, so this can be removed, which then allows using the
single-parameter i2c probe function ("probe_new") for probes which
don't use the id information either.
This avoids scanning the identifier tables during probes.
Additionally, in cases where the id information (driver_data) isn't
used, the corresponding declarations are removed from the id_table,
and .name is specified explicitly.
Signed-off-by: Stephen Kitt
---
drivers/hwmon/pmbus/bel-pfe.c | 2 +-
drivers/hwmon/pmbus/ibm-cffps.c| 2 +-
drivers/hwmon/pmbus/inspur-ipsps.c | 9 -
drivers/hwmon/pmbus/ir35221.c | 9 -
drivers/hwmon/pmbus/ir38064.c | 9 -
drivers/hwmon/pmbus/irps5401.c | 9 -
drivers/hwmon/pmbus/isl68137.c | 2 +-
drivers/hwmon/pmbus/lm25066.c | 2 +-
drivers/hwmon/pmbus/ltc2978.c | 2 +-
drivers/hwmon/pmbus/ltc3815.c | 9 -
drivers/hwmon/pmbus/max16064.c | 9 -
drivers/hwmon/pmbus/max16601.c | 9 -
drivers/hwmon/pmbus/max20730.c | 2 +-
drivers/hwmon/pmbus/max20751.c | 9 -
drivers/hwmon/pmbus/max31785.c | 13 ++---
drivers/hwmon/pmbus/max34440.c | 2 +-
drivers/hwmon/pmbus/max8688.c | 9 -
drivers/hwmon/pmbus/pmbus.c| 2 +-
drivers/hwmon/pmbus/pmbus.h| 3 +--
drivers/hwmon/pmbus/pmbus_core.c | 3 +--
drivers/hwmon/pmbus/pxe1610.c | 13 ++---
drivers/hwmon/pmbus/tps40422.c | 9 -
drivers/hwmon/pmbus/tps53679.c | 2 +-
drivers/hwmon/pmbus/ucd9000.c | 2 +-
drivers/hwmon/pmbus/ucd9200.c | 2 +-
drivers/hwmon/pmbus/xdpe12284.c| 11 +--
drivers/hwmon/pmbus/zl6100.c | 2 +-
27 files changed, 71 insertions(+), 86 deletions(-)
diff --git a/drivers/hwmon/pmbus/bel-pfe.c b/drivers/hwmon/pmbus/bel-pfe.c
index f236e18f45a5..240d86043d75 100644
--- a/drivers/hwmon/pmbus/bel-pfe.c
+++ b/drivers/hwmon/pmbus/bel-pfe.c
@@ -104,7 +104,7 @@ static int pfe_pmbus_probe(struct i2c_client *client,
i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
}
- return pmbus_do_probe(client, id, _driver_info[model]);
+ return pmbus_do_probe(client, _driver_info[model]);
}
static const struct i2c_device_id pfe_device_id[] = {
diff --git a/drivers/hwmon/pmbus/ibm-cffps.c b/drivers/hwmon/pmbus/ibm-cffps.c
index 7d300f2f338d..0215e792ec7e 100644
--- a/drivers/hwmon/pmbus/ibm-cffps.c
+++ b/drivers/hwmon/pmbus/ibm-cffps.c
@@ -519,7 +519,7 @@ static int ibm_cffps_probe(struct i2c_client *client,
}
client->dev.platform_data = _cffps_pdata;
- rc = pmbus_do_probe(client, id, _cffps_info[vs]);
+ rc = pmbus_do_probe(client, _cffps_info[vs]);
if (rc)
return rc;
diff --git a/drivers/hwmon/pmbus/inspur-ipsps.c
b/drivers/hwmon/pmbus/inspur-ipsps.c
index 42e01549184a..b97848e05ae2 100644
--- a/drivers/hwmon/pmbus/inspur-ipsps.c
+++ b/drivers/hwmon/pmbus/inspur-ipsps.c
@@ -190,15 +190,14 @@ static struct pmbus_platform_data ipsps_pdata = {
.flags = PMBUS_SKIP_STATUS_CHECK,
};
-static int ipsps_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+static int ipsps_probe(struct i2c_client *client)
{
client->dev.platform_data = _pdata;
- return pmbus_do_probe(client, id, _info);
+ return pmbus_do_probe(client, _info);
}
static const struct i2c_device_id ipsps_id[] = {
- { "ipsps1", 0 },
+ { .name = "ipsps1" },
{}
};
MODULE_DEVICE_TABLE(i2c, ipsps_id);
@@ -216,7 +215,7 @@ static struct i2c_driver ipsps_driver = {
.name = "inspur-ipsps",
.of_match_table = of_match_ptr(ipsps_of_match),
},
- .probe = ipsps_probe,
+ .probe_new = ipsps_probe,
.remove = pmbus_do_remove,
.id_table = ipsps_id,
};
diff --git a/drivers/hwmon/pmbus/ir35221.c b/drivers/hwmon/pmbus/ir35221.c
index 3eea3e006a96..e7fabafca2f1 100644
--- a/drivers/hwmon/pmbus/ir35221.c
+++ b/drivers/hwmon/pmbus/ir35221.c
@@ -67,8 +67,7 @@ static int ir35221_read_word_data(struct i2c_client *client,
int page,
return ret;
}
-static int ir35221_probe(struct i2c_client *client,
-const struct i2c_device_id *id)
+static int ir35221_probe(struct i2c_client *client)
{
struct pmbus_driver_info *info;
u8 buf[I2C_SMBUS_BLOCK_MAX];
@@ -123,11 +122,11 @@ static int ir35221_probe(struct i2c_client *client,
| PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP;
info->func[1] = info->func[0];
- return pmbus_do_probe(client, id, info);
+ return pmbus_do_probe(client, info);
}
static const struct i2c_device_id ir35221_id[] = {
- {"ir35221", 0},
+ { .name = "ir35221" },
{}
};
@@ -137
[PATCH v2] docs: remove the 2.6 "Upgrading I2C Drivers" guide
All the drivers have long since been upgraded, and all the important
information here is also included in the "Implementing I2C device
drivers" guide.
Signed-off-by: Stephen Kitt
---
Documentation/i2c/index.rst | 1 -
Documentation/i2c/upgrading-clients.rst | 285
2 files changed, 286 deletions(-)
delete mode 100644 Documentation/i2c/upgrading-clients.rst
diff --git a/Documentation/i2c/index.rst b/Documentation/i2c/index.rst
index fee4744475df..8a2ad3845191 100644
--- a/Documentation/i2c/index.rst
+++ b/Documentation/i2c/index.rst
@@ -62,7 +62,6 @@ Legacy documentation
.. toctree::
:maxdepth: 1
- upgrading-clients
old-module-parameters
.. only:: subproject and html
diff --git a/Documentation/i2c/upgrading-clients.rst
b/Documentation/i2c/upgrading-clients.rst
deleted file mode 100644
index 1708090d7b8f..
--- a/Documentation/i2c/upgrading-clients.rst
+++ /dev/null
@@ -1,285 +0,0 @@
-=
-Upgrading I2C Drivers to the new 2.6 Driver Model
-=
-
-Ben Dooks
-
-Introduction
-
-
-This guide outlines how to alter existing Linux 2.6 client drivers from
-the old to the new binding methods.
-
-
-Example old-style driver
-
-
-::
-
- struct example_state {
- struct i2c_client client;
-
- };
-
- static struct i2c_driver example_driver;
-
- static unsigned short ignore[] = { I2C_CLIENT_END };
- static unsigned short normal_addr[] = { OUR_ADDR, I2C_CLIENT_END };
-
- I2C_CLIENT_INSMOD;
-
- static int example_attach(struct i2c_adapter *adap, int addr, int kind)
- {
- struct example_state *state;
- struct device *dev = >dev; /* to use for dev_ reports */
- int ret;
-
- state = kzalloc(sizeof(struct example_state), GFP_KERNEL);
- if (state == NULL) {
- dev_err(dev, "failed to create our state\n");
- return -ENOMEM;
- }
-
- example->client.addr= addr;
- example->client.flags = 0;
- example->client.adapter = adap;
-
- i2c_set_clientdata(>i2c_client, state);
- strscpy(client->i2c_client.name, "example",
sizeof(client->i2c_client.name));
-
- ret = i2c_attach_client(>i2c_client);
- if (ret < 0) {
- dev_err(dev, "failed to attach client\n");
- kfree(state);
- return ret;
- }
-
- dev = >i2c_client.dev;
-
- /* rest of the initialisation goes here. */
-
- dev_info(dev, "example client created\n");
-
- return 0;
- }
-
- static int example_detach(struct i2c_client *client)
- {
- struct example_state *state = i2c_get_clientdata(client);
-
- i2c_detach_client(client);
- kfree(state);
- return 0;
- }
-
- static int example_attach_adapter(struct i2c_adapter *adap)
- {
- return i2c_probe(adap, _data, example_attach);
- }
-
- static struct i2c_driver example_driver = {
- .driver = {
- .owner = THIS_MODULE,
- .name = "example",
- .pm = _pm_ops,
- },
- .attach_adapter = example_attach_adapter,
- .detach_client = example_detach,
- };
-
-
-Updating the client
-
-The new style binding model will check against a list of supported
-devices and their associated address supplied by the code registering
-the busses. This means that the driver .attach_adapter and
-.detach_client methods can be removed, along with the addr_data,
-as follows::
-
- - static struct i2c_driver example_driver;
-
- - static unsigned short ignore[] = { I2C_CLIENT_END };
- - static unsigned short normal_addr[] = { OUR_ADDR, I2C_CLIENT_END };
-
- - I2C_CLIENT_INSMOD;
-
- - static int example_attach_adapter(struct i2c_adapter *adap)
- - {
- -return i2c_probe(adap, _data, example_attach);
- - }
-
-static struct i2c_driver example_driver = {
- -.attach_adapter = example_attach_adapter,
- -.detach_client = example_detach,
-}
-
-Add the probe and remove methods to the i2c_driver, as so::
-
- static struct i2c_driver example_driver = {
- +.probe = example_probe,
- +.remove = example_remove,
- }
-
-Change the example_attach method to accept the new parameters
-which include the i2c_client that it will be working with::
-
- - static int example_attach(struct i2c_adapter *adap, int addr, int kind)
- + static int example_probe(struct i2c_client *client,
- + const struct i2c_device_id *id)
-
-Change the name of example_attach to example_probe to align it with the
-i2c_driver entry names. The rest of the probe routine will now need to be
-changed as the i2c_client has already been setup for use.
-
-The necessary client fields have a
Re: [PATCH] docs: remove the 2.6 "Upgrading I2C Drivers" guide
Hi Wolfram, On Wed, 5 Aug 2020 23:53:51 +0200, Wolfram Sang wrote: > On Wed, Aug 05, 2020 at 08:31:49PM +0200, Stephen Kitt wrote: > > All the drivers have long since been upgraded, and all the important > > information here is also included in the "Implementing I2C device > > drivers" guide. > > > > Signed-off-by: Stephen Kitt > > True! Thanks. > > But I can't apply the patch? > > > base-commit: 2324d50d051ec0f14a548e78554fb02513d6dcef > > Maybe because I don't have the commit in my tree? Can you rebase on top > of 5.8? Ah, yes, the commit is on top of Linus’ current master, following the merge of docs-5.9 from Jon’s tree. In 5.8 the file is a .txt file, but Mauro converted it to .rst for 5.9, and this patch removes the latter file (to avoid a merge conflict later on...). If you prefer, I can submit it to the docs tree instead! > And please also remove the reference in Documentation/i2c/index.rst Oops, yes, I’ll do that in v2 once we decide where it should go. Regards, Stephen pgpyI_S92lBh5.pgp Description: OpenPGP digital signature
[PATCH] docs: remove the 2.6 "Upgrading I2C Drivers" guide
All the drivers have long since been upgraded, and all the important
information here is also included in the "Implementing I2C device
drivers" guide.
Signed-off-by: Stephen Kitt
---
Documentation/i2c/upgrading-clients.rst | 285
1 file changed, 285 deletions(-)
delete mode 100644 Documentation/i2c/upgrading-clients.rst
diff --git a/Documentation/i2c/upgrading-clients.rst
b/Documentation/i2c/upgrading-clients.rst
deleted file mode 100644
index 1708090d7b8f..
--- a/Documentation/i2c/upgrading-clients.rst
+++ /dev/null
@@ -1,285 +0,0 @@
-=
-Upgrading I2C Drivers to the new 2.6 Driver Model
-=
-
-Ben Dooks
-
-Introduction
-
-
-This guide outlines how to alter existing Linux 2.6 client drivers from
-the old to the new binding methods.
-
-
-Example old-style driver
-
-
-::
-
- struct example_state {
- struct i2c_client client;
-
- };
-
- static struct i2c_driver example_driver;
-
- static unsigned short ignore[] = { I2C_CLIENT_END };
- static unsigned short normal_addr[] = { OUR_ADDR, I2C_CLIENT_END };
-
- I2C_CLIENT_INSMOD;
-
- static int example_attach(struct i2c_adapter *adap, int addr, int kind)
- {
- struct example_state *state;
- struct device *dev = >dev; /* to use for dev_ reports */
- int ret;
-
- state = kzalloc(sizeof(struct example_state), GFP_KERNEL);
- if (state == NULL) {
- dev_err(dev, "failed to create our state\n");
- return -ENOMEM;
- }
-
- example->client.addr= addr;
- example->client.flags = 0;
- example->client.adapter = adap;
-
- i2c_set_clientdata(>i2c_client, state);
- strscpy(client->i2c_client.name, "example",
sizeof(client->i2c_client.name));
-
- ret = i2c_attach_client(>i2c_client);
- if (ret < 0) {
- dev_err(dev, "failed to attach client\n");
- kfree(state);
- return ret;
- }
-
- dev = >i2c_client.dev;
-
- /* rest of the initialisation goes here. */
-
- dev_info(dev, "example client created\n");
-
- return 0;
- }
-
- static int example_detach(struct i2c_client *client)
- {
- struct example_state *state = i2c_get_clientdata(client);
-
- i2c_detach_client(client);
- kfree(state);
- return 0;
- }
-
- static int example_attach_adapter(struct i2c_adapter *adap)
- {
- return i2c_probe(adap, _data, example_attach);
- }
-
- static struct i2c_driver example_driver = {
- .driver = {
- .owner = THIS_MODULE,
- .name = "example",
- .pm = _pm_ops,
- },
- .attach_adapter = example_attach_adapter,
- .detach_client = example_detach,
- };
-
-
-Updating the client
-
-The new style binding model will check against a list of supported
-devices and their associated address supplied by the code registering
-the busses. This means that the driver .attach_adapter and
-.detach_client methods can be removed, along with the addr_data,
-as follows::
-
- - static struct i2c_driver example_driver;
-
- - static unsigned short ignore[] = { I2C_CLIENT_END };
- - static unsigned short normal_addr[] = { OUR_ADDR, I2C_CLIENT_END };
-
- - I2C_CLIENT_INSMOD;
-
- - static int example_attach_adapter(struct i2c_adapter *adap)
- - {
- -return i2c_probe(adap, _data, example_attach);
- - }
-
-static struct i2c_driver example_driver = {
- -.attach_adapter = example_attach_adapter,
- -.detach_client = example_detach,
-}
-
-Add the probe and remove methods to the i2c_driver, as so::
-
- static struct i2c_driver example_driver = {
- +.probe = example_probe,
- +.remove = example_remove,
- }
-
-Change the example_attach method to accept the new parameters
-which include the i2c_client that it will be working with::
-
- - static int example_attach(struct i2c_adapter *adap, int addr, int kind)
- + static int example_probe(struct i2c_client *client,
- + const struct i2c_device_id *id)
-
-Change the name of example_attach to example_probe to align it with the
-i2c_driver entry names. The rest of the probe routine will now need to be
-changed as the i2c_client has already been setup for use.
-
-The necessary client fields have already been setup before
-the probe function is called, so the following client setup
-can be removed::
-
- -example->client.addr= addr;
- -example->client.flags = 0;
- -example->client.adapter = adap;
- -
- -strscpy(client->i2c_client.name, "example",
sizeof(client->i2c_client.name));
-
-The i2c_set_clientdata is now::
-
- -
Re: [PATCH] docs: process/index.rst: Fix reference to nonexistent document
On Sat, 18 Jul 2020 13:50:59 -0300, "Daniel W. S. Almeida"
wrote:
> From: Daniel W. S. Almeida
>
> Fix the following warning:
>
> WARNING: toctree contains reference to nonexistent document
> 'process/unaligned-memory-access'
>
> The path to the document was wrong.
>
> Signed-off-by: Daniel W. S. Almeida
Fixes: c9b54d6f362c ("docs: move other kAPI documents to core-api")
Reviewed-by: Stephen Kitt
pgpWZ1bKPx1pE.pgp
Description: OpenPGP digital signature
[PATCH] docs: sysctl/kernel: document random
This documents the random directory, based on the behaviour seen in drivers/char/random.c. Signed-off-by: Stephen Kitt --- Documentation/admin-guide/sysctl/kernel.rst | 32 + 1 file changed, 32 insertions(+) diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst index 6d96f17b74a4..5f432f51f23e 100644 --- a/Documentation/admin-guide/sysctl/kernel.rst +++ b/Documentation/admin-guide/sysctl/kernel.rst @@ -996,6 +996,38 @@ pty See Documentation/filesystems/devpts.rst. +random +== + +This is a directory, with the following entries: + +* ``boot_id``: a UUID generated the first time this is retrieved, and + unvarying after that; + +* ``entropy_avail``: the pool's entropy count, in bits; + +* ``poolsize``: the entropy pool size, in bits; + +* ``urandom_min_reseed_secs``: obsolete (used to determine the minimum + number of seconds between urandom pool reseeding). + +* ``uuid``: a UUID generated every time this is retrieved (this can + thus be used to generate UUIDs at will); + +* ``write_wakeup_threshold``: when the entropy count drops below this + (as a number of bits), processes waiting to write to ``/dev/random`` + are woken up. + +If ``drivers/char/random.c`` is built with ``ADD_INTERRUPT_BENCH`` +defined, these additional entries are present: + +* ``add_interrupt_avg_cycles``: the average number of cycles between + interrupts used to feed the pool; + +* ``add_interrupt_avg_deviation``: the standard deviation seen on the + number of cycles between interrupts used to feed the pool. + + randomize_va_space == base-commit: 46e906144c3f4b0a7b6dcc7713fafad65b1859e0 -- 2.20.1
Re: linux-next: manual merge of the jc_docs tree with the vfs and net-next trees
Le 18/05/2020 14:22, Jonathan Corbet a écrit :
On Mon, 18 May 2020 12:30:13 +1000
Stephen Rothwell wrote:
Today's linux-next merge of the jc_docs tree got a conflict in:
kernel/sysctl.c
between commit:
f461d2dcd511 ("sysctl: avoid forward declarations")
from the vfs tree and commit:
2f4c33063ad7 ("docs: sysctl/kernel: document ngroups_max")
from the jc_docs tree.
Hmm...that's somewhat messy. I somehow managed to miss the change to
kernel/sysctl.c that doesn't have much to do with documentation.
Stephen
(Kitt): I've reverted that change for now. Could I ask you to resubmit
it
as two different patches? I'll happily take the actual docs change;
the
sysctl change can be sent to the VFS tree on top of the changes there.
Done, thanks.
In general we really don't want to mix unrelated changes like this.
Noted, I’ll avoid mixing changes in future.
Regards,
Stephen
[PATCH] sysctl: const-ify ngroups_max
ngroups_max is a read-only sysctl entry, reflecting NGROUPS_MAX. Make
it const, in the same way as cap_last_cap.
Signed-off-by: Stephen Kitt
---
This is split out from 2f4c33063ad7 ("docs: sysctl/kernel: document
ngroups_max") which conflicted with f461d2dcd511 ("sysctl: avoid forward
declarations").
kernel/sysctl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 349cab382081..cc1fcba9d4d2 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -133,7 +133,7 @@ static unsigned long dirty_bytes_min = 2 * PAGE_SIZE;
static int maxolduid = 65535;
static int minolduid;
-static int ngroups_max = NGROUPS_MAX;
+static const int ngroups_max = NGROUPS_MAX;
static const int cap_last_cap = CAP_LAST_CAP;
/*
@@ -2232,7 +2232,7 @@ static struct ctl_table kern_table[] = {
#endif
{
.procname = "ngroups_max",
- .data = _max,
+ .data = (void *)_max,
.maxlen = sizeof (int),
.mode = 0444,
.proc_handler = proc_dointvec,
base-commit: bdecf38f228bcca73b31ada98b5b7ba1215eb9c9
--
2.20.1
[PATCH v2] docs: sysctl/kernel: document ngroups_max
This is a read-only export of NGROUPS_MAX. Signed-off-by: Stephen Kitt --- Changes since v1: - drop changes to kernel/sysctl.c Documentation/admin-guide/sysctl/kernel.rst | 9 + 1 file changed, 9 insertions(+) diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst index 0d427fd10941..5f12ee07665c 100644 --- a/Documentation/admin-guide/sysctl/kernel.rst +++ b/Documentation/admin-guide/sysctl/kernel.rst @@ -459,6 +459,15 @@ Notes: successful IPC object allocation. If an IPC object allocation syscall fails, it is undefined if the value remains unmodified or is reset to -1. + +ngroups_max +=== + +Maximum number of supplementary groups, _i.e._ the maximum size which +``setgroups`` will accept. Exports ``NGROUPS_MAX`` from the kernel. + + + nmi_watchdog base-commit: 1ae7efb388540adc1653a51a3bc3b2c9cef5ec1a -- 2.20.1
Re: [PATCH] docs: sysctl/kernel: document unaligned controls
On Fri, 15 May 2020 20:36:01 +0200, Stephen Kitt wrote: > On Fri, 15 May 2020 11:27:35 -0600, Jonathan Corbet wrote: > > On Fri, 15 May 2020 18:04:06 +0200 > > Stephen Kitt wrote: > > > > > diff --git a/Documentation/index.rst b/Documentation/index.rst > > > index 9599c0f3eea8..17c38d899572 100644 > > > --- a/Documentation/index.rst > > > +++ b/Documentation/index.rst > > > @@ -143,6 +143,7 @@ Architecture-agnostic documentation > > > :maxdepth: 2 > > > > > > asm-annotations > > > + unaligned-memory-access > > > > > > Architecture-specific documentation > > > --- > > > diff --git a/Documentation/unaligned-memory-access.txt > > > b/Documentation/unaligned-memory-access.rst similarity index 100% > > > rename from Documentation/unaligned-memory-access.txt > > > rename to Documentation/unaligned-memory-access.rst > > > > Adding this to the toctree is great, but I'd just as soon not leave it in > > the top-level directory while we do that. Since you're renaming it > > anyway, can you move it into process/? It's not a perfect fit, but that's > > where that type of material has been going so far. > > I can indeed. Should it still be listed in the main toctree, or in the > process toctree? Never mind, I found the answer, “some overall technical guides that have been put here for now for lack of a better place” ;-). Regards, Stephen pgpn6aiJolbX_.pgp Description: OpenPGP digital signature
[PATCH v2] docs: sysctl/kernel: document unaligned controls
This documents ignore-unaligned-usertrap, unaligned-dump-stack, and
unaligned-trap, based on arch/arc/kernel/unaligned.c,
arch/ia64/kernel/unaligned.c, and arch/parisc/kernel/unaligned.c.
While we're at it, integrate unaligned-memory-access.txt into the docs
tree.
Signed-off-by: Stephen Kitt
---
Changes since v1:
- move unaligned-memory-access.txt to process/
- removed UTF-8 apostrophe from the commit message
Documentation/admin-guide/sysctl/kernel.rst | 51 +++
Documentation/process/index.rst | 1 +
.../unaligned-memory-access.rst} | 0
3 files changed, 52 insertions(+)
rename Documentation/{unaligned-memory-access.txt =>
process/unaligned-memory-access.rst} (100%)
diff --git a/Documentation/admin-guide/sysctl/kernel.rst
b/Documentation/admin-guide/sysctl/kernel.rst
index eb6bc9cc0318..4bb4d55f20ff 100644
--- a/Documentation/admin-guide/sysctl/kernel.rst
+++ b/Documentation/admin-guide/sysctl/kernel.rst
@@ -402,6 +402,25 @@ Controls whether the panic kmsg data should be reported to
Hyper-V.
= =
+ignore-unaligned-usertrap
+=
+
+On architectures where unaligned accesses cause traps, and where this
+feature is supported (``CONFIG_SYSCTL_ARCH_UNALIGN_NO_WARN``;
+currently, ``arc`` and ``ia64``), controls whether all unaligned traps
+are logged.
+
+= =
+0 Log all unaligned accesses.
+1 Only warn the first time a process traps. This is the default
+ setting.
+= =
+
+See also `unaligned-trap`_ and `unaligned-dump-stack`_. On ``ia64``,
+this allows system administrators to override the
+``IA64_THREAD_UAC_NOPRINT`` ``prctl`` and avoid logs being flooded.
+
+
kexec_load_disabled
===
@@ -1252,6 +1271,38 @@ See :doc:`/admin-guide/kernel-parameters` and
:doc:`/trace/boottime-trace`.
+.. _unaligned-dump-stack:
+
+unaligned-dump-stack (ia64)
+===
+
+When logging unaligned accesses, controls whether the stack is
+dumped.
+
+= ===
+0 Do not dump the stack. This is the default setting.
+1 Dump the stack.
+= ===
+
+See also `ignore-unaligned-usertrap`_.
+
+
+unaligned-trap
+==
+
+On architectures where unaligned accesses cause traps, and where this
+feature is supported (``CONFIG_SYSCTL_ARCH_UNALIGN_ALLOW``; currently,
+``arc`` and ``parisc``), controls whether unaligned traps are caught
+and emulated (instead of failing).
+
+=
+0 Do not emulate unaligned accesses.
+1 Emulate unaligned accesses. This is the default setting.
+=
+
+See also `ignore-unaligned-usertrap`_.
+
+
unknown_nmi_panic
=
diff --git a/Documentation/process/index.rst b/Documentation/process/index.rst
index 6399d92f0b21..f07c9250c3ac 100644
--- a/Documentation/process/index.rst
+++ b/Documentation/process/index.rst
@@ -61,6 +61,7 @@ lack of a better place.
botching-up-ioctls
clang-format
../riscv/patch-acceptance
+ unaligned-memory-access
.. only:: subproject and html
diff --git a/Documentation/unaligned-memory-access.txt
b/Documentation/process/unaligned-memory-access.rst
similarity index 100%
rename from Documentation/unaligned-memory-access.txt
rename to Documentation/process/unaligned-memory-access.rst
base-commit: 56b62540782bfde459acc8eb15b949eaf151c881
--
2.20.1
Re: [PATCH] docs: sysctl/kernel: document unaligned controls
On Fri, 15 May 2020 11:27:35 -0600, Jonathan Corbet wrote: > On Fri, 15 May 2020 18:04:06 +0200 > Stephen Kitt wrote: > > > diff --git a/Documentation/index.rst b/Documentation/index.rst > > index 9599c0f3eea8..17c38d899572 100644 > > --- a/Documentation/index.rst > > +++ b/Documentation/index.rst > > @@ -143,6 +143,7 @@ Architecture-agnostic documentation > > :maxdepth: 2 > > > > asm-annotations > > + unaligned-memory-access > > > > Architecture-specific documentation > > --- > > diff --git a/Documentation/unaligned-memory-access.txt > > b/Documentation/unaligned-memory-access.rst similarity index 100% > > rename from Documentation/unaligned-memory-access.txt > > rename to Documentation/unaligned-memory-access.rst > > Adding this to the toctree is great, but I'd just as soon not leave it in > the top-level directory while we do that. Since you're renaming it > anyway, can you move it into process/? It's not a perfect fit, but that's > where that type of material has been going so far. I can indeed. Should it still be listed in the main toctree, or in the process toctree? Regards, Stephen pgpSzbvzYB3i0.pgp Description: OpenPGP digital signature
[PATCH] docs: sysctl/kernel: document unaligned controls
This documents ignore-unaligned-usertrap, unaligned-dump-stack, and
unaligned-trap, based on arch/arc/kernel/unaligned.c,
arch/ia64/kernel/unaligned.c, and arch/parisc/kernel/unaligned.c.
While we’re at it, integrate unaligned-memory-access.txt into the docs
tree.
Signed-off-by: Stephen Kitt
---
Documentation/admin-guide/sysctl/kernel.rst | 51 +++
Documentation/index.rst | 1 +
...access.txt => unaligned-memory-access.rst} | 0
3 files changed, 52 insertions(+)
rename Documentation/{unaligned-memory-access.txt =>
unaligned-memory-access.rst} (100%)
diff --git a/Documentation/admin-guide/sysctl/kernel.rst
b/Documentation/admin-guide/sysctl/kernel.rst
index eb6bc9cc0318..4bb4d55f20ff 100644
--- a/Documentation/admin-guide/sysctl/kernel.rst
+++ b/Documentation/admin-guide/sysctl/kernel.rst
@@ -402,6 +402,25 @@ Controls whether the panic kmsg data should be reported to
Hyper-V.
= =
+ignore-unaligned-usertrap
+=
+
+On architectures where unaligned accesses cause traps, and where this
+feature is supported (``CONFIG_SYSCTL_ARCH_UNALIGN_NO_WARN``;
+currently, ``arc`` and ``ia64``), controls whether all unaligned traps
+are logged.
+
+= =
+0 Log all unaligned accesses.
+1 Only warn the first time a process traps. This is the default
+ setting.
+= =
+
+See also `unaligned-trap`_ and `unaligned-dump-stack`_. On ``ia64``,
+this allows system administrators to override the
+``IA64_THREAD_UAC_NOPRINT`` ``prctl`` and avoid logs being flooded.
+
+
kexec_load_disabled
===
@@ -1252,6 +1271,38 @@ See :doc:`/admin-guide/kernel-parameters` and
:doc:`/trace/boottime-trace`.
+.. _unaligned-dump-stack:
+
+unaligned-dump-stack (ia64)
+===
+
+When logging unaligned accesses, controls whether the stack is
+dumped.
+
+= ===
+0 Do not dump the stack. This is the default setting.
+1 Dump the stack.
+= ===
+
+See also `ignore-unaligned-usertrap`_.
+
+
+unaligned-trap
+==
+
+On architectures where unaligned accesses cause traps, and where this
+feature is supported (``CONFIG_SYSCTL_ARCH_UNALIGN_ALLOW``; currently,
+``arc`` and ``parisc``), controls whether unaligned traps are caught
+and emulated (instead of failing).
+
+=
+0 Do not emulate unaligned accesses.
+1 Emulate unaligned accesses. This is the default setting.
+=
+
+See also `ignore-unaligned-usertrap`_.
+
+
unknown_nmi_panic
=
diff --git a/Documentation/index.rst b/Documentation/index.rst
index 9599c0f3eea8..17c38d899572 100644
--- a/Documentation/index.rst
+++ b/Documentation/index.rst
@@ -143,6 +143,7 @@ Architecture-agnostic documentation
:maxdepth: 2
asm-annotations
+ unaligned-memory-access
Architecture-specific documentation
---
diff --git a/Documentation/unaligned-memory-access.txt
b/Documentation/unaligned-memory-access.rst
similarity index 100%
rename from Documentation/unaligned-memory-access.txt
rename to Documentation/unaligned-memory-access.rst
base-commit: 56b62540782bfde459acc8eb15b949eaf151c881
--
2.20.1
[PATCH] docs: sysctl/kernel: document ngroups_max
This is a read-only export of NGROUPS_MAX, so this patch also changes
the declarations in kernel/sysctl.c to const.
Signed-off-by: Stephen Kitt
---
Documentation/admin-guide/sysctl/kernel.rst | 9 +
kernel/sysctl.c | 4 ++--
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/Documentation/admin-guide/sysctl/kernel.rst
b/Documentation/admin-guide/sysctl/kernel.rst
index 0d427fd10941..5f12ee07665c 100644
--- a/Documentation/admin-guide/sysctl/kernel.rst
+++ b/Documentation/admin-guide/sysctl/kernel.rst
@@ -459,6 +459,15 @@ Notes:
successful IPC object allocation. If an IPC object allocation syscall
fails, it is undefined if the value remains unmodified or is reset to -1.
+
+ngroups_max
+===
+
+Maximum number of supplementary groups, _i.e._ the maximum size which
+``setgroups`` will accept. Exports ``NGROUPS_MAX`` from the kernel.
+
+
+
nmi_watchdog
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 8a176d8727a3..2ba9f449d273 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -146,7 +146,7 @@ static unsigned long dirty_bytes_min = 2 * PAGE_SIZE;
static int maxolduid = 65535;
static int minolduid;
-static int ngroups_max = NGROUPS_MAX;
+static const int ngroups_max = NGROUPS_MAX;
static const int cap_last_cap = CAP_LAST_CAP;
/*
@@ -883,7 +883,7 @@ static struct ctl_table kern_table[] = {
#endif
{
.procname = "ngroups_max",
- .data = _max,
+ .data = (void *)_max,
.maxlen = sizeof (int),
.mode = 0444,
.proc_handler = proc_dointvec,
base-commit: 1ae7efb388540adc1653a51a3bc3b2c9cef5ec1a
--
2.20.1
Re: [PATCH v2] net: Protect INET_ADDR_COOKIE on 32-bit architectures
On Sat, 9 May 2020 21:05:48 +0200, Stephen Kitt wrote: > On Sat, 9 May 2020 10:59:14 -0700, Jakub Kicinski wrote: > > What if we went back to your original proposal of an empty struct but > > added in an extern in front? That way we should get linker error on > > pointer references. > > That silently fails to fail if any other link object provides a definition > for the symbol, even if the type doesn’t match... And it breaks the build if INET_ADDR_COOKIE is used twice in the same unit, e.g. in inet_hashtables.c. Regards, Stephen pgpA0fVQ_ZGh8.pgp Description: OpenPGP digital signature
Re: [PATCH v2] net: Protect INET_ADDR_COOKIE on 32-bit architectures
On Sat, 9 May 2020 10:59:14 -0700, Jakub Kicinski wrote:
> On Sat, 9 May 2020 10:13:22 +0200 Stephen Kitt wrote:
> > On Fri, 8 May 2020 20:50:25 -0700, Jakub Kicinski
> > wrote:
> > > On Fri, 8 May 2020 14:04:57 +0200 Stephen Kitt wrote:
> > > > Commit c7228317441f ("net: Use a more standard macro for
> > > > INET_ADDR_COOKIE") added a __deprecated marker to the cookie name on
> > > > 32-bit architectures, with the intent that the compiler would flag
> > > > uses of the name. However since commit 771c035372a0 ("deprecate the
> > > > '__deprecated' attribute warnings entirely and for good"),
> > > > __deprecated doesn't do anything and should be avoided.
> > > >
> > > > This patch changes INET_ADDR_COOKIE to declare a dummy struct so that
> > > > any subsequent use of the cookie's name will in all likelihood break
> > > > the build. It also removes the __deprecated marker.
> > >
> > > I think the macro is supposed to cause a warning when the variable
> > > itself is accessed. And I don't think that happens with your patch
> > > applied.
> >
> > Yes, the warning is what was lost when __deprecated lost its meaning. I
> > was trying to preserve that, or rather extend it so that the build would
> > break if the cookie was used on 32-bit architectures, and my patch
> > ensures it does if the cookie is used in a comparison or assignment,
> > but ...
> > > + kfree();
> >
> > I hadn’t thought of taking a pointer to it.
> >
> > If we want to preserve the use of the macro with a semi-colon, which is
> > what Joe’s patch introduced (along with the deprecation warning), we
> > still need some sort of declaration which can’t be used. Perhaps
> >
> > #define INET_ADDR_COOKIE(__name, __saddr, __daddr) \
> > struct __name {} __attribute__((unused))
> >
> > would be better — it declares the cookie as a struct, not a variable, so
> > then the build fails if the cookie is used as anything other than a
> > struct. If anyone does try to use it as a struct, the build will fail on
> > 64-bit architectures...
> >
> > CC net/ipv4/inet_hashtables.o
> > net/ipv4/inet_hashtables.c: In function ‘__inet_lookup_established’:
> > net/ipv4/inet_hashtables.c:362:9: error: ‘acookie’ undeclared (first use
> > in this function) kfree();
> > ^~~
> > net/ipv4/inet_hashtables.c:362:9: note: each undeclared identifier is
> > reported only once for each function it appears in make[2]: ***
> > [scripts/Makefile.build:267: net/ipv4/inet_hashtables.o] Error 1 make[1]:
> > *** [scripts/Makefile.build:488: net/ipv4] Error 2 make: ***
> > Makefile:1722: net] Error 2
>
> Hm. That does seem better. Although thinking about it - we will not get
> a warning when someone declares a variable with the same name..
Good point!
> What if we went back to your original proposal of an empty struct but
> added in an extern in front? That way we should get linker error on
> pointer references.
That silently fails to fail if any other link object provides a definition
for the symbol, even if the type doesn’t match...
I thought of
register struct {} __name __attribute__((unused))
but that really feels like tacking on more stuff to handle cases as we think
of them, which makes me wonder what cases I’m not thinking of.
Regards,
Stephen
pgpXLGFD6G1Hb.pgp
Description: OpenPGP digital signature
Re: [PATCH v2] net: Protect INET_ADDR_COOKIE on 32-bit architectures
Hi,
Thanks for taking the time to review my patch.
On Fri, 8 May 2020 20:50:25 -0700, Jakub Kicinski wrote:
> On Fri, 8 May 2020 14:04:57 +0200 Stephen Kitt wrote:
> > Commit c7228317441f ("net: Use a more standard macro for
> > INET_ADDR_COOKIE") added a __deprecated marker to the cookie name on
> > 32-bit architectures, with the intent that the compiler would flag
> > uses of the name. However since commit 771c035372a0 ("deprecate the
> > '__deprecated' attribute warnings entirely and for good"),
> > __deprecated doesn't do anything and should be avoided.
> >
> > This patch changes INET_ADDR_COOKIE to declare a dummy struct so that
> > any subsequent use of the cookie's name will in all likelihood break
> > the build. It also removes the __deprecated marker.
>
> I think the macro is supposed to cause a warning when the variable
> itself is accessed. And I don't think that happens with your patch
> applied.
Yes, the warning is what was lost when __deprecated lost its meaning. I was
trying to preserve that, or rather extend it so that the build would break if
the cookie was used on 32-bit architectures, and my patch ensures it does if
the cookie is used in a comparison or assignment, but ...
> + kfree();
I hadn’t thought of taking a pointer to it.
If we want to preserve the use of the macro with a semi-colon, which is what
Joe’s patch introduced (along with the deprecation warning), we still need
some sort of declaration which can’t be used. Perhaps
#define INET_ADDR_COOKIE(__name, __saddr, __daddr) \
struct __name {} __attribute__((unused))
would be better — it declares the cookie as a struct, not a variable, so then
the build fails if the cookie is used as anything other than a struct. If
anyone does try to use it as a struct, the build will fail on 64-bit
architectures...
CC net/ipv4/inet_hashtables.o
net/ipv4/inet_hashtables.c: In function ‘__inet_lookup_established’:
net/ipv4/inet_hashtables.c:362:9: error: ‘acookie’ undeclared (first use in
this function)
kfree();
^~~
net/ipv4/inet_hashtables.c:362:9: note: each undeclared identifier is reported
only once for each function it appears in
make[2]: *** [scripts/Makefile.build:267: net/ipv4/inet_hashtables.o] Error 1
make[1]: *** [scripts/Makefile.build:488: net/ipv4] Error 2
make: *** Makefile:1722: net] Error 2
Regards,
Stephen
pgpWvNgiFKYvG.pgp
Description: OpenPGP digital signature
Re: [PATCH] net: Protect INET_ADDR_COOKIE on 32-bit architectures
On Tue, 28 Apr 2020 14:07:46 -0700 (PDT), David Miller wrote: > From: Stephen Kitt > Date: Tue, 28 Apr 2020 09:52:31 +0200 > > > This patch changes INET_ADDR_COOKIE to declare a dummy typedef (so it > > makes checkpatch.pl complain, sorry...) > > This is trading one problem for another, so in the end doesn't really > move us forward. Right, a dummy struct is probably better, I’ll send a v2. Regards, Stephen pgpgCR4CjEsv9.pgp Description: OpenPGP digital signature
[PATCH v2] net: Protect INET_ADDR_COOKIE on 32-bit architectures
Commit c7228317441f ("net: Use a more standard macro for
INET_ADDR_COOKIE") added a __deprecated marker to the cookie name on
32-bit architectures, with the intent that the compiler would flag
uses of the name. However since commit 771c035372a0 ("deprecate the
'__deprecated' attribute warnings entirely and for good"),
__deprecated doesn't do anything and should be avoided.
This patch changes INET_ADDR_COOKIE to declare a dummy struct so that
any subsequent use of the cookie's name will in all likelihood break
the build. It also removes the __deprecated marker.
Signed-off-by: Stephen Kitt
---
Changes since v1:
- use a dummy struct rather than a typedef
include/net/inet_hashtables.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/net/inet_hashtables.h b/include/net/inet_hashtables.h
index ad64ba6a057f..889d9b00c905 100644
--- a/include/net/inet_hashtables.h
+++ b/include/net/inet_hashtables.h
@@ -301,8 +301,9 @@ static inline struct sock *inet_lookup_listener(struct net
*net,
((__sk)->sk_bound_dev_if == (__sdif)))&& \
net_eq(sock_net(__sk), (__net)))
#else /* 32-bit arch */
+/* Break the build if anything tries to use the cookie's name. */
#define INET_ADDR_COOKIE(__name, __saddr, __daddr) \
- const int __name __deprecated __attribute__((unused))
+ struct {} __name __attribute__((unused))
#define INET_MATCH(__sk, __net, __cookie, __saddr, __daddr, __ports, __dif,
__sdif) \
(((__sk)->sk_portpair == (__ports)) && \
--
2.20.1
[PATCH v2 1/2] docs: sysctl/kernel: document ftrace entries
Based on the ftrace documentation, the tp_printk boot parameter
documentation, and the implementation in kernel/trace/trace.c.
Signed-off-by: Stephen Kitt
---
Changes since v2:
- spelling and grammar fixes in ftrace_dump_on_oops ("outputing it"
-> "outputting them")
- "and::" in a single line in tracepoint_printk
Documentation/admin-guide/sysctl/kernel.rst | 48 +
1 file changed, 48 insertions(+)
diff --git a/Documentation/admin-guide/sysctl/kernel.rst
b/Documentation/admin-guide/sysctl/kernel.rst
index 82bfd5892663..b1ad4c86274d 100644
--- a/Documentation/admin-guide/sysctl/kernel.rst
+++ b/Documentation/admin-guide/sysctl/kernel.rst
@@ -265,6 +265,27 @@ domain names are in general different. For a detailed
discussion
see the ``hostname(1)`` man page.
+ftrace_dump_on_oops
+===
+
+Determines whether ``ftrace_dump()`` should be called on an oops (or
+kernel panic). This will output the contents of the ftrace buffers to
+the console. This is very useful for capturing traces that lead to
+crashes and outputting them to a serial console.
+
+= ===
+0 Disabled (default).
+1 Dump buffers of all CPUs.
+2 Dump the buffer of the CPU that triggered the oops.
+= ===
+
+
+ftrace_enabled, stack_tracer_enabled
+
+
+See :doc:`/trace/ftrace`.
+
+
hardlockup_all_cpu_backtrace
@@ -1191,6 +1212,33 @@ If a value outside of this range is written to
``threads-max`` an
``EINVAL`` error occurs.
+traceoff_on_warning
+===
+
+When set, disables tracing (see :doc:`/trace/ftrace`) when a
+``WARN()`` is hit.
+
+
+tracepoint_printk
+=
+
+When tracepoints are sent to printk() (enabled by the ``tp_printk``
+boot parameter), this entry provides runtime control::
+
+echo 0 > /proc/sys/kernel/tracepoint_printk
+
+will stop tracepoints from being sent to printk(), and::
+
+echo 1 > /proc/sys/kernel/tracepoint_printk
+
+will send them to printk() again.
+
+This only works if the kernel was booted with ``tp_printk`` enabled.
+
+See :doc:`/admin-guide/kernel-parameters` and
+:doc:`/trace/boottime-trace`.
+
+
unknown_nmi_panic
=
--
2.20.1
[PATCH v2 2/2] docs: sysctl/kernel: document firmware_config
Based on the firmware fallback mechanisms documentation and the implementation in drivers/base/firmware_loader/fallback.c. Signed-off-by: Stephen Kitt --- Documentation/admin-guide/sysctl/kernel.rst | 13 + 1 file changed, 13 insertions(+) diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst index b1ad4c86274d..1779455fb2c5 100644 --- a/Documentation/admin-guide/sysctl/kernel.rst +++ b/Documentation/admin-guide/sysctl/kernel.rst @@ -265,6 +265,19 @@ domain names are in general different. For a detailed discussion see the ``hostname(1)`` man page. +firmware_config +=== + +See :doc:`/driver-api/firmware/fallback-mechanisms`. + +The entries in this directory allow the firmware loader helper +fallback to be controlled: + +* ``force_sysfs_fallback``, when set to 1, forces the use of the + fallback; +* ``ignore_sysfs_fallback``, when set to 1, ignores any fallback. + + ftrace_dump_on_oops === -- 2.20.1
Re: [PATCH 2/3] docs: sysctl/kernel: document ftrace entries
On Tue, 28 Apr 2020 12:41:33 -0600, Jonathan Corbet wrote: > On Thu, 23 Apr 2020 20:36:50 +0200 > Stephen Kitt wrote: > > Based on the ftrace documentation, the tp_printk boot parameter > > documentation, and the implementation in kernel/trace/trace.c. > > > > Signed-off-by: Stephen Kitt > > This one could benefit from an ack from Steve (CC'd). Also one other > little nit below: Thanks (and thanks for the review Steve). I’ll follow up with v2 shortly, fixing the spelling and grammar here: > > +the console. This is very useful for capturing traces that lead to > > +crashes and outputing it to a serial console. in addition to this: > > +will stop tracepoints from being sent to printk(), and > > + > > +:: > > I would just make that ", and::" and avoid the separate line. Regards, Stephen pgp9Se7DJrbaM.pgp Description: OpenPGP digital signature
[PATCH] net: Protect INET_ADDR_COOKIE on 32-bit architectures
Commit c7228317441f ("net: Use a more standard macro for
INET_ADDR_COOKIE") added a __deprecated marker to the cookie name on
32-bit architectures, with the intent that the compiler would flag
uses of the name. However since commit 771c035372a0 ("deprecate the
'__deprecated' attribute warnings entirely and for good"),
__deprecated doesn't do anything and should be avoided.
This patch changes INET_ADDR_COOKIE to declare a dummy typedef (so it
makes checkpatch.pl complain, sorry...) so that any subsequent use of
the cookie's name will in all likelihood break the build. It also
removes the __deprecated marker.
Signed-off-by: Stephen Kitt
---
include/net/inet_hashtables.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/net/inet_hashtables.h b/include/net/inet_hashtables.h
index ad64ba6a057f..8a1391d82406 100644
--- a/include/net/inet_hashtables.h
+++ b/include/net/inet_hashtables.h
@@ -301,8 +301,9 @@ static inline struct sock *inet_lookup_listener(struct net
*net,
((__sk)->sk_bound_dev_if == (__sdif)))&& \
net_eq(sock_net(__sk), (__net)))
#else /* 32-bit arch */
+/* Break the build if anything tries to use the cookie's name. */
#define INET_ADDR_COOKIE(__name, __saddr, __daddr) \
- const int __name __deprecated __attribute__((unused))
+ typedef void __name __attribute__((unused))
#define INET_MATCH(__sk, __net, __cookie, __saddr, __daddr, __ports, __dif,
__sdif) \
(((__sk)->sk_portpair == (__ports)) && \
--
2.20.1
Re: [PATCH v2] clk/ti/adpll: allocate room for terminating null
On Thu, 17 Oct 2019 08:48:53 -0700, Stephen Boyd wrote:
> Quoting Stephen Kitt (2019-09-27 11:05:59)
> > The buffer allocated in ti_adpll_clk_get_name doesn't account for the
> > terminating null. This patch switches to ka_sprintf to avoid
> > overflowing.
> >
> > Signed-off-by: Stephen Kitt
> > ---
> > drivers/clk/ti/adpll.c | 10 ++
> > 1 file changed, 2 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/clk/ti/adpll.c b/drivers/clk/ti/adpll.c
> > index fdfb90058504..021cf9e2b4db 100644
> > --- a/drivers/clk/ti/adpll.c
> > +++ b/drivers/clk/ti/adpll.c
> > @@ -195,14 +195,8 @@ static const char *ti_adpll_clk_get_name(struct
> > ti_adpll_data *d, return NULL;
> > } else {
> > const char *base_name = "adpll";
>
> This is used once.
>
> > - char *buf;
> > -
> > - buf = devm_kzalloc(d->dev, 8 + 1 + strlen(base_name) + 1 +
> > - strlen(postfix), GFP_KERNEL);
> > - if (!buf)
> > - return NULL;
> > - sprintf(buf, "%08lx.%s.%s", d->pa, base_name, postfix);
> > - name = buf;
> > + name = devm_kasprintf(d->dev, GFP_KERNEL, "%08lx.%s.%s",
>
> So why not make this "%08lx.adpll.%s"?
Thanks for the review! I hesitated to do this because I thought the purely
formatting string "%08lx.%s.%s" made the resulting code easier to understand
than a combined "%08lx.adpll.%s". I’ll follow up with a v3 which merges the
"adpll" string into the format string.
Regards,
Stephen
pgpURlvLzUhji.pgp
Description: OpenPGP digital signature
[PATCH v3] clk/ti/adpll: allocate room for terminating null
The buffer allocated in ti_adpll_clk_get_name doesn't account for the
terminating null. This patch switches to devm_kasprintf to avoid
overflowing.
Signed-off-by: Stephen Kitt
---
Changes since v2:
- Move "adpll" into the format string and drop base_name entirely.
Changes since v1:
- Use devm_kasprintf instead of manually allocating the target
buffer.
---
drivers/clk/ti/adpll.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/clk/ti/adpll.c b/drivers/clk/ti/adpll.c
index fdfb90058504..bb2f2836dab2 100644
--- a/drivers/clk/ti/adpll.c
+++ b/drivers/clk/ti/adpll.c
@@ -194,15 +194,8 @@ static const char *ti_adpll_clk_get_name(struct
ti_adpll_data *d,
if (err)
return NULL;
} else {
- const char *base_name = "adpll";
- char *buf;
-
- buf = devm_kzalloc(d->dev, 8 + 1 + strlen(base_name) + 1 +
- strlen(postfix), GFP_KERNEL);
- if (!buf)
- return NULL;
- sprintf(buf, "%08lx.%s.%s", d->pa, base_name, postfix);
- name = buf;
+ name = devm_kasprintf(d->dev, GFP_KERNEL, "%08lx.adpll.%s",
+ d->pa, postfix);
}
return name;
--
2.20.1
[PATCH] drivers/clk: convert VL struct to struct_size
There are a few manually-calculated variable-length struct allocations
left, this converts them to use struct_size.
Signed-off-by: Stephen Kitt
---
drivers/clk/at91/sckc.c | 3 +--
drivers/clk/imgtec/clk-boston.c | 3 +--
drivers/clk/ingenic/tcu.c | 3 +--
drivers/clk/mvebu/ap-cpu-clk.c | 4 ++--
drivers/clk/mvebu/cp110-system-controller.c | 4 ++--
drivers/clk/samsung/clk.c | 3 +--
drivers/clk/uniphier/clk-uniphier-core.c| 3 +--
7 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/clk/at91/sckc.c b/drivers/clk/at91/sckc.c
index 9bfe9a28294a..5ad6180449cb 100644
--- a/drivers/clk/at91/sckc.c
+++ b/drivers/clk/at91/sckc.c
@@ -478,8 +478,7 @@ static void __init of_sam9x60_sckc_setup(struct device_node
*np)
if (IS_ERR(slow_osc))
goto unregister_slow_rc;
- clk_data = kzalloc(sizeof(*clk_data) + (2 * sizeof(struct clk_hw *)),
- GFP_KERNEL);
+ clk_data = kzalloc(struct_size(clk_data, hws, 2), GFP_KERNEL);
if (!clk_data)
goto unregister_slow_osc;
diff --git a/drivers/clk/imgtec/clk-boston.c b/drivers/clk/imgtec/clk-boston.c
index 33ab4ff61165..b00cbd045af5 100644
--- a/drivers/clk/imgtec/clk-boston.c
+++ b/drivers/clk/imgtec/clk-boston.c
@@ -58,8 +58,7 @@ static void __init clk_boston_setup(struct device_node *np)
cpu_div = ext_field(mmcmdiv, BOSTON_PLAT_MMCMDIV_CLK1DIV);
cpu_freq = mult_frac(in_freq, mul, cpu_div);
- onecell = kzalloc(sizeof(*onecell) +
- (BOSTON_CLK_COUNT * sizeof(struct clk_hw *)),
+ onecell = kzalloc(struct_size(onecell, hws, BOSTON_CLK_COUNT),
GFP_KERNEL);
if (!onecell)
return;
diff --git a/drivers/clk/ingenic/tcu.c b/drivers/clk/ingenic/tcu.c
index a1a5f9cb439e..ad7daa494fd4 100644
--- a/drivers/clk/ingenic/tcu.c
+++ b/drivers/clk/ingenic/tcu.c
@@ -358,8 +358,7 @@ static int __init ingenic_tcu_probe(struct device_node *np)
}
}
- tcu->clocks = kzalloc(sizeof(*tcu->clocks) +
- sizeof(*tcu->clocks->hws) * TCU_CLK_COUNT,
+ tcu->clocks = kzalloc(struct_size(tcu->clocks, hws, TCU_CLK_COUNT),
GFP_KERNEL);
if (!tcu->clocks) {
ret = -ENOMEM;
diff --git a/drivers/clk/mvebu/ap-cpu-clk.c b/drivers/clk/mvebu/ap-cpu-clk.c
index af5e5acad370..6b394302c76a 100644
--- a/drivers/clk/mvebu/ap-cpu-clk.c
+++ b/drivers/clk/mvebu/ap-cpu-clk.c
@@ -274,8 +274,8 @@ static int ap_cpu_clock_probe(struct platform_device *pdev)
if (!ap_cpu_clk)
return -ENOMEM;
- ap_cpu_data = devm_kzalloc(dev, sizeof(*ap_cpu_data) +
- sizeof(struct clk_hw *) * nclusters,
+ ap_cpu_data = devm_kzalloc(dev, struct_size(ap_cpu_data, hws,
+ nclusters),
GFP_KERNEL);
if (!ap_cpu_data)
return -ENOMEM;
diff --git a/drivers/clk/mvebu/cp110-system-controller.c
b/drivers/clk/mvebu/cp110-system-controller.c
index 808463276145..84c8900542e4 100644
--- a/drivers/clk/mvebu/cp110-system-controller.c
+++ b/drivers/clk/mvebu/cp110-system-controller.c
@@ -235,8 +235,8 @@ static int cp110_syscon_common_probe(struct platform_device
*pdev,
if (ret)
return ret;
- cp110_clk_data = devm_kzalloc(dev, sizeof(*cp110_clk_data) +
- sizeof(struct clk_hw *) * CP110_CLK_NUM,
+ cp110_clk_data = devm_kzalloc(dev, struct_size(cp110_clk_data, hws,
+ CP110_CLK_NUM),
GFP_KERNEL);
if (!cp110_clk_data)
return -ENOMEM;
diff --git a/drivers/clk/samsung/clk.c b/drivers/clk/samsung/clk.c
index e544a38106dd..dad31308c071 100644
--- a/drivers/clk/samsung/clk.c
+++ b/drivers/clk/samsung/clk.c
@@ -60,8 +60,7 @@ struct samsung_clk_provider *__init samsung_clk_init(struct
device_node *np,
struct samsung_clk_provider *ctx;
int i;
- ctx = kzalloc(sizeof(struct samsung_clk_provider) +
- sizeof(*ctx->clk_data.hws) * nr_clks, GFP_KERNEL);
+ ctx = kzalloc(struct_size(ctx, clk_data.hws, nr_clks), GFP_KERNEL);
if (!ctx)
panic("could not allocate clock provider context.\n");
diff --git a/drivers/clk/uniphier/clk-uniphier-core.c
b/drivers/clk/uniphier/clk-uniphier-core.c
index c6aaca73cf86..12380236d7ab 100644
--- a/drivers/clk/uniphier/clk-uniphier-core.c
+++ b/drivers/clk/uniphier/clk-uniphier-core.c
@@ -64,8 +64,7 @@ static int uniphier_clk_probe(struct platform_device *pdev)
for (p = data; p->name; p++)
clk_num = max(clk_num, p->idx + 1);
- hw_data = devm_kzalloc(dev,
Re: [PATCH] clk/ti/adpll: allocate room for terminating null
Le 27/09/2019 17:23, Tony Lindgren a écrit :
* Stephen Kitt [190927 15:13]:
The buffer allocated in ti_adpll_clk_get_name doesn't account for the
terminating null. This patch adds the extra byte, and switches to
snprintf to avoid overflowing.
Signed-off-by: Stephen Kitt
---
drivers/clk/ti/adpll.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/ti/adpll.c b/drivers/clk/ti/adpll.c
index fdfb90058504..27933c4e8a27 100644
--- a/drivers/clk/ti/adpll.c
+++ b/drivers/clk/ti/adpll.c
@@ -196,12 +196,13 @@ static const char *ti_adpll_clk_get_name(struct
ti_adpll_data *d,
} else {
const char *base_name = "adpll";
char *buf;
+ size_t size = 8 + 1 + strlen(base_name) + 1 +
+ strlen(postfix) + 1;
- buf = devm_kzalloc(d->dev, 8 + 1 + strlen(base_name) + 1 +
- strlen(postfix), GFP_KERNEL);
+ buf = devm_kzalloc(d->dev, size, GFP_KERNEL);
if (!buf)
return NULL;
- sprintf(buf, "%08lx.%s.%s", d->pa, base_name, postfix);
+ snprintf(buf, size, "%08lx.%s.%s", d->pa, base_name, postfix);
name = buf;
}
Thanks for catching this. Maybe just use devm_kasprintf() here?
Ah yes, that would be much better! V2 coming up, thanks for the
suggestion.
Regards,
Stephen
Re: [PATCH v2] clk/ti/adpll: allocate room for terminating null
Le 27/09/2019 20:05, Stephen Kitt a écrit :
The buffer allocated in ti_adpll_clk_get_name doesn't account for the
terminating null. This patch switches to ka_sprintf to avoid
Aargh, devm_kasprintf of course...
overflowing.
Signed-off-by: Stephen Kitt
---
drivers/clk/ti/adpll.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/ti/adpll.c b/drivers/clk/ti/adpll.c
index fdfb90058504..021cf9e2b4db 100644
--- a/drivers/clk/ti/adpll.c
+++ b/drivers/clk/ti/adpll.c
@@ -195,14 +195,8 @@ static const char *ti_adpll_clk_get_name(struct
ti_adpll_data *d,
return NULL;
} else {
const char *base_name = "adpll";
- char *buf;
-
- buf = devm_kzalloc(d->dev, 8 + 1 + strlen(base_name) + 1 +
- strlen(postfix), GFP_KERNEL);
- if (!buf)
- return NULL;
- sprintf(buf, "%08lx.%s.%s", d->pa, base_name, postfix);
- name = buf;
+ name = devm_kasprintf(d->dev, GFP_KERNEL, "%08lx.%s.%s",
+ d->pa, base_name, postfix);
}
return name;
[PATCH v2] clk/ti/adpll: allocate room for terminating null
The buffer allocated in ti_adpll_clk_get_name doesn't account for the
terminating null. This patch switches to ka_sprintf to avoid
overflowing.
Signed-off-by: Stephen Kitt
---
drivers/clk/ti/adpll.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/ti/adpll.c b/drivers/clk/ti/adpll.c
index fdfb90058504..021cf9e2b4db 100644
--- a/drivers/clk/ti/adpll.c
+++ b/drivers/clk/ti/adpll.c
@@ -195,14 +195,8 @@ static const char *ti_adpll_clk_get_name(struct
ti_adpll_data *d,
return NULL;
} else {
const char *base_name = "adpll";
- char *buf;
-
- buf = devm_kzalloc(d->dev, 8 + 1 + strlen(base_name) + 1 +
- strlen(postfix), GFP_KERNEL);
- if (!buf)
- return NULL;
- sprintf(buf, "%08lx.%s.%s", d->pa, base_name, postfix);
- name = buf;
+ name = devm_kasprintf(d->dev, GFP_KERNEL, "%08lx.%s.%s",
+ d->pa, base_name, postfix);
}
return name;
--
2.20.1
[PATCH] clk/ti/adpll: allocate room for terminating null
The buffer allocated in ti_adpll_clk_get_name doesn't account for the
terminating null. This patch adds the extra byte, and switches to
snprintf to avoid overflowing.
Signed-off-by: Stephen Kitt
---
drivers/clk/ti/adpll.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/ti/adpll.c b/drivers/clk/ti/adpll.c
index fdfb90058504..27933c4e8a27 100644
--- a/drivers/clk/ti/adpll.c
+++ b/drivers/clk/ti/adpll.c
@@ -196,12 +196,13 @@ static const char *ti_adpll_clk_get_name(struct
ti_adpll_data *d,
} else {
const char *base_name = "adpll";
char *buf;
+ size_t size = 8 + 1 + strlen(base_name) + 1 +
+ strlen(postfix) + 1;
- buf = devm_kzalloc(d->dev, 8 + 1 + strlen(base_name) + 1 +
- strlen(postfix), GFP_KERNEL);
+ buf = devm_kzalloc(d->dev, size, GFP_KERNEL);
if (!buf)
return NULL;
- sprintf(buf, "%08lx.%s.%s", d->pa, base_name, postfix);
+ snprintf(buf, size, "%08lx.%s.%s", d->pa, base_name, postfix);
name = buf;
}
--
2.20.1
Re: [PATCH V2 1/2] string: Add stracpy and stracpy_pad mechanisms
Le 26/09/2019 09:29, Rasmus Villemoes a écrit :
On 26/09/2019 02.01, Stephen Kitt wrote:
Le 25/09/2019 23:50, Andrew Morton a écrit :
On Tue, 23 Jul 2019 06:51:36 -0700 Joe Perches
wrote:
Several uses of strlcpy and strscpy have had defects because the
last argument of each function is misused or typoed.
Add macro mechanisms to avoid this defect.
stracpy (copy a string to a string array) must have a string
array as the first argument (dest) and uses sizeof(dest) as the
count of bytes to copy.
These mechanisms verify that the dest argument is an array of
char or other compatible types like u8 or s8 or equivalent.
A BUILD_BUG is emitted when the type of dest is not compatible.
I'm still reluctant to merge this because we don't have code in -next
which *uses* it. You did have a patch for that against v1, I
believe?
Please dust it off and send it along?
Joe had a Coccinelle script to mass-convert strlcpy and strscpy.
Here's a different patch which converts some of ALSA's strcpy calls to
stracpy:
Please don't. At least not for the cases where the source is a string
literal - that just gives worse code generation (because gcc doesn't
know anything about strscpy or strlcpy), and while a run-time (silent)
truncation is better than a run-time buffer overflow, wouldn't it be
even better with a build time error?
Yes, that was the plan once Joe's patch gets merged (if it does), and my
patch was only an example of using stracpy, as a step on the road. I was
intending to follow up with a patch converting stracpy to something like
https://www.openwall.com/lists/kernel-hardening/2019/07/06/14
__FORTIFY_INLINE ssize_t strscpy(char *dest, const char *src, size_t
count)
{
size_t dest_size = __builtin_object_size(dest, 0);
size_t src_size = __builtin_object_size(src, 0);
if (__builtin_constant_p(count) &&
__builtin_constant_p(src_size) &&
__builtin_constant_p(dest_size) &&
src_size <= count &&
src_size <= dest_size &&
src[src_size - 1] == '\0') {
strcpy(dest, src);
return src_size - 1;
} else {
return __strscpy(dest, src, count);
}
}
which, as a macro, would become
#define stracpy(dest, src) \
({ \
size_t count = ARRAY_SIZE(dest);\
size_t dest_size = __builtin_object_size(dest, 0); \
size_t src_size = __builtin_object_size(src, 0);\
BUILD_BUG_ON(!(__same_type(dest, char[]) || \
__same_type(dest, unsigned char[]) ||\
__same_type(dest, signed char[]))); \
\
(__builtin_constant_p(count) && \
__builtin_constant_p(src_size) && \
__builtin_constant_p(dest_size) && \
src_size <= count &&\
src_size <= dest_size &&\
src[src_size - 1] == '\0') ? \
(((size_t) strcpy(dest, src)) & 0) + src_size - 1 \
: \
strscpy(dest, src, count); \
})
and both of these get optimised to movs when copying a constant string
which fits in the target.
I was going at this from the angle of improving the existing APIs and
their resulting code. But I like your approach of failing at compile
time.
Perhaps we could do both ;-).
Regards,
Stephen
Re: [PATCH V2 1/2] string: Add stracpy and stracpy_pad mechanisms
Le 25/09/2019 23:50, Andrew Morton a écrit : On Tue, 23 Jul 2019 06:51:36 -0700 Joe Perches wrote: Several uses of strlcpy and strscpy have had defects because the last argument of each function is misused or typoed. Add macro mechanisms to avoid this defect. stracpy (copy a string to a string array) must have a string array as the first argument (dest) and uses sizeof(dest) as the count of bytes to copy. These mechanisms verify that the dest argument is an array of char or other compatible types like u8 or s8 or equivalent. A BUILD_BUG is emitted when the type of dest is not compatible. I'm still reluctant to merge this because we don't have code in -next which *uses* it. You did have a patch for that against v1, I believe? Please dust it off and send it along? Joe had a Coccinelle script to mass-convert strlcpy and strscpy. Here's a different patch which converts some of ALSA's strcpy calls to stracpy: From 89e9afa562f3351bc000f3aacb1041eafbe0204c Mon Sep 17 00:00:00 2001 From: Stephen Kitt Date: Thu, 26 Sep 2019 01:36:20 +0200 Subject: [PATCH] ALSA: use stracpy in docs, USB and Intel HDMI This converts the Writing an ALSA driver manual to stracpy instead of strcpy, and applies the change to the USB drivers and the Intel HDMI audio driver. Using stracpy ensures that the target is a char array and that the copy doesn't overflow (using strscpy). (This requires Joe Perches' stracpy patch.) Signed-off-by: Stephen Kitt Cc: Joe Perches Cc: Andrew Morton Cc: Takashi Iwai Cc: Jonathan Corbet Cc: Clemens Ladisch --- .../sound/kernel-api/writing-an-alsa-driver.rst | 16 sound/usb/6fire/chip.c | 4 ++-- sound/usb/6fire/midi.c | 2 +- sound/usb/6fire/pcm.c| 2 +- sound/usb/card.c | 2 +- sound/usb/line6/driver.c | 8 sound/usb/line6/midi.c | 4 ++-- sound/usb/line6/pcm.c| 2 +- sound/usb/line6/toneport.c | 4 ++-- sound/usb/midi.c | 2 +- sound/usb/misc/ua101.c | 6 +++--- sound/usb/mixer.c| 2 +- sound/usb/stream.c | 2 +- sound/usb/usx2y/us122l.c | 2 +- sound/usb/usx2y/usX2Yhwdep.c | 2 +- sound/usb/usx2y/usbusx2y.c | 4 ++-- sound/x86/intel_hdmi_audio.c | 6 +++--- 17 files changed, 35 insertions(+), 35 deletions(-) diff --git a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst index 132f5eb9b530..90170d853a35 100644 --- a/Documentation/sound/kernel-api/writing-an-alsa-driver.rst +++ b/Documentation/sound/kernel-api/writing-an-alsa-driver.rst @@ -321,8 +321,8 @@ to details explained in the following section. goto error; /* (4) */ - strcpy(card->driver, "My Chip"); - strcpy(card->shortname, "My Own Chip 123"); + stracpy(card->driver, "My Chip"); + stracpy(card->shortname, "My Own Chip 123"); sprintf(card->longname, "%s at 0x%lx irq %i", card->shortname, chip->port, chip->irq); @@ -434,8 +434,8 @@ Since each component can be properly freed, the single :: - strcpy(card->driver, "My Chip"); - strcpy(card->shortname, "My Own Chip 123"); + stracpy(card->driver, "My Chip"); + stracpy(card->shortname, "My Own Chip 123"); sprintf(card->longname, "%s at 0x%lx irq %i", card->shortname, chip->port, chip->irq); @@ -1373,7 +1373,7 @@ shows only the skeleton, how to build up the PCM interfaces. if (err < 0) return err; pcm->private_data = chip; - strcpy(pcm->name, "My Chip"); + stracpy(pcm->name, "My Chip"); chip->pcm = pcm; /* set operators */ snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, @@ -1406,7 +1406,7 @@ function. It would be better to create a constructor for pcm, namely, if (err < 0) return err; pcm->private_data = chip; - strcpy(pcm->name, "My Chip"); + stracpy(pcm->name, "My Chip"); chip->pcm = pcm; return 0; @@ -2590,7 +2590,7 @@ the string for the currently given item index. uinfo->value.enumerated.items = 4; if (uinfo->value.enumerated.item > 3) uinfo->value.enumerated.item = 3; - strcpy(uinfo->value.enumerat
Re: [PATCH] checkpatch: Added warnings in favor of strscpy().
On Mon, 22 Jul 2019 10:59:00 -0700, Joe Perches wrote:
> On Mon, 2019-07-22 at 10:50 -0700, Kees Cook wrote:
> > On Sat, Jul 06, 2019 at 02:42:04PM +0200, Stephen Kitt wrote:
> > > On Tue, 2 Jul 2019 10:25:04 -0700, Kees Cook
> > > wrote:
> > > > On Sat, Jun 29, 2019 at 06:15:37PM +0200, Stephen Kitt wrote:
> > > > > On Fri, 28 Jun 2019 17:25:48 +0530, Nitin Gote
> > > > > wrote:
> > > > > > 1. Deprecate strcpy() in favor of strscpy().
> > > > >
> > > > > This isn’t a comment “against” this patch, but something I’ve been
> > > > > wondering recently and which raises a question about how to handle
> > > > > strcpy’s deprecation in particular. There is still one scenario
> > > > > where strcpy is useful: when GCC replaces it with its builtin,
> > > > > inline version...
> > > > >
> > > > > Would it be worth introducing a macro for
> > > > > strcpy-from-constant-string, which would check that GCC’s builtin
> > > > > is being used (when building with GCC), and fall back to strscpy
> > > > > otherwise?
> > > >
> > > > How would you suggest it operate? A separate API, or something like
> > > > the existing overloaded strcpy() macros in string.h?
> > >
> > > The latter; in my mind the point is to simplify the thought process for
> > > developers, so strscpy should be the “obvious” choice in all cases,
> > > even when dealing with constant strings in hot paths. Something like
> > >
> > > __FORTIFY_INLINE ssize_t strscpy(char *dest, const char *src, size_t
> > > count) {
> > > size_t dest_size = __builtin_object_size(dest, 0);
> > > size_t src_size = __builtin_object_size(src, 0);
> > > if (__builtin_constant_p(count) &&
> > > __builtin_constant_p(src_size) &&
> > > __builtin_constant_p(dest_size) &&
> > > src_size <= count &&
> > > src_size <= dest_size &&
> > > src[src_size - 1] == '\0') {
> > > strcpy(dest, src);
> > > return src_size - 1;
> > > } else {
> > > return __strscpy(dest, src, count);
> > > }
> > > }
> > >
> > > with the current strscpy renamed to __strscpy. I imagine it’s not
> > > necessary to tie this to FORTIFY — __OPTIMIZE__ should be sufficient,
> > > shouldn’t it? Although building on top of the fortified strcpy is
> > > reassuring, and I might be missing something. I’m also not sure how to
> > > deal with the backing strscpy: weak symbol, or something else... At
> > > least there aren’t (yet) any arch-specific implementations of strscpy
> > > to deal with, but obviously they’d still need to be supportable.
> > >
> > > In my tests, this all gets optimised away, and we end up with code such
> > > as
> > >
> > > strscpy(raead.type, "aead", sizeof(raead.type));
> > >
> > > being compiled down to
> > >
> > > movl$1684104545, 4(%rsp)
> > >
> > > on x86-64, and non-constant code being compiled down to a direct
> > > __strscpy call.
> >
> > Thanks for the details! Yeah, that seems nice. I wonder if there is a
> > sensible way to combine these also with the stracpy*() proposal[1], so the
> > call in your example above could just be:
> >
> > stracpy(raead.type, "aead");
> >
> > (It seems both proposals together would have the correct result...)
> >
> > [1] https://lkml.kernel.org/r/201907221031.8B87A9DE@keescook
>
> Easy enough to do.
How about you submit your current patch set, and I follow up with the above
adapted to stracpy?
Regards,
Stephen
pgpmu_OBlu2ni.pgp
Description: OpenPGP digital signature
Re: [PATCH] docs: format kernel-parameters -- as code
On Fri, 28 Jun 2019 09:10:21 -0600, Jonathan Corbet wrote:
> On Thu, 27 Jun 2019 15:59:38 +0200
> > The current ReStructuredText formatting results in "--", used to
> > indicate the end of the kernel command-line parameters, appearing as
> > an en-dash instead of two hyphens; this patch formats them as code,
> > "``--``", as done elsewhere in the documentation.
> >
> > Signed-off-by: Stephen Kitt
>
> A worthy fix, I've applied it. This seems like the sort of annoyance that
> will bite us over and over, though. We might want to find a more
> comprehensive way to turn this behavior off.
Right, looking further shows a large number of places where -- is handled
incorrectly. The following patch disables this “smart” handling wholesale;
I’ll follow up (in the next few days) with patches to use real em- and
en-dashes where appropriate.
Regards,
Stephen
From 3b10d01decfec38a124be75739b398a3dcd9d5ce Mon Sep 17 00:00:00 2001
From: Stephen Kitt
Date: Fri, 28 Jun 2019 20:34:31 +0200
Subject: [PATCH] Disable Sphinx SmartyPants in HTML output
The handling of dashes in particular results in confusing
documentation in a number of instances, since "--" becomes an
en-dash. This disables SmartyPants wholesale, losing smart quotes
along with smart dashes.
With Sphinx 1.6 we could fine-tune the conversion, using the new
smartquotes and smartquotes_action settings.
Signed-off-by: Stephen Kitt
---
Documentation/conf.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/conf.py b/Documentation/conf.py
index 7ace3f8852bd..966dbdfafcd1 100644
--- a/Documentation/conf.py
+++ b/Documentation/conf.py
@@ -200,7 +200,7 @@ html_context = {
# If true, SmartyPants will be used to convert quotes and dashes to
# typographically correct entities.
-#html_use_smartypants = True
+html_use_smartypants = False
# Custom sidebar templates, maps document names to template names.
#html_sidebars = {}
--
2.11.0
pgpaRNMC63Hpq.pgp
Description: OpenPGP digital signature
[PATCH] docs: stop suggesting strlcpy
Since strlcpy is deprecated, the documentation shouldn't suggest using
it. This patch fixes the examples to use strscpy instead. It also uses
sizeof instead of underlying constants as far as possible, to simplify
future changes to the corresponding data structures.
Signed-off-by: Stephen Kitt
---
Documentation/hid/hid-transport.txt | 6 +++---
Documentation/i2c/instantiating-devices | 2 +-
Documentation/i2c/upgrading-clients | 4 ++--
Documentation/kernel-hacking/locking.rst| 6 +++---
Documentation/translations/it_IT/kernel-hacking/locking.rst | 6 +++---
5 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/Documentation/hid/hid-transport.txt
b/Documentation/hid/hid-transport.txt
index 3dcba9fd4a3a..4f41d67f1b4b 100644
--- a/Documentation/hid/hid-transport.txt
+++ b/Documentation/hid/hid-transport.txt
@@ -194,9 +194,9 @@ with HID core:
goto err_<...>;
}
- strlcpy(hid->name, , 127);
- strlcpy(hid->phys, , 63);
- strlcpy(hid->uniq, , 63);
+ strscpy(hid->name, , sizeof(hid->name));
+ strscpy(hid->phys, , sizeof(hid->phys));
+ strscpy(hid->uniq, , sizeof(hid->uniq));
hid->ll_driver = _ll_driver;
hid->bus = ;
diff --git a/Documentation/i2c/instantiating-devices
b/Documentation/i2c/instantiating-devices
index 0d85ac1935b7..8bc7d99133e3 100644
--- a/Documentation/i2c/instantiating-devices
+++ b/Documentation/i2c/instantiating-devices
@@ -137,7 +137,7 @@ static int usb_hcd_nxp_probe(struct platform_device *pdev)
(...)
i2c_adap = i2c_get_adapter(2);
memset(_info, 0, sizeof(struct i2c_board_info));
- strlcpy(i2c_info.type, "isp1301_nxp", I2C_NAME_SIZE);
+ strscpy(i2c_info.type, "isp1301_nxp", sizeof(i2c_info.type));
isp1301_i2c_client = i2c_new_probed_device(i2c_adap, _info,
normal_i2c, NULL);
i2c_put_adapter(i2c_adap);
diff --git a/Documentation/i2c/upgrading-clients
b/Documentation/i2c/upgrading-clients
index ccba3ffd6e80..96392cc5b5c7 100644
--- a/Documentation/i2c/upgrading-clients
+++ b/Documentation/i2c/upgrading-clients
@@ -43,7 +43,7 @@ static int example_attach(struct i2c_adapter *adap, int addr,
int kind)
example->client.adapter = adap;
i2c_set_clientdata(>i2c_client, state);
- strlcpy(client->i2c_client.name, "example", I2C_NAME_SIZE);
+ strscpy(client->i2c_client.name, "example",
sizeof(client->i2c_client.name));
ret = i2c_attach_client(>i2c_client);
if (ret < 0) {
@@ -138,7 +138,7 @@ can be removed:
- example->client.flags = 0;
- example->client.adapter = adap;
-
-- strlcpy(client->i2c_client.name, "example", I2C_NAME_SIZE);
+- strscpy(client->i2c_client.name, "example",
sizeof(client->i2c_client.name));
The i2c_set_clientdata is now:
diff --git a/Documentation/kernel-hacking/locking.rst
b/Documentation/kernel-hacking/locking.rst
index 519673df0e82..dc698ea456e0 100644
--- a/Documentation/kernel-hacking/locking.rst
+++ b/Documentation/kernel-hacking/locking.rst
@@ -451,7 +451,7 @@ to protect the cache and all the objects within it. Here's
the code::
if ((obj = kmalloc(sizeof(*obj), GFP_KERNEL)) == NULL)
return -ENOMEM;
-strlcpy(obj->name, name, sizeof(obj->name));
+strscpy(obj->name, name, sizeof(obj->name));
obj->id = id;
obj->popularity = 0;
@@ -660,7 +660,7 @@ Here is the code::
}
@@ -63,6 +94,7 @@
- strlcpy(obj->name, name, sizeof(obj->name));
+ strscpy(obj->name, name, sizeof(obj->name));
obj->id = id;
obj->popularity = 0;
+obj->refcnt = 1; /* The cache holds a reference */
@@ -774,7 +774,7 @@ the lock is no longer used to protect the reference count
itself.
}
@@ -94,7 +76,7 @@
- strlcpy(obj->name, name, sizeof(obj->name));
+ strscpy(obj->name, name, sizeof(obj->name));
obj->id = id;
obj->popularity = 0;
-obj->refcnt = 1; /* The cache holds a reference */
diff --git a/Documentation/translations/it_IT/kernel-hacking/locking.rst
b/Documentation/translations/it_IT/kernel-hacking/locking.rst
index 0ef3163b..5fd8a1abd2be 100644
--- a/Documentation/translations/it_IT/kernel-hacking/locking.rst
+++ b/Documentation/translations/it_IT/kernel-hacking/locking.rst
@@ -468,7 +468,7 @@ e tutti gli oggetti che contiene. Ecco il codice::
if ((obj = kmalloc(sizeof(*obj), GFP_KERNEL)) == NULL)
return -ENOMEM;
-strlcpy(obj->name, name, sizeof(ob
[PATCH] Drop unused isa_page_to_bus
isa_page_to_bus is deprecated and no longer used anywhere, this patch
removes it entirely.
Signed-off-by: Stephen Kitt
---
arch/alpha/include/asm/io.h | 5 -
arch/arm/include/asm/io.h | 1 -
arch/mips/include/asm/io.h | 2 --
arch/x86/include/asm/io.h | 1 -
4 files changed, 9 deletions(-)
diff --git a/arch/alpha/include/asm/io.h b/arch/alpha/include/asm/io.h
index ccf9d65166bb..af2c0063dc75 100644
--- a/arch/alpha/include/asm/io.h
+++ b/arch/alpha/include/asm/io.h
@@ -93,11 +93,6 @@ static inline void * phys_to_virt(unsigned long address)
#define page_to_phys(page) page_to_pa(page)
-static inline dma_addr_t __deprecated isa_page_to_bus(struct page *page)
-{
- return page_to_phys(page);
-}
-
/* Maximum PIO space address supported? */
#define IO_SPACE_LIMIT 0x
diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index 7e22c81398c4..f96ec93679b7 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -33,7 +33,6 @@
* ISA I/O bus memory addresses are 1:1 with the physical address.
*/
#define isa_virt_to_bus virt_to_phys
-#define isa_page_to_bus page_to_phys
#define isa_bus_to_virt phys_to_virt
/*
diff --git a/arch/mips/include/asm/io.h b/arch/mips/include/asm/io.h
index 29997e42480e..1790274c27eb 100644
--- a/arch/mips/include/asm/io.h
+++ b/arch/mips/include/asm/io.h
@@ -149,8 +149,6 @@ static inline void *isa_bus_to_virt(unsigned long address)
return phys_to_virt(address);
}
-#define isa_page_to_bus page_to_phys
-
/*
* However PCI ones are not necessarily 1:1 and therefore these interfaces
* are forbidden in portable PCI drivers.
diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index a06a9f8294ea..6bed97ff6db2 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -165,7 +165,6 @@ static inline unsigned int isa_virt_to_bus(volatile void
*address)
{
return (unsigned int)virt_to_phys(address);
}
-#define isa_page_to_bus(page) ((unsigned int)page_to_phys(page))
#define isa_bus_to_virtphys_to_virt
/*
--
2.11.0
[tip:x86/mm] x86/mm: Fix the 56-bit addresses memory map in Documentation/x86/x86_64/mm.txt
Commit-ID: 89502a01979033a1c0c0c4d6d9aef07e59021005 Gitweb: https://git.kernel.org/tip/89502a01979033a1c0c0c4d6d9aef07e59021005 Author: Stephen Kitt AuthorDate: Mon, 15 Apr 2019 17:08:53 +0200 Committer: Ingo Molnar CommitDate: Tue, 16 Apr 2019 09:00:34 +0200 x86/mm: Fix the 56-bit addresses memory map in Documentation/x86/x86_64/mm.txt This fixes a PT typo, and the following 56-bit address-space addresses: * the hole extends from 0100 to feff * the KASAN shadow memory area stops at fbff (see kasan.h) Signed-off-by: Stephen Kitt Cc: Andy Lutomirski Cc: Borislav Petkov Cc: Brian Gerst Cc: Dave Hansen Cc: H. Peter Anvin Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Rik van Riel Cc: Thomas Gleixner Cc: alex.po...@linux.com Cc: b...@redhat.com Cc: cor...@lwn.net Cc: kirill.shute...@linux.intel.com Cc: linux-...@vger.kernel.org Link: http://lkml.kernel.org/r/20190415150853.10354-1-st...@sk2.org Signed-off-by: Ingo Molnar --- Documentation/x86/x86_64/mm.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/x86/x86_64/mm.txt b/Documentation/x86/x86_64/mm.txt index 804f9426ed17..6cbe652d7a49 100644 --- a/Documentation/x86/x86_64/mm.txt +++ b/Documentation/x86/x86_64/mm.txt @@ -72,7 +72,7 @@ Complete virtual memory map with 5-level page tables Notes: - With 56-bit addresses, user-space memory gets expanded by a factor of 512x, - from 0.125 PB to 64 PB. All kernel mappings shift down to the -64 PT starting + from 0.125 PB to 64 PB. All kernel mappings shift down to the -64 PB starting offset and many of the regions expand to support the much larger physical memory supported. @@ -83,7 +83,7 @@ Notes: |0 | 00ff | 64 PB | user-space virtual memory, different per mm __||__|_|___ || | | - 8000 | +64PB | 7fff | ~16K PB | ... huge, still almost 64 bits wide hole of non-canonical + 0100 | +64PB | feff | ~16K PB | ... huge, still almost 64 bits wide hole of non-canonical || | | virtual memory addresses up to the -64 PB || | | starting offset of kernel mappings. __||__|_|___ @@ -99,7 +99,7 @@ |___ ffd2 | -11.5 PB | ffd3 | 0.5 PB | ... unused hole ffd4 | -11PB | ffd5 | 0.5 PB | virtual memory map (vmemmap_base) ffd6 | -10.5 PB | ffde | 2.25 PB | ... unused hole - ffdf | -8.25 PB | fdff | ~8 PB | KASAN shadow memory + ffdf | -8.25 PB | fbff | ~8 PB | KASAN shadow memory __||__|_| | | Identical layout to the 47-bit one from here on:
[PATCH] Fix 56-bit addresses in Documentation/x86/x86_64/mm.txt
This fixes a PT typo, and the following 56-bit address-space addresses: * the hole extends from 0100 to feff * the KASAN shadow memory area stops at fbff (see kasan.h) Signed-off-by: Stephen Kitt --- Documentation/x86/x86_64/mm.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/x86/x86_64/mm.txt b/Documentation/x86/x86_64/mm.txt index 804f9426ed17..6cbe652d7a49 100644 --- a/Documentation/x86/x86_64/mm.txt +++ b/Documentation/x86/x86_64/mm.txt @@ -72,7 +72,7 @@ Complete virtual memory map with 5-level page tables Notes: - With 56-bit addresses, user-space memory gets expanded by a factor of 512x, - from 0.125 PB to 64 PB. All kernel mappings shift down to the -64 PT starting + from 0.125 PB to 64 PB. All kernel mappings shift down to the -64 PB starting offset and many of the regions expand to support the much larger physical memory supported. @@ -83,7 +83,7 @@ Notes: |0 | 00ff | 64 PB | user-space virtual memory, different per mm __||__|_|___ || | | - 8000 | +64PB | 7fff | ~16K PB | ... huge, still almost 64 bits wide hole of non-canonical + 0100 | +64PB | feff | ~16K PB | ... huge, still almost 64 bits wide hole of non-canonical || | | virtual memory addresses up to the -64 PB || | | starting offset of kernel mappings. __||__|_|___ @@ -99,7 +99,7 @@ |___ ffd2 | -11.5 PB | ffd3 | 0.5 PB | ... unused hole ffd4 | -11PB | ffd5 | 0.5 PB | virtual memory map (vmemmap_base) ffd6 | -10.5 PB | ffde | 2.25 PB | ... unused hole - ffdf | -8.25 PB | fdff | ~8 PB | KASAN shadow memory + ffdf | -8.25 PB | fbff | ~8 PB | KASAN shadow memory __||__|_| | | Identical layout to the 47-bit one from here on: -- 2.11.0
Re: [PATCH] device_handler: remove VLAs
On Mon, 12 Mar 2018 15:41:26 +, Bart Van Assche <bart.vanass...@wdc.com> wrote: > On Sat, 2018-03-10 at 14:14 +0100, Stephen Kitt wrote: > > The two patches I sent were supposed to be alternative solutions; see > > https://marc.info/?l=linux-scsi=152063671005295=2 for the > > introduction (I seem to have messed up the headers, so the mails didn’t > > end up threaded properly). > > The two patches arrived in my inbox several minutes before the cover > letter. In the e-mail header of the cover letter I found the following: > > X-Greylist: delayed 1810 seconds by postgrey-1.27 at vger.kernel.org; Fri, > 09 Mar 2018 18:05:08 EST > > Does this mean that the delay happened due to vger server's anti-spam > algorithm? That’s right, the greylisting part of its anti-spam measures. Regards, Stephen pgptyelins4RZ.pgp Description: OpenPGP digital signature
Re: [PATCH] device_handler: remove VLAs
On Mon, 12 Mar 2018 15:41:26 +, Bart Van Assche wrote: > On Sat, 2018-03-10 at 14:14 +0100, Stephen Kitt wrote: > > The two patches I sent were supposed to be alternative solutions; see > > https://marc.info/?l=linux-scsi=152063671005295=2 for the > > introduction (I seem to have messed up the headers, so the mails didn’t > > end up threaded properly). > > The two patches arrived in my inbox several minutes before the cover > letter. In the e-mail header of the cover letter I found the following: > > X-Greylist: delayed 1810 seconds by postgrey-1.27 at vger.kernel.org; Fri, > 09 Mar 2018 18:05:08 EST > > Does this mean that the delay happened due to vger server's anti-spam > algorithm? That’s right, the greylisting part of its anti-spam measures. Regards, Stephen pgptyelins4RZ.pgp Description: OpenPGP digital signature
Re: [PATCH] device_handler: remove VLAs
Hi Bart,
On Fri, 9 Mar 2018 22:48:10 +, Bart Van Assche <bart.vanass...@wdc.com>
wrote:
> On Fri, 2018-03-09 at 23:32 +0100, Stephen Kitt wrote:
> > In preparation to enabling -Wvla, remove VLAs and replace them with
> > fixed-length arrays instead.
> >
> > scsi_dh_{alua,emc,rdac} use variable-length array declarations to
> > store command blocks, with the appropriate size as determined by
> > COMMAND_SIZE. This patch replaces these with fixed-sized arrays using
> > MAX_COMMAND_SIZE, so that the array size can be determined at compile
> > time.
>
> If COMMAND_SIZE() is evaluated at compile time, do we still need this patch?
The two patches I sent were supposed to be alternative solutions; see
https://marc.info/?l=linux-scsi=152063671005295=2 for the introduction (I
seem to have messed up the headers, so the mails didn’t end up threaded
properly).
The MAX_COMMAND_SIZE approach is nice and simple, but I thought it worth
eliminating scsi_command_size_tbl while I was at it...
Regards,
Stephen
pgpMBvDcL8Ium.pgp
Description: OpenPGP digital signature
Re: [PATCH] device_handler: remove VLAs
Hi Bart,
On Fri, 9 Mar 2018 22:48:10 +, Bart Van Assche
wrote:
> On Fri, 2018-03-09 at 23:32 +0100, Stephen Kitt wrote:
> > In preparation to enabling -Wvla, remove VLAs and replace them with
> > fixed-length arrays instead.
> >
> > scsi_dh_{alua,emc,rdac} use variable-length array declarations to
> > store command blocks, with the appropriate size as determined by
> > COMMAND_SIZE. This patch replaces these with fixed-sized arrays using
> > MAX_COMMAND_SIZE, so that the array size can be determined at compile
> > time.
>
> If COMMAND_SIZE() is evaluated at compile time, do we still need this patch?
The two patches I sent were supposed to be alternative solutions; see
https://marc.info/?l=linux-scsi=152063671005295=2 for the introduction (I
seem to have messed up the headers, so the mails didn’t end up threaded
properly).
The MAX_COMMAND_SIZE approach is nice and simple, but I thought it worth
eliminating scsi_command_size_tbl while I was at it...
Regards,
Stephen
pgpMBvDcL8Ium.pgp
Description: OpenPGP digital signature
Re: [PATCH] scsi: resolve COMMAND_SIZE at compile time
Hi Bart,
On Fri, 9 Mar 2018 22:47:12 +, Bart Van Assche <bart.vanass...@wdc.com>
wrote:
> On Fri, 2018-03-09 at 23:33 +0100, Stephen Kitt wrote:
> > +/*
> > + * SCSI command sizes are as follows, in bytes, for fixed size commands,
> > per
> > + * group: 6, 10, 10, 12, 16, 12, 10, 10. The top three bits of an opcode
> > + * determine its group.
> > + * The size table is encoded into a 32-bit value by subtracting each
> > value
> > + * from 16, resulting in a value of 1715488362
> > + * (6 << 28 + 6 << 24 + 4 << 20 + 0 << 16 + 4 << 12 + 6 << 8 + 6 << 4 +
> > 10).
> > + * Command group 3 is reserved and should never be used.
> > + */
> > +#define COMMAND_SIZE(opcode) \
> > + (16 - (15 & (1715488362 >> (4 * (((opcode) >> 5) & 7)
>
> To me this seems hard to read and hard to verify. Could this have been
> written as a combination of ternary expressions, e.g. using a gcc statement
> expression to ensure that opcode is evaluated once?
That’s what I’d tried initially, e.g.
#define COMMAND_SIZE(opcode) ({ \
int index = ((opcode) >> 5) & 7; \
index == 0 ? 6 : (index == 4 ? 16 : index == 3 || index == 5 ? 12 : 10); \
})
But gcc still reckons that results in a VLA, defeating the initial purpose of
the exercise.
Does it help if I make the magic value construction clearer?
#define SCSI_COMMAND_SIZE_TBL ( \
(16 - 6)\
+ ((16 - 10) << 4) \
+ ((16 - 10) << 8) \
+ ((16 - 12) << 12) \
+ ((16 - 16) << 16) \
+ ((16 - 12) << 20) \
+ ((16 - 10) << 24) \
+ ((16 - 10) << 28))
#define COMMAND_SIZE(opcode)\
(16 - (15 & (SCSI_COMMAND_SIZE_TBL >> (4 * (((opcode) >> 5) & 7)
Regards,
Stephen
pgp03pWqk7H6k.pgp
Description: OpenPGP digital signature
Re: [PATCH] scsi: resolve COMMAND_SIZE at compile time
Hi Bart,
On Fri, 9 Mar 2018 22:47:12 +, Bart Van Assche
wrote:
> On Fri, 2018-03-09 at 23:33 +0100, Stephen Kitt wrote:
> > +/*
> > + * SCSI command sizes are as follows, in bytes, for fixed size commands,
> > per
> > + * group: 6, 10, 10, 12, 16, 12, 10, 10. The top three bits of an opcode
> > + * determine its group.
> > + * The size table is encoded into a 32-bit value by subtracting each
> > value
> > + * from 16, resulting in a value of 1715488362
> > + * (6 << 28 + 6 << 24 + 4 << 20 + 0 << 16 + 4 << 12 + 6 << 8 + 6 << 4 +
> > 10).
> > + * Command group 3 is reserved and should never be used.
> > + */
> > +#define COMMAND_SIZE(opcode) \
> > + (16 - (15 & (1715488362 >> (4 * (((opcode) >> 5) & 7)
>
> To me this seems hard to read and hard to verify. Could this have been
> written as a combination of ternary expressions, e.g. using a gcc statement
> expression to ensure that opcode is evaluated once?
That’s what I’d tried initially, e.g.
#define COMMAND_SIZE(opcode) ({ \
int index = ((opcode) >> 5) & 7; \
index == 0 ? 6 : (index == 4 ? 16 : index == 3 || index == 5 ? 12 : 10); \
})
But gcc still reckons that results in a VLA, defeating the initial purpose of
the exercise.
Does it help if I make the magic value construction clearer?
#define SCSI_COMMAND_SIZE_TBL ( \
(16 - 6)\
+ ((16 - 10) << 4) \
+ ((16 - 10) << 8) \
+ ((16 - 12) << 12) \
+ ((16 - 16) << 16) \
+ ((16 - 12) << 20) \
+ ((16 - 10) << 24) \
+ ((16 - 10) << 28))
#define COMMAND_SIZE(opcode)\
(16 - (15 & (SCSI_COMMAND_SIZE_TBL >> (4 * (((opcode) >> 5) & 7)
Regards,
Stephen
pgp03pWqk7H6k.pgp
Description: OpenPGP digital signature
VLA removal, device_handler and COMMAND_SIZE
Hi, I’ve been looking into removing some VLAs from device_handler drivers, prompted by https://lkml.org/lkml/2018/3/7/621 The uses in question here are quite straightforward, e.g. in drivers/scsi/device_handler/scsi_dh_alua.c: u8 cdb[COMMAND_SIZE(MAINTENANCE_IN)]; There’s no trivial way of keeping the compiler happy with -Wvla though here, at least not while keeping the behaviour strictly identical. I’ve come up with two approaches, and I’m curious whether they’re appropriate or if there’s a better way... The first approach is to use MAX_COMMAND_SIZE instead; this wastes a few bytes on the stack here and there, and stays reasonably maintainable. The second approach might be symptomatic of a twisted mind, and involves replacing COMMAND_SIZE so that it can be calculated at compile time when the opcode is known: /* * SCSI command sizes are as follows, in bytes, for fixed size commands, per * group: 6, 10, 10, 12, 16, 12, 10, 10. The top three bits of an opcode * determine its group. * The size table is encoded into a 32-bit value by subtracting each value * from 16, resulting in a value of 1715488362 * (6 << 28 + 6 << 24 + 4 << 20 + 0 << 16 + 4 << 12 + 6 << 8 + 6 << 4 + 10). * Command group 3 is reserved and should never be used. */ #define COMMAND_SIZE(opcode) \ (16 - (15 & (1715488362 >> (4 * (((opcode) >> 5) & 7) This has the side-effect of making some of the call sites more complex, and the macro itself isn’t the most maintainer-friendly. It does mean we can drop BLK_SCSI_REQUEST from drivers/target/Kconfig so it’s not all bad... Both patches will follow in reply to this email, I’ll let more familiar developers judge which is appropriate (if any). Regards, Stephen pgpYjyFbY64Zj.pgp Description: OpenPGP digital signature
VLA removal, device_handler and COMMAND_SIZE
Hi, I’ve been looking into removing some VLAs from device_handler drivers, prompted by https://lkml.org/lkml/2018/3/7/621 The uses in question here are quite straightforward, e.g. in drivers/scsi/device_handler/scsi_dh_alua.c: u8 cdb[COMMAND_SIZE(MAINTENANCE_IN)]; There’s no trivial way of keeping the compiler happy with -Wvla though here, at least not while keeping the behaviour strictly identical. I’ve come up with two approaches, and I’m curious whether they’re appropriate or if there’s a better way... The first approach is to use MAX_COMMAND_SIZE instead; this wastes a few bytes on the stack here and there, and stays reasonably maintainable. The second approach might be symptomatic of a twisted mind, and involves replacing COMMAND_SIZE so that it can be calculated at compile time when the opcode is known: /* * SCSI command sizes are as follows, in bytes, for fixed size commands, per * group: 6, 10, 10, 12, 16, 12, 10, 10. The top three bits of an opcode * determine its group. * The size table is encoded into a 32-bit value by subtracting each value * from 16, resulting in a value of 1715488362 * (6 << 28 + 6 << 24 + 4 << 20 + 0 << 16 + 4 << 12 + 6 << 8 + 6 << 4 + 10). * Command group 3 is reserved and should never be used. */ #define COMMAND_SIZE(opcode) \ (16 - (15 & (1715488362 >> (4 * (((opcode) >> 5) & 7) This has the side-effect of making some of the call sites more complex, and the macro itself isn’t the most maintainer-friendly. It does mean we can drop BLK_SCSI_REQUEST from drivers/target/Kconfig so it’s not all bad... Both patches will follow in reply to this email, I’ll let more familiar developers judge which is appropriate (if any). Regards, Stephen pgpYjyFbY64Zj.pgp Description: OpenPGP digital signature
[PATCH] scsi: resolve COMMAND_SIZE at compile time
COMMAND_SIZE currently uses an array of values in block/scsi_ioctl.c.
A number of device_handler functions use this to initialise arrays,
and this is flagged by -Wvla.
This patch replaces COMMAND_SIZE with a variant using a formula which
can be resolved at compile time in cases where the opcode is fixed,
resolving the array size and avoiding the VLA. The downside is that
the code is less maintainable and that some call sites end up having
more complex generated code.
Since scsi_command_size_tbl is dropped, we can remove the dependency
on BLK_SCSI_REQUEST from drivers/target/Kconfig.
This was prompted by https://lkml.org/lkml/2018/3/7/621
Signed-off-by: Stephen Kitt <st...@sk2.org>
---
block/scsi_ioctl.c | 8
drivers/target/Kconfig | 1 -
include/scsi/scsi_common.h | 13 +++--
3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index 60b471f8621b..b9e176e537be 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -41,14 +41,6 @@ struct blk_cmd_filter {
static struct blk_cmd_filter blk_default_cmd_filter;
-/* Command group 3 is reserved and should never be used. */
-const unsigned char scsi_command_size_tbl[8] =
-{
- 6, 10, 10, 12,
- 16, 12, 10, 10
-};
-EXPORT_SYMBOL(scsi_command_size_tbl);
-
#include
static int sg_get_version(int __user *p)
diff --git a/drivers/target/Kconfig b/drivers/target/Kconfig
index 4c44d7bed01a..b5f05b60cf06 100644
--- a/drivers/target/Kconfig
+++ b/drivers/target/Kconfig
@@ -4,7 +4,6 @@ menuconfig TARGET_CORE
depends on SCSI && BLOCK
select CONFIGFS_FS
select CRC_T10DIF
- select BLK_SCSI_REQUEST # only for scsi_command_size_tbl..
select SGL_ALLOC
default n
help
diff --git a/include/scsi/scsi_common.h b/include/scsi/scsi_common.h
index 731ac09ed231..48d950666376 100644
--- a/include/scsi/scsi_common.h
+++ b/include/scsi/scsi_common.h
@@ -15,8 +15,17 @@ scsi_varlen_cdb_length(const void *hdr)
return ((struct scsi_varlen_cdb_hdr *)hdr)->additional_cdb_length + 8;
}
-extern const unsigned char scsi_command_size_tbl[8];
-#define COMMAND_SIZE(opcode) scsi_command_size_tbl[((opcode) >> 5) & 7]
+/*
+ * SCSI command sizes are as follows, in bytes, for fixed size commands, per
+ * group: 6, 10, 10, 12, 16, 12, 10, 10. The top three bits of an opcode
+ * determine its group.
+ * The size table is encoded into a 32-bit value by subtracting each value
+ * from 16, resulting in a value of 1715488362
+ * (6 << 28 + 6 << 24 + 4 << 20 + 0 << 16 + 4 << 12 + 6 << 8 + 6 << 4 + 10).
+ * Command group 3 is reserved and should never be used.
+ */
+#define COMMAND_SIZE(opcode) \
+ (16 - (15 & (1715488362 >> (4 * (((opcode) >> 5) & 7)
static inline unsigned
scsi_command_size(const unsigned char *cmnd)
--
2.11.0
[PATCH] scsi: resolve COMMAND_SIZE at compile time
COMMAND_SIZE currently uses an array of values in block/scsi_ioctl.c.
A number of device_handler functions use this to initialise arrays,
and this is flagged by -Wvla.
This patch replaces COMMAND_SIZE with a variant using a formula which
can be resolved at compile time in cases where the opcode is fixed,
resolving the array size and avoiding the VLA. The downside is that
the code is less maintainable and that some call sites end up having
more complex generated code.
Since scsi_command_size_tbl is dropped, we can remove the dependency
on BLK_SCSI_REQUEST from drivers/target/Kconfig.
This was prompted by https://lkml.org/lkml/2018/3/7/621
Signed-off-by: Stephen Kitt
---
block/scsi_ioctl.c | 8
drivers/target/Kconfig | 1 -
include/scsi/scsi_common.h | 13 +++--
3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index 60b471f8621b..b9e176e537be 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -41,14 +41,6 @@ struct blk_cmd_filter {
static struct blk_cmd_filter blk_default_cmd_filter;
-/* Command group 3 is reserved and should never be used. */
-const unsigned char scsi_command_size_tbl[8] =
-{
- 6, 10, 10, 12,
- 16, 12, 10, 10
-};
-EXPORT_SYMBOL(scsi_command_size_tbl);
-
#include
static int sg_get_version(int __user *p)
diff --git a/drivers/target/Kconfig b/drivers/target/Kconfig
index 4c44d7bed01a..b5f05b60cf06 100644
--- a/drivers/target/Kconfig
+++ b/drivers/target/Kconfig
@@ -4,7 +4,6 @@ menuconfig TARGET_CORE
depends on SCSI && BLOCK
select CONFIGFS_FS
select CRC_T10DIF
- select BLK_SCSI_REQUEST # only for scsi_command_size_tbl..
select SGL_ALLOC
default n
help
diff --git a/include/scsi/scsi_common.h b/include/scsi/scsi_common.h
index 731ac09ed231..48d950666376 100644
--- a/include/scsi/scsi_common.h
+++ b/include/scsi/scsi_common.h
@@ -15,8 +15,17 @@ scsi_varlen_cdb_length(const void *hdr)
return ((struct scsi_varlen_cdb_hdr *)hdr)->additional_cdb_length + 8;
}
-extern const unsigned char scsi_command_size_tbl[8];
-#define COMMAND_SIZE(opcode) scsi_command_size_tbl[((opcode) >> 5) & 7]
+/*
+ * SCSI command sizes are as follows, in bytes, for fixed size commands, per
+ * group: 6, 10, 10, 12, 16, 12, 10, 10. The top three bits of an opcode
+ * determine its group.
+ * The size table is encoded into a 32-bit value by subtracting each value
+ * from 16, resulting in a value of 1715488362
+ * (6 << 28 + 6 << 24 + 4 << 20 + 0 << 16 + 4 << 12 + 6 << 8 + 6 << 4 + 10).
+ * Command group 3 is reserved and should never be used.
+ */
+#define COMMAND_SIZE(opcode) \
+ (16 - (15 & (1715488362 >> (4 * (((opcode) >> 5) & 7)
static inline unsigned
scsi_command_size(const unsigned char *cmnd)
--
2.11.0
