Linux userland API discussions
 help / color / mirror / Atom feed
* Re: [RFC PATCH 0/3] restartable sequences: fast user-space percpu critical sections
From: Andy Lutomirski @ 2015-06-27 16:25 UTC (permalink / raw)
  To: Paul Turner
  Cc: Mathieu Desnoyers, Peter Zijlstra, Paul E. McKenney,
	Andrew Hunter, Andi Kleen, Lai Jiangshan, linux-api, LKML,
	rostedt, Josh Triplett, Ingo Molnar, Andrew Morton,
	Linus Torvalds, Chris Lameter
In-Reply-To: <CAPM31RJwFJXjxLnwFMCBE1=wVXyU06ttrT-_fNr1rb=+ewxVrg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Let me try to summarize some of the approaches with their pros and cons:

--- percpu segment ---

This is probably the simplest and might make sense regardless.
cmpxchg can be used to do an atomic push onto a linked list.  I think
that unlocked cmpxchg16b can be used to get an atomic pop.  (You'd
have the list head pointer next to an auxiliary pointer to the second
element in the list, perhaps.)

You can also use this for limited forms of speculative locking.
Aborting cleanly if your lock is stolen might require the kernel's
help, though (you're now on the wrong cpu, so you can't atomically
poke the lock variable any more).

The ABI is straightforward, and the only limitation on multiple users
in the same process is that they need to coordinate their offsets into
the percpu segment.

--- vdso-provided atomic ops ---

This could be quite flexible.  The upside is that the ABI would be
straightforward (call a function with clearly-specified behavior).
The downside is that implementing it well might require percpu
segments and a certain amount of coordination, and it requires a
function call.

One nice thing about doing it in the vdso is that we can change the
implementation down the road.

--- kernel preemption hooks ---

I'm defining a preemption hook as an action taken by the kernel when a
user task is preempted during a critical section.

As an upside, we get extremely efficient, almost arbitrary percpu
operations.  We don't need to worry about memory ordering at all,
because the whole sequence aborts if anything else might run on the
same cpu.  Push and pop are both easy.

One con is that actually defining where the critical section is might
be nasty.  If there's a single IP range, then two libraries could
fight over it.  We could have a variable somewhere that you write to
arm the critical section, but that's a bit slower.

Another con is that you can't single-step through this type of
critical section.  It will be preempted every time.

--- kernel migration hooks ---

I'm not sure either Paul or Mattieu discussed this, but another option
would be to have some special handling if a task is migrated during a
critical section or to allow a task to prevent migration entirely
during a critical section.  From the user's point of view, this is
weaker than preemption hooks: it's possible to start your critical
section, be preempted, and have another thread enter its own critical
section, then get rescheduled on the same cpu without aborting.  Users
would have to use local atomics (like cmpxchg) to make it useful.

As a major advantage, single-stepping still works.

This shares the coordination downside with preemption hooks (users
have to tell the kernel about their critical sections somehow).

Push can certainly be implemented using cmpxchg.  The gs prefix isn't
even needed.  Pop might be harder to implement directly without
resorting to cmpxchg16b or similar.

--- Unnamed trick ---

On entry to a critical section, try to take a per-cpu lock that stores
the holder's tid.  This might require percpu segments.

If you get the lock, then start doing your thing.  For example, you
could pop by reading head->next and writing it back to head.

If, however, you miss the lock, then you need to either wait or
forcibly abort the lock holder.  You could do the latter by sending a
signal or possibly using a new syscall that atomically aborts the lock
holder and takes the lock.  You don't need to wait, though -- all you
need to do is queue the signal and, if the lock holder is actually
running, wait for signal delivery to start.


Thoughts?  I personally like the other options better than preemption
hooks.  I prefer solutions that don't interfere with debugging.

--Andy

^ permalink raw reply

* Re: [RFC PATCH 2/3] restartable sequences: x86 ABI
From: Paul Turner @ 2015-06-27  1:33 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Mathieu Desnoyers, Peter Zijlstra, Paul E. McKenney,
	Andrew Hunter, Andi Kleen, Lai Jiangshan, linux-api,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, rostedt,
	Josh Triplett, Ingo Molnar, Andrew Morton, Linus Torvalds,
	Chris Lameter
In-Reply-To: <CALCETrWKzP8UPH2OEmwbC4egcAa6NA+VkQD6OuA-LhFv-Aqg6Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Jun 26, 2015 at 12:31 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> On Fri, Jun 26, 2015 at 11:09 AM, Mathieu Desnoyers
> <mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
>> ----- On Jun 24, 2015, at 6:26 PM, Paul Turner pjt-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org wrote:
>>
>>> Implements the x86 (i386 & x86-64) ABIs for interrupting and restarting
>>> execution within restartable sequence sections.
>>>
>>> With respect to the x86-specific ABI:
>>>  On 32-bit:           Upon restart, the interrupted rip is placed in %ecx
>>>  On 64-bit (or x32):  Upon restart, the interrupted rip is placed in %r10
>>>
>>> While potentially surprising at first glance, this choice is strongly motivated
>>> by the fact that the available scratch registers under the i386 function call
>>> ABI overlap with those used as argument registers under x86_64.
>>>
>>> Given that sequences are already personality specific and that we always want
>>> the arguments to be available for sequence restart, it's much more natural to
>>> ultimately differentiate the ABI in these two cases.
>>>
>>> Signed-off-by: Paul Turner <pjt-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>>> ---
>>> arch/x86/include/asm/restartable_sequences.h |   50 +++++++++++++++++++
>>> arch/x86/kernel/Makefile                     |    2 +
>>> arch/x86/kernel/restartable_sequences.c      |   69 ++++++++++++++++++++++++++
>>> arch/x86/kernel/signal.c                     |   12 +++++
>>> kernel/restartable_sequences.c               |   11 +++-
>>> 5 files changed, 141 insertions(+), 3 deletions(-)
>>> create mode 100644 arch/x86/include/asm/restartable_sequences.h
>>> create mode 100644 arch/x86/kernel/restartable_sequences.c
>>>
>>> diff --git a/arch/x86/include/asm/restartable_sequences.h
>>> b/arch/x86/include/asm/restartable_sequences.h
>>> new file mode 100644
>>> index 0000000..0ceb024
>>> --- /dev/null
>>> +++ b/arch/x86/include/asm/restartable_sequences.h
>>> @@ -0,0 +1,50 @@
>>> +#ifndef _ASM_X86_RESTARTABLE_SEQUENCES_H
>>> +#define _ASM_X86_RESTARTABLE_SEQUENCES_H
>>> +
>>> +#include <asm/processor.h>
>>> +#include <asm/ptrace.h>
>>> +#include <linux/sched.h>
>>> +
>>> +#ifdef CONFIG_RESTARTABLE_SEQUENCES
>>> +
>>> +static inline bool arch_rseq_in_crit_section(struct task_struct *p,
>>> +                                          struct pt_regs *regs)
>>> +{
>>> +     struct task_struct *leader = p->group_leader;
>>> +     struct restartable_sequence_state *rseq_state = &leader->rseq_state;
>>> +
>>> +     unsigned long ip = (unsigned long)regs->ip;
>>> +     if (unlikely(ip < (unsigned long)rseq_state->crit_end &&
>>> +                  ip >= (unsigned long)rseq_state->crit_start))
>>> +             return true;
>>> +
>>> +     return false;
>>> +}
>>> +
>>> +static inline bool arch_rseq_needs_notify_resume(struct task_struct *p)
>>> +{
>>> +#ifdef CONFIG_PREEMPT
>>> +     /*
>>> +      * Under CONFIG_PREEMPT it's possible for regs to be incoherent in the
>>> +      * case that we took an interrupt during syscall entry.  Avoid this by
>>> +      * always deferring to our notify-resume handler.
>>> +      */
>>> +     return true;
>>
>> I'm a bit puzzled about this. If I look at perf_get_regs_user() in the perf
>> code, task_pt_regs() seems to return the user-space pt_regs for a task with
>> a current->mm set (iow, not a kernel thread), even if an interrupt nests on
>> top of a system call. The only corner-case is NMIs, where an NMI may interrupt
>> in the middle of setting up the task pt_regs, but scheduling should never happen
>> there, right ?
>
> Careful, here!  task_pt_regs returns a pointer to the place where regs
> would be if they were fully initialized.  We can certainly take an
> interrupt in the middle of pt_regs setup (entry_SYSCALL_64 enables
> interrupts very early, for example).  To me, the question is whether
> we can ever be preemptable at such a time.
>
> It's a bit worse, though: we can certainly be preemptible when other
> code is accessing pt_regs.  clone, execve, sigreturn, and signal
> delivery come to mind.

Yeah Andy covered it exactly: interrupt in pt_regs setup.

With respect to whether we can be preemptible; I think we were
concerned about rescheduling during syscall entry but I'd have to
re-audit the current state of entry_64.S :)

Mathieu also wrote:
> Moving ENABLE_INTERRUPTS(CLBR_NONE) 3 instructions down, just after
> pushq   %rcx                            /* pt_regs->ip */
> might solve your issue here. (in entry_SYSCALL_64_after_swapgs)

We considered doing something exactly like this; but I think any
potential changes here should be made in isolation of this series.

>
> Why don't we give up on poking at user state from the scheduler and do
> it on exit to user mode instead?  Starting in 4.3 (hopefully landing
> in -tip in a week or two), we should have a nice function
> prepare_exit_to_usermode that runs with well-defined state,
> non-reentrantly, that can do whatever you want here, *including user
> memory access*.

So this series already does the exact approximation of that:
The only thing we touch in the scheduler is looking at the kernel copy
pt_regs in the case we know it's safe to.

The entirety of *any* poking (both current cpu pointer updates and
potential rip manipulation) at user-state exactly happens in the
exit-to-user path via TIF_NOTIFY_RESUME.


>
> The remaining question would be what the ABI should be.
>
> Could we get away with a vDSO function along the lines of "set *A=B
> and *X=Y if we're on cpu N and *X=Z"?  Straight-up cmpxchg would be
> even simpler.

The short answer is yes [*]; but I don't think it should live in the vDSO.

a) vdso-Call overhead is fairly high
b) I don't think there are any properties of being in the vDSO that we
benefit from.
c) It would be nice if these sequences were inlinable.

I have an alternate implementation that satisfies (c) which I'm
looking to propose early next week (I've got it baking on some tests
over the weekend).

[*]  I mean the very simplest implementation of taking this patch and
putting the implementation of the critical section is clearly
sufficient.

Moving ENABLE_INTERRUPTS(CLBR_NONE) 3 instructions down, just after
pushq   %rcx                            /* pt_regs->ip */
might solve your issue here. (in entry_SYSCALL_64_after_swapgs)

^ permalink raw reply

* [PATCH v2] selftests/exec: Fix build on older distros.
From: Vinson Lee @ 2015-06-26 23:11 UTC (permalink / raw)
  To: Shuah Khan, David Drysdale, Andrew Morton, Geert Uytterhoeven,
	Michael Ellerman
  Cc: linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Vinson Lee
In-Reply-To: <CAHse=S817frpt-SGVQCWf5Fq3jV3mHdYEkU4XFhu48jkT6CKog-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

From: Vinson Lee <vlee-1v8oiQdgUNlBDgjK7y7TUQ@public.gmane.org>

This patch fixes this build error on CentOS 5.

execveat.c: In function ‘check_execveat_pathmax’:
execveat.c:185: error: ‘AT_EMPTY_PATH’ undeclared (first use in this function)
execveat.c:185: error: (Each undeclared identifier is reported only once
execveat.c:185: error: for each function it appears in.)
execveat.c: In function ‘run_tests’:
execveat.c:221: error: ‘O_PATH’ undeclared (first use in this function)
execveat.c:222: error: ‘O_CLOEXEC’ undeclared (first use in this function)
execveat.c:258: error: ‘AT_EMPTY_PATH’ undeclared (first use in this function)

Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org # 3.19+
Signed-off-by: Vinson Lee <vlee-1v8oiQdgUNlBDgjK7y7TUQ@public.gmane.org>
---
 tools/testing/selftests/exec/execveat.c | 34 +++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/tools/testing/selftests/exec/execveat.c b/tools/testing/selftests/exec/execveat.c
index 8d5d1d2..30d2555 100644
--- a/tools/testing/selftests/exec/execveat.c
+++ b/tools/testing/selftests/exec/execveat.c
@@ -20,6 +20,40 @@
 #include <string.h>
 #include <unistd.h>
 
+#if defined( __alpha__)
+# ifndef O_CLOEXEC
+#  define O_CLOEXEC	010000000
+# endif
+# ifndef O_PATH
+#  define O_PATH	040000000
+# endif
+#elif defined(__hppa__)
+# ifndef O_CLOEXEC
+#  define O_CLOEXEC	010000000
+# endif
+# ifndef O_PATH
+#  define O_PATH	020000000
+# endif
+#elif defined(__sparc__)
+# ifndef O_CLOEXEC
+#  define O_CLOEXEC	0x400000
+# endif
+# ifndef O_PATH
+#  define O_PATH 	0x1000000
+# endif
+#else
+# ifndef O_CLOEXEC
+#  define O_CLOEXEC	02000000
+# endif
+# ifndef O_PATH
+#  define O_PATH	010000000
+# endif
+#endif
+
+#ifndef AT_EMPTY_PATH
+# define AT_EMPTY_PATH	0x1000
+#endif
+
 static char longpath[2 * PATH_MAX] = "";
 static char *envp[] = { "IN_TEST=yes", NULL, NULL };
 static char *argv[] = { "execveat", "99", NULL };
-- 
1.8.2.1

^ permalink raw reply related

* [GIT PULL] User namespace related fixes for v4.2
From: Eric W. Biederman @ 2015-06-26 20:50 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Seth Forshee, Linux API, Linux Containers, Greg Kroah-Hartman,
	Andy Lutomirski, Kenton Varda, Michael Kerrisk-manpages,
	Richard Weinberger, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	Tejun Heo, Ivan Delalande


Date: Fri, 22 May 2015 15:41:45 -0500 (4 weeks, 6 days, 23 hours ago)

Linus,

Please pull the for-linus branch from the git tree:

   git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git for-linus

   HEAD: 81909cb3350299977a88f72264651f6cec06c836 mnt: Avoid unnecessary regressions in fs_fully_visible

Long ago and far away when user namespaces where young and I was a more
optimistic man it was realized that allowing fresh mounts of proc and
sysfs with only user namespace permissions could violate the basic rule
that only root gets to decide if proc or sysfs should be mounted at all.

Some hacks were put in place to reduce the worst of the damage could be
done, and the common sense rule was adopted that fresh mounts of proc
and sysfs should allow no more than bind mounts of proc and sysfs.
Unfortunately that rule has not been fully enforced.

There are two kinds of gaps in that enforcement.  Only filesystems mount
on empty directories on proc and sysfs should be ignored but the test
for empty directories was insufficient.  So this patchset requires
directories on proc, sysctl and sysfs that are will always be empty to
be created specially.  Every other technique is lossy as an ordinary
directory can dynamically be added to later.  This actually makes this
code in the kernel a smidge clearer about it's purpose.  I asked
container developers from the various container projects to help test
this and no holes were found in the set of mount points on proc and
sysfs that this patchset identifies.

This set of changes also starts enforcing the mount flags of fresh
mounts of proc and sysfs are consistent with the existing mount of proc
and sysfs.  I expected this to be the boring part of this patchset but
unfortunately userspace has been stupid and extra work has to be done to
avoid regressions.  The atime, read-only, and nodev attributes were not
a problem and as such are enforced absolutely.

People have been winding up mounting proc and sysfs in contaners with
nosuid and noexec clear, when the global root had set nosuid and noexec.
In practice this does not make a hill of beans difference today because
currently there are no exectuables on proc and sysfs.  Unfortunately
that can not be guaranteed in the future.  People refactor code and bugs
get reintroduced, or people find a good reason to do something that
today seems ludicrous.  Give people 20 more years and who knows what
will happen.

The libvirt-lxc and lxc developers have been contacted so they can
correct the bugs where they clear noexec and nosuid on proc and sysfs
through oversights when they wrote their code.  Thos bugs should be
fixed in those projects shortly.  These bugs are an issue however
libvirt-lxc or lxc create containers.  However they only violate kernel
permission checks in the case of containers created by unprivileged
users, which is a niche case today.

Therefore this changeset marks for backporting the attribute enforcement
that do not cause regressions in the existing userspace. Implements
enforcement of nosuid and noexec.  Then disables that enforcement of
nosuid and nosexec and replaces that enforcment with a big fat warning.
Userspace should be fixed before 4.2 ships so I do not expect these
warnings to fire.  However the warnings give userspace time to get their
act together.  I am optimistic that all of userspace that cares will be
fixed and for v4.3 I can remove the warning messages and enforce the
attribute checks.

It is a fine line on the regression front and I hate walking it, but now
is the best time to address the issue of clearing attributes that should
not be cleared before lots of unprivileged container implementations
accumulate, and before nosid and noexec proc and sysfs matter.

This set of changes also addresses how open file descriptors from
/proc/<pid>/ns/* are displayed.  Recently readlink of /proc/<pid>/fd has
been triggering a WARN_ON that has not been meaningful in nearly a
decade, and is actively wrong now.  An old bug (2 years?) in
/proc/<pid>/mountinfo where bind mounts of these descriptors were
not meaningfully show is fixed.

Eric W. Biederman (14):
      mnt: Refactor the logic for mounting sysfs and proc in a user namespace
      mnt: Modify fs_fully_visible to deal with locked ro nodev and atime
      mnt: Modify fs_fully_visible to deal with locked nosuid and noexec
      vfs: Ignore unlocked mounts in fs_fully_visible
      fs: Add helper functions for permanently empty directories.
      sysctl: Allow creating permanently empty directories that serve as mountpoints.
      proc: Allow creating permanently empty directories that serve as mount points
      kernfs: Add support for always empty directories.
      sysfs: Add support for permanently empty directories to serve as mount points.
      sysfs: Create mountpoints with sysfs_create_mount_point
      mnt: Update fs_fully_visible to test for permanently empty directories
      vfs: Remove incorrect debugging WARN in prepend_path
      nsfs: Add a show_path method to fix mountinfo
      mnt: Avoid unnecessary regressions in fs_fully_visible

 arch/s390/hypfs/inode.c      | 12 ++----
 drivers/firmware/efi/efi.c   |  6 +--
 fs/configfs/mount.c          | 10 ++---
 fs/dcache.c                  | 11 -----
 fs/debugfs/inode.c           | 11 ++---
 fs/fuse/inode.c              |  9 ++---
 fs/kernfs/dir.c              | 38 +++++++++++++++++-
 fs/kernfs/inode.c            |  2 +
 fs/libfs.c                   | 96 ++++++++++++++++++++++++++++++++++++++++++++
 fs/namespace.c               | 80 +++++++++++++++++++++++++++++++++---
 fs/nsfs.c                    | 10 +++++
 fs/proc/generic.c            | 23 +++++++++++
 fs/proc/inode.c              |  4 ++
 fs/proc/internal.h           |  6 +++
 fs/proc/proc_sysctl.c        | 37 +++++++++++++++++
 fs/proc/root.c               |  9 ++---
 fs/pstore/inode.c            | 12 ++----
 fs/sysfs/dir.c               | 34 ++++++++++++++++
 fs/sysfs/mount.c             |  5 +--
 fs/tracefs/inode.c           |  6 +--
 include/linux/fs.h           |  4 +-
 include/linux/kernfs.h       |  3 ++
 include/linux/mount.h        |  5 +++
 include/linux/sysctl.h       |  3 ++
 include/linux/sysfs.h        | 15 +++++++
 kernel/cgroup.c              | 10 ++---
 kernel/sysctl.c              |  8 +---
 security/inode.c             | 10 ++---
 security/selinux/selinuxfs.c | 11 +++--
 security/smack/smackfs.c     |  8 ++--
 30 files changed, 397 insertions(+), 101 deletions(-)

^ permalink raw reply

* [GIT PULL] Kselftest updates for 4.2-rc1
From: Shuah Khan @ 2015-06-26 20:02 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	open list:KERNEL SELFTEST F..., Shuah Khan

Hi Linus,

Please pull the following Kselftest updates for 4.2-rc1

thanks,
-- Shuah

 The following changes since commit
ba155e2d21f6bf05de86a78dbe5bfd8757604a65:

  Linux 4.1-rc5 (2015-05-24 18:22:35 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest
tags/linux-kselftest-4.2-rc1

for you to fetch changes up to 2278e5ed9f36baca7c972ed17aae7467ca91b2b9:

  selftests: add quicktest support (2015-06-23 07:20:16 -0600)

----------------------------------------------------------------
linux-kselftest-4.2-rc1

This update adds two new test suites: futex and seccomp.
In addition, it includes fixes for bugs in timers, other
tests, and compile framework. It introduces new quicktest
feature to enable users to choose to run tests that complete
in a short time..

----------------------------------------------------------------
Arnaldo Carvalho de Melo (1):
      tools selftests: Fix 'clean' target with make 3.81

Darren Hart (6):
      selftests: Add futex functional tests
      selftests/futex: Update Makefile to use lib.mk
      selftests/futex: Increment ksft pass and fail counters
      selftests: Add futex tests to the top-level Makefile
      kselftest: Add exit code defines
      selftests/futex: Add .gitignore

John Stultz (3):
      kselftests: timers: Increase delay between suspends in
alarmtimer-suspend
      kselftests: timers: Ease alarmtimer-suspend unreasonable latency value
      kselftests: timers: Check _ALARM clockids are supported before
suspending

Kees Cook (1):
      selftests: add seccomp suite

Martin Kelly (1):
      selftest, x86: fix incorrect comment

Shuah Khan (1):
      selftests: add quicktest support

Sri Jayaramappa (1):
      Test compaction of mlocked memory

Tyler Baker (3):
      selftests: copy TEST_DIRS to INSTALL_PATH
      selftests/ftrace: install test.d
      selftests/exec: do not install subdir as it is already created

Zhang Zhen (2):
      selftests/timers: Make git ignore all binaries in timers test suite
      selftests/mount: output WARN messages when mount test skipped

 MAINTAINERS                                        |    1 +
 tools/testing/selftests/Makefile                   |    8 +-
 tools/testing/selftests/exec/Makefile              |    2 +-
 tools/testing/selftests/ftrace/Makefile            |    1 +
 tools/testing/selftests/futex/Makefile             |   29 +
 tools/testing/selftests/futex/README               |   62 +
 .../testing/selftests/futex/functional/.gitignore  |    7 +
 tools/testing/selftests/futex/functional/Makefile  |   25 +
 .../selftests/futex/functional/futex_requeue_pi.c  |  409 ++++
 .../functional/futex_requeue_pi_mismatched_ops.c   |  135 ++
 .../functional/futex_requeue_pi_signal_restart.c   |  223 +++
 .../functional/futex_wait_private_mapped_file.c    |  125 ++
 .../futex/functional/futex_wait_timeout.c          |   86 +
 .../functional/futex_wait_uninitialized_heap.c     |  124 ++
 .../futex/functional/futex_wait_wouldblock.c       |   79 +
 tools/testing/selftests/futex/functional/run.sh    |   79 +
 tools/testing/selftests/futex/include/atomic.h     |   83 +
 tools/testing/selftests/futex/include/futextest.h  |  266 +++
 tools/testing/selftests/futex/include/logging.h    |  153 ++
 tools/testing/selftests/futex/run.sh               |   33 +
 tools/testing/selftests/kselftest.h                |   17 +-
 tools/testing/selftests/lib.mk                     |    3 +
 tools/testing/selftests/mount/Makefile             |    7 +-
 tools/testing/selftests/seccomp/.gitignore         |    1 +
 tools/testing/selftests/seccomp/Makefile           |   10 +
 tools/testing/selftests/seccomp/seccomp_bpf.c      | 2109
++++++++++++++++++++
 tools/testing/selftests/seccomp/test_harness.h     |  537 +++++
 tools/testing/selftests/timers/.gitignore          |   18 +
 .../testing/selftests/timers/alarmtimer-suspend.c  |   10 +-
 tools/testing/selftests/vm/Makefile                |    7 +-
 tools/testing/selftests/vm/compaction_test.c       |  225 +++
 tools/testing/selftests/vm/run_vmtests             |   12 +
 .../testing/selftests/x86/trivial_64bit_program.c  |    2 +-
 33 files changed, 4875 insertions(+), 13 deletions(-)
 create mode 100644 tools/testing/selftests/futex/Makefile
 create mode 100644 tools/testing/selftests/futex/README
 create mode 100644 tools/testing/selftests/futex/functional/.gitignore
 create mode 100644 tools/testing/selftests/futex/functional/Makefile
 create mode 100644
tools/testing/selftests/futex/functional/futex_requeue_pi.c
 create mode 100644
tools/testing/selftests/futex/functional/futex_requeue_pi_mismatched_ops.c
 create mode 100644
tools/testing/selftests/futex/functional/futex_requeue_pi_signal_restart.c
 create mode 100644
tools/testing/selftests/futex/functional/futex_wait_private_mapped_file.c
 create mode 100644
tools/testing/selftests/futex/functional/futex_wait_timeout.c
 create mode 100644
tools/testing/selftests/futex/functional/futex_wait_uninitialized_heap.c
 create mode 100644
tools/testing/selftests/futex/functional/futex_wait_wouldblock.c
 create mode 100755 tools/testing/selftests/futex/functional/run.sh
 create mode 100644 tools/testing/selftests/futex/include/atomic.h
 create mode 100644 tools/testing/selftests/futex/include/futextest.h
 create mode 100644 tools/testing/selftests/futex/include/logging.h
 create mode 100755 tools/testing/selftests/futex/run.sh
 create mode 100644 tools/testing/selftests/seccomp/.gitignore
 create mode 100644 tools/testing/selftests/seccomp/Makefile
 create mode 100644 tools/testing/selftests/seccomp/seccomp_bpf.c
 create mode 100644 tools/testing/selftests/seccomp/test_harness.h
 create mode 100644 tools/testing/selftests/timers/.gitignore
 create mode 100644 tools/testing/selftests/vm/compaction_test.c


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978

^ permalink raw reply

* Re: [RFC PATCH 2/3] restartable sequences: x86 ABI
From: Andy Lutomirski @ 2015-06-26 19:31 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Paul Turner, Peter Zijlstra, Paul E. McKenney, Andrew Hunter,
	Andi Kleen, Lai Jiangshan, linux-api,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, rostedt,
	Josh Triplett, Ingo Molnar, Andrew Morton, Linus Torvalds,
	Chris Lameter
In-Reply-To: <1050218158.4054.1435342186284.JavaMail.zimbra-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>

On Fri, Jun 26, 2015 at 11:09 AM, Mathieu Desnoyers
<mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
> ----- On Jun 24, 2015, at 6:26 PM, Paul Turner pjt-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org wrote:
>
>> Implements the x86 (i386 & x86-64) ABIs for interrupting and restarting
>> execution within restartable sequence sections.
>>
>> With respect to the x86-specific ABI:
>>  On 32-bit:           Upon restart, the interrupted rip is placed in %ecx
>>  On 64-bit (or x32):  Upon restart, the interrupted rip is placed in %r10
>>
>> While potentially surprising at first glance, this choice is strongly motivated
>> by the fact that the available scratch registers under the i386 function call
>> ABI overlap with those used as argument registers under x86_64.
>>
>> Given that sequences are already personality specific and that we always want
>> the arguments to be available for sequence restart, it's much more natural to
>> ultimately differentiate the ABI in these two cases.
>>
>> Signed-off-by: Paul Turner <pjt-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>> ---
>> arch/x86/include/asm/restartable_sequences.h |   50 +++++++++++++++++++
>> arch/x86/kernel/Makefile                     |    2 +
>> arch/x86/kernel/restartable_sequences.c      |   69 ++++++++++++++++++++++++++
>> arch/x86/kernel/signal.c                     |   12 +++++
>> kernel/restartable_sequences.c               |   11 +++-
>> 5 files changed, 141 insertions(+), 3 deletions(-)
>> create mode 100644 arch/x86/include/asm/restartable_sequences.h
>> create mode 100644 arch/x86/kernel/restartable_sequences.c
>>
>> diff --git a/arch/x86/include/asm/restartable_sequences.h
>> b/arch/x86/include/asm/restartable_sequences.h
>> new file mode 100644
>> index 0000000..0ceb024
>> --- /dev/null
>> +++ b/arch/x86/include/asm/restartable_sequences.h
>> @@ -0,0 +1,50 @@
>> +#ifndef _ASM_X86_RESTARTABLE_SEQUENCES_H
>> +#define _ASM_X86_RESTARTABLE_SEQUENCES_H
>> +
>> +#include <asm/processor.h>
>> +#include <asm/ptrace.h>
>> +#include <linux/sched.h>
>> +
>> +#ifdef CONFIG_RESTARTABLE_SEQUENCES
>> +
>> +static inline bool arch_rseq_in_crit_section(struct task_struct *p,
>> +                                          struct pt_regs *regs)
>> +{
>> +     struct task_struct *leader = p->group_leader;
>> +     struct restartable_sequence_state *rseq_state = &leader->rseq_state;
>> +
>> +     unsigned long ip = (unsigned long)regs->ip;
>> +     if (unlikely(ip < (unsigned long)rseq_state->crit_end &&
>> +                  ip >= (unsigned long)rseq_state->crit_start))
>> +             return true;
>> +
>> +     return false;
>> +}
>> +
>> +static inline bool arch_rseq_needs_notify_resume(struct task_struct *p)
>> +{
>> +#ifdef CONFIG_PREEMPT
>> +     /*
>> +      * Under CONFIG_PREEMPT it's possible for regs to be incoherent in the
>> +      * case that we took an interrupt during syscall entry.  Avoid this by
>> +      * always deferring to our notify-resume handler.
>> +      */
>> +     return true;
>
> I'm a bit puzzled about this. If I look at perf_get_regs_user() in the perf
> code, task_pt_regs() seems to return the user-space pt_regs for a task with
> a current->mm set (iow, not a kernel thread), even if an interrupt nests on
> top of a system call. The only corner-case is NMIs, where an NMI may interrupt
> in the middle of setting up the task pt_regs, but scheduling should never happen
> there, right ?

Careful, here!  task_pt_regs returns a pointer to the place where regs
would be if they were fully initialized.  We can certainly take an
interrupt in the middle of pt_regs setup (entry_SYSCALL_64 enables
interrupts very early, for example).  To me, the question is whether
we can ever be preemptable at such a time.

It's a bit worse, though: we can certainly be preemptible when other
code is accessing pt_regs.  clone, execve, sigreturn, and signal
delivery come to mind.

Why don't we give up on poking at user state from the scheduler and do
it on exit to user mode instead?  Starting in 4.3 (hopefully landing
in -tip in a week or two), we should have a nice function
prepare_exit_to_usermode that runs with well-defined state,
non-reentrantly, that can do whatever you want here, *including user
memory access*.

The remaining question would be what the ABI should be.

Could we get away with a vDSO function along the lines of "set *A=B
and *X=Y if we're on cpu N and *X=Z"?  Straight-up cmpxchg would be
even simpler.

--Andy

^ permalink raw reply

* Re: [RFC PATCH 2/3] restartable sequences: x86 ABI
From: Mathieu Desnoyers @ 2015-06-26 19:04 UTC (permalink / raw)
  To: Paul Turner
  Cc: Peter Zijlstra, Paul E. McKenney, Andrew Hunter, Andi Kleen,
	Lai Jiangshan, linux-api, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	rostedt, Josh Triplett, Ingo Molnar, Andrew Morton,
	Andy Lutomirski, Linus Torvalds, Chris Lameter
In-Reply-To: <1050218158.4054.1435342186284.JavaMail.zimbra-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>

----- On Jun 26, 2015, at 2:09 PM, Mathieu Desnoyers mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org wrote:

> ----- On Jun 24, 2015, at 6:26 PM, Paul Turner pjt-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org wrote:
> 
>> Implements the x86 (i386 & x86-64) ABIs for interrupting and restarting
>> execution within restartable sequence sections.
>> 
>> With respect to the x86-specific ABI:
>>  On 32-bit:           Upon restart, the interrupted rip is placed in %ecx
>>  On 64-bit (or x32):  Upon restart, the interrupted rip is placed in %r10
>> 
>> While potentially surprising at first glance, this choice is strongly motivated
>> by the fact that the available scratch registers under the i386 function call
>> ABI overlap with those used as argument registers under x86_64.
>> 
>> Given that sequences are already personality specific and that we always want
>> the arguments to be available for sequence restart, it's much more natural to
>> ultimately differentiate the ABI in these two cases.
>> 
>> Signed-off-by: Paul Turner <pjt-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>> ---
>> arch/x86/include/asm/restartable_sequences.h |   50 +++++++++++++++++++
>> arch/x86/kernel/Makefile                     |    2 +
>> arch/x86/kernel/restartable_sequences.c      |   69 ++++++++++++++++++++++++++
>> arch/x86/kernel/signal.c                     |   12 +++++
>> kernel/restartable_sequences.c               |   11 +++-
>> 5 files changed, 141 insertions(+), 3 deletions(-)
>> create mode 100644 arch/x86/include/asm/restartable_sequences.h
>> create mode 100644 arch/x86/kernel/restartable_sequences.c
>> 
>> diff --git a/arch/x86/include/asm/restartable_sequences.h
>> b/arch/x86/include/asm/restartable_sequences.h
>> new file mode 100644
>> index 0000000..0ceb024
>> --- /dev/null
>> +++ b/arch/x86/include/asm/restartable_sequences.h
>> @@ -0,0 +1,50 @@
>> +#ifndef _ASM_X86_RESTARTABLE_SEQUENCES_H
>> +#define _ASM_X86_RESTARTABLE_SEQUENCES_H
>> +
>> +#include <asm/processor.h>
>> +#include <asm/ptrace.h>
>> +#include <linux/sched.h>
>> +
>> +#ifdef CONFIG_RESTARTABLE_SEQUENCES
>> +
>> +static inline bool arch_rseq_in_crit_section(struct task_struct *p,
>> +					     struct pt_regs *regs)
>> +{
>> +	struct task_struct *leader = p->group_leader;
>> +	struct restartable_sequence_state *rseq_state = &leader->rseq_state;
>> +
>> +	unsigned long ip = (unsigned long)regs->ip;
>> +	if (unlikely(ip < (unsigned long)rseq_state->crit_end &&
>> +		     ip >= (unsigned long)rseq_state->crit_start))
>> +		return true;
>> +
>> +	return false;
>> +}
>> +
>> +static inline bool arch_rseq_needs_notify_resume(struct task_struct *p)
>> +{
>> +#ifdef CONFIG_PREEMPT
>> +	/*
>> +	 * Under CONFIG_PREEMPT it's possible for regs to be incoherent in the
>> +	 * case that we took an interrupt during syscall entry.  Avoid this by
>> +	 * always deferring to our notify-resume handler.
>> +	 */
>> +	return true;
> 
> I'm a bit puzzled about this. If I look at perf_get_regs_user() in the perf
> code, task_pt_regs() seems to return the user-space pt_regs for a task with
> a current->mm set (iow, not a kernel thread), even if an interrupt nests on
> top of a system call. The only corner-case is NMIs, where an NMI may interrupt
> in the middle of setting up the task pt_regs, but scheduling should never happen
> there, right ?
> 
> Since it's impossible for kernel threads to have a rseq critical section,
> we should be able to assume that every time task_pt_regs() returns a
> non-userspace (user_mode(regs) != 0) pt_regs implies that scheduling applies
> to a kernel thread. Therefore, following this line of thoughts,
> arch_rseq_in_crit_section() should work for CONFIG_PREEMPT kernels too.
> 
> So what I am missing here ?

AFAIU, the comment near this check in perf_get_regs_user() is bogus.
It does not only apply to NMIs, but also applies to normal interrupt
handlers that nest over the stack setup on syscall entry (below
entry_SYSCALL_64_after_swapgs in entry_64.S):

        struct pt_regs *user_regs = task_pt_regs(current);

        /*
         * If we're in an NMI that interrupted task_pt_regs setup, then
         * we can't sample user regs at all.  This check isn't really
         * sufficient, though, as we could be in an NMI inside an interrupt
         * that happened during task_pt_regs setup.
         */
        if (regs->sp > (unsigned long)&user_regs->r11 &&
            regs->sp <= (unsigned long)(user_regs + 1)) {
                regs_user->abi = PERF_SAMPLE_REGS_ABI_NONE;
                regs_user->regs = NULL;
                return;
        }

That would be how, for tracing, those races can be avoided. It
might not be a huge issue for perf samples to lose one sample once
in a while, but I understand that this statistical approach would
be incorrect in the context of RSEQ.

Moving ENABLE_INTERRUPTS(CLBR_NONE) 3 instructions down, just after
pushq   %rcx                            /* pt_regs->ip */
might solve your issue here. (in entry_SYSCALL_64_after_swapgs)

Thoughts ?

Thanks,

Mathieu


> 
> Thanks,
> 
> Mathieu
> 
>> +#else
>> +	return arch_rseq_in_crit_section(p, task_pt_regs(p));
>> +#endif
>> +}
>> +
>> +void arch_rseq_handle_notify_resume(struct pt_regs *regs);
>> +void arch_rseq_check_critical_section(struct task_struct *p,
>> +				      struct pt_regs *regs);
>> +
>> +#else /* !CONFIG_RESTARTABLE_SEQUENCES */
>> +
>> +static inline void arch_rseq_handle_notify_resume(struct pt_regs *regs) {}
>> +static inline void arch_rseq_check_critical_section(struct task_struct *p,
>> +						    struct pt_regs *regs) {}
>> +
>> +#endif
>> +
>> +#endif /* _ASM_X86_RESTARTABLE_SEQUENCES_H */
>> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
>> index febaf18..bd7827d 100644
>> --- a/arch/x86/kernel/Makefile
>> +++ b/arch/x86/kernel/Makefile
>> @@ -113,6 +113,8 @@ obj-$(CONFIG_TRACING)			+= tracepoint.o
>> obj-$(CONFIG_IOSF_MBI)			+= iosf_mbi.o
>> obj-$(CONFIG_PMC_ATOM)			+= pmc_atom.o
>> 
>> +obj-$(CONFIG_RESTARTABLE_SEQUENCES)	+= restartable_sequences.o
>> +
>> ###
>> # 64 bit specific files
>> ifeq ($(CONFIG_X86_64),y)
>> diff --git a/arch/x86/kernel/restartable_sequences.c
>> b/arch/x86/kernel/restartable_sequences.c
>> new file mode 100644
>> index 0000000..3b38013
>> --- /dev/null
>> +++ b/arch/x86/kernel/restartable_sequences.c
>> @@ -0,0 +1,69 @@
>> +/*
>> + * Restartable Sequences: x86 ABI.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
>> + *
>> + * Copyright (C) 2015, Google, Inc.,
>> + * Paul Turner <pjt-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> and Andrew Hunter <ahh-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>> + *
>> + */
>> +
>> +#include <linux/sched.h>
>> +#include <linux/uaccess.h>
>> +#include <asm/restartable_sequences.h>
>> +
>> +void arch_rseq_check_critical_section(struct task_struct *p,
>> +				      struct pt_regs *regs)
>> +{
>> +	if (!arch_rseq_in_crit_section(p, regs))
>> +		return;
>> +
>> +	/* RSEQ only applies to user-mode execution */
>> +	BUG_ON(!user_mode(regs));
>> +
>> +	/*
>> +	 * The ABI is slightly different for {32,64}-bit threads on x86
>> +	 *
>> +	 * Short version:
>> +	 *   x86-64 (or x32): interrupted rip => %r10
>> +	 *   i386:            interrupted rip => %ecx
>> +	 *
>> +	 * Longer version:
>> +	 * The scratch registers available under the i386 function call ABI
>> +	 * overlap with those used by argument registers under the x86_64 ABI.
>> +	 *
>> +	 * Given that the sequence block is already personality specific in
>> +	 * that it must be entered by 'call' and that we always want the
>> +	 * arguments available for a sequence restart; it's more natural to
>> +	 * differentiate the ABI in these two cases.
>> +	 */
>> +	if (unlikely(test_tsk_thread_flag(p, TIF_IA32)))
>> +		regs->cx = regs->ip; /* i386 */
>> +	else
>> +		regs->r10 = regs->ip; /* x86-64/x32 */
>> +
>> +	regs->ip = (unsigned long)p->group_leader->rseq_state.crit_restart;
>> +}
>> +
>> +void arch_rseq_handle_notify_resume(struct pt_regs *regs)
>> +{
>> +	struct restartable_sequence_state *rseq_state = &current->rseq_state;
>> +
>> +	/* If this update fails our user-state is incoherent. */
>> +	if (put_user(task_cpu(current), rseq_state->cpu_pointer))
>> +		force_sig(SIGSEGV, current);
>> +
>> +	arch_rseq_check_critical_section(current, regs);
>> +}
>> diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
>> index 206996c..987c50b 100644
>> --- a/arch/x86/kernel/signal.c
>> +++ b/arch/x86/kernel/signal.c
>> @@ -31,6 +31,7 @@
>> #include <asm/vdso.h>
>> #include <asm/mce.h>
>> #include <asm/sighandling.h>
>> +#include <asm/restartable_sequences.h>
>> 
>> #ifdef CONFIG_X86_64
>> #include <asm/proto.h>
>> @@ -617,6 +618,15 @@ setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs)
>> 	sigset_t *set = sigmask_to_save();
>> 	compat_sigset_t *cset = (compat_sigset_t *) set;
>> 
>> +	/*
>> +	 * If we are executing in the critical section of a restartable
>> +	 * sequence we need to fix up the user's stack saved ip at this point
>> +	 * so that signal handler return does not allow us to jump back into
>> +	 * the block across a context switch boundary.
>> +	 */
>> +	if (rseq_active(current))
>> +		arch_rseq_check_critical_section(current, regs);
>> +
>> 	/* Set up the stack frame */
>> 	if (is_ia32_frame()) {
>> 		if (ksig->ka.sa.sa_flags & SA_SIGINFO)
>> @@ -755,6 +765,8 @@ do_notify_resume(struct pt_regs *regs, void *unused, __u32
>> thread_info_flags)
>> 	if (thread_info_flags & _TIF_NOTIFY_RESUME) {
>> 		clear_thread_flag(TIF_NOTIFY_RESUME);
>> 		tracehook_notify_resume(regs);
>> +		if (rseq_active(current))
>> +			arch_rseq_handle_notify_resume(regs);
>> 	}
>> 	if (thread_info_flags & _TIF_USER_RETURN_NOTIFY)
>> 		fire_user_return_notifiers();
>> diff --git a/kernel/restartable_sequences.c b/kernel/restartable_sequences.c
>> index 72945f2..9102241 100644
>> --- a/kernel/restartable_sequences.c
>> +++ b/kernel/restartable_sequences.c
>> @@ -24,17 +24,22 @@
>> 
>> #ifdef CONFIG_RESTARTABLE_SEQUENCES
>> 
>> +#include <asm/restartable_sequences.h>
>> #include <linux/uaccess.h>
>> #include <linux/preempt.h>
>> #include <linux/syscalls.h>
>> 
>> static void rseq_sched_in_nop(struct preempt_notifier *pn, int cpu) {}
>> -static void rseq_sched_out_nop(struct preempt_notifier *pn,
>> -			       struct task_struct *next) {}
>> +static void rseq_sched_out(struct preempt_notifier *pn,
>> +			   struct task_struct *next)
>> +{
>> +	if (arch_rseq_needs_notify_resume(current))
>> +		set_thread_flag(TIF_NOTIFY_RESUME);
>> +}
>> 
>> static __read_mostly struct preempt_ops rseq_preempt_ops = {
>> 	.sched_in = rseq_sched_in_nop,
>> -	.sched_out = rseq_sched_out_nop,
>> +	.sched_out = rseq_sched_out,
>> };
>> 
>>  int rseq_register_cpu_pointer_current(int __user *cpu_pointer)
> 
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply

* Re: [RFC PATCH 2/3] restartable sequences: x86 ABI
From: Mathieu Desnoyers @ 2015-06-26 18:09 UTC (permalink / raw)
  To: Paul Turner
  Cc: Peter Zijlstra, Paul E. McKenney, Andrew Hunter, Andi Kleen,
	Lai Jiangshan, linux-api, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	rostedt, Josh Triplett, Ingo Molnar, Andrew Morton,
	Andy Lutomirski, Linus Torvalds, Chris Lameter
In-Reply-To: <20150624222609.6116.30992.stgit-tdHu5vqousHHt/MElyovVYaSKrA+ACpX0E9HWUfgJXw@public.gmane.org>

----- On Jun 24, 2015, at 6:26 PM, Paul Turner pjt-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org wrote:

> Implements the x86 (i386 & x86-64) ABIs for interrupting and restarting
> execution within restartable sequence sections.
> 
> With respect to the x86-specific ABI:
>  On 32-bit:           Upon restart, the interrupted rip is placed in %ecx
>  On 64-bit (or x32):  Upon restart, the interrupted rip is placed in %r10
> 
> While potentially surprising at first glance, this choice is strongly motivated
> by the fact that the available scratch registers under the i386 function call
> ABI overlap with those used as argument registers under x86_64.
> 
> Given that sequences are already personality specific and that we always want
> the arguments to be available for sequence restart, it's much more natural to
> ultimately differentiate the ABI in these two cases.
> 
> Signed-off-by: Paul Turner <pjt-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> ---
> arch/x86/include/asm/restartable_sequences.h |   50 +++++++++++++++++++
> arch/x86/kernel/Makefile                     |    2 +
> arch/x86/kernel/restartable_sequences.c      |   69 ++++++++++++++++++++++++++
> arch/x86/kernel/signal.c                     |   12 +++++
> kernel/restartable_sequences.c               |   11 +++-
> 5 files changed, 141 insertions(+), 3 deletions(-)
> create mode 100644 arch/x86/include/asm/restartable_sequences.h
> create mode 100644 arch/x86/kernel/restartable_sequences.c
> 
> diff --git a/arch/x86/include/asm/restartable_sequences.h
> b/arch/x86/include/asm/restartable_sequences.h
> new file mode 100644
> index 0000000..0ceb024
> --- /dev/null
> +++ b/arch/x86/include/asm/restartable_sequences.h
> @@ -0,0 +1,50 @@
> +#ifndef _ASM_X86_RESTARTABLE_SEQUENCES_H
> +#define _ASM_X86_RESTARTABLE_SEQUENCES_H
> +
> +#include <asm/processor.h>
> +#include <asm/ptrace.h>
> +#include <linux/sched.h>
> +
> +#ifdef CONFIG_RESTARTABLE_SEQUENCES
> +
> +static inline bool arch_rseq_in_crit_section(struct task_struct *p,
> +					     struct pt_regs *regs)
> +{
> +	struct task_struct *leader = p->group_leader;
> +	struct restartable_sequence_state *rseq_state = &leader->rseq_state;
> +
> +	unsigned long ip = (unsigned long)regs->ip;
> +	if (unlikely(ip < (unsigned long)rseq_state->crit_end &&
> +		     ip >= (unsigned long)rseq_state->crit_start))
> +		return true;
> +
> +	return false;
> +}
> +
> +static inline bool arch_rseq_needs_notify_resume(struct task_struct *p)
> +{
> +#ifdef CONFIG_PREEMPT
> +	/*
> +	 * Under CONFIG_PREEMPT it's possible for regs to be incoherent in the
> +	 * case that we took an interrupt during syscall entry.  Avoid this by
> +	 * always deferring to our notify-resume handler.
> +	 */
> +	return true;

I'm a bit puzzled about this. If I look at perf_get_regs_user() in the perf
code, task_pt_regs() seems to return the user-space pt_regs for a task with
a current->mm set (iow, not a kernel thread), even if an interrupt nests on
top of a system call. The only corner-case is NMIs, where an NMI may interrupt
in the middle of setting up the task pt_regs, but scheduling should never happen
there, right ?

Since it's impossible for kernel threads to have a rseq critical section,
we should be able to assume that every time task_pt_regs() returns a
non-userspace (user_mode(regs) != 0) pt_regs implies that scheduling applies
to a kernel thread. Therefore, following this line of thoughts,
arch_rseq_in_crit_section() should work for CONFIG_PREEMPT kernels too.

So what I am missing here ?

Thanks,

Mathieu

> +#else
> +	return arch_rseq_in_crit_section(p, task_pt_regs(p));
> +#endif
> +}
> +
> +void arch_rseq_handle_notify_resume(struct pt_regs *regs);
> +void arch_rseq_check_critical_section(struct task_struct *p,
> +				      struct pt_regs *regs);
> +
> +#else /* !CONFIG_RESTARTABLE_SEQUENCES */
> +
> +static inline void arch_rseq_handle_notify_resume(struct pt_regs *regs) {}
> +static inline void arch_rseq_check_critical_section(struct task_struct *p,
> +						    struct pt_regs *regs) {}
> +
> +#endif
> +
> +#endif /* _ASM_X86_RESTARTABLE_SEQUENCES_H */
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index febaf18..bd7827d 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -113,6 +113,8 @@ obj-$(CONFIG_TRACING)			+= tracepoint.o
> obj-$(CONFIG_IOSF_MBI)			+= iosf_mbi.o
> obj-$(CONFIG_PMC_ATOM)			+= pmc_atom.o
> 
> +obj-$(CONFIG_RESTARTABLE_SEQUENCES)	+= restartable_sequences.o
> +
> ###
> # 64 bit specific files
> ifeq ($(CONFIG_X86_64),y)
> diff --git a/arch/x86/kernel/restartable_sequences.c
> b/arch/x86/kernel/restartable_sequences.c
> new file mode 100644
> index 0000000..3b38013
> --- /dev/null
> +++ b/arch/x86/kernel/restartable_sequences.c
> @@ -0,0 +1,69 @@
> +/*
> + * Restartable Sequences: x86 ABI.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
> + *
> + * Copyright (C) 2015, Google, Inc.,
> + * Paul Turner <pjt-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> and Andrew Hunter <ahh-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> + *
> + */
> +
> +#include <linux/sched.h>
> +#include <linux/uaccess.h>
> +#include <asm/restartable_sequences.h>
> +
> +void arch_rseq_check_critical_section(struct task_struct *p,
> +				      struct pt_regs *regs)
> +{
> +	if (!arch_rseq_in_crit_section(p, regs))
> +		return;
> +
> +	/* RSEQ only applies to user-mode execution */
> +	BUG_ON(!user_mode(regs));
> +
> +	/*
> +	 * The ABI is slightly different for {32,64}-bit threads on x86
> +	 *
> +	 * Short version:
> +	 *   x86-64 (or x32): interrupted rip => %r10
> +	 *   i386:            interrupted rip => %ecx
> +	 *
> +	 * Longer version:
> +	 * The scratch registers available under the i386 function call ABI
> +	 * overlap with those used by argument registers under the x86_64 ABI.
> +	 *
> +	 * Given that the sequence block is already personality specific in
> +	 * that it must be entered by 'call' and that we always want the
> +	 * arguments available for a sequence restart; it's more natural to
> +	 * differentiate the ABI in these two cases.
> +	 */
> +	if (unlikely(test_tsk_thread_flag(p, TIF_IA32)))
> +		regs->cx = regs->ip; /* i386 */
> +	else
> +		regs->r10 = regs->ip; /* x86-64/x32 */
> +
> +	regs->ip = (unsigned long)p->group_leader->rseq_state.crit_restart;
> +}
> +
> +void arch_rseq_handle_notify_resume(struct pt_regs *regs)
> +{
> +	struct restartable_sequence_state *rseq_state = &current->rseq_state;
> +
> +	/* If this update fails our user-state is incoherent. */
> +	if (put_user(task_cpu(current), rseq_state->cpu_pointer))
> +		force_sig(SIGSEGV, current);
> +
> +	arch_rseq_check_critical_section(current, regs);
> +}
> diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
> index 206996c..987c50b 100644
> --- a/arch/x86/kernel/signal.c
> +++ b/arch/x86/kernel/signal.c
> @@ -31,6 +31,7 @@
> #include <asm/vdso.h>
> #include <asm/mce.h>
> #include <asm/sighandling.h>
> +#include <asm/restartable_sequences.h>
> 
> #ifdef CONFIG_X86_64
> #include <asm/proto.h>
> @@ -617,6 +618,15 @@ setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs)
> 	sigset_t *set = sigmask_to_save();
> 	compat_sigset_t *cset = (compat_sigset_t *) set;
> 
> +	/*
> +	 * If we are executing in the critical section of a restartable
> +	 * sequence we need to fix up the user's stack saved ip at this point
> +	 * so that signal handler return does not allow us to jump back into
> +	 * the block across a context switch boundary.
> +	 */
> +	if (rseq_active(current))
> +		arch_rseq_check_critical_section(current, regs);
> +
> 	/* Set up the stack frame */
> 	if (is_ia32_frame()) {
> 		if (ksig->ka.sa.sa_flags & SA_SIGINFO)
> @@ -755,6 +765,8 @@ do_notify_resume(struct pt_regs *regs, void *unused, __u32
> thread_info_flags)
> 	if (thread_info_flags & _TIF_NOTIFY_RESUME) {
> 		clear_thread_flag(TIF_NOTIFY_RESUME);
> 		tracehook_notify_resume(regs);
> +		if (rseq_active(current))
> +			arch_rseq_handle_notify_resume(regs);
> 	}
> 	if (thread_info_flags & _TIF_USER_RETURN_NOTIFY)
> 		fire_user_return_notifiers();
> diff --git a/kernel/restartable_sequences.c b/kernel/restartable_sequences.c
> index 72945f2..9102241 100644
> --- a/kernel/restartable_sequences.c
> +++ b/kernel/restartable_sequences.c
> @@ -24,17 +24,22 @@
> 
> #ifdef CONFIG_RESTARTABLE_SEQUENCES
> 
> +#include <asm/restartable_sequences.h>
> #include <linux/uaccess.h>
> #include <linux/preempt.h>
> #include <linux/syscalls.h>
> 
> static void rseq_sched_in_nop(struct preempt_notifier *pn, int cpu) {}
> -static void rseq_sched_out_nop(struct preempt_notifier *pn,
> -			       struct task_struct *next) {}
> +static void rseq_sched_out(struct preempt_notifier *pn,
> +			   struct task_struct *next)
> +{
> +	if (arch_rseq_needs_notify_resume(current))
> +		set_thread_flag(TIF_NOTIFY_RESUME);
> +}
> 
> static __read_mostly struct preempt_ops rseq_preempt_ops = {
> 	.sched_in = rseq_sched_in_nop,
> -	.sched_out = rseq_sched_out_nop,
> +	.sched_out = rseq_sched_out,
> };
> 
>  int rseq_register_cpu_pointer_current(int __user *cpu_pointer)

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply

* [RFCv2 5/5] mm: remove direct calling of migration
From: Gioh Kim @ 2015-06-26  9:58 UTC (permalink / raw)
  To: jlayton-vpEMnDpepFuMZCB2o+C8xQ, bfields-uC3wQj2KruNg9hUCZPvPmw,
	vbabka-AlSwsSmVLrQ, iamjoonsoo.kim-Hm3cg6mZ9cc,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, mst-H+wXaHxf7aLQT0dZR+AlfA,
	koct9i-Re5JQEeQqe8AvxtiuMwx3w, minchan-DgEjT+Ai2ygdnm+yROfE0A,
	aquini-H+wXaHxf7aLQT0dZR+AlfA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg
  Cc: akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, Gioh Kim
In-Reply-To: <1435312710-15108-1-git-send-email-gioh.kim-Hm3cg6mZ9cc@public.gmane.org>

Migration is completely generalized.

Signed-off-by: Gioh Kim <gioh.kim-Hm3cg6mZ9cc@public.gmane.org>
---
 mm/balloon_compaction.c |  8 --------
 mm/migrate.c            | 15 ---------------
 2 files changed, 23 deletions(-)

diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
index df72846..a7b7c79 100644
--- a/mm/balloon_compaction.c
+++ b/mm/balloon_compaction.c
@@ -206,13 +206,6 @@ int balloon_page_migrate(struct address_space *mapping,
 	if (!isolated_balloon_page(page))
 		return rc;
 
-	/*
-	 * Block others from accessing the 'newpage' when we get around to
-	 * establishing additional references. We should be the only one
-	 * holding a reference to the 'newpage' at this point.
-	 */
-	BUG_ON(!trylock_page(newpage));
-
 	if (WARN_ON(!__is_movable_balloon_page(page))) {
 		dump_page(page, "not movable balloon page");
 		unlock_page(newpage);
@@ -222,7 +215,6 @@ int balloon_page_migrate(struct address_space *mapping,
 	if (balloon && balloon->migratepage)
 		rc = balloon->migratepage(balloon, newpage, page, mode);
 
-	unlock_page(newpage);
 	return rc;
 }
 
diff --git a/mm/migrate.c b/mm/migrate.c
index a0bc1e4..0b52fa4 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -844,21 +844,6 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
 		}
 	}
 
-	if (unlikely(driver_page_migratable(page))) {
-		/*
-		 * A driver page does not need any special attention from
-		 * physical to virtual reverse mapping procedures.
-		 * Skip any attempt to unmap PTEs or to remap swap cache,
-		 * in order to avoid burning cycles at rmap level, and perform
-		 * the page migration right away (proteced by page lock).
-		 */
-		rc = page->mapping->a_ops->migratepage(page->mapping,
-						       newpage,
-						       page,
-						       mode);
-		goto out_unlock;
-	}
-
 	/*
 	 * Corner case handling:
 	 * 1. When a new swap-cache page is read into, it is added to the LRU
-- 
1.9.1

^ permalink raw reply related

* [RFCv2 4/5] mm/compaction: compaction calls generic migration
From: Gioh Kim @ 2015-06-26  9:58 UTC (permalink / raw)
  To: jlayton, bfields, vbabka, iamjoonsoo.kim, viro, mst, koct9i,
	minchan, aquini, linux-fsdevel, virtualization, linux-kernel,
	linux-api, linux-mm
  Cc: akpm, Gioh Kim
In-Reply-To: <1435312710-15108-1-git-send-email-gioh.kim@lge.com>

Compaction calls interfaces of driver page migration
instead of calling balloon migration directly.

Signed-off-by: Gioh Kim <gioh.kim@lge.com>
---
 drivers/virtio/virtio_balloon.c |  1 +
 mm/compaction.c                 |  9 +++++----
 mm/migrate.c                    | 21 ++++++++++++---------
 3 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index c49b553..5e5cbea 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -30,6 +30,7 @@
 #include <linux/balloon_compaction.h>
 #include <linux/oom.h>
 #include <linux/wait.h>
+#include <linux/anon_inodes.h>
 
 /*
  * Balloon device works in 4K page units.  So each page is pointed to by
diff --git a/mm/compaction.c b/mm/compaction.c
index 16e1b57..cc5ec81 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -14,7 +14,7 @@
 #include <linux/backing-dev.h>
 #include <linux/sysctl.h>
 #include <linux/sysfs.h>
-#include <linux/balloon_compaction.h>
+#include <linux/compaction.h>
 #include <linux/page-isolation.h>
 #include <linux/kasan.h>
 #include "internal.h"
@@ -714,12 +714,13 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
 
 		/*
 		 * Check may be lockless but that's ok as we recheck later.
-		 * It's possible to migrate LRU pages and balloon pages
+		 * It's possible to migrate LRU pages and driver pages
 		 * Skip any other type of page
 		 */
 		if (!PageLRU(page)) {
-			if (unlikely(balloon_page_movable(page))) {
-				if (balloon_page_isolate(page)) {
+			if (unlikely(driver_page_migratable(page))) {
+				if (page->mapping->a_ops->isolatepage(page,
+								isolate_mode)) {
 					/* Successfully isolated */
 					goto isolate_success;
 				}
diff --git a/mm/migrate.c b/mm/migrate.c
index 236ee25..a0bc1e4 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -35,7 +35,7 @@
 #include <linux/hugetlb.h>
 #include <linux/hugetlb_cgroup.h>
 #include <linux/gfp.h>
-#include <linux/balloon_compaction.h>
+#include <linux/compaction.h>
 #include <linux/mmu_notifier.h>
 
 #include <asm/tlbflush.h>
@@ -76,7 +76,7 @@ int migrate_prep_local(void)
  * from where they were once taken off for compaction/migration.
  *
  * This function shall be used whenever the isolated pageset has been
- * built from lru, balloon, hugetlbfs page. See isolate_migratepages_range()
+ * built from lru, driver, hugetlbfs page. See isolate_migratepages_range()
  * and isolate_huge_page().
  */
 void putback_movable_pages(struct list_head *l)
@@ -92,8 +92,8 @@ void putback_movable_pages(struct list_head *l)
 		list_del(&page->lru);
 		dec_zone_page_state(page, NR_ISOLATED_ANON +
 				page_is_file_cache(page));
-		if (unlikely(isolated_balloon_page(page)))
-			balloon_page_putback(page);
+		if (unlikely(driver_page_migratable(page)))
+			page->mapping->a_ops->putbackpage(page);
 		else
 			putback_lru_page(page);
 	}
@@ -844,15 +844,18 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
 		}
 	}
 
-	if (unlikely(isolated_balloon_page(page))) {
+	if (unlikely(driver_page_migratable(page))) {
 		/*
-		 * A ballooned page does not need any special attention from
+		 * A driver page does not need any special attention from
 		 * physical to virtual reverse mapping procedures.
 		 * Skip any attempt to unmap PTEs or to remap swap cache,
 		 * in order to avoid burning cycles at rmap level, and perform
 		 * the page migration right away (proteced by page lock).
 		 */
-		rc = balloon_page_migrate(newpage, page, mode);
+		rc = page->mapping->a_ops->migratepage(page->mapping,
+						       newpage,
+						       page,
+						       mode);
 		goto out_unlock;
 	}
 
@@ -962,8 +965,8 @@ out:
 	if (rc != MIGRATEPAGE_SUCCESS && put_new_page) {
 		ClearPageSwapBacked(newpage);
 		put_new_page(newpage, private);
-	} else if (unlikely(__is_movable_balloon_page(newpage))) {
-		/* drop our reference, page already in the balloon */
+	} else if (unlikely(driver_page_migratable(newpage))) {
+		/* drop our reference */
 		put_page(newpage);
 	} else
 		putback_lru_page(newpage);
-- 
1.9.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related

* [RFCv2 3/5] mm/balloon: apply driver page migratable into balloon
From: Gioh Kim @ 2015-06-26  9:58 UTC (permalink / raw)
  To: jlayton, bfields, vbabka, iamjoonsoo.kim, viro, mst, koct9i,
	minchan, aquini, linux-fsdevel, virtualization, linux-kernel,
	linux-api, linux-mm
  Cc: akpm, Gioh Kim
In-Reply-To: <1435312710-15108-1-git-send-email-gioh.kim@lge.com>

Apply driver page migration into balloon driver.

Signed-off-by: Gioh Kim <gioh.kim@lge.com>
---
 drivers/virtio/virtio_balloon.c        |  3 +++
 fs/proc/page.c                         |  3 +++
 include/linux/balloon_compaction.h     | 33 +++++++++++++++++++++------------
 include/uapi/linux/kernel-page-flags.h |  2 +-
 mm/balloon_compaction.c                | 19 +++++++++++++++++--
 5 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 82e80e0..c49b553 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -505,6 +505,9 @@ static int virtballoon_probe(struct virtio_device *vdev)
 	balloon_devinfo_init(&vb->vb_dev_info);
 #ifdef CONFIG_BALLOON_COMPACTION
 	vb->vb_dev_info.migratepage = virtballoon_migratepage;
+	vb->vb_dev_info.inode = anon_inode_new();
+	vb->vb_dev_info.inode->i_mapping->a_ops = &balloon_aops;
+	mapping_set_migratable(vb->vb_dev_info.inode->i_mapping);
 #endif
 
 	err = init_vqs(vb);
diff --git a/fs/proc/page.c b/fs/proc/page.c
index 7eee2d8..2dc3673 100644
--- a/fs/proc/page.c
+++ b/fs/proc/page.c
@@ -146,6 +146,9 @@ u64 stable_page_flags(struct page *page)
 	if (PageBalloon(page))
 		u |= 1 << KPF_BALLOON;
 
+	if (PageMigratable(page))
+		u |= 1 << KPF_MIGRATABLE;
+
 	u |= kpf_copy_bit(k, KPF_LOCKED,	PG_locked);
 
 	u |= kpf_copy_bit(k, KPF_SLAB,		PG_slab);
diff --git a/include/linux/balloon_compaction.h b/include/linux/balloon_compaction.h
index 9b0a15d..e8a3670 100644
--- a/include/linux/balloon_compaction.h
+++ b/include/linux/balloon_compaction.h
@@ -48,6 +48,7 @@
 #include <linux/migrate.h>
 #include <linux/gfp.h>
 #include <linux/err.h>
+#include <linux/fs.h>
 
 /*
  * Balloon device information descriptor.
@@ -62,6 +63,7 @@ struct balloon_dev_info {
 	struct list_head pages;		/* Pages enqueued & handled to Host */
 	int (*migratepage)(struct balloon_dev_info *, struct page *newpage,
 			struct page *page, enum migrate_mode mode);
+	struct inode *inode;
 };
 
 extern struct page *balloon_page_enqueue(struct balloon_dev_info *b_dev_info);
@@ -73,24 +75,28 @@ static inline void balloon_devinfo_init(struct balloon_dev_info *balloon)
 	spin_lock_init(&balloon->pages_lock);
 	INIT_LIST_HEAD(&balloon->pages);
 	balloon->migratepage = NULL;
+	balloon->inode = NULL;
 }
 
 #ifdef CONFIG_BALLOON_COMPACTION
-extern bool balloon_page_isolate(struct page *page);
+extern const struct address_space_operations balloon_aops;
+extern bool balloon_page_isolate(struct page *page,
+				 isolate_mode_t mode);
 extern void balloon_page_putback(struct page *page);
-extern int balloon_page_migrate(struct page *newpage,
+extern int balloon_page_migrate(struct address_space *mapping,
+				struct page *newpage,
 				struct page *page, enum migrate_mode mode);
 
 /*
- * __is_movable_balloon_page - helper to perform @page PageBalloon tests
+ * __is_movable_balloon_page - helper to perform @page PageMigratable tests
  */
 static inline bool __is_movable_balloon_page(struct page *page)
 {
-	return PageBalloon(page);
+	return PageMigratable(page);
 }
 
 /*
- * balloon_page_movable - test PageBalloon to identify balloon pages
+ * balloon_page_movable - test PageMigratable to identify balloon pages
  *			  and PagePrivate to check that the page is not
  *			  isolated and can be moved by compaction/migration.
  *
@@ -99,7 +105,7 @@ static inline bool __is_movable_balloon_page(struct page *page)
  */
 static inline bool balloon_page_movable(struct page *page)
 {
-	return PageBalloon(page) && PagePrivate(page);
+	return PageMigratable(page) && PagePrivate(page);
 }
 
 /*
@@ -108,7 +114,7 @@ static inline bool balloon_page_movable(struct page *page)
  */
 static inline bool isolated_balloon_page(struct page *page)
 {
-	return PageBalloon(page);
+	return PageMigratable(page);
 }
 
 /*
@@ -123,7 +129,8 @@ static inline bool isolated_balloon_page(struct page *page)
 static inline void balloon_page_insert(struct balloon_dev_info *balloon,
 				       struct page *page)
 {
-	__SetPageBalloon(page);
+	page->mapping = balloon->inode->i_mapping;
+	__SetPageMigratable(page);
 	SetPagePrivate(page);
 	set_page_private(page, (unsigned long)balloon);
 	list_add(&page->lru, &balloon->pages);
@@ -139,7 +146,8 @@ static inline void balloon_page_insert(struct balloon_dev_info *balloon,
  */
 static inline void balloon_page_delete(struct page *page)
 {
-	__ClearPageBalloon(page);
+	page->mapping = NULL;
+	__ClearPageMigratable(page);
 	set_page_private(page, 0);
 	if (PagePrivate(page)) {
 		ClearPagePrivate(page);
@@ -166,13 +174,13 @@ static inline gfp_t balloon_mapping_gfp_mask(void)
 static inline void balloon_page_insert(struct balloon_dev_info *balloon,
 				       struct page *page)
 {
-	__SetPageBalloon(page);
+	__SetPageMigratable(page);
 	list_add(&page->lru, &balloon->pages);
 }
 
 static inline void balloon_page_delete(struct page *page)
 {
-	__ClearPageBalloon(page);
+	__ClearPageMigratable(page);
 	list_del(&page->lru);
 }
 
@@ -191,7 +199,8 @@ static inline bool isolated_balloon_page(struct page *page)
 	return false;
 }
 
-static inline bool balloon_page_isolate(struct page *page)
+static inline bool balloon_page_isolate(struct page *page,
+					isolate_mode_t mode)
 {
 	return false;
 }
diff --git a/include/uapi/linux/kernel-page-flags.h b/include/uapi/linux/kernel-page-flags.h
index a6c4962..65db3a6 100644
--- a/include/uapi/linux/kernel-page-flags.h
+++ b/include/uapi/linux/kernel-page-flags.h
@@ -33,6 +33,6 @@
 #define KPF_THP			22
 #define KPF_BALLOON		23
 #define KPF_ZERO_PAGE		24
-
+#define KPF_MIGRATABLE		25
 
 #endif /* _UAPILINUX_KERNEL_PAGE_FLAGS_H */
diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
index fcad832..df72846 100644
--- a/mm/balloon_compaction.c
+++ b/mm/balloon_compaction.c
@@ -131,7 +131,7 @@ static inline void __putback_balloon_page(struct page *page)
 }
 
 /* __isolate_lru_page() counterpart for a ballooned page */
-bool balloon_page_isolate(struct page *page)
+bool balloon_page_isolate(struct page *page, isolate_mode_t mode)
 {
 	/*
 	 * Avoid burning cycles with pages that are yet under __free_pages(),
@@ -175,6 +175,9 @@ bool balloon_page_isolate(struct page *page)
 /* putback_lru_page() counterpart for a ballooned page */
 void balloon_page_putback(struct page *page)
 {
+	if (!isolated_balloon_page(page))
+		return;
+
 	/*
 	 * 'lock_page()' stabilizes the page and prevents races against
 	 * concurrent isolation threads attempting to re-isolate it.
@@ -193,12 +196,16 @@ void balloon_page_putback(struct page *page)
 }
 
 /* move_to_new_page() counterpart for a ballooned page */
-int balloon_page_migrate(struct page *newpage,
+int balloon_page_migrate(struct address_space *mapping,
+			 struct page *newpage,
 			 struct page *page, enum migrate_mode mode)
 {
 	struct balloon_dev_info *balloon = balloon_page_device(page);
 	int rc = -EAGAIN;
 
+	if (!isolated_balloon_page(page))
+		return rc;
+
 	/*
 	 * Block others from accessing the 'newpage' when we get around to
 	 * establishing additional references. We should be the only one
@@ -218,4 +225,12 @@ int balloon_page_migrate(struct page *newpage,
 	unlock_page(newpage);
 	return rc;
 }
+
+/* define the balloon_mapping->a_ops callback to allow balloon page migration */
+const struct address_space_operations balloon_aops = {
+	.migratepage = balloon_page_migrate,
+	.isolatepage = balloon_page_isolate,
+	.putbackpage = balloon_page_putback,
+};
+EXPORT_SYMBOL_GPL(balloon_aops);
 #endif /* CONFIG_BALLOON_COMPACTION */
-- 
1.9.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related

* [RFCv2 2/5] fs/anon_inodes: get a new inode
From: Gioh Kim @ 2015-06-26  9:58 UTC (permalink / raw)
  To: jlayton, bfields, vbabka, iamjoonsoo.kim, viro, mst, koct9i,
	minchan, aquini, linux-fsdevel, virtualization, linux-kernel,
	linux-api, linux-mm
  Cc: akpm, Gioh Kim
In-Reply-To: <1435312710-15108-1-git-send-email-gioh.kim@lge.com>

A inode is necessary for some drivers that needs special address_space
and address_space_operation for page migration. Each drivers can create
inode with the anon_inodefs.

Signed-off-by: Gioh Kim <gioh.kim@lge.com>
---
 fs/anon_inodes.c            | 6 ++++++
 include/linux/anon_inodes.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index 80ef38c..1d51f96 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -162,6 +162,12 @@ err_put_unused_fd:
 }
 EXPORT_SYMBOL_GPL(anon_inode_getfd);
 
+struct inode *anon_inode_new(void)
+{
+	return alloc_anon_inode(anon_inode_mnt->mnt_sb);
+}
+EXPORT_SYMBOL_GPL(anon_inode_new);
+
 static int __init anon_inode_init(void)
 {
 	anon_inode_mnt = kern_mount(&anon_inode_fs_type);
diff --git a/include/linux/anon_inodes.h b/include/linux/anon_inodes.h
index 8013a45..ddbd67f 100644
--- a/include/linux/anon_inodes.h
+++ b/include/linux/anon_inodes.h
@@ -15,6 +15,7 @@ struct file *anon_inode_getfile(const char *name,
 				void *priv, int flags);
 int anon_inode_getfd(const char *name, const struct file_operations *fops,
 		     void *priv, int flags);
+struct inode *anon_inode_new(void);
 
 #endif /* _LINUX_ANON_INODES_H */
 
-- 
1.9.1

^ permalink raw reply related

* [RFCv2 1/5] mm/compaction: enable driver page migration
From: Gioh Kim @ 2015-06-26  9:58 UTC (permalink / raw)
  To: jlayton-vpEMnDpepFuMZCB2o+C8xQ, bfields-uC3wQj2KruNg9hUCZPvPmw,
	vbabka-AlSwsSmVLrQ, iamjoonsoo.kim-Hm3cg6mZ9cc,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, mst-H+wXaHxf7aLQT0dZR+AlfA,
	koct9i-Re5JQEeQqe8AvxtiuMwx3w, minchan-DgEjT+Ai2ygdnm+yROfE0A,
	aquini-H+wXaHxf7aLQT0dZR+AlfA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg
  Cc: akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, Gioh Kim
In-Reply-To: <1435312710-15108-1-git-send-email-gioh.kim-Hm3cg6mZ9cc@public.gmane.org>

Add framework to register callback functions and
check pages migratable.
There are some modes of page isolation so that isolate interface
has an arguments of page address and isolation mode.

Signed-off-by: Gioh Kim <gioh.kim-Hm3cg6mZ9cc@public.gmane.org>
---
 include/linux/compaction.h | 11 +++++++++++
 include/linux/fs.h         |  2 ++
 include/linux/page-flags.h | 19 +++++++++++++++++++
 include/linux/pagemap.h    | 27 +++++++++++++++++++++++++++
 4 files changed, 59 insertions(+)

diff --git a/include/linux/compaction.h b/include/linux/compaction.h
index aa8f61c..4e91a07 100644
--- a/include/linux/compaction.h
+++ b/include/linux/compaction.h
@@ -1,6 +1,9 @@
 #ifndef _LINUX_COMPACTION_H
 #define _LINUX_COMPACTION_H
 
+#include <linux/pagemap.h>
+#include <linux/mm.h>
+
 /* Return values for compact_zone() and try_to_compact_pages() */
 /* compaction didn't start as it was deferred due to past failures */
 #define COMPACT_DEFERRED	0
@@ -51,6 +54,10 @@ extern void compaction_defer_reset(struct zone *zone, int order,
 				bool alloc_success);
 extern bool compaction_restarting(struct zone *zone, int order);
 
+static inline bool driver_page_migratable(struct page *page)
+{
+	return PageMigratable(page) && mapping_migratable(page->mapping);
+}
 #else
 static inline unsigned long try_to_compact_pages(gfp_t gfp_mask,
 			unsigned int order, int alloc_flags,
@@ -83,6 +90,10 @@ static inline bool compaction_deferred(struct zone *zone, int order)
 	return true;
 }
 
+static inline bool driver_page_migratable(struct page *page)
+{
+	return false
+}
 #endif /* CONFIG_COMPACTION */
 
 #if defined(CONFIG_COMPACTION) && defined(CONFIG_SYSFS) && defined(CONFIG_NUMA)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index a0653e5..2cc4b24 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -396,6 +396,8 @@ struct address_space_operations {
 	 */
 	int (*migratepage) (struct address_space *,
 			struct page *, struct page *, enum migrate_mode);
+	bool (*isolatepage) (struct page *, isolate_mode_t);
+	void (*putbackpage) (struct page *);
 	int (*launder_page) (struct page *);
 	int (*is_partially_uptodate) (struct page *, unsigned long,
 					unsigned long);
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 91b7f9b..c8a66de 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -649,6 +649,25 @@ static inline void __ClearPageBalloon(struct page *page)
 	atomic_set(&page->_mapcount, -1);
 }
 
+#define PAGE_MIGRATABLE_MAPCOUNT_VALUE (-255)
+
+static inline int PageMigratable(struct page *page)
+{
+	return atomic_read(&page->_mapcount) == PAGE_MIGRATABLE_MAPCOUNT_VALUE;
+}
+
+static inline void __SetPageMigratable(struct page *page)
+{
+	VM_BUG_ON_PAGE(atomic_read(&page->_mapcount) != -1, page);
+	atomic_set(&page->_mapcount, PAGE_MIGRATABLE_MAPCOUNT_VALUE);
+}
+
+static inline void __ClearPageMigratable(struct page *page)
+{
+	VM_BUG_ON_PAGE(!PageMigratable(page), page);
+	atomic_set(&page->_mapcount, -1);
+}
+
 /*
  * If network-based swap is enabled, sl*b must keep track of whether pages
  * were allocated from pfmemalloc reserves.
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 3e95fb6..a306798 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -25,8 +25,35 @@ enum mapping_flags {
 	AS_MM_ALL_LOCKS	= __GFP_BITS_SHIFT + 2,	/* under mm_take_all_locks() */
 	AS_UNEVICTABLE	= __GFP_BITS_SHIFT + 3,	/* e.g., ramdisk, SHM_LOCK */
 	AS_EXITING	= __GFP_BITS_SHIFT + 4, /* final truncate in progress */
+	AS_MIGRATABLE   = __GFP_BITS_SHIFT + 5,
 };
 
+static inline void mapping_set_migratable(struct address_space *mapping)
+{
+	set_bit(AS_MIGRATABLE, &mapping->flags);
+}
+
+static inline void mapping_clear_migratable(struct address_space *mapping)
+{
+	clear_bit(AS_MIGRATABLE, &mapping->flags);
+}
+
+static inline int __mapping_ops(struct address_space *mapping)
+{
+	/* migrating page should define all following methods */
+	return mapping->a_ops &&
+		mapping->a_ops->migratepage &&
+		mapping->a_ops->isolatepage &&
+		mapping->a_ops->putbackpage;
+}
+
+static inline int mapping_migratable(struct address_space *mapping)
+{
+	if (mapping && __mapping_ops(mapping))
+		return test_bit(AS_MIGRATABLE, &mapping->flags);
+	return !!mapping;
+}
+
 static inline void mapping_set_error(struct address_space *mapping, int error)
 {
 	if (unlikely(error)) {
-- 
1.9.1

^ permalink raw reply related

* [RFCv2 0/5] enable migration of driver pages
From: Gioh Kim @ 2015-06-26  9:58 UTC (permalink / raw)
  To: jlayton-vpEMnDpepFuMZCB2o+C8xQ, bfields-uC3wQj2KruNg9hUCZPvPmw,
	vbabka-AlSwsSmVLrQ, iamjoonsoo.kim-Hm3cg6mZ9cc,
	viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn, mst-H+wXaHxf7aLQT0dZR+AlfA,
	koct9i-Re5JQEeQqe8AvxtiuMwx3w, minchan-DgEjT+Ai2ygdnm+yROfE0A,
	aquini-H+wXaHxf7aLQT0dZR+AlfA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg
  Cc: akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, Gioh Kim

Hello,

This series try to enable migration of non-LRU pages, such as driver's page.

My ARM-based platform occured severe fragmentation problem after long-term
(several days) test. Sometimes even order-3 page allocation failed. It has
memory size 512MB ~ 1024MB. 30% ~ 40% memory is consumed for graphic processing
and 20~30 memory is reserved for zram.

I found that many pages of GPU driver and zram are non-movable pages. So I
reported Minchan Kim, the maintainer of zram, and he made the internal 
compaction logic of zram. And I made the internal compaction of GPU driver.

They reduced some fragmentation but they are not enough effective.
They are activated by its own interface, /sys, so they are not cooperative
with kernel compaction. If there is too much fragmentation and kernel starts
to compaction, zram and GPU driver cannot work with the kernel compaction.

The first this patch adds a generic isolate/migrate/putback callbacks for page
address-space. The zram and GPU, and any other modules can register
its own migration method. The kernel compaction can call the registered
migration when it works. Therefore all page in the system can be migrated
at once.

The 2nd the generic migration callbacks are applied into balloon driver.
My gpu driver code is not open so I apply generic migration into balloon
to show how it works. I've tested it with qemu enabled by kvm like followings:
- turn on Ubuntu 14.04 with 1G memory on qemu.
- do kernel building
- after several seconds check more than 512MB is used with free command
- command "balloon 512" in qemu monitor
- check hundreds MB of pages are migrated

Next kernel compaction code can call generic migration callbacks instead of
balloon driver interface.
Finally calling migration of balloon driver is removed.

This patch-set is based on v4.1

Gioh Kim (5):
  mm/compaction: enable driver page migration
  fs/anon_inode: get a new inode
  mm/balloon: apply driver page migratable into balloon driver
  mm/compaction: compaction calls generic migration
  mm: remove direct calling of migration

 drivers/virtio/virtio_balloon.c        |  4 ++++
 fs/anon_inodes.c                       |  6 ++++++
 fs/proc/page.c                         |  3 +++
 include/linux/anon_inodes.h            |  1 +
 include/linux/balloon_compaction.h     | 33 +++++++++++++++++++++------------
 include/linux/compaction.h             | 11 +++++++++++
 include/linux/fs.h                     |  2 ++
 include/linux/page-flags.h             | 19 +++++++++++++++++++
 include/linux/pagemap.h                | 27 +++++++++++++++++++++++++++
 include/uapi/linux/kernel-page-flags.h |  2 +-
 mm/balloon_compaction.c                | 25 ++++++++++++++++---------
 mm/compaction.c                        |  9 +++++----
 mm/migrate.c                           | 24 ++++++------------------
 13 files changed, 122 insertions(+), 44 deletions(-)

-- 
1.9.1

^ permalink raw reply

* Re: [RFC v4 06/31] richacl: In-memory representation and helper functions
From: Andreas Grünbacher @ 2015-06-26  7:55 UTC (permalink / raw)
  To: Stefan (metze) Metzmacher
  Cc: Linux Kernel Mailing List, Linux FS-devel Mailing List,
	Linux NFS Mailing List, Linux API Mailing List, samba-technical,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <558C7535.9050502-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>

2015-06-25 23:40 GMT+02:00 Stefan (metze) Metzmacher <metze-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>:
>>> I'm wondering if the size of an ace should be dynamic,
>>> which might make it possible to support other ace types
>>> in future. E.g. supporting other identities like 128-bit values
>>> to make it easier to map Windows SIDS.
>>
>> I'm working on additionally supporting unmapped user@domain and
>> group@domain identifier strings; we have to deal with that case in the
>> nfs client; that may be useful for Samba as well.
>
> Can this be any string? So would
> "S-1-5-21-4052121579-2079768045-1474639452-1001" also work?

I don't see why not, we'd just need to prevent namespace clashes.

> How would the current thread/process get a "token" that would match such
> an ace?

Solaris seems to solve this by what they call ephemeral ids; that concept may
become useful.

> [...]
> In general shouldn't kuid_t uid = current_fsuid(); be at the top of the
> function just once?

It really is just a pointer dereference.

Thanks,
Andreas

^ permalink raw reply

* Re: [RFC v3 1/4] fs: Add generic file system event notifications
From: Beata Michalska @ 2015-06-26  7:30 UTC (permalink / raw)
  To: Steve French
  Cc: Dmitry Monakhov, LKML, linux-fsdevel, linux-api@vger.kernel.org,
	Greg Kroah-Hartman, Jan Kara, Theodore Ts'o, adilger.kernel,
	hughd, lczerner, Christoph Hellwig, linux-ext4@vger.kernel.org,
	linux-mm, kyungmin.park, kmpark
In-Reply-To: <CAH2r5msAncF_KOxK-Wt_sZs-fOOMRh7KMqVJ80MK=KimUC7NLg@mail.gmail.com>

On 06/24/2015 06:26 PM, Steve French wrote:
> On Wed, Jun 24, 2015 at 10:31 AM, Beata Michalska
> <b.michalska@samsung.com> wrote:
>> On 06/24/2015 10:47 AM, Dmitry Monakhov wrote:
>>> Beata Michalska <b.michalska@samsung.com> writes:
>>>
>>>> Introduce configurable generic interface for file
>>>> system-wide event notifications, to provide file
>>>> systems with a common way of reporting any potential
>>>> issues as they emerge.
>>>>
>>>> The notifications are to be issued through generic
>>>> netlink interface by newly introduced multicast group.
>>>>
>>>> Threshold notifications have been included, allowing
>>>> triggering an event whenever the amount of free space drops
>>>> below a certain level - or levels to be more precise as two
>>>> of them are being supported: the lower and the upper range.
>>>> The notifications work both ways: once the threshold level
>>>> has been reached, an event shall be generated whenever
>>>> the number of available blocks goes up again re-activating
>>>> the threshold.
>>>>
>>>> The interface has been exposed through a vfs. Once mounted,
>>>> it serves as an entry point for the set-up where one can
>>>> register for particular file system events.
>>>>
>>>> Signed-off-by: Beata Michalska <b.michalska@samsung.com>
>>>> ---
>>>>  Documentation/filesystems/events.txt |  232 ++++++++++
>>>>  fs/Kconfig                           |    2 +
>>>>  fs/Makefile                          |    1 +
>>>>  fs/events/Kconfig                    |    7 +
>>>>  fs/events/Makefile                   |    5 +
>>>>  fs/events/fs_event.c                 |  809 ++++++++++++++++++++++++++++++++++
>>>>  fs/events/fs_event.h                 |   22 +
>>>>  fs/events/fs_event_netlink.c         |  104 +++++
>>>>  fs/namespace.c                       |    1 +
>>>>  include/linux/fs.h                   |    6 +-
>>>>  include/linux/fs_event.h             |   72 +++
>>>>  include/uapi/linux/Kbuild            |    1 +
>>>>  include/uapi/linux/fs_event.h        |   58 +++
>>>>  13 files changed, 1319 insertions(+), 1 deletion(-)
>>>>  create mode 100644 Documentation/filesystems/events.txt
>>>>  create mode 100644 fs/events/Kconfig
>>>>  create mode 100644 fs/events/Makefile
>>>>  create mode 100644 fs/events/fs_event.c
>>>>  create mode 100644 fs/events/fs_event.h
>>>>  create mode 100644 fs/events/fs_event_netlink.c
>>>>  create mode 100644 include/linux/fs_event.h
>>>>  create mode 100644 include/uapi/linux/fs_event.h
>>>>
>>>> diff --git a/Documentation/filesystems/events.txt b/Documentation/filesystems/events.txt
>>>> new file mode 100644
>>>> index 0000000..c2e6227
>>>> --- /dev/null
>>>> +++ b/Documentation/filesystems/events.txt
>>>> @@ -0,0 +1,232 @@
>>>> +
>>>> +    Generic file system event notification interface
>>>> +
>>>> +Document created 23 April 2015 by Beata Michalska <b.michalska@samsung.com>
>>>> +
>>>> +1. The reason behind:
>>>> +=====================
>>>> +
>>>> +There are many corner cases when things might get messy with the filesystems.
>>>> +And it is not always obvious what and when went wrong. Sometimes you might
>>>> +get some subtle hints that there is something going on - but by the time
>>>> +you realise it, it might be too late as you are already out-of-space
>>>> +or the filesystem has been remounted as read-only (i.e.). The generic
>>>> +interface for the filesystem events fills the gap by providing a rather
>>>> +easy way of real-time notifications triggered whenever something interesting
>>>> +happens, allowing filesystems to report events in a common way, as they occur.
>>>> +
>>>> +2. How does it work:
>>>> +====================
>>>> +
>>>> +The interface itself has been exposed as fstrace-type Virtual File System,
>>>> +primarily to ease the process of setting up the configuration for the
>>>> +notifications. So for starters, it needs to get mounted (obviously):
>>>> +
>>>> +    mount -t fstrace none /sys/fs/events
>>>> +
>>>> +This will unveil the single fstrace filesystem entry - the 'config' file,
>>>> +through which the notification are being set-up.
>>>> +
>>>> +Activating notifications for particular filesystem is as straightforward
>>>> +as writing into the 'config' file. Note that by default all events, despite
>>>> +the actual filesystem type, are being disregarded.
>>>> +
>>>> +Synopsis of config:
>>>> +------------------
>>>> +
>>>> +    MOUNT EVENT_TYPE [L1] [L2]
>>>> +
>>>> + MOUNT      : the filesystem's mount point
>>>> + EVENT_TYPE : event types - currently two of them are being supported:
>>>> +
>>>> +          * generic events ("G") covering most common warnings
>>>> +          and errors that might be reported by any filesystem;
>>>> +          this option does not take any arguments;
>>>> +
>>>> +          * threshold notifications ("T") - events sent whenever
>>>> +          the amount of available space drops below certain level;
>>>> +          it is possible to specify two threshold levels though
>>>> +          only one is required to properly setup the notifications;
>>>> +          as those refer to the number of available blocks, the lower
>>>> +          level [L1] needs to be higher than the upper one [L2]
>>>> +
>>>> +Sample request could look like the following:
>>>> +
>>>> + echo /sample/mount/point G T 710000 500000 > /sys/fs/events/config
>>>> +
>>>> +Multiple request might be specified provided they are separated with semicolon.
>>>> +
>>>> +The configuration itself might be modified at any time. One can add/remove
>>>> +particular event types for given fielsystem, modify the threshold levels,
>>>> +and remove single or all entries from the 'config' file.
>>>> +
>>>> + - Adding new event type:
>>>> +
>>>> + $ echo MOUNT EVENT_TYPE > /sys/fs/events/config
>>>> +
>>>> +(Note that is is enough to provide the event type to be enabled without
>>>> +the already set ones.)
>>>> +
>>>> + - Removing event type:
>>>> +
>>>> + $ echo '!MOUNT EVENT_TYPE' > /sys/fs/events/config
>>>> +
>>>> + - Updating threshold limits:
>>>> +
>>>> + $ echo MOUNT T L1 L2 > /sys/fs/events/config
>>>> +
>>>> + - Removing single entry:
>>>> +
>>>> + $ echo '!MOUNT' > /sys/fs/events/config
>>>> +
>>>> + - Removing all entries:
>>>> +
>>>> + $ echo > /sys/fs/events/config
>>>> +
>>>> +Reading the file will list all registered entries with their current set-up
>>>> +along with some additional info like the filesystem type and the backing device
>>>> +name if available.
>>>> +
>>>> +Final, though a very important note on the configuration: when and if the
>>>> +actual events are being triggered falls way beyond the scope of the generic
>>>> +filesystem events interface. It is up to a particular filesystem
>>>> +implementation which events are to be supported - if any at all. So if
>>>> +given filesystem does not support the event notifications, an attempt to
>>>> +enable those through 'config' file will fail.
>>>> +
>>>> +
>>>> +3. The generic netlink interface support:
>>>> +=========================================
>>>> +
>>>> +Whenever an event notification is triggered (by given filesystem) the current
>>>> +configuration is being validated to decide whether a userpsace notification
>>>> +should be launched. If there has been no request (in a mean of 'config' file
>>>> +entry) for given event, one will be silently disregarded. If, on the other
>>>> +hand, someone is 'watching' given filesystem for specific events, a generic
>>>> +netlink message will be sent. A dedicated multicast group has been provided
>>>> +solely for this purpose so in order to receive such notifications, one should
>>>> +subscribe to this new multicast group. As for now only the init network
>>>> +namespace is being supported.
>>>> +
>>>> +3.1 Message format
>>>> +
>>>> +The FS_NL_C_EVENT shall be stored within the generic netlink message header
>>>> +as the command field. The message payload will provide more detailed info:
>>>> +the backing device major and minor numbers, the event code and the id of
>>>> +the process which action led to the event occurrence. In case of threshold
>>>> +notifications, the current number of available blocks will be included
>>>> +in the payload as well.
>>>> +
>>>> +
>>>> +     0                   1                   2                   3
>>>> +     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>>>> +    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>> +    |                   NETLINK MESSAGE HEADER                      |
>>>> +    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>> +    |               GENERIC NETLINK MESSAGE HEADER                  |
>>>> +    |          (with FS_NL_C_EVENT as genlmsghdr cdm field)         |
>>>> +    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>> +    |             Optional user specific message header             |
>>>> +    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>>> +    |                  GENERIC MESSAGE PAYLOAD:                     |
>>>> +    +---------------------------------------------------------------+
>>>> +    |                 FS_NL_A_EVENT_ID  (NLA_U32)                   |
>>>> +    +---------------------------------------------------------------+
>>>> +    |                 FS_NL_A_DEV_MAJOR (NLA_U32)                   |
>>>> +    +---------------------------------------------------------------+
>>>> +    |                 FS_NL_A_DEV_MINOR (NLA_U32)                   |
>>>
>> ...
>>
>>>> +
>>>> +static int create_common_msg(struct sk_buff *skb, void *data)
>>>> +{
>>>> +    struct fs_trace_entry *en = (struct fs_trace_entry *)data;
>>>> +    struct super_block *sb = en->sb;
>>>> +
>>>> +    if (nla_put_u32(skb, FS_NL_A_DEV_MAJOR, MAJOR(sb->s_dev))
>>>> +    ||  nla_put_u32(skb, FS_NL_A_DEV_MINOR, MINOR(sb->s_dev)))
>>>> +            return -EINVAL;
>>> What about diskless(nfs,cifs,etc) filesystem? btrfs also has no
>>> valid sb->s_dev
> 
> And note that filesystem notifications and also file/directory change
> notification are particularly useful in the case of a a network file
> system (and heavily used by Windows desktop, Mac etc.) since when a
> file is shared a user may not necessarily know that a file (or file
> system as a whole) changed via another client (or on the server, or on
> the server via a different protocol  e.g.SMB3 vs NFSv4), but is more
> likely to know about local changes to the same file.   In some sense
> the users of mounts on network file systems get more benefit from
> notifications than a mount on a local file system would.
> 

As for the network file systems...
As it has been pointed out there are some serious scalability/performance
issues with the current version of the events interface. As it also has been
suggested I plan to modify the way the threshold notifications are being handled
by shuffling the responsibility for tracking the amount of available space 
through querying file systems for an update. Thus I'm wondering, if this
will not result in yet another issue in case of the network file systems, as for
them, handling such query means asking the sever for an update
(there is basically no caching on the client side).


Best Regards
Beata

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH] selftests/exec: Fix build on older distros.
From: David Drysdale @ 2015-06-26  6:17 UTC (permalink / raw)
  To: Vinson Lee
  Cc: Shuah Khan, Andrew Morton, Geert Uytterhoeven, Michael Ellerman,
	Linux API, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Vinson Lee
In-Reply-To: <1435274950-17106-1-git-send-email-vlee-xCSkyg8dI+0RB7SZvlqPiA@public.gmane.org>

On Fri, Jun 26, 2015 at 12:29 AM, Vinson Lee <vlee-xCSkyg8dI+0RB7SZvlqPiA@public.gmane.org> wrote:
> From: Vinson Lee <vlee-1v8oiQdgUNlBDgjK7y7TUQ@public.gmane.org>
>
> This patch fixes this build error on CentOS 5.
>
> execveat.c: In function ‘check_execveat_pathmax’:
> execveat.c:185: error: ‘AT_EMPTY_PATH’ undeclared (first use in this function)
> execveat.c:185: error: (Each undeclared identifier is reported only once
> execveat.c:185: error: for each function it appears in.)
> execveat.c: In function ‘run_tests’:
> execveat.c:221: error: ‘O_PATH’ undeclared (first use in this function)
> execveat.c:222: error: ‘O_CLOEXEC’ undeclared (first use in this function)
> execveat.c:258: error: ‘AT_EMPTY_PATH’ undeclared (first use in this function)
>
> Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org # 3.19+
> Signed-off-by: Vinson Lee <vlee-1v8oiQdgUNlBDgjK7y7TUQ@public.gmane.org>
> ---
>  tools/testing/selftests/exec/execveat.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/tools/testing/selftests/exec/execveat.c b/tools/testing/selftests/exec/execveat.c
> index 8d5d1d2..170148d 100644
> --- a/tools/testing/selftests/exec/execveat.c
> +++ b/tools/testing/selftests/exec/execveat.c
> @@ -20,6 +20,16 @@
>  #include <string.h>
>  #include <unistd.h>
>
> +#ifndef AT_EMPTY_PATH
> +# define AT_EMPTY_PATH 0x1000
> +#endif
> +#ifndef O_PATH
> +# define O_PATH 010000000
> +#endif
> +#ifndef O_CLOEXEC
> +# define O_CLOEXEC 02000000
> +#endif
> +

Won't those definitions affect the arch-independence of the test?

(The O_* constants are sadly arch-specific:
http://lxr.free-electrons.com/ident?i=O_PATH)

>  static char longpath[2 * PATH_MAX] = "";
>  static char *envp[] = { "IN_TEST=yes", NULL, NULL };
>  static char *argv[] = { "execveat", "99", NULL };
> --
> 1.8.2.1
>

^ permalink raw reply

* Re: [RFC PATCH 0/3] restartable sequences: fast user-space percpu critical sections
From: Paul Turner @ 2015-06-26  2:05 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Andy Lutomirski, Peter Zijlstra, Paul E. McKenney, Andrew Hunter,
	Andi Kleen, Lai Jiangshan, linux-api, LKML, rostedt,
	Josh Triplett, Ingo Molnar, Andrew Morton, Linus Torvalds,
	Chris Lameter
In-Reply-To: <842897619.3710.1435281350583.JavaMail.zimbra@efficios.com>

On Thu, Jun 25, 2015 at 6:15 PM, Mathieu Desnoyers
<mathieu.desnoyers@efficios.com> wrote:
> ----- On Jun 24, 2015, at 10:54 PM, Paul Turner pjt@google.com wrote:
>
>> On Wed, Jun 24, 2015 at 5:07 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>> On Wed, Jun 24, 2015 at 3:26 PM, Paul Turner <pjt@google.com> wrote:
>>>> This is a fairly small series demonstrating a feature we've found to be quite
>>>> powerful in practice, "restartable sequences".
>>>>
>>>
>>> On an extremely short glance, I'm starting to think that the right
>>> approach, at least for x86, is to implement per-cpu gsbase.  Then you
>>> could do cmpxchg with a gs prefix to atomically take a percpu lock and
>>> atomically release a percpu lock and check whether someone else stole
>>> the lock from you.  (Note: cmpxchg, unlike lock cmpxchg, is very
>>> fast.)
>>>
>>> This is totally useless for other architectures, but I think it would
>>> be reasonable clean on x86.  Thoughts?
>>
>> So this gives semantics that are obviously similar to this_cpu().
>> This provides allows reasonable per-cpu counters (which is alone
>> almost sufficient for a strong user-space RCU implementation giving
>> this some legs).
>>
>> However, unless there's a nice implementation trick I'm missing, the
>> thing that stands out to me for locks (or other primitives) is that
>> this forces a two-phase commit.  There's no way (short of say,
>> cmpxchg16b) to perform a write conditional on the lock not having been
>> stolen from us (and subsequently release the lock).
>>
>> e.g.
>> 1) We take the operation in some sort of speculative mode, that
>> another thread on the same cpu is stilled allowed to steal from us
>> 2) We prepare what we want to commit
>> 3) At this point we have to promote the lock taken in (1) to perform
>> our actual commit, or see that someone else has stolen (1)
>> 4) Release the promoted lock in (3)
>>
>> However, this means that if we're preempted at (3) then no other
>> thread on that cpu can make progress until we've been rescheduled and
>> released the lock; a nice property of the model we have today is that
>> threads sharing a cpu can not impede each other beyond what the
>> scheduler allows.
>>
>> A lesser concern, but worth mentioning, is that there are also
>> potential pitfalls in the interaction with signal handlers,
>> particularly if a 2-phase commit is used.
>
> Assuming we have a gs segment we can use to address per-cpu locks
> in userspace, would the following scheme take care of some of your
> concerns ?
>
> per-cpu int32_t: each lock initialized to "cpu_nr" value
>
> per-cpu lock:
>   get current cpu number. Remember this value as "CPU lock nr".
>   use cmpxchg on gs:lock to grab the lock.
>   - Expect old value to be "CPU lock nr".
>   - Update with a lock flag in most significant bit, "CPU lock nr"
>     in lower bits.
>   - Retry if fails. Can be caused by migration or lock being already
>     held.

So this is equivalent to just taking the lock in a non-speculative
mode (e.g. non-stealable) from the start.

The challenge remains exactly the same, as soon as an exclusive mode
exists you have some 'critical' critical inner section about the
commit, which impedes the progress of other threads if interrupted.
[The only reason you'd take the lock in any sort of speculative mode
above would be to reduce the length of this.]

I definitely agree that this gets us locks and counters, there's lots
of good things we can do with those and those alone may be sufficient
to implement it this way but I feel there is a lot of power compared
to what can be done with full lockless semantics that this leaves on
the table.  As a concrete example, consider what this does to the
complexity and usefulness of Pop().

>
> per-cpu unlock:
>   clear lock flag within the "CPU lock nr" lock.
>
> Thanks,
>
> Mathieu
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com

^ permalink raw reply

* Re: [RFC PATCH 0/3] restartable sequences: fast user-space percpu critical sections
From: Mathieu Desnoyers @ 2015-06-26  1:15 UTC (permalink / raw)
  To: Paul Turner
  Cc: Andy Lutomirski, Peter Zijlstra, Paul E. McKenney, Andrew Hunter,
	Andi Kleen, Lai Jiangshan, linux-api,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, rostedt, Josh Triplett,
	Ingo Molnar, Andrew Morton, Linus Torvalds, Chris Lameter
In-Reply-To: <CAPM31R+GUtD_9S+m7U0DGpeqSCT1n98bvQ0NUOwMHX7-CoKigQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

----- On Jun 24, 2015, at 10:54 PM, Paul Turner pjt-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org wrote:

> On Wed, Jun 24, 2015 at 5:07 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>> On Wed, Jun 24, 2015 at 3:26 PM, Paul Turner <pjt-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
>>> This is a fairly small series demonstrating a feature we've found to be quite
>>> powerful in practice, "restartable sequences".
>>>
>>
>> On an extremely short glance, I'm starting to think that the right
>> approach, at least for x86, is to implement per-cpu gsbase.  Then you
>> could do cmpxchg with a gs prefix to atomically take a percpu lock and
>> atomically release a percpu lock and check whether someone else stole
>> the lock from you.  (Note: cmpxchg, unlike lock cmpxchg, is very
>> fast.)
>>
>> This is totally useless for other architectures, but I think it would
>> be reasonable clean on x86.  Thoughts?
> 
> So this gives semantics that are obviously similar to this_cpu().
> This provides allows reasonable per-cpu counters (which is alone
> almost sufficient for a strong user-space RCU implementation giving
> this some legs).
> 
> However, unless there's a nice implementation trick I'm missing, the
> thing that stands out to me for locks (or other primitives) is that
> this forces a two-phase commit.  There's no way (short of say,
> cmpxchg16b) to perform a write conditional on the lock not having been
> stolen from us (and subsequently release the lock).
> 
> e.g.
> 1) We take the operation in some sort of speculative mode, that
> another thread on the same cpu is stilled allowed to steal from us
> 2) We prepare what we want to commit
> 3) At this point we have to promote the lock taken in (1) to perform
> our actual commit, or see that someone else has stolen (1)
> 4) Release the promoted lock in (3)
> 
> However, this means that if we're preempted at (3) then no other
> thread on that cpu can make progress until we've been rescheduled and
> released the lock; a nice property of the model we have today is that
> threads sharing a cpu can not impede each other beyond what the
> scheduler allows.
> 
> A lesser concern, but worth mentioning, is that there are also
> potential pitfalls in the interaction with signal handlers,
> particularly if a 2-phase commit is used.

Assuming we have a gs segment we can use to address per-cpu locks
in userspace, would the following scheme take care of some of your
concerns ?

per-cpu int32_t: each lock initialized to "cpu_nr" value

per-cpu lock:
  get current cpu number. Remember this value as "CPU lock nr".
  use cmpxchg on gs:lock to grab the lock.
  - Expect old value to be "CPU lock nr".
  - Update with a lock flag in most significant bit, "CPU lock nr"
    in lower bits.
  - Retry if fails. Can be caused by migration or lock being already
    held.

per-cpu unlock:
  clear lock flag within the "CPU lock nr" lock.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply

* [PATCH] selftests/exec: Fix build on older distros.
From: Vinson Lee @ 2015-06-25 23:29 UTC (permalink / raw)
  To: Shuah Khan, David Drysdale, Andrew Morton, Geert Uytterhoeven,
	Michael Ellerman
  Cc: linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Vinson Lee

From: Vinson Lee <vlee-1v8oiQdgUNlBDgjK7y7TUQ@public.gmane.org>

This patch fixes this build error on CentOS 5.

execveat.c: In function ‘check_execveat_pathmax’:
execveat.c:185: error: ‘AT_EMPTY_PATH’ undeclared (first use in this function)
execveat.c:185: error: (Each undeclared identifier is reported only once
execveat.c:185: error: for each function it appears in.)
execveat.c: In function ‘run_tests’:
execveat.c:221: error: ‘O_PATH’ undeclared (first use in this function)
execveat.c:222: error: ‘O_CLOEXEC’ undeclared (first use in this function)
execveat.c:258: error: ‘AT_EMPTY_PATH’ undeclared (first use in this function)

Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org # 3.19+
Signed-off-by: Vinson Lee <vlee-1v8oiQdgUNlBDgjK7y7TUQ@public.gmane.org>
---
 tools/testing/selftests/exec/execveat.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tools/testing/selftests/exec/execveat.c b/tools/testing/selftests/exec/execveat.c
index 8d5d1d2..170148d 100644
--- a/tools/testing/selftests/exec/execveat.c
+++ b/tools/testing/selftests/exec/execveat.c
@@ -20,6 +20,16 @@
 #include <string.h>
 #include <unistd.h>
 
+#ifndef AT_EMPTY_PATH
+# define AT_EMPTY_PATH 0x1000
+#endif
+#ifndef O_PATH
+# define O_PATH 010000000
+#endif
+#ifndef O_CLOEXEC
+# define O_CLOEXEC 02000000
+#endif
+
 static char longpath[2 * PATH_MAX] = "";
 static char *envp[] = { "IN_TEST=yes", NULL, NULL };
 static char *argv[] = { "execveat", "99", NULL };
-- 
1.8.2.1

^ permalink raw reply related

* Re: [RFC v4 06/31] richacl: In-memory representation and helper functions
From: Stefan (metze) Metzmacher @ 2015-06-25 21:40 UTC (permalink / raw)
  To: Andreas Grünbacher
  Cc: Linux Kernel Mailing List, Linux FS-devel Mailing List,
	Linux NFS Mailing List, Linux API Mailing List, samba-technical,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAHpGcM+AwRubQqX96V3WbwLKXKTfk3YcgFG_eqG6r7cVbCTV-w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 2706 bytes --]

Hi Andreas,

>> I'm wondering if the size of an ace should be dynamic,
>> which might make it possible to support other ace types
>> in future. E.g. supporting other identities like 128-bit values
>> to make it easier to map Windows SIDS.
> 
> I'm working on additionally supporting unmapped user@domain and
> group@domain identifier strings; we have to deal with that case in the
> nfs client; that may be useful for Samba as well.

Can this be any string? So would
"S-1-5-21-4052121579-2079768045-1474639452-1001" also work?

How would the current thread/process get a "token" that would match such
an ace?

>> Even without 128-bit ids, it would be very useful to mark an
>> ace so that it applies to a uid or gid at the same time.
>> This would reduce the size of the ace list when Samba uses
>> IDMAP_TYPE_BOTH, which means a SID is mapped to a unix id, which
>> is user (uid) and group (gid) at the same time. This feature is required
>> in order to support SID-Histories on accounts.
>> Currently Samba needs to add two aces (one uid and one gid)
>> in order to represent one Windows ace.
> 
> It's not clear to me if supporting this would be a good idea right now.
> The kernel would have to treat each such entry like two separate entries
> internally. How would we map a combined user-space "uid + gid"
> number to a kernel uid and gid since it could map to two distinct
> numbers there?

No, the numeric value is the same.

I think richacl_permission() is the only place that requires any action.

	richacl_for_each_entry(ace, acl) {
		unsigned int ace_mask = ace->e_mask;

		if (richace_is_inherit_only(ace))
			continue;
		if (richace_is_owner(ace)) {
			if (!uid_eq(current_fsuid(), inode->i_uid))
				continue;
		} else if (richace_is_group(ace)) {
			if (!in_owning_group)
				continue;
+		} else if (richace_is_unix_both(ace)) {
+			kuid_t uid = current_fsuid();
+
+			if (!uid_eq(uid, ace->e_id.xid) && !in_group_p(ace->e_id.xid))
+				continue;
		} else if (richace_is_unix_user(ace)) {
			kuid_t uid = current_fsuid();

			if (!uid_eq(uid, ace->e_id.uid))
				continue;
		} else if (richace_is_unix_group(ace)) {
			if (!in_group_p(ace->e_id.gid))
				continue;
		} else
			goto entry_matches_everyone;

In general shouldn't kuid_t uid = current_fsuid(); be at the top of the
function just once?

>> I haven't looked at the claims based acls on Windows, but it would be
>> good if the new infrastructure is dynamic enough to support something
>> like that in a future version.
> 
> I don't know, I have yet to see a use case that isn't totally crazy.

Ok, I found the a_version in struct richacl_xattr.

metze


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [RFC v4 06/31] richacl: In-memory representation and helper functions
From: Andreas Grünbacher @ 2015-06-25 21:06 UTC (permalink / raw)
  To: Stefan (metze) Metzmacher
  Cc: Linux Kernel Mailing List, Linux FS-devel Mailing List,
	Linux NFS Mailing List, Linux API Mailing List, samba-technical,
	linux-security-module
In-Reply-To: <558C5D75.6050608@samba.org>

Hi,

2015-06-25 21:58 GMT+02:00 Stefan (metze) Metzmacher <metze@samba.org>:
> Is that also the on disk representation?

that's not the xattr representation, no.

> I'm wondering if the size of an ace should be dynamic,
> which might make it possible to support other ace types
> in future. E.g. supporting other identities like 128-bit values
> to make it easier to map Windows SIDS.

I'm working on additionally supporting unmapped user@domain and
group@domain identifier strings; we have to deal with that case in the
nfs client; that may be useful for Samba as well.

> Even without 128-bit ids, it would be very useful to mark an
> ace so that it applies to a uid or gid at the same time.
> This would reduce the size of the ace list when Samba uses
> IDMAP_TYPE_BOTH, which means a SID is mapped to a unix id, which
> is user (uid) and group (gid) at the same time. This feature is required
> in order to support SID-Histories on accounts.
> Currently Samba needs to add two aces (one uid and one gid)
> in order to represent one Windows ace.

It's not clear to me if supporting this would be a good idea right now.
The kernel would have to treat each such entry like two separate entries
internally. How would we map a combined user-space "uid + gid"
number to a kernel uid and gid since it could map to two distinct
numbers there?

> I haven't looked at the claims based acls on Windows, but it would be
> good if the new infrastructure is dynamic enough to support something
> like that in a future version.

I don't know, I have yet to see a use case that isn't totally crazy.

Thanks,
Andreas

^ permalink raw reply

* Re: [RFC v3 00/45] Richacls
From: Stefan (metze) Metzmacher @ 2015-06-25 20:03 UTC (permalink / raw)
  To: Jeremy Allison, Andreas Gruenbacher
  Cc: linux-nfs, linux-api, samba-technical, linux-kernel,
	linux-security-module, linux-fsdevel
In-Reply-To: <20150523155056.GA2845@jeremy-HP>

[-- Attachment #1: Type: text/plain, Size: 1512 bytes --]

Hi Andreas,

>> here's another update of the richacl patch queue.  The changes since the last
>> posting (https://lwn.net/Articles/638242/) include:
>>
>>  * The nfs client now allocates pages for received acls on demand like the
>>    server does.  It no longer caches the acl size between calls.
>>
>>  * All possible acls consisting of only owner@, group@, and everyone@ entries
>>    which are equivalent to the file mode permission bits are now recognized.
>>    This is needed because by the NFSv4 specification, the nfs server must
>>    translate the file mode permission bits into an acl if it supports acls at
>>    all.
>>
>>  * Support for the dacl attribute over NFSv4.1 for Automatic Inheritance, and
>>    also for the write_retention and write_retention_hold permissions.
>>
>>  * The richacl_compute_max_masks() documentation has been improved.
>>
>>  * Various minor bug fixes.
>>
>> The git version is available here:
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/agruen/linux-richacl.git \
>> 	richacl-2015-04-24
> 
> FYI. I have a mostly (needs test suite adding) working module
> for Samba for Andreas's richacls code.
> 
> Using it we map incoming Windows ACLs directly to richacls
> using the same mapping as we use for existing ZFS ACLs.

Yes, we need to make sure the code supports everything we require,
proved by working code.

We should avoid the disaster with the mostly unuseable F_SETLEASE
code for kernel oplocks.

Thanks!
metze


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [RFC v4 06/31] richacl: In-memory representation and helper functions
From: Stefan (metze) Metzmacher @ 2015-06-25 19:58 UTC (permalink / raw)
  To: Andreas Gruenbacher, linux-kernel, linux-fsdevel, linux-nfs,
	linux-api, samba-technical, linux-security-module
In-Reply-To: <1435183040-22726-7-git-send-email-agruenba@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1578 bytes --]

Hi Andreas,

> +#define RICHACE_OWNER_SPECIAL_ID	0
> +#define RICHACE_GROUP_SPECIAL_ID	1
> +#define RICHACE_EVERYONE_SPECIAL_ID	2
> +
> +struct richace {
> +	unsigned short	e_type;
> +	unsigned short	e_flags;
> +	unsigned int	e_mask;
> +	union {
> +		kuid_t		uid;
> +		kgid_t		gid;
> +		unsigned int	special;
> +	} e_id;
> +};
> +
> +struct richacl {
> +	atomic_t	a_refcount;
> +	unsigned int	a_owner_mask;
> +	unsigned int	a_group_mask;
> +	unsigned int	a_other_mask;
> +	unsigned short	a_count;
> +	unsigned short	a_flags;
> +	struct richace	a_entries[0];
> +};

Is that also the on disk representation?

I'm wondering if the size of an ace should be dynamic,
which might make it possible to support other ace types
in future. E.g. supporting other identities like 128-bit values
to make it easier to map Windows SIDS.

Even without 128-bit ids, it would be very useful to mark an
ace so that it applies to a uid or gid at the same time.
This would reduce the size of the ace list when Samba uses
IDMAP_TYPE_BOTH, which means a SID is mapped to a unix id, which
is user (uid) and group (gid) at the same time. This feature is required
in order to support SID-Histories on accounts.
Currently Samba needs to add two aces (one uid and one gid)
in order to represent one Windows ace.

I haven't looked at the claims based acls on Windows, but it would be
good if the new infrastructure is dynamic enough to support something
like that in a future version.

Thanks very much on your persistent to bring richacls forward!

metze


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [PATCH v2] ipc: Modify message queue accounting to reflect both total user data and auxiliary kernel data
From: Marcus Gelderie @ 2015-06-25 18:50 UTC (permalink / raw)
  To: Michael Kerrisk (man-pages)
  Cc: Davidlohr Bueso, Doug Ledford, lkml, David Howells,
	Alexander Viro, John Duffy, Arto Bendiken, Linux API
In-Reply-To: <CAKgNAkieR5zdpKm=P2dcTDJ_3X4HMRoeOQ2D8yghYVKOjDsYAg@mail.gmail.com>

Hey all,

answers in text below... 

TLDR: I can remove the QKERSIZE field, as I believe that it does not affect he
RLIMIT accounting (details [=code] below). Question is: Should it?

Before I provide another patch, I would appreciate another round of feedback, though.

So...

On Thu, Jun 25, 2015 at 09:23:33AM +0200, Michael Kerrisk (man-pages) wrote:
> On 25 June 2015 at 07:47, Davidlohr Bueso <dave@stgolabs.net> wrote:
> > Good catch, and a nice opportunity to make the mq manpage more specific
> > wrt to queue sizes.
> >
> > [...]
> >
ACK, I can write a patch for the manpage once we figure out what to do
here.
> >> Reporting the size of the message queue in kernel has its merits, but
> >> doing so in the QSIZE field of the pseudo file corresponding to the
> >> queue is a breaking change, as mentioned above. This patch therefore
> >> returns the QSIZE  field to its original meaning. At the same time,
> >> it introduces a new field QKERSIZE that reflects the size of the queue
> >> in kernel (user data + kernel data).
> >
> > Hmmm I'm not sure about this. What are the specific benefits of having
> > QKERSIZE? We don't export in-kernel data like this in any other ipc
> > (posix or sysv) mechanism, afaik. Plus, we do not compromise kernel data
> > structures like this, as we would break userspace if later we change
> > posix_msg_tree_node. So NAK to this.
> >
> > I would just remove the extra
> > +       info->qsize += sizeof(struct posix_msg_tree_node);
> >
> > bits from d6629859b36 (along with -stable v3.5), plus a patch updating
> > the manpage that this field only reflects user data.
> 
This can be done if the RLIMIT accounting is not done against that
value (more below).

> I've been hoping that Doug would jump into this discussion...
> 
> If I recall/understand Doug correctly (see
> http://thread.gmane.org/gmane.linux.man/7050/focus=1797645 ), his
> rationale for the QSIZE change was that it then revealed a value that
> was closer to what was being used to account against the
> RLIMIT_MSGQUEUE resource limit. (Even with these changes, the QSIZE
> value was not 100% accurate for accounting against RLIMIT_MSGQUEUE,
> since some pieces of kernel overhead were still not being accounted
> for. Nevertheless, it's much closer than the old (pre 3.5) QSIZE for
> some corner cases.) Thus, Marcus's rationale for preserving this info
> as QKERSIZE.
> 

Actually, looking at the code, it seems to me that the QKERSIZE
(a.k.a. the current qsize field) is actually not used for the purpose of
accounting at all. From ipc/mqueue.c (line 281 ff, comments added by me):
		
		/* worst case estimate for the btree in memory */
		
		mq_treesize = info->attr.mq_maxmsg * sizeof(struct msg_msg) +
			min_t(unsigned int, info->attr.mq_maxmsg, MQ_PRIO_MAX) *
			sizeof(struct posix_msg_tree_node);

		/* worst case estimate for the user data in the queue */
		
		mq_bytes = mq_treesize + (info->attr.mq_maxmsg *
					  info->attr.mq_msgsize);

		spin_lock(&mq_lock);
		    
		    /* acutal accounting; u->mq_bytes is the other
		     * message queus for this user (computed in the way
		     * above (i.e. worst-case estimates) 
		     */

		if (u->mq_bytes + mq_bytes < u->mq_bytes ||
		    u->mq_bytes + mq_bytes > rlimit(RLIMIT_MSGQUEUE)) {
			[....]
			ret = -EMFILE;
			[.....]

So, I think that if the RLIMIT should take the actual size (not the
worst case) into account, then we have a bug. I can write a patch, but that
should be separate from this... meaning I'll turn this into a patchset.

However, we should decide what we want first: If I read the manpage (mq_overview), I
am inclined to think that the RLIMIT should use the acutal amount of
data occupied by the queue (or at least the user data). But it does not
necessarily rule out an accounting against worst-case estimation (to me
at least).


> Now whether QKERSIZE is actually useful or used by anyone is another
> question. As far as I know, there was no user request that drove the
> change. But Doug can perhaps say something to this. QSIZE should I
> think definitely be fixed (reverted to pre-3.5 behavior). I'm agnostic
> about QKERSIZE.
> 
> Cheers,
> 
> Michael
> 
> 
> -- 
> Michael Kerrisk
> Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
> Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox