* [PATCH -tip v6 01/27] Documentation/kprobes: Fix to remove remaining jprobe
From: Masami Hiramatsu @ 2018-06-19 16:04 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar
Cc: Masami Hiramatsu, Ingo Molnar, H . Peter Anvin, linux-kernel,
Ananth N Mavinakayanahalli, Andrew Morton, Steven Rostedt,
linux-arch, Jonathan Corbet, linux-doc
In-Reply-To: <152942424698.15209.15245996287444292393.stgit@devbox>
Remove jps from the document, since jprobe is removed.
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: linux-doc@vger.kernel.org
---
Documentation/kprobes.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/kprobes.txt b/Documentation/kprobes.txt
index 22208bf2386d..5ae80baf3921 100644
--- a/Documentation/kprobes.txt
+++ b/Documentation/kprobes.txt
@@ -474,7 +474,7 @@ error occurs during registration, all probes in the array, up to
the bad probe, are safely unregistered before the register_*probes
function returns.
-- kps/rps/jps: an array of pointers to ``*probe`` data structures
+- kps/rps: an array of pointers to ``*probe`` data structures
- num: the number of the array entries.
.. note::
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH] coda: stop using 'struct timespec' in user API
From: Arnd Bergmann @ 2018-06-19 15:37 UTC (permalink / raw)
To: Jan Harkes, coda
Cc: y2038, Arnd Bergmann, Jonathan Corbet, Deepa Dinamani, codalist,
linux-doc, linux-kernel
We exchange file timestamps with user space using psdev device
read/write operations with a fixed but architecture specific binary
layout.
On 32-bit systems, this uses a 'timespec' structure that is defined by
the C library to contain two 32-bit values for seconds and nanoseconds.
As we get ready for the year 2038 overflow of the 32-bit signed seconds,
the kernel now uses 64-bit timestamps internally, and user space will
do the same change by changing the 'timespec' definition in the future.
Unfortunately, this breaks the layout of the coda_vattr structure, so
we need to redefine that in terms of something that does not change.
I'm introducing a new 'struct vtimespec' structure here that keeps
the existing layout, and the same change has to be done in the coda
user space copy of linux/coda.h before anyone can use that on a 32-bit
architecture with 64-bit time_t.
An open question is what should happen to actual times past y2038,
as they are now truncated to the last valid date when sent to user
space, and interpreted as pre-1970 times when a timestamp with the
MSB set is read back into the kernel. Alternatively, we could
change the new timespec64_to_coda()/coda_to_timespec64() functions
to use a different interpretation and extend the available range
further to the future by disallowing past timestamps. This would
require more changes in the user space side though.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Documentation/filesystems/coda.txt | 11 ++++++---
fs/coda/coda_linux.c | 50 +++++++++++++++++++++++++++++---------
include/uapi/linux/coda.h | 20 ++++++++++++---
3 files changed, 62 insertions(+), 19 deletions(-)
diff --git a/Documentation/filesystems/coda.txt b/Documentation/filesystems/coda.txt
index 61311356025d..ea5969068895 100644
--- a/Documentation/filesystems/coda.txt
+++ b/Documentation/filesystems/coda.txt
@@ -481,7 +481,10 @@ kernel support.
-
+ struct vtimespec {
+ long tv_sec; /* seconds */
+ long tv_nsec; /* nanoseconds */
+ };
struct coda_vattr {
enum coda_vtype va_type; /* vnode type (for create) */
@@ -493,9 +496,9 @@ kernel support.
long va_fileid; /* file id */
u_quad_t va_size; /* file size in bytes */
long va_blocksize; /* blocksize preferred for i/o */
- struct timespec va_atime; /* time of last access */
- struct timespec va_mtime; /* time of last modification */
- struct timespec va_ctime; /* time file changed */
+ struct vtimespec va_atime; /* time of last access */
+ struct vtimespec va_mtime; /* time of last modification */
+ struct vtimespec va_ctime; /* time file changed */
u_long va_gen; /* generation number of file */
u_long va_flags; /* flags defined for file */
dev_t va_rdev; /* device special file represents */
diff --git a/fs/coda/coda_linux.c b/fs/coda/coda_linux.c
index f3d543dd9a98..8addcd166908 100644
--- a/fs/coda/coda_linux.c
+++ b/fs/coda/coda_linux.c
@@ -66,6 +66,32 @@ unsigned short coda_flags_to_cflags(unsigned short flags)
return coda_flags;
}
+static struct timespec64 coda_to_timespec64(struct vtimespec ts)
+{
+ /*
+ * We interpret incoming timestamps as 'signed' to match traditional
+ * usage and support pre-1970 timestamps, but this breaks in y2038
+ * on 32-bit machines.
+ */
+ struct timespec64 ts64 = {
+ .tv_sec = ts.tv_sec,
+ .tv_nsec = ts.tv_nsec,
+ };
+
+ return ts64;
+}
+
+static struct vtimespec timespec64_to_coda(struct timespec64 ts64)
+{
+ /* clamp the timestamps to the maximum range rather than wrapping */
+ struct vtimespec ts = {
+ .tv_sec = lower_32_bits(clamp_t(time64_t, ts64.tv_sec,
+ LONG_MIN, LONG_MAX)),
+ .tv_nsec = ts64.tv_nsec,
+ };
+
+ return ts;
+}
/* utility functions below */
void coda_vattr_to_iattr(struct inode *inode, struct coda_vattr *attr)
@@ -105,11 +131,11 @@ void coda_vattr_to_iattr(struct inode *inode, struct coda_vattr *attr)
if (attr->va_size != -1)
inode->i_blocks = (attr->va_size + 511) >> 9;
if (attr->va_atime.tv_sec != -1)
- inode->i_atime = timespec_to_timespec64(attr->va_atime);
+ inode->i_atime = coda_to_timespec64(attr->va_atime);
if (attr->va_mtime.tv_sec != -1)
- inode->i_mtime = timespec_to_timespec64(attr->va_mtime);
+ inode->i_mtime = coda_to_timespec64(attr->va_mtime);
if (attr->va_ctime.tv_sec != -1)
- inode->i_ctime = timespec_to_timespec64(attr->va_ctime);
+ inode->i_ctime = coda_to_timespec64(attr->va_ctime);
}
@@ -130,12 +156,12 @@ void coda_iattr_to_vattr(struct iattr *iattr, struct coda_vattr *vattr)
vattr->va_uid = (vuid_t) -1;
vattr->va_gid = (vgid_t) -1;
vattr->va_size = (off_t) -1;
- vattr->va_atime.tv_sec = (time_t) -1;
- vattr->va_atime.tv_nsec = (time_t) -1;
- vattr->va_mtime.tv_sec = (time_t) -1;
- vattr->va_mtime.tv_nsec = (time_t) -1;
- vattr->va_ctime.tv_sec = (time_t) -1;
- vattr->va_ctime.tv_nsec = (time_t) -1;
+ vattr->va_atime.tv_sec = (long) -1;
+ vattr->va_atime.tv_nsec = (long) -1;
+ vattr->va_mtime.tv_sec = (long) -1;
+ vattr->va_mtime.tv_nsec = (long) -1;
+ vattr->va_ctime.tv_sec = (long) -1;
+ vattr->va_ctime.tv_nsec = (long) -1;
vattr->va_type = C_VNON;
vattr->va_fileid = -1;
vattr->va_gen = -1;
@@ -175,13 +201,13 @@ void coda_iattr_to_vattr(struct iattr *iattr, struct coda_vattr *vattr)
vattr->va_size = iattr->ia_size;
}
if ( valid & ATTR_ATIME ) {
- vattr->va_atime = timespec64_to_timespec(iattr->ia_atime);
+ vattr->va_atime = timespec64_to_coda(iattr->ia_atime);
}
if ( valid & ATTR_MTIME ) {
- vattr->va_mtime = timespec64_to_timespec(iattr->ia_mtime);
+ vattr->va_mtime = timespec64_to_coda(iattr->ia_mtime);
}
if ( valid & ATTR_CTIME ) {
- vattr->va_ctime = timespec64_to_timespec(iattr->ia_ctime);
+ vattr->va_ctime = timespec64_to_coda(iattr->ia_ctime);
}
}
diff --git a/include/uapi/linux/coda.h b/include/uapi/linux/coda.h
index 695fade33c64..027a8eb04423 100644
--- a/include/uapi/linux/coda.h
+++ b/include/uapi/linux/coda.h
@@ -211,6 +211,20 @@ struct CodaFid {
*/
enum coda_vtype { C_VNON, C_VREG, C_VDIR, C_VBLK, C_VCHR, C_VLNK, C_VSOCK, C_VFIFO, C_VBAD };
+#ifdef __linux__
+/*
+ * This matches the traditional Linux 'timespec' structure binary layout,
+ * before using 64-bit time_t everywhere. Overflows in y2038 on 32-bit
+ * architectures.
+ */
+struct vtimespec {
+ long tv_sec; /* seconds */
+ long tv_nsec; /* nanoseconds */
+};
+#else
+#define vtimespec timespec
+#endif
+
struct coda_vattr {
long va_type; /* vnode type (for create) */
u_short va_mode; /* files access mode and type */
@@ -220,9 +234,9 @@ struct coda_vattr {
long va_fileid; /* file id */
u_quad_t va_size; /* file size in bytes */
long va_blocksize; /* blocksize preferred for i/o */
- struct timespec va_atime; /* time of last access */
- struct timespec va_mtime; /* time of last modification */
- struct timespec va_ctime; /* time file changed */
+ struct vtimespec va_atime; /* time of last access */
+ struct vtimespec va_mtime; /* time of last modification */
+ struct vtimespec va_ctime; /* time file changed */
u_long va_gen; /* generation number of file */
u_long va_flags; /* flags defined for file */
cdev_t va_rdev; /* device special file represents */
--
2.9.0
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v11 00/13] Intel SGX1 support
From: Jarkko Sakkinen @ 2018-06-19 14:59 UTC (permalink / raw)
To: Pavel Machek
Cc: x86, platform-driver-x86, dave.hansen, sean.j.christopherson,
nhorman, npmccallum, Alexei Starovoitov, Andi Kleen,
Andrew Morton, Andy Lutomirski, Borislav Petkov, David S. Miller,
David Woodhouse, Greg Kroah-Hartman, H. Peter Anvin, Ingo Molnar,
open list:INTEL SGX, Janakarajan Natarajan, Kirill A. Shutemov,
Konrad Rzeszutek Wilk,
open list:KERNEL VIRTUAL MACHINE FOR X86 (KVM/x86), Len Brown,
Linus Walleij, open list:CRYPTO API, open list:DOCUMENTATION,
open list, open list:SPARSE CHECKER, Mauro Carvalho Chehab,
Peter Zijlstra, Rafael J. Wysocki, Randy Dunlap, Ricardo Neri,
Thomas Gleixner, Tom Lendacky, Vikas Shivappa
In-Reply-To: <20180612105011.GA26931@amd>
On Tue, Jun 12, 2018 at 12:50:12PM +0200, Pavel Machek wrote:
> On Fri 2018-06-08 19:09:35, Jarkko Sakkinen wrote:
> > Intel(R) SGX is a set of CPU instructions that can be used by applications
> > to set aside private regions of code and data. The code outside the enclave
> > is disallowed to access the memory inside the enclave by the CPU access
> > control. In a way you can think that SGX provides inverted sandbox. It
> > protects the application from a malicious host.
>
> Do you intend to allow non-root applications to use SGX?
>
> What are non-evil uses for SGX?
>
> ...because it is quite useful for some kinds of evil:
The default permissions for the device are 600.
/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 06/10] x86/cet: Add arch_prctl functions for shadow stack
From: Andy Lutomirski @ 2018-06-19 14:50 UTC (permalink / raw)
To: Kees Cook
Cc: Andy Lutomirski, H. J. Lu, Thomas Gleixner, Yu-cheng Yu, LKML,
linux-doc, Linux-MM, linux-arch, X86 ML, H. Peter Anvin,
Ingo Molnar, Shanbhogue, Vedvyas, Ravi V. Shankar, Dave Hansen,
Jonathan Corbet, Oleg Nesterov, Arnd Bergmann, mike.kravetz,
Florian Weimer
In-Reply-To: <CAGXu5jK0gospOXRpN6zYiQPXOZeE=YpVAz2qu4Zc3-32v85+EQ@mail.gmail.com>
> On Jun 18, 2018, at 5:52 PM, Kees Cook <keescook@chromium.org> wrote:
>
>> On Mon, Jun 18, 2018 at 3:03 PM, Andy Lutomirski <luto@kernel.org> wrote:
>>> On Tue, Jun 12, 2018 at 12:34 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>>>
>>>> On Tue, Jun 12, 2018 at 11:59 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>>>>> On Tue, 12 Jun 2018, H.J. Lu wrote:
>>>>>> On Tue, Jun 12, 2018 at 9:34 AM, Andy Lutomirski <luto@kernel.org> wrote:
>>>>>>> On Tue, Jun 12, 2018 at 9:05 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>>> On Tue, Jun 12, 2018 at 9:01 AM, Andy Lutomirski <luto@kernel.org> wrote:
>>>>>>>>> On Tue, Jun 12, 2018 at 4:43 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>>>>> On Tue, Jun 12, 2018 at 3:03 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>>>>>>>>>> That works for stuff which loads all libraries at start time, but what
>>>>>>>>>> happens if the program uses dlopen() later on? If CET is force locked and
>>>>>>>>>> the library is not CET enabled, it will fail.
>>>>>>>>>
>>>>>>>>> That is to prevent disabling CET by dlopening a legacy shared library.
>>>>>>>>>
>>>>>>>>>> I don't see the point of trying to support CET by magic. It adds complexity
>>>>>>>>>> and you'll never be able to handle all corner cases correctly. dlopen() is
>>>>>>>>>> not even a corner case.
>>>>>>>>>
>>>>>>>>> That is a price we pay for security. To enable CET, especially shadow
>>>>>>>>> shack, the program and all of shared libraries it uses should be CET
>>>>>>>>> enabled. Most of programs can be enabled with CET by compiling them
>>>>>>>>> with -fcf-protection.
>>>>>>>>
>>>>>>>> If you charge too high a price for security, people may turn it off.
>>>>>>>> I think we're going to need a mode where a program says "I want to use
>>>>>>>> the CET, but turn it off if I dlopen an unsupported library". There
>>>>>>>> are programs that load binary-only plugins.
>>>>>>>
>>>>>>> You can do
>>>>>>>
>>>>>>> # export GLIBC_TUNABLES=glibc.tune.hwcaps=-SHSTK
>>>>>>>
>>>>>>> which turns off shadow stack.
>>>>>>>
>>>>>>
>>>>>> Which exactly illustrates my point. By making your security story too
>>>>>> absolute, you'll force people to turn it off when they don't need to.
>>>>>> If I'm using a fully CET-ified distro and I'm using a CET-aware
>>>>>> program that loads binary plugins, and I may or may not have an old
>>>>>> (binary-only, perhaps) plugin that doesn't support CET, then the
>>>>>> behavior I want is for CET to be on until I dlopen() a program that
>>>>>> doesn't support it. Unless there's some ABI reason why that can't be
>>>>>> done, but I don't think there is.
>>>>>
>>>>> We can make it opt-in via GLIBC_TUNABLES. But by default, the legacy
>>>>> shared object is disallowed when CET is enabled.
>>>>
>>>> That's a bad idea. Stuff has launchers which people might not be able to
>>>> change. So they will simply turn of CET completely or it makes them hack
>>>> horrible crap into init, e.g. the above export.
>>>>
>>>> Give them sane kernel options:
>>>>
>>>> cet = off, relaxed, forced
>>>>
>>>> where relaxed allows to run binary plugins. Then let dlopen() call into the
>>>> kernel with the filepath of the library to check for CET and that will tell
>>>> you whether its ok or or not and do the necessary magic in the kernel when
>>>> CET has to be disabled due to a !CET library/application.
>>>>
>>>> That's also making the whole thing independent of magic glibc environment
>>>> options and allows it to be used all over the place in the same way.
>>>
>>> This is very similar to our ARCH_CET_EXEC proposal which controls how
>>> CET should be enforced. But Andy thinks it is a bad idea.
>>
>> I do think it's a bad idea to have a new piece of state that survives
>> across exec(). It's going to have nasty usability problems and nasty
>> security problems.
>>
>> We may need a mode by which glibc can turn CET *back off* even after a
>> program had it on if it dlopens() an old binary. Or maybe there won't
>> be demand. I can certainly understand why the CET_LOCK feature is
>> there, although I think we need a way to override it using something
>> like ptrace(). I'm not convinced that CET_LOCK is really needed, but
>> someone who understand the thread model should chime in.
>>
>> Kees, do you know anyone who has a good enough understanding of
>> usermode exploits and how they'll interact with CET?
>
> Adding Florian to CC, but if something gets CET enabled, it really
> shouldn't have a way to turn it off. If there's a way to turn it off,
> all the ROP research will suddenly turn to exactly one gadget before
> doing the rest of the ROP: turning off CET. Right now ROP is: use
> stack-pivot gadget, do everything else. Allowed CET to turn off will
> just add one step: use CET-off gadget, use stack-pivot gadget, do
> everything else. :P
Fair enough
>
> Following Linus's request for "slow introduction" of new security
> features, likely the best approach is to default to "relaxed" (with a
> warning about down-grades), and allow distros/end-users to pick
> "forced" if they know their libraries are all CET-enabled.
I still don’t get what “relaxed” is for. I think the right design is:
Processes start with CET on or off depending on the ELF note, but they start with CET unlocked no matter what. They can freely switch CET on and off (subject to being clever enough not to crash if they turn it on and then return right off the end of the shadow stack) until they call ARCH_CET_LOCK.
Ptrace gets new APIs to turn CET on and off and to lock and unlock it. If an attacker finds a “ptrace me and turn off CET” gadget, then they might as well just do “ptrace me and write shell code” instead. It’s basically the same gadget. Keep in mind that the actual sequence of syscalls to do this is incredibly complicated.
It’s unclear to me that forcing CET on belongs in the kernel at all. By the time an attacker can find a non-CET ELF binary and can exec it in a context where it does their bidding, the attacker is far beyond what CET can even try to help. At this point we’re talking about an attacker who can effectively invoke system(3) with arbitrary parameters, and attackers with *that* power don’t need ROP and the like.
There is a new feature I’d like to see, though: add an ELF note to bless a binary as being an ELF interpreter. And add an LSM callback to validate an ELF interpreter. Let’s minimize the shenanigans that people who control containers can get up to. (Obviously the ELF note part would need to be opt-in.)--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 0/2] Documentation/sphinx: add "nodocs" directive
From: Mike Rapoport @ 2018-06-19 14:49 UTC (permalink / raw)
To: Matthew Wilcox; +Cc: Jonathan Corbet, Matthew Wilcox, Jani Nikula, linux-doc
In-Reply-To: <20180619141156.GC1438@bombadil.infradead.org>
On Tue, Jun 19, 2018 at 07:11:56AM -0700, Matthew Wilcox wrote:
> On Mon, Jun 18, 2018 at 10:10:28AM -0700, Matthew Wilcox wrote:
> > On Mon, Jun 18, 2018 at 04:36:34PM +0300, Mike Rapoport wrote:
> > > Hi,
> > >
> > > These patches allow passing "-no-doc-sections" option to scripts/kernel-doc
> > > from the sphinx generator.
> > >
> > > This allows to avoid duplicated DOC: sections when "kernel-doc:" directive
> > > is used without explicit selection of functions or function types. For
> > > instance, [1] has "IDA description" and "idr synchronization" twice.
> >
> > Hah, I just found an abandoned patch for this in a disused git tree.
> > I was wondering whether I needed to resurrect it. Enthusiastically,
> >
> > Acked-by: Matthew Wilcox <willy@infradead.org>
>
> Here's the patch I found (I couldn't refind it at the time):
>
> diff --git a/Documentation/sphinx/kerneldoc.py b/Documentation/sphinx/kerneldoc.py
> index d15e07f36881..d86d88da1d75 100644
> --- a/Documentation/sphinx/kerneldoc.py
> +++ b/Documentation/sphinx/kerneldoc.py
> @@ -47,6 +47,7 @@ class KernelDocDirective(Directive):
> optional_arguments = 4
> option_spec = {
> 'doc': directives.unchanged_required,
> + 'nodoc': directives.unchanged,
> 'functions': directives.unchanged_required,
> 'export': directives.unchanged,
> 'internal': directives.unchanged,
> @@ -74,6 +75,8 @@ class KernelDocDirective(Directive):
> export_file_patterns = str(self.options.get('internal')).split()
> elif 'doc' in self.options:
> cmd += ['-function', str(self.options.get('doc'))]
> + elif 'nodoc' in self.options:
> + cmd += ['-no-doc-sections']
> elif 'functions' in self.options:
> for f in str(self.options.get('functions')).split():
> cmd += ['-function', f]
>
> I did it while I was trying to create good radix tree documentation,
> which led to me realising that was a Herculean task (specifically: the
> stables).
>
> I ended up doing this instead:
>
> +The Public API
> +==============
> +
> +The public API can be found in ``<linux/radix-tree.h>``. To use a
> +radix tree in your data structure, embed a :c:type:`struct radix_tree_root`
> +in it, and initialise it using ``INIT_RADIX_TREE``. You can also use
> +a file-local or global radix tree by defining a :c:type:`RADIX_TREE` as you
> +would a :c:type:`LIST_HEAD`.
> +
> +.. Not actually "internal", but I need to exclude the 'doc' paragraph, and
> + this is the best way to do it.
> +.. kernel-doc:: include/linux/radix-tree.h
> + :internal:
> +
> +.. kernel-doc:: lib/radix-tree.c
> + :export:
>
> I'm not sure if I agree with me-of-January-2017 that this is the "best"
> way to do it, but maybe that'll point to another way of achieving the
> same thing.
>
Actually, in the IDR case the same trick would work.
In more general case it's even possible to use
.. kernel-doc:: path/to/file.c
:export:
.. kernel-doc: path/to/file.c
:internal:
and get all the functions without the docs sections :)
But IMHO having a directive for this would be nicer. If there are no
objections to Jani's suggestion to use empty :functions: instead of
:nodoc: I'm going to send v2.
--
Sincerely yours,
Mike.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: overlayfs: caller_credentials option bypass creator_cred
From: Vivek Goyal @ 2018-06-19 14:36 UTC (permalink / raw)
To: Mark Salyzyn
Cc: linux-kernel, Miklos Szeredi, Jonathan Corbet, linux-unionfs,
linux-doc, Daniel Walsh, Stephen Smalley
In-Reply-To: <aecdeaca-74cf-54b1-3ea9-4751972074f5@android.com>
On Mon, Jun 18, 2018 at 02:59:50PM -0700, Mark Salyzyn wrote:
> On 06/18/2018 12:43 PM, Vivek Goyal wrote:
> > Will it be acceptable to write security policies in such a way so that
> > mounter has access as well.
> Unfortunately No. Policy of minimizing attack surface for a contained root
> service (init in this case). Just because it can mount, does not mean it can
> modify critical content; an attacker could use this to open a hole.
>
> > Current model does assume that mounter has privileges on underlying files.
>
> Only ones it appears to need is the workdir AFAIK, had to add ability to
> create in the <wordir> xattr in order to enable r/w mounts later. Although
> not all corners were tested, I did not see any copy_up issues b/c the caller
> had the privs in the Android security model when mounted with this new flag.
So in this system all callers are priviliged and have the capability to
mknod and set trusted xattrs. (Amir mentioned the reason why we switch
creds). If not, then file unlink (Should do mknod), lower non-empty directory
rename (should set trusted REDIRECT) and bunch of other operations should fail.
Thanks
Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] overlayfs: caller_credentials option bypass creator_cred
From: Vivek Goyal @ 2018-06-19 14:16 UTC (permalink / raw)
To: Mark Salyzyn
Cc: linux-kernel, Miklos Szeredi, Jonathan Corbet, linux-unionfs,
linux-doc
In-Reply-To: <20180618192726.67981-1-salyzyn@android.com>
On Mon, Jun 18, 2018 at 12:27:19PM -0700, Mark Salyzyn wrote:
> All accesses to the lower filesystems reference the creator (mount)
> context. This is a security issue as the user context does not
> overlay the creator context.
>
> A module bool parameter and mount option caller_credentials is added
> to set the default, and to act as a presence check for this "feature".
> The module parameter is used to change the default behavior, But
> since I can not break user API, I have to preserve the original
> behavior as default.
>
> On MAC security model expect to set the caller_credentials to Y as
> part of early initialization to preserve security model when the
> option of iether caller_credentials or creator_credentials is not
> explicitly specified in the mount.
>
> Signed-off-by: Mark Salyzyn <salyzyn@android.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Vivek Goyal <vgoyal@redhat.com>
> Cc: linux-unionfs@vger.kernel.org
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
>
> v2:
> - forward port changed attr to stat, resulting in a build error
> - wording in commit message was too adversarial and needed correction
> ---
> Documentation/filesystems/overlayfs.txt | 7 +++++++
> fs/overlayfs/copy_up.c | 3 ++-
> fs/overlayfs/dir.c | 12 ++++++++----
> fs/overlayfs/inode.c | 24 ++++++++++++++++--------
> fs/overlayfs/namei.c | 9 ++++++---
> fs/overlayfs/ovl_entry.h | 1 +
> fs/overlayfs/readdir.c | 6 ++++--
> fs/overlayfs/super.c | 22 ++++++++++++++++++++++
> fs/overlayfs/util.c | 8 ++++++--
> 9 files changed, 72 insertions(+), 20 deletions(-)
>
> diff --git a/Documentation/filesystems/overlayfs.txt b/Documentation/filesystems/overlayfs.txt
> index 72615a2c0752..4328be5cdaa9 100644
> --- a/Documentation/filesystems/overlayfs.txt
> +++ b/Documentation/filesystems/overlayfs.txt
> @@ -106,6 +106,13 @@ Only the lists of names from directories are merged. Other content
> such as metadata and extended attributes are reported for the upper
> directory only. These attributes of the lower directory are hidden.
>
> +credentials
> +-----------
> +
> +All access to the upper, lower and work directories is the creator's
> +credentials.
Can we call it "mounter's credentials" instead of "creator's credential"
everywhere in the patch. It just feels more intuitive and that's how
we have been referrring to it.
> If caller_credentials is set, then the access caller's
> +instance credentials will be used.
> +
> whiteouts and opaque directories
> --------------------------------
>
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index ddaddb4ce4c3..abc21844f712 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -790,7 +790,8 @@ int ovl_copy_up_flags(struct dentry *dentry, int flags)
> dput(parent);
> dput(next);
> }
> - revert_creds(old_cred);
> + if (old_cred)
> + revert_creds(old_cred);
How about defining ovl_revert_creds(old_cred) helper and do nothing if old_cred
is null. This will be similar to ovl_override_creds() helper which does
not switch creds based on a mount option.
>
> return err;
> }
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index f480b1a2cd2e..8b75e0899ee1 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -561,7 +561,8 @@ static int ovl_create_or_link(struct dentry *dentry, struct inode *inode,
> override_cred->fsgid = inode->i_gid;
> if (!attr->hardlink) {
> err = security_dentry_create_files_as(dentry,
> - attr->mode, &dentry->d_name, old_cred,
> + attr->mode, &dentry->d_name,
> + old_cred ? old_cred : current_cred(),
> override_cred);
Calling this security hook should be unnecessary if actual file creation
is happening in the context of caller's creds. This hook was needed when
new file creation was happening with mounter's creds.
We probably will have to define another helper to organize this code
better. For now, it probably is fine, and not a big deal.
> if (err) {
> put_cred(override_cred);
> @@ -577,7 +578,8 @@ static int ovl_create_or_link(struct dentry *dentry, struct inode *inode,
> err = ovl_create_over_whiteout(dentry, inode, attr);
> }
> out_revert_creds:
> - revert_creds(old_cred);
> + if (old_cred)
> + revert_creds(old_cred);
> return err;
> }
>
> @@ -824,7 +826,8 @@ static int ovl_do_remove(struct dentry *dentry, bool is_dir)
> err = ovl_remove_upper(dentry, is_dir, &list);
> else
> err = ovl_remove_and_whiteout(dentry, &list);
> - revert_creds(old_cred);
> + if (old_cred)
> + revert_creds(old_cred);
> if (!err) {
> if (is_dir)
> clear_nlink(dentry->d_inode);
> @@ -1150,7 +1153,8 @@ static int ovl_rename(struct inode *olddir, struct dentry *old,
> out_unlock:
> unlock_rename(new_upperdir, old_upperdir);
> out_revert_creds:
> - revert_creds(old_cred);
> + if (old_cred)
> + revert_creds(old_cred);
> ovl_nlink_end(new, locked);
> out_drop_write:
> ovl_drop_write(old);
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index ed16a898caeb..222678b6e67e 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -49,7 +49,8 @@ int ovl_setattr(struct dentry *dentry, struct iattr *attr)
> inode_lock(upperdentry->d_inode);
> old_cred = ovl_override_creds(dentry->d_sb);
> err = notify_change(upperdentry, attr, NULL);
> - revert_creds(old_cred);
> + if (old_cred)
> + revert_creds(old_cred);
> if (!err)
> ovl_copyattr(upperdentry->d_inode, dentry->d_inode);
> inode_unlock(upperdentry->d_inode);
> @@ -208,7 +209,8 @@ int ovl_getattr(const struct path *path, struct kstat *stat,
> stat->nlink = dentry->d_inode->i_nlink;
>
> out:
> - revert_creds(old_cred);
> + if (old_cred)
> + revert_creds(old_cred);
>
> return err;
> }
> @@ -242,7 +244,8 @@ int ovl_permission(struct inode *inode, int mask)
> mask |= MAY_READ;
> }
> err = inode_permission(realinode, mask);
> - revert_creds(old_cred);
> + if (old_cred)
> + revert_creds(old_cred);
>
> return err;
> }
> @@ -259,7 +262,8 @@ static const char *ovl_get_link(struct dentry *dentry,
>
> old_cred = ovl_override_creds(dentry->d_sb);
> p = vfs_get_link(ovl_dentry_real(dentry), done);
> - revert_creds(old_cred);
> + if (old_cred)
> + revert_creds(old_cred);
> return p;
> }
>
> @@ -302,7 +306,8 @@ int ovl_xattr_set(struct dentry *dentry, struct inode *inode, const char *name,
> WARN_ON(flags != XATTR_REPLACE);
> err = vfs_removexattr(realdentry, name);
> }
> - revert_creds(old_cred);
> + if (old_cred)
> + revert_creds(old_cred);
>
> out_drop_write:
> ovl_drop_write(dentry);
> @@ -320,7 +325,8 @@ int ovl_xattr_get(struct dentry *dentry, struct inode *inode, const char *name,
>
> old_cred = ovl_override_creds(dentry->d_sb);
> res = vfs_getxattr(realdentry, name, value, size);
> - revert_creds(old_cred);
> + if (old_cred)
> + revert_creds(old_cred);
> return res;
> }
>
> @@ -344,7 +350,8 @@ ssize_t ovl_listxattr(struct dentry *dentry, char *list, size_t size)
>
> old_cred = ovl_override_creds(dentry->d_sb);
> res = vfs_listxattr(realdentry, list, size);
> - revert_creds(old_cred);
> + if (old_cred)
> + revert_creds(old_cred);
> if (res <= 0 || size == 0)
> return res;
>
> @@ -379,7 +386,8 @@ struct posix_acl *ovl_get_acl(struct inode *inode, int type)
>
> old_cred = ovl_override_creds(inode->i_sb);
> acl = get_acl(realinode, type);
> - revert_creds(old_cred);
> + if (old_cred)
> + revert_creds(old_cred);
>
> return acl;
> }
> diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
> index c993dd8db739..c8b84d262ec2 100644
> --- a/fs/overlayfs/namei.c
> +++ b/fs/overlayfs/namei.c
> @@ -1024,7 +1024,8 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
> OVL_I(inode)->redirect = upperredirect;
> }
>
> - revert_creds(old_cred);
> + if (old_cred)
> + revert_creds(old_cred);
> dput(index);
> kfree(stack);
> kfree(d.redirect);
> @@ -1043,7 +1044,8 @@ struct dentry *ovl_lookup(struct inode *dir, struct dentry *dentry,
> kfree(upperredirect);
> out:
> kfree(d.redirect);
> - revert_creds(old_cred);
> + if (old_cred)
> + revert_creds(old_cred);
> return ERR_PTR(err);
> }
>
> @@ -1097,7 +1099,8 @@ bool ovl_lower_positive(struct dentry *dentry)
> dput(this);
> }
> }
> - revert_creds(old_cred);
> + if (old_cred)
> + revert_creds(old_cred);
>
> return positive;
> }
> diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
> index 41655a7d6894..7e17db561a04 100644
> --- a/fs/overlayfs/ovl_entry.h
> +++ b/fs/overlayfs/ovl_entry.h
> @@ -18,6 +18,7 @@ struct ovl_config {
> const char *redirect_mode;
> bool index;
> bool nfs_export;
> + bool caller_credentials;
> int xino;
> };
>
> diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> index ef1fe42ff7bb..af3874d589ad 100644
> --- a/fs/overlayfs/readdir.c
> +++ b/fs/overlayfs/readdir.c
> @@ -289,7 +289,8 @@ static int ovl_check_whiteouts(struct dentry *dir, struct ovl_readdir_data *rdd)
> }
> inode_unlock(dir->d_inode);
> }
> - revert_creds(old_cred);
> + if (old_cred)
> + revert_creds(old_cred);
>
> return err;
> }
> @@ -906,7 +907,8 @@ int ovl_check_empty_dir(struct dentry *dentry, struct list_head *list)
>
> old_cred = ovl_override_creds(dentry->d_sb);
> err = ovl_dir_read_merged(dentry, list, &root);
> - revert_creds(old_cred);
> + if (old_cred)
> + revert_creds(old_cred);
> if (err)
> return err;
>
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index 704b37311467..184688ab35cb 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -56,6 +56,11 @@ module_param_named(xino_auto, ovl_xino_auto_def, bool, 0644);
> MODULE_PARM_DESC(ovl_xino_auto_def,
> "Auto enable xino feature");
>
> +static bool __read_mostly ovl_caller_credentials;
> +module_param_named(caller_credentials, ovl_caller_credentials, bool, 0644);
> +MODULE_PARM_DESC(ovl_caller_credentials,
> + "Use caller credentials rather than creator credentials for accesses");
> +
> static void ovl_entry_stack_free(struct ovl_entry *oe)
> {
> unsigned int i;
> @@ -376,6 +381,10 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry)
> "on" : "off");
> if (ofs->config.xino != ovl_xino_def())
> seq_printf(m, ",xino=%s", ovl_xino_str[ofs->config.xino]);
> + if (ofs->config.caller_credentials)
> + seq_puts(m, ",caller_credentials");
> + else
> + seq_puts(m, ",creator_credentials");
> return 0;
> }
>
> @@ -413,6 +422,8 @@ enum {
> OPT_XINO_ON,
> OPT_XINO_OFF,
> OPT_XINO_AUTO,
> + OPT_CREATOR_CREDENTIALS,
> + OPT_CALLER_CREDENTIALS,
> OPT_ERR,
> };
>
> @@ -429,6 +440,8 @@ static const match_table_t ovl_tokens = {
> {OPT_XINO_ON, "xino=on"},
> {OPT_XINO_OFF, "xino=off"},
> {OPT_XINO_AUTO, "xino=auto"},
> + {OPT_CREATOR_CREDENTIALS, "creator_credentials"},
> + {OPT_CALLER_CREDENTIALS, "caller_credentials"},
I like Amir's suggestion of using override_creds=on/off, with
override_creds=on being default.
Thanks
Vivek
> {OPT_ERR, NULL}
> };
>
> @@ -486,6 +499,7 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
> if (!config->redirect_mode)
> return -ENOMEM;
>
> + config->caller_credentials = ovl_caller_credentials;
> while ((p = ovl_next_opt(&opt)) != NULL) {
> int token;
> substring_t args[MAX_OPT_ARGS];
> @@ -555,6 +569,14 @@ static int ovl_parse_opt(char *opt, struct ovl_config *config)
> config->xino = OVL_XINO_AUTO;
> break;
>
> + case OPT_CREATOR_CREDENTIALS:
> + config->caller_credentials = false;
> + break;
> +
> + case OPT_CALLER_CREDENTIALS:
> + config->caller_credentials = true;
> + break;
> +
> default:
> pr_err("overlayfs: unrecognized mount option \"%s\" or missing value\n", p);
> return -EINVAL;
> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
> index 6f1078028c66..538802289511 100644
> --- a/fs/overlayfs/util.c
> +++ b/fs/overlayfs/util.c
> @@ -40,6 +40,8 @@ const struct cred *ovl_override_creds(struct super_block *sb)
> {
> struct ovl_fs *ofs = sb->s_fs_info;
>
> + if (ofs->config.caller_credentials)
> + return NULL;
> return override_creds(ofs->creator_cred);
> }
>
> @@ -630,7 +632,8 @@ int ovl_nlink_start(struct dentry *dentry, bool *locked)
> * value relative to the upper inode nlink in an upper inode xattr.
> */
> err = ovl_set_nlink_upper(dentry);
> - revert_creds(old_cred);
> + if (old_cred)
> + revert_creds(old_cred);
>
> out:
> if (err)
> @@ -650,7 +653,8 @@ void ovl_nlink_end(struct dentry *dentry, bool locked)
>
> old_cred = ovl_override_creds(dentry->d_sb);
> ovl_cleanup_index(dentry);
> - revert_creds(old_cred);
> + if (old_cred)
> + revert_creds(old_cred);
> }
>
> mutex_unlock(&OVL_I(d_inode(dentry))->lock);
> --
> 2.18.0.rc1.244.gcf134e6275-goog
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 0/2] Documentation/sphinx: add "nodocs" directive
From: Matthew Wilcox @ 2018-06-19 14:11 UTC (permalink / raw)
To: Mike Rapoport; +Cc: Jonathan Corbet, Matthew Wilcox, Jani Nikula, linux-doc
In-Reply-To: <20180618171028.GD28748@bombadil.infradead.org>
On Mon, Jun 18, 2018 at 10:10:28AM -0700, Matthew Wilcox wrote:
> On Mon, Jun 18, 2018 at 04:36:34PM +0300, Mike Rapoport wrote:
> > Hi,
> >
> > These patches allow passing "-no-doc-sections" option to scripts/kernel-doc
> > from the sphinx generator.
> >
> > This allows to avoid duplicated DOC: sections when "kernel-doc:" directive
> > is used without explicit selection of functions or function types. For
> > instance, [1] has "IDA description" and "idr synchronization" twice.
>
> Hah, I just found an abandoned patch for this in a disused git tree.
> I was wondering whether I needed to resurrect it. Enthusiastically,
>
> Acked-by: Matthew Wilcox <willy@infradead.org>
Here's the patch I found (I couldn't refind it at the time):
diff --git a/Documentation/sphinx/kerneldoc.py b/Documentation/sphinx/kerneldoc.py
index d15e07f36881..d86d88da1d75 100644
--- a/Documentation/sphinx/kerneldoc.py
+++ b/Documentation/sphinx/kerneldoc.py
@@ -47,6 +47,7 @@ class KernelDocDirective(Directive):
optional_arguments = 4
option_spec = {
'doc': directives.unchanged_required,
+ 'nodoc': directives.unchanged,
'functions': directives.unchanged_required,
'export': directives.unchanged,
'internal': directives.unchanged,
@@ -74,6 +75,8 @@ class KernelDocDirective(Directive):
export_file_patterns = str(self.options.get('internal')).split()
elif 'doc' in self.options:
cmd += ['-function', str(self.options.get('doc'))]
+ elif 'nodoc' in self.options:
+ cmd += ['-no-doc-sections']
elif 'functions' in self.options:
for f in str(self.options.get('functions')).split():
cmd += ['-function', f]
I did it while I was trying to create good radix tree documentation,
which led to me realising that was a Herculean task (specifically: the
stables).
I ended up doing this instead:
+The Public API
+==============
+
+The public API can be found in ``<linux/radix-tree.h>``. To use a
+radix tree in your data structure, embed a :c:type:`struct radix_tree_root`
+in it, and initialise it using ``INIT_RADIX_TREE``. You can also use
+a file-local or global radix tree by defining a :c:type:`RADIX_TREE` as you
+would a :c:type:`LIST_HEAD`.
+
+.. Not actually "internal", but I need to exclude the 'doc' paragraph, and
+ this is the best way to do it.
+.. kernel-doc:: include/linux/radix-tree.h
+ :internal:
+
+.. kernel-doc:: lib/radix-tree.c
+ :export:
I'm not sure if I agree with me-of-January-2017 that this is the "best"
way to do it, but maybe that'll point to another way of achieving the
same thing.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v11 12/13] intel_sgx: driver documentation
From: Jarkko Sakkinen @ 2018-06-19 13:31 UTC (permalink / raw)
To: Randy Dunlap
Cc: x86, platform-driver-x86, dave.hansen, sean.j.christopherson,
nhorman, npmccallum, Jonathan Corbet, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, open list:DOCUMENTATION, open list
In-Reply-To: <d88234f6-d9f9-561e-b1ab-4bfaddc3dd72@infradead.org>
On Fri, Jun 08, 2018 at 02:41:07PM -0700, Randy Dunlap wrote:
> On 06/08/2018 10:09 AM, Jarkko Sakkinen wrote:
> > Documentation of the features of the Software Guard eXtensions usable
> > for the Linux kernel and how the driver internals uses these features.
> > In addition, contains documentation for the ioctl API.
> >
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>
> Hi,
>
> I have a few corrections below...
>
>
> > ---
> > Documentation/index.rst | 1 +
> > Documentation/x86/intel_sgx.rst | 195 ++++++++++++++++++++++++++++++++
> > 2 files changed, 196 insertions(+)
> > create mode 100644 Documentation/x86/intel_sgx.rst
> >
> > diff --git a/Documentation/index.rst b/Documentation/index.rst
> > index 3b99ab931d41..b9fb92928e8c 100644
> > --- a/Documentation/index.rst
> > +++ b/Documentation/index.rst
> > @@ -100,6 +100,7 @@ implementation.
> > :maxdepth: 2
> >
> > sh/index
> > + x86/index
> >
> > Korean translations
> > -------------------
> > diff --git a/Documentation/x86/intel_sgx.rst b/Documentation/x86/intel_sgx.rst
> > new file mode 100644
> > index 000000000000..ecbe544eb2cb
> > --- /dev/null
> > +++ b/Documentation/x86/intel_sgx.rst
> > @@ -0,0 +1,195 @@
> > +===================
> > +Intel(R) SGX driver
> > +===================
> > +
> > +Introduction
> > +============
> > +
> > +Intel(R) SGX is a set of CPU instructions that can be used by applications to
> > +set aside private regions of code and data. The code outside the enclave is
> > +disallowed to access the memory inside the enclave by the CPU access control.
> > +In a way you can think that SGX provides inverted sandbox. It protects the
> > +application from a malicious host.
> > +
> > +You can tell if your CPU supports SGX by looking into ``/proc/cpuinfo``:
> > +
> > + ``cat /proc/cpuinfo | grep sgx``
> > +
> > +Overview of SGX
> > +===============
> > +
> > +SGX has a set of data structures to maintain information about the enclaves and
> > +their security properties. BIOS reserves a fixed size region of physical memory
> > +for these structures by setting Processor Reserved Memory Range Registers
> > +(PRMRR).
> > +
> > +This memory range is protected from outside access by the CPU and all the data
> > +coming in and out of the CPU package is encrypted by a key that is generated for
> > +each boot cycle.
> > +
> > +Enclaves execute in ring-3 in a special enclave submode using pages from the
> > +reserved memory range. A fixed logical address range for the enclave is reserved
> > +by ENCLS(ECREATE), a leaf instruction used to create enclaves. It is referred in
> > +the documentation commonly as the ELRANGE.
> > +
> > +Every memory access to the ELRANGE is asserted by the CPU. If the CPU is not
> > +executing in the enclave mode inside the enclave, #GP is raised. On the other
> > +hand enclave code can make memory accesses both inside and outside of the
> > +ELRANGE.
> > +
> > +Enclave can only execute code inside the ELRANGE. Instructions that may cause
> > +VMEXIT, IO instructions and instructions that require a privilege change are
> > +prohibited inside the enclave. Interrupts and exceptions always cause enclave
> > +to exit and jump to an address outside the enclave given when the enclave is
> > +entered by using the leaf instruction ENCLS(EENTER).
> > +
> > +Data types
> > +----------
> > +
> > +The protected memory range contains the following data:
> > +
> > +* **Enclave Page Cache (EPC):** protected pages
> > +* **Enclave Page Cache Map (EPCM):** a database that describes the state of the
> > + pages and link them to an enclave.
> > +
> > +EPC has a number of different types of pages:
> > +
> > +* **SGX Enclave Control Structure (SECS)**: describes the global
> > + properties of an enclave.
> > +* **Regular (REG):** code and data pages in the ELRANGE.
> > +* **Thread Control Structure (TCS):** pages that define entry points inside an
> > + enclave. The enclave can only be entered through these entry points and each
> > + can host a single hardware thread at a time.
> > +* **Version Array (VA)**: 64-bit version numbers for pages that have been
> > + swapped outside the enclave. Each page contains 512 version numbers.
> > +
> > +Launch control
> > +--------------
> > +
> > +To launch an enclave, two structures must be provided for ENCLS(EINIT):
> > +
> > +1. **SIGSTRUCT:** signed measurement of the enclave binary.
> > +2. **EINITTOKEN:** a cryptographic token CMAC-signed with a AES256-key called
> > + *launch key*, which is re-generated for each boot cycle.
> > +
> > +The CPU holds a SHA256 hash of a 3072-bit RSA public key inside
> > +IA32_SGXLEPUBKEYHASHn MSRs. Enclaves with a SIGSTRUCT that is signed with this
> > +key do not require a valid EINITTOKEN and can be authorized with special
> > +privileges. One of those privileges is ability to acquire the launch key with
> > +ENCLS(EGETKEY).
> > +
> > +**IA32_FEATURE_CONTROL[17]** is used by to BIOS configure whether
>
> by the BIOS to configure whether
>
> > +IA32_SGXLEPUBKEYHASH MSRs are read-only or read-write before locking the
> > +feature control register and handing over control to the operating system.
> > +
> > +Enclave construction
> > +--------------------
> > +
> > +The construction is started by filling out the SECS that contains enclave
> > +address range, privileged attributes and measurement of TCS and REG pages (pages
> > +that will be mapped to the address range) among the other things. This structure
> > +is passed out to the ENCLS(ECREATE) together with a physical address of a page
> > +in EPC that will hold the SECS.
> > +
> > +Then pages are added with ENCLS(EADD) and measured with ENCLS(EEXTEND). Finally
>
> "measured"? what does that mean?
>
> > +enclave is initialized with ENCLS(EINIT). ENCLS(INIT) checks that the SIGSTRUCT
> > +is signed with the contained public key and that the supplied EINITTOKEN is
> > +valid (CMAC'd with the launch key). If these hold, the enclave is successfully
> > +initialized.
> > +
> > +Swapping pages
> > +--------------
> > +
> > +Enclave pages can be swapped out with ENCLS(EWB) to the unprotected memory. In
> > +addition to the EPC page, ENCLS(EWB) takes in a VA page and address for PCMD
> > +structure (Page Crypto MetaData) as input. The VA page will seal a version
> > +number for the page. PCMD is 128 byte structure that contains tracking
> > +information for the page, most importantly its MAC. With these structures the
> > +enclave is sealed and rollback protected while it resides in the unprotected
> > +memory.
> > +
> > +Before the page can be swapped out it must not have any active TLB references.
> > +By using ENCLS(EBLOCK) instructions no new TLB entries can be created to it.
> > +After this the a counter called *epoch* associated hardware threads inside the
>
> huh?
>
> > +enclave is increased with ENCLS(ETRACK). After all the threads from the previous
> > +epoch have exited the page can be safely swapped out.
> > +
> > +An enclave memory access to a swapped out pages will cause #PF. #PF handler can
> > +fault the page back by using ENCLS(ELDU).
> > +
> > +Kernel internals
> > +================
> > +
> > +Requirements
> > +------------
> > +
> > +Because SGX has an ever evolving and expanding feature set, it's possible for
> > +a BIOS or VMM to configure a system in such a way that not all cpus are equal,
>
> CPUs
>
> > +e.g. where Launch Control is only enabled on a subset of cpus. Linux does
>
> CPUs.
>
> > +*not* support such a heterogenous system configuration, nor does it even
>
> heterogeneous
>
> > +attempt to play nice in the face of a misconfigured system. With the exception
> > +of Launch Control's hash MSRs, which can vary per cpu, Linux assumes that all
>
> CPU,
>
> > +cpus have a configuration that is identical to the boot cpu.
>
> CPUs CPU.
>
> > +
> > +
> > +Roles and responsibilities
> > +--------------------------
> > +
> > +SGX introduces system resources, e.g. EPC memory, that must be accessible to
> > +multiple entities, e.g. the native kernel driver (to expose SGX to userspace)
> > +and KVM (to expose SGX to VMs), ideally without introducing any dependencies
> > +between each SGX entity. To that end, the kernel owns and manages the shared
> > +system resources, i.e. the EPC and Launch Control MSRs, and defines functions
> > +that provide appropriate access to the shared resources. SGX support for
> > +userpace and VMs is left to the SGX platform driver and KVM respectively.
>
> userspace
>
> > +
> > +Launching enclaves
> > +------------------
> > +
> > +For privileged enclaves the launch is performed simply by submitting the
> > +SIGSTRUCT for that enclave to ENCLS(EINIT). For unprivileged enclaves the
> > +driver hosts a process in ring-3 that hosts a launch enclave signed with a key
> > +supplied for kbuild.
> > +
> > +The current implementation of the launch enclave generates a token for any
> > +enclave. In the future it could be potentially extended to have ways to
> > +configure policy what can be lauched.
>
> launched.
>
> > +
> > +The driver will fail to initialize if it cannot start its own launch enclave.
> > +A user space application can submit a SIGSTRUCT instance through the ioctl API.
> > +The kernel will take care of the rest.
> > +
> > +This design assures that the Linux kernel has always full control, which
> > +enclaves get to launch and which do not, even if the public key MSRs are
> > +read-only. Having launch intrinsics inside the kernel also enables easy
> > +development of enclaves without necessarily needing any heavy weight SDK.
> > +Having a low-barrier to implement enclaves could make sense for example for
>
> low barrier
>
> > +system daemons where amount of dependecies ought to be minimized.
>
> dependencies
>
> > +
> > +EPC management
> > +--------------
> > +
> > +Due to the unique requirements for swapping EPC pages, and because EPC pages
> > +(currently) do not have associated page structures, management of the EPC is
> > +not handled by the standard Linux swapper. SGX directly handles swapping
> > +of EPC pages, including a kthread to initiate reclaim and a rudimentary LRU
> > +mechanism. Consumsers of EPC pages, e.g. the SGX driver, are required to
>
> Consumers
>
> > +implement function callbacks that can be invoked by the kernel to age,
> > +swap, and/or forcefully reclaim a target EPC page. In effect, the kernel
> > +controls what happens and when, while the consumers (driver, KVM, etc..) do
> > +the actual work.
> > +
> > +SGX uapi
> > +========
> > +
> > +.. kernel-doc:: drivers/platform/x86/intel_sgx/sgx_ioctl.c
> > + :functions: sgx_ioc_enclave_create
> > + sgx_ioc_enclave_add_page
> > + sgx_ioc_enclave_init
> > +
> > +.. kernel-doc:: arch/x86/include/uapi/asm/sgx.h
> > +
> > +References
> > +==========
> > +
> > +* System Programming Manual: 39.1.4 Intel® SGX Launch Control Configuration
> >
>
>
> --
> ~Randy
Thank you, I'll refine the parts that you pointed out for the next
version.
/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v11 12/13] intel_sgx: driver documentation
From: Jarkko Sakkinen @ 2018-06-19 13:30 UTC (permalink / raw)
To: Jethro Beekman
Cc: x86, platform-driver-x86, dave.hansen, sean.j.christopherson,
nhorman, npmccallum, Jonathan Corbet, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, open list:DOCUMENTATION, open list
In-Reply-To: <6200618e-bcc8-23ba-afe8-4c6be8b364f0@fortanix.com>
On Fri, Jun 08, 2018 at 11:32:17AM -0700, Jethro Beekman wrote:
> On 2018-06-08 10:09, Jarkko Sakkinen wrote:
> > +Launching enclaves
> > +------------------
> > +
> > +For privileged enclaves the launch is performed simply by submitting the
> > +SIGSTRUCT for that enclave to ENCLS(EINIT). For unprivileged enclaves the
> > +driver hosts a process in ring-3 that hosts a launch enclave signed with a key
> > +supplied for kbuild.
> > +
> > +The current implementation of the launch enclave generates a token for any
> > +enclave. In the future it could be potentially extended to have ways to
> > +configure policy what can be lauched.
> > +
> > +The driver will fail to initialize if it cannot start its own launch enclave.
> > +A user space application can submit a SIGSTRUCT instance through the ioctl API.
> > +The kernel will take care of the rest.
> > +
> > +This design assures that the Linux kernel has always full control, which
> > +enclaves get to launch and which do not, even if the public key MSRs are
>
> As discussed previously at length, since the kernel needs to execute
> ENCLS[EINIT], it has full control to deny the launching of enclaves
> regardless of any launch enclave implementation. Please change this
> misleading statement.
Remember the discussion, forgot to change it. I'll fix this for the next
version.
/Jarkko
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] Documentation/sphinx: add "nodocs" directive
From: Mike Rapoport @ 2018-06-19 13:09 UTC (permalink / raw)
To: Jani Nikula; +Cc: Jonathan Corbet, Matthew Wilcox, linux-doc
In-Reply-To: <871sd3xkmx.fsf@intel.com>
On Tue, Jun 19, 2018 at 11:09:42AM +0300, Jani Nikula wrote:
> On Tue, 19 Jun 2018, Mike Rapoport <rppt@linux.vnet.ibm.com> wrote:
> > On Tue, Jun 19, 2018 at 10:29:20AM +0300, Jani Nikula wrote:
> >> On Tue, 19 Jun 2018, Mike Rapoport <rppt@linux.vnet.ibm.com> wrote:
> >> > On Mon, Jun 18, 2018 at 11:01:32PM +0300, Jani Nikula wrote:
> >> >> On Mon, 18 Jun 2018, Mike Rapoport <rppt@linux.vnet.ibm.com> wrote:
> >> >> > When kernel-doc:: specified in .rst document without explicit directives,
> >> >> > it outputs both comment and DOC: sections. If a DOC: section was explictly
> >> >> > included in the same document it will be duplicated. For example, the
> >> >> > output generated for Documentation/core-api/idr.rst [1] has "IDA
> >> >> > description" in the "IDA usage" section and in the middle of the API
> >> >> > reference.
> >> >> >
> >> >> > Addition of "nodocs" directive prevents the duplication without the need to
> >> >> > explicitly define what functions should be include in the API reference.
> >> >> >
> >> >> > [1] https://www.kernel.org/doc/html/v4.17/core-api/idr.html
> >> >> >
> >> >> > Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> >> >> > ---
> >> >> > Documentation/sphinx/kerneldoc.py | 3 +++
> >> >> > 1 file changed, 3 insertions(+)
> >> >> >
> >> >> > diff --git a/Documentation/sphinx/kerneldoc.py b/Documentation/sphinx/kerneldoc.py
> >> >> > index fbedcc3..bc5dd05 100644
> >> >> > --- a/Documentation/sphinx/kerneldoc.py
> >> >> > +++ b/Documentation/sphinx/kerneldoc.py
> >> >> > @@ -50,6 +50,7 @@ class KernelDocDirective(Directive):
> >> >> > 'functions': directives.unchanged_required,
> >> >> > 'export': directives.unchanged,
> >> >> > 'internal': directives.unchanged,
> >> >> > + 'nodocs': directives.unchanged,
> >> >>
> >> >> I'm not convinved this is the prettiest way to achieve what you
> >> >> want. 'nodocs' seems kind of clunky.
> >> >>
> >> >> I'd suggest supporting 'functions' without option arguments, and turning
> >> >> that into kernel-doc -no-doc-sections.
> >> >
> >> > Do you mean something like this:
> >>
> >> Yes.
> >>
> >> >
> >> > diff --git a/Documentation/sphinx/kerneldoc.py b/Documentation/sphinx/kerneldoc.py
> >> > index fbedcc3..9d0a7f0 100644
> >> > --- a/Documentation/sphinx/kerneldoc.py
> >> > +++ b/Documentation/sphinx/kerneldoc.py
> >> > @@ -47,7 +47,7 @@ class KernelDocDirective(Directive):
> >> > optional_arguments = 4
> >> > option_spec = {
> >> > 'doc': directives.unchanged_required,
> >> > - 'functions': directives.unchanged_required,
> >> > + 'functions': directives.unchanged,
> >> > 'export': directives.unchanged,
> >> > 'internal': directives.unchanged,
> >> > }
> >> > @@ -75,8 +75,12 @@ class KernelDocDirective(Directive):
> >> > elif 'doc' in self.options:
> >> > cmd += ['-function', str(self.options.get('doc'))]
> >> > elif 'functions' in self.options:
> >> > - for f in str(self.options.get('functions')).split():
> >> > - cmd += ['-function', f]
> >> > + functions = self.options.get('functions').split()
> >>
> >> Does .split() get upset if there's no argument? Or do you get an empty
> >> string if there are no options? I forget.
> >
> > "".split() gives an empty list.
>
> I tried to say, does self.options.get('functions') return an empty
> string or None if there are no options?
It returns an empty string.
> BR,
> Jani.
>
>
> >
> >> BR,
> >> Jani.
> >>
> >> > + if functions:
> >> > + for f in functions:
> >> > + cmd += ['-function', f]
> >> > + else:
> >> > + cmd += ['-no-doc-sections']
> >> >
> >> > for pattern in export_file_patterns:
> >> > for f in glob.glob(env.config.kerneldoc_srctree + '/' + pattern):
> >> >
> >> >> The usage in patch 2/2 would turn into:
> >> >>
> >> >> .. kernel-doc:: include/linux/idr.h
> >> >> :functions:
> >> >>
> >> >> which I think is much better overall in the rst source, complementing
> >> >> the places where you use :doc:.
> >> >>
> >> >> BR,
> >> >> Jani.
> >> >>
> >> >> > }
> >> >> > has_content = False
> >> >> >
> >> >> > @@ -77,6 +78,8 @@ class KernelDocDirective(Directive):
> >> >> > elif 'functions' in self.options:
> >> >> > for f in str(self.options.get('functions')).split():
> >> >> > cmd += ['-function', f]
> >> >> > + elif 'nodocs' in self.options:
> >> >> > + cmd += ['-no-doc-sections']
> >> >> >
> >> >> > for pattern in export_file_patterns:
> >> >> > for f in glob.glob(env.config.kerneldoc_srctree + '/' + pattern):
> >> >>
> >> >> --
> >> >> Jani Nikula, Intel Open Source Graphics Center
> >> >>
> >>
> >> --
> >> Jani Nikula, Intel Open Source Graphics Center
> >>
>
> --
> Jani Nikula, Intel Open Source Graphics Center
>
--
Sincerely yours,
Mike.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/3] locking: WW mutex cleanup
From: Peter Zijlstra @ 2018-06-19 11:02 UTC (permalink / raw)
To: Thomas Hellstrom
Cc: dri-devel, linux-kernel, linux-graphics-maintainer, pv-drivers,
Ingo Molnar, Jonathan Corbet, Gustavo Padovan, Maarten Lankhorst,
Sean Paul, David Airlie, Davidlohr Bueso, Paul E. McKenney,
Josh Triplett, Thomas Gleixner, Kate Stewart, Philippe Ombredanne,
Greg Kroah-Hartman, linux-doc, linux-media, linaro-mm-sig
In-Reply-To: <8ea67e74-5ed4-2157-8e51-43f15047b3e4@vmware.com>
On Tue, Jun 19, 2018 at 12:44:52PM +0200, Thomas Hellstrom wrote:
> On 06/19/2018 11:44 AM, Peter Zijlstra wrote:
> > On Tue, Jun 19, 2018 at 10:24:43AM +0200, Thomas Hellstrom wrote:
> > > From: Peter Ziljstra <peterz@infradead.org>
> > >
> > > Make the WW mutex code more readable by adding comments, splitting up
> > > functions and pointing out that we're actually using the Wait-Die
> > > algorithm.
> > >
> > > Cc: Ingo Molnar <mingo@redhat.com>
> > > Cc: Jonathan Corbet <corbet@lwn.net>
> > > Cc: Gustavo Padovan <gustavo@padovan.org>
> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Cc: Sean Paul <seanpaul@chromium.org>
> > > Cc: David Airlie <airlied@linux.ie>
> > > Cc: Davidlohr Bueso <dave@stgolabs.net>
> > > Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> > > Cc: Josh Triplett <josh@joshtriplett.org>
> > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > Cc: Kate Stewart <kstewart@linuxfoundation.org>
> > > Cc: Philippe Ombredanne <pombredanne@nexb.com>
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: linux-doc@vger.kernel.org
> > > Cc: linux-media@vger.kernel.org
> > > Cc: linaro-mm-sig@lists.linaro.org
> > > Co-authored-by: Thomas Hellstrom <thellstrom@vmware.com>
> > > Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
> > > ---
> > > Documentation/locking/ww-mutex-design.txt | 12 +-
> > > include/linux/ww_mutex.h | 28 ++---
> > > kernel/locking/mutex.c | 202 ++++++++++++++++++------------
> > > 3 files changed, 145 insertions(+), 97 deletions(-)
> > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>
> Hi Peter,
>
> Do you want to add a SOB, since you're the main author?
Sure, here goes:
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/3] locking: WW mutex cleanup
From: Thomas Hellstrom @ 2018-06-19 10:44 UTC (permalink / raw)
To: Peter Zijlstra
Cc: dri-devel, linux-kernel, linux-graphics-maintainer, pv-drivers,
Ingo Molnar, Jonathan Corbet, Gustavo Padovan, Maarten Lankhorst,
Sean Paul, David Airlie, Davidlohr Bueso, Paul E. McKenney,
Josh Triplett, Thomas Gleixner, Kate Stewart, Philippe Ombredanne,
Greg Kroah-Hartman, linux-doc, linux-media, linaro-mm-sig
In-Reply-To: <20180619094409.GK2458@hirez.programming.kicks-ass.net>
On 06/19/2018 11:44 AM, Peter Zijlstra wrote:
> On Tue, Jun 19, 2018 at 10:24:43AM +0200, Thomas Hellstrom wrote:
>> From: Peter Ziljstra <peterz@infradead.org>
>>
>> Make the WW mutex code more readable by adding comments, splitting up
>> functions and pointing out that we're actually using the Wait-Die
>> algorithm.
>>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: Gustavo Padovan <gustavo@padovan.org>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Sean Paul <seanpaul@chromium.org>
>> Cc: David Airlie <airlied@linux.ie>
>> Cc: Davidlohr Bueso <dave@stgolabs.net>
>> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
>> Cc: Josh Triplett <josh@joshtriplett.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Kate Stewart <kstewart@linuxfoundation.org>
>> Cc: Philippe Ombredanne <pombredanne@nexb.com>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-media@vger.kernel.org
>> Cc: linaro-mm-sig@lists.linaro.org
>> Co-authored-by: Thomas Hellstrom <thellstrom@vmware.com>
>> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
>> ---
>> Documentation/locking/ww-mutex-design.txt | 12 +-
>> include/linux/ww_mutex.h | 28 ++---
>> kernel/locking/mutex.c | 202 ++++++++++++++++++------------
>> 3 files changed, 145 insertions(+), 97 deletions(-)
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Hi Peter,
Do you want to add a SOB, since you're the main author?
Thomas
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v10 0/9] cpuset: Enable cpuset controller in default hierarchy
From: Juri Lelli @ 2018-06-19 9:52 UTC (permalink / raw)
To: Waiman Long
Cc: Tejun Heo, Li Zefan, Johannes Weiner, Peter Zijlstra, Ingo Molnar,
cgroups, linux-kernel, linux-doc, kernel-team, pjt, luto,
Mike Galbraith, torvalds, Roman Gushchin, Patrick Bellasi
In-Reply-To: <1529295249-5207-1-git-send-email-longman@redhat.com>
Hi,
On 18/06/18 12:13, Waiman Long wrote:
> v10:
> - Remove the cpuset.sched.load_balance patch for now as it may not
> be that useful.
> - Break the large patch 2 into smaller patches to make them a bit
> easier to review.
> - Test and fix issues related to changing "cpuset.cpus" and cpu
> online/offline in a domain root.
> - Rename isolated_cpus to reserved_cpus as this cpumask holds CPUs
> reserved for child sched domains.
> - Rework the scheduling domain debug printing code in the last patch.
> - Document update to the newly moved
> Documentation/admin-guide/cgroup-v2.rst.
By removing the redundant 6/9 as advised, I could test that this is
working as expected (at least by me :): https://git.io/fURIC
I played a bit with creating and modifying domain root(s), both with
legacy and default hierarchies. Looks good to me, you can add
Tested-by: Juri Lelli <juri.lelli@redhat.com>
to the series.
Best,
- Juri
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 0/4] locking,drm: Fix ww mutex naming / algorithm inconsistency
From: Peter Zijlstra @ 2018-06-19 9:45 UTC (permalink / raw)
To: Thomas Hellstrom
Cc: dri-devel, linux-kernel, linux-graphics-maintainer, pv-drivers,
Ingo Molnar, Jonathan Corbet, Gustavo Padovan, Maarten Lankhorst,
Sean Paul, David Airlie, Davidlohr Bueso, Paul E. McKenney,
Josh Triplett, Thomas Gleixner, Kate Stewart, Philippe Ombredanne,
Greg Kroah-Hartman, linux-doc, linux-media, linaro-mm-sig
In-Reply-To: <20180619082445.11062-1-thellstrom@vmware.com>
I suspect you want this through the DRM tree? Ingo are you OK with that?
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 2/3] locking: Implement an algorithm choice for Wound-Wait mutexes
From: Peter Zijlstra @ 2018-06-19 9:45 UTC (permalink / raw)
To: Thomas Hellstrom
Cc: dri-devel, linux-kernel, linux-graphics-maintainer, pv-drivers,
Ingo Molnar, Jonathan Corbet, Gustavo Padovan, Maarten Lankhorst,
Sean Paul, David Airlie, Davidlohr Bueso, Paul E. McKenney,
Josh Triplett, Thomas Gleixner, Kate Stewart, Philippe Ombredanne,
Greg Kroah-Hartman, linux-doc, linux-media, linaro-mm-sig
In-Reply-To: <20180619082445.11062-3-thellstrom@vmware.com>
On Tue, Jun 19, 2018 at 10:24:44AM +0200, Thomas Hellstrom wrote:
> The current Wound-Wait mutex algorithm is actually not Wound-Wait but
> Wait-Die. Implement also Wound-Wait as a per-ww-class choice. Wound-Wait
> is, contrary to Wait-Die a preemptive algorithm and is known to generate
> fewer backoffs. Testing reveals that this is true if the
> number of simultaneous contending transactions is small.
> As the number of simultaneous contending threads increases, Wait-Wound
> becomes inferior to Wait-Die in terms of elapsed time.
> Possibly due to the larger number of held locks of sleeping transactions.
>
> Update documentation and callers.
>
> Timings using git://people.freedesktop.org/~thomash/ww_mutex_test
> tag patch-18-06-15
>
> Each thread runs 100000 batches of lock / unlock 800 ww mutexes randomly
> chosen out of 100000. Four core Intel x86_64:
>
> Algorithm #threads Rollbacks time
> Wound-Wait 4 ~100 ~17s.
> Wait-Die 4 ~150000 ~19s.
> Wound-Wait 16 ~360000 ~109s.
> Wait-Die 16 ~450000 ~82s.
>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Gustavo Padovan <gustavo@padovan.org>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Davidlohr Bueso <dave@stgolabs.net>
> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Cc: Josh Triplett <josh@joshtriplett.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Kate Stewart <kstewart@linuxfoundation.org>
> Cc: Philippe Ombredanne <pombredanne@nexb.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-doc@vger.kernel.org
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org
> Co-authored-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
>
> ---
> Documentation/locking/ww-mutex-design.txt | 57 +++++++++--
> drivers/dma-buf/reservation.c | 2 +-
> drivers/gpu/drm/drm_modeset_lock.c | 2 +-
> include/linux/ww_mutex.h | 17 ++-
> kernel/locking/locktorture.c | 2 +-
> kernel/locking/mutex.c | 165 +++++++++++++++++++++++++++---
> kernel/locking/test-ww_mutex.c | 2 +-
> lib/locking-selftest.c | 2 +-
> 8 files changed, 213 insertions(+), 36 deletions(-)
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/3] locking: WW mutex cleanup
From: Peter Zijlstra @ 2018-06-19 9:44 UTC (permalink / raw)
To: Thomas Hellstrom
Cc: dri-devel, linux-kernel, linux-graphics-maintainer, pv-drivers,
Ingo Molnar, Jonathan Corbet, Gustavo Padovan, Maarten Lankhorst,
Sean Paul, David Airlie, Davidlohr Bueso, Paul E. McKenney,
Josh Triplett, Thomas Gleixner, Kate Stewart, Philippe Ombredanne,
Greg Kroah-Hartman, linux-doc, linux-media, linaro-mm-sig
In-Reply-To: <20180619082445.11062-2-thellstrom@vmware.com>
On Tue, Jun 19, 2018 at 10:24:43AM +0200, Thomas Hellstrom wrote:
> From: Peter Ziljstra <peterz@infradead.org>
>
> Make the WW mutex code more readable by adding comments, splitting up
> functions and pointing out that we're actually using the Wait-Die
> algorithm.
>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Gustavo Padovan <gustavo@padovan.org>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Davidlohr Bueso <dave@stgolabs.net>
> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Cc: Josh Triplett <josh@joshtriplett.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Kate Stewart <kstewart@linuxfoundation.org>
> Cc: Philippe Ombredanne <pombredanne@nexb.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: linux-doc@vger.kernel.org
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org
> Co-authored-by: Thomas Hellstrom <thellstrom@vmware.com>
> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
> ---
> Documentation/locking/ww-mutex-design.txt | 12 +-
> include/linux/ww_mutex.h | 28 ++---
> kernel/locking/mutex.c | 202 ++++++++++++++++++------------
> 3 files changed, 145 insertions(+), 97 deletions(-)
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 00/10] Control Flow Enforcement - Part (3)
From: Balbir Singh @ 2018-06-19 8:52 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Yu-cheng Yu, LKML, linux-doc, Linux-MM, linux-arch, X86 ML,
H. Peter Anvin, Thomas Gleixner, Ingo Molnar, H. J. Lu,
Shanbhogue, Vedvyas, Ravi V. Shankar, Dave Hansen,
Jonathan Corbet, Oleg Nesterov, Arnd Bergmann, mike.kravetz
In-Reply-To: <CALCETrWCEjuM56J8dqXPR==MevJTYKan5dnAMFJaXzFMYr8Q_A@mail.gmail.com>
On Mon, 2018-06-18 at 14:44 -0700, Andy Lutomirski wrote:
> On Sat, Jun 16, 2018 at 8:16 PM Balbir Singh <bsingharora@gmail.com> wrote:
> >
> > On Thu, 2018-06-14 at 07:56 -0700, Yu-cheng Yu wrote:
> > > On Thu, 2018-06-14 at 11:07 +1000, Balbir Singh wrote:
> > > > On Tue, 2018-06-12 at 08:03 -0700, Yu-cheng Yu wrote:
> > > > > On Tue, 2018-06-12 at 20:56 +1000, Balbir Singh wrote:
> > > > > >
> > > > > > On 08/06/18 00:37, Yu-cheng Yu wrote:
> > > > > > > This series introduces CET - Shadow stack
> > > > > > >
> > > > > > > At the high level, shadow stack is:
> > > > > > >
> > > > > > > Allocated from a task's address space with vm_flags VM_SHSTK;
> > > > > > > Its PTEs must be read-only and dirty;
> > > > > > > Fixed sized, but the default size can be changed by sys admin.
> > > > > > >
> > > > > > > For a forked child, the shadow stack is duplicated when the next
> > > > > > > shadow stack access takes place.
> > > > > > >
> > > > > > > For a pthread child, a new shadow stack is allocated.
> > > > > > >
> > > > > > > The signal handler uses the same shadow stack as the main program.
> > > > > > >
> > > > > >
> > > > > > Even with sigaltstack()?
> > > > > >
> > > > >
> > > > > Yes.
> > > >
> > > > I am not convinced that it would work, as we switch stacks, oveflow might
> > > > be an issue. I also forgot to bring up setcontext(2), I presume those
> > > > will get new shadow stacks
> > >
> > > Do you mean signal stack/sigaltstack overflow or swapcontext in a signal
> > > handler?
> > >
> >
> > I meant any combination of that. If there is a user space threads implementation that uses sigaltstack for switching threads
> >
>
> Anyone who does that is nuts. The whole point of user space threads
> is speed, and signals are very slow. For userspace threads to work,
> we need an API to allocate new shadow stacks, and we need to use the
> extremely awkwardly defined RSTORSSP stuff to switch. (I assume this
> is possible on an ISA level. The docs are bad, and the mnemonics for
> the relevant instructions are nonsensical.)
The whole point was to ensure we don't break applications/code that work
today. I think as long as there is a shadow stack allocated corresponding
to the user space stack and we can Restore SSP as we switch things should be
fine.
Balbir Singh.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v4 0/4] locking,drm: Fix ww mutex naming / algorithm inconsistency
From: Thomas Hellstrom @ 2018-06-19 8:24 UTC (permalink / raw)
To: dri-devel, linux-kernel, peterz
Cc: linux-graphics-maintainer, pv-drivers, Ingo Molnar,
Jonathan Corbet, Gustavo Padovan, Maarten Lankhorst, Sean Paul,
David Airlie, Davidlohr Bueso, Paul E. McKenney, Josh Triplett,
Thomas Gleixner, Kate Stewart, Philippe Ombredanne,
Greg Kroah-Hartman, linux-doc, linux-media, linaro-mm-sig
This is a small fallout from a work to allow batching WW mutex locks and
unlocks.
Our Wound-Wait mutexes actually don't use the Wound-Wait algorithm but
the Wait-Die algorithm. One could perhaps rename those mutexes tree-wide to
"Wait-Die mutexes" or "Deadlock Avoidance mutexes". Another approach suggested
here is to implement also the "Wound-Wait" algorithm as a per-WW-class
choice, as it has advantages in some cases. See for example
http://www.mathcs.emory.edu/~cheung/Courses/554/Syllabus/8-recv+serial/deadlock-compare.html
Now Wound-Wait is a preemptive algorithm, and the preemption is implemented
using a lazy scheme: If a wounded transaction is about to go to sleep on
a contended WW mutex, we return -EDEADLK. That is sufficient for deadlock
prevention. Since with WW mutexes we also require the aborted transaction to
sleep waiting to lock the WW mutex it was aborted on, this choice also provides
a suitable WW mutex to sleep on. If we were to return -EDEADLK on the first
WW mutex lock after the transaction was wounded whether the WW mutex was
contended or not, the transaction might frequently be restarted without a wait,
which is far from optimal. Note also that with the lazy preemption scheme,
contrary to Wait-Die there will be no rollbacks on lock contention of locks
held by a transaction that has completed its locking sequence.
The modeset locks are then changed from Wait-Die to Wound-Wait since the
typical locking pattern of those locks very well matches the criterion for
a substantial reduction in the number of rollbacks. For reservation objects,
the benefit is more unclear at this point and they remain using Wait-Die.
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Gustavo Padovan <gustavo@padovan.org>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Sean Paul <seanpaul@chromium.org>
Cc: David Airlie <airlied@linux.ie>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Kate Stewart <kstewart@linuxfoundation.org>
Cc: Philippe Ombredanne <pombredanne@nexb.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-doc@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
v2:
Updated DEFINE_WW_CLASS() API, minor code- and comment fixes as
detailed in each patch.
v3:
Included cleanup patch from Peter Zijlstra. Documentation fixes and
a small correctness fix.
v4:
Reworked the correctness fix.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v4 2/3] locking: Implement an algorithm choice for Wound-Wait mutexes
From: Thomas Hellstrom @ 2018-06-19 8:24 UTC (permalink / raw)
To: dri-devel, linux-kernel, peterz
Cc: linux-graphics-maintainer, pv-drivers, Thomas Hellstrom,
Ingo Molnar, Jonathan Corbet, Gustavo Padovan, Maarten Lankhorst,
Sean Paul, David Airlie, Davidlohr Bueso, Paul E. McKenney,
Josh Triplett, Thomas Gleixner, Kate Stewart, Philippe Ombredanne,
Greg Kroah-Hartman, linux-doc, linux-media, linaro-mm-sig
In-Reply-To: <20180619082445.11062-1-thellstrom@vmware.com>
The current Wound-Wait mutex algorithm is actually not Wound-Wait but
Wait-Die. Implement also Wound-Wait as a per-ww-class choice. Wound-Wait
is, contrary to Wait-Die a preemptive algorithm and is known to generate
fewer backoffs. Testing reveals that this is true if the
number of simultaneous contending transactions is small.
As the number of simultaneous contending threads increases, Wait-Wound
becomes inferior to Wait-Die in terms of elapsed time.
Possibly due to the larger number of held locks of sleeping transactions.
Update documentation and callers.
Timings using git://people.freedesktop.org/~thomash/ww_mutex_test
tag patch-18-06-15
Each thread runs 100000 batches of lock / unlock 800 ww mutexes randomly
chosen out of 100000. Four core Intel x86_64:
Algorithm #threads Rollbacks time
Wound-Wait 4 ~100 ~17s.
Wait-Die 4 ~150000 ~19s.
Wound-Wait 16 ~360000 ~109s.
Wait-Die 16 ~450000 ~82s.
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Gustavo Padovan <gustavo@padovan.org>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Sean Paul <seanpaul@chromium.org>
Cc: David Airlie <airlied@linux.ie>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Kate Stewart <kstewart@linuxfoundation.org>
Cc: Philippe Ombredanne <pombredanne@nexb.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-doc@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
Co-authored-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
---
v2:
* Update API according to review comment by Greg Kroah-Hartman.
* Address review comments by Peter Zijlstra:
- Avoid _Bool in composites
- Fix typo
- Use __mutex_owner() where applicable
- Rely on built-in barriers for the main loop exit condition,
struct ww_acquire_ctx::wounded. Update code comments.
- Explain unlocked use of list_empty().
v3:
* Adapt to and incorporate cleanup by Peter Zijlstra
* Remove unlocked use of list_empty().
v4:
* Move code related to adding a waiter to the lock waiter list to a
separate function.
---
Documentation/locking/ww-mutex-design.txt | 57 +++++++++--
drivers/dma-buf/reservation.c | 2 +-
drivers/gpu/drm/drm_modeset_lock.c | 2 +-
include/linux/ww_mutex.h | 17 ++-
kernel/locking/locktorture.c | 2 +-
kernel/locking/mutex.c | 165 +++++++++++++++++++++++++++---
kernel/locking/test-ww_mutex.c | 2 +-
lib/locking-selftest.c | 2 +-
8 files changed, 213 insertions(+), 36 deletions(-)
diff --git a/Documentation/locking/ww-mutex-design.txt b/Documentation/locking/ww-mutex-design.txt
index 2fd7f2a2af21..f0ed7c30e695 100644
--- a/Documentation/locking/ww-mutex-design.txt
+++ b/Documentation/locking/ww-mutex-design.txt
@@ -1,4 +1,4 @@
-Wait/Wound Deadlock-Proof Mutex Design
+Wound/Wait Deadlock-Proof Mutex Design
======================================
Please read mutex-design.txt first, as it applies to wait/wound mutexes too.
@@ -32,10 +32,26 @@ the oldest task) wins, and the one with the higher reservation id (i.e. the
younger task) unlocks all of the buffers that it has already locked, and then
tries again.
-In the RDBMS literature this deadlock handling approach is called wait/die:
-The older tasks waits until it can acquire the contended lock. The younger tasks
-needs to back off and drop all the locks it is currently holding, i.e. the
-younger task dies.
+In the RDBMS literature, a reservation ticket is associated with a transaction.
+and the deadlock handling approach is called Wait-Die. The name is based on
+the actions of a locking thread when it encounters an already locked mutex.
+If the transaction holding the lock is younger, the locking transaction waits.
+If the transaction holding the lock is older, the locking transaction backs off
+and dies. Hence Wait-Die.
+There is also another algorithm called Wound-Wait:
+If the transaction holding the lock is younger, the locking transaction
+wounds the transaction holding the lock, requesting it to die.
+If the transaction holding the lock is older, it waits for the other
+transaction. Hence Wound-Wait.
+The two algorithms are both fair in that a transaction will eventually succeed.
+However, the Wound-Wait algorithm is typically stated to generate fewer backoffs
+compared to Wait-Die, but is, on the other hand, associated with more work than
+Wait-Die when recovering from a backoff. Wound-Wait is also a preemptive
+algorithm in that transactions are wounded by other transactions, and that
+requires a reliable way to pick up up the wounded condition and preempt the
+running transaction. Note that this is not the same as process preemption. A
+Wound-Wait transaction is considered preempted when it dies (returning
+-EDEADLK) following a wound.
Concepts
--------
@@ -47,10 +63,12 @@ Acquire context: To ensure eventual forward progress it is important the a task
trying to acquire locks doesn't grab a new reservation id, but keeps the one it
acquired when starting the lock acquisition. This ticket is stored in the
acquire context. Furthermore the acquire context keeps track of debugging state
-to catch w/w mutex interface abuse.
+to catch w/w mutex interface abuse. An acquire context is representing a
+transaction.
W/w class: In contrast to normal mutexes the lock class needs to be explicit for
-w/w mutexes, since it is required to initialize the acquire context.
+w/w mutexes, since it is required to initialize the acquire context. The lock
+class also specifies what algorithm to use, Wound-Wait or Wait-Die.
Furthermore there are three different class of w/w lock acquire functions:
@@ -90,6 +108,12 @@ provided.
Usage
-----
+The algorithm (Wait-Die vs Wound-Wait) is chosen by using either
+DEFINE_WW_CLASS() (Wound-Wait) or DEFINE_WD_CLASS() (Wait-Die)
+As a rough rule of thumb, use Wound-Wait iff you
+expect the number of simultaneous competing transactions to be typically small,
+and you want to reduce the number of rollbacks.
+
Three different ways to acquire locks within the same w/w class. Common
definitions for methods #1 and #2:
@@ -312,12 +336,23 @@ Design:
We maintain the following invariants for the wait list:
(1) Waiters with an acquire context are sorted by stamp order; waiters
without an acquire context are interspersed in FIFO order.
- (2) Among waiters with contexts, only the first one can have other locks
- acquired already (ctx->acquired > 0). Note that this waiter may come
- after other waiters without contexts in the list.
+ (2) For Wait-Die, among waiters with contexts, only the first one can have
+ other locks acquired already (ctx->acquired > 0). Note that this waiter
+ may come after other waiters without contexts in the list.
+
+ The Wound-Wait preemption is implemented with a lazy-preemption scheme:
+ The wounded status of the transaction is checked only when there is
+ contention for a new lock and hence a true chance of deadlock. In that
+ situation, if the transaction is wounded, it backs off, clears the
+ wounded status and retries. A great benefit of implementing preemption in
+ this way is that the wounded transaction can identify a contending lock to
+ wait for before restarting the transaction. Just blindly restarting the
+ transaction would likely make the transaction end up in a situation where
+ it would have to back off again.
In general, not much contention is expected. The locks are typically used to
- serialize access to resources for devices.
+ serialize access to resources for devices, and optimization focus should
+ therefore be directed towards the uncontended cases.
Lockdep:
Special care has been taken to warn for as many cases of api abuse
diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c
index 314eb1071cce..20bf90f4ee63 100644
--- a/drivers/dma-buf/reservation.c
+++ b/drivers/dma-buf/reservation.c
@@ -46,7 +46,7 @@
* write-side updates.
*/
-DEFINE_WW_CLASS(reservation_ww_class);
+DEFINE_WD_CLASS(reservation_ww_class);
EXPORT_SYMBOL(reservation_ww_class);
struct lock_class_key reservation_seqcount_class;
diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
index 8a5100685875..638be2eb67b4 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -70,7 +70,7 @@
* lists and lookup data structures.
*/
-static DEFINE_WW_CLASS(crtc_ww_class);
+static DEFINE_WD_CLASS(crtc_ww_class);
/**
* drm_modeset_lock_all - take all modeset locks
diff --git a/include/linux/ww_mutex.h b/include/linux/ww_mutex.h
index f82fce2229c8..3af7c0e03be5 100644
--- a/include/linux/ww_mutex.h
+++ b/include/linux/ww_mutex.h
@@ -8,6 +8,8 @@
*
* Wait/Die implementation:
* Copyright (C) 2013 Canonical Ltd.
+ * Choice of algorithm:
+ * Copyright (C) 2018 WMWare Inc.
*
* This file contains the main data structure and API definitions.
*/
@@ -23,12 +25,15 @@ struct ww_class {
struct lock_class_key mutex_key;
const char *acquire_name;
const char *mutex_name;
+ unsigned int is_wait_die;
};
struct ww_acquire_ctx {
struct task_struct *task;
unsigned long stamp;
unsigned int acquired;
+ unsigned short wounded;
+ unsigned short is_wait_die;
#ifdef CONFIG_DEBUG_MUTEXES
unsigned int done_acquire;
struct ww_class *ww_class;
@@ -58,17 +63,21 @@ struct ww_mutex {
# define __WW_CLASS_MUTEX_INITIALIZER(lockname, class)
#endif
-#define __WW_CLASS_INITIALIZER(ww_class) \
+#define __WW_CLASS_INITIALIZER(ww_class, _is_wait_die) \
{ .stamp = ATOMIC_LONG_INIT(0) \
, .acquire_name = #ww_class "_acquire" \
- , .mutex_name = #ww_class "_mutex" }
+ , .mutex_name = #ww_class "_mutex" \
+ , .is_wait_die = _is_wait_die }
#define __WW_MUTEX_INITIALIZER(lockname, class) \
{ .base = __MUTEX_INITIALIZER(lockname.base) \
__WW_CLASS_MUTEX_INITIALIZER(lockname, class) }
+#define DEFINE_WD_CLASS(classname) \
+ struct ww_class classname = __WW_CLASS_INITIALIZER(classname, 1)
+
#define DEFINE_WW_CLASS(classname) \
- struct ww_class classname = __WW_CLASS_INITIALIZER(classname)
+ struct ww_class classname = __WW_CLASS_INITIALIZER(classname, 0)
#define DEFINE_WW_MUTEX(mutexname, ww_class) \
struct ww_mutex mutexname = __WW_MUTEX_INITIALIZER(mutexname, ww_class)
@@ -123,6 +132,8 @@ static inline void ww_acquire_init(struct ww_acquire_ctx *ctx,
ctx->task = current;
ctx->stamp = atomic_long_inc_return_relaxed(&ww_class->stamp);
ctx->acquired = 0;
+ ctx->wounded = false;
+ ctx->is_wait_die = ww_class->is_wait_die;
#ifdef CONFIG_DEBUG_MUTEXES
ctx->ww_class = ww_class;
ctx->done_acquire = 0;
diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index 6850ffd69125..907e0325892c 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -365,7 +365,7 @@ static struct lock_torture_ops mutex_lock_ops = {
};
#include <linux/ww_mutex.h>
-static DEFINE_WW_CLASS(torture_ww_class);
+static DEFINE_WD_CLASS(torture_ww_class);
static DEFINE_WW_MUTEX(torture_ww_mutex_0, &torture_ww_class);
static DEFINE_WW_MUTEX(torture_ww_mutex_1, &torture_ww_class);
static DEFINE_WW_MUTEX(torture_ww_mutex_2, &torture_ww_class);
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index 412b4fc08235..8ca83a5e3d24 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -172,6 +172,21 @@ static inline bool __mutex_waiter_is_first(struct mutex *lock, struct mutex_wait
return list_first_entry(&lock->wait_list, struct mutex_waiter, list) == waiter;
}
+/*
+ * Add @waiter to a given location in the lock wait_list and set the
+ * FLAG_WAITERS flag if it's the first waiter.
+ */
+static void __sched
+__mutex_add_waiter(struct mutex *lock, struct mutex_waiter *waiter,
+ struct list_head *list)
+{
+ debug_mutex_add_waiter(lock, waiter, current);
+
+ list_add_tail(&waiter->list, list);
+ if (__mutex_waiter_is_first(lock, waiter))
+ __mutex_set_flag(lock, MUTEX_FLAG_WAITERS);
+}
+
/*
* Give up ownership to a specific task, when @task = NULL, this is equivalent
* to a regular unlock. Sets PICKUP on a handoff, clears HANDOF, preserves
@@ -248,6 +263,11 @@ EXPORT_SYMBOL(mutex_lock);
* The newer transactions are killed when:
* It (the new transaction) makes a request for a lock being held
* by an older transaction.
+ *
+ * Wound-Wait:
+ * The newer transactions are wounded when:
+ * An older transaction makes a request for a lock being held by
+ * the newer transaction.
*/
/*
@@ -319,6 +339,9 @@ static bool __sched
__ww_mutex_die(struct mutex *lock, struct mutex_waiter *waiter,
struct ww_acquire_ctx *ww_ctx)
{
+ if (!ww_ctx->is_wait_die)
+ return false;
+
if (waiter->ww_ctx->acquired > 0 &&
__ww_ctx_stamp_after(waiter->ww_ctx, ww_ctx)) {
debug_mutex_wake_waiter(lock, waiter);
@@ -328,13 +351,65 @@ __ww_mutex_die(struct mutex *lock, struct mutex_waiter *waiter,
return true;
}
+/*
+ * Wound-Wait; wound a younger @hold_ctx if it holds the lock.
+ *
+ * Wound the lock holder if there are waiters with older transactions than
+ * the lock holders. Even if multiple waiters may wound the lock holder,
+ * it's sufficient that only one does.
+ */
+static bool __ww_mutex_wound(struct mutex *lock,
+ struct ww_acquire_ctx *ww_ctx,
+ struct ww_acquire_ctx *hold_ctx)
+{
+ struct task_struct *owner = __mutex_owner(lock);
+
+ lockdep_assert_held(&lock->wait_lock);
+
+ /*
+ * Possible through __ww_mutex_add_waiter() when we race with
+ * ww_mutex_set_context_fastpath(). In that case we'll get here again
+ * through __ww_mutex_check_waiters().
+ */
+ if (!hold_ctx)
+ return false;
+
+ /*
+ * Can have !owner because of __mutex_unlock_slowpath(), but if owner,
+ * it cannot go away because we'll have FLAG_WAITERS set and hold
+ * wait_lock.
+ */
+ if (!owner)
+ return false;
+
+ if (ww_ctx->acquired > 0 && __ww_ctx_stamp_after(hold_ctx, ww_ctx)) {
+ hold_ctx->wounded = 1;
+
+ /*
+ * wake_up_process() paired with set_current_state()
+ * inserts sufficient barriers to make sure @owner either sees
+ * it's wounded in __ww_mutex_lock_check_stamp() or has a
+ * wakeup pending to re-read the wounded state.
+ */
+ if (owner != current)
+ wake_up_process(owner);
+
+ return true;
+ }
+
+ return false;
+}
+
/*
* We just acquired @lock under @ww_ctx, if there are later contexts waiting
- * behind us on the wait-list, check if they need to die.
+ * behind us on the wait-list, check if they need to die, or wound us.
*
* See __ww_mutex_add_waiter() for the list-order construction; basically the
* list is ordered by stamp, smallest (oldest) first.
*
+ * This relies on never mixing wait-die/wound-wait on the same wait-list;
+ * which is currently ensured by that being a ww_class property.
+ *
* The current task must not be on the wait list.
*/
static void __sched
@@ -348,7 +423,8 @@ __ww_mutex_check_waiters(struct mutex *lock, struct ww_acquire_ctx *ww_ctx)
if (!cur->ww_ctx)
continue;
- if (__ww_mutex_die(lock, cur, ww_ctx))
+ if (__ww_mutex_die(lock, cur, ww_ctx) ||
+ __ww_mutex_wound(lock, cur->ww_ctx, ww_ctx))
break;
}
}
@@ -369,17 +445,23 @@ ww_mutex_set_context_fastpath(struct ww_mutex *lock, struct ww_acquire_ctx *ctx)
* and keep spinning, or it will acquire wait_lock, add itself
* to waiter list and sleep.
*/
- smp_mb(); /* ^^^ */
+ smp_mb(); /* See comments above and below. */
/*
- * Check if lock is contended, if not there is nobody to wake up
+ * [W] ww->ctx = ctx [W] MUTEX_FLAG_WAITERS
+ * MB MB
+ * [R] MUTEX_FLAG_WAITERS [R] ww->ctx
+ *
+ * The memory barrier above pairs with the memory barrier in
+ * __ww_mutex_add_waiter() and makes sure we either observe ww->ctx
+ * and/or !empty list.
*/
if (likely(!(atomic_long_read(&lock->base.owner) & MUTEX_FLAG_WAITERS)))
return;
/*
* Uh oh, we raced in fastpath, check if any of the waiters need to
- * die.
+ * die or wound us.
*/
spin_lock(&lock->base.wait_lock);
__ww_mutex_check_waiters(&lock->base, ctx);
@@ -681,7 +763,9 @@ __ww_mutex_kill(struct mutex *lock, struct ww_acquire_ctx *ww_ctx)
/*
- * Check whether we need to kill the transaction for the current lock acquire.
+ * Check the wound condition for the current lock acquire.
+ *
+ * Wound-Wait: If we're wounded, kill ourself.
*
* Wait-Die: If we're trying to acquire a lock already held by an older
* context, kill ourselves.
@@ -700,6 +784,13 @@ __ww_mutex_check_kill(struct mutex *lock, struct mutex_waiter *waiter,
if (ctx->acquired == 0)
return 0;
+ if (!ctx->is_wait_die) {
+ if (ctx->wounded)
+ return __ww_mutex_kill(lock, ctx);
+
+ return 0;
+ }
+
if (hold_ctx && __ww_ctx_stamp_after(ctx, hold_ctx))
return __ww_mutex_kill(lock, ctx);
@@ -726,7 +817,8 @@ __ww_mutex_check_kill(struct mutex *lock, struct mutex_waiter *waiter,
* Waiters without context are interspersed in FIFO order.
*
* Furthermore, for Wait-Die kill ourself immediately when possible (there are
- * older contexts already waiting) to avoid unnecessary waiting.
+ * older contexts already waiting) to avoid unnecessary waiting and for
+ * Wound-Wait ensure we wound the owning context when it is younger.
*/
static inline int __sched
__ww_mutex_add_waiter(struct mutex_waiter *waiter,
@@ -735,16 +827,21 @@ __ww_mutex_add_waiter(struct mutex_waiter *waiter,
{
struct mutex_waiter *cur;
struct list_head *pos;
+ bool is_wait_die;
if (!ww_ctx) {
- list_add_tail(&waiter->list, &lock->wait_list);
+ __mutex_add_waiter(lock, waiter, &lock->wait_list);
return 0;
}
+ is_wait_die = ww_ctx->is_wait_die;
+
/*
* Add the waiter before the first waiter with a higher stamp.
* Waiters without a context are skipped to avoid starving
- * them. Wait-Die waiters may die here.
+ * them. Wait-Die waiters may die here. Wound-Wait waiters
+ * never die here, but they are sorted in stamp order and
+ * may wound the lock holder.
*/
pos = &lock->wait_list;
list_for_each_entry_reverse(cur, &lock->wait_list, list) {
@@ -757,10 +854,12 @@ __ww_mutex_add_waiter(struct mutex_waiter *waiter,
* is no point in queueing behind it, as we'd have to
* die the moment it would acquire the lock.
*/
- int ret = __ww_mutex_kill(lock, ww_ctx);
+ if (is_wait_die) {
+ int ret = __ww_mutex_kill(lock, ww_ctx);
- if (ret)
- return ret;
+ if (ret)
+ return ret;
+ }
break;
}
@@ -771,7 +870,23 @@ __ww_mutex_add_waiter(struct mutex_waiter *waiter,
__ww_mutex_die(lock, cur, ww_ctx);
}
- list_add_tail(&waiter->list, pos);
+ __mutex_add_waiter(lock, waiter, pos);
+
+ /*
+ * Wound-Wait: if we're blocking on a mutex owned by a younger context,
+ * wound that such that we might proceed.
+ */
+ if (!is_wait_die) {
+ struct ww_mutex *ww = container_of(lock, struct ww_mutex, base);
+
+ /*
+ * See ww_mutex_set_context_fastpath(). Orders setting
+ * MUTEX_FLAG_WAITERS vs the ww->ctx load,
+ * such that either we or the fastpath will wound @ww->ctx.
+ */
+ smp_mb();
+ __ww_mutex_wound(lock, ww_ctx, ww->ctx);
+ }
return 0;
}
@@ -795,6 +910,14 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
if (use_ww_ctx && ww_ctx) {
if (unlikely(ww_ctx == READ_ONCE(ww->ctx)))
return -EALREADY;
+
+ /*
+ * Reset the wounded flag after a kill. No other process can
+ * race and wound us here since they can't have a valid owner
+ * pointer if we don't have any locks held.
+ */
+ if (ww_ctx->acquired == 0)
+ ww_ctx->wounded = 0;
}
preempt_disable();
@@ -828,7 +951,8 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
if (!use_ww_ctx) {
/* add waiting tasks to the end of the waitqueue (FIFO): */
- list_add_tail(&waiter.list, &lock->wait_list);
+ __mutex_add_waiter(lock, &waiter, &lock->wait_list);
+
#ifdef CONFIG_DEBUG_MUTEXES
waiter.ww_ctx = MUTEX_POISON_WW_CTX;
@@ -847,9 +971,6 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
waiter.task = current;
- if (__mutex_waiter_is_first(lock, &waiter))
- __mutex_set_flag(lock, MUTEX_FLAG_WAITERS);
-
set_current_state(state);
for (;;) {
/*
@@ -906,6 +1027,16 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
acquired:
__set_current_state(TASK_RUNNING);
+ if (use_ww_ctx && ww_ctx) {
+ /*
+ * Wound-Wait; we stole the lock (!first_waiter), check the
+ * waiters as anyone might want to wound us.
+ */
+ if (!ww_ctx->is_wait_die &&
+ !__mutex_waiter_is_first(lock, &waiter))
+ __ww_mutex_check_waiters(lock, ww_ctx);
+ }
+
mutex_remove_waiter(lock, &waiter, current);
if (likely(list_empty(&lock->wait_list)))
__mutex_clear_flag(lock, MUTEX_FLAGS);
diff --git a/kernel/locking/test-ww_mutex.c b/kernel/locking/test-ww_mutex.c
index 0e4cd64ad2c0..5b915b370d5a 100644
--- a/kernel/locking/test-ww_mutex.c
+++ b/kernel/locking/test-ww_mutex.c
@@ -26,7 +26,7 @@
#include <linux/slab.h>
#include <linux/ww_mutex.h>
-static DEFINE_WW_CLASS(ww_class);
+static DEFINE_WD_CLASS(ww_class);
struct workqueue_struct *wq;
struct test_mutex {
diff --git a/lib/locking-selftest.c b/lib/locking-selftest.c
index b5c1293ce147..1e1bbf171eca 100644
--- a/lib/locking-selftest.c
+++ b/lib/locking-selftest.c
@@ -29,7 +29,7 @@
*/
static unsigned int debug_locks_verbose;
-static DEFINE_WW_CLASS(ww_lockdep);
+static DEFINE_WD_CLASS(ww_lockdep);
static int __init setup_debug_locks_verbose(char *str)
{
--
2.14.3
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 1/3] locking: WW mutex cleanup
From: Thomas Hellstrom @ 2018-06-19 8:24 UTC (permalink / raw)
To: dri-devel, linux-kernel, peterz
Cc: linux-graphics-maintainer, pv-drivers, Ingo Molnar,
Jonathan Corbet, Gustavo Padovan, Maarten Lankhorst, Sean Paul,
David Airlie, Davidlohr Bueso, Paul E. McKenney, Josh Triplett,
Thomas Gleixner, Kate Stewart, Philippe Ombredanne,
Greg Kroah-Hartman, linux-doc, linux-media, linaro-mm-sig,
Thomas Hellstrom
In-Reply-To: <20180619082445.11062-1-thellstrom@vmware.com>
From: Peter Ziljstra <peterz@infradead.org>
Make the WW mutex code more readable by adding comments, splitting up
functions and pointing out that we're actually using the Wait-Die
algorithm.
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Gustavo Padovan <gustavo@padovan.org>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Sean Paul <seanpaul@chromium.org>
Cc: David Airlie <airlied@linux.ie>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Kate Stewart <kstewart@linuxfoundation.org>
Cc: Philippe Ombredanne <pombredanne@nexb.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-doc@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
Co-authored-by: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
---
Documentation/locking/ww-mutex-design.txt | 12 +-
include/linux/ww_mutex.h | 28 ++---
kernel/locking/mutex.c | 202 ++++++++++++++++++------------
3 files changed, 145 insertions(+), 97 deletions(-)
diff --git a/Documentation/locking/ww-mutex-design.txt b/Documentation/locking/ww-mutex-design.txt
index 34c3a1b50b9a..2fd7f2a2af21 100644
--- a/Documentation/locking/ww-mutex-design.txt
+++ b/Documentation/locking/ww-mutex-design.txt
@@ -32,10 +32,10 @@ the oldest task) wins, and the one with the higher reservation id (i.e. the
younger task) unlocks all of the buffers that it has already locked, and then
tries again.
-In the RDBMS literature this deadlock handling approach is called wait/wound:
+In the RDBMS literature this deadlock handling approach is called wait/die:
The older tasks waits until it can acquire the contended lock. The younger tasks
needs to back off and drop all the locks it is currently holding, i.e. the
-younger task is wounded.
+younger task dies.
Concepts
--------
@@ -56,9 +56,9 @@ Furthermore there are three different class of w/w lock acquire functions:
* Normal lock acquisition with a context, using ww_mutex_lock.
-* Slowpath lock acquisition on the contending lock, used by the wounded task
- after having dropped all already acquired locks. These functions have the
- _slow postfix.
+* Slowpath lock acquisition on the contending lock, used by the task that just
+ killed its transaction after having dropped all already acquired locks.
+ These functions have the _slow postfix.
From a simple semantics point-of-view the _slow functions are not strictly
required, since simply calling the normal ww_mutex_lock functions on the
@@ -220,7 +220,7 @@ mutexes are a natural fit for such a case for two reasons:
Note that this approach differs in two important ways from the above methods:
- Since the list of objects is dynamically constructed (and might very well be
- different when retrying due to hitting the -EDEADLK wound condition) there's
+ different when retrying due to hitting the -EDEADLK die condition) there's
no need to keep any object on a persistent list when it's not locked. We can
therefore move the list_head into the object itself.
- On the other hand the dynamic object list construction also means that the -EALREADY return
diff --git a/include/linux/ww_mutex.h b/include/linux/ww_mutex.h
index 39fda195bf78..f82fce2229c8 100644
--- a/include/linux/ww_mutex.h
+++ b/include/linux/ww_mutex.h
@@ -6,7 +6,7 @@
*
* Copyright (C) 2004, 2005, 2006 Red Hat, Inc., Ingo Molnar <mingo@redhat.com>
*
- * Wound/wait implementation:
+ * Wait/Die implementation:
* Copyright (C) 2013 Canonical Ltd.
*
* This file contains the main data structure and API definitions.
@@ -28,9 +28,9 @@ struct ww_class {
struct ww_acquire_ctx {
struct task_struct *task;
unsigned long stamp;
- unsigned acquired;
+ unsigned int acquired;
#ifdef CONFIG_DEBUG_MUTEXES
- unsigned done_acquire;
+ unsigned int done_acquire;
struct ww_class *ww_class;
struct ww_mutex *contending_lock;
#endif
@@ -38,8 +38,8 @@ struct ww_acquire_ctx {
struct lockdep_map dep_map;
#endif
#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH
- unsigned deadlock_inject_interval;
- unsigned deadlock_inject_countdown;
+ unsigned int deadlock_inject_interval;
+ unsigned int deadlock_inject_countdown;
#endif
};
@@ -102,7 +102,7 @@ static inline void ww_mutex_init(struct ww_mutex *lock,
*
* Context-based w/w mutex acquiring can be done in any order whatsoever within
* a given lock class. Deadlocks will be detected and handled with the
- * wait/wound logic.
+ * wait/die logic.
*
* Mixing of context-based w/w mutex acquiring and single w/w mutex locking can
* result in undetected deadlocks and is so forbidden. Mixing different contexts
@@ -195,13 +195,13 @@ static inline void ww_acquire_fini(struct ww_acquire_ctx *ctx)
* Lock the w/w mutex exclusively for this task.
*
* Deadlocks within a given w/w class of locks are detected and handled with the
- * wait/wound algorithm. If the lock isn't immediately avaiable this function
+ * wait/die algorithm. If the lock isn't immediately available this function
* will either sleep until it is (wait case). Or it selects the current context
- * for backing off by returning -EDEADLK (wound case). Trying to acquire the
+ * for backing off by returning -EDEADLK (die case). Trying to acquire the
* same lock with the same context twice is also detected and signalled by
* returning -EALREADY. Returns 0 if the mutex was successfully acquired.
*
- * In the wound case the caller must release all currently held w/w mutexes for
+ * In the die case the caller must release all currently held w/w mutexes for
* the given context and then wait for this contending lock to be available by
* calling ww_mutex_lock_slow. Alternatively callers can opt to not acquire this
* lock and proceed with trying to acquire further w/w mutexes (e.g. when
@@ -226,14 +226,14 @@ extern int /* __must_check */ ww_mutex_lock(struct ww_mutex *lock, struct ww_acq
* Lock the w/w mutex exclusively for this task.
*
* Deadlocks within a given w/w class of locks are detected and handled with the
- * wait/wound algorithm. If the lock isn't immediately avaiable this function
+ * wait/die algorithm. If the lock isn't immediately available this function
* will either sleep until it is (wait case). Or it selects the current context
- * for backing off by returning -EDEADLK (wound case). Trying to acquire the
+ * for backing off by returning -EDEADLK (die case). Trying to acquire the
* same lock with the same context twice is also detected and signalled by
* returning -EALREADY. Returns 0 if the mutex was successfully acquired. If a
* signal arrives while waiting for the lock then this function returns -EINTR.
*
- * In the wound case the caller must release all currently held w/w mutexes for
+ * In the die case the caller must release all currently held w/w mutexes for
* the given context and then wait for this contending lock to be available by
* calling ww_mutex_lock_slow_interruptible. Alternatively callers can opt to
* not acquire this lock and proceed with trying to acquire further w/w mutexes
@@ -256,7 +256,7 @@ extern int __must_check ww_mutex_lock_interruptible(struct ww_mutex *lock,
* @lock: the mutex to be acquired
* @ctx: w/w acquire context
*
- * Acquires a w/w mutex with the given context after a wound case. This function
+ * Acquires a w/w mutex with the given context after a die case. This function
* will sleep until the lock becomes available.
*
* The caller must have released all w/w mutexes already acquired with the
@@ -290,7 +290,7 @@ ww_mutex_lock_slow(struct ww_mutex *lock, struct ww_acquire_ctx *ctx)
* @lock: the mutex to be acquired
* @ctx: w/w acquire context
*
- * Acquires a w/w mutex with the given context after a wound case. This function
+ * Acquires a w/w mutex with the given context after a die case. This function
* will sleep until the lock becomes available and returns 0 when the lock has
* been acquired. If a signal arrives while waiting for the lock then this
* function returns -EINTR.
diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
index 2048359f33d2..412b4fc08235 100644
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -243,6 +243,17 @@ void __sched mutex_lock(struct mutex *lock)
EXPORT_SYMBOL(mutex_lock);
#endif
+/*
+ * Wait-Die:
+ * The newer transactions are killed when:
+ * It (the new transaction) makes a request for a lock being held
+ * by an older transaction.
+ */
+
+/*
+ * Associate the ww_mutex @ww with the context @ww_ctx under which we acquired
+ * it.
+ */
static __always_inline void
ww_mutex_lock_acquired(struct ww_mutex *ww, struct ww_acquire_ctx *ww_ctx)
{
@@ -281,26 +292,53 @@ ww_mutex_lock_acquired(struct ww_mutex *ww, struct ww_acquire_ctx *ww_ctx)
DEBUG_LOCKS_WARN_ON(ww_ctx->ww_class != ww->ww_class);
#endif
ww_ctx->acquired++;
+ ww->ctx = ww_ctx;
}
+/*
+ * Determine if context @a is 'after' context @b. IOW, @a is a younger
+ * transaction than @b and depending on algorithm either needs to wait for
+ * @b or die.
+ */
static inline bool __sched
__ww_ctx_stamp_after(struct ww_acquire_ctx *a, struct ww_acquire_ctx *b)
{
- return a->stamp - b->stamp <= LONG_MAX &&
- (a->stamp != b->stamp || a > b);
+
+ return (signed long)(a->stamp - b->stamp) > 0;
+}
+
+/*
+ * Wait-Die; wake a younger waiter context (when locks held) such that it can
+ * die.
+ *
+ * Among waiters with context, only the first one can have other locks acquired
+ * already (ctx->acquired > 0), because __ww_mutex_add_waiter() and
+ * __ww_mutex_check_kill() wake any but the earliest context.
+ */
+static bool __sched
+__ww_mutex_die(struct mutex *lock, struct mutex_waiter *waiter,
+ struct ww_acquire_ctx *ww_ctx)
+{
+ if (waiter->ww_ctx->acquired > 0 &&
+ __ww_ctx_stamp_after(waiter->ww_ctx, ww_ctx)) {
+ debug_mutex_wake_waiter(lock, waiter);
+ wake_up_process(waiter->task);
+ }
+
+ return true;
}
/*
- * Wake up any waiters that may have to back off when the lock is held by the
- * given context.
+ * We just acquired @lock under @ww_ctx, if there are later contexts waiting
+ * behind us on the wait-list, check if they need to die.
*
- * Due to the invariants on the wait list, this can only affect the first
- * waiter with a context.
+ * See __ww_mutex_add_waiter() for the list-order construction; basically the
+ * list is ordered by stamp, smallest (oldest) first.
*
* The current task must not be on the wait list.
*/
static void __sched
-__ww_mutex_wakeup_for_backoff(struct mutex *lock, struct ww_acquire_ctx *ww_ctx)
+__ww_mutex_check_waiters(struct mutex *lock, struct ww_acquire_ctx *ww_ctx)
{
struct mutex_waiter *cur;
@@ -310,30 +348,23 @@ __ww_mutex_wakeup_for_backoff(struct mutex *lock, struct ww_acquire_ctx *ww_ctx)
if (!cur->ww_ctx)
continue;
- if (cur->ww_ctx->acquired > 0 &&
- __ww_ctx_stamp_after(cur->ww_ctx, ww_ctx)) {
- debug_mutex_wake_waiter(lock, cur);
- wake_up_process(cur->task);
- }
-
- break;
+ if (__ww_mutex_die(lock, cur, ww_ctx))
+ break;
}
}
/*
- * After acquiring lock with fastpath or when we lost out in contested
- * slowpath, set ctx and wake up any waiters so they can recheck.
+ * After acquiring lock with fastpath, where we do not hold wait_lock, set ctx
+ * and wake up any waiters so they can recheck.
*/
static __always_inline void
ww_mutex_set_context_fastpath(struct ww_mutex *lock, struct ww_acquire_ctx *ctx)
{
ww_mutex_lock_acquired(lock, ctx);
- lock->ctx = ctx;
-
/*
* The lock->ctx update should be visible on all cores before
- * the atomic read is done, otherwise contended waiters might be
+ * the WAITERS check is done, otherwise contended waiters might be
* missed. The contended waiters will either see ww_ctx == NULL
* and keep spinning, or it will acquire wait_lock, add itself
* to waiter list and sleep.
@@ -347,29 +378,14 @@ ww_mutex_set_context_fastpath(struct ww_mutex *lock, struct ww_acquire_ctx *ctx)
return;
/*
- * Uh oh, we raced in fastpath, wake up everyone in this case,
- * so they can see the new lock->ctx.
+ * Uh oh, we raced in fastpath, check if any of the waiters need to
+ * die.
*/
spin_lock(&lock->base.wait_lock);
- __ww_mutex_wakeup_for_backoff(&lock->base, ctx);
+ __ww_mutex_check_waiters(&lock->base, ctx);
spin_unlock(&lock->base.wait_lock);
}
-/*
- * After acquiring lock in the slowpath set ctx.
- *
- * Unlike for the fast path, the caller ensures that waiters are woken up where
- * necessary.
- *
- * Callers must hold the mutex wait_lock.
- */
-static __always_inline void
-ww_mutex_set_context_slowpath(struct ww_mutex *lock, struct ww_acquire_ctx *ctx)
-{
- ww_mutex_lock_acquired(lock, ctx);
- lock->ctx = ctx;
-}
-
#ifdef CONFIG_MUTEX_SPIN_ON_OWNER
static inline
@@ -645,37 +661,73 @@ void __sched ww_mutex_unlock(struct ww_mutex *lock)
}
EXPORT_SYMBOL(ww_mutex_unlock);
+
+static __always_inline int __sched
+__ww_mutex_kill(struct mutex *lock, struct ww_acquire_ctx *ww_ctx)
+{
+ if (ww_ctx->acquired > 0) {
+#ifdef CONFIG_DEBUG_MUTEXES
+ struct ww_mutex *ww;
+
+ ww = container_of(lock, struct ww_mutex, base);
+ DEBUG_LOCKS_WARN_ON(ww_ctx->contending_lock);
+ ww_ctx->contending_lock = ww;
+#endif
+ return -EDEADLK;
+ }
+
+ return 0;
+}
+
+
+/*
+ * Check whether we need to kill the transaction for the current lock acquire.
+ *
+ * Wait-Die: If we're trying to acquire a lock already held by an older
+ * context, kill ourselves.
+ *
+ * Since __ww_mutex_add_waiter() orders the wait-list on stamp, we only have to
+ * look at waiters before us in the wait-list.
+ */
static inline int __sched
-__ww_mutex_lock_check_stamp(struct mutex *lock, struct mutex_waiter *waiter,
- struct ww_acquire_ctx *ctx)
+__ww_mutex_check_kill(struct mutex *lock, struct mutex_waiter *waiter,
+ struct ww_acquire_ctx *ctx)
{
struct ww_mutex *ww = container_of(lock, struct ww_mutex, base);
struct ww_acquire_ctx *hold_ctx = READ_ONCE(ww->ctx);
struct mutex_waiter *cur;
+ if (ctx->acquired == 0)
+ return 0;
+
if (hold_ctx && __ww_ctx_stamp_after(ctx, hold_ctx))
- goto deadlock;
+ return __ww_mutex_kill(lock, ctx);
/*
* If there is a waiter in front of us that has a context, then its
- * stamp is earlier than ours and we must back off.
+ * stamp is earlier than ours and we must kill ourself.
*/
cur = waiter;
list_for_each_entry_continue_reverse(cur, &lock->wait_list, list) {
- if (cur->ww_ctx)
- goto deadlock;
+ if (!cur->ww_ctx)
+ continue;
+
+ return __ww_mutex_kill(lock, ctx);
}
return 0;
-
-deadlock:
-#ifdef CONFIG_DEBUG_MUTEXES
- DEBUG_LOCKS_WARN_ON(ctx->contending_lock);
- ctx->contending_lock = ww;
-#endif
- return -EDEADLK;
}
+/*
+ * Add @waiter to the wait-list, keep the wait-list ordered by stamp, smallest
+ * first. Such that older contexts are preferred to acquire the lock over
+ * younger contexts.
+ *
+ * Waiters without context are interspersed in FIFO order.
+ *
+ * Furthermore, for Wait-Die kill ourself immediately when possible (there are
+ * older contexts already waiting) to avoid unnecessary waiting.
+ */
static inline int __sched
__ww_mutex_add_waiter(struct mutex_waiter *waiter,
struct mutex *lock,
@@ -692,7 +744,7 @@ __ww_mutex_add_waiter(struct mutex_waiter *waiter,
/*
* Add the waiter before the first waiter with a higher stamp.
* Waiters without a context are skipped to avoid starving
- * them.
+ * them. Wait-Die waiters may die here.
*/
pos = &lock->wait_list;
list_for_each_entry_reverse(cur, &lock->wait_list, list) {
@@ -700,34 +752,27 @@ __ww_mutex_add_waiter(struct mutex_waiter *waiter,
continue;
if (__ww_ctx_stamp_after(ww_ctx, cur->ww_ctx)) {
- /* Back off immediately if necessary. */
- if (ww_ctx->acquired > 0) {
-#ifdef CONFIG_DEBUG_MUTEXES
- struct ww_mutex *ww;
+ /*
+ * Wait-Die: if we find an older context waiting, there
+ * is no point in queueing behind it, as we'd have to
+ * die the moment it would acquire the lock.
+ */
+ int ret = __ww_mutex_kill(lock, ww_ctx);
- ww = container_of(lock, struct ww_mutex, base);
- DEBUG_LOCKS_WARN_ON(ww_ctx->contending_lock);
- ww_ctx->contending_lock = ww;
-#endif
- return -EDEADLK;
- }
+ if (ret)
+ return ret;
break;
}
pos = &cur->list;
- /*
- * Wake up the waiter so that it gets a chance to back
- * off.
- */
- if (cur->ww_ctx->acquired > 0) {
- debug_mutex_wake_waiter(lock, cur);
- wake_up_process(cur->task);
- }
+ /* Wait-Die: ensure younger waiters die. */
+ __ww_mutex_die(lock, cur, ww_ctx);
}
list_add_tail(&waiter->list, pos);
+
return 0;
}
@@ -771,7 +816,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
*/
if (__mutex_trylock(lock)) {
if (use_ww_ctx && ww_ctx)
- __ww_mutex_wakeup_for_backoff(lock, ww_ctx);
+ __ww_mutex_check_waiters(lock, ww_ctx);
goto skip_wait;
}
@@ -789,10 +834,13 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
waiter.ww_ctx = MUTEX_POISON_WW_CTX;
#endif
} else {
- /* Add in stamp order, waking up waiters that must back off. */
+ /*
+ * Add in stamp order, waking up waiters that must kill
+ * themselves.
+ */
ret = __ww_mutex_add_waiter(&waiter, lock, ww_ctx);
if (ret)
- goto err_early_backoff;
+ goto err_early_kill;
waiter.ww_ctx = ww_ctx;
}
@@ -814,7 +862,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
goto acquired;
/*
- * Check for signals and wound conditions while holding
+ * Check for signals and kill conditions while holding
* wait_lock. This ensures the lock cancellation is ordered
* against mutex_unlock() and wake-ups do not go missing.
*/
@@ -823,8 +871,8 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
goto err;
}
- if (use_ww_ctx && ww_ctx && ww_ctx->acquired > 0) {
- ret = __ww_mutex_lock_check_stamp(lock, &waiter, ww_ctx);
+ if (use_ww_ctx && ww_ctx) {
+ ret = __ww_mutex_check_kill(lock, &waiter, ww_ctx);
if (ret)
goto err;
}
@@ -869,7 +917,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
lock_acquired(&lock->dep_map, ip);
if (use_ww_ctx && ww_ctx)
- ww_mutex_set_context_slowpath(ww, ww_ctx);
+ ww_mutex_lock_acquired(ww, ww_ctx);
spin_unlock(&lock->wait_lock);
preempt_enable();
@@ -878,7 +926,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
err:
__set_current_state(TASK_RUNNING);
mutex_remove_waiter(lock, &waiter, current);
-err_early_backoff:
+err_early_kill:
spin_unlock(&lock->wait_lock);
debug_mutex_free_waiter(&waiter);
mutex_release(&lock->dep_map, 1, ip);
--
2.14.3
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH 1/2] Documentation/sphinx: add "nodocs" directive
From: Jani Nikula @ 2018-06-19 8:09 UTC (permalink / raw)
To: Mike Rapoport; +Cc: Jonathan Corbet, Matthew Wilcox, linux-doc
In-Reply-To: <20180619075017.GB19099@rapoport-lnx>
On Tue, 19 Jun 2018, Mike Rapoport <rppt@linux.vnet.ibm.com> wrote:
> On Tue, Jun 19, 2018 at 10:29:20AM +0300, Jani Nikula wrote:
>> On Tue, 19 Jun 2018, Mike Rapoport <rppt@linux.vnet.ibm.com> wrote:
>> > On Mon, Jun 18, 2018 at 11:01:32PM +0300, Jani Nikula wrote:
>> >> On Mon, 18 Jun 2018, Mike Rapoport <rppt@linux.vnet.ibm.com> wrote:
>> >> > When kernel-doc:: specified in .rst document without explicit directives,
>> >> > it outputs both comment and DOC: sections. If a DOC: section was explictly
>> >> > included in the same document it will be duplicated. For example, the
>> >> > output generated for Documentation/core-api/idr.rst [1] has "IDA
>> >> > description" in the "IDA usage" section and in the middle of the API
>> >> > reference.
>> >> >
>> >> > Addition of "nodocs" directive prevents the duplication without the need to
>> >> > explicitly define what functions should be include in the API reference.
>> >> >
>> >> > [1] https://www.kernel.org/doc/html/v4.17/core-api/idr.html
>> >> >
>> >> > Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
>> >> > ---
>> >> > Documentation/sphinx/kerneldoc.py | 3 +++
>> >> > 1 file changed, 3 insertions(+)
>> >> >
>> >> > diff --git a/Documentation/sphinx/kerneldoc.py b/Documentation/sphinx/kerneldoc.py
>> >> > index fbedcc3..bc5dd05 100644
>> >> > --- a/Documentation/sphinx/kerneldoc.py
>> >> > +++ b/Documentation/sphinx/kerneldoc.py
>> >> > @@ -50,6 +50,7 @@ class KernelDocDirective(Directive):
>> >> > 'functions': directives.unchanged_required,
>> >> > 'export': directives.unchanged,
>> >> > 'internal': directives.unchanged,
>> >> > + 'nodocs': directives.unchanged,
>> >>
>> >> I'm not convinved this is the prettiest way to achieve what you
>> >> want. 'nodocs' seems kind of clunky.
>> >>
>> >> I'd suggest supporting 'functions' without option arguments, and turning
>> >> that into kernel-doc -no-doc-sections.
>> >
>> > Do you mean something like this:
>>
>> Yes.
>>
>> >
>> > diff --git a/Documentation/sphinx/kerneldoc.py b/Documentation/sphinx/kerneldoc.py
>> > index fbedcc3..9d0a7f0 100644
>> > --- a/Documentation/sphinx/kerneldoc.py
>> > +++ b/Documentation/sphinx/kerneldoc.py
>> > @@ -47,7 +47,7 @@ class KernelDocDirective(Directive):
>> > optional_arguments = 4
>> > option_spec = {
>> > 'doc': directives.unchanged_required,
>> > - 'functions': directives.unchanged_required,
>> > + 'functions': directives.unchanged,
>> > 'export': directives.unchanged,
>> > 'internal': directives.unchanged,
>> > }
>> > @@ -75,8 +75,12 @@ class KernelDocDirective(Directive):
>> > elif 'doc' in self.options:
>> > cmd += ['-function', str(self.options.get('doc'))]
>> > elif 'functions' in self.options:
>> > - for f in str(self.options.get('functions')).split():
>> > - cmd += ['-function', f]
>> > + functions = self.options.get('functions').split()
>>
>> Does .split() get upset if there's no argument? Or do you get an empty
>> string if there are no options? I forget.
>
> "".split() gives an empty list.
I tried to say, does self.options.get('functions') return an empty
string or None if there are no options?
BR,
Jani.
>
>> BR,
>> Jani.
>>
>> > + if functions:
>> > + for f in functions:
>> > + cmd += ['-function', f]
>> > + else:
>> > + cmd += ['-no-doc-sections']
>> >
>> > for pattern in export_file_patterns:
>> > for f in glob.glob(env.config.kerneldoc_srctree + '/' + pattern):
>> >
>> >> The usage in patch 2/2 would turn into:
>> >>
>> >> .. kernel-doc:: include/linux/idr.h
>> >> :functions:
>> >>
>> >> which I think is much better overall in the rst source, complementing
>> >> the places where you use :doc:.
>> >>
>> >> BR,
>> >> Jani.
>> >>
>> >> > }
>> >> > has_content = False
>> >> >
>> >> > @@ -77,6 +78,8 @@ class KernelDocDirective(Directive):
>> >> > elif 'functions' in self.options:
>> >> > for f in str(self.options.get('functions')).split():
>> >> > cmd += ['-function', f]
>> >> > + elif 'nodocs' in self.options:
>> >> > + cmd += ['-no-doc-sections']
>> >> >
>> >> > for pattern in export_file_patterns:
>> >> > for f in glob.glob(env.config.kerneldoc_srctree + '/' + pattern):
>> >>
>> >> --
>> >> Jani Nikula, Intel Open Source Graphics Center
>> >>
>>
>> --
>> Jani Nikula, Intel Open Source Graphics Center
>>
--
Jani Nikula, Intel Open Source Graphics Center
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] Documentation/sphinx: add "nodocs" directive
From: Mike Rapoport @ 2018-06-19 7:50 UTC (permalink / raw)
To: Jani Nikula; +Cc: Jonathan Corbet, Matthew Wilcox, linux-doc
In-Reply-To: <87602fxmi7.fsf@intel.com>
On Tue, Jun 19, 2018 at 10:29:20AM +0300, Jani Nikula wrote:
> On Tue, 19 Jun 2018, Mike Rapoport <rppt@linux.vnet.ibm.com> wrote:
> > On Mon, Jun 18, 2018 at 11:01:32PM +0300, Jani Nikula wrote:
> >> On Mon, 18 Jun 2018, Mike Rapoport <rppt@linux.vnet.ibm.com> wrote:
> >> > When kernel-doc:: specified in .rst document without explicit directives,
> >> > it outputs both comment and DOC: sections. If a DOC: section was explictly
> >> > included in the same document it will be duplicated. For example, the
> >> > output generated for Documentation/core-api/idr.rst [1] has "IDA
> >> > description" in the "IDA usage" section and in the middle of the API
> >> > reference.
> >> >
> >> > Addition of "nodocs" directive prevents the duplication without the need to
> >> > explicitly define what functions should be include in the API reference.
> >> >
> >> > [1] https://www.kernel.org/doc/html/v4.17/core-api/idr.html
> >> >
> >> > Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> >> > ---
> >> > Documentation/sphinx/kerneldoc.py | 3 +++
> >> > 1 file changed, 3 insertions(+)
> >> >
> >> > diff --git a/Documentation/sphinx/kerneldoc.py b/Documentation/sphinx/kerneldoc.py
> >> > index fbedcc3..bc5dd05 100644
> >> > --- a/Documentation/sphinx/kerneldoc.py
> >> > +++ b/Documentation/sphinx/kerneldoc.py
> >> > @@ -50,6 +50,7 @@ class KernelDocDirective(Directive):
> >> > 'functions': directives.unchanged_required,
> >> > 'export': directives.unchanged,
> >> > 'internal': directives.unchanged,
> >> > + 'nodocs': directives.unchanged,
> >>
> >> I'm not convinved this is the prettiest way to achieve what you
> >> want. 'nodocs' seems kind of clunky.
> >>
> >> I'd suggest supporting 'functions' without option arguments, and turning
> >> that into kernel-doc -no-doc-sections.
> >
> > Do you mean something like this:
>
> Yes.
>
> >
> > diff --git a/Documentation/sphinx/kerneldoc.py b/Documentation/sphinx/kerneldoc.py
> > index fbedcc3..9d0a7f0 100644
> > --- a/Documentation/sphinx/kerneldoc.py
> > +++ b/Documentation/sphinx/kerneldoc.py
> > @@ -47,7 +47,7 @@ class KernelDocDirective(Directive):
> > optional_arguments = 4
> > option_spec = {
> > 'doc': directives.unchanged_required,
> > - 'functions': directives.unchanged_required,
> > + 'functions': directives.unchanged,
> > 'export': directives.unchanged,
> > 'internal': directives.unchanged,
> > }
> > @@ -75,8 +75,12 @@ class KernelDocDirective(Directive):
> > elif 'doc' in self.options:
> > cmd += ['-function', str(self.options.get('doc'))]
> > elif 'functions' in self.options:
> > - for f in str(self.options.get('functions')).split():
> > - cmd += ['-function', f]
> > + functions = self.options.get('functions').split()
>
> Does .split() get upset if there's no argument? Or do you get an empty
> string if there are no options? I forget.
"".split() gives an empty list.
> BR,
> Jani.
>
> > + if functions:
> > + for f in functions:
> > + cmd += ['-function', f]
> > + else:
> > + cmd += ['-no-doc-sections']
> >
> > for pattern in export_file_patterns:
> > for f in glob.glob(env.config.kerneldoc_srctree + '/' + pattern):
> >
> >> The usage in patch 2/2 would turn into:
> >>
> >> .. kernel-doc:: include/linux/idr.h
> >> :functions:
> >>
> >> which I think is much better overall in the rst source, complementing
> >> the places where you use :doc:.
> >>
> >> BR,
> >> Jani.
> >>
> >> > }
> >> > has_content = False
> >> >
> >> > @@ -77,6 +78,8 @@ class KernelDocDirective(Directive):
> >> > elif 'functions' in self.options:
> >> > for f in str(self.options.get('functions')).split():
> >> > cmd += ['-function', f]
> >> > + elif 'nodocs' in self.options:
> >> > + cmd += ['-no-doc-sections']
> >> >
> >> > for pattern in export_file_patterns:
> >> > for f in glob.glob(env.config.kerneldoc_srctree + '/' + pattern):
> >>
> >> --
> >> Jani Nikula, Intel Open Source Graphics Center
> >>
>
> --
> Jani Nikula, Intel Open Source Graphics Center
>
--
Sincerely yours,
Mike.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/2] Documentation/sphinx: add "nodocs" directive
From: Jani Nikula @ 2018-06-19 7:29 UTC (permalink / raw)
To: Mike Rapoport; +Cc: Jonathan Corbet, Matthew Wilcox, linux-doc
In-Reply-To: <20180619050349.GA19099@rapoport-lnx>
On Tue, 19 Jun 2018, Mike Rapoport <rppt@linux.vnet.ibm.com> wrote:
> On Mon, Jun 18, 2018 at 11:01:32PM +0300, Jani Nikula wrote:
>> On Mon, 18 Jun 2018, Mike Rapoport <rppt@linux.vnet.ibm.com> wrote:
>> > When kernel-doc:: specified in .rst document without explicit directives,
>> > it outputs both comment and DOC: sections. If a DOC: section was explictly
>> > included in the same document it will be duplicated. For example, the
>> > output generated for Documentation/core-api/idr.rst [1] has "IDA
>> > description" in the "IDA usage" section and in the middle of the API
>> > reference.
>> >
>> > Addition of "nodocs" directive prevents the duplication without the need to
>> > explicitly define what functions should be include in the API reference.
>> >
>> > [1] https://www.kernel.org/doc/html/v4.17/core-api/idr.html
>> >
>> > Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
>> > ---
>> > Documentation/sphinx/kerneldoc.py | 3 +++
>> > 1 file changed, 3 insertions(+)
>> >
>> > diff --git a/Documentation/sphinx/kerneldoc.py b/Documentation/sphinx/kerneldoc.py
>> > index fbedcc3..bc5dd05 100644
>> > --- a/Documentation/sphinx/kerneldoc.py
>> > +++ b/Documentation/sphinx/kerneldoc.py
>> > @@ -50,6 +50,7 @@ class KernelDocDirective(Directive):
>> > 'functions': directives.unchanged_required,
>> > 'export': directives.unchanged,
>> > 'internal': directives.unchanged,
>> > + 'nodocs': directives.unchanged,
>>
>> I'm not convinved this is the prettiest way to achieve what you
>> want. 'nodocs' seems kind of clunky.
>>
>> I'd suggest supporting 'functions' without option arguments, and turning
>> that into kernel-doc -no-doc-sections.
>
> Do you mean something like this:
Yes.
>
> diff --git a/Documentation/sphinx/kerneldoc.py b/Documentation/sphinx/kerneldoc.py
> index fbedcc3..9d0a7f0 100644
> --- a/Documentation/sphinx/kerneldoc.py
> +++ b/Documentation/sphinx/kerneldoc.py
> @@ -47,7 +47,7 @@ class KernelDocDirective(Directive):
> optional_arguments = 4
> option_spec = {
> 'doc': directives.unchanged_required,
> - 'functions': directives.unchanged_required,
> + 'functions': directives.unchanged,
> 'export': directives.unchanged,
> 'internal': directives.unchanged,
> }
> @@ -75,8 +75,12 @@ class KernelDocDirective(Directive):
> elif 'doc' in self.options:
> cmd += ['-function', str(self.options.get('doc'))]
> elif 'functions' in self.options:
> - for f in str(self.options.get('functions')).split():
> - cmd += ['-function', f]
> + functions = self.options.get('functions').split()
Does .split() get upset if there's no argument? Or do you get an empty
string if there are no options? I forget.
BR,
Jani.
> + if functions:
> + for f in functions:
> + cmd += ['-function', f]
> + else:
> + cmd += ['-no-doc-sections']
>
> for pattern in export_file_patterns:
> for f in glob.glob(env.config.kerneldoc_srctree + '/' + pattern):
>
>> The usage in patch 2/2 would turn into:
>>
>> .. kernel-doc:: include/linux/idr.h
>> :functions:
>>
>> which I think is much better overall in the rst source, complementing
>> the places where you use :doc:.
>>
>> BR,
>> Jani.
>>
>> > }
>> > has_content = False
>> >
>> > @@ -77,6 +78,8 @@ class KernelDocDirective(Directive):
>> > elif 'functions' in self.options:
>> > for f in str(self.options.get('functions')).split():
>> > cmd += ['-function', f]
>> > + elif 'nodocs' in self.options:
>> > + cmd += ['-no-doc-sections']
>> >
>> > for pattern in export_file_patterns:
>> > for f in glob.glob(env.config.kerneldoc_srctree + '/' + pattern):
>>
>> --
>> Jani Nikula, Intel Open Source Graphics Center
>>
--
Jani Nikula, Intel Open Source Graphics Center
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 06/10] x86/cet: Add arch_prctl functions for shadow stack
From: Florian Weimer @ 2018-06-19 6:40 UTC (permalink / raw)
To: Kees Cook, Andy Lutomirski
Cc: H. J. Lu, Thomas Gleixner, Yu-cheng Yu, LKML, linux-doc, Linux-MM,
linux-arch, X86 ML, H. Peter Anvin, Ingo Molnar,
Shanbhogue, Vedvyas, Ravi V. Shankar, Dave Hansen,
Jonathan Corbet, Oleg Nesterov, Arnd Bergmann, mike.kravetz
In-Reply-To: <CAGXu5jK0gospOXRpN6zYiQPXOZeE=YpVAz2qu4Zc3-32v85+EQ@mail.gmail.com>
On 06/19/2018 02:52 AM, Kees Cook wrote:
> Adding Florian to CC, but if something gets CET enabled, it really
> shouldn't have a way to turn it off. If there's a way to turn it off,
> all the ROP research will suddenly turn to exactly one gadget before
> doing the rest of the ROP: turning off CET. Right now ROP is: use
> stack-pivot gadget, do everything else. Allowed CET to turn off will
> just add one step: use CET-off gadget, use stack-pivot gadget, do
> everything else. :P
>
> Following Linus's request for "slow introduction" of new security
> features, likely the best approach is to default to "relaxed" (with a
> warning about down-grades), and allow distros/end-users to pick
> "forced" if they know their libraries are all CET-enabled.
The dynamic linker can tell beforehand (before executing any user code)
whether a process image supports CET. So there doesn't have to be
anything gradual about it per se to preserve backwards compatibility.
The idea to turn off CET probably comes from the desire to support
dlopen. I'm not sure if this is really necessary because the complexity
is rather nasty. (We currently do something similar for executable
stacks.) I'd rather have a switch to turn off the feature upon process
start. Things like NSS and PAM modules need to be recompiled early. (I
hope that everything that goes directly to the network via custom
protocols or hardware such as smartcards is proxied via daemons these days.)
Thanks,
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox