* Re: [PATCH v9 0/9] Add simple NVMEM Framework via regmap.
From: Kevin Hilman @ 2015-08-07 16:30 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: Kevin Hilman, linux-arm-kernel@lists.infradead.org,
Greg Kroah-Hartman, Rob Herring, Mark Brown, Sascha Hauer,
linux-api, lkml, devicetree@vger.kernel.org, linux-arm-msm,
Arnd Bergmann, sboyd@codeaurora.org, Pantelis Antoniou, mporter,
Stefan Wahren, Caesar Wang
In-Reply-To: <55C4DA42.909@linaro.org>
On Fri, Aug 7, 2015 at 9:18 AM, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
> Thanks Kevin,
>
> On 07/08/15 17:13, Kevin Hilman wrote:
>>
>> Srinivas Kandagatla <srinivas.kandagatla@linaro.org> writes:
>>
>>> This patchset adds a new simple NVMEM framework to kernel, and it is
>>> tested
>>> with various drivers like "QCOM thermal sensors", "QCOM cpr driver",
>>> "begal bone cape manager" and few more on the way.
>>>
>>> Can you please consider this as 4.3 material, AFAIK there are more than 3
>>> drivers
>>> depending on this framework which are wating since last 2 merge windows.
>>>
>>> Should these patches go via ARM-SOC tree or via Greg's tree?
>>
>>
>> With Greg's ack, we can take this through the arm-soc tree (unless Greg
>> wants to queue it, which is fine also.)
>>
>
> Greg already merged these patches yesterday on to
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
> in the char-misc-next branch.
Ah, great. I missed that (still catching up on post-vacation mail backlog.)
Thanks,
Kevin
^ permalink raw reply
* Re: [PATCH v9 0/9] Add simple NVMEM Framework via regmap.
From: Srinivas Kandagatla @ 2015-08-07 16:18 UTC (permalink / raw)
To: Kevin Hilman
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Greg Kroah-Hartman, Rob Herring, Mark Brown,
s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, arnd-r2nGTMty4D4,
sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w,
mporter-OWPKS81ov/FWk0Htik3J/w, stefan.wahren-eS4NqCHxEME,
wxt-TNX95d0MmH7DzftRWevZcw
In-Reply-To: <7hwpx7vzyo.fsf-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org>
Thanks Kevin,
On 07/08/15 17:13, Kevin Hilman wrote:
> Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes:
>
>> This patchset adds a new simple NVMEM framework to kernel, and it is tested
>> with various drivers like "QCOM thermal sensors", "QCOM cpr driver",
>> "begal bone cape manager" and few more on the way.
>>
>> Can you please consider this as 4.3 material, AFAIK there are more than 3 drivers
>> depending on this framework which are wating since last 2 merge windows.
>>
>> Should these patches go via ARM-SOC tree or via Greg's tree?
>
> With Greg's ack, we can take this through the arm-soc tree (unless Greg
> wants to queue it, which is fine also.)
>
Greg already merged these patches yesterday on to
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
in the char-misc-next branch.
Now the patches are available in linux-next for more testing.
--srini
> Can you collect the final Tested-by's and prepare a pull request?
>
> Thanks,
>
> Kevin
>
^ permalink raw reply
* Re: [PATCH v9 0/9] Add simple NVMEM Framework via regmap.
From: Kevin Hilman @ 2015-08-07 16:13 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: linux-arm-kernel, Greg Kroah-Hartman, Rob Herring, Mark Brown,
s.hauer, linux-api, linux-kernel, devicetree, linux-arm-msm, arnd,
sboyd, pantelis.antoniou, mporter, stefan.wahren, wxt
In-Reply-To: <1437995567-11203-1-git-send-email-srinivas.kandagatla@linaro.org>
Srinivas Kandagatla <srinivas.kandagatla@linaro.org> writes:
> This patchset adds a new simple NVMEM framework to kernel, and it is tested
> with various drivers like "QCOM thermal sensors", "QCOM cpr driver",
> "begal bone cape manager" and few more on the way.
>
> Can you please consider this as 4.3 material, AFAIK there are more than 3 drivers
> depending on this framework which are wating since last 2 merge windows.
>
> Should these patches go via ARM-SOC tree or via Greg's tree?
With Greg's ack, we can take this through the arm-soc tree (unless Greg
wants to queue it, which is fine also.)
Can you collect the final Tested-by's and prepare a pull request?
Thanks,
Kevin
^ permalink raw reply
* [PATCH 4/4] selftests: vm: Pick up sanitized kernel headers
From: Thierry Reding @ 2015-08-07 13:44 UTC (permalink / raw)
To: Shuah Khan, Andrew Morton
Cc: Eric B Munson, Andrea Arcangeli, linux-api, linux-kernel
In-Reply-To: <1438955055-587-1-git-send-email-thierry.reding@gmail.com>
From: Thierry Reding <treding@nvidia.com>
Add the usr/include subdirectory of the top-level tree to the include
path, and make sure to include headers without relative paths to make
sure the sanitized headers get picked up. Otherwise the compiler will
not be able to find the linux/compiler.h header included by the non-
sanitized include/uapi/linux/userfaultfd.h.
While at it, make sure to only hardcode the syscall numbers on x86 and
PowerPC if they haven't been properly picked up from the headers.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
tools/testing/selftests/vm/Makefile | 2 +-
tools/testing/selftests/vm/userfaultfd.c | 4 +++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
index bb888c651bf2..1dacac81bb68 100644
--- a/tools/testing/selftests/vm/Makefile
+++ b/tools/testing/selftests/vm/Makefile
@@ -1,6 +1,6 @@
# Makefile for vm selftests
-CFLAGS = -Wall
+CFLAGS = -Wall -I ../../../../usr/include $(EXTRA_CFLAGS)
BINARIES = compaction_test
BINARIES += hugepage-mmap
BINARIES += hugepage-shm
diff --git a/tools/testing/selftests/vm/userfaultfd.c b/tools/testing/selftests/vm/userfaultfd.c
index 76071b14cb93..2bf1fc3f562b 100644
--- a/tools/testing/selftests/vm/userfaultfd.c
+++ b/tools/testing/selftests/vm/userfaultfd.c
@@ -64,8 +64,9 @@
#include <sys/syscall.h>
#include <sys/ioctl.h>
#include <pthread.h>
-#include "../../../../include/uapi/linux/userfaultfd.h"
+#include <linux/userfaultfd.h>
+#ifndef __NR_userfaultfd
#ifdef __x86_64__
#define __NR_userfaultfd 323
#elif defined(__i386__)
@@ -75,6 +76,7 @@
#else
#error "missing __NR_userfaultfd definition"
#endif
+#endif
static unsigned long nr_cpus, nr_pages, nr_pages_per_cpu, page_size;
--
2.4.5
^ permalink raw reply related
* [PATCH 3/4] selftests: vm: Use the right arguments for main()
From: Thierry Reding @ 2015-08-07 13:44 UTC (permalink / raw)
To: Shuah Khan, Andrew Morton
Cc: Eric B Munson, Andrea Arcangeli, linux-api, linux-kernel
In-Reply-To: <1438955055-587-1-git-send-email-thierry.reding@gmail.com>
From: Thierry Reding <treding@nvidia.com>
The prototype for the main() function is:
int main(int argc, char **argv);
but the mlock2-tests test program lists the arguments in the wrong
order. It gets away with this because the arguments are never used. Fix
it nevertheless to keep recent versions of GCC from complaining.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
tools/testing/selftests/vm/mlock2-tests.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/vm/mlock2-tests.c b/tools/testing/selftests/vm/mlock2-tests.c
index 2949fd949973..af4bc752797d 100644
--- a/tools/testing/selftests/vm/mlock2-tests.c
+++ b/tools/testing/selftests/vm/mlock2-tests.c
@@ -643,7 +643,7 @@ static int test_mlockall(int (test_function)(bool call_mlock))
return ret;
}
-int main(char **argv, int argc)
+int main(int argc, char **argv)
{
int ret = 0;
ret += test_mlock_lock();
--
2.4.5
^ permalink raw reply related
* [PATCH 2/4] selftests: vm: Ensure the mlock2 syscall number can be found
From: Thierry Reding @ 2015-08-07 13:44 UTC (permalink / raw)
To: Shuah Khan, Andrew Morton
Cc: Eric B Munson, Andrea Arcangeli, linux-api, linux-kernel
In-Reply-To: <1438955055-587-1-git-send-email-thierry.reding@gmail.com>
From: Thierry Reding <treding@nvidia.com>
Include the syscall.h header to ensure that the mlock2 syscall number is
available. Otherwise the test program will always return ENOSYS from the
mlock2() implementation.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
tools/testing/selftests/vm/mlock2-tests.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/vm/mlock2-tests.c b/tools/testing/selftests/vm/mlock2-tests.c
index 11c38e96f336..2949fd949973 100644
--- a/tools/testing/selftests/vm/mlock2-tests.c
+++ b/tools/testing/selftests/vm/mlock2-tests.c
@@ -6,6 +6,7 @@
#include <string.h>
#include <sys/time.h>
#include <sys/resource.h>
+#include <syscall.h>
#include <errno.h>
#include <stdbool.h>
--
2.4.5
^ permalink raw reply related
* [PATCH 1/4] selftests: vm: Fix mlock2-tests for 32-bit architectures
From: Thierry Reding @ 2015-08-07 13:44 UTC (permalink / raw)
To: Shuah Khan, Andrew Morton
Cc: Eric B Munson, Andrea Arcangeli, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
According to Documentation/vm/pagemap.txt, the /proc/pid/pagemap file
contains one 64-bit value for each virtual page. The test code relies
on the size of unsigned long being 64-bit, which breaks the test when
run on 32-bit architectures. Use a uint64_t to store values read from
the file instead, so that it works irrespective of the architecture's
word size.
Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
tools/testing/selftests/vm/mlock2-tests.c | 33 +++++++++++++------------------
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/tools/testing/selftests/vm/mlock2-tests.c b/tools/testing/selftests/vm/mlock2-tests.c
index c49122bebf3c..11c38e96f336 100644
--- a/tools/testing/selftests/vm/mlock2-tests.c
+++ b/tools/testing/selftests/vm/mlock2-tests.c
@@ -1,4 +1,5 @@
#include <sys/mman.h>
+#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
@@ -82,10 +83,10 @@ out:
return ret;
}
-static unsigned long get_pageflags(unsigned long addr)
+static uint64_t get_pageflags(unsigned long addr)
{
FILE *file;
- unsigned long pfn;
+ uint64_t pfn;
unsigned long offset;
file = fopen("/proc/self/pagemap", "r");
@@ -94,13 +95,14 @@ static unsigned long get_pageflags(unsigned long addr)
_exit(1);
}
- offset = addr / getpagesize() * sizeof(unsigned long);
+ offset = addr / getpagesize() * sizeof(pfn);
+
if (fseek(file, offset, SEEK_SET)) {
perror("fseek pagemap");
_exit(1);
}
- if (fread(&pfn, sizeof(unsigned long), 1, file) != 1) {
+ if (fread(&pfn, sizeof(pfn), 1, file) != 1) {
perror("fread pagemap");
_exit(1);
}
@@ -111,7 +113,7 @@ static unsigned long get_pageflags(unsigned long addr)
static unsigned long get_kpageflags(unsigned long pfn)
{
- unsigned long flags;
+ uint64_t flags;
FILE *file;
file = fopen("/proc/kpageflags", "r");
@@ -120,12 +122,12 @@ static unsigned long get_kpageflags(unsigned long pfn)
_exit(1);
}
- if (fseek(file, pfn * sizeof(unsigned long), SEEK_SET)) {
+ if (fseek(file, pfn * sizeof(flags), SEEK_SET)) {
perror("fseek kpageflags");
_exit(1);
}
- if (fread(&flags, sizeof(unsigned long), 1, file) != 1) {
+ if (fread(&flags, sizeof(flags), 1, file) != 1) {
perror("fread kpageflags");
_exit(1);
}
@@ -211,9 +213,8 @@ out:
static int lock_check(char *map)
{
- unsigned long page1_flags;
- unsigned long page2_flags;
unsigned long page_size = getpagesize();
+ uint64_t page1_flags, page2_flags;
page1_flags = get_pageflags((unsigned long)map);
page2_flags = get_pageflags((unsigned long)map + page_size);
@@ -246,9 +247,8 @@ static int lock_check(char *map)
static int unlock_lock_check(char *map)
{
- unsigned long page1_flags;
- unsigned long page2_flags;
unsigned long page_size = getpagesize();
+ uint64_t page1_flags, page2_flags;
page1_flags = get_pageflags((unsigned long)map);
page2_flags = get_pageflags((unsigned long)map + page_size);
@@ -310,9 +310,8 @@ out:
static int onfault_check(char *map)
{
- unsigned long page1_flags;
- unsigned long page2_flags;
unsigned long page_size = getpagesize();
+ uint64_t page1_flags, page2_flags;
page1_flags = get_pageflags((unsigned long)map);
page2_flags = get_pageflags((unsigned long)map + page_size);
@@ -355,9 +354,8 @@ static int onfault_check(char *map)
static int unlock_onfault_check(char *map)
{
- unsigned long page1_flags;
- unsigned long page2_flags;
unsigned long page_size = getpagesize();
+ uint64_t page1_flags;
page1_flags = get_pageflags((unsigned long)map);
page1_flags = get_kpageflags(page1_flags & PFN_MASK);
@@ -422,9 +420,8 @@ static int test_lock_onfault_of_present()
{
char *map;
int ret = 1;
- unsigned long page1_flags;
- unsigned long page2_flags;
unsigned long page_size = getpagesize();
+ uint64_t page1_flags, page2_flags;
map = mmap(NULL, 2 * page_size, PROT_READ | PROT_WRITE,
MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
@@ -471,8 +468,6 @@ static int test_munlockall()
{
char *map;
int ret = 1;
- unsigned long page1_flags;
- unsigned long page2_flags;
unsigned long page_size = getpagesize();
map = mmap(NULL, 2 * page_size, PROT_READ | PROT_WRITE,
--
2.4.5
^ permalink raw reply related
* Re: [PATCH V6 3/6] mm: Introduce VM_LOCKONFAULT
From: Thierry Reding @ 2015-08-07 12:34 UTC (permalink / raw)
To: Eric B Munson, Andrew Morton
Cc: Andrea Arcangeli, Jonathan Corbet, linux-api, linux-kernel,
dri-devel, Michal Hocko, linux-mm, Kirill A. Shutemov,
Vlastimil Babka
In-Reply-To: <1438184575-10537-4-git-send-email-emunson@akamai.com>
[-- Attachment #1.1: Type: text/plain, Size: 4325 bytes --]
On Wed, Jul 29, 2015 at 11:42:52AM -0400, Eric B Munson wrote:
[...]
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index ca1e091..38d69fc 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -579,6 +579,7 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
> #ifdef CONFIG_X86_INTEL_MPX
> [ilog2(VM_MPX)] = "mp",
> #endif
> + [ilog2(VM_LOCKONFAULT)] = "lf",
> [ilog2(VM_LOCKED)] = "lo",
> [ilog2(VM_IO)] = "io",
> [ilog2(VM_SEQ_READ)] = "sr",
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 2e872f9..c2f3551 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -127,6 +127,7 @@ extern unsigned int kobjsize(const void *objp);
> #define VM_PFNMAP 0x00000400 /* Page-ranges managed without "struct page", just pure PFN */
> #define VM_DENYWRITE 0x00000800 /* ETXTBSY on write attempts.. */
>
> +#define VM_LOCKONFAULT 0x00001000 /* Lock the pages covered when they are faulted in */
> #define VM_LOCKED 0x00002000
> #define VM_IO 0x00004000 /* Memory mapped I/O or similar */
>
This clashes with another change currently in linux-next:
81d056997385 userfaultfd: add VM_UFFD_MISSING and VM_UFFD_WP
Adding Andrea for visibility.
I noticed this because I was trying to make selftests/vm/mlock2-tests
work to see if the new mlock2 syscall would work on ARM. It didn't, so I
had to investigate and noticed that two symbolic names resolve to the
same value, which results in the mnemonics table (first hunk above)
overwriting the VM_LOCKONFAULT entry with the VM_UFFD_WP entry.
I've applied the following patch locally to fix this up.
Andrew, I think both of those patches came in via your tree, so perhaps
the best thing would be to squash the below (provided everybody agrees
that it's the right fix) into Eric's patch, adding the VM_LOCKONFAULT
flag?
Thierry
---- >8 ----
From a0003ebfeb15f91094d17961633cabb4e1beed21 Mon Sep 17 00:00:00 2001
From: Thierry Reding <treding@nvidia.com>
Date: Fri, 7 Aug 2015 14:23:42 +0200
Subject: [PATCH] mm: Fix VM_LOCKONFAULT clash with VM_UFFD_WP
Currently two patches in linux-next add new VM flags and unfortunately
two flags end up using the same value. This results for example in the
/proc/pid/smaps file not listing the VM_LOCKONFAULT flag, which breaks
tools/testing/selftests/vm/mlock2-tests.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
fs/proc/task_mmu.c | 2 +-
include/linux/mm.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index bdd7e48a85f0..893e4b9bb2da 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -592,13 +592,13 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
#ifdef CONFIG_X86_INTEL_MPX
[ilog2(VM_MPX)] = "mp",
#endif
- [ilog2(VM_LOCKONFAULT)] = "lf",
[ilog2(VM_LOCKED)] = "lo",
[ilog2(VM_IO)] = "io",
[ilog2(VM_SEQ_READ)] = "sr",
[ilog2(VM_RAND_READ)] = "rr",
[ilog2(VM_DONTCOPY)] = "dc",
[ilog2(VM_DONTEXPAND)] = "de",
+ [ilog2(VM_LOCKONFAULT)] = "lf",
[ilog2(VM_ACCOUNT)] = "ac",
[ilog2(VM_NORESERVE)] = "nr",
[ilog2(VM_HUGETLB)] = "ht",
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 363ea2cda35f..cb4e1737d669 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -129,7 +129,6 @@ extern unsigned int kobjsize(const void *objp);
#define VM_DENYWRITE 0x00000800 /* ETXTBSY on write attempts.. */
#define VM_UFFD_WP 0x00001000 /* wrprotect pages tracking */
-#define VM_LOCKONFAULT 0x00001000 /* Lock the pages covered when they are faulted in */
#define VM_LOCKED 0x00002000
#define VM_IO 0x00004000 /* Memory mapped I/O or similar */
@@ -139,6 +138,7 @@ extern unsigned int kobjsize(const void *objp);
#define VM_DONTCOPY 0x00020000 /* Do not copy this vma on fork */
#define VM_DONTEXPAND 0x00040000 /* Cannot expand with mremap() */
+#define VM_LOCKONFAULT 0x00080000 /* Lock the pages covered when they are faulted in */
#define VM_ACCOUNT 0x00100000 /* Is a VM accounted object */
#define VM_NORESERVE 0x00200000 /* should the VM suppress accounting */
#define VM_HUGETLB 0x00400000 /* Huge TLB Page VM */
--
2.4.5
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related
* Re: [PATCH V6 4/6] mm: mlock: Add mlock flags to enable VM_LOCKONFAULT usage
From: Vlastimil Babka @ 2015-08-07 11:50 UTC (permalink / raw)
To: Eric B Munson, Andrew Morton
Cc: Michal Hocko, Jonathan Corbet, Kirill A. Shutemov,
linux-alpha-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mips-6z/3iImG2C8G8FEW9MqTrA,
linux-parisc-u79uwXL29TY76Z2rM5mHXA,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
sparclinux-u79uwXL29TY76Z2rM5mHXA,
linux-xtensa-PjhNF2WwrV/0Sa2dR60CXw,
linux-arch-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg
In-Reply-To: <1438184575-10537-5-git-send-email-emunson-JqFfY2XvxFXQT0dZR+AlfA@public.gmane.org>
On 07/29/2015 05:42 PM, Eric B Munson wrote:
> The previous patch introduced a flag that specified pages in a VMA
> should be placed on the unevictable LRU, but they should not be made
> present when the area is created. This patch adds the ability to set
> this state via the new mlock system calls.
>
> We add MLOCK_ONFAULT for mlock2 and MCL_ONFAULT for mlockall.
> MLOCK_ONFAULT will set the VM_LOCKONFAULT modifier for VM_LOCKED.
> MCL_ONFAULT should be used as a modifier to the two other mlockall
> flags. When used with MCL_CURRENT, all current mappings will be marked
> with VM_LOCKED | VM_LOCKONFAULT. When used with MCL_FUTURE, the
> mm->def_flags will be marked with VM_LOCKED | VM_LOCKONFAULT. When used
> with both MCL_CURRENT and MCL_FUTURE, all current mappings and
> mm->def_flags will be marked with VM_LOCKED | VM_LOCKONFAULT.
>
> Prior to this patch, mlockall() will unconditionally clear the
> mm->def_flags any time it is called without MCL_FUTURE. This behavior
> is maintained after adding MCL_ONFAULT. If a call to
> mlockall(MCL_FUTURE) is followed by mlockall(MCL_CURRENT), the
> mm->def_flags will be cleared and new VMAs will be unlocked. This
> remains true with or without MCL_ONFAULT in either mlockall()
> invocation.
>
> munlock() will unconditionally clear both vma flags. munlockall()
> unconditionally clears for VMA flags on all VMAs and in the
> mm->def_flags field.
>
> Signed-off-by: Eric B Munson <emunson-JqFfY2XvxFXQT0dZR+AlfA@public.gmane.org>
> Cc: Michal Hocko <mhocko-AlSwsSmVLrQ@public.gmane.org>
> Cc: Vlastimil Babka <vbabka-AlSwsSmVLrQ@public.gmane.org>
The logic seems ok, although the fact that apply_mlockall_flags() is
shared by both mlockall and munlockall makes it even more subtle than
before :)
Anyway, just some nitpicks below.
Acked-by: Vlastimil Babka <vbabka-AlSwsSmVLrQ@public.gmane.org>
[...]
> +/*
> + * Take the MCL_* flags passed into mlockall (or 0 if called from munlockall)
> + * and translate into the appropriate modifications to mm->def_flags and/or the
> + * flags for all current VMAs.
> + *
> + * There are a couple of sublties with this. If mlockall() is called multiple
^ typo
> + * times with different flags, the values do not necessarily stack. If mlockall
> + * is called once including the MCL_FUTURE flag and then a second time without
> + * it, VM_LOCKED and VM_LOCKONFAULT will be cleared from mm->def_flags.
> + */
> static int apply_mlockall_flags(int flags)
> {
> struct vm_area_struct * vma, * prev = NULL;
> + vm_flags_t to_add = 0;
>
> - if (flags & MCL_FUTURE)
> + current->mm->def_flags &= ~(VM_LOCKED | VM_LOCKONFAULT);
> + if (flags & MCL_FUTURE) {
> current->mm->def_flags |= VM_LOCKED;
> - else
> - current->mm->def_flags &= ~VM_LOCKED;
>
> - if (flags == MCL_FUTURE)
> - goto out;
> + if (flags & MCL_ONFAULT)
> + current->mm->def_flags |= VM_LOCKONFAULT;
> +
> + /*
> + * When there were only two flags, we used to early out if only
> + * MCL_FUTURE was set. Now that we have MCL_ONFAULT, we can
> + * only early out if MCL_FUTURE is set, but MCL_CURRENT is not.
Describing the relation to history of individual code lines in such
detail is noise imho. The stacking subtleties is already described above.
> + * This is done, even though it promotes odd behavior, to
> + * maintain behavior from older kernels
> + */
> + if (!(flags & MCL_CURRENT))
> + goto out;
^ permalink raw reply
* Re: [PATCH V6 3/6] mm: Introduce VM_LOCKONFAULT
From: Eric B Munson @ 2015-08-07 10:33 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Andrew Morton, Michal Hocko, Jonathan Corbet, Kirill A. Shutemov,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <55C37E62.6020909-AlSwsSmVLrQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2892 bytes --]
On Thu, 06 Aug 2015, Vlastimil Babka wrote:
...
> >
> >diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> >index ca1e091..38d69fc 100644
> >--- a/fs/proc/task_mmu.c
> >+++ b/fs/proc/task_mmu.c
> >@@ -579,6 +579,7 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
>
> This function has the following comment:
>
> Don't forget to update Documentation/ on changes.
>
> [...]
>
> >--- a/mm/gup.c
> >+++ b/mm/gup.c
> >@@ -92,7 +92,7 @@ retry:
> > */
> > mark_page_accessed(page);
> > }
> >- if ((flags & FOLL_POPULATE) && (vma->vm_flags & VM_LOCKED)) {
> >+ if ((flags & FOLL_MLOCK) && (vma->vm_flags & VM_LOCKED)) {
> > /*
> > * The preliminary mapping check is mainly to avoid the
> > * pointless overhead of lock_page on the ZERO_PAGE
> >@@ -265,6 +265,9 @@ static int faultin_page(struct task_struct *tsk, struct vm_area_struct *vma,
> > unsigned int fault_flags = 0;
> > int ret;
> >
> >+ /* mlock all present pages, but do not fault in new pages */
> >+ if ((*flags & (FOLL_POPULATE | FOLL_MLOCK)) == FOLL_MLOCK)
> >+ return -ENOENT;
> > /* For mm_populate(), just skip the stack guard page. */
> > if ((*flags & FOLL_POPULATE) &&
> > (stack_guard_page_start(vma, address) ||
> >@@ -850,7 +853,10 @@ long populate_vma_page_range(struct vm_area_struct *vma,
> > VM_BUG_ON_VMA(end > vma->vm_end, vma);
> > VM_BUG_ON_MM(!rwsem_is_locked(&mm->mmap_sem), mm);
> >
> >- gup_flags = FOLL_TOUCH | FOLL_POPULATE;
> >+ gup_flags = FOLL_TOUCH | FOLL_MLOCK;
> >+ if ((vma->vm_flags & (VM_LOCKED | VM_LOCKONFAULT)) == VM_LOCKED)
> >+ gup_flags |= FOLL_POPULATE;
> >+
> > /*
> > * We want to touch writable mappings with a write fault in order
> > * to break COW, except for shared mappings because these don't COW
>
> I think this might be breaking the populate part of
> mmap(MAP_POPULATE & ~MAP_LOCKED) case, if I follow the execution
> correctly (it's far from simple...)
>
> SYSCALL_DEFINE6(mmap_pgoff... with MAP_POPULATE
> vm_mmap_pgoff(..., MAP_POPULATE...)
> do_mmap_pgoff(...MAP_POPULATE... &populate) -> populate == TRUE
> mm_populate()
> __mm_populate()
> populate_vma_page_range()
>
> Previously, this path would have FOLL_POPULATE in gup_flags and
> continue with __get_user_pages() and faultin_page() (actually
> regardless of FOLL_POPULATE) which would fault in the pages.
>
> After your patch, populate_vma_page_range() will set FOLL_MLOCK, but
> since VM_LOCKED is not set, FOLL_POPULATE won't be set either.
> Then faultin_page() will return on the new check:
>
> flags & (FOLL_POPULATE | FOLL_MLOCK)) == FOLL_MLOCK
>
>
I am on vacation atm but I will try and get to respin this series after
making sure there aren't any more FOLL flag issues.
Thanks for keeping with these :)
Eric
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v3 0/4] enhance shmem process and swap accounting
From: Jerome Marchand @ 2015-08-07 9:37 UTC (permalink / raw)
To: Vlastimil Babka, Andrew Morton
Cc: linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Hugh Dickins, Michal Hocko,
Kirill A. Shutemov, Cyrill Gorcunov, Randy Dunlap,
linux-s390-u79uwXL29TY76Z2rM5mHXA, Martin Schwidefsky,
Heiko Carstens, Peter Zijlstra, Paul Mackerras,
Arnaldo Carvalho de Melo, Oleg Nesterov, Linux API,
Konstantin Khlebnikov, Minchan Kim
In-Reply-To: <1438779685-5227-1-git-send-email-vbabka-AlSwsSmVLrQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 5917 bytes --]
On 08/05/2015 03:01 PM, Vlastimil Babka wrote:
> Reposting due to lack of feedback in May. I hope at least patches 1 and 2
> could be merged as they are IMHO bugfixes. 3 and 4 is optional but IMHO useful.
>
> Changes since v2:
> o Rebase on next-20150805.
> o This means that /proc/pid/maps has the proportional swap share (SwapPss:)
> field as per https://lkml.org/lkml/2015/6/15/274
> It's not clear what to do with shmem here so it's 0 for now.
> - swapped out shmem doesn't have swap entries, so we would have to look at who
> else has the shmem object (partially) mapped
> - to be more precise we should also check if his range actually includes
> the offset in question, which could get rather involved
> - or is there some easy way I don't see?
Hmm... This is much more difficult than I envision when commenting on
Minchan patch. One possibility could be to have the pte of paged out
shmem pages set in a similar way than regular swap entry are. But that
would need to use some very precious estate on the pte.
As it is, a zero value, while obviously wrong, has the advantage of not
being misleading like a bad approximation would be (like the kind which
doesn't properly accounts for partial mapping).
Jerome
> o Konstantin suggested for patch 3/4 that I drop the CONFIG_SHMEM #ifdefs
> I didn't see the point in going against tinyfication when the work is
> already done, but I can do that if more people think it's better and it
> would block the series.
>
> Changes since v1:
> o In Patch 2, rely on SHMEM_I(inode)->swapped if possible, and fallback to
> radix tree iterator on partially mapped shmem objects, i.e. decouple shmem
> swap usage determination from the page walk, for performance reasons.
> Thanks to Jerome and Konstantin for the tips.
> The downside is that mm/shmem.c had to be touched.
>
> This series is based on Jerome Marchand's [1] so let me quote the first
> paragraph from there:
>
> There are several shortcomings with the accounting of shared memory
> (sysV shm, shared anonymous mapping, mapping to a tmpfs file). The
> values in /proc/<pid>/status and statm don't allow to distinguish
> between shmem memory and a shared mapping to a regular file, even
> though theirs implication on memory usage are quite different: at
> reclaim, file mapping can be dropped or write back on disk while shmem
> needs a place in swap. As for shmem pages that are swapped-out or in
> swap cache, they aren't accounted at all.
>
> The original motivation for myself is that a customer found (IMHO rightfully)
> confusing that e.g. top output for process swap usage is unreliable with
> respect to swapped out shmem pages, which are not accounted for.
>
> The fundamental difference between private anonymous and shmem pages is that
> the latter has PTE's converted to pte_none, and not swapents. As such, they are
> not accounted to the number of swapents visible e.g. in /proc/pid/status VmSwap
> row. It might be theoretically possible to use swapents when swapping out shmem
> (without extra cost, as one has to change all mappers anyway), and on swap in
> only convert the swapent for the faulting process, leaving swapents in other
> processes until they also fault (so again no extra cost). But I don't know how
> many assumptions this would break, and it would be too disruptive change for a
> relatively small benefit.
>
> Instead, my approach is to document the limitation of VmSwap, and provide means
> to determine the swap usage for shmem areas for those who are interested and
> willing to pay the price, using /proc/pid/smaps. Because outside of ipcs, I
> don't think it's possible to currently to determine the usage at all. The
> previous patchset [1] did introduce new shmem-specific fields into smaps
> output, and functions to determine the values. I take a simpler approach,
> noting that smaps output already has a "Swap: X kB" line, where currently X ==
> 0 always for shmem areas. I think we can just consider this a bug and provide
> the proper value by consulting the radix tree, as e.g. mincore_page() does. In the
> patch changelog I explain why this is also not perfect (and cannot be without
> swapents), but still arguably much better than showing a 0.
>
> The last two patches are adapted from Jerome's patchset and provide a VmRSS
> breakdown to VmAnon, VmFile and VmShm in /proc/pid/status. Hugh noted that
> this is a welcome addition, and I agree that it might help e.g. debugging
> process memory usage at albeit non-zero, but still rather low cost of extra
> per-mm counter and some page flag checks. I updated these patches to 4.0-rc1,
> made them respect !CONFIG_SHMEM so that tiny systems don't pay the cost, and
> optimized the page flag checking somewhat.
>
> [1] http://lwn.net/Articles/611966/
>
> Jerome Marchand (2):
> mm, shmem: Add shmem resident memory accounting
> mm, procfs: Display VmAnon, VmFile and VmShm in /proc/pid/status
>
> Vlastimil Babka (2):
> mm, documentation: clarify /proc/pid/status VmSwap limitations
> mm, proc: account for shmem swap in /proc/pid/smaps
>
> Documentation/filesystems/proc.txt | 18 ++++++++++---
> arch/s390/mm/pgtable.c | 5 +---
> fs/proc/task_mmu.c | 52 ++++++++++++++++++++++++++++++++++--
> include/linux/mm.h | 28 ++++++++++++++++++++
> include/linux/mm_types.h | 9 ++++---
> include/linux/shmem_fs.h | 6 +++++
> kernel/events/uprobes.c | 2 +-
> mm/memory.c | 30 +++++++--------------
> mm/oom_kill.c | 5 ++--
> mm/rmap.c | 15 +++--------
> mm/shmem.c | 54 ++++++++++++++++++++++++++++++++++++++
> 11 files changed, 178 insertions(+), 46 deletions(-)
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH V6 3/6] mm: Introduce VM_LOCKONFAULT
From: Kirill A. Shutemov @ 2015-08-06 15:53 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Eric B Munson, Andrew Morton, Michal Hocko, Jonathan Corbet,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <55C37E62.6020909-AlSwsSmVLrQ@public.gmane.org>
On Thu, Aug 06, 2015 at 05:33:54PM +0200, Vlastimil Babka wrote:
> On 07/29/2015 05:42 PM, Eric B Munson wrote:
> >The cost of faulting in all memory to be locked can be very high when
> >working with large mappings. If only portions of the mapping will be
> >used this can incur a high penalty for locking.
> >
> >For the example of a large file, this is the usage pattern for a large
> >statical language model (probably applies to other statical or graphical
> >models as well). For the security example, any application transacting
> >in data that cannot be swapped out (credit card data, medical records,
> >etc).
> >
> >This patch introduces the ability to request that pages are not
> >pre-faulted, but are placed on the unevictable LRU when they are finally
> >faulted in. The VM_LOCKONFAULT flag will be used together with
> >VM_LOCKED and has no effect when set without VM_LOCKED. Setting the
> >VM_LOCKONFAULT flag for a VMA will cause pages faulted into that VMA to
> >be added to the unevictable LRU when they are faulted or if they are
> >already present, but will not cause any missing pages to be faulted in.
> >
> >Exposing this new lock state means that we cannot overload the meaning
> >of the FOLL_POPULATE flag any longer. Prior to this patch it was used
> >to mean that the VMA for a fault was locked. This means we need the
> >new FOLL_MLOCK flag to communicate the locked state of a VMA.
> >FOLL_POPULATE will now only control if the VMA should be populated and
> >in the case of VM_LOCKONFAULT, it will not be set.
> >
> >Signed-off-by: Eric B Munson <emunson-JqFfY2XvxFXQT0dZR+AlfA@public.gmane.org>
> >Acked-by: Kirill A. Shutemov <kirill.shutemov-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> >Cc: Michal Hocko <mhocko-AlSwsSmVLrQ@public.gmane.org>
> >Cc: Vlastimil Babka <vbabka-AlSwsSmVLrQ@public.gmane.org>
> >Cc: Jonathan Corbet <corbet-T1hC0tSOHrs@public.gmane.org>
> >Cc: "Kirill A. Shutemov" <kirill-oKw7cIdHH8eLwutG50LtGA@public.gmane.org>
> >Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> >Cc: linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org
> >Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >---
> > drivers/gpu/drm/drm_vm.c | 8 +++++++-
> > fs/proc/task_mmu.c | 1 +
> > include/linux/mm.h | 2 ++
> > kernel/fork.c | 2 +-
> > mm/debug.c | 1 +
> > mm/gup.c | 10 ++++++++--
> > mm/huge_memory.c | 2 +-
> > mm/hugetlb.c | 4 ++--
> > mm/mlock.c | 2 +-
> > mm/mmap.c | 2 +-
> > mm/rmap.c | 4 ++--
> > 11 files changed, 27 insertions(+), 11 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
> >index aab49ee..103a5f6 100644
> >--- a/drivers/gpu/drm/drm_vm.c
> >+++ b/drivers/gpu/drm/drm_vm.c
> >@@ -699,9 +699,15 @@ int drm_vma_info(struct seq_file *m, void *data)
> > (void *)(unsigned long)virt_to_phys(high_memory));
> >
> > list_for_each_entry(pt, &dev->vmalist, head) {
> >+ char lock_flag = '-';
> >+
> > vma = pt->vma;
> > if (!vma)
> > continue;
> >+ if (vma->vm_flags & VM_LOCKONFAULT)
> >+ lock_flag = 'f';
> >+ else if (vma->vm_flags & VM_LOCKED)
> >+ lock_flag = 'l';
> > seq_printf(m,
> > "\n%5d 0x%pK-0x%pK %c%c%c%c%c%c 0x%08lx000",
> > pt->pid,
> >@@ -710,7 +716,7 @@ int drm_vma_info(struct seq_file *m, void *data)
> > vma->vm_flags & VM_WRITE ? 'w' : '-',
> > vma->vm_flags & VM_EXEC ? 'x' : '-',
> > vma->vm_flags & VM_MAYSHARE ? 's' : 'p',
> >- vma->vm_flags & VM_LOCKED ? 'l' : '-',
> >+ lock_flag,
> > vma->vm_flags & VM_IO ? 'i' : '-',
> > vma->vm_pgoff);
> >
> >diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> >index ca1e091..38d69fc 100644
> >--- a/fs/proc/task_mmu.c
> >+++ b/fs/proc/task_mmu.c
> >@@ -579,6 +579,7 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
>
> This function has the following comment:
>
> Don't forget to update Documentation/ on changes.
>
> [...]
>
> >--- a/mm/gup.c
> >+++ b/mm/gup.c
> >@@ -92,7 +92,7 @@ retry:
> > */
> > mark_page_accessed(page);
> > }
> >- if ((flags & FOLL_POPULATE) && (vma->vm_flags & VM_LOCKED)) {
> >+ if ((flags & FOLL_MLOCK) && (vma->vm_flags & VM_LOCKED)) {
> > /*
> > * The preliminary mapping check is mainly to avoid the
> > * pointless overhead of lock_page on the ZERO_PAGE
> >@@ -265,6 +265,9 @@ static int faultin_page(struct task_struct *tsk, struct vm_area_struct *vma,
> > unsigned int fault_flags = 0;
> > int ret;
> >
> >+ /* mlock all present pages, but do not fault in new pages */
> >+ if ((*flags & (FOLL_POPULATE | FOLL_MLOCK)) == FOLL_MLOCK)
> >+ return -ENOENT;
> > /* For mm_populate(), just skip the stack guard page. */
> > if ((*flags & FOLL_POPULATE) &&
> > (stack_guard_page_start(vma, address) ||
> >@@ -850,7 +853,10 @@ long populate_vma_page_range(struct vm_area_struct *vma,
> > VM_BUG_ON_VMA(end > vma->vm_end, vma);
> > VM_BUG_ON_MM(!rwsem_is_locked(&mm->mmap_sem), mm);
> >
> >- gup_flags = FOLL_TOUCH | FOLL_POPULATE;
> >+ gup_flags = FOLL_TOUCH | FOLL_MLOCK;
> >+ if ((vma->vm_flags & (VM_LOCKED | VM_LOCKONFAULT)) == VM_LOCKED)
> >+ gup_flags |= FOLL_POPULATE;
> >+
> > /*
> > * We want to touch writable mappings with a write fault in order
> > * to break COW, except for shared mappings because these don't COW
>
> I think this might be breaking the populate part of mmap(MAP_POPULATE &
> ~MAP_LOCKED) case, if I follow the execution correctly (it's far from
> simple...)
>
> SYSCALL_DEFINE6(mmap_pgoff... with MAP_POPULATE
> vm_mmap_pgoff(..., MAP_POPULATE...)
> do_mmap_pgoff(...MAP_POPULATE... &populate) -> populate == TRUE
> mm_populate()
> __mm_populate()
> populate_vma_page_range()
>
> Previously, this path would have FOLL_POPULATE in gup_flags and continue
> with __get_user_pages() and faultin_page() (actually regardless of
> FOLL_POPULATE) which would fault in the pages.
>
> After your patch, populate_vma_page_range() will set FOLL_MLOCK, but since
> VM_LOCKED is not set, FOLL_POPULATE won't be set either.
> Then faultin_page() will return on the new check:
>
> flags & (FOLL_POPULATE | FOLL_MLOCK)) == FOLL_MLOCK
Good catch!
I guess it should be something like:
gup_flags = FOLL_TOUCH | FOLL_POPULATE | FOLL_MLOCK;
if (vma->vm_flags & VM_LOCKONFAULT)
gup_flags &= ~FOLL_POPULATE;
--
Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH V6 3/6] mm: Introduce VM_LOCKONFAULT
From: Vlastimil Babka @ 2015-08-06 15:33 UTC (permalink / raw)
To: Eric B Munson, Andrew Morton
Cc: Michal Hocko, Jonathan Corbet, Kirill A. Shutemov, linux-kernel,
dri-devel, linux-mm, linux-api
In-Reply-To: <1438184575-10537-4-git-send-email-emunson@akamai.com>
On 07/29/2015 05:42 PM, Eric B Munson wrote:
> The cost of faulting in all memory to be locked can be very high when
> working with large mappings. If only portions of the mapping will be
> used this can incur a high penalty for locking.
>
> For the example of a large file, this is the usage pattern for a large
> statical language model (probably applies to other statical or graphical
> models as well). For the security example, any application transacting
> in data that cannot be swapped out (credit card data, medical records,
> etc).
>
> This patch introduces the ability to request that pages are not
> pre-faulted, but are placed on the unevictable LRU when they are finally
> faulted in. The VM_LOCKONFAULT flag will be used together with
> VM_LOCKED and has no effect when set without VM_LOCKED. Setting the
> VM_LOCKONFAULT flag for a VMA will cause pages faulted into that VMA to
> be added to the unevictable LRU when they are faulted or if they are
> already present, but will not cause any missing pages to be faulted in.
>
> Exposing this new lock state means that we cannot overload the meaning
> of the FOLL_POPULATE flag any longer. Prior to this patch it was used
> to mean that the VMA for a fault was locked. This means we need the
> new FOLL_MLOCK flag to communicate the locked state of a VMA.
> FOLL_POPULATE will now only control if the VMA should be populated and
> in the case of VM_LOCKONFAULT, it will not be set.
>
> Signed-off-by: Eric B Munson <emunson@akamai.com>
> Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: Michal Hocko <mhocko@suse.cz>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: "Kirill A. Shutemov" <kirill@shutemov.name>
> Cc: linux-kernel@vger.kernel.org
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-mm@kvack.org
> Cc: linux-api@vger.kernel.org
> ---
> drivers/gpu/drm/drm_vm.c | 8 +++++++-
> fs/proc/task_mmu.c | 1 +
> include/linux/mm.h | 2 ++
> kernel/fork.c | 2 +-
> mm/debug.c | 1 +
> mm/gup.c | 10 ++++++++--
> mm/huge_memory.c | 2 +-
> mm/hugetlb.c | 4 ++--
> mm/mlock.c | 2 +-
> mm/mmap.c | 2 +-
> mm/rmap.c | 4 ++--
> 11 files changed, 27 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
> index aab49ee..103a5f6 100644
> --- a/drivers/gpu/drm/drm_vm.c
> +++ b/drivers/gpu/drm/drm_vm.c
> @@ -699,9 +699,15 @@ int drm_vma_info(struct seq_file *m, void *data)
> (void *)(unsigned long)virt_to_phys(high_memory));
>
> list_for_each_entry(pt, &dev->vmalist, head) {
> + char lock_flag = '-';
> +
> vma = pt->vma;
> if (!vma)
> continue;
> + if (vma->vm_flags & VM_LOCKONFAULT)
> + lock_flag = 'f';
> + else if (vma->vm_flags & VM_LOCKED)
> + lock_flag = 'l';
> seq_printf(m,
> "\n%5d 0x%pK-0x%pK %c%c%c%c%c%c 0x%08lx000",
> pt->pid,
> @@ -710,7 +716,7 @@ int drm_vma_info(struct seq_file *m, void *data)
> vma->vm_flags & VM_WRITE ? 'w' : '-',
> vma->vm_flags & VM_EXEC ? 'x' : '-',
> vma->vm_flags & VM_MAYSHARE ? 's' : 'p',
> - vma->vm_flags & VM_LOCKED ? 'l' : '-',
> + lock_flag,
> vma->vm_flags & VM_IO ? 'i' : '-',
> vma->vm_pgoff);
>
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index ca1e091..38d69fc 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -579,6 +579,7 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
This function has the following comment:
Don't forget to update Documentation/ on changes.
[...]
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -92,7 +92,7 @@ retry:
> */
> mark_page_accessed(page);
> }
> - if ((flags & FOLL_POPULATE) && (vma->vm_flags & VM_LOCKED)) {
> + if ((flags & FOLL_MLOCK) && (vma->vm_flags & VM_LOCKED)) {
> /*
> * The preliminary mapping check is mainly to avoid the
> * pointless overhead of lock_page on the ZERO_PAGE
> @@ -265,6 +265,9 @@ static int faultin_page(struct task_struct *tsk, struct vm_area_struct *vma,
> unsigned int fault_flags = 0;
> int ret;
>
> + /* mlock all present pages, but do not fault in new pages */
> + if ((*flags & (FOLL_POPULATE | FOLL_MLOCK)) == FOLL_MLOCK)
> + return -ENOENT;
> /* For mm_populate(), just skip the stack guard page. */
> if ((*flags & FOLL_POPULATE) &&
> (stack_guard_page_start(vma, address) ||
> @@ -850,7 +853,10 @@ long populate_vma_page_range(struct vm_area_struct *vma,
> VM_BUG_ON_VMA(end > vma->vm_end, vma);
> VM_BUG_ON_MM(!rwsem_is_locked(&mm->mmap_sem), mm);
>
> - gup_flags = FOLL_TOUCH | FOLL_POPULATE;
> + gup_flags = FOLL_TOUCH | FOLL_MLOCK;
> + if ((vma->vm_flags & (VM_LOCKED | VM_LOCKONFAULT)) == VM_LOCKED)
> + gup_flags |= FOLL_POPULATE;
> +
> /*
> * We want to touch writable mappings with a write fault in order
> * to break COW, except for shared mappings because these don't COW
I think this might be breaking the populate part of mmap(MAP_POPULATE &
~MAP_LOCKED) case, if I follow the execution correctly (it's far from
simple...)
SYSCALL_DEFINE6(mmap_pgoff... with MAP_POPULATE
vm_mmap_pgoff(..., MAP_POPULATE...)
do_mmap_pgoff(...MAP_POPULATE... &populate) -> populate == TRUE
mm_populate()
__mm_populate()
populate_vma_page_range()
Previously, this path would have FOLL_POPULATE in gup_flags and continue
with __get_user_pages() and faultin_page() (actually regardless of
FOLL_POPULATE) which would fault in the pages.
After your patch, populate_vma_page_range() will set FOLL_MLOCK, but
since VM_LOCKED is not set, FOLL_POPULATE won't be set either.
Then faultin_page() will return on the new check:
flags & (FOLL_POPULATE | FOLL_MLOCK)) == FOLL_MLOCK
--
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 V6 2/6] mm: mlock: Add new mlock system call
From: Vlastimil Babka @ 2015-08-06 14:00 UTC (permalink / raw)
To: Eric B Munson, Andrew Morton
Cc: Michal Hocko, Heiko Carstens, Geert Uytterhoeven, Catalin Marinas,
Stephen Rothwell, Guenter Roeck, linux-alpha, linux-kernel,
linux-arm-kernel, adi-buildroot-devel, linux-cris-kernel,
linux-ia64, linux-m68k, linux-am33-list, linux-parisc,
linuxppc-dev, linux-s390, linux-sh, sparclinux, linux-xtensa,
linux-api, linux-arch, linux-mm
In-Reply-To: <1438184575-10537-3-git-send-email-emunson@akamai.com>
On 07/29/2015 05:42 PM, Eric B Munson wrote:
> With the refactored mlock code, introduce a new system call for mlock.
> The new call will allow the user to specify what lock states are being
> added. mlock2 is trivial at the moment, but a follow on patch will add
> a new mlock state making it useful.
>
> Signed-off-by: Eric B Munson <emunson@akamai.com>
> Cc: Michal Hocko <mhocko@suse.cz>
> Cc: Vlastimil Babka <vbabka@suse.cz>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
--
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: Persistent Reservation API
From: Christoph Hellwig @ 2015-08-06 13:45 UTC (permalink / raw)
To: Keith Busch
Cc: Christoph Hellwig, Jens Axboe, linux-api, dm-devel, linux-nvme,
linux-scsi, linux-kernel
In-Reply-To: <alpine.LNX.2.00.1508041742360.15930@localhost.lm.intel.com>
On Tue, Aug 04, 2015 at 06:04:04PM +0000, Keith Busch wrote:
> On Tue, 4 Aug 2015, Christoph Hellwig wrote:
>> NVMe support currently isn't included as I don't have a multihost
>> NVMe setup to test on, but if I can find a volunteer to test it I'm
>> happy to write the code for it.
>
> Looks pretty good so far. I'd be happy to give try it out with NVMe
> subsystems.
Thanks, I'll prepare a version once I get a few free minutes.
^ permalink raw reply
* Re: [PATCH 5/6] dm: split out a helper to find the ioctl target
From: Christoph Hellwig @ 2015-08-06 13:45 UTC (permalink / raw)
To: Mike Snitzer
Cc: Christoph Hellwig, Jens Axboe, linux-api-u79uwXL29TY76Z2rM5mHXA,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-scsi-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150804180558.GB28971-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Tue, Aug 04, 2015 at 02:05:59PM -0400, Mike Snitzer wrote:
> static int dm_get_live_table_for_ioctl(struct mapped_device *md, int *srcu_idx,
> struct dm_target **tgt, struct block_device **bdev, fmode_t *mode)
>
> Otherwise, looks good.
I'll update it.
^ permalink raw reply
* Re: [PATCH 4/6] dm: refactor ioctl handling
From: Christoph Hellwig @ 2015-08-06 13:45 UTC (permalink / raw)
To: Mike Snitzer
Cc: Christoph Hellwig, Jens Axboe, linux-api, dm-devel, linux-nvme,
linux-scsi, linux-kernel
In-Reply-To: <20150804175616.GA28971@redhat.com>
On Tue, Aug 04, 2015 at 01:56:16PM -0400, Mike Snitzer wrote:
> This should be renamed to dm_prepare_ioctl_fn and the targets' hook
> would be .prepare_ioctl
>
> Open to other names but if the targets no longer issue the ioctl there
> is little point to call it .ioctl
Sure. I had ioctl_allowed first but that sounded so ugly that I
undid the renaming..
^ permalink raw reply
* Re: Persistent Reservation API
From: Christoph Hellwig @ 2015-08-06 13:44 UTC (permalink / raw)
To: Mike Snitzer
Cc: Christoph Hellwig, Jens Axboe, linux-api, dm-devel, linux-nvme,
linux-scsi, linux-kernel
In-Reply-To: <20150804175349.GA29013@redhat.com>
On Tue, Aug 04, 2015 at 01:53:49PM -0400, Mike Snitzer wrote:
> The DM changes need to go through linux-dm.git. Once the block and SCSI
> bits land I'll rebase accordingly. That cross-maintainer logisitics
> aside, I'll reply with feedback on the DM patches.
That sounds fine to me, thanks!
^ permalink raw reply
* Re: [PATCH v2] iio: Documentation: Add trigger name attribute ABI documentation
From: Daniel Baluta @ 2015-08-06 12:00 UTC (permalink / raw)
To: Cristina Opriceana
Cc: Jonathan Cameron, Daniel Baluta, linux-api-u79uwXL29TY76Z2rM5mHXA,
Linux Kernel Mailing List,
linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <7d408fb8def6478565e30d01edcfc98d9cf42b48.1438859058.git.cristina.opriceana-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Thu, Aug 6, 2015 at 2:17 PM, Cristina Opriceana
<cristina.opriceana-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> This patch adds an entry in ABI Documentation for the name attribute
> issued when a trigger is created.
>
> Signed-off-by: Cristina Opriceana <cristina.opriceana-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Acked-by: Daniel Baluta <daniel.baluta-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
> Changes in v2:
> - deviceX to deviceY, to avoid confusion
> - clearer description
>
> Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs b/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs
> index 5235e6c..0c1e843 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs
> @@ -9,3 +9,12 @@ Description:
> automated testing or in situations, where other trigger methods
> are not applicable. For example no RTC or spare GPIOs.
> X is the IIO index of the trigger.
> +
> +What: /sys/bus/iio/devices/triggerX/name
> +KernelVersion: 2.6.39
> +Contact: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> +Description:
> + The name attribute holds a description string for the current
> + trigger. In order to associate the trigger with an IIO device
> + one should write this name string to
> + /sys/bus/iio/devices/iio:deviceY/trigger/current_trigger.
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2] iio: Documentation: Add trigger name attribute ABI documentation
From: Cristina Opriceana @ 2015-08-06 11:17 UTC (permalink / raw)
To: jic23; +Cc: daniel.baluta, linux-api, linux-kernel, linux-iio
This patch adds an entry in ABI Documentation for the name attribute
issued when a trigger is created.
Signed-off-by: Cristina Opriceana <cristina.opriceana@gmail.com>
---
Changes in v2:
- deviceX to deviceY, to avoid confusion
- clearer description
Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs b/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs
index 5235e6c..0c1e843 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs
+++ b/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs
@@ -9,3 +9,12 @@ Description:
automated testing or in situations, where other trigger methods
are not applicable. For example no RTC or spare GPIOs.
X is the IIO index of the trigger.
+
+What: /sys/bus/iio/devices/triggerX/name
+KernelVersion: 2.6.39
+Contact: linux-iio@vger.kernel.org
+Description:
+ The name attribute holds a description string for the current
+ trigger. In order to associate the trigger with an IIO device
+ one should write this name string to
+ /sys/bus/iio/devices/iio:deviceY/trigger/current_trigger.
--
1.9.1
^ permalink raw reply related
* [PATCH v9 5/6] block: loop: introduce ioctl command of LOOP_SET_DIRECT_IO
From: Ming Lei @ 2015-08-06 8:42 UTC (permalink / raw)
To: Jens Axboe, linux-kernel, Dave Kleikamp
Cc: Zach Brown, Christoph Hellwig, Maxim Patlasov, Andrew Morton,
Alexander Viro, Tejun Heo, Dave Chinner, Ming Lei, linux-api
In-Reply-To: <1438850538-15682-1-git-send-email-ming.lei@canonical.com>
If loop block is mounted via 'mount -o loop', it isn't easy
to pass file descriptor opened as O_DIRECT, so this patch
introduces a new command to support direct IO for this case.
Cc: linux-api@vger.kernel.org
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/block/loop.c | 19 +++++++++++++++++++
include/uapi/linux/loop.h | 1 +
2 files changed, 20 insertions(+)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index a56f973..06a737b 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1214,6 +1214,20 @@ static int loop_set_capacity(struct loop_device *lo, struct block_device *bdev)
return figure_loop_size(lo, lo->lo_offset, lo->lo_sizelimit);
}
+static int loop_set_dio(struct loop_device *lo, unsigned long arg)
+{
+ int error = -ENXIO;
+ if (lo->lo_state != Lo_bound)
+ goto out;
+
+ __loop_update_dio(lo, !!arg);
+ if (lo->use_dio == !!arg)
+ return 0;
+ error = -EINVAL;
+ out:
+ return error;
+}
+
static int lo_ioctl(struct block_device *bdev, fmode_t mode,
unsigned int cmd, unsigned long arg)
{
@@ -1257,6 +1271,11 @@ static int lo_ioctl(struct block_device *bdev, fmode_t mode,
if ((mode & FMODE_WRITE) || capable(CAP_SYS_ADMIN))
err = loop_set_capacity(lo, bdev);
break;
+ case LOOP_SET_DIRECT_IO:
+ err = -EPERM;
+ if ((mode & FMODE_WRITE) || capable(CAP_SYS_ADMIN))
+ err = loop_set_dio(lo, arg);
+ break;
default:
err = lo->ioctl ? lo->ioctl(lo, cmd, arg) : -EINVAL;
}
diff --git a/include/uapi/linux/loop.h b/include/uapi/linux/loop.h
index 949851c..c8125ec 100644
--- a/include/uapi/linux/loop.h
+++ b/include/uapi/linux/loop.h
@@ -87,6 +87,7 @@ struct loop_info64 {
#define LOOP_GET_STATUS64 0x4C05
#define LOOP_CHANGE_FD 0x4C06
#define LOOP_SET_CAPACITY 0x4C07
+#define LOOP_SET_DIRECT_IO 0x4C08
/* /dev/loop-control interface */
#define LOOP_CTL_ADD 0x4C80
--
1.9.1
^ permalink raw reply related
* [PATCH v9 4/6] block: loop: prepare for supporing direct IO
From: Ming Lei @ 2015-08-06 8:42 UTC (permalink / raw)
To: Jens Axboe, linux-kernel, Dave Kleikamp
Cc: Zach Brown, Christoph Hellwig, Maxim Patlasov, Andrew Morton,
Alexander Viro, Tejun Heo, Dave Chinner, Ming Lei, linux-api
In-Reply-To: <1438850538-15682-1-git-send-email-ming.lei@canonical.com>
This patches provides one interface for enabling direct IO
from user space:
- userspace(such as losetup) can pass 'file' which is
opened/fcntl as O_DIRECT
Also __loop_update_dio() is introduced to check if direct I/O
can be used on current loop setting.
The last big change is to introduce LO_FLAGS_DIRECT_IO flag
for userspace to know if direct IO is used to access backing
file.
Cc: linux-api@vger.kernel.org
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
drivers/block/loop.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++-
drivers/block/loop.h | 2 ++
include/uapi/linux/loop.h | 1 +
3 files changed, 67 insertions(+), 1 deletion(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 1875aad..a56f973 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -164,6 +164,49 @@ static loff_t get_loop_size(struct loop_device *lo, struct file *file)
return get_size(lo->lo_offset, lo->lo_sizelimit, file);
}
+static void __loop_update_dio(struct loop_device *lo, bool dio)
+{
+ struct file *file = lo->lo_backing_file;
+ struct address_space *mapping = file->f_mapping;
+ struct inode *inode = mapping->host;
+ bool use_dio;
+ unsigned dio_align = inode->i_sb->s_bdev ?
+ (bdev_io_min(inode->i_sb->s_bdev) - 1) : 0;
+
+ /*
+ * We support direct I/O only if lo_offset is aligned
+ * with the min I/O size of backing device.
+ *
+ * Request's offset and size will be checked in I/O path.
+ */
+ if (dio) {
+ if (!(lo->lo_offset & dio_align) &&
+ mapping->a_ops->direct_IO)
+ use_dio = true;
+ else
+ use_dio = false;
+ } else {
+ use_dio = false;
+ }
+
+ /* flush dirty pages before changing direct IO */
+ vfs_fsync(file, 0);
+
+ /*
+ * The flag of LO_FLAGS_DIRECT_IO is handled similarly with
+ * LO_FLAGS_READ_ONLY, both are set from kernel, and losetup
+ * will get updated by ioctl(LOOP_GET_STATUS)
+ */
+ blk_mq_freeze_queue(lo->lo_queue);
+ lo->use_dio = use_dio;
+ lo->dio_align = dio_align;
+ if (use_dio)
+ lo->lo_flags |= LO_FLAGS_DIRECT_IO;
+ else
+ lo->lo_flags &= ~LO_FLAGS_DIRECT_IO;
+ blk_mq_unfreeze_queue(lo->lo_queue);
+}
+
static int
figure_loop_size(struct loop_device *lo, loff_t offset, loff_t sizelimit)
{
@@ -173,8 +216,12 @@ figure_loop_size(struct loop_device *lo, loff_t offset, loff_t sizelimit)
if (unlikely((loff_t)x != size))
return -EFBIG;
- if (lo->lo_offset != offset)
+ if (lo->lo_offset != offset) {
lo->lo_offset = offset;
+
+ /* update dio if lo_offset is changed*/
+ __loop_update_dio(lo, lo->use_dio);
+ }
if (lo->lo_sizelimit != sizelimit)
lo->lo_sizelimit = sizelimit;
set_capacity(lo->lo_disk, x);
@@ -421,6 +468,11 @@ struct switch_request {
struct completion wait;
};
+static inline void loop_update_dio(struct loop_device *lo)
+{
+ __loop_update_dio(lo, io_is_direct(lo->lo_backing_file));
+}
+
/*
* Do the actual switch; called from the BIO completion routine
*/
@@ -441,6 +493,7 @@ static void do_loop_switch(struct loop_device *lo, struct switch_request *p)
mapping->host->i_bdev->bd_block_size : PAGE_SIZE;
lo->old_gfp_mask = mapping_gfp_mask(mapping);
mapping_set_gfp_mask(mapping, lo->old_gfp_mask & ~(__GFP_IO|__GFP_FS));
+ loop_update_dio(lo);
}
/*
@@ -627,11 +680,19 @@ static ssize_t loop_attr_partscan_show(struct loop_device *lo, char *buf)
return sprintf(buf, "%s\n", partscan ? "1" : "0");
}
+static ssize_t loop_attr_dio_show(struct loop_device *lo, char *buf)
+{
+ int dio = (lo->lo_flags & LO_FLAGS_DIRECT_IO);
+
+ return sprintf(buf, "%s\n", dio ? "1" : "0");
+}
+
LOOP_ATTR_RO(backing_file);
LOOP_ATTR_RO(offset);
LOOP_ATTR_RO(sizelimit);
LOOP_ATTR_RO(autoclear);
LOOP_ATTR_RO(partscan);
+LOOP_ATTR_RO(dio);
static struct attribute *loop_attrs[] = {
&loop_attr_backing_file.attr,
@@ -639,6 +700,7 @@ static struct attribute *loop_attrs[] = {
&loop_attr_sizelimit.attr,
&loop_attr_autoclear.attr,
&loop_attr_partscan.attr,
+ &loop_attr_dio.attr,
NULL,
};
@@ -783,6 +845,7 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
if (!(lo_flags & LO_FLAGS_READ_ONLY) && file->f_op->fsync)
blk_queue_flush(lo->lo_queue, REQ_FLUSH);
+ loop_update_dio(lo);
set_capacity(lo->lo_disk, size);
bd_set_size(bdev, size << 9);
loop_sysfs_init(lo);
diff --git a/drivers/block/loop.h b/drivers/block/loop.h
index b6c7d21..63f8e14 100644
--- a/drivers/block/loop.h
+++ b/drivers/block/loop.h
@@ -58,6 +58,8 @@ struct loop_device {
struct mutex lo_ctl_mutex;
struct kthread_worker worker;
struct task_struct *worker_task;
+ unsigned dio_align;
+ bool use_dio;
struct request_queue *lo_queue;
struct blk_mq_tag_set tag_set;
diff --git a/include/uapi/linux/loop.h b/include/uapi/linux/loop.h
index e0cecd2..949851c 100644
--- a/include/uapi/linux/loop.h
+++ b/include/uapi/linux/loop.h
@@ -21,6 +21,7 @@ enum {
LO_FLAGS_READ_ONLY = 1,
LO_FLAGS_AUTOCLEAR = 4,
LO_FLAGS_PARTSCAN = 8,
+ LO_FLAGS_DIRECT_IO = 16,
};
#include <asm/posix_types.h> /* for __kernel_old_dev_t */
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] iio: Documentation: Add trigger name attribute ABI documentation
From: Daniel Baluta @ 2015-08-06 7:33 UTC (permalink / raw)
To: Cristina Opriceana
Cc: Jonathan Cameron, Daniel Baluta, linux-api-u79uwXL29TY76Z2rM5mHXA,
Linux Kernel Mailing List,
linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <998736994e24f0bc3e7864b1e6b6e0de607c8447.1438793211.git.cristina.opriceana-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+ linux-iio.
On Wed, Aug 5, 2015 at 8:09 PM, Cristina Opriceana
<cristina.opriceana-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> This patch adds an entry in the ABI Documentation for the
> name attribute issued when a trigger is created.
>
> Signed-off-by: Cristina Opriceana <cristina.opriceana-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs b/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs
> index 5235e6c..85c8e4d 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-trigger-sysfs
> @@ -9,3 +9,12 @@ Description:
> automated testing or in situations, where other trigger methods
> are not applicable. For example no RTC or spare GPIOs.
> X is the IIO index of the trigger.
> +
> +What: /sys/bus/iio/devices/triggerX/name
> +KernelVersion: 2.6.39
> +Contact: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> +Description:
> + The name attribute holds a description string for the current
> + trigger. A device should write this name string to
> + /sys/bus/iio/devices/iio:deviceX/trigger/current_trigger
> + in order to use this trigger.
Better, "In order to associate this trigger with an IIO device one
should write this name string to /sys/bus/...".
Also, please use iio:deviceY instead of iio:deviceX to make it clear
that trigger and device indexes are not
related.
thanks,
Daniel.
^ permalink raw reply
* Re: [PATCH v3 01/11] stm class: Introduce an abstraction for System Trace Module devices
From: Mathieu Poirier @ 2015-08-05 23:01 UTC (permalink / raw)
To: Alexander Shishkin
Cc: Greg Kroah-Hartman,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
peter.lachner-ral2JQCrhuEAvxtiuMwx3w,
norbert.schulz-ral2JQCrhuEAvxtiuMwx3w,
keven.boell-ral2JQCrhuEAvxtiuMwx3w,
yann.fouassier-ral2JQCrhuEAvxtiuMwx3w,
laurent.fert-ral2JQCrhuEAvxtiuMwx3w,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1436177344-16751-2-git-send-email-alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
On 6 July 2015 at 04:08, Alexander Shishkin
<alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> A System Trace Module (STM) is a device exporting data in System Trace
> Protocol (STP) format as defined by MIPI STP standards. Examples of such
> devices are Intel Trace Hub and Coresight STM.
>
> This abstraction provides a unified interface for software trace sources
> to send their data over an STM device to a debug host. In order to do
> that, such a trace source needs to be assigned a pair of master/channel
> identifiers that all the data from this source will be tagged with. The
> STP decoder on the debug host side will use these master/channel tags to
> distinguish different trace streams from one another inside one STP
> stream.
>
> This abstraction provides a configfs-based policy management mechanism
> for dynamic allocation of these master/channel pairs based on trace
> source-supplied string identifier. It has the flexibility of being
> defined at runtime and at the same time (provided that the policy
> definition is aligned with the decoding end) consistency.
>
> For userspace trace sources, this abstraction provides write()-based and
> mmap()-based (if the underlying stm device allows this) output mechanism.
>
> For kernel-side trace sources, we provide "stm_source" device class that
> can be connected to an stm device at run time.
>
> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Alexander Shishkin <alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---
> Documentation/ABI/testing/configfs-stp-policy | 48 +
> Documentation/ABI/testing/sysfs-class-stm | 14 +
> Documentation/ABI/testing/sysfs-class-stm_source | 11 +
> Documentation/ioctl/ioctl-number.txt | 3 +
> Documentation/trace/stm.txt | 80 ++
> drivers/Kconfig | 2 +
> drivers/Makefile | 1 +
> drivers/hwtracing/stm/Kconfig | 8 +
> drivers/hwtracing/stm/Makefile | 3 +
> drivers/hwtracing/stm/core.c | 1029 ++++++++++++++++++++++
> drivers/hwtracing/stm/policy.c | 529 +++++++++++
> drivers/hwtracing/stm/stm.h | 87 ++
> include/linux/stm.h | 126 +++
> include/uapi/linux/stm.h | 50 ++
> 14 files changed, 1991 insertions(+)
> create mode 100644 Documentation/ABI/testing/configfs-stp-policy
> create mode 100644 Documentation/ABI/testing/sysfs-class-stm
> create mode 100644 Documentation/ABI/testing/sysfs-class-stm_source
> create mode 100644 Documentation/trace/stm.txt
> create mode 100644 drivers/hwtracing/stm/Kconfig
> create mode 100644 drivers/hwtracing/stm/Makefile
> create mode 100644 drivers/hwtracing/stm/core.c
> create mode 100644 drivers/hwtracing/stm/policy.c
> create mode 100644 drivers/hwtracing/stm/stm.h
> create mode 100644 include/linux/stm.h
> create mode 100644 include/uapi/linux/stm.h
>
> diff --git a/Documentation/ABI/testing/configfs-stp-policy b/Documentation/ABI/testing/configfs-stp-policy
> new file mode 100644
> index 0000000000..421ce6825c
> --- /dev/null
> +++ b/Documentation/ABI/testing/configfs-stp-policy
> @@ -0,0 +1,48 @@
> +What: /config/stp-policy
> +Date: June 2015
> +KernelVersion: 4.3
> +Description:
> + This group contains policies mandating Master/Channel allocation
> + for software sources wishing to send trace data over an STM
> + device.
> +
> +What: /config/stp-policy/<device>.<policy>
> +Date: June 2015
> +KernelVersion: 4.3
> +Description:
> + This group is the root of a policy; its name is a concatenation
> + of an stm device name to which this policy applies and an
> + arbitrary string. If <device> part doesn't match an existing
> + stm device, mkdir will fail with ENODEV; if that device already
> + has a policy assigned to it, mkdir will fail with EBUSY.
> +
> +What: /config/stp-policy/<device>.<policy>/device
> +Date: June 2015
> +KernelVersion: 4.3
> +Description:
> + STM device to which this policy applies, read only. Same as the
> + <device> component of its parent directory.
> +
> +What: /config/stp-policy/<device>.<policy>/<node>
> +Date: June 2015
> +KernelVersion: 4.3
> +Description:
> + Policy node is a string identifier that software clients will
> + use to request a master/channel to be allocated and assigned to
> + them.
> +
> +What: /config/stp-policy/<device>.<policy>/<node>/masters
> +Date: June 2015
> +KernelVersion: 4.3
> +Description:
> + Range of masters from which to allocate for users of this node.
> + Write two numbers: the first master and the last master number.
> +
> +What: /config/stp-policy/<device>.<policy>/<node>/channels
> +Date: June 2015
> +KernelVersion: 4.3
> +Description:
> + Range of channels from which to allocate for users of this node.
> + Write two numbers: the first channel and the last channel
> + number.
> +
> diff --git a/Documentation/ABI/testing/sysfs-class-stm b/Documentation/ABI/testing/sysfs-class-stm
> new file mode 100644
> index 0000000000..c9aa4f3fc9
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-stm
> @@ -0,0 +1,14 @@
> +What: /sys/class/stm/<stm>/masters
> +Date: June 2015
> +KernelVersion: 4.3
> +Contact: Alexander Shishkin <alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> +Description:
> + Shows first and last available to software master numbers on
> + this STM device.
> +
> +What: /sys/class/stm/<stm>/channels
> +Date: June 2015
> +KernelVersion: 4.3
> +Contact: Alexander Shishkin <alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> +Description:
> + Shows the number of channels per master on this STM device.
> diff --git a/Documentation/ABI/testing/sysfs-class-stm_source b/Documentation/ABI/testing/sysfs-class-stm_source
> new file mode 100644
> index 0000000000..57b8dd39bb
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-stm_source
> @@ -0,0 +1,11 @@
> +What: /sys/class/stm_source/<stm_source>/stm_source_link
> +Date: June 2015
> +KernelVersion: 4.3
> +Contact: Alexander Shishkin <alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> +Description:
> + stm_source device linkage to stm device, where its tracing data
> + is directed. Reads return an existing connection or "<none>" if
> + this stm_source is not connected to any stm device yet.
> + Write an existing (registered) stm device's name here to
> + connect that device. If a device is already connected to this
> + stm_source, it will first be disconnected.
> diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
> index 611c52267d..7fa29b4ece 100644
> --- a/Documentation/ioctl/ioctl-number.txt
> +++ b/Documentation/ioctl/ioctl-number.txt
> @@ -81,6 +81,9 @@ Code Seq#(hex) Include File Comments
> 0x22 all scsi/sg.h
> '#' 00-3F IEEE 1394 Subsystem Block for the entire subsystem
> '$' 00-0F linux/perf_counter.h, linux/perf_event.h
> +'%' 00-0F include/uapi/linux/stm.h
> + System Trace Module subsystem
> + <mailto:alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> '&' 00-07 drivers/firewire/nosy-user.h
> '1' 00-1F <linux/timepps.h> PPS kit from Ulrich Windl
> <ftp://ftp.de.kernel.org/pub/linux/daemons/ntp/PPS/>
> diff --git a/Documentation/trace/stm.txt b/Documentation/trace/stm.txt
> new file mode 100644
> index 0000000000..e140f2e8d7
> --- /dev/null
> +++ b/Documentation/trace/stm.txt
> @@ -0,0 +1,80 @@
> +System Trace Module
> +===================
> +
> +System Trace Module (STM) is a device described in MIPI STP specs as
> +STP trace stream generator. STP (System Trace Protocol) is a trace
> +protocol multiplexing data from multiple trace sources, each one of
> +which is assigned a unique pair of master and channel. While some of
> +these masters and channels are statically allocated to certain
> +hardware trace sources, others are available to software. Software
> +trace sources are usually free to pick for themselves any
> +master/channel combination from this pool.
> +
> +On the receiving end of this STP stream (the decoder side), trace
> +sources can only be identified by master/channel combination, so in
> +order for the decoder to be able to make sense of the trace that
> +involves multiple trace sources, it needs to be able to map those
> +master/channel pairs to the trace sources that it understands.
> +
> +For instance, it is helpful to know that syslog messages come on
> +master 7 channel 15, while arbitrary user applications can use masters
> +48 to 63 and channels 0 to 127.
> +
> +To solve this mapping problem, stm class provides a policy management
> +mechanism via configfs, that allows defining rules that map string
> +identifiers to ranges of masters and channels. If these rules (policy)
> +are consistent with what decoder expects, it will be able to properly
> +process the trace data.
> +
> +This policy is a tree structure containing rules (policy_node) that
> +have a name (string identifier) and a range of masters and channels
> +associated with it, located in "stp-policy" subsystem directory in
> +configfs. The topmost directory's name (the policy) is formatted as
> +the STM device name to which this policy applies and and arbitrary
> +string identifier separated by a stop. From the examle above, a rule
> +may look like this:
> +
> +$ ls /config/stp-policy/dummy_stm.my-policy/user
> +channels masters
> +$ cat /config/stp-policy/dummy_stm.my-policy/user/masters
> +48 63
> +$ cat /config/stp-policy/dummy_stm.my-policy/user/channels
> +0 127
> +
> +which means that the master allocation pool for this rule consists of
> +masters 48 through 63 and channel allocation pool has channels 0
> +through 127 in it. Now, any producer (trace source) identifying itself
> +with "user" identification string will be allocated a master and
> +channel from within these ranges.
> +
> +These rules can be nested, for example, one can define a rule "dummy"
> +under "user" directory from the example above and this new rule will
> +be used for trace sources with the id string of "user/dummy".
> +
> +Trace sources have to open the stm class device's node and write their
> +trace data into its file descriptor. In order to identify themselves
> +to the policy, they need to do a STP_POLICY_ID_SET ioctl on this file
> +descriptor providing their id string. Otherwise, they will be
> +automatically allocated a master/channel pair upon first write to this
> +file descriptor according to the "default" rule of the policy, if such
> +exists.
> +
> +Some STM devices may allow direct mapping of the channel mmio regions
> +to userspace for zero-copy writing. One mappable page (in terms of
> +mmu) will usually contain multiple channels' mmios, so the user will
> +need to allocate that many channels to themselves (via the
> +aforementioned ioctl() call) to be able to do this. That is, if your
> +stm device's channel mmio region is 64 bytes and hardware page size is
> +4096 bytes, after a successful STP_POLICY_ID_SET ioctl() call with
> +width==64, you should be able to mmap() one page on this file
> +descriptor and obtain direct access to an mmio region for 64 channels.
> +
> +For kernel-based trace sources, there is "stm_source" device
> +class. Devices of this class can be connected and disconnected to/from
> +stm devices at runtime via a sysfs attribute.
> +
> +Examples of STM devices are Intel Trace Hub [1] and Coresight STM
> +[2].
> +
> +[1] https://software.intel.com/sites/default/files/managed/d3/3c/intel-th-developer-manual.pdf
> +[2] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0444b/index.html
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 6e973b8e3a..96efe4522c 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -184,4 +184,6 @@ source "drivers/android/Kconfig"
>
> source "drivers/nvdimm/Kconfig"
>
> +source "drivers/hwtracing/stm/Kconfig"
> +
> endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index b64b49f6e0..6647bc923f 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -164,4 +164,5 @@ obj-$(CONFIG_MCB) += mcb/
> obj-$(CONFIG_RAS) += ras/
> obj-$(CONFIG_THUNDERBOLT) += thunderbolt/
> obj-$(CONFIG_CORESIGHT) += hwtracing/coresight/
> +obj-$(CONFIG_STM) += hwtracing/stm/
> obj-$(CONFIG_ANDROID) += android/
> diff --git a/drivers/hwtracing/stm/Kconfig b/drivers/hwtracing/stm/Kconfig
> new file mode 100644
> index 0000000000..90ed327461
> --- /dev/null
> +++ b/drivers/hwtracing/stm/Kconfig
> @@ -0,0 +1,8 @@
> +config STM
> + tristate "System Trace Module devices"
> + help
> + A System Trace Module (STM) is a device exporting data in System
> + Trace Protocol (STP) format as defined by MIPI STP standards.
> + Examples of such devices are Intel Trace Hub and Coresight STM.
> +
> + Say Y here to enable System Trace Module device support.
> diff --git a/drivers/hwtracing/stm/Makefile b/drivers/hwtracing/stm/Makefile
> new file mode 100644
> index 0000000000..adec701649
> --- /dev/null
> +++ b/drivers/hwtracing/stm/Makefile
> @@ -0,0 +1,3 @@
> +obj-$(CONFIG_STM) += stm_core.o
> +
> +stm_core-y := core.o policy.o
> diff --git a/drivers/hwtracing/stm/core.c b/drivers/hwtracing/stm/core.c
> new file mode 100644
> index 0000000000..b79c42c625
> --- /dev/null
> +++ b/drivers/hwtracing/stm/core.c
> @@ -0,0 +1,1029 @@
> +/*
> + * System Trace Module (STM) infrastructure
> + * Copyright (c) 2014, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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.
> + *
> + * STM class implements generic infrastructure for System Trace Module devices
> + * as defined in MIPI STPv2 specification.
> + */
> +
> +#include <linux/uaccess.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/compat.h>
> +#include <linux/kdev_t.h>
> +#include <linux/srcu.h>
> +#include <linux/slab.h>
> +#include <linux/stm.h>
> +#include <linux/fs.h>
> +#include <linux/mm.h>
> +#include "stm.h"
> +
> +#include <uapi/linux/stm.h>
> +
> +static unsigned int stm_core_up;
> +
> +/*
> + * The SRCU here makes sure that STM device doesn't disappear from under a
> + * stm_source_write() caller, which may want to have as little overhead as
> + * possible.
> + */
> +static struct srcu_struct stm_source_srcu;
> +
> +static ssize_t masters_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct stm_device *stm = to_stm_device(dev);
> + int ret;
> +
> + ret = sprintf(buf, "%u %u\n", stm->data->sw_start, stm->data->sw_end);
> +
> + return ret;
> +}
> +
> +static DEVICE_ATTR_RO(masters);
> +
> +static ssize_t channels_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct stm_device *stm = to_stm_device(dev);
> + int ret;
> +
> + ret = sprintf(buf, "%u\n", stm->data->sw_nchannels);
> +
> + return ret;
> +}
> +
> +static DEVICE_ATTR_RO(channels);
> +
> +static struct attribute *stm_attrs[] = {
> + &dev_attr_masters.attr,
> + &dev_attr_channels.attr,
> + NULL,
> +};
> +
> +ATTRIBUTE_GROUPS(stm);
> +
> +static struct class stm_class = {
> + .name = "stm",
> + .dev_groups = stm_groups,
> +};
> +
> +static int stm_dev_match(struct device *dev, const void *data)
> +{
> + const char *name = data;
> +
> + return sysfs_streq(name, dev_name(dev));
> +}
> +
> +/**
> + * stm_find_device() - find stm device by name
> + * @buf: character buffer containing the name
> + *
> + * This is called when either policy gets assigned to an stm device or an
> + * stm_source device gets linked to an stm device.
> + *
> + * This grabs device's reference (get_device()) and module reference, both
> + * of which the calling path needs to make sure to drop with stm_put_device().
> + *
> + * Return: stm device pointer or null if lookup failed.
> + */
> +struct stm_device *stm_find_device(const char *buf)
> +{
> + struct stm_device *stm;
> + struct device *dev;
> +
> + if (!stm_core_up)
> + return NULL;
> +
> + dev = class_find_device(&stm_class, NULL, buf, stm_dev_match);
> + if (!dev)
> + return NULL;
> +
> + stm = to_stm_device(dev);
> + if (!try_module_get(stm->owner)) {
> + put_device(dev);
> + return NULL;
> + }
> +
> + return stm;
> +}
> +
> +/**
> + * stm_put_device() - drop references on the stm device
> + * @stm: stm device, previously acquired by stm_find_device()
> + *
> + * This drops the module reference and device reference taken by
> + * stm_find_device().
> + */
> +void stm_put_device(struct stm_device *stm)
> +{
> + module_put(stm->owner);
> + put_device(&stm->dev);
> +}
> +
> +/*
> + * Internally we only care about software-writable masters here, that is the
> + * ones in the range [stm_data->sw_start..stm_data..sw_end], however we need
> + * original master numbers to be visible externally, since they are the ones
> + * that will appear in the STP stream. Thus, the internal bookkeeping uses
> + * $master - stm_data->sw_start to reference master descriptors and such.
> + */
> +
> +#define __stm_master(_s, _m) \
> + ((_s)->masters[(_m) - (_s)->data->sw_start])
> +
> +static inline struct stp_master *
> +stm_master(struct stm_device *stm, unsigned int idx)
> +{
> + if (idx < stm->data->sw_start || idx > stm->data->sw_end)
> + return NULL;
> +
> + return __stm_master(stm, idx);
> +}
> +
> +static int stp_master_alloc(struct stm_device *stm, unsigned int idx)
> +{
> + struct stp_master *master;
> + size_t size;
> +
> + size = ALIGN(stm->data->sw_nchannels, 8) / 8;
> + size += sizeof(struct stp_master);
> + master = kzalloc(size, GFP_ATOMIC);
> + if (!master)
> + return -ENOMEM;
> +
> + master->nr_free = stm->data->sw_nchannels;
> + __stm_master(stm, idx) = master;
> +
> + return 0;
> +}
> +
> +static void stp_master_free(struct stm_device *stm, unsigned int idx)
> +{
> + struct stp_master *master = stm_master(stm, idx);
> +
> + if (!master)
> + return;
> +
> + __stm_master(stm, idx) = NULL;
> + kfree(master);
> +}
> +
> +static void stm_output_claim(struct stm_device *stm, struct stm_output *output)
> +{
> + struct stp_master *master = stm_master(stm, output->master);
> +
> + if (WARN_ON_ONCE(master->nr_free < output->nr_chans))
> + return;
> +
> + bitmap_allocate_region(&master->chan_map[0], output->channel,
> + ilog2(output->nr_chans));
> +
> + master->nr_free -= output->nr_chans;
> +}
> +
> +static void
> +stm_output_disclaim(struct stm_device *stm, struct stm_output *output)
> +{
> + struct stp_master *master = stm_master(stm, output->master);
> +
> + bitmap_release_region(&master->chan_map[0], output->channel,
> + ilog2(output->nr_chans));
> +
> + output->nr_chans = 0;
> + master->nr_free += output->nr_chans;
> +}
> +
> +/*
> + * This is like bitmap_find_free_region(), except it can ignore @start bits
> + * at the beginning.
> + */
> +static int find_free_channels(unsigned long *bitmap, unsigned int start,
> + unsigned int end, unsigned int width)
> +{
> + unsigned int pos;
> + int i;
> +
> + for (pos = start; pos < end + 1; pos = ALIGN(pos, width)) {
> + pos = find_next_zero_bit(bitmap, end + 1, pos);
> + if (pos + width > end + 1)
> + break;
> +
> + if (pos & (width - 1))
> + continue;
> +
> + for (i = 1; i < width && !test_bit(pos + i, bitmap); i++)
> + ;
> + if (i == width)
> + return pos;
> + }
> +
> + return -1;
> +}
> +
> +static unsigned int
> +stm_find_master_chan(struct stm_device *stm, unsigned int width,
> + unsigned int *mstart, unsigned int mend,
> + unsigned int *cstart, unsigned int cend)
> +{
> + struct stp_master *master;
> + unsigned int midx;
> + int pos, err;
> +
> + for (midx = *mstart; midx <= mend; midx++) {
> + if (!stm_master(stm, midx)) {
> + err = stp_master_alloc(stm, midx);
> + if (err)
> + return err;
> + }
> +
> + master = stm_master(stm, midx);
> +
> + if (!master->nr_free)
> + continue;
> +
> + pos = find_free_channels(master->chan_map, *cstart, cend,
> + width);
> + if (pos < 0)
> + continue;
> +
> + *mstart = midx;
> + *cstart = pos;
> + return 0;
> + }
> +
> + return -ENOSPC;
> +}
> +
> +static int stm_output_assign(struct stm_device *stm, unsigned int width,
> + struct stp_policy_node *policy_node,
> + struct stm_output *output)
> +{
> + unsigned int midx, cidx, mend, cend;
> + int ret = -EINVAL;
> +
> + if (width > stm->data->sw_nchannels)
> + return -EINVAL;
> +
> + if (policy_node) {
> + stp_policy_node_get_ranges(policy_node,
> + &midx, &mend, &cidx, &cend);
> + } else {
> + midx = stm->data->sw_start;
> + cidx = 0;
> + mend = stm->data->sw_end;
> + cend = stm->data->sw_nchannels - 1;
> + }
> +
> + spin_lock(&stm->mc_lock);
> + /* output is already assigned -- shouldn't happen */
> + if (WARN_ON_ONCE(output->nr_chans))
> + goto unlock;
> +
> + ret = stm_find_master_chan(stm, width, &midx, mend, &cidx, cend);
> + if (ret)
> + goto unlock;
> +
> + output->master = midx;
> + output->channel = cidx;
> + output->nr_chans = width;
> + stm_output_claim(stm, output);
> + dev_dbg(&stm->dev, "assigned %u:%u (+%u)\n", midx, cidx, width);
> +
> + ret = 0;
> +unlock:
> + spin_unlock(&stm->mc_lock);
> +
> + return ret;
> +}
> +
> +static void stm_output_free(struct stm_device *stm, struct stm_output *output)
> +{
> + spin_lock(&stm->mc_lock);
> + if (output->nr_chans)
> + stm_output_disclaim(stm, output);
> + spin_unlock(&stm->mc_lock);
> +}
> +
> +static int major_match(struct device *dev, const void *data)
> +{
> + unsigned int major = *(unsigned int *)data;
> +
> + return MAJOR(dev->devt) == major;
> +}
> +
> +static int stm_char_open(struct inode *inode, struct file *file)
> +{
> + struct stm_file *stmf;
> + struct device *dev;
> + unsigned int major = imajor(inode);
> + int err = -ENODEV;
> +
> + dev = class_find_device(&stm_class, NULL, &major, major_match);
> + if (!dev)
> + return -ENODEV;
> +
> + stmf = kzalloc(sizeof(*stmf), GFP_KERNEL);
> + if (!stmf)
> + return -ENOMEM;
> +
> + stmf->stm = to_stm_device(dev);
> +
> + if (!try_module_get(stmf->stm->owner))
> + goto err_free;
> +
> + file->private_data = stmf;
> +
> + return nonseekable_open(inode, file);
> +
> +err_free:
> + kfree(stmf);
> +
> + return err;
> +}
> +
> +static int stm_char_release(struct inode *inode, struct file *file)
> +{
> + struct stm_file *stmf = file->private_data;
> +
> + stm_output_free(stmf->stm, &stmf->output);
> + stm_put_device(stmf->stm);
> + kfree(stmf);
> +
> + return 0;
> +}
> +
> +static int stm_file_assign(struct stm_file *stmf, char *id, unsigned int width)
> +{
> + struct stm_device *stm = stmf->stm;
> + int ret;
> +
> + stmf->policy_node = stp_policy_node_lookup(stm, id);
> +
> + ret = stm_output_assign(stm, width, stmf->policy_node, &stmf->output);
> +
> + if (stmf->policy_node)
> + stp_policy_node_put(stmf->policy_node);
> +
> + return ret;
> +}
> +
> +static void stm_write(struct stm_data *data, unsigned int master,
> + unsigned int channel, const char *buf, size_t count)
> +{
> + unsigned int flags = STP_PACKET_TIMESTAMPED;
> + const unsigned char *p = buf, nil = 0;
> + size_t pos;
> + ssize_t sz;
> +
> + for (pos = 0, p = buf; count > pos; pos += sz, p += sz) {
> + sz = min_t(unsigned int, count - pos, 8);
> + sz = data->packet(data, master, channel, STP_PACKET_DATA, flags,
> + sz, p);
> + flags = 0;
> + }
> +
> + data->packet(data, master, channel, STP_PACKET_FLAG, 0, 0, &nil);
> +}
> +
> +static ssize_t stm_char_write(struct file *file, const char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + struct stm_file *stmf = file->private_data;
> + struct stm_device *stm = stmf->stm;
> + char *kbuf;
> + int err;
> +
> + /*
> + * if no m/c have been assigned to this writer up to this
> + * point, use "default" policy entry
> + */
> + if (!stmf->output.nr_chans) {
> + err = stm_file_assign(stmf, "default", 1);
> + /*
> + * EBUSY means that somebody else just assigned this
> + * output, which is just fine for write()
> + */
> + if (err && err != -EBUSY)
> + return err;
> + }
> +
> + kbuf = kmalloc(count + 1, GFP_KERNEL);
> + if (!kbuf)
> + return -ENOMEM;
> +
> + err = copy_from_user(kbuf, buf, count);
> + if (err) {
> + kfree(kbuf);
> + return -EFAULT;
> + }
> +
> + stm_write(stm->data, stmf->output.master, stmf->output.channel, kbuf,
> + count);
> +
> + kfree(kbuf);
> +
> + return count;
> +}
> +
> +static int stm_char_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> + struct stm_file *stmf = file->private_data;
> + struct stm_device *stm = stmf->stm;
> + unsigned long size, phys;
> +
> + if (!stm->data->mmio_addr)
> + return -EOPNOTSUPP;
> +
> + if (vma->vm_pgoff)
> + return -EINVAL;
> +
> + size = vma->vm_end - vma->vm_start;
> +
> + if (stmf->output.nr_chans * stm->data->sw_mmiosz != size)
> + return -EINVAL;
> +
> + phys = stm->data->mmio_addr(stm->data, stmf->output.master,
> + stmf->output.channel,
> + stmf->output.nr_chans);
> +
> + if (!phys)
> + return -EINVAL;
> +
> + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> + vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
> + vm_iomap_memory(vma, phys, size);
> +
> + return 0;
> +}
> +
> +static int stm_char_policy_set_ioctl(struct stm_file *stmf, void __user *arg)
> +{
> + struct stm_device *stm = stmf->stm;
> + struct stp_policy_id *id;
> + int ret = -EINVAL;
> + u32 size;
> +
> + if (stmf->output.nr_chans)
> + return -EBUSY;
> +
> + if (copy_from_user(&size, arg, sizeof(size)))
> + return -EFAULT;
> +
> + if (size >= PATH_MAX + sizeof(*id))
> + return -EINVAL;
> +
> + /*
> + * size + 1 to make sure the .id string at the bottom is terminated,
> + * which is also why memdup_user() is not useful here
> + */
> + id = kzalloc(size + 1, GFP_KERNEL);
> + if (!id)
> + return -ENOMEM;
> +
> + if (copy_from_user(id, arg, size)) {
> + ret = -EFAULT;
> + goto err_free;
> + }
> +
> + if (id->__reserved_0 || id->__reserved_1)
> + goto err_free;
> +
> + if (id->width < 1 ||
> + id->width > PAGE_SIZE / stm->data->sw_mmiosz)
> + goto err_free;
> +
> + ret = stm_file_assign(stmf, id->id, id->width);
> + if (ret)
> + goto err_free;
> +
> + ret = 0;
> +
> + if (stm->data->link)
> + ret = stm->data->link(stm->data, stmf->output.master,
> + stmf->output.channel);
> +
> + if (ret) {
> + stm_output_free(stmf->stm, &stmf->output);
> + stm_put_device(stmf->stm);
> + }
> +
> +err_free:
> + kfree(id);
> +
> + return ret;
> +}
> +
> +static int stm_char_policy_get_ioctl(struct stm_file *stmf, void __user *arg)
> +{
> + struct stp_policy_id id = {
> + .size = sizeof(id),
> + .master = stmf->output.master,
> + .channel = stmf->output.channel,
> + .width = stmf->output.nr_chans,
> + .__reserved_0 = 0,
> + .__reserved_1 = 0,
> + };
> +
> + return copy_to_user(arg, &id, id.size) ? -EFAULT : 0;
> +}
> +
> +static long
> +stm_char_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> + struct stm_file *stmf = file->private_data;
> + struct stm_data *stm_data = stmf->stm->data;
> + int err = -ENOTTY;
> + u64 options;
> +
> + switch (cmd) {
> + case STP_POLICY_ID_SET:
> + err = stm_char_policy_set_ioctl(stmf, (void __user *)arg);
> + if (err)
> + return err;
> +
> + return stm_char_policy_get_ioctl(stmf, (void __user *)arg);
> +
> + case STP_POLICY_ID_GET:
> + return stm_char_policy_get_ioctl(stmf, (void __user *)arg);
> +
> + case STP_SET_OPTIONS:
> + if (copy_from_user(&options, (u64 __user *)arg, sizeof(u64)))
> + return -EFAULT;
> +
> + if (stm_data->set_options)
> + err = stm_data->set_options(stm_data,
> + stmf->output.master,
> + stmf->output.channel,
> + stmf->output.nr_chans,
> + options);
> +
> + break;
> + default:
> + break;
> + }
> +
> + return err;
> +}
> +
> +#ifdef CONFIG_COMPAT
> +static long
> +stm_char_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> + return stm_char_ioctl(file, cmd, (unsigned long)compat_ptr(arg));
> +}
> +#else
> +#define stm_char_compat_ioctl NULL
> +#endif
> +
> +static const struct file_operations stm_fops = {
> + .open = stm_char_open,
> + .release = stm_char_release,
> + .write = stm_char_write,
> + .mmap = stm_char_mmap,
> + .unlocked_ioctl = stm_char_ioctl,
> + .compat_ioctl = stm_char_compat_ioctl,
> + .llseek = no_llseek,
> +};
> +
> +static void stm_device_release(struct device *dev)
> +{
> + struct stm_device *stm = to_stm_device(dev);
> +
> + kfree(stm);
> +}
> +
> +int stm_register_device(struct device *parent, struct stm_data *stm_data,
> + struct module *owner)
> +{
> + struct stm_device *stm;
> + unsigned int nmasters;
> + int err = -ENOMEM;
> +
> + if (!stm_core_up)
> + return -EPROBE_DEFER;
> +
> + if (!stm_data->packet || !stm_data->sw_nchannels)
> + return -EINVAL;
> +
> + nmasters = stm_data->sw_end - stm_data->sw_start;
> + stm = kzalloc(sizeof(*stm) + nmasters * sizeof(void *), GFP_KERNEL);
> + if (!stm)
> + return -ENOMEM;
> +
> + stm->major = register_chrdev(0, stm_data->name, &stm_fops);
> + if (stm->major < 0)
> + goto err_free;
> +
> + device_initialize(&stm->dev);
> + stm->dev.devt = MKDEV(stm->major, 0);
> + stm->dev.class = &stm_class;
> + stm->dev.parent = parent;
> + stm->dev.release = stm_device_release;
> +
> + err = kobject_set_name(&stm->dev.kobj, "%s", stm_data->name);
> + if (err)
> + goto err_device;
> +
> + err = device_add(&stm->dev);
> + if (err)
> + goto err_device;
> +
> + spin_lock_init(&stm->link_lock);
> + INIT_LIST_HEAD(&stm->link_list);
> +
> + spin_lock_init(&stm->mc_lock);
> + mutex_init(&stm->policy_mutex);
> + stm->sw_nmasters = nmasters;
> + stm->owner = owner;
> + stm->data = stm_data;
> + stm_data->stm = stm;
> +
> + return 0;
> +
> +err_device:
> + put_device(&stm->dev);
> +err_free:
> + kfree(stm);
> +
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(stm_register_device);
> +
> +static void __stm_source_link_drop(struct stm_source_device *src,
> + struct stm_device *stm);
> +
> +void stm_unregister_device(struct stm_data *stm_data)
> +{
> + struct stm_device *stm = stm_data->stm;
> + struct stm_source_device *src, *iter;
> + int i;
> +
> + spin_lock(&stm->link_lock);
> + list_for_each_entry_safe(src, iter, &stm->link_list, link_entry) {
> + __stm_source_link_drop(src, stm);
> + }
> + spin_unlock(&stm->link_lock);
> +
> + synchronize_srcu(&stm_source_srcu);
> +
> + unregister_chrdev(stm->major, stm_data->name);
> +
> + mutex_lock(&stm->policy_mutex);
> + if (stm->policy)
> + stp_policy_unbind(stm->policy);
> + mutex_unlock(&stm->policy_mutex);
> +
> + for (i = 0; i < stm->sw_nmasters; i++)
> + stp_master_free(stm, i);
> +
> + device_unregister(&stm->dev);
> + stm_data->stm = NULL;
> +}
> +EXPORT_SYMBOL_GPL(stm_unregister_device);
> +
> +/**
> + * stm_source_link_add() - connect an stm_source device to an stm device
> + * @src: stm_source device
> + * @stm: stm device
> + *
> + * This function establishes a link from stm_source to an stm device so that
> + * the former can send out trace data to the latter.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int stm_source_link_add(struct stm_source_device *src,
> + struct stm_device *stm)
> +{
> + char *id;
> + int err;
> +
> + spin_lock(&stm->link_lock);
> + spin_lock(&src->link_lock);
> +
> + /* src->link is dereferenced under stm_source_srcu but not the list */
> + rcu_assign_pointer(src->link, stm);
> + list_add_tail(&src->link_entry, &stm->link_list);
> +
> + spin_unlock(&src->link_lock);
> + spin_unlock(&stm->link_lock);
> +
> + id = kstrdup(src->data->name, GFP_KERNEL);
> + if (id) {
> + src->policy_node =
> + stp_policy_node_lookup(stm, id);
> +
> + kfree(id);
> + }
> +
> + err = stm_output_assign(stm, src->data->nr_chans,
> + src->policy_node, &src->output);
> +
> + if (src->policy_node)
> + stp_policy_node_put(src->policy_node);
> +
> + if (err)
> + goto fail_detach;
> +
> + /* this is to notify the STM device that a new link has been made */
> + if (stm->data->link)
> + err = stm->data->link(stm->data, src->output.master,
> + src->output.channel);
> +
> + if (err)
> + goto fail_free_output;
> +
> + /* this is to let the source carry out all necessary preparations */
> + if (src->data->link)
> + src->data->link(src->data);
> +
> + return 0;
> +
> +fail_free_output:
> + stm_output_free(stm, &src->output);
> + stm_put_device(stm);
> +
> +fail_detach:
> + spin_lock(&stm->link_lock);
> + spin_lock(&src->link_lock);
> +
> + rcu_assign_pointer(src->link, NULL);
> + list_del_init(&src->link_entry);
> +
> + spin_unlock(&src->link_lock);
> + spin_unlock(&stm->link_lock);
> +
> + return err;
> +}
> +
> +/**
> + * __stm_source_link_drop() - detach stm_source from an stm device
> + * @src: stm_source device
> + * @stm: stm device
> + *
> + * If @stm is @src::link, disconnect them from one another and put the
> + * reference on the @stm device.
> + *
> + * Caller must hold stm::link_lock.
> + */
> +static void __stm_source_link_drop(struct stm_source_device *src,
> + struct stm_device *stm)
> +{
> + spin_lock(&src->link_lock);
> + if (WARN_ON_ONCE(src->link != stm)) {
> + spin_unlock(&src->link_lock);
> + return;
> + }
> +
> + stm_output_free(src->link, &src->output);
> + /* caller must hold stm::link_lock */
> + list_del_init(&src->link_entry);
> + /* matches stm_find_device() from stm_source_link_store() */
> + stm_put_device(src->link);
> + rcu_assign_pointer(src->link, NULL);
> +
> + spin_unlock(&src->link_lock);
> +}
> +
> +/**
> + * stm_source_link_drop() - detach stm_source from its stm device
> + * @src: stm_source device
> + *
> + * Unlinking means disconnecting from source's STM device; after this
> + * writes will be unsuccessful until it is linked to a new STM device.
> + *
> + * This will happen on "stm_source_link" sysfs attribute write to undo
> + * the existing link (if any), or on linked STM device's de-registration.
> + */
> +static void stm_source_link_drop(struct stm_source_device *src)
> +{
> + struct stm_device *stm;
> + int idx;
> +
> + idx = srcu_read_lock(&stm_source_srcu);
> + stm = srcu_dereference(src->link, &stm_source_srcu);
> +
> + if (stm) {
> + if (src->data->unlink)
> + src->data->unlink(src->data);
> +
> + spin_lock(&stm->link_lock);
> + __stm_source_link_drop(src, stm);
> + spin_unlock(&stm->link_lock);
> + }
> +
> + srcu_read_unlock(&stm_source_srcu, idx);
> +}
> +
> +static ssize_t stm_source_link_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct stm_source_device *src = to_stm_source_device(dev);
> + struct stm_device *stm;
> + int idx, ret;
> +
> + idx = srcu_read_lock(&stm_source_srcu);
> + stm = srcu_dereference(src->link, &stm_source_srcu);
> + ret = sprintf(buf, "%s\n",
> + stm ? dev_name(&stm->dev) : "<none>");
> + srcu_read_unlock(&stm_source_srcu, idx);
> +
> + return ret;
> +}
> +
> +static ssize_t stm_source_link_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct stm_source_device *src = to_stm_source_device(dev);
> + struct stm_device *link;
> + int err;
> +
> + stm_source_link_drop(src);
> +
> + link = stm_find_device(buf);
> + if (!link)
> + return -EINVAL;
> +
> + err = stm_source_link_add(src, link);
> + if (err)
> + stm_put_device(link);
> +
> + return err ? : count;
> +}
> +
> +static DEVICE_ATTR_RW(stm_source_link);
> +
> +static struct attribute *stm_source_attrs[] = {
> + &dev_attr_stm_source_link.attr,
> + NULL,
> +};
> +
> +ATTRIBUTE_GROUPS(stm_source);
> +
> +static struct class stm_source_class = {
> + .name = "stm_source",
> + .dev_groups = stm_source_groups,
> +};
> +
> +static void stm_source_device_release(struct device *dev)
> +{
> + struct stm_source_device *src = to_stm_source_device(dev);
> +
> + kfree(src);
> +}
> +
> +/**
> + * stm_source_register_device() - register an stm_source device
> + * @parent: parent device
> + * @data: device description structure
> + *
> + * This will create a device of stm_source class that can write
> + * data to an stm device once linked.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +int stm_source_register_device(struct device *parent,
> + struct stm_source_data *data)
> +{
> + struct stm_source_device *src;
> + int err;
> +
> + if (!stm_core_up)
> + return -EPROBE_DEFER;
> +
> + src = kzalloc(sizeof(*src), GFP_KERNEL);
> + if (!src)
> + return -ENOMEM;
> +
> + device_initialize(&src->dev);
> + src->dev.class = &stm_source_class;
> + src->dev.parent = parent;
> + src->dev.release = stm_source_device_release;
> +
> + err = kobject_set_name(&src->dev.kobj, "%s", data->name);
> + if (err)
> + goto err;
> +
> + err = device_add(&src->dev);
> + if (err)
> + goto err;
> +
> + spin_lock_init(&src->link_lock);
> + INIT_LIST_HEAD(&src->link_entry);
> + src->data = data;
> + data->src = src;
> +
> + return 0;
> +
> +err:
> + put_device(&src->dev);
> + kfree(src);
> +
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(stm_source_register_device);
> +
> +/**
> + * stm_source_unregister_device() - unregister an stm_source device
> + * @data: device description that was used to register the device
> + *
> + * This will remove a previously created stm_source device from the system.
> + */
> +void stm_source_unregister_device(struct stm_source_data *data)
> +{
> + struct stm_source_device *src = data->src;
> +
> + stm_source_link_drop(src);
> +
> + device_destroy(&stm_source_class, src->dev.devt);
> +}
> +EXPORT_SYMBOL_GPL(stm_source_unregister_device);
> +
> +int stm_source_write(struct stm_source_data *data, unsigned int chan,
> + const char *buf, size_t count)
> +{
> + struct stm_source_device *src = data->src;
> + struct stm_device *stm;
> + int idx;
> +
> + if (!src->output.nr_chans)
> + return -ENODEV;
> +
> + if (chan >= src->output.nr_chans)
> + return -EINVAL;
> +
> + idx = srcu_read_lock(&stm_source_srcu);
> +
> + stm = srcu_dereference(src->link, &stm_source_srcu);
> + if (stm)
> + stm_write(stm->data, src->output.master,
> + src->output.channel + chan,
> + buf, count);
> + else
> + count = -ENODEV;
> +
> + srcu_read_unlock(&stm_source_srcu, idx);
> +
> + return count;
> +}
> +EXPORT_SYMBOL_GPL(stm_source_write);
> +
> +static int __init stm_core_init(void)
> +{
> + int err;
> +
> + err = class_register(&stm_class);
> + if (err)
> + return err;
> +
> + err = class_register(&stm_source_class);
> + if (err)
> + goto err_stm;
> +
> + err = stp_configfs_init();
> + if (err)
> + goto err_src;
> +
> + init_srcu_struct(&stm_source_srcu);
> +
> + stm_core_up++;
> +
> + return 0;
> +
> +err_src:
> + class_unregister(&stm_source_class);
> +err_stm:
> + class_unregister(&stm_class);
> +
> + return err;
> +}
> +
> +module_init(stm_core_init);
> +
> +static void __exit stm_core_exit(void)
> +{
> + cleanup_srcu_struct(&stm_source_srcu);
> + class_unregister(&stm_source_class);
> + class_unregister(&stm_class);
> + stp_configfs_exit();
> +}
> +
> +module_exit(stm_core_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("System Trace Module device class");
> +MODULE_AUTHOR("Alexander Shishkin <alexander.shishkin-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>");
> diff --git a/drivers/hwtracing/stm/policy.c b/drivers/hwtracing/stm/policy.c
> new file mode 100644
> index 0000000000..6498a9dbb7
> --- /dev/null
> +++ b/drivers/hwtracing/stm/policy.c
> @@ -0,0 +1,529 @@
> +/*
> + * System Trace Module (STM) master/channel allocation policy management
> + * Copyright (c) 2014, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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.
> + *
> + * A master/channel allocation policy allows mapping string identifiers to
> + * master and channel ranges, where allocation can be done.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/types.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/configfs.h>
> +#include <linux/slab.h>
> +#include <linux/stm.h>
> +#include "stm.h"
> +
> +/*
> + * STP Master/Channel allocation policy configfs layout.
> + */
> +
> +struct stp_policy {
> + struct config_group group;
> + struct stm_device *stm;
> +};
> +
> +struct stp_policy_node {
> + struct config_group group;
> + struct stp_policy *policy;
> + unsigned int first_master;
> + unsigned int last_master;
> + unsigned int first_channel;
> + unsigned int last_channel;
> +};
> +
> +static struct configfs_subsystem stp_policy_subsys;
> +
> +void stp_policy_node_get_ranges(struct stp_policy_node *policy_node,
> + unsigned int *mstart, unsigned int *mend,
> + unsigned int *cstart, unsigned int *cend)
> +{
> + *mstart = policy_node->first_master;
> + *mend = policy_node->last_master;
> + *cstart = policy_node->first_channel;
> + *cend = policy_node->last_channel;
> +}
> +
> +static inline char *stp_policy_node_name(struct stp_policy_node *policy_node)
> +{
> + return policy_node->group.cg_item.ci_name ? : "<none>";
> +}
> +
> +static inline struct stp_policy *to_stp_policy(struct config_item *item)
> +{
> + return item ?
> + container_of(to_config_group(item), struct stp_policy, group) :
> + NULL;
> +}
> +
> +static inline struct stp_policy_node *
> +to_stp_policy_node(struct config_item *item)
> +{
> + return item ?
> + container_of(to_config_group(item), struct stp_policy_node,
> + group) :
> + NULL;
> +}
> +
> +static ssize_t stp_policy_node_masters_show(struct stp_policy_node *policy_node,
> + char *page)
> +{
> + ssize_t count;
> +
> + count = sprintf(page, "%u %u\n", policy_node->first_master,
> + policy_node->last_master);
> +
> + return count;
> +}
> +
> +static ssize_t
> +stp_policy_node_masters_store(struct stp_policy_node *policy_node,
> + const char *page, size_t count)
> +{
> + unsigned int first, last;
> + struct stm_device *stm;
> + char *p = (char *)page;
> + ssize_t ret = -ENODEV;
> +
> + if (sscanf(p, "%u %u", &first, &last) != 2)
> + return -EINVAL;
> +
> + mutex_lock(&stp_policy_subsys.su_mutex);
> + stm = policy_node->policy->stm;
> + if (!stm)
> + goto unlock;
> +
> + /* must be within [sw_start..sw_end], which is an inclusive range */
> + if (first > INT_MAX || last > INT_MAX || first > last ||
> + first < stm->data->sw_start ||
> + last > stm->data->sw_end) {
> + ret = -ERANGE;
> + goto unlock;
> + }
> +
> + ret = count;
> + policy_node->first_master = first;
> + policy_node->last_master = last;
> +
> +unlock:
> + mutex_unlock(&stp_policy_subsys.su_mutex);
> +
> + return ret;
> +}
> +
> +static ssize_t
> +stp_policy_node_channels_show(struct stp_policy_node *policy_node, char *page)
> +{
> + ssize_t count;
> +
> + count = sprintf(page, "%u %u\n", policy_node->first_channel,
> + policy_node->last_channel);
> +
> + return count;
> +}
> +
> +static ssize_t
> +stp_policy_node_channels_store(struct stp_policy_node *policy_node,
> + const char *page, size_t count)
> +{
> + unsigned int first, last;
> + struct stm_device *stm;
> + char *p = (char *)page;
> + ssize_t ret = -ENODEV;
> +
> + if (sscanf(p, "%u %u", &first, &last) != 2)
> + return -EINVAL;
> +
> + mutex_lock(&stp_policy_subsys.su_mutex);
> + stm = policy_node->policy->stm;
> + if (!stm)
> + goto unlock;
> +
> + if (first > INT_MAX || last > INT_MAX || first > last ||
> + last >= stm->data->sw_nchannels) {
> + ret = -ERANGE;
> + goto unlock;
> + }
> +
> + ret = count;
> + policy_node->first_channel = first;
> + policy_node->last_channel = last;
> +
> +unlock:
> + mutex_unlock(&stp_policy_subsys.su_mutex);
> +
> + return ret;
> +}
> +
> +static void stp_policy_node_release(struct config_item *item)
> +{
> + kfree(to_stp_policy_node(item));
> +}
> +
> +struct stp_policy_node_attribute {
> + struct configfs_attribute attr;
> + ssize_t (*show)(struct stp_policy_node *, char *);
> + ssize_t (*store)(struct stp_policy_node *, const char *, size_t);
> +};
> +
> +static ssize_t stp_policy_node_attr_show(struct config_item *item,
> + struct configfs_attribute *attr,
> + char *page)
> +{
> + struct stp_policy_node *policy_node = to_stp_policy_node(item);
> + struct stp_policy_node_attribute *pn_attr =
> + container_of(attr, struct stp_policy_node_attribute, attr);
> + ssize_t count = 0;
> +
> + if (pn_attr->show)
> + count = pn_attr->show(policy_node, page);
> +
> + return count;
> +}
> +
> +static ssize_t stp_policy_node_attr_store(struct config_item *item,
> + struct configfs_attribute *attr,
> + const char *page, size_t len)
> +{
> + struct stp_policy_node *policy_node = to_stp_policy_node(item);
> + struct stp_policy_node_attribute *pn_attr =
> + container_of(attr, struct stp_policy_node_attribute, attr);
> + ssize_t count = -EINVAL;
> +
> + if (pn_attr->store)
> + count = pn_attr->store(policy_node, page, len);
> +
> + return count;
> +}
> +
> +static struct configfs_item_operations stp_policy_node_item_ops = {
> + .release = stp_policy_node_release,
> + .show_attribute = stp_policy_node_attr_show,
> + .store_attribute = stp_policy_node_attr_store,
> +};
> +
> +static struct stp_policy_node_attribute stp_policy_node_attr_range = {
> + .attr = {
> + .ca_owner = THIS_MODULE,
> + .ca_name = "masters",
> + .ca_mode = S_IRUGO | S_IWUSR,
> + },
> + .show = stp_policy_node_masters_show,
> + .store = stp_policy_node_masters_store,
> +};
> +
> +static struct stp_policy_node_attribute stp_policy_node_attr_channels = {
> + .attr = {
> + .ca_owner = THIS_MODULE,
> + .ca_name = "channels",
> + .ca_mode = S_IRUGO | S_IWUSR,
> + },
> + .show = stp_policy_node_channels_show,
> + .store = stp_policy_node_channels_store,
> +};
> +
> +static struct configfs_attribute *stp_policy_node_attrs[] = {
> + &stp_policy_node_attr_range.attr,
> + &stp_policy_node_attr_channels.attr,
> + NULL,
> +};
> +
> +static struct config_item_type stp_policy_type;
> +static struct config_item_type stp_policy_node_type;
> +
> +static struct config_group *
> +stp_policy_node_make(struct config_group *group, const char *name)
> +{
> + struct stp_policy_node *policy_node, *parent_node;
> + struct stp_policy *policy;
> +
> + if (group->cg_item.ci_type == &stp_policy_type) {
> + policy = container_of(group, struct stp_policy, group);
> + } else {
> + parent_node = container_of(group, struct stp_policy_node,
> + group);
> + policy = parent_node->policy;
> + }
> +
> + if (!policy->stm)
> + return ERR_PTR(-ENODEV);
> +
> + policy_node = kzalloc(sizeof(struct stp_policy_node), GFP_KERNEL);
> + if (!policy_node)
> + return ERR_PTR(-ENOMEM);
> +
> + config_group_init_type_name(&policy_node->group, name,
> + &stp_policy_node_type);
> +
> + policy_node->policy = policy;
> +
> + /* default values for the attributes */
> + policy_node->first_master = policy->stm->data->sw_start;
> + policy_node->last_master = policy->stm->data->sw_end;
> + policy_node->first_channel = 0;
> + policy_node->last_channel = policy->stm->data->sw_nchannels - 1;
> +
> + return &policy_node->group;
> +}
> +
> +static void
> +stp_policy_node_drop(struct config_group *group, struct config_item *item)
> +{
> + config_item_put(item);
> +}
> +
> +static struct configfs_group_operations stp_policy_node_group_ops = {
> + .make_group = stp_policy_node_make,
> + .drop_item = stp_policy_node_drop,
> +};
> +
> +static struct config_item_type stp_policy_node_type = {
> + .ct_item_ops = &stp_policy_node_item_ops,
> + .ct_group_ops = &stp_policy_node_group_ops,
> + .ct_attrs = stp_policy_node_attrs,
> + .ct_owner = THIS_MODULE,
> +};
> +
> +/*
> + * Root group: policies.
> + */
> +static struct configfs_attribute stp_policy_attr_device = {
> + .ca_owner = THIS_MODULE,
> + .ca_name = "device",
> + .ca_mode = S_IRUGO,
> +};
> +
> +static struct configfs_attribute *stp_policy_attrs[] = {
> + &stp_policy_attr_device,
> + NULL,
> +};
> +
> +static ssize_t stp_policy_attr_show(struct config_item *item,
> + struct configfs_attribute *attr,
> + char *page)
> +{
> + struct stp_policy *policy = to_stp_policy(item);
> + ssize_t count;
> +
> + count = sprintf(page, "%s\n",
> + (policy && policy->stm) ?
> + policy->stm->data->name :
> + "<none>");
> +
> + return count;
> +}
> +
> +void stp_policy_unbind(struct stp_policy *policy)
> +{
> + struct stm_device *stm = policy->stm;
> +
> + if (WARN_ON_ONCE(!policy->stm))
> + return;
> +
> + mutex_lock(&stm->policy_mutex);
> + stm->policy = NULL;
> + mutex_unlock(&stm->policy_mutex);
> +
> + policy->stm = NULL;
> +
> + stm_put_device(stm);
> +}
> +
> +static void stp_policy_release(struct config_item *item)
> +{
> + struct stp_policy *policy = to_stp_policy(item);
> +
> + stp_policy_unbind(policy);
> + kfree(policy);
> +}
> +
> +static struct configfs_item_operations stp_policy_item_ops = {
> + .release = stp_policy_release,
> + .show_attribute = stp_policy_attr_show,
> +};
> +
> +static struct configfs_group_operations stp_policy_group_ops = {
> + .make_group = stp_policy_node_make,
> +};
> +
> +static struct config_item_type stp_policy_type = {
> + .ct_item_ops = &stp_policy_item_ops,
> + .ct_group_ops = &stp_policy_group_ops,
> + .ct_attrs = stp_policy_attrs,
> + .ct_owner = THIS_MODULE,
> +};
> +
> +static struct config_group *
> +stp_policies_make(struct config_group *group, const char *name)
> +{
> + struct config_group *ret;
> + struct stm_device *stm;
> + char *devname, *p;
> +
> + devname = kasprintf(GFP_KERNEL, "%s", name);
> + if (!devname)
> + return ERR_PTR(-ENOMEM);
> +
> + /*
> + * node must look like <device_name>.<policy_name>, where
> + * <device_name> is the name of an existing stm device and
> + * <policy_name> is an arbitrary string
> + */
> + p = strchr(devname, '.');
> + if (!p) {
> + kfree(devname);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + *p++ = '\0';
> +
> + stm = stm_find_device(devname);
> + kfree(devname);
> +
> + if (!stm)
> + return ERR_PTR(-ENODEV);
> +
> + mutex_lock(&stm->policy_mutex);
> + if (stm->policy) {
> + ret = ERR_PTR(-EBUSY);
> + goto unlock_policy;
> + }
> +
> + stm->policy = kzalloc(sizeof(*stm->policy), GFP_KERNEL);
> + if (!stm->policy) {
> + ret = ERR_PTR(-ENOMEM);
> + goto unlock_policy;
> + }
> +
> + config_group_init_type_name(&stm->policy->group, name,
> + &stp_policy_type);
> + stm->policy->stm = stm;
> +
> + ret = &stm->policy->group;
> +
> +unlock_policy:
> + mutex_unlock(&stm->policy_mutex);
> +
> + if (IS_ERR(ret))
> + stm_put_device(stm);
> +
> + return ret;
> +}
> +
> +static struct configfs_group_operations stp_policies_group_ops = {
> + .make_group = stp_policies_make,
> +};
> +
> +static struct config_item_type stp_policies_type = {
> + .ct_group_ops = &stp_policies_group_ops,
> + .ct_owner = THIS_MODULE,
> +};
> +
> +static struct configfs_subsystem stp_policy_subsys = {
> + .su_group = {
> + .cg_item = {
> + .ci_namebuf = "stp-policy",
> + .ci_type = &stp_policies_type,
> + },
> + },
> +};
> +
> +/*
> + * Lock the policy mutex from the outside
> + */
> +static struct stp_policy_node *
> +__stp_policy_node_lookup(struct stp_policy *policy, char *s)
> +{
> + struct stp_policy_node *policy_node, *ret;
> + struct list_head *head = &policy->group.cg_children;
> + struct config_item *item;
> + char *start, *end = s;
> +
> + if (list_empty(head))
> + return NULL;
> +
> + /* return the first entry if everything else fails */
> + item = list_entry(head->next, struct config_item, ci_entry);
> + ret = to_stp_policy_node(item);
> +
> +next:
> + for (;;) {
> + start = strsep(&end, "/");
> + if (!start)
> + break;
> +
> + if (!*start)
> + continue;
> +
> + list_for_each_entry(item, head, ci_entry) {
> + policy_node = to_stp_policy_node(item);
> +
> + if (!strcmp(start,
> + policy_node->group.cg_item.ci_name)) {
> + ret = policy_node;
> +
> + if (!end)
> + goto out;
> +
> + head = &policy_node->group.cg_children;
> + goto next;
> + }
> + }
> + break;
> + }
> +
> +out:
> + return ret;
> +}
> +
> +
> +struct stp_policy_node *
> +stp_policy_node_lookup(struct stm_device *stm, char *s)
> +{
> + struct stp_policy_node *policy_node = NULL;
> +
> + mutex_lock(&stp_policy_subsys.su_mutex);
> +
> + mutex_lock(&stm->policy_mutex);
> + if (stm->policy)
> + policy_node = __stp_policy_node_lookup(stm->policy, s);
> + mutex_unlock(&stm->policy_mutex);
> +
> + if (policy_node)
> + config_item_get(&policy_node->group.cg_item);
> + mutex_unlock(&stp_policy_subsys.su_mutex);
> +
> + return policy_node;
> +}
> +
> +void stp_policy_node_put(struct stp_policy_node *policy_node)
> +{
> + config_item_put(&policy_node->group.cg_item);
> +}
> +
> +int __init stp_configfs_init(void)
> +{
> + int err;
> +
> + config_group_init(&stp_policy_subsys.su_group);
> + mutex_init(&stp_policy_subsys.su_mutex);
> + err = configfs_register_subsystem(&stp_policy_subsys);
> +
> + return err;
> +}
> +
> +void __exit stp_configfs_exit(void)
> +{
> + configfs_unregister_subsystem(&stp_policy_subsys);
> +}
> diff --git a/drivers/hwtracing/stm/stm.h b/drivers/hwtracing/stm/stm.h
> new file mode 100644
> index 0000000000..cf33bf976a
> --- /dev/null
> +++ b/drivers/hwtracing/stm/stm.h
> @@ -0,0 +1,87 @@
> +/*
> + * System Trace Module (STM) infrastructure
> + * Copyright (c) 2014, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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.
> + *
> + * STM class implements generic infrastructure for System Trace Module devices
> + * as defined in MIPI STPv2 specification.
> + */
> +
> +#ifndef _STM_STM_H_
> +#define _STM_STM_H_
> +
> +struct stp_policy;
> +struct stp_policy_node;
> +
> +struct stp_policy_node *
> +stp_policy_node_lookup(struct stm_device *stm, char *s);
> +void stp_policy_node_put(struct stp_policy_node *policy_node);
> +void stp_policy_unbind(struct stp_policy *policy);
> +
> +void stp_policy_node_get_ranges(struct stp_policy_node *policy_node,
> + unsigned int *mstart, unsigned int *mend,
> + unsigned int *cstart, unsigned int *cend);
> +int stp_configfs_init(void);
> +void stp_configfs_exit(void);
> +
> +struct stp_master {
> + unsigned int nr_free;
> + unsigned long chan_map[0];
> +};
> +
> +struct stm_device {
> + struct device dev;
> + struct module *owner;
> + struct stp_policy *policy;
> + struct mutex policy_mutex;
> + int major;
> + unsigned int sw_nmasters;
> + struct stm_data *data;
> + spinlock_t link_lock;
> + struct list_head link_list;
> + /* master allocation */
> + spinlock_t mc_lock;
> + struct stp_master *masters[0];
> +};
> +
> +#define to_stm_device(_d) \
> + container_of((_d), struct stm_device, dev)
> +
> +struct stm_output {
> + unsigned int master;
> + unsigned int channel;
> + unsigned int nr_chans;
> +};
> +
> +struct stm_file {
> + struct stm_device *stm;
> + struct stp_policy_node *policy_node;
> + struct stm_output output;
> +};
> +
> +struct stm_device *stm_find_device(const char *name);
> +void stm_put_device(struct stm_device *stm);
> +
> +struct stm_source_device {
> + struct device dev;
> + struct stm_source_data *data;
> + spinlock_t link_lock;
> + struct stm_device *link;
> + struct list_head link_entry;
> + /* one output per stm_source device */
> + struct stp_policy_node *policy_node;
> + struct stm_output output;
> +};
> +
> +#define to_stm_source_device(_d) \
> + container_of((_d), struct stm_source_device, dev)
> +
> +#endif /* _STM_STM_H_ */
> diff --git a/include/linux/stm.h b/include/linux/stm.h
> new file mode 100644
> index 0000000000..9d0083d364
> --- /dev/null
> +++ b/include/linux/stm.h
> @@ -0,0 +1,126 @@
> +/*
> + * System Trace Module (STM) infrastructure apis
> + * Copyright (C) 2014 Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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.
> + */
> +
> +#ifndef _STM_H_
> +#define _STM_H_
> +
> +#include <linux/device.h>
> +
> +/**
> + * enum stp_packet_type - STP packets that an STM driver sends
> + */
> +enum stp_packet_type {
> + STP_PACKET_DATA = 0,
> + STP_PACKET_FLAG,
> + STP_PACKET_USER,
> + STP_PACKET_MERR,
> + STP_PACKET_GERR,
> + STP_PACKET_TRIG,
> + STP_PACKET_XSYNC,
> +};
> +
> +/**
> + * enum stp_packet_flags - STP packet modifiers
> + */
> +enum stp_packet_flags {
> + STP_PACKET_MARKED = 0x1,
> + STP_PACKET_TIMESTAMPED = 0x2,
> +};
> +
> +struct stp_policy;
> +
> +struct stm_device;
> +
> +/**
> + * struct stm_data - STM device description and callbacks
> + * @name: device name
> + * @stm: internal structure, only used by stm class code
> + * @sw_start: first STP master available to software
> + * @sw_end: last STP master available to software
> + * @sw_nchannels: number of STP channels per master
> + * @sw_mmiosz: size of one channel's IO space, for mmap, optional
> + * @packet: callback that sends an STP packet
> + * @mmio_addr: mmap callback, optional
> + * @link: called when a new stm_source gets linked to us, optional
> + * @unlink: likewise for unlinking, again optional
> + * @set_options: set device-specific options on a channel
> + *
> + * Fill out this structure before calling stm_register_device() to create
> + * an STM device and stm_unregister_device() to destroy it. It will also be
> + * passed back to @packet(), @mmio_addr(), @link(), @unlink() and @set_options()
> + * callbacks.
> + *
> + * Normally, an STM device will have a range of masters available to software
> + * and the rest being statically assigned to various hardware trace sources.
> + * The former is defined by the the range [@sw_start..@sw_end] of the device
> + * description. That is, the lowest master that can be allocated to software
> + * writers is @sw_start and data from this writer will appear is @sw_start
> + * master in the STP stream.
> + */
> +struct stm_data {
> + const char *name;
> + struct stm_device *stm;
> + unsigned int sw_start;
> + unsigned int sw_end;
> + unsigned int sw_nchannels;
> + unsigned int sw_mmiosz;
> + ssize_t (*packet)(struct stm_data *, unsigned int,
> + unsigned int, unsigned int,
> + unsigned int, unsigned int,
> + const unsigned char *);
> + phys_addr_t (*mmio_addr)(struct stm_data *, unsigned int,
> + unsigned int, unsigned int);
> + int (*link)(struct stm_data *, unsigned int,
> + unsigned int);
> + void (*unlink)(struct stm_data *, unsigned int,
> + unsigned int);
> + long (*set_options)(struct stm_data *, unsigned int,
> + unsigned int, unsigned int,
> + unsigned long);
> +};
> +
> +int stm_register_device(struct device *parent, struct stm_data *stm_data,
> + struct module *owner);
> +void stm_unregister_device(struct stm_data *stm_data);
> +
> +struct stm_source_device;
> +
> +/**
> + * struct stm_source_data - STM source device description and callbacks
> + * @name: device name, will be used for policy lookup
> + * @src: internal structure, only used by stm class code
> + * @nr_chans: number of channels to allocate
> + * @link: called when this source gets linked to an STM device
> + * @unlink: called when this source is about to get unlinked from its STM
> + *
> + * Fill in this structure before calling stm_source_register_device() to
> + * register a source device. Also pass it to unregister and write calls.
> + */
> +struct stm_source_data {
> + const char *name;
> + struct stm_source_device *src;
> + unsigned int percpu;
> + unsigned int nr_chans;
> + int (*link)(struct stm_source_data *data);
> + void (*unlink)(struct stm_source_data *data);
> +};
> +
> +int stm_source_register_device(struct device *parent,
> + struct stm_source_data *data);
> +void stm_source_unregister_device(struct stm_source_data *data);
> +
> +int stm_source_write(struct stm_source_data *data, unsigned int chan,
> + const char *buf, size_t count);
> +
> +#endif /* _STM_H_ */
> diff --git a/include/uapi/linux/stm.h b/include/uapi/linux/stm.h
> new file mode 100644
> index 0000000000..626a8d3f63
> --- /dev/null
> +++ b/include/uapi/linux/stm.h
> @@ -0,0 +1,50 @@
> +/*
> + * System Trace Module (STM) userspace interfaces
> + * Copyright (c) 2014, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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.
> + *
> + * STM class implements generic infrastructure for System Trace Module devices
> + * as defined in MIPI STPv2 specification.
> + */
> +
> +#ifndef _UAPI_LINUX_STM_H
> +#define _UAPI_LINUX_STM_H
> +
> +#include <linux/types.h>
> +
> +/**
> + * struct stp_policy_id - identification for the STP policy
> + * @size: size of the structure including real id[] length
> + * @master: assigned master
> + * @channel: first assigned channel
> + * @width: number of requested channels
> + * @id: identification string
> + *
> + * User must calculate the total size of the structure and put it into
> + * @size field, fill out the @id and desired @width. In return, kernel
> + * fills out @master, @channel and @width.
> + */
> +struct stp_policy_id {
> + __u32 size;
> + __u16 master;
> + __u16 channel;
> + __u16 width;
> + /* padding */
> + __u16 __reserved_0;
> + __u32 __reserved_1;
> + char id[0];
> +};
> +
> +#define STP_POLICY_ID_SET _IOWR('%', 0, struct stp_policy_id)
> +#define STP_POLICY_ID_GET _IOR('%', 1, struct stp_policy_id)
> +#define STP_SET_OPTIONS _IOW('%', 2, __u64)
> +
> +#endif /* _UAPI_LINUX_STM_H */
> --
> 2.1.4
>
Reviewed-by: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
^ permalink raw reply
* Re: Next round: revised futex(2) man page for review
From: Darren Hart @ 2015-08-05 22:21 UTC (permalink / raw)
To: Michael Kerrisk (man-pages)
Cc: Thomas Gleixner, Torvald Riegel, Carlos O'Donell, Ingo Molnar,
Jakub Jelinek, linux-man, lkml, Davidlohr Bueso, Arnd Bergmann,
Steven Rostedt, Peter Zijlstra, Linux API, Roland McGrath,
Anton Blanchard, Eric Dumazet, bill o gallmeister, Jan Kiszka,
Daniel Wagner, Rich Felker, Andy Lutomirski, bert hubert,
Rusty Russell, Heinrich Schuchardt
In-Reply-To: <55B61EF3.7080302@gmail.com>
On Mon, Jul 27, 2015 at 02:07:15PM +0200, Michael Kerrisk (man-pages) wrote:
> Hello all,
>
Michael, thank you for your diligence in following up and collecting
reviews. I've attempted to respond to what I was specifically called out
in or where I had something specific to add in addition to other
replies.
After this, will you send another version (numbered for reference
maybe?) with any remaining FIXMEs that haven't yet been addressed
according to your accounting?
...
> Priority-inheritance futexes
> Linux supports priority-inheritance (PI) futexes in order to han‐
> dle priority-inversion problems that can be encountered with nor‐
> mal futex locks. Priority inversion is the problem that occurs
> when a high-priority task is blocked waiting to acquire a lock
> held by a low-priority task, while tasks at an intermediate pri‐
> ority continuously preempt the low-priority task from the CPU.
> Consequently, the low-priority task makes no progress toward
> releasing the lock, and the high-priority task remains blocked.
>
> Priority inheritance is a mechanism for dealing with the prior‐
> ity-inversion problem. With this mechanism, when a high-priority
> task becomes blocked by a lock held by a low-priority task, the
> latter's priority is temporarily raised to that of the former, so
> that it is not preempted by any intermediate level tasks, and can
> thus make progress toward releasing the lock. To be effective,
> priority inheritance must be transitive, meaning that if a high-
> priority task blocks on a lock held by a lower-priority task that
> is itself blocked by lock held by another intermediate-priority
> task (and so on, for chains of arbitrary length), then both of
> those task (or more generally, all of the tasks in a lock chain)
> have their priorities raised to be the same as the high-priority
> task.
>
> .\" FIXME XXX The following is my attempt at a definition of PI futexes,
> .\" based on mail discussions with Darren Hart. Does it seem okay?
>
> From a user-space perspective, what makes a futex PI-aware is a
> policy agreement between user space and the kernel about the
> value of the futex word (described in a moment), coupled with the
> use of the PI futex operations described below (in particular,
> FUTEX_LOCK_PI, FUTEX_TRYLOCK_PI, and FUTEX_CMP_REQUEUE_PI).
Yes. Was this intended to be a complete opcode list? PI operations must
use paired operations.
(FUTEX_LOCK_PI | FUTEX_TRYLOCK_PI) : FUTEX_UNLOCK_PI
FUTEX_WAIT_REQUEUE_PI : FUTEX_CMP_REQUEUE_PI
And their PRIVATE counterparts of course (which is assumed as it is a
flag to the opcode).
>
> .\" FIXME XXX ===== Start of adapted Hart/Guniguntala text =====
> .\" The following text is drawn from the Hart/Guniguntala paper
> .\" (listed in SEE ALSO), but I have reworded some pieces
> .\" significantly. Please check it.
>
> The PI futex operations described below differ from the other
> futex operations in that they impose policy on the use of the
> value of the futex word:
>
> * If the lock is not acquired, the futex word's value shall be
> 0.
>
> * If the lock is acquired, the futex word's value shall be the
> thread ID (TID; see gettid(2)) of the owning thread.
>
> * If the lock is owned and there are threads contending for the
> lock, then the FUTEX_WAITERS bit shall be set in the futex
> word's value; in other words, this value is:
>
> FUTEX_WAITERS | TID
>
>
> Note that a PI futex word never just has the value FUTEX_WAITERS,
> which is a permissible state for non-PI futexes.
The second clause is inappropriate. I don't know if that was yours or
mine, but non-PI futexes do not have a kernel defined value policy, so
==FUTEX_WAITERS cannot be a "permissible state" as any value is
permissible for non-PI futexes, and none have a kernel defined state.
Perhaps include a Note under the third bullet as:
Note: It is invalid for a PI futex word to have no owner and
FUTEX_WAITERS set.
>
> With this policy in place, a user-space application can acquire a
> not-acquired lock or release a lock that no other threads try to
"that no other threads try to acquire" seems out of place. I think
"atomic instructions" is sufficient to express how contention is
handled.
> acquire using atomic instructions executed in user space (e.g., a
> compare-and-swap operation such as cmpxchg on the x86 architec‐
> ture). Acquiring a lock simply consists of using compare-and-
> swap to atomically set the futex word's value to the caller's TID
> if its previous value was 0. Releasing a lock requires using
> compare-and-swap to set the futex word's value to 0 if the previ‐
> ous value was the expected TID.
>
> If a futex is already acquired (i.e., has a nonzero value), wait‐
> ers must employ the FUTEX_LOCK_PI operation to acquire the lock.
> If other threads are waiting for the lock, then the FUTEX_WAITERS
> bit is set in the futex value; in this case, the lock owner must
> employ the FUTEX_UNLOCK_PI operation to release the lock.
>
> In the cases where callers are forced into the kernel (i.e.,
> required to perform a futex() call), they then deal directly with
> a so-called RT-mutex, a kernel locking mechanism which implements
> the required priority-inheritance semantics. After the RT-mutex
> is acquired, the futex value is updated accordingly, before the
> calling thread returns to user space.
This last paragraph relies on kernel implementation rather than
behavior. If the RT-mutex is renamed or the mechanism is implemented
differently in futexes, this section will require updating. Is that
appropriate for a user-space man page?
> .\" FIXME ===== End of adapted Hart/Guniguntala text =====
>
>
>
> .\" FIXME We need some explanation in the following paragraph of *why*
> .\" it is important to note that "the kernel will update the
> .\" futex word's value prior
> It is important to note to returning to user space" . Can someone
> explain? that the kernel will update the futex word's value
> prior to returning to user space. Unlike the other futex opera‐
> tions described above, the PI futex operations are designed for
> the implementation of very specific IPC mechanisms.
If the kernel didn't perform the update prior to returning to userspace,
we could end up in an invalid state. Such as having an owner, but the
value being 0. Or having waiters, but not having FUTEX_WAITERS set.
> .\"
> .\" FIXME XXX In discussing errors for FUTEX_CMP_REQUEUE_PI, Darren Hart
> .\" made the observation that "EINVAL is returned if the non-pi
> .\" to pi or op pairing semantics are violated."
> .\" Probably there needs to be a general statement about this
> .\" requirement, probably located at about this point in the page.
> .\" Darren (or someone else), care to take a shot at this?
We can probably borrow from either the futex.c comments or the
futex-requeue-pi.txt in Documentation. Also, it is important to note
that the PI requeue operations require two distinct uadders (although
that is implied by requiring "non-pi to pi" as a futex cannot be both.
Or... perhaps something like:
Due to the kernel imposed futex word value policy, PI futex
operations have additional usage requirements:
FUTEX_WAIT_REQUEUE_PI must be paired with FUTEX_CMP_REQUEUE_PI
and be performed from a non-pi futex to a distinct pi futex.
Failing to do so will return EINVAL. Additionally,
FUTEX_CMP_REQUEUE_PI requires that nr_wake=1. [We state in the
docs that nr_requeue should be INT_MAX for broadcast and 0 for
signal... but that may be overly specific to libc for this
manual]
Similarly, FUTEX_UNLOCK_PI must only be called on a futex owned
by the calling thread as defined by the value policy, otherwise
EPERM is returned.
Were you looking for something like that - or were you looking for
justification for these requirements?
...
> FUTEX_LOCK_PI (since Linux 2.6.18)
> .\" FIXME I did some significant rewording of tglx's text to create
> .\" the text below.
> .\" Please check the following paragraph, in case I injected
> .\" errors.
> This operation is used after after an attempt to acquire
> the lock via an atomic user-space instruction failed
> because the futex word has a nonzero value—specifically,
> because it contained the namespace-specific TID of the
> lock owner.
Acked.
...
> FUTEX_TRYLOCK_PI (since Linux 2.6.18)
> .\" FIXME I think it would be helpful here to say a few more words about
> .\" the difference(s) between FUTEX_LOCK_PI and FUTEX_TRYLOCK_PI.
> .\" Can someone propose something?
> This operation tries to acquire the futex at uaddr. It
> deals with the situation where the TID value at uaddr is
> 0, but the FUTEX_WAITERS bit is set. User space cannot
> handle this condition in a race-free manner
> .\" FIXME How does the situation in the previous sentence come about?
> .\" Probably it would be helpful to say something about that in
> .\" the man page.
> .\" FIXME And *how* does FUTEX_TRYLOCK_PI deal with this situation?
I guess I wouldn't expect to see this detail in the manual. That state
should never exist in userspace as far as I understand it, which makes
it a kernel implementation detail and not relevant to a usage manual.
...
>
> FUTEX_CMP_REQUEUE_PI (since Linux 2.6.31)
> This operation is a PI-aware variant of FUTEX_CMP_REQUEUE.
> It requeues waiters that are blocked via
> FUTEX_WAIT_REQUEUE_PI on uaddr from a non-PI source futex
> (uaddr) to a PI target futex (uaddr2).
>
> As with FUTEX_CMP_REQUEUE, this operation wakes up a maxi‐
> mum of val waiters that are waiting on the futex at uaddr.
> However, for FUTEX_CMP_REQUEUE_PI, val is required to be 1
> (since the main point is to avoid a thundering herd). The
> remaining waiters are removed from the wait queue of the
> source futex at uaddr and added to the wait queue of the
> target futex at uaddr2.
>
> The val2 and val3 arguments serve the same purposes as for
> FUTEX_CMP_REQUEUE.
> .\" FIXME The page at http://locklessinc.com/articles/futex_cheat_sheet/
> .\" notes that "priority-inheritance Futex to priority-inheritance
> .\" Futex requeues are currently unsupported". Do we need to say
> .\" something in the man page about that?
>
I noted this above in response to your request for detail about the
*REQUEUE_PI semantics and error codes. Was that sufficient?
>
>
> FUTEX_WAIT_REQUEUE_PI (since Linux 2.6.31)
>
> .\" FIXME I find the next sentence (from tglx) pretty hard to grok.
> .\" Could someone explain it a bit more?
>
> Wait operation to wait on a non-PI futex at uaddr and
> potentially be requeued onto a PI futex at uaddr2. The
> wait operation on uaddr is the same as FUTEX_WAIT.
The point tglx is making here is that you must know ahead of time and
tell the kernel that you intend to use this futex in a REQUEUE_PI
operation, and not a regular REQUEUE. This is determined by the both the
op codes as well as the required arguments, which I also documented
above. Is more detail required?
>
> .\" FIXME I'm not quite clear on the meaning of the following sentence.
> .\" Is this trying to say that while blocked in a
> .\" FUTEX_WAIT_REQUEUE_PI, it could happen that another
> .\" task does a FUTEX_WAKE on uaddr that simply causes
> .\" a normal wake, with the result that the FUTEX_WAIT_REQUEUE_PI
> .\" does not complete? What happens then to the FUTEX_WAIT_REQUEUE_PI
> .\" opertion? Does it remain blocked, or does it unblock
> .\" In which case, what does user space see?
>
> The
> waiter can be removed from the wait on uaddr via
> FUTEX_WAKE without requeueing on uaddr2.
Userspace should see the task wake and continue executing. This would
effectively be a cancelation operation - which I didn't think was
supported. Thomas?
...
>
> .\" FIXME XXX The following is a reworded version of Darren Hart's text.
> .\" Please check that I did not introduce any errors.
> EINVAL (FUTEX_CMP_REQUEUE_PI) An attempt was made to requeue a
> waiter to a futex other than that specified by the match‐
> ing FUTEX_WAIT_REQUEUE_PI call for that waiter.
Acked.
Thanks Michael!
--
Darren Hart
Intel Open Source Technology Center
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox