Re: [PATCH] Documentation: sysrq: don't recommend 'S' 'U' before 'B'
On Tue 2019-09-03 18:08:40, Adam Borowski wrote: > This advice is obsolete and slightly harmful for filesystems from this > millenium: any modern filesystem can handle unexpected crashes without > requiring fsck -- and on the other hand, trying to write to the disk when > the kernel is in a bad state risks introducing corruption. Actually no, I don't think it is good idea. sync is still useful these days -- you want the current data to be written to disk; true, you'll not have to do fsck, but you may lose your current data. Best regards, Pavel > For ext2, any unsafe shutdown meant widespread breakage, but it's no longer > a reasonable filesystem for any non-special use. > > Signed-off-by: Adam Borowski > --- > Documentation/admin-guide/sysrq.rst | 20 +--- > 1 file changed, 9 insertions(+), 11 deletions(-) > > diff --git a/Documentation/admin-guide/sysrq.rst > b/Documentation/admin-guide/sysrq.rst > index 7b9035c01a2e..72b2cfb066f4 100644 > --- a/Documentation/admin-guide/sysrq.rst > +++ b/Documentation/admin-guide/sysrq.rst > @@ -171,22 +171,20 @@ It seems others find it useful as (System Attention > Key) which is > useful when you want to exit a program that will not let you switch consoles. > (For example, X or a svgalib program.) > > -``reboot(b)`` is good when you're unable to shut down. But you should also > -``sync(s)`` and ``umount(u)`` first. > +``reboot(b)`` is good when you're unable to shut down, it is an equivalent > +of pressing the "reset" button. > > ``crash(c)`` can be used to manually trigger a crashdump when the system is > hung. > Note that this just triggers a crash if there is no dump mechanism available. > > -``sync(s)`` is great when your system is locked up, it allows you to sync > your > -disks and will certainly lessen the chance of data loss and fscking. Note > -that the sync hasn't taken place until you see the "OK" and "Done" appear > -on the screen. (If the kernel is really in strife, you may not ever get the > -OK or Done message...) > +``sync(s)`` is handy before yanking removable medium or after using a rescue > +shell that provides no graceful shutdown -- it will ensure your data is > +safely written to the disk. Note that the sync hasn't taken place until you > see > +the "OK" and "Done" appear on the screen. > > -``umount(u)`` is basically useful in the same ways as ``sync(s)``. I > generally > -``sync(s)``, ``umount(u)``, then ``reboot(b)`` when my system locks. It's > saved > -me many a fsck. Again, the unmount (remount read-only) hasn't taken place > until > -you see the "OK" and "Done" message appear on the screen. > +``umount(u)`` can be used to mark filesystems as properly unmounted. From the > +running system's point of view, they will be remounted read-only. The remount > +isn't complete until you see the "OK" and "Done" message appear on the > screen. > > The loglevels ``0``-``9`` are useful when your console is being flooded with > kernel messages you do not want to see. Selecting ``0`` will prevent all but -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH] docs: printk-formats: Stop encouraging use of unnecessary %h[xudi] and %hh[xudi]
On Fri, 06 Sep 2019, Joe Perches wrote: > Link: > https://lore.kernel.org/lkml/CAHk-=wgoxnmsj8GEVFJSvTwdnWm8wVJthefNk2n6+4TC=20...@mail.gmail.com/ I thought Link: was for referencing the patch on the mailing list that became the commit in git. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH] Documentation: sysrq: don't recommend 'S' 'U' before 'B'
On Mon, Sep 09, 2019 at 10:33:31AM +0200, Pavel Machek wrote: > On Tue 2019-09-03 18:08:40, Adam Borowski wrote: > > This advice is obsolete and slightly harmful for filesystems from this > > millenium: any modern filesystem can handle unexpected crashes without > > requiring fsck -- and on the other hand, trying to write to the disk when > > the kernel is in a bad state risks introducing corruption. > > Actually no, I don't think it is good idea. > > sync is still useful these days -- you want the current data to be > written to disk; true, you'll not have to do fsck, but you may lose > your current data. Well yeah, but that's only if you have a reason to suspect there's some data you care about. I'd say that in the usual case, saving whatever volatile state the system has tends to be not worth risking corruption. Ie, the default advice for a locked-up system should be SysRq B. Is there some other wording that you would be happier with? > > For ext2, any unsafe shutdown meant widespread breakage, but it's no longer > > a reasonable filesystem for any non-special use. > > > > Signed-off-by: Adam Borowski > > --- > > Documentation/admin-guide/sysrq.rst | 20 +--- > > 1 file changed, 9 insertions(+), 11 deletions(-) > > > > diff --git a/Documentation/admin-guide/sysrq.rst > > b/Documentation/admin-guide/sysrq.rst > > index 7b9035c01a2e..72b2cfb066f4 100644 > > --- a/Documentation/admin-guide/sysrq.rst > > +++ b/Documentation/admin-guide/sysrq.rst > > @@ -171,22 +171,20 @@ It seems others find it useful as (System Attention > > Key) which is > > useful when you want to exit a program that will not let you switch > > consoles. > > (For example, X or a svgalib program.) > > > > -``reboot(b)`` is good when you're unable to shut down. But you should also > > -``sync(s)`` and ``umount(u)`` first. > > +``reboot(b)`` is good when you're unable to shut down, it is an equivalent > > +of pressing the "reset" button. > > > > ``crash(c)`` can be used to manually trigger a crashdump when the system > > is hung. > > Note that this just triggers a crash if there is no dump mechanism > > available. > > > > -``sync(s)`` is great when your system is locked up, it allows you to sync > > your > > -disks and will certainly lessen the chance of data loss and fscking. Note > > -that the sync hasn't taken place until you see the "OK" and "Done" appear > > -on the screen. (If the kernel is really in strife, you may not ever get the > > -OK or Done message...) > > +``sync(s)`` is handy before yanking removable medium or after using a > > rescue > > +shell that provides no graceful shutdown -- it will ensure your data is > > +safely written to the disk. Note that the sync hasn't taken place until > > you see > > +the "OK" and "Done" appear on the screen. > > > > -``umount(u)`` is basically useful in the same ways as ``sync(s)``. I > > generally > > -``sync(s)``, ``umount(u)``, then ``reboot(b)`` when my system locks. It's > > saved > > -me many a fsck. Again, the unmount (remount read-only) hasn't taken place > > until > > -you see the "OK" and "Done" message appear on the screen. > > +``umount(u)`` can be used to mark filesystems as properly unmounted. From > > the > > +running system's point of view, they will be remounted read-only. The > > remount > > +isn't complete until you see the "OK" and "Done" message appear on the > > screen. > > > > The loglevels ``0``-``9`` are useful when your console is being flooded > > with > > kernel messages you do not want to see. Selecting ``0`` will prevent all > > but Meow! -- ⢀⣴⠾⠻⢶⣦⠀ ⣾⠁⢰⠒⠀⣿⡁ ⢿⡄⠘⠷⠚⠋⠀ I was born a dumb, ugly and work-loving kid, then I got swapped on ⠈⠳⣄ the maternity ward.
[PATCH v2] printf: add support for printing symbolic error codes
It has been suggested several times to extend vsnprintf() to be able
to convert the numeric value of ENOSPC to print "ENOSPC". This is yet
another attempt. Rather than adding another %p extension, simply teach
plain %p to convert ERR_PTRs. While the primary use case is
if (IS_ERR(foo)) {
pr_err("Sorry, can't do that: %p\n", foo);
return PTR_ERR(foo);
}
it is also more helpful to get a symbolic error code (or, worst case,
a decimal number) in case an ERR_PTR is accidentally passed to some
%p, rather than the (efault) that check_pointer() would
result in.
With my embedded hat on, I've made it possible to remove this.
I've tested that the #ifdeffery in errcode.c is sufficient to make
this compile on arm, arm64, mips, powerpc, s390, x86 - I'm sure the
0day bot will tell me which ones I've missed.
The symbols to include have been found by massaging the output of
find arch include -iname 'errno*.h' | xargs grep -E 'define\s*E'
In the cases where some common aliasing exists
(e.g. EAGAIN=EWOULDBLOCK on all platforms, EDEADLOCK=EDEADLK on most),
I've moved the more popular one (in terms of 'git grep -w Efoo | wc)
to the bottom so that one takes precedence.
Acked-by: Uwe Kleine-König
Signed-off-by: Rasmus Villemoes
---
v2:
- add #include to errcode.h (0day)
- keep 'x' handling in switch() (Andy)
- add paragraph to Documentation/core-api/printk-formats.rst
- add ack from Uwe
Documentation/core-api/printk-formats.rst | 8 +
include/linux/errcode.h | 16 ++
lib/Kconfig.debug | 8 +
lib/Makefile | 1 +
lib/errcode.c | 215 ++
lib/test_printf.c | 14 ++
lib/vsprintf.c| 26 +++
7 files changed, 288 insertions(+)
create mode 100644 include/linux/errcode.h
create mode 100644 lib/errcode.c
diff --git a/Documentation/core-api/printk-formats.rst
b/Documentation/core-api/printk-formats.rst
index c6224d039bcb..7d3bf3cb207b 100644
--- a/Documentation/core-api/printk-formats.rst
+++ b/Documentation/core-api/printk-formats.rst
@@ -66,6 +66,14 @@ might be printed instead of the unreachable information::
(efault) data on invalid address
(einval) invalid data on a valid address
+Error pointers, i.e. pointers for which IS_ERR() is true, are handled
+as follows: If CONFIG_SYMBOLIC_ERRCODE is set, an appropriate symbolic
+constant is printed. For example, '"%p", PTR_ERR(-ENOSPC)' results in
+"ENOSPC", while '"%p", PTR_ERR(-EWOULDBLOCK)' results in "EAGAIN"
+(since EAGAIN == EWOULDBLOCK, and the former is the most common). If
+CONFIG_SYMBOLIC_ERRCODE is not set, ERR_PTRs are printed as their
+decimal representation ("-28" and "-11" for the two examples).
+
Plain Pointers
--
diff --git a/include/linux/errcode.h b/include/linux/errcode.h
new file mode 100644
index ..c6a4c1b04f9c
--- /dev/null
+++ b/include/linux/errcode.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_ERRCODE_H
+#define _LINUX_ERRCODE_H
+
+#include
+
+#ifdef CONFIG_SYMBOLIC_ERRCODE
+const char *errcode(int err);
+#else
+static inline const char *errcode(int err)
+{
+ return NULL;
+}
+#endif
+
+#endif /* _LINUX_ERRCODE_H */
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 5960e2980a8a..dc1b20872774 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -164,6 +164,14 @@ config DYNAMIC_DEBUG
See Documentation/admin-guide/dynamic-debug-howto.rst for additional
information.
+config SYMBOLIC_ERRCODE
+ bool "Support symbolic error codes in printf"
+ help
+ If you say Y here, the kernel's printf implementation will
+ be able to print symbolic errors such as ENOSPC instead of
+ the number 28. It makes the kernel image slightly larger
+ (about 3KB), but can make the kernel logs easier to read.
+
endmenu # "printk and dmesg options"
menu "Compile-time checks and compiler options"
diff --git a/lib/Makefile b/lib/Makefile
index 29c02a924973..664ecf10ee2a 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -185,6 +185,7 @@ lib-$(CONFIG_GENERIC_BUG) += bug.o
obj-$(CONFIG_HAVE_ARCH_TRACEHOOK) += syscall.o
obj-$(CONFIG_DYNAMIC_DEBUG) += dynamic_debug.o
+obj-$(CONFIG_SYMBOLIC_ERRCODE) += errcode.o
obj-$(CONFIG_NLATTR) += nlattr.o
diff --git a/lib/errcode.c b/lib/errcode.c
new file mode 100644
index ..7dcf97d5307f
--- /dev/null
+++ b/lib/errcode.c
@@ -0,0 +1,215 @@
+// SPDX-License-Identifier: GPL-2.0
+#include
+#include
+#include
+#include
+
+/*
+ * Ensure these tables to not accidentally become gigantic if some
+ * huge errno makes it in. On most architectures, the first table will
+ * only have about 140 entries, but mips and parisc have more sparsely
+ * allocated errnos (with EHWPOISON = 257 on parisc, and EDQUOT = 1133
+ * on mips), so this wastes a bit of space on those - though we
+ * special case t
Re: [RFC 02/19] ktf: Introduce the main part of the kernel side of ktf
On Sun, 2019-09-08 at 18:23 -0700, Brendan Higgins wrote:
> On Tue, Aug 13, 2019 at 08:09:17AM +0200, Knut Omang wrote:
>
> Sorry, it's taken me way too long to get down to a proper code review on
> this. I was hoping to send you something a couple weeks ago in
> preparation for Tuesday, but I have been crazy busy.
>
> > The ktf module itself and basic data structures for management
> > of test cases and tests and contexts for tests.
> > Also contains the top level include file for kernel clients
> > in ktf.h.
> >
> > More elaborate documentation follows towards the end of the
> > patch set.
> >
> > This patch set contains both user level and kernel code,
> > we'll provide the full implementation of ktf on the kernel side in
> > this and forthcoming patches, then the user space code to execute
> > tests within the kernel and report results, then documentation
> > before introducing a small self test suite of tests to test ktf
> > itself, and some very simple additional example tests.
> >
> > ktf.h: Defines the KTF user API for kernel clients
> > ktf_test.c: Kernel side code for tracking and reporting ktf test
> > results
> >
> > Signed-off-by: Knut Omang
> > ---
> > tools/testing/selftests/ktf/kernel/Makefile | 15 +-
> > tools/testing/selftests/ktf/kernel/ktf.h | 604 -
> > tools/testing/selftests/ktf/kernel/ktf_context.c | 409 +++-
> > tools/testing/selftests/ktf/kernel/ktf_test.c| 397 +++-
> > tools/testing/selftests/ktf/kernel/ktf_test.h| 381 ++-
> > 5 files changed, 1806 insertions(+)
> > create mode 100644 tools/testing/selftests/ktf/kernel/Makefile
> > create mode 100644 tools/testing/selftests/ktf/kernel/ktf.h
> > create mode 100644 tools/testing/selftests/ktf/kernel/ktf_context.c
> > create mode 100644 tools/testing/selftests/ktf/kernel/ktf_test.c
> > create mode 100644 tools/testing/selftests/ktf/kernel/ktf_test.h
> [...]
> > diff --git a/tools/testing/selftests/ktf/kernel/ktf.h
> > b/tools/testing/selftests/ktf/kernel/ktf.h
> > new file mode 100644
> > index 000..ea270e7
> > --- /dev/null
> > +++ b/tools/testing/selftests/ktf/kernel/ktf.h
> > @@ -0,0 +1,604 @@
> > +/*
> > + * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
> > + *Author: Knut Omang
> > + *
> > + * SPDX-License-Identifier: GPL-2.0
> > + *
> > + * ktf.h: Defines the KTF user API for kernel clients
> > + */
> > +#ifndef _KTF_H
> > +#define _KTF_H
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include "ktf_test.h"
> > +#include "ktf_override.h"
> > +#include "ktf_map.h"
>
> Where do you add this file? I don't see any definitions of
> `struct ktf_map` in either this or the preceding patches, so I don't
> think that this will compile.
Compiling is not enabled until patch 17, so that should not be a problem.
I wanted to convey the core KTF API early in the set to not let it "drown"
in the utilities, but I get your point. One way to do it would be to move
the header files up front and keep the implementations in the later patches,
even though that would separate the API definition from the implementation.
> > +#include "ktf_unlproto.h"
>
> Same here. This looks important for understanding what you presented
> here.
yes
Thanks,
Knut
> > +#defineKTF_MAX_LOG 2048
> > +
> > +/* Type for an optional configuration callback for contexts.
> > + * Implementations should copy and store data into their private
> > + * extensions of the context structure. The data pointer is
> > + * only valid inside the callback:
> > + */
> > +typedef int (*ktf_config_cb)(struct ktf_context *ctx, const void* data,
> > size_t data_sz);
> > +typedef void (*ktf_context_cb)(struct ktf_context *ctx);
> > +
> > +struct ktf_context_type;
> > +
> > +struct ktf_context {
> > + struct ktf_map_elem elem; /* Linkage for ctx_map in handle */
> > + char name[KTF_MAX_KEY];/* Context name used in map */
> > + struct ktf_handle *handle; /* Owner of this context */
> > + ktf_config_cb config_cb; /* Optional configuration callback */
> > + ktf_context_cb cleanup;/* Optional callback upon context release
> > */
> > + int config_errno; /* If config_cb set: state of configuration
> > */
> > + struct ktf_context_type *type; /* Associated type, must be set */
> > +};
> > +
> > +typedef struct ktf_context* (*ktf_context_alloc)(struct ktf_context_type
> > *ct);
> > +
> > +struct ktf_context_type {
> > + struct ktf_map_elem elem; /* Linkage for map in handle */
> > + char name[KTF_MAX_KEY];/* Context type name */
> > + struct ktf_handle *handle; /* Owner of this context type */
> > + ktf_context_alloc alloc; /* Allocate a new context of this type */
> > + ktf_config_cb config_cb; /* Configuration callback */
> > + ktf_context_cb cleanup;/* Optional callback upon context release
> > */
> > +};
> > +
> > +#include "ktf_netctx.h"
> > +
> > +/* type for a
Re: [PATCH 0/6] Address issues with SPDX requirements and PEP-263
* Thomas Gleixner wrote: > On Sun, 8 Sep 2019, Matthew Wilcox wrote: > > On Sat, Sep 07, 2019 at 11:17:22PM +0200, Thomas Gleixner wrote: > > > On Sat, 7 Sep 2019, Markus Heiser wrote: > > > > Am 07.09.19 um 20:04 schrieb Mauro Carvalho Chehab: > > > > > No idea. I would actually prefer to just remove the restriction, and > > > > > let > > > > > the SPDX header to be anywhere inside the first comment block inside a > > > > > file [2]. > > > > > [2] I *suspect* that the restriction was added in order to make > > > > > ./scripts/spdxcheck.py to run faster and to avoid false > > > > > positives. > > > > > Right now, if the maximum limit is removed (or set to a very high > > > > > value), there will be one false positive: > > > > > > Nope. The intention was to have a well define place and format instead of > > > everyone and his dog deciding to put it somewhere. SPDX is not intended to > > > replace the existing licensing mess with some other randomly placed and > > > formatted licensing mess. > > > > I find the current style quite unaesthetic: > > > > // SPDX-License-Identifier: GPL-2.0-only > > /* > > * linux/mm/memory.c > > * > > * Copyright (C) 1991, 1992, 1993, 1994 Linus Torvalds > > */ > > > > I'd much rather see > > > > /* > > * SPDX-License-Identifier: GPL-2.0-only > > * Copyright (C) 1991, 1992, 1993, 1994 Linus Torvalds > > */ > > > > but I appreciate the desire to force it to be on the first line if at all > > possible. > > That style is inflicted upon you by Penguin Emperor Decree. :) I'd also say that it's a rather tooling-friendly format which mandates a single-line representation, which will be less likely to be morphed into a zillion variants like the original boilerplates. So the Penguin Emperor Decree also makes sense, which helps. ;-) Thanks, Ingo
Re: [RFC 03/19] ktf: Introduce a generic netlink protocol for test result communication
On Sun, 2019-09-08 at 18:28 -0700, Brendan Higgins wrote: > On Tue, Aug 13, 2019 at 08:09:18AM +0200, Knut Omang wrote: > > The generic netlink protocol used to communicate between > > kernel and user space about tests and test results, as well as some > > means for configuring tests within the kernel. > > > > Unlike other kernel side test code in the kernel, ktf does not print > > anything from inside the kernel (except for optional debugging > > features to help "internal" debugging of ktf or ktf tests). > > Instead all test results are communicated back to the user space > > frontend, which decides how to do the reporting. > > So why netlink? Why not just a file interface? Netlink allows more flexibility in that it is bidirectional and asynchronous. User space may query the kernel for available tests and then decide which tests to invoke. User land test frameworks like Googletest allows use of wildcards and exceptions to select particular tests to run. This is in my opinion very important functionality as we want the tests to be valuable as developer tools, not just to check the code as part of a later QA cycle. Being able to run a single test or a small subset of the tests is very useful. Wrt test reporting, the kernel side just dispatches off messages about test results as they are gathered. Compare this to the complexities, side effects and limitations of printk. Besides, for hybrid tests, bidirectional communication allows a test to contain a mix (or a function) of results gathered in the kernel and in user space. We also use it for network tests, where user space needs to tell the kernel what peer(s) to communicate with, and for certain minimal configuration, such as which device instance to use for device testing. Test nodes may vary in what they offer of hardware. Although we'd like to minimize the need for configuration, as results should be easily reproducable, sometimes there is no good way around. Thanks, Knut > [...] > > Cheers
Re: [PATCH] doc: replace IFF abbreviation with 'if and only if'
On Sat, Sep 07, 2019 at 12:51:16PM +0200, Federico Vaga wrote: > In a normal piece of text the use of 'iff' does not guarantee a correct > interpretation because it is easy to confuse it for a typo (if or iff?). > > I believe that IFF should not be used outside a logical/mathematical > expression. For this reason with this patch I am replacing 'iff' with > 'if an only if' in text, and I am leaving it as it is in logical formulae. Hell no. If you want to avoid the usage in your own docs please go for it, but as seen in your patch we commonly use and that is a good thing as it brings the information across in a very compact way.
