Re: [GIT PULL] Btrfs
On Sun, 2 Feb 2014 01:28:07 AM Filipe David Manana wrote: One of the kbuild test robots reported this a few days ago too. The following patch, sent shortly after the robot's warning, fixes it: https://patchwork.kernel.org/patch/3554671/ I can confirm that fixes the bug for me (with a different config than my usual to trigger the bug) at v3.13-11307-g5cb480f. Much obliged! Tested-by: Chris Samuel ch...@csamuel.org All the best, Chris -- Chris Samuel : http://www.csamuel.org/ : Melbourne, VIC This email may come with a PGP signature as a file. Do not panic. For more info see: http://en.wikipedia.org/wiki/OpenPGP signature.asc Description: This is a digitally signed message part.
Re: [GIT PULL 00/12] perf/urgent fixes
* Arnaldo Carvalho de Melo a...@infradead.org wrote: From: Arnaldo Carvalho de Melo a...@ghostprotocols.net Hi Ingo, Please consider pulling, - Arnaldo The following changes since commit 0d4dd797564cddc1f71ab0b239e9ea50ddd40b2a: perf/doc: Remove mention of non-existent set_perf_event_pending() from design.txt (2014-01-26 09:37:48 +0100) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-urgent-for-mingo for you to fetch changes up to d3b70220292c40d3b499797fd2f33f608fc35edb: perf buildid-cache: Check relocation when checking for existing kcore (2014-01-31 17:21:54 -0300) perf/urgent fixes: . Fix annotation for relocated kernel (Adrian Hunter) . Fix demangling of symbols in kernel and kernel modules (Avi Kivity) . Fix include for non x86 architectures (Francesco Fusco) . Fix ARM64 memory barriers (Peter Zijlstra) Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com Adrian Hunter (9): perf symbols: Fix symbol annotation for relocated kernel perf tools: Add kallsyms__get_function_start() perf machine: Add machine__get_kallsyms_filename() perf machine: Set up ref_reloc_sym in machine__create_kernel_maps() perf record: Get ref_reloc_sym from kernel map perf symbols: Prevent the use of kcore if the kernel has moved perf tests: No need to set up ref_reloc_sym perf tools: Adjust kallsyms for relocated kernel perf buildid-cache: Check relocation when checking for existing kcore Avi Kivity (1): perf tools: Demangle kernel and kernel module symbols too Francesco Fusco (1): perf tools: Fix include for non x86 architectures Peter Zijlstra (1): perf tools: Fix ARGH64 memory barriers tools/perf/builtin-buildid-cache.c | 33 --- tools/perf/builtin-record.c | 10 ++ tools/perf/perf.h | 4 +-- tools/perf/tests/vmlinux-kallsyms.c | 10 -- tools/perf/util/event.c | 36 ++-- tools/perf/util/event.h | 6 ++-- tools/perf/util/include/asm/hash.h | 6 tools/perf/util/machine.c | 42 +++- tools/perf/util/machine.h | 2 ++ tools/perf/util/map.c | 5 +-- tools/perf/util/map.h | 1 + tools/perf/util/symbol-elf.c| 4 ++- tools/perf/util/symbol.c| 65 + 13 files changed, 162 insertions(+), 62 deletions(-) create mode 100644 tools/perf/util/include/asm/hash.h Pulled, thanks a lot Arnaldo! Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[tip:perf/urgent] perf tools: Add kallsyms__get_function_start()
Commit-ID: 29b596b57426831fce92cd0ebb01c77627616fdf
Gitweb: http://git.kernel.org/tip/29b596b57426831fce92cd0ebb01c77627616fdf
Author: Adrian Hunter adrian.hun...@intel.com
AuthorDate: Wed, 29 Jan 2014 16:14:37 +0200
Committer: Arnaldo Carvalho de Melo a...@redhat.com
CommitDate: Fri, 31 Jan 2014 17:21:47 -0300
perf tools: Add kallsyms__get_function_start()
Separate out the logic used to find the start address of the reference
symbol used to track kernel relocation. kallsyms__get_function_start()
is used in subsequent patches.
Signed-off-by: Adrian Hunter adrian.hun...@intel.com
Tested-by: Jiri Olsa jo...@redhat.com
Cc: David Ahern dsah...@gmail.com
Cc: Frederic Weisbecker fweis...@gmail.com
Cc: Ingo Molnar mi...@redhat.com
Cc: Jiri Olsa jo...@redhat.com
Cc: Mike Galbraith efa...@gmx.de
Cc: Namhyung Kim namhy...@gmail.com
Cc: Paul Mackerras pau...@samba.org
Cc: Peter Zijlstra a.p.zijls...@chello.nl
Cc: Stephane Eranian eran...@google.com
Link:
http://lkml.kernel.org/r/1391004884-10334-3-git-send-email-adrian.hun...@intel.com
Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com
---
tools/perf/util/event.c | 18 +++---
tools/perf/util/event.h | 3 +++
2 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 1fc1c2f..17476df 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -470,6 +470,17 @@ static int find_symbol_cb(void *arg, const char *name,
char type,
return 1;
}
+u64 kallsyms__get_function_start(const char *kallsyms_filename,
+const char *symbol_name)
+{
+ struct process_symbol_args args = { .name = symbol_name, };
+
+ if (kallsyms__parse(kallsyms_filename, args, find_symbol_cb) = 0)
+ return 0;
+
+ return args.start;
+}
+
int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
perf_event__handler_t process,
struct machine *machine,
@@ -480,13 +491,13 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool
*tool,
char path[PATH_MAX];
char name_buff[PATH_MAX];
struct map *map;
+ u64 start;
int err;
/*
* We should get this from /sys/kernel/sections/.text, but till that is
* available use this, and after it is use this as a fallback for older
* kernels.
*/
- struct process_symbol_args args = { .name = symbol_name, };
union perf_event *event = zalloc((sizeof(event-mmap) +
machine-id_hdr_size));
if (event == NULL) {
@@ -513,7 +524,8 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool
*tool,
}
}
- if (kallsyms__parse(filename, args, find_symbol_cb) = 0) {
+ start = kallsyms__get_function_start(filename, symbol_name);
+ if (!start) {
free(event);
return -ENOENT;
}
@@ -525,7 +537,7 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool
*tool,
event-mmap.header.type = PERF_RECORD_MMAP;
event-mmap.header.size = (sizeof(event-mmap) -
(sizeof(event-mmap.filename) - size) +
machine-id_hdr_size);
- event-mmap.pgoff = args.start;
+ event-mmap.pgoff = start;
event-mmap.start = map-start;
event-mmap.len = map-end - event-mmap.start;
event-mmap.pid = machine-pid;
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index faf6e21..66a0c03 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -279,4 +279,7 @@ size_t perf_event__fprintf_mmap2(union perf_event *event,
FILE *fp);
size_t perf_event__fprintf_task(union perf_event *event, FILE *fp);
size_t perf_event__fprintf(union perf_event *event, FILE *fp);
+u64 kallsyms__get_function_start(const char *kallsyms_filename,
+const char *symbol_name);
+
#endif /* __PERF_RECORD_H */
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[tip:perf/urgent] perf tools: Demangle kernel and kernel module symbols too
Commit-ID: 950b8354716eb1f9c0b39777d379efa5f4125c04 Gitweb: http://git.kernel.org/tip/950b8354716eb1f9c0b39777d379efa5f4125c04 Author: Avi Kivity a...@cloudius-systems.com AuthorDate: Wed, 22 Jan 2014 21:58:46 +0200 Committer: Arnaldo Carvalho de Melo a...@redhat.com CommitDate: Mon, 27 Jan 2014 11:47:27 -0300 perf tools: Demangle kernel and kernel module symbols too Some kernels contain C++ code, and thus their symbols need to be demangled. This allows 'perf kvm top' to generate readable output. Signed-off-by: Avi Kivity a...@cloudius-systems.com Cc: Ingo Molnar mi...@kernel.org Cc: Paul Mackerras pau...@samba.org Cc: Peter Zijlstra a.p.zijls...@chello.nl Link: http://lkml.kernel.org/r/26f71bf5bf7ee1408e3f1a803556d5df18223ef1.1390420726.git@cloudius-systems.com Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com --- tools/perf/util/symbol-elf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c index 7594567..8f12f0f 100644 --- a/tools/perf/util/symbol-elf.c +++ b/tools/perf/util/symbol-elf.c @@ -922,6 +922,7 @@ int dso__load_sym(struct dso *dso, struct map *map, (u64)shdr.sh_offset); sym.st_value -= shdr.sh_addr - shdr.sh_offset; } +new_symbol: /* * We need to figure out if the object was created from C++ sources * DWARF DW_compile_unit has this, but we don't always have access @@ -933,7 +934,6 @@ int dso__load_sym(struct dso *dso, struct map *map, if (demangled != NULL) elf_name = demangled; } -new_symbol: f = symbol__new(sym.st_value, sym.st_size, GELF_ST_BIND(sym.st_info), elf_name); free(demangled); -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[tip:perf/urgent] perf tests: No need to set up ref_reloc_sym
Commit-ID: c080f72753def150993144d755379941f8b14683
Gitweb: http://git.kernel.org/tip/c080f72753def150993144d755379941f8b14683
Author: Adrian Hunter adrian.hun...@intel.com
AuthorDate: Wed, 29 Jan 2014 16:14:42 +0200
Committer: Arnaldo Carvalho de Melo a...@redhat.com
CommitDate: Fri, 31 Jan 2014 17:21:52 -0300
perf tests: No need to set up ref_reloc_sym
Now that ref_reloc_sym is set up by machine__create_kernel_maps(), the
vmlinux symtab matches kallsyms test does have to do it.
Signed-off-by: Adrian Hunter adrian.hun...@intel.com
Tested-by: Jiri Olsa jo...@redhat.com
Cc: David Ahern dsah...@gmail.com
Cc: Frederic Weisbecker fweis...@gmail.com
Cc: Ingo Molnar mi...@redhat.com
Cc: Jiri Olsa jo...@redhat.com
Cc: Mike Galbraith efa...@gmx.de
Cc: Namhyung Kim namhy...@gmail.com
Cc: Paul Mackerras pau...@samba.org
Cc: Peter Zijlstra a.p.zijls...@chello.nl
Cc: Stephane Eranian eran...@google.com
Link:
http://lkml.kernel.org/r/1391004884-10334-8-git-send-email-adrian.hun...@intel.com
Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com
---
tools/perf/tests/vmlinux-kallsyms.c | 10 --
1 file changed, 10 deletions(-)
diff --git a/tools/perf/tests/vmlinux-kallsyms.c
b/tools/perf/tests/vmlinux-kallsyms.c
index 2bd13ed..3d90880 100644
--- a/tools/perf/tests/vmlinux-kallsyms.c
+++ b/tools/perf/tests/vmlinux-kallsyms.c
@@ -26,7 +26,6 @@ int test__vmlinux_matches_kallsyms(void)
struct map *kallsyms_map, *vmlinux_map;
struct machine kallsyms, vmlinux;
enum map_type type = MAP__FUNCTION;
- struct ref_reloc_sym ref_reloc_sym = { .name = _stext, };
u64 mem_start, mem_end;
/*
@@ -70,14 +69,6 @@ int test__vmlinux_matches_kallsyms(void)
*/
kallsyms_map = machine__kernel_map(kallsyms, type);
- sym = map__find_symbol_by_name(kallsyms_map, ref_reloc_sym.name, NULL);
- if (sym == NULL) {
- pr_debug(dso__find_symbol_by_name );
- goto out;
- }
-
- ref_reloc_sym.addr = UM(sym-start);
-
/*
* Step 5:
*
@@ -89,7 +80,6 @@ int test__vmlinux_matches_kallsyms(void)
}
vmlinux_map = machine__kernel_map(vmlinux, type);
- map__kmap(vmlinux_map)-ref_reloc_sym = ref_reloc_sym;
/*
* Step 6:
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[tip:perf/urgent] perf machine: Add machine__get_kallsyms_filename()
Commit-ID: 15a0a8706c32bd38bff9ebf7c6ef24f32d1ea921
Gitweb: http://git.kernel.org/tip/15a0a8706c32bd38bff9ebf7c6ef24f32d1ea921
Author: Adrian Hunter adrian.hun...@intel.com
AuthorDate: Wed, 29 Jan 2014 16:14:38 +0200
Committer: Arnaldo Carvalho de Melo a...@redhat.com
CommitDate: Fri, 31 Jan 2014 17:21:48 -0300
perf machine: Add machine__get_kallsyms_filename()
Separate out the logic used to make the kallsyms full path name for a
machine. It will be reused in a subsequent patch.
Signed-off-by: Adrian Hunter adrian.hun...@intel.com
Tested-by: Jiri Olsa jo...@redhat.com
Cc: David Ahern dsah...@gmail.com
Cc: Frederic Weisbecker fweis...@gmail.com
Cc: Ingo Molnar mi...@redhat.com
Cc: Jiri Olsa jo...@redhat.com
Cc: Mike Galbraith efa...@gmx.de
Cc: Namhyung Kim namhy...@gmail.com
Cc: Paul Mackerras pau...@samba.org
Cc: Peter Zijlstra a.p.zijls...@chello.nl
Cc: Stephane Eranian eran...@google.com
Link:
http://lkml.kernel.org/r/1391004884-10334-4-git-send-email-adrian.hun...@intel.com
Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com
---
tools/perf/util/machine.c | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index ded7459..290c2e6 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -496,19 +496,22 @@ static int symbol__in_kernel(void *arg, const char *name,
return 1;
}
+static void machine__get_kallsyms_filename(struct machine *machine, char *buf,
+ size_t bufsz)
+{
+ if (machine__is_default_guest(machine))
+ scnprintf(buf, bufsz, %s, symbol_conf.default_guest_kallsyms);
+ else
+ scnprintf(buf, bufsz, %s/proc/kallsyms, machine-root_dir);
+}
+
/* Figure out the start address of kernel map from /proc/kallsyms */
static u64 machine__get_kernel_start_addr(struct machine *machine)
{
- const char *filename;
- char path[PATH_MAX];
+ char filename[PATH_MAX];
struct process_args args;
- if (machine__is_default_guest(machine))
- filename = (char *)symbol_conf.default_guest_kallsyms;
- else {
- sprintf(path, %s/proc/kallsyms, machine-root_dir);
- filename = path;
- }
+ machine__get_kallsyms_filename(machine, filename, PATH_MAX);
if (symbol__restricted_filename(filename, /proc/kallsyms))
return 0;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[tip:perf/urgent] perf machine: Set up ref_reloc_sym in machine__create_kernel_maps()
Commit-ID: 5512cf24bed2de56f1ef44b6cc9a0a9b15499cea
Gitweb: http://git.kernel.org/tip/5512cf24bed2de56f1ef44b6cc9a0a9b15499cea
Author: Adrian Hunter adrian.hun...@intel.com
AuthorDate: Wed, 29 Jan 2014 16:14:39 +0200
Committer: Arnaldo Carvalho de Melo a...@redhat.com
CommitDate: Fri, 31 Jan 2014 17:21:49 -0300
perf machine: Set up ref_reloc_sym in machine__create_kernel_maps()
The ref_reloc_sym is always needed for the kernel map in order to check
for relocation. Consequently set it up when the kernel map is created.
Otherwise it was only being set up by 'perf record'.
Signed-off-by: Adrian Hunter adrian.hun...@intel.com
Tested-by: Jiri Olsa jo...@redhat.com
Cc: David Ahern dsah...@gmail.com
Cc: Frederic Weisbecker fweis...@gmail.com
Cc: Ingo Molnar mi...@redhat.com
Cc: Jiri Olsa jo...@redhat.com
Cc: Mike Galbraith efa...@gmx.de
Cc: Namhyung Kim namhy...@gmail.com
Cc: Paul Mackerras pau...@samba.org
Cc: Peter Zijlstra a.p.zijls...@chello.nl
Cc: Stephane Eranian eran...@google.com
Link:
http://lkml.kernel.org/r/1391004884-10334-5-git-send-email-adrian.hun...@intel.com
Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com
---
tools/perf/util/machine.c | 23 +++
tools/perf/util/machine.h | 2 ++
2 files changed, 25 insertions(+)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 290c2e6..c872991 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -832,9 +832,25 @@ static int machine__create_modules(struct machine *machine)
return 0;
}
+const char *ref_reloc_sym_names[] = {_text, _stext, NULL};
+
int machine__create_kernel_maps(struct machine *machine)
{
struct dso *kernel = machine__get_kernel(machine);
+ char filename[PATH_MAX];
+ const char *name;
+ u64 addr = 0;
+ int i;
+
+ machine__get_kallsyms_filename(machine, filename, PATH_MAX);
+
+ for (i = 0; (name = ref_reloc_sym_names[i]) != NULL; i++) {
+ addr = kallsyms__get_function_start(filename, name);
+ if (addr)
+ break;
+ }
+ if (!addr)
+ return -1;
if (kernel == NULL ||
__machine__create_kernel_maps(machine, kernel) 0)
@@ -853,6 +869,13 @@ int machine__create_kernel_maps(struct machine *machine)
* Now that we have all the maps created, just set the -end of them:
*/
map_groups__fixup_end(machine-kmaps);
+
+ if (maps__set_kallsyms_ref_reloc_sym(machine-vmlinux_maps, name,
+addr)) {
+ machine__destroy_kernel_maps(machine);
+ return -1;
+ }
+
return 0;
}
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index 4771330..f77e91e 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -18,6 +18,8 @@ union perf_event;
#defineHOST_KERNEL_ID (-1)
#defineDEFAULT_GUEST_KERNEL_ID (0)
+extern const char *ref_reloc_sym_names[];
+
struct machine {
struct rb_noderb_node;
pid_t pid;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[tip:perf/urgent] perf symbols: Prevent the use of kcore if the kernel has moved
Commit-ID: a00d28cb72d3629c6481fe21ba6c6b4f96caed49
Gitweb: http://git.kernel.org/tip/a00d28cb72d3629c6481fe21ba6c6b4f96caed49
Author: Adrian Hunter adrian.hun...@intel.com
AuthorDate: Wed, 29 Jan 2014 16:14:41 +0200
Committer: Arnaldo Carvalho de Melo a...@redhat.com
CommitDate: Fri, 31 Jan 2014 17:21:51 -0300
perf symbols: Prevent the use of kcore if the kernel has moved
Use of kcore is predicated upon it matching the recorded data. If the
kernel has been relocated at boot time (i.e. since the data was
recorded) then do not use kcore.
Note that it is possible to make a copy of kcore at the time the data is
recorded using 'perf buildid-cache'. Then the perf tools will use the
copy because it does match the data.
Signed-off-by: Adrian Hunter adrian.hun...@intel.com
Tested-by: Jiri Olsa jo...@redhat.com
Cc: David Ahern dsah...@gmail.com
Cc: Frederic Weisbecker fweis...@gmail.com
Cc: Ingo Molnar mi...@redhat.com
Cc: Jiri Olsa jo...@redhat.com
Cc: Mike Galbraith efa...@gmx.de
Cc: Namhyung Kim namhy...@gmail.com
Cc: Paul Mackerras pau...@samba.org
Cc: Peter Zijlstra a.p.zijls...@chello.nl
Cc: Stephane Eranian eran...@google.com
Link:
http://lkml.kernel.org/r/1391004884-10334-7-git-send-email-adrian.hun...@intel.com
Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com
---
tools/perf/util/symbol.c | 25 +
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 39ce9ad..4ac1f87 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -976,6 +976,23 @@ static int validate_kcore_modules(const char
*kallsyms_filename,
return 0;
}
+static int validate_kcore_addresses(const char *kallsyms_filename,
+ struct map *map)
+{
+ struct kmap *kmap = map__kmap(map);
+
+ if (kmap-ref_reloc_sym kmap-ref_reloc_sym-name) {
+ u64 start;
+
+ start = kallsyms__get_function_start(kallsyms_filename,
+kmap-ref_reloc_sym-name);
+ if (start != kmap-ref_reloc_sym-addr)
+ return -EINVAL;
+ }
+
+ return validate_kcore_modules(kallsyms_filename, map);
+}
+
struct kcore_mapfn_data {
struct dso *dso;
enum map_type type;
@@ -1019,8 +1036,8 @@ static int dso__load_kcore(struct dso *dso, struct map
*map,
kallsyms_filename))
return -EINVAL;
- /* All modules must be present at their original addresses */
- if (validate_kcore_modules(kallsyms_filename, map))
+ /* Modules and kernel must be present at their original addresses */
+ if (validate_kcore_addresses(kallsyms_filename, map))
return -EINVAL;
md.dso = dso;
@@ -1424,7 +1441,7 @@ static int find_matching_kcore(struct map *map, char
*dir, size_t dir_sz)
continue;
scnprintf(kallsyms_filename, sizeof(kallsyms_filename),
%s/%s/kallsyms, dir, dent-d_name);
- if (!validate_kcore_modules(kallsyms_filename, map)) {
+ if (!validate_kcore_addresses(kallsyms_filename, map)) {
strlcpy(dir, kallsyms_filename, dir_sz);
ret = 0;
break;
@@ -1479,7 +1496,7 @@ static char *dso__find_kallsyms(struct dso *dso, struct
map *map)
if (fd != -1) {
close(fd);
/* If module maps match go with /proc/kallsyms */
- if (!validate_kcore_modules(/proc/kallsyms, map))
+ if (!validate_kcore_addresses(/proc/kallsyms, map))
goto proc_kallsyms;
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[tip:perf/urgent] perf record: Get ref_reloc_sym from kernel map
Commit-ID: 0ae617bedde062003fd70e566e9a2601e273ea0e
Gitweb: http://git.kernel.org/tip/0ae617bedde062003fd70e566e9a2601e273ea0e
Author: Adrian Hunter adrian.hun...@intel.com
AuthorDate: Wed, 29 Jan 2014 16:14:40 +0200
Committer: Arnaldo Carvalho de Melo a...@redhat.com
CommitDate: Fri, 31 Jan 2014 17:21:50 -0300
perf record: Get ref_reloc_sym from kernel map
Now that ref_reloc_sym is set up when the kernel map is created,
'perf record' does not need to pass the symbol names to
perf_event__synthesize_kernel_mmap() which can read the values needed
from ref_reloc_sym directly.
Signed-off-by: Adrian Hunter adrian.hun...@intel.com
Tested-by: Jiri Olsa jo...@redhat.com
Cc: David Ahern dsah...@gmail.com
Cc: Frederic Weisbecker fweis...@gmail.com
Cc: Ingo Molnar mi...@redhat.com
Cc: Jiri Olsa jo...@redhat.com
Cc: Mike Galbraith efa...@gmx.de
Cc: Namhyung Kim namhy...@gmail.com
Cc: Paul Mackerras pau...@samba.org
Cc: Peter Zijlstra a.p.zijls...@chello.nl
Cc: Stephane Eranian eran...@google.com
Link:
http://lkml.kernel.org/r/1391004884-10334-6-git-send-email-adrian.hun...@intel.com
Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com
---
tools/perf/builtin-record.c | 10 ++
tools/perf/util/event.c | 26 ++
tools/perf/util/event.h | 3 +--
3 files changed, 9 insertions(+), 30 deletions(-)
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 3c394bf..af47531 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -287,10 +287,7 @@ static void perf_event__synthesize_guest_os(struct machine
*machine, void *data)
* have no _text sometimes.
*/
err = perf_event__synthesize_kernel_mmap(tool,
process_synthesized_event,
-machine, _text);
- if (err 0)
- err = perf_event__synthesize_kernel_mmap(tool,
process_synthesized_event,
-machine, _stext);
+machine);
if (err 0)
pr_err(Couldn't record guest kernel [%d]'s reference
relocation symbol.\n, machine-pid);
@@ -457,10 +454,7 @@ static int __cmd_record(struct record *rec, int argc,
const char **argv)
}
err = perf_event__synthesize_kernel_mmap(tool,
process_synthesized_event,
-machine, _text);
- if (err 0)
- err = perf_event__synthesize_kernel_mmap(tool,
process_synthesized_event,
-machine, _stext);
+machine);
if (err 0)
pr_err(Couldn't record kernel reference relocation symbol\n
Symbol resolution may be skewed if relocation was used
(e.g. kexec).\n
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 17476df..b0f3ca8 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -483,15 +483,13 @@ u64 kallsyms__get_function_start(const char
*kallsyms_filename,
int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
perf_event__handler_t process,
- struct machine *machine,
- const char *symbol_name)
+ struct machine *machine)
{
size_t size;
- const char *filename, *mmap_name;
- char path[PATH_MAX];
+ const char *mmap_name;
char name_buff[PATH_MAX];
struct map *map;
- u64 start;
+ struct kmap *kmap;
int err;
/*
* We should get this from /sys/kernel/sections/.text, but till that is
@@ -513,31 +511,19 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool
*tool,
* see kernel/perf_event.c __perf_event_mmap
*/
event-header.misc = PERF_RECORD_MISC_KERNEL;
- filename = /proc/kallsyms;
} else {
event-header.misc = PERF_RECORD_MISC_GUEST_KERNEL;
- if (machine__is_default_guest(machine))
- filename = (char *) symbol_conf.default_guest_kallsyms;
- else {
- sprintf(path, %s/proc/kallsyms, machine-root_dir);
- filename = path;
- }
- }
-
- start = kallsyms__get_function_start(filename, symbol_name);
- if (!start) {
- free(event);
- return -ENOENT;
}
map = machine-vmlinux_maps[MAP__FUNCTION];
+ kmap = map__kmap(map);
size = snprintf(event-mmap.filename, sizeof(event-mmap.filename),
- %s%s, mmap_name, symbol_name) + 1;
+ %s%s, mmap_name, kmap-ref_reloc_sym-name) + 1;
size = PERF_ALIGN(size, sizeof(u64));
[tip:perf/urgent] perf buildid-cache: Check relocation when checking for existing kcore
Commit-ID: d3b70220292c40d3b499797fd2f33f608fc35edb
Gitweb: http://git.kernel.org/tip/d3b70220292c40d3b499797fd2f33f608fc35edb
Author: Adrian Hunter adrian.hun...@intel.com
AuthorDate: Wed, 29 Jan 2014 16:14:44 +0200
Committer: Arnaldo Carvalho de Melo a...@redhat.com
CommitDate: Fri, 31 Jan 2014 17:21:54 -0300
perf buildid-cache: Check relocation when checking for existing kcore
perf buildid-cache does not make another copy of kcore if the buildid
and modules match an existing copy.
That does not take into account the possibility that the kernel has been
relocated.
Extend the check to check if the reference relocation symbol matches
too, otherwise do make a copy.
Signed-off-by: Adrian Hunter adrian.hun...@intel.com
Tested-by: Jiri Olsa jo...@redhat.com
Cc: David Ahern dsah...@gmail.com
Cc: Frederic Weisbecker fweis...@gmail.com
Cc: Ingo Molnar mi...@redhat.com
Cc: Jiri Olsa jo...@redhat.com
Cc: Mike Galbraith efa...@gmx.de
Cc: Namhyung Kim namhy...@gmail.com
Cc: Paul Mackerras pau...@samba.org
Cc: Peter Zijlstra a.p.zijls...@chello.nl
Cc: Stephane Eranian eran...@google.com
Link:
http://lkml.kernel.org/r/1391004884-10334-10-git-send-email-adrian.hun...@intel.com
Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com
---
tools/perf/builtin-buildid-cache.c | 33 +
1 file changed, 29 insertions(+), 4 deletions(-)
diff --git a/tools/perf/builtin-buildid-cache.c
b/tools/perf/builtin-buildid-cache.c
index cfede86..b22dbb1 100644
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -63,11 +63,35 @@ static int build_id_cache__kcore_dir(char *dir, size_t sz)
return 0;
}
+static bool same_kallsyms_reloc(const char *from_dir, char *to_dir)
+{
+ char from[PATH_MAX];
+ char to[PATH_MAX];
+ const char *name;
+ u64 addr1 = 0, addr2 = 0;
+ int i;
+
+ scnprintf(from, sizeof(from), %s/kallsyms, from_dir);
+ scnprintf(to, sizeof(to), %s/kallsyms, to_dir);
+
+ for (i = 0; (name = ref_reloc_sym_names[i]) != NULL; i++) {
+ addr1 = kallsyms__get_function_start(from, name);
+ if (addr1)
+ break;
+ }
+
+ if (name)
+ addr2 = kallsyms__get_function_start(to, name);
+
+ return addr1 == addr2;
+}
+
static int build_id_cache__kcore_existing(const char *from_dir, char *to_dir,
size_t to_dir_sz)
{
char from[PATH_MAX];
char to[PATH_MAX];
+ char to_subdir[PATH_MAX];
struct dirent *dent;
int ret = -1;
DIR *d;
@@ -86,10 +110,11 @@ static int build_id_cache__kcore_existing(const char
*from_dir, char *to_dir,
continue;
scnprintf(to, sizeof(to), %s/%s/modules, to_dir,
dent-d_name);
- if (!compare_proc_modules(from, to)) {
- scnprintf(to, sizeof(to), %s/%s, to_dir,
- dent-d_name);
- strlcpy(to_dir, to, to_dir_sz);
+ scnprintf(to_subdir, sizeof(to_subdir), %s/%s,
+ to_dir, dent-d_name);
+ if (!compare_proc_modules(from, to)
+ same_kallsyms_reloc(from_dir, to_subdir)) {
+ strlcpy(to_dir, to_subdir, to_dir_sz);
ret = 0;
break;
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[tip:perf/urgent] perf tools: Adjust kallsyms for relocated kernel
Commit-ID: d9b62aba87a82939c73f451a166c7a21342350d6
Gitweb: http://git.kernel.org/tip/d9b62aba87a82939c73f451a166c7a21342350d6
Author: Adrian Hunter adrian.hun...@intel.com
AuthorDate: Wed, 29 Jan 2014 16:14:43 +0200
Committer: Arnaldo Carvalho de Melo a...@redhat.com
CommitDate: Fri, 31 Jan 2014 17:21:53 -0300
perf tools: Adjust kallsyms for relocated kernel
If the kernel is relocated at boot time, kallsyms will not match data
recorded previously.
That does not matter for modules because they are corrected anyway. It
also does not matter if vmlinux is being used for symbols. But if perf
tools has only kallsyms then the symbols will not match.
Fix by applying the delta gained by comparing the old and current
addresses of the relocation reference symbol.
Signed-off-by: Adrian Hunter adrian.hun...@intel.com
Tested-by: Jiri Olsa jo...@redhat.com
Cc: David Ahern dsah...@gmail.com
Cc: Frederic Weisbecker fweis...@gmail.com
Cc: Ingo Molnar mi...@redhat.com
Cc: Jiri Olsa jo...@redhat.com
Cc: Mike Galbraith efa...@gmx.de
Cc: Namhyung Kim namhy...@gmail.com
Cc: Paul Mackerras pau...@samba.org
Cc: Peter Zijlstra a.p.zijls...@chello.nl
Cc: Stephane Eranian eran...@google.com
Link:
http://lkml.kernel.org/r/1391004884-10334-9-git-send-email-adrian.hun...@intel.com
Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com
---
tools/perf/util/symbol.c | 40 ++--
1 file changed, 38 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 4ac1f87..a9d758a 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -627,7 +627,7 @@ static int dso__split_kallsyms_for_kcore(struct dso *dso,
struct map *map,
* kernel range is broken in several maps, named [kernel].N, as we don't have
* the original ELF section names vmlinux have.
*/
-static int dso__split_kallsyms(struct dso *dso, struct map *map,
+static int dso__split_kallsyms(struct dso *dso, struct map *map, u64 delta,
symbol_filter_t filter)
{
struct map_groups *kmaps = map__kmap(map)-kmaps;
@@ -692,6 +692,12 @@ static int dso__split_kallsyms(struct dso *dso, struct map
*map,
char dso_name[PATH_MAX];
struct dso *ndso;
+ if (delta) {
+ /* Kernel was relocated at boot time */
+ pos-start -= delta;
+ pos-end -= delta;
+ }
+
if (count == 0) {
curr_map = map;
goto filter_symbol;
@@ -721,6 +727,10 @@ static int dso__split_kallsyms(struct dso *dso, struct map
*map,
curr_map-map_ip = curr_map-unmap_ip =
identity__map_ip;
map_groups__insert(kmaps, curr_map);
++kernel_range;
+ } else if (delta) {
+ /* Kernel was relocated at boot time */
+ pos-start -= delta;
+ pos-end -= delta;
}
filter_symbol:
if (filter filter(curr_map, pos)) {
@@ -1130,15 +1140,41 @@ out_err:
return -EINVAL;
}
+/*
+ * If the kernel is relocated at boot time, kallsyms won't match. Compute the
+ * delta based on the relocation reference symbol.
+ */
+static int kallsyms__delta(struct map *map, const char *filename, u64 *delta)
+{
+ struct kmap *kmap = map__kmap(map);
+ u64 addr;
+
+ if (!kmap-ref_reloc_sym || !kmap-ref_reloc_sym-name)
+ return 0;
+
+ addr = kallsyms__get_function_start(filename,
+ kmap-ref_reloc_sym-name);
+ if (!addr)
+ return -1;
+
+ *delta = addr - kmap-ref_reloc_sym-addr;
+ return 0;
+}
+
int dso__load_kallsyms(struct dso *dso, const char *filename,
struct map *map, symbol_filter_t filter)
{
+ u64 delta = 0;
+
if (symbol__restricted_filename(filename, /proc/kallsyms))
return -1;
if (dso__load_all_kallsyms(dso, filename, map) 0)
return -1;
+ if (kallsyms__delta(map, filename, delta))
+ return -1;
+
symbols__fixup_duplicate(dso-symbols[map-type]);
symbols__fixup_end(dso-symbols[map-type]);
@@ -1150,7 +1186,7 @@ int dso__load_kallsyms(struct dso *dso, const char
*filename,
if (!dso__load_kcore(dso, map, filename))
return dso__split_kallsyms_for_kcore(dso, map, filter);
else
- return dso__split_kallsyms(dso, map, filter);
+ return dso__split_kallsyms(dso, map, delta, filter);
}
static int dso__load_perf_map(struct dso *dso, struct map *map,
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info
[tip:perf/urgent] perf tools: Fix AAAAARGH64 memory barriers
Commit-ID: f428ebd184c82a7914b2aa7e9f868918aaf7ea78 Gitweb: http://git.kernel.org/tip/f428ebd184c82a7914b2aa7e9f868918aaf7ea78 Author: Peter Zijlstra pet...@infradead.org AuthorDate: Fri, 24 Jan 2014 16:40:02 +0100 Committer: Arnaldo Carvalho de Melo a...@redhat.com CommitDate: Wed, 29 Jan 2014 15:50:57 -0300 perf tools: Fix ARGH64 memory barriers Someone got the load and store barriers mixed up for ARGH64. Turn them the right side up. Reported-by: Will Deacon will.dea...@arm.com Signed-off-by: Peter Zijlstra pet...@infradead.org Fixes: a94d342b9cb0 (tools/perf: Add required memory barriers) Cc: Ingo Molnar mi...@kernel.org Cc: Will Deacon will.dea...@arm.com Link: http://lkml.kernel.org/r/20140124154002.gf31...@twins.programming.kicks-ass.net Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com --- tools/perf/perf.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/perf/perf.h b/tools/perf/perf.h index 7daa806..e84fa26 100644 --- a/tools/perf/perf.h +++ b/tools/perf/perf.h @@ -100,8 +100,8 @@ #ifdef __aarch64__ #define mb() asm volatile(dmb ish ::: memory) -#define wmb() asm volatile(dmb ishld ::: memory) -#define rmb() asm volatile(dmb ishst ::: memory) +#define wmb() asm volatile(dmb ishst ::: memory) +#define rmb() asm volatile(dmb ishld ::: memory) #define cpu_relax()asm volatile(yield ::: memory) #endif -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[tip:perf/urgent] perf symbols: Fix symbol annotation for relocated kernel
Commit-ID: 9176753d1ed56951a6ee2a0f0a3f367904e35567
Gitweb: http://git.kernel.org/tip/9176753d1ed56951a6ee2a0f0a3f367904e35567
Author: Adrian Hunter adrian.hun...@intel.com
AuthorDate: Wed, 29 Jan 2014 16:14:36 +0200
Committer: Arnaldo Carvalho de Melo a...@redhat.com
CommitDate: Fri, 31 Jan 2014 17:21:47 -0300
perf symbols: Fix symbol annotation for relocated kernel
Kernel maps map memory addresses to file offsets.
For symbol annotation, objdump needs the object VMA addresses. For an
unrelocated kernel, that is the same as the memory address.
The addresses passed to objdump for symbol annotation did not take into
account kernel relocation.
This patch fixes that.
Reported-by: Linus Torvalds torva...@linux-foundation.org
Signed-off-by: Adrian Hunter adrian.hun...@intel.com
Tested-by: Jiri Olsa jo...@redhat.com
Cc: David Ahern dsah...@gmail.com
Cc: Frederic Weisbecker fweis...@gmail.com
Cc: Ingo Molnar mi...@redhat.com
Cc: Jiri Olsa jo...@redhat.com
Cc: Mike Galbraith efa...@gmx.de
Cc: Namhyung Kim namhy...@gmail.com
Cc: Paul Mackerras pau...@samba.org
Cc: Peter Zijlstra a.p.zijls...@chello.nl
Cc: Stephane Eranian eran...@google.com
Link:
http://lkml.kernel.org/r/1391004884-10334-2-git-send-email-adrian.hun...@intel.com
Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com
---
tools/perf/util/map.c| 5 +++--
tools/perf/util/map.h| 1 +
tools/perf/util/symbol-elf.c | 2 ++
3 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 3b97513..39cd2d0 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -39,6 +39,7 @@ void map__init(struct map *map, enum map_type type,
map-start= start;
map-end = end;
map-pgoff= pgoff;
+ map-reloc= 0;
map-dso = dso;
map-map_ip = map__map_ip;
map-unmap_ip = map__unmap_ip;
@@ -288,7 +289,7 @@ u64 map__rip_2objdump(struct map *map, u64 rip)
if (map-dso-rel)
return rip - map-pgoff;
- return map-unmap_ip(map, rip);
+ return map-unmap_ip(map, rip) - map-reloc;
}
/**
@@ -311,7 +312,7 @@ u64 map__objdump_2mem(struct map *map, u64 ip)
if (map-dso-rel)
return map-unmap_ip(map, ip + map-pgoff);
- return ip;
+ return ip + map-reloc;
}
void map_groups__init(struct map_groups *mg)
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 18068c6..257e513 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -36,6 +36,7 @@ struct map {
boolerange_warned;
u32 priv;
u64 pgoff;
+ u64 reloc;
u32 maj, min; /* only valid for MMAP2 record */
u64 ino; /* only valid for MMAP2 record */
u64 ino_generation;/* only valid for MMAP2 record */
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 8f12f0f..3e9f336 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -751,6 +751,8 @@ int dso__load_sym(struct dso *dso, struct map *map,
if (strcmp(elf_name, kmap-ref_reloc_sym-name))
continue;
kmap-ref_reloc_sym-unrelocated_addr = sym.st_value;
+ map-reloc = kmap-ref_reloc_sym-addr -
+kmap-ref_reloc_sym-unrelocated_addr;
break;
}
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[tip:perf/urgent] perf tools: Fix include for non x86 architectures
Commit-ID: 6a02652df511029127406cf8fa89cdf5e987f963 Gitweb: http://git.kernel.org/tip/6a02652df511029127406cf8fa89cdf5e987f963 Author: Francesco Fusco ffu...@redhat.com AuthorDate: Mon, 27 Jan 2014 14:39:13 +0100 Committer: Arnaldo Carvalho de Melo a...@redhat.com CommitDate: Fri, 31 Jan 2014 17:21:42 -0300 perf tools: Fix include for non x86 architectures Commit 71ae8aac (lib: introduce arch optimized hash library) added an include to linux/hash.h for setting up an architecture specific fast hash. Since perf includes directly the non-uapi kernel header, it cannot find asm/hash.h on non-x86 and thus prevents perf to be compiled on every architecture other than x86. The problem is the inclusion of asm/hash.h in hash.h that results in the following error originating from util/evlist.c: fatal error: asm/hash.h: No such file or directory This commit simply adds an empty asm/hash.h stub/file to fix the compile issue on non-x86 architectures. As perf does not use any of these new functions, it fixes the compilation and therefore seems to be the most appropriate solution to go with. Signed-off-by: Francesco Fusco ffu...@redhat.com Link: http://lkml.kernel.org/r/2cf8143aad65a6aa6fe30325ef8a65847141afa2.1390829373.git.ffu...@redhat.com Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com --- tools/perf/util/include/asm/hash.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/tools/perf/util/include/asm/hash.h b/tools/perf/util/include/asm/hash.h new file mode 100644 index 000..d82b170b --- /dev/null +++ b/tools/perf/util/include/asm/hash.h @@ -0,0 +1,6 @@ +#ifndef __ASM_GENERIC_HASH_H +#define __ASM_GENERIC_HASH_H + +/* Stub */ + +#endif /* __ASM_GENERIC_HASH_H */ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v11 0/4] Introducing a queue read/write lock implementation
* Waiman Long waiman.l...@hp.com wrote: How about making the selection of MCS or ticket queuing either user configurable or depending on the setting of NR_CPUS, NUMA, etc? No! There are lots of disadvantages to adding such CONFIG_NUMA Kconfig variants for locking primitives: - an doubling of the test matrix - an doubling of the review matrix and a halving of effective review capacity: we've just about go the capacity to review and validate patches like this. Splitting out a 'only NUMA cares' variant is a non-starter really. - but most importantly, there's absolutely no reason to not be fast on 128 CPU systems in the low contended case either! Sacrificing the low contended case with 'on 128 CPU systems it is the contended path that matters' is an idiotic argument. Essentially the only area were we allow Kconfig dependencies are unyielding physical forces: such as lots of CPUs needing a wider CPU mask. As Peter said it, the right solution is to fix the contended case. If that also happens to speed up or better organize the uncondended code then that's good, but it should not make it worse. Thanks, Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Make math_state_restore() save and restore the interrupt flag
On Sat, 1 Feb 2014, Linus Torvalds wrote: We could do that with the whole task_work thing (or perhaps just do_notify_resume(), especially after merging the don't necessarily return with iret patch I sent out earlier), with additionally making sure that scheduling does the right thing wrt a currently dirty math state due to kernel use. The advantage of that would be that we really could do a *lot* of FP math very cheaply in the kernel, because we'd pay the overhead of kernel_fpu_begin/end() just once (well, the end part would be just setting the bit that we now have dirty state, the cost would be in the return-to-user-space-and-restore-fp-state part). Comments? That would be much more invasive than just changing __kernel_fpu_end(), but would bring in possibly quite noticeable advantages under loads that use the FP/vector resources in the kernel. This would be very good and it needs to work in interrupt context (softirq) also, and when we interrupt idle task. It's with networking we can really hit kernel_fpu_begin()/end() millions of times per second and there's really only need to do it once per interrupt. This is actually similar what I was doing (in do_softirq)) when I noticed eagerfpu was broken and now Nate's bug AFAICS happens there as well. Pekka -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] SLAB changes for v3.14-rc1
Hi Linus, Please pull the latest SLAB tree from: git://git.kernel.org/pub/scm/linux/kernel/git/penberg/linux.git slab/next It contains random bug fixes that have accumulated in my inbox over the past few months. Pekka -- The following changes since commit 6ce4eac1f600b34f2f7f58f9cd8f0503d79e42ae: Linux 3.13-rc1 (2013-11-22 11:30:55 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/penberg/linux.git slab/next for you to fetch changes up to cb8ee1a3d429f8898972c869dd4792afb04e961a: mm: Fix warning on make htmldocs caused by slab.c (2014-01-31 13:52:25 +0200) Dave Hansen (2): mm: sl[uo]b: fix misleading comments mm: slub: work around unneeded lockdep warning Li Zefan (1): slub: Fix calculation of cpu slabs Masanari Iida (1): mm: Fix warning on make htmldocs caused by slab.c Peter Zijlstra (1): slub: use lockdep_assert_held Randy Dunlap (1): slab.h: remove duplicate kmalloc declaration and fix kernel-doc warnings Tetsuo Handa (1): slub: Fix possible format string bug. include/linux/slab.h | 110 +++ mm/slab.c| 2 +- mm/slub.c| 56 +++--- 3 files changed, 85 insertions(+), 83 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
..another device for the ftdi_sio driver, 2nd infusion
Thanks, Greg to your bot for the hint,
gmail seems to screw up tabs for spaces. Good to know, learning by doing..
Lets give it another try:
finally I prepared a patch for the famous ftdi_sio usb serial driver.
Adding two Tagsys RFID readers I am using.
After having asked Bill Ryder he suggested adressing the
linux-usb-devel mailing list. (what?)
Sorry for the mess below - I am not doing this frequently - being
rather a leecher over the last 20 years:-)
Signed-off-by: Ulrich Hahn uh...@eanco.de
diff -uprN linux-source-3.2.0/drivers/usb/serial/ftdi_sio.c
linux-source-3.2.0+tagsysRFID/drivers/usb/serial/ftdi_sio.c
--- linux-source-3.2.0/drivers/usb/serial/ftdi_sio.c2013-12-03
18:40:26.0 +0100
+++ linux-source-3.2.0+tagsysRFID/drivers/usb/serial/ftdi_sio.c 2014-02-01
16:32:40.146698791 +0100
@@ -202,6 +202,8 @@ static struct usb_device_id id_table_com
{ USB_DEVICE(INTERBIOMETRICS_VID, INTERBIOMETRICS_IOBOARD_PID) },
{ USB_DEVICE(INTERBIOMETRICS_VID, INTERBIOMETRICS_MINI_IOBOARD_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_SPROG_II) },
+ { USB_DEVICE(FTDI_VID, FTDI_TAGSYS_LP101_PID) },
+ { USB_DEVICE(FTDI_VID, FTDI_TAGSYS_P200X_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_LENZ_LIUSB_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_XF_632_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_XF_634_PID) },
diff -uprN linux-source-3.2.0/drivers/usb/serial/ftdi_sio_ids.h
linux-source-3.2.0+tagsysRFID/drivers/usb/serial/ftdi_sio_ids.h
--- linux-source-3.2.0/drivers/usb/serial/ftdi_sio_ids.h2013-12-03
18:40:26.0 +0100
+++ linux-source-3.2.0+tagsysRFID/drivers/usb/serial/ftdi_sio_ids.h
2014-02-02 09:11:35.183155244 +0100
@@ -363,6 +363,13 @@
/* Sprog II (Andrew Crosland's SprogII DCC interface) */
#define FTDI_SPROG_II 0xF0C8
+/*
+ * Tagsys RFID Readers
+ * survey (two of them) by Ulrich Hahn
+ */
+#define FTDI_TAGSYS_LP101_PID 0xF0E9 /* Tagsys L-P101 RFID*/
+#define FTDI_TAGSYS_P200X_PID 0xF0EE /* Tagsys Medio P200x RFID*/
+
/* an infrared receiver for user access control with IR tags */
#define FTDI_PIEGROUP_PID 0xF208 /* Product Id */
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: ..another device for the ftdi_sio driver, 2nd infusion
On Sun, Feb 02, 2014 at 10:22:16AM +0100, Ulrich Hahn wrote: Thanks, Greg to your bot for the hint, gmail seems to screw up tabs for spaces. Good to know, learning by doing.. Lets give it another try: finally I prepared a patch for the famous ftdi_sio usb serial driver. Adding two Tagsys RFID readers I am using. After having asked Bill Ryder he suggested adressing the linux-usb-devel mailing list. (what?) Sorry for the mess below - I am not doing this frequently - being rather a leecher over the last 20 years:-) Signed-off-by: Ulrich Hahn uh...@eanco.de That's better, but I have to edit the above by hand, which isn't a big deal, but when you deal with tens of thousands of patches a year, doesn't scale :( diff -uprN linux-source-3.2.0/drivers/usb/serial/ftdi_sio.c linux-source-3.2.0+tagsysRFID/drivers/usb/serial/ftdi_sio.c --- linux-source-3.2.0/drivers/usb/serial/ftdi_sio.c 2013-12-03 18:40:26.0 +0100 +++ linux-source-3.2.0+tagsysRFID/drivers/usb/serial/ftdi_sio.c 2014-02-01 16:32:40.146698791 +0100 3.2.0 is over 2 years old, things have moved on a bit since then :) Can you redo this patch against the 3.13.0 kernel release, to have a chance for it to be able to be applied? thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 13/16] drm/nouveau/ibus: add GK20A support
On Sun, Feb 2, 2014 at 3:35 PM, Ilia Mirkin imir...@alum.mit.edu wrote:
Some very trivial comments below:
On Fri, Jan 31, 2014 at 10:16 PM, Alexandre Courbot acour...@nvidia.com
wrote:
Add support for initializing the priv ring of GK20A. This is done by the
BIOS on desktop GPUs, but needs to be done by hand on Tegra.
Signed-off-by: Alexandre Courbot acour...@nvidia.com
---
drivers/gpu/drm/nouveau/Makefile | 1 +
drivers/gpu/drm/nouveau/core/include/subdev/ibus.h | 1 +
drivers/gpu/drm/nouveau/core/subdev/ibus/nvea.c| 108
+
3 files changed, 110 insertions(+)
create mode 100644 drivers/gpu/drm/nouveau/core/subdev/ibus/nvea.c
diff --git a/drivers/gpu/drm/nouveau/Makefile
b/drivers/gpu/drm/nouveau/Makefile
index 6c4b76d..3548fcd 100644
--- a/drivers/gpu/drm/nouveau/Makefile
+++ b/drivers/gpu/drm/nouveau/Makefile
@@ -132,6 +132,7 @@ nouveau-y += core/subdev/i2c/nv94.o
nouveau-y += core/subdev/i2c/nvd0.o
nouveau-y += core/subdev/ibus/nvc0.o
nouveau-y += core/subdev/ibus/nve0.o
+nouveau-y += core/subdev/ibus/nvea.o
nouveau-y += core/subdev/instmem/base.o
nouveau-y += core/subdev/instmem/nv04.o
nouveau-y += core/subdev/instmem/nv40.o
diff --git a/drivers/gpu/drm/nouveau/core/include/subdev/ibus.h
b/drivers/gpu/drm/nouveau/core/include/subdev/ibus.h
index 88814f1..056a42f 100644
--- a/drivers/gpu/drm/nouveau/core/include/subdev/ibus.h
+++ b/drivers/gpu/drm/nouveau/core/include/subdev/ibus.h
@@ -30,5 +30,6 @@ nouveau_ibus(void *obj)
extern struct nouveau_oclass nvc0_ibus_oclass;
extern struct nouveau_oclass nve0_ibus_oclass;
+extern struct nouveau_oclass nvea_ibus_oclass;
#endif
diff --git a/drivers/gpu/drm/nouveau/core/subdev/ibus/nvea.c
b/drivers/gpu/drm/nouveau/core/subdev/ibus/nvea.c
new file mode 100644
index 000..0bcd281
--- /dev/null
+++ b/drivers/gpu/drm/nouveau/core/subdev/ibus/nvea.c
@@ -0,0 +1,108 @@
+/*
+ * Copyright (c) 2014, NVIDIA Corporation. All rights reserved.
+ *
+ * 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.
+ *
+ */
+
+#include subdev/ibus.h
+
+struct nvea_ibus_priv {
+ struct nouveau_ibus base;
+};
+
+static void
+nvea_ibus_init_priv_ring(struct nvea_ibus_priv *priv)
+{
+ u32 data;
+
+ data = nv_rd32(priv, 0x137250);
+ data = (~0x3f);
+ nv_wr32(priv, 0x137250, data);
nv_mask(priv, 0x137250, 0x3f, 0) should do this, right?
+
+ nv_mask(priv, 0x000200, 0x20, 0);
+ udelay(20);
+ nv_mask(priv, 0x000200, 0x20, 0x20);
+
+ nv_wr32(priv, 0x12004c, 0x4);
+ nv_wr32(priv, 0x122204, 0x2);
+ nv_rd32(priv, 0x122204);
+}
+
+static void
+nvea_ibus_intr(struct nouveau_subdev *subdev)
+{
+ struct nvea_ibus_priv *priv = (void *)subdev;
+ u32 status0 = nv_rd32(priv, 0x120058);
+ s32 retry = 100;
+ u32 command;
+
+ if (status0 0x7) {
+ nv_debug(priv, resetting priv ring\n);
+ nvea_ibus_init_priv_ring(priv);
+ }
+
+ /* Acknowledge interrupt */
+ command = nv_rd32(priv, 0x0012004c);
+ command |= 0x2;
+ nv_wr32(priv, 0x0012004c, command);
nv_mask(priv, 0x12004c, 0x2, 0x2)
Absolutely correct for both.
+
+ while (--retry = 0) {
+ command = nv_rd32(priv, 0x12004c) 0x3f;
+ if (command == 0)
+ break;
+ }
+
+ if (retry 0)
+ nv_debug(priv, timeout waiting for ringmaster ack\n);
this sounds kinda bad, no? perhaps a nv_warn?
Sounds more adequate indeed.
Thanks,
Alex.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: mlx4: Use pci_enable_msix_range()
On 31/01/14 16:08 +0100, Alexander Gordeev wrote: As result of deprecation of MSI-X/MSI enablement functions pci_enable_msix() and pci_enable_msi_block() all drivers using these two interfaces need to be updated to use the new pci_enable_msi_range() and pci_enable_msix_range() interfaces. Signed-off-by: Alexander Gordeev agord...@redhat.com --- drivers/net/ethernet/mellanox/mlx4/main.c | 19 --- 1 files changed, 4 insertions(+), 15 deletions(-) Acked By: Amir Vadai am...@mellanox.com -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] Kconfig: organize memory-related config options
+config MMAP_ALLOW_UNINITIALIZED + bool Allow mmapped anonymous memory to be uninitialized + depends on EXPERT !MMU + default n + help + Normally, and according to the Linux spec, anonymous memory obtained + from mmap() has it's contents cleared before it is passed to its, please. If you really want to make me happy, clarify the CONFIG_SLOB help to explain what a large system is. More than 4 CPUs? More tha 32 GB of RAM? E-ATX motherboard? -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [PATCH v6] xen/grant-table: Avoid m2p_override during mapping
On 23/01/14 23:34, Stefano Stabellini wrote:
On Thu, 23 Jan 2014, Zoltan Kiss wrote:
The grant mapping API does m2p_override unnecessarily: only gntdev needs it,
for blkback and future netback patches it just cause a lock contention, as
those pages never go to userspace. Therefore this series does the following:
- the original functions were renamed to __gnttab_[un]map_refs, with a new
parameter m2p_override
- based on m2p_override either they follow the original behaviour, or just
set
the private flag and call set_phys_to_machine
- gnttab_[un]map_refs are now a wrapper to call __gnttab_[un]map_refs with
m2p_override false
- a new function gnttab_[un]map_refs_userspace provides the old behaviour
It also removes a stray space from page.h and change ret to 0 if
XENFEAT_auto_translated_physmap, as that is the only possible return value
there.
v2:
- move the storing of the old mfn in page-index to gnttab_map_refs
- move the function header update to a separate patch
v3:
- a new approach to retain old behaviour where it needed
- squash the patches into one
v4:
- move out the common bits from m2p* functions, and pass pfn/mfn as parameter
- clear page-private before doing anything with the page, so
m2p_find_override
won't race with this
v5:
- change return value handling in __gnttab_[un]map_refs
- remove a stray space in page.h
- add detail why ret = 0 now at some places
v6:
- don't pass pfn to m2p* functions, just get it locally
Signed-off-by: Zoltan Kiss zoltan.k...@citrix.com
Suggested-by: David Vrabel david.vra...@citrix.com
Reviewed-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
Hello,
This patch is breaking Linux compilation on ARM:
drivers/xen/grant-table.c: In function ‘__gnttab_map_refs’:
drivers/xen/grant-table.c:989:3: error: implicit declaration of function
‘FOREIGN_FRAME’ [-Werror=implicit-function-declaration]
if (unlikely(!set_phys_to_machine(pfn, FOREIGN_FRAME(mfn {
^
drivers/xen/grant-table.c: In function ‘__gnttab_unmap_refs’:
drivers/xen/grant-table.c:1054:3: error: implicit declaration of function
‘get_phys_to_machine’ [-Werror=implicit-function-declaration]
mfn = get_phys_to_machine(pfn);
^
drivers/xen/grant-table.c:1055:43: error: ‘FOREIGN_FRAME_BIT’ undeclared (first
use in this function)
if (mfn == INVALID_P2M_ENTRY || !(mfn FOREIGN_FRAME_BIT)) {
^
drivers/xen/grant-table.c:1055:43: note: each undeclared identifier is reported
only once for each function it appears in
drivers/xen/grant-table.c:1068:9: error: too many arguments to function
‘m2p_remove_override’
mfn);
^
In file included from include/xen/page.h:4:0,
from drivers/xen/grant-table.c:48:
/local/home/julien/works/midway/linux/arch/arm/include/asm/xen/page.h:106:19:
note: declared here
static inline int m2p_remove_override(struct page *page, bool clear_pte)
^
cc1: some warnings being treated as errors
arch/x86/include/asm/xen/page.h |5 +-
arch/x86/xen/p2m.c | 17 +--
drivers/block/xen-blkback/blkback.c | 15 +++---
drivers/xen/gntdev.c| 13 +++--
drivers/xen/grant-table.c | 89
++-
include/xen/grant_table.h |8 +++-
6 files changed, 101 insertions(+), 46 deletions(-)
diff --git a/arch/x86/include/asm/xen/page.h
b/arch/x86/include/asm/xen/page.h
index b913915..ce47243 100644
--- a/arch/x86/include/asm/xen/page.h
+++ b/arch/x86/include/asm/xen/page.h
@@ -52,7 +52,8 @@ extern unsigned long set_phys_range_identity(unsigned long
pfn_s,
extern int m2p_add_override(unsigned long mfn, struct page *page,
struct gnttab_map_grant_ref *kmap_op);
extern int m2p_remove_override(struct page *page,
-struct gnttab_map_grant_ref *kmap_op);
+ struct gnttab_map_grant_ref *kmap_op,
+ unsigned long mfn);
extern struct page *m2p_find_override(unsigned long mfn);
extern unsigned long m2p_find_override_pfn(unsigned long mfn, unsigned
long pfn);
@@ -121,7 +122,7 @@ static inline unsigned long mfn_to_pfn(unsigned long mfn)
pfn = m2p_find_override_pfn(mfn, ~0);
}
-/*
+/*
* pfn is ~0 if there are no entries in the m2p for mfn or if the
* entry doesn't map back to the mfn and m2p_override doesn't have a
* valid entry for it.
diff --git a/arch/x86/xen/p2m.c b/arch/x86/xen/p2m.c
index 2ae8699..bd4724b 100644
--- a/arch/x86/xen/p2m.c
+++ b/arch/x86/xen/p2m.c
@@ -888,13 +888,6 @@ int m2p_add_override(unsigned long mfn, struct page
*page,
m2p_add_override: pfn %lx not mapped,
pfn))
return -EINVAL;
}
-WARN_ON(PagePrivate(page));
-SetPagePrivate(page);
-
HID bluetooth regression
Commit b1a1442a2 (HID: core: fix reporting of raw events) introduced a regression that causes lockups for bluetooth trackpads. This was already discussed in this thread in September: http://www.kernelhub.org/?p=2msg=329823 but it seems nothing has been fixed so far, I still get the lockups in 3.13. Reverting that patch fixes it for me, but it would be great if someone could push a proper fix upstream. If you need me to test anything, please let me know. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
..another device for the ftdi_sio driver, 3rd infusion
Signed-off-by: Ulrich Hahn uh...@eanco.de
diff -ur linux-3.13.1/drivers/usb/serial/ftdi_sio.c
linux-3.13.1+tagsys/drivers/usb/serial/ftdi_sio.c
--- linux-3.13.1/drivers/usb/serial/ftdi_sio.c 2014-01-29 14:06:37.0
+0100
+++ linux-3.13.1+tagsys/drivers/usb/serial/ftdi_sio.c 2014-02-02
11:57:08.625914749 +0100
@@ -192,6 +192,8 @@
{ USB_DEVICE(INTERBIOMETRICS_VID, INTERBIOMETRICS_IOBOARD_PID) },
{ USB_DEVICE(INTERBIOMETRICS_VID, INTERBIOMETRICS_MINI_IOBOARD_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_SPROG_II) },
+ { USB_DEVICE(FTDI_VID, FTDI_TAGSYS_LP101_PID) },
+ { USB_DEVICE(FTDI_VID, FTDI_TAGSYS_P200X_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_LENZ_LIUSB_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_XF_632_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_XF_634_PID) },
Only in linux-3.13.1+tagsys/drivers/usb/serial: ftdi_sio.c.orig
diff -ur linux-3.13.1/drivers/usb/serial/ftdi_sio_ids.h
linux-3.13.1+tagsys/drivers/usb/serial/ftdi_sio_ids.h
--- linux-3.13.1/drivers/usb/serial/ftdi_sio_ids.h 2014-01-29
14:06:37.0 +0100
+++ linux-3.13.1+tagsys/drivers/usb/serial/ftdi_sio_ids.h 2014-02-02
11:57:08.629914782 +0100
@@ -363,6 +363,13 @@
/* Sprog II (Andrew Crosland's SprogII DCC interface) */
#define FTDI_SPROG_II 0xF0C8
+/*
+ * Tagsys RFID Readers
+ * survey (two of them) by Ulrich Hahn
+ */
+#define FTDI_TAGSYS_LP101_PID 0xF0E9 /* Tagsys L-P101 RFID*/
+#define FTDI_TAGSYS_P200X_PID 0xF0EE /* Tagsys Medio P200x RFID*/
+
/* an infrared receiver for user access control with IR tags */
#define FTDI_PIEGROUP_PID 0xF208 /* Product Id */
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] parisc updates for v3.14
* Richard Weinberger richard.weinber...@gmail.com: On Sat, Feb 1, 2014 at 9:23 PM, Helge Deller del...@gmx.de wrote: please pull the latest updates for the parisc architecture from: git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux.git parisc-for-3.14 The three major changes in this patchset is a implementation for flexible userspace memory maps, The change to fs/exec.c only touches code which affects parisc since it's inside a #ifdef CONFIG_STACK_GROWSUP section (and parisc is the only platform where the stack grows upwards). What about metag? Great! I didn't know that we now have another platform where the stack grows upwards! Now we are not alone in this regard! Anyway, the suggested untested patch below should fix the metag arch to cope which my changes to fs/exec.c (unless the metag people want to implement proper stack randomization too). Either I can push it via the parisc tree, or you can take it via the metag tree. Please let me know. Helge Deller (5): parisc: add flexible mmap memory layout support ... fs/exec.c | 3 + (see: http://git.kernel.org/cgit/linux/kernel/git/deller/parisc-linux.git/commit/?h=for-nextid=13de8ec38997357c9865a36a587439d9c5678932 ) [PATCH] metag: define STACK_RND_MASK as -1 Signed-off-by: Helge Deller del...@gmx.de diff --git a/arch/metag/include/asm/elf.h b/arch/metag/include/asm/elf.h index d2baf69..089e37a 100644 --- a/arch/metag/include/asm/elf.h +++ b/arch/metag/include/asm/elf.h @@ -100,7 +100,7 @@ typedef unsigned long elf_fpregset_t; #define ELF_PLATFORM (NULL) -#define STACK_RND_MASK (0) +#define STACK_RND_MASK (-1) #ifdef CONFIG_METAG_USER_TCM -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] Add 32 bit VDSO time support for 32 bit kernel
-#define VDSO_HIGH_BASE 0xe000U /* CONFIG_COMPAT_VDSO address */ +#define VDSO_HIGH_BASE 0xc000U /* CONFIG_COMPAT_VDSO address */ This is odd. Can you explain it? He needs 3 pages instead of 1 after his changes. Not every kernel hackers a male. By definition i am femal, so please replace he and his with she and her. - Stefani -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 6/8] cleanup __vdso_gettimeofday
From: Stefani Seibold stef...@seibold.net
This patch do a little cleanup for the __vdso_gettimeofday() function.
It kick out an unneeded ret local variable and makes the code faster if
only the timezone is needed.
Signed-off-by: Stefani Seibold stef...@seibold.net
---
arch/x86/vdso/vclock_gettime.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/x86/vdso/vclock_gettime.c b/arch/x86/vdso/vclock_gettime.c
index 743f277..bf969a0 100644
--- a/arch/x86/vdso/vclock_gettime.c
+++ b/arch/x86/vdso/vclock_gettime.c
@@ -259,13 +259,12 @@ int clock_gettime(clockid_t, struct timespec *)
notrace int __vdso_gettimeofday(struct timeval *tv, struct timezone *tz)
{
- long ret = VCLOCK_NONE;
-
if (likely(tv != NULL)) {
BUILD_BUG_ON(offsetof(struct timeval, tv_usec) !=
offsetof(struct timespec, tv_nsec) ||
sizeof(*tv) != sizeof(struct timespec));
- ret = do_realtime((struct timespec *)tv);
+ if (do_realtime((struct timespec *)tv) == VCLOCK_NONE)
+ return vdso_fallback_gtod(tv, tz);
tv-tv_usec /= 1000;
}
if (unlikely(tz != NULL)) {
@@ -274,8 +273,6 @@ notrace int __vdso_gettimeofday(struct timeval *tv, struct
timezone *tz)
tz-tz_dsttime = gtod-sys_tz.tz_dsttime;
}
- if (ret == VCLOCK_NONE)
- return vdso_fallback_gtod(tv, tz);
return 0;
}
int gettimeofday(struct timeval *, struct timezone *)
--
1.8.5.3
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 7/8] Add 32 bit VDSO time support for 32 bit kernel
From: Stefani Seibold stef...@seibold.net
This patch add the time support for 32 bit a VDSO to a 32 bit kernel.
For 32 bit programs running on a 32 bit kernel, the same mechanism is
used as for 64 bit programs running on a 64 bit kernel.
Signed-off-by: Stefani Seibold stef...@seibold.net
---
arch/x86/include/asm/elf.h| 2 +-
arch/x86/include/asm/vdso.h | 3 ++
arch/x86/include/asm/vdso32.h | 10 ++
arch/x86/vdso/Makefile| 7 +
arch/x86/vdso/vclock_gettime.c| 59 +--
arch/x86/vdso/vdso-layout.lds.S | 25 +++
arch/x86/vdso/vdso32-setup.c | 47 ++--
arch/x86/vdso/vdso32/vclock_gettime.c | 16 ++
arch/x86/vdso/vdso32/vdso32.lds.S | 11 ++-
9 files changed, 167 insertions(+), 13 deletions(-)
create mode 100644 arch/x86/include/asm/vdso32.h
create mode 100644 arch/x86/vdso/vdso32/vclock_gettime.c
diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
index 9c999c1..21bae90 100644
--- a/arch/x86/include/asm/elf.h
+++ b/arch/x86/include/asm/elf.h
@@ -289,7 +289,7 @@ do {
\
#else /* CONFIG_X86_32 */
-#define VDSO_HIGH_BASE 0xe000U /* CONFIG_COMPAT_VDSO address */
+#define VDSO_HIGH_BASE 0xc000U /* CONFIG_COMPAT_VDSO address */
/* 1GB for 64bit, 8MB for 32bit */
#define STACK_RND_MASK (test_thread_flag(TIF_ADDR32) ? 0x7ff : 0x3f)
diff --git a/arch/x86/include/asm/vdso.h b/arch/x86/include/asm/vdso.h
index fddb53d..fe3cef9 100644
--- a/arch/x86/include/asm/vdso.h
+++ b/arch/x86/include/asm/vdso.h
@@ -2,6 +2,9 @@
#define _ASM_X86_VDSO_H
#if defined CONFIG_X86_32 || defined CONFIG_COMPAT
+
+#include asm/vdso32.h
+
extern const char VDSO32_PRELINK[];
/*
diff --git a/arch/x86/include/asm/vdso32.h b/arch/x86/include/asm/vdso32.h
new file mode 100644
index 000..7dd2eb8
--- /dev/null
+++ b/arch/x86/include/asm/vdso32.h
@@ -0,0 +1,10 @@
+#ifndef _ASM_X86_VDSO32_H
+#define _ASM_X86_VDSO32_H
+
+#define VDSO_BASE_PAGE 0
+#define VDSO_VVAR_PAGE 1
+#define VDSO_HPET_PAGE 2
+#defineVDSO_PAGES 3
+#defineVDSO_OFFSET(x) ((x) * PAGE_SIZE)
+
+#endif
diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile
index fd14be1..1ff5b0a 100644
--- a/arch/x86/vdso/Makefile
+++ b/arch/x86/vdso/Makefile
@@ -145,8 +145,15 @@ KBUILD_AFLAGS_32 := $(filter-out -m64,$(KBUILD_AFLAGS))
$(vdso32-images:%=$(obj)/%.dbg): KBUILD_AFLAGS = $(KBUILD_AFLAGS_32)
$(vdso32-images:%=$(obj)/%.dbg): asflags-$(CONFIG_X86_64) += -m32
+KBUILD_CFLAGS_32 := $(filter-out -m64,$(KBUILD_CFLAGS))
+KBUILD_CFLAGS_32 := $(filter-out -mcmodel=kernel,$(KBUILD_CFLAGS_32))
+KBUILD_CFLAGS_32 := $(filter-out -fno-pic,$(KBUILD_CFLAGS_32))
+KBUILD_CFLAGS_32 += -m32 -msoft-float -mregparm=3 -freg-struct-return -fpic
+$(vdso32-images:%=$(obj)/%.dbg): KBUILD_CFLAGS = $(KBUILD_CFLAGS_32)
+
$(vdso32-images:%=$(obj)/%.dbg): $(obj)/vdso32-%.so.dbg: FORCE \
$(obj)/vdso32/vdso32.lds \
+$(obj)/vdso32/vclock_gettime.o \
$(obj)/vdso32/note.o \
$(obj)/vdso32/%.o
$(call if_changed,vdso)
diff --git a/arch/x86/vdso/vclock_gettime.c b/arch/x86/vdso/vclock_gettime.c
index bf969a0..42f641c 100644
--- a/arch/x86/vdso/vclock_gettime.c
+++ b/arch/x86/vdso/vclock_gettime.c
@@ -4,6 +4,9 @@
*
* Fast user context implementation of clock_gettime, gettimeofday, and time.
*
+ * 32 Bit compat layer by Stefani Seibold stef...@seibold.net
+ * sponsored by Rohde Schwarz GmbH Co. KG Munich/Germany
+ *
* The code should have no internal unresolved relocations.
* Check with readelf after changing.
*/
@@ -24,6 +27,8 @@
#include asm/io.h
#include asm/pvclock.h
+#ifndef BUILD_VDSO32
+
#define gtod (VVAR(vsyscall_gtod_data))
static notrace cycle_t vread_hpet(void)
@@ -47,6 +52,54 @@ notrace static long vdso_fallback_gtod(struct timeval *tv,
struct timezone *tz)
0 (__NR_gettimeofday), D (tv), S (tz) : memory);
return ret;
}
+#else
+
+struct vsyscall_gtod_data vvar_vsyscall_gtod_data
+ __attribute__((visibility(hidden)));
+
+u8 hpet_page
+ __attribute__((visibility(hidden)));
+
+#define gtod (vvar_vsyscall_gtod_data)
+
+#ifdef CONFIG_HPET_TIMER
+static notrace cycle_t vread_hpet(void)
+{
+ return readl(hpet_page + HPET_COUNTER);
+}
+#endif
+
+notrace static long vdso_fallback_gettime(long clock, struct timespec *ts)
+{
+ long ret;
+
+ asm(
+ push %%ebx \n
+ mov %2,%%ebx \n
+ call VDSO32_vsyscall \n
+ pop %%ebx \n
+ : =a (ret)
+ : 0 (__NR_clock_gettime), d (clock), c (ts)
+ : memory);
+ return ret;
+}
+
+notrace static long vdso_fallback_gtod(struct timeval *tv,
+ struct
[PATCH 8/8] Add 32 bit VDSO time support for 64 bit kernel
From: Stefani Seibold stef...@seibold.net
This patch add the VDSO time support for the IA32 Emulation Layer.
Due the nature of the kernel headers and the LP64 compiler where the
size of a long and a pointer differs against a 32 bit compiler, there
is a lot of type hacking necessary.
Signed-off-by: Stefani Seibold stef...@seibold.net
---
arch/x86/vdso/vclock_gettime.c| 109 +++---
arch/x86/vdso/vdso32/vclock_gettime.c | 7 +++
2 files changed, 95 insertions(+), 21 deletions(-)
diff --git a/arch/x86/vdso/vclock_gettime.c b/arch/x86/vdso/vclock_gettime.c
index 42f641c..cd33f05 100644
--- a/arch/x86/vdso/vclock_gettime.c
+++ b/arch/x86/vdso/vclock_gettime.c
@@ -31,12 +31,24 @@
#define gtod (VVAR(vsyscall_gtod_data))
+struct api_timeval {
+ longtv_sec; /* seconds */
+ longtv_usec;/* microseconds */
+};
+
+struct api_timespec {
+ longtv_sec; /* seconds */
+ longtv_nsec;/* microseconds */
+};
+
+typedef long api_time_t;
+
static notrace cycle_t vread_hpet(void)
{
return readl((const void __iomem *)fix_to_virt(VSYSCALL_HPET) +
HPET_COUNTER);
}
-notrace static long vdso_fallback_gettime(long clock, struct timespec *ts)
+notrace static long vdso_fallback_gettime(long clock, struct api_timespec *ts)
{
long ret;
asm(syscall : =a (ret) :
@@ -44,7 +56,8 @@ notrace static long vdso_fallback_gettime(long clock, struct
timespec *ts)
return ret;
}
-notrace static long vdso_fallback_gtod(struct timeval *tv, struct timezone *tz)
+notrace static long vdso_fallback_gtod(struct api_timeval *tv,
+ struct timezone *tz)
{
long ret;
@@ -54,14 +67,62 @@ notrace static long vdso_fallback_gtod(struct timeval *tv,
struct timezone *tz)
}
#else
+#ifdef CONFIG_IA32_EMULATION
+typedef s64arch_time_t;
+
+struct arch_timespec {
+ s64 tv_sec;
+ s64 tv_nsec;
+};
+
+#define ALIGN8 __attribute__ ((aligned (8)))
+
+struct arch_vsyscall_gtod_data {
+ seqcount_t seq ALIGN8;
+
+ struct { /* extract of a clocksource struct */
+ int vclock_mode ALIGN8;
+ cycle_t cycle_last ALIGN8;
+ cycle_t mask ALIGN8;
+ u32 mult;
+ u32 shift;
+ } clock;
+
+ /* open coded 'struct timespec' */
+ arch_time_t wall_time_sec;
+ u64 wall_time_snsec;
+ u64 monotonic_time_snsec;
+ arch_time_t monotonic_time_sec;
+
+ struct timezone sys_tz;
+ struct arch_timespec wall_time_coarse;
+ struct arch_timespec monotonic_time_coarse;
+};
+
+struct arch_vsyscall_gtod_data vvar_vsyscall_gtod_data
+ __attribute__((visibility(hidden)));
+#else
struct vsyscall_gtod_data vvar_vsyscall_gtod_data
__attribute__((visibility(hidden)));
+#endif
u8 hpet_page
__attribute__((visibility(hidden)));
#define gtod (vvar_vsyscall_gtod_data)
+struct api_timeval {
+ s32 tv_sec; /* seconds */
+ s32 tv_usec;/* microseconds */
+};
+
+struct api_timespec {
+ s32 tv_sec; /* seconds */
+ s32 tv_nsec;/* microseconds */
+};
+
+typedef s32 api_time_t;
+
#ifdef CONFIG_HPET_TIMER
static notrace cycle_t vread_hpet(void)
{
@@ -69,7 +130,7 @@ static notrace cycle_t vread_hpet(void)
}
#endif
-notrace static long vdso_fallback_gettime(long clock, struct timespec *ts)
+notrace static long vdso_fallback_gettime(long clock, struct api_timespec *ts)
{
long ret;
@@ -79,12 +140,12 @@ notrace static long vdso_fallback_gettime(long clock,
struct timespec *ts)
call VDSO32_vsyscall \n
pop %%ebx \n
: =a (ret)
- : 0 (__NR_clock_gettime), d (clock), c (ts)
+ : 0 (__NR_ia32_clock_gettime), d (clock), c (ts)
: memory);
return ret;
}
-notrace static long vdso_fallback_gtod(struct timeval *tv,
+notrace static long vdso_fallback_gtod(struct api_timeval *tv,
struct timezone *tz)
{
long ret;
@@ -95,7 +156,7 @@ notrace static long vdso_fallback_gtod(struct timeval *tv,
call VDSO32_vsyscall \n
pop %%ebx \n
: =a (ret)
- : 0 (__NR_gettimeofday), d (tv), c (tz)
+ : 0 (__NR_ia32_gettimeofday), d (tv), c (tz)
: memory);
return ret;
}
@@ -284,42 +345,48 @@ notrace static void do_monotonic_coarse(struct timespec
*ts)
} while (unlikely(read_seqcount_retry(gtod-seq, seq)));
}
-notrace int __vdso_clock_gettime(clockid_t clock, struct timespec *ts)
+notrace int __vdso_clock_gettime(clockid_t clock, struct api_timespec *ts)
{
+ struct timespec tmp;
+
switch (clock) {
case CLOCK_REALTIME:
- if (do_realtime(ts) == VCLOCK_NONE)
+ if (do_realtime(tmp) ==
[PATCH 1/8] Make vsyscall_gtod_data handling x86 generic
From: Stefani Seibold stef...@seibold.net
This patch move the vsyscall_gtod_data handling out of vsyscall_64.c
into an additonal file vsyscall_gtod.c to make the functionality
available for x86 32 bit kernel.
It also adds a new vsyscall_32.c which setup the VVAR page.
Signed-off-by: Stefani Seibold stef...@seibold.net
---
arch/x86/Kconfig | 4 +--
arch/x86/include/asm/clocksource.h | 4 ---
arch/x86/include/asm/fixmap.h | 2 ++
arch/x86/include/asm/vvar.h| 4 +++
arch/x86/kernel/Makefile | 3 +-
arch/x86/kernel/hpet.c | 4 ---
arch/x86/kernel/setup.c| 2 --
arch/x86/kernel/tsc.c | 2 --
arch/x86/kernel/vmlinux.lds.S | 3 --
arch/x86/kernel/vsyscall_32.c | 24 +++
arch/x86/kernel/vsyscall_64.c | 44
arch/x86/kernel/vsyscall_gtod.c| 60 ++
12 files changed, 94 insertions(+), 62 deletions(-)
create mode 100644 arch/x86/kernel/vsyscall_32.c
create mode 100644 arch/x86/kernel/vsyscall_gtod.c
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 940e50e..b556f00 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -107,9 +107,9 @@ config X86
select HAVE_ARCH_SOFT_DIRTY
select CLOCKSOURCE_WATCHDOG
select GENERIC_CLOCKEVENTS
- select ARCH_CLOCKSOURCE_DATA if X86_64
+ select ARCH_CLOCKSOURCE_DATA
select GENERIC_CLOCKEVENTS_BROADCAST if X86_64 || (X86_32
X86_LOCAL_APIC)
- select GENERIC_TIME_VSYSCALL if X86_64
+ select GENERIC_TIME_VSYSCALL
select KTIME_SCALAR if X86_32
select GENERIC_STRNCPY_FROM_USER
select GENERIC_STRNLEN_USER
diff --git a/arch/x86/include/asm/clocksource.h
b/arch/x86/include/asm/clocksource.h
index 16a57f4..eda81dc 100644
--- a/arch/x86/include/asm/clocksource.h
+++ b/arch/x86/include/asm/clocksource.h
@@ -3,8 +3,6 @@
#ifndef _ASM_X86_CLOCKSOURCE_H
#define _ASM_X86_CLOCKSOURCE_H
-#ifdef CONFIG_X86_64
-
#define VCLOCK_NONE 0 /* No vDSO clock available. */
#define VCLOCK_TSC 1 /* vDSO should use vread_tsc. */
#define VCLOCK_HPET 2 /* vDSO should use vread_hpet. */
@@ -14,6 +12,4 @@ struct arch_clocksource_data {
int vclock_mode;
};
-#endif /* CONFIG_X86_64 */
-
#endif /* _ASM_X86_CLOCKSOURCE_H */
diff --git a/arch/x86/include/asm/fixmap.h b/arch/x86/include/asm/fixmap.h
index 7252cd3..504c04a 100644
--- a/arch/x86/include/asm/fixmap.h
+++ b/arch/x86/include/asm/fixmap.h
@@ -74,6 +74,8 @@ extern unsigned long __FIXADDR_TOP;
enum fixed_addresses {
#ifdef CONFIG_X86_32
FIX_HOLE,
+ VSYSCALL_HPET,
+ VVAR_PAGE,
FIX_VDSO,
#else
VSYSCALL_LAST_PAGE,
diff --git a/arch/x86/include/asm/vvar.h b/arch/x86/include/asm/vvar.h
index d76ac40..c442782 100644
--- a/arch/x86/include/asm/vvar.h
+++ b/arch/x86/include/asm/vvar.h
@@ -17,7 +17,11 @@
*/
/* Base address of vvars. This is not ABI. */
+#ifdef CONFIG_X86_64
#define VVAR_ADDRESS (-10*1024*1024 - 4096)
+#else
+#define VVAR_ADDRESS 0xd000
+#endif
#if defined(__VVAR_KERNEL_LDS)
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index cb648c8..3282eda 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -26,7 +26,8 @@ obj-$(CONFIG_IRQ_WORK) += irq_work.o
obj-y += probe_roms.o
obj-$(CONFIG_X86_32) += i386_ksyms_32.o
obj-$(CONFIG_X86_64) += sys_x86_64.o x8664_ksyms_64.o
-obj-y += syscall_$(BITS).o
+obj-y += syscall_$(BITS).o vsyscall_gtod.o
+obj-$(CONFIG_X86_32) += vsyscall_32.o
obj-$(CONFIG_X86_64) += vsyscall_64.o
obj-$(CONFIG_X86_64) += vsyscall_emu_64.o
obj-$(CONFIG_SYSFS)+= ksysfs.o
diff --git a/arch/x86/kernel/hpet.c b/arch/x86/kernel/hpet.c
index da85a8e..54263f0 100644
--- a/arch/x86/kernel/hpet.c
+++ b/arch/x86/kernel/hpet.c
@@ -74,9 +74,7 @@ static inline void hpet_writel(unsigned int d, unsigned int a)
static inline void hpet_set_mapping(void)
{
hpet_virt_address = ioremap_nocache(hpet_address, HPET_MMAP_SIZE);
-#ifdef CONFIG_X86_64
__set_fixmap(VSYSCALL_HPET, hpet_address, PAGE_KERNEL_VVAR_NOCACHE);
-#endif
}
static inline void hpet_clear_mapping(void)
@@ -752,9 +750,7 @@ static struct clocksource clocksource_hpet = {
.mask = HPET_MASK,
.flags = CLOCK_SOURCE_IS_CONTINUOUS,
.resume = hpet_resume_counter,
-#ifdef CONFIG_X86_64
.archdata = { .vclock_mode = VCLOCK_HPET },
-#endif
};
static int hpet_clocksource_register(void)
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 06853e6..56ff330 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1182,9 +1182,7 @@ void __init setup_arch(char **cmdline_p)
tboot_probe();
-#ifdef CONFIG_X86_64
map_vsyscall();
-#endif
generic_apic_probe();
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
[PATCH 5/8] replace VVAR(vsyscall_gtod_data) by gtod macro
From: Stefani Seibold stef...@seibold.net
There a currently more than 30 users of the gtod macro, so replace the
last VVAR(vsyscall_gtod_data) by gtod macro.
Signed-off-by: Stefani Seibold stef...@seibold.net
---
arch/x86/vdso/vclock_gettime.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/vdso/vclock_gettime.c b/arch/x86/vdso/vclock_gettime.c
index fd074dd..743f277 100644
--- a/arch/x86/vdso/vclock_gettime.c
+++ b/arch/x86/vdso/vclock_gettime.c
@@ -109,7 +109,7 @@ static notrace cycle_t vread_pvclock(int *mode)
*mode = VCLOCK_NONE;
/* refer to tsc.c read_tsc() comment for rationale */
- last = VVAR(vsyscall_gtod_data).clock.cycle_last;
+ last = gtod-clock.cycle_last;
if (likely(ret = last))
return ret;
@@ -133,7 +133,7 @@ notrace static cycle_t vread_tsc(void)
rdtsc_barrier();
ret = (cycle_t)vget_cycles();
- last = VVAR(vsyscall_gtod_data).clock.cycle_last;
+ last = gtod-clock.cycle_last;
if (likely(ret = last))
return ret;
@@ -288,7 +288,7 @@ int gettimeofday(struct timeval *, struct timezone *)
notrace time_t __vdso_time(time_t *t)
{
/* This is atomic on x86_64 so we don't need any locks. */
- time_t result = ACCESS_ONCE(VVAR(vsyscall_gtod_data).wall_time_sec);
+ time_t result = ACCESS_ONCE(gtod-wall_time_sec);
if (t)
*t = result;
--
1.8.5.3
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/8] Add 32 bit VDSO time function support
From: Stefani Seibold stef...@seibold.net This patch add the functions vdso_gettimeofday(), vdso_clock_gettime() and vdso_time() to the 32 bit VDSO. The reason to do this was to get a fast reliable time stamp. Many developers uses TSC to get a fast time stamp, without knowing the pitfalls. VDSO time functions a fast and a reliable way, because the kernel knows the best time source and the P- and C-state of the CPU. The helper library to use the VDSO functions can be download at http://http://seibold.net/vdso.c The libary is very small, only 228 lines of code. Compile it with gcc -Wall -O3 -fpic vdso.c -lrt -shared -o libvdso.so and use it with LD_PRELOAD=path/libvdso.so This kind of helper must be integrated into glibc, for x86 64 bit and PowerPC it is already there. Some linux 32 bit kernel benchmark results (all measurements are in nano seconds): Intel(R) Celeron(TM) CPU 400MHz Average time kernel call: gettimeofday(): 1039 clock_gettime(): 1578 time(): 526 Average time VDSO call: gettimeofday(): 378 clock_gettime(): 303 time(): 60 Celeron(R) Dual-Core CPU T3100 1.90GHz Average time kernel call: gettimeofday(): 209 clock_gettime(): 406 time(): 135 Average time VDSO call: gettimeofday(): 51 clock_gettime(): 43 time(): 10 So you can see a performance increase between 4 and 13, depending on the CPU and the function. The address layout of the VDSO has changed, because there is no fixed address space available on a x86 32 bit kernel, despite the name. Because someone decided to add an offset to the __FIXADDR_TOP for virtualization. Also the IA32 Emulation uses the whole 4 GB address space, so there is no fixed address available. This was the reason not depend on this kind of address and change the layout of the VDSO. The VDSO for a 32 bit application has now three pages: ++ + VDSO page (includes code) ro+x + ++ + VVAR page (export kernel variables) ro + ++ + HPET page (mapped registers) ro ++ The VDSO page for a 32 bit resided now on 0xc000, followed by the VVAR and HPET page. In the non compat mode the VMA of the VDSO is now 3 pages for a 32 bit kernel. So this decrease the available logical address room by 2 pages. The patch is against kernel 3.14 (e7651b819e90da924991d727d3c007200a18670d) Changelog: 25.11.2012 - first release and proof of concept for linux 3.4 11.12.2012 - Port to linux 3.7 and code cleanup 12.12.2012 - fixes suggested by Andy Lutomirski - fixes suggested by John Stultz - use call VDSO32_vsyscall instead of int 80 - code cleanup 17.12.2012 - support for IA32_EMULATION, this includes - code cleanup - include cleanup to fix compile warnings and errors - move out seqcount from seqlock, enable use in VDSO - map FIXMAP and HPET into the 32 bit address space 18.12.2012 - split into separate patches 30.01.2014 - revamp the code - code clean up - VDSO layout changed - no fixed addresses - port to 3.14 01.02.2014 - code cleanup 02.02.2014 - code cleanup - split into more patches - use HPET_COUNTER instead of hard coded value - fix changelog to the right year ;-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/8] revamp vclock_gettime.c
From: Stefani Seibold stef...@seibold.net
This intermediate patch revamps the vclock_gettime.c by moving some functions
around. It is only for spliting purpose, to make whole the 32 bit vdso timer
patch easier to review.
Signed-off-by: Stefani Seibold stef...@seibold.net
---
arch/x86/vdso/vclock_gettime.c | 85 +-
1 file changed, 42 insertions(+), 43 deletions(-)
diff --git a/arch/x86/vdso/vclock_gettime.c b/arch/x86/vdso/vclock_gettime.c
index eb5d7a5..bbc8065 100644
--- a/arch/x86/vdso/vclock_gettime.c
+++ b/arch/x86/vdso/vclock_gettime.c
@@ -26,41 +26,26 @@
#define gtod (VVAR(vsyscall_gtod_data))
-notrace static cycle_t vread_tsc(void)
+static notrace cycle_t vread_hpet(void)
{
- cycle_t ret;
- u64 last;
-
- /*
-* Empirically, a fence (of type that depends on the CPU)
-* before rdtsc is enough to ensure that rdtsc is ordered
-* with respect to loads. The various CPU manuals are unclear
-* as to whether rdtsc can be reordered with later loads,
-* but no one has ever seen it happen.
-*/
- rdtsc_barrier();
- ret = (cycle_t)vget_cycles();
-
- last = VVAR(vsyscall_gtod_data).clock.cycle_last;
-
- if (likely(ret = last))
- return ret;
+ return readl((const void __iomem *)fix_to_virt(VSYSCALL_HPET) +
HPET_COUNTER);
+}
- /*
-* GCC likes to generate cmov here, but this branch is extremely
-* predictable (it's just a funciton of time and the likely is
-* very likely) and there's a data dependence, so force GCC
-* to generate a branch instead. I don't barrier() because
-* we don't actually need a barrier, and if this function
-* ever gets inlined it will generate worse code.
-*/
- asm volatile ();
- return last;
+notrace static long vdso_fallback_gettime(long clock, struct timespec *ts)
+{
+ long ret;
+ asm(syscall : =a (ret) :
+ 0 (__NR_clock_gettime), D (clock), S (ts) : memory);
+ return ret;
}
-static notrace cycle_t vread_hpet(void)
+notrace static long vdso_fallback_gtod(struct timeval *tv, struct timezone *tz)
{
- return readl((const void __iomem *)fix_to_virt(VSYSCALL_HPET) +
HPET_COUNTER);
+ long ret;
+
+ asm(syscall : =a (ret) :
+ 0 (__NR_gettimeofday), D (tv), S (tz) : memory);
+ return ret;
}
#ifdef CONFIG_PARAVIRT_CLOCK
@@ -133,23 +118,37 @@ static notrace cycle_t vread_pvclock(int *mode)
}
#endif
-notrace static long vdso_fallback_gettime(long clock, struct timespec *ts)
+notrace static cycle_t vread_tsc(void)
{
- long ret;
- asm(syscall : =a (ret) :
- 0 (__NR_clock_gettime),D (clock), S (ts) : memory);
- return ret;
-}
+ cycle_t ret;
+ u64 last;
-notrace static long vdso_fallback_gtod(struct timeval *tv, struct timezone *tz)
-{
- long ret;
+ /*
+* Empirically, a fence (of type that depends on the CPU)
+* before rdtsc is enough to ensure that rdtsc is ordered
+* with respect to loads. The various CPU manuals are unclear
+* as to whether rdtsc can be reordered with later loads,
+* but no one has ever seen it happen.
+*/
+ rdtsc_barrier();
+ ret = (cycle_t)vget_cycles();
- asm(syscall : =a (ret) :
- 0 (__NR_gettimeofday), D (tv), S (tz) : memory);
- return ret;
-}
+ last = VVAR(vsyscall_gtod_data).clock.cycle_last;
+ if (likely(ret = last))
+ return ret;
+
+ /*
+* GCC likes to generate cmov here, but this branch is extremely
+* predictable (it's just a funciton of time and the likely is
+* very likely) and there's a data dependence, so force GCC
+* to generate a branch instead. I don't barrier() because
+* we don't actually need a barrier, and if this function
+* ever gets inlined it will generate worse code.
+*/
+ asm volatile ();
+ return last;
+}
notrace static inline u64 vgetsns(int *mode)
{
--
1.8.5.3
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/8] Add new func _install_special_mapping() to mmap.c
From: Stefani Seibold stef...@seibold.net
The _install_special_mapping() is the new base function for
install_special_mapping(). This function will return a pointer of the
created VMA or a error code in an ERR_PTR()
This new function will be needed by the for the vdso 32 bit support to map the
additonal vvar and hpet pages into the 32 bit address space. This will be done
with io_remap_pfn_range() and remap_pfn_range, which requieres a vm_area_struct.
Signed-off-by: Stefani Seibold stef...@seibold.net
---
include/linux/mm.h | 3 +++
mm/mmap.c | 20
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index f28f46e..55342aa 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1740,6 +1740,9 @@ extern void set_mm_exe_file(struct mm_struct *mm, struct
file *new_exe_file);
extern struct file *get_mm_exe_file(struct mm_struct *mm);
extern int may_expand_vm(struct mm_struct *mm, unsigned long npages);
+extern struct vm_area_struct *_install_special_mapping(struct mm_struct *mm,
+ unsigned long addr, unsigned long len,
+ unsigned long flags, struct page **pages);
extern int install_special_mapping(struct mm_struct *mm,
unsigned long addr, unsigned long len,
unsigned long flags, struct page **pages);
diff --git a/mm/mmap.c b/mm/mmap.c
index 20ff0c3..81ba54f 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2918,7 +2918,7 @@ static const struct vm_operations_struct
special_mapping_vmops = {
* The array pointer and the pages it points to are assumed to stay alive
* for as long as this mapping might exist.
*/
-int install_special_mapping(struct mm_struct *mm,
+struct vm_area_struct *_install_special_mapping(struct mm_struct *mm,
unsigned long addr, unsigned long len,
unsigned long vm_flags, struct page **pages)
{
@@ -2927,7 +2927,7 @@ int install_special_mapping(struct mm_struct *mm,
vma = kmem_cache_zalloc(vm_area_cachep, GFP_KERNEL);
if (unlikely(vma == NULL))
- return -ENOMEM;
+ return ERR_PTR(-ENOMEM);
INIT_LIST_HEAD(vma-anon_vma_chain);
vma-vm_mm = mm;
@@ -2948,11 +2948,23 @@ int install_special_mapping(struct mm_struct *mm,
perf_event_mmap(vma);
- return 0;
+ return vma;
out:
kmem_cache_free(vm_area_cachep, vma);
- return ret;
+ return ERR_PTR(ret);
+}
+
+int install_special_mapping(struct mm_struct *mm,
+ unsigned long addr, unsigned long len,
+ unsigned long vm_flags, struct page **pages)
+{
+ struct vm_area_struct *vma = _install_special_mapping(mm,
+ addr, len, vm_flags, pages);
+
+ if (IS_ERR(vma))
+ return PTR_ERR(vma);
+ return 0;
}
static DEFINE_MUTEX(mm_all_locks_mutex);
--
1.8.5.3
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 4/8] vclock_gettime.c __vdso_clock_gettime cleanup
From: Stefani Seibold stef...@seibold.net
This patch is a small code cleanup for the __vdso_clock_gettime() function.
It removes the unneeded return values from do_monotonic_coarse() and
do_realtime_coarse() and add a fallback label for doing the kernel
gettimeofday() system call.
Signed-off-by: Stefani Seibold stef...@seibold.net
---
arch/x86/vdso/vclock_gettime.c | 27 ++-
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/arch/x86/vdso/vclock_gettime.c b/arch/x86/vdso/vclock_gettime.c
index bbc8065..fd074dd 100644
--- a/arch/x86/vdso/vclock_gettime.c
+++ b/arch/x86/vdso/vclock_gettime.c
@@ -209,7 +209,7 @@ notrace static int do_monotonic(struct timespec *ts)
return mode;
}
-notrace static int do_realtime_coarse(struct timespec *ts)
+notrace static void do_realtime_coarse(struct timespec *ts)
{
unsigned long seq;
do {
@@ -217,10 +217,9 @@ notrace static int do_realtime_coarse(struct timespec *ts)
ts-tv_sec = gtod-wall_time_coarse.tv_sec;
ts-tv_nsec = gtod-wall_time_coarse.tv_nsec;
} while (unlikely(read_seqcount_retry(gtod-seq, seq)));
- return 0;
}
-notrace static int do_monotonic_coarse(struct timespec *ts)
+notrace static void do_monotonic_coarse(struct timespec *ts)
{
unsigned long seq;
do {
@@ -228,30 +227,32 @@ notrace static int do_monotonic_coarse(struct timespec
*ts)
ts-tv_sec = gtod-monotonic_time_coarse.tv_sec;
ts-tv_nsec = gtod-monotonic_time_coarse.tv_nsec;
} while (unlikely(read_seqcount_retry(gtod-seq, seq)));
-
- return 0;
}
notrace int __vdso_clock_gettime(clockid_t clock, struct timespec *ts)
{
- int ret = VCLOCK_NONE;
-
switch (clock) {
case CLOCK_REALTIME:
- ret = do_realtime(ts);
+ if (do_realtime(ts) == VCLOCK_NONE)
+ goto fallback;
break;
case CLOCK_MONOTONIC:
- ret = do_monotonic(ts);
+ if (do_monotonic(ts) == VCLOCK_NONE)
+ goto fallback;
break;
case CLOCK_REALTIME_COARSE:
- return do_realtime_coarse(ts);
+ do_realtime_coarse(ts);
+ break;
case CLOCK_MONOTONIC_COARSE:
- return do_monotonic_coarse(ts);
+ do_monotonic_coarse(ts);
+ break;
+ default:
+ goto fallback;
}
- if (ret == VCLOCK_NONE)
- return vdso_fallback_gettime(clock, ts);
return 0;
+fallback:
+ return vdso_fallback_gettime(clock, ts);
}
int clock_gettime(clockid_t, struct timespec *)
__attribute__((weak, alias(__vdso_clock_gettime)));
--
1.8.5.3
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: NFS client broken in Linus' tip
On Sat, Feb 01, 2014 at 01:03:28AM +, Russell King - ARM Linux wrote: On Fri, Jan 31, 2014 at 03:59:30PM -0500, Trond Myklebust wrote: On Thu, 2014-01-30 at 15:38 +, Russell King - ARM Linux wrote: On Thu, Jan 30, 2014 at 06:32:08AM -0800, Christoph Hellwig wrote: On Thu, Jan 30, 2014 at 02:27:52PM +, Russell King - ARM Linux wrote: Yes and no. I still end up with an empty /etc/mtab, but the file now exists. However, I can create and echo data into /etc/mtab, but it seems that can't happen at boot time. Odd. Can you disable CONFIG_NFSD_V3_ACL for now to isolate the issue? Unfortunately, that results in some problem at boot time, which ultimately ends up with the other three CPUs being stopped, and hence the original reason scrolls off the screen before it can be read... even at 1920p. Hi Russell, The following patch fixes the issue for me. It doesn't entirely fix the issue for me, instead we've got even weirder behaviour: root@cubox-i4:~# ls -al test ls: cannot access test: No such file or directory root@cubox-i4:~# touch test root@cubox-i4:~# ls -al test -rw-r--r-- 1 root root 0 Feb 1 01:01 test root@cubox-i4:~# echo foo test root@cubox-i4:~# ls -al test -rw-r--r-- 1 root root 4 Feb 1 01:01 test root@cubox-i4:~# cat test foo root@cubox-i4:~# rm test root@cubox-i4:~# echo foo test -bash: test: Operation not supported root@cubox-i4:~# ls -al test -rw-r--r-- 1 root root 0 Feb 1 01:01 test FYI, I just tested Linus' tip, and NFS is still broken. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] spi: switch to devm_spi_alloc_master
On Fri, Jan 31, 2014 at 11:23 +0100, Maxime Ripard wrote:
Make the existing users of devm_spi_register_master use the
devm_spi_alloc_master function to avoid leaking memory.
[ ... ]
drivers/spi/spi-mpc512x-psc.c| 19 ---
Note that the context for the MPC512x SPI driver will change in
3.14-rc1, so you will have to rebase after the merge window.
diff --git a/drivers/spi/spi-mpc512x-psc.c b/drivers/spi/spi-mpc512x-psc.c
index 46d2313..f376595 100644
--- a/drivers/spi/spi-mpc512x-psc.c
+++ b/drivers/spi/spi-mpc512x-psc.c
@@ -479,7 +479,7 @@ static int mpc512x_psc_spi_do_probe(struct device *dev,
u32 regaddr,
char clk_name[16];
struct clk *clk;
- master = spi_alloc_master(dev, sizeof *mps);
+ master = devm_spi_alloc_master(dev, sizeof *mps);
if (master == NULL)
return -ENOMEM;
@@ -507,8 +507,7 @@ static int mpc512x_psc_spi_do_probe(struct device *dev,
u32 regaddr,
tempp = devm_ioremap(dev, regaddr, size);
if (!tempp) {
dev_err(dev, could not ioremap I/O port range\n);
- ret = -EFAULT;
- goto free_master;
+ return -EFAULT;
}
mps-psc = tempp;
mps-fifo =
@@ -516,19 +515,19 @@ static int mpc512x_psc_spi_do_probe(struct device *dev,
u32 regaddr,
ret = devm_request_irq(dev, mps-irq, mpc512x_psc_spi_isr, IRQF_SHARED,
mpc512x-psc-spi, mps);
if (ret)
- goto free_master;
+ return ret;
init_completion(mps-txisrdone);
psc_num = master-bus_num;
snprintf(clk_name, sizeof(clk_name), psc%d_mclk, psc_num);
clk = devm_clk_get(dev, clk_name);
- if (IS_ERR(clk)) {
- ret = PTR_ERR(clk);
- goto free_master;
- }
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
ret = clk_prepare_enable(clk);
if (ret)
- goto free_master;
+ return ret;
+
mps-clk_mclk = clk;
mps-mclk_rate = clk_get_rate(clk);
@@ -544,8 +543,6 @@ static int mpc512x_psc_spi_do_probe(struct device *dev,
u32 regaddr,
free_clock:
clk_disable_unprepare(mps-clk_mclk);
-free_master:
- spi_master_put(master);
return ret;
}
Reading the diff in the SPI master driver, the change appears to
be balanced, and replacing 'goto free_master' with immediate
return looks appropriate.
Can't comment on the correctness of removing the spi_master_put()
call when switching from spi_alloc_master() to
devm_spi_alloc_master(). This gets discussed in the other
subthread, dealing with the generic subsystem approach.
virtually yours
Gerhard Sittig
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 00/23]
On Wed, Jan 29, 2014 at 10:01:22AM +0100, Jean-Francois Moine wrote: This patch set contains various extensions to the tda998x driver: - simplify the i2c read/write - code cleanup and fix some small errors - use global constants - don't read write-only registers - add DT support - use IRQ for connection status and EDID read I discussed these patches with Rob Clark recently, and the conclusion we came to is that I'll merge them into a git tree, test them, and once I'm happy I'll send a pull request as appropriate. I'll go through them later today. Those patches which have been re- posted without any change for the last few times (the first few) I'll take into my git tree today so you don't have to keep re-posting them (more importantly, I won't have to keep on looking at them either.) -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: ..another device for the ftdi_sio driver, 3rd infusion
On Sun, Feb 02, 2014 at 12:10:56PM +0100, Ulrich Hahn wrote:
Please use a more descriptive subject line (which will end up as the
commit log summary), e.g.
[PATCH v3] USB: ftdi_sio: add Tagsys RFID Reader ids
Note that the patch version should go in the [PATCH vX] (which will not
show up in the git log).
You should also consider adding a slightly more verbose message in the
body as well (e.g. mentioning which two devices you add support for).
Right now your commit log would be empty.
Signed-off-by: Ulrich Hahn uh...@eanco.de
diff -ur linux-3.13.1/drivers/usb/serial/ftdi_sio.c
linux-3.13.1+tagsys/drivers/usb/serial/ftdi_sio.c
--- linux-3.13.1/drivers/usb/serial/ftdi_sio.c2014-01-29
14:06:37.0 +0100
+++ linux-3.13.1+tagsys/drivers/usb/serial/ftdi_sio.c 2014-02-02
11:57:08.625914749 +0100
@@ -192,6 +192,8 @@
{ USB_DEVICE(INTERBIOMETRICS_VID, INTERBIOMETRICS_IOBOARD_PID) },
{ USB_DEVICE(INTERBIOMETRICS_VID, INTERBIOMETRICS_MINI_IOBOARD_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_SPROG_II) },
+ { USB_DEVICE(FTDI_VID, FTDI_TAGSYS_LP101_PID) },
+ { USB_DEVICE(FTDI_VID, FTDI_TAGSYS_P200X_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_LENZ_LIUSB_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_XF_632_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_XF_634_PID) },
Only in linux-3.13.1+tagsys/drivers/usb/serial: ftdi_sio.c.orig
diff -ur linux-3.13.1/drivers/usb/serial/ftdi_sio_ids.h
linux-3.13.1+tagsys/drivers/usb/serial/ftdi_sio_ids.h
--- linux-3.13.1/drivers/usb/serial/ftdi_sio_ids.h2014-01-29
14:06:37.0 +0100
+++ linux-3.13.1+tagsys/drivers/usb/serial/ftdi_sio_ids.h 2014-02-02
11:57:08.629914782 +0100
@@ -363,6 +363,13 @@
/* Sprog II (Andrew Crosland's SprogII DCC interface) */
#define FTDI_SPROG_II0xF0C8
+/*
+ * Tagsys RFID Readers
+ * survey (two of them) by Ulrich Hahn
No need to include the author in the comment. It will be apparent from
the git log.
+ */
+#define FTDI_TAGSYS_LP101_PID0xF0E9 /* Tagsys L-P101 RFID*/
+#define FTDI_TAGSYS_P200X_PID0xF0EE /* Tagsys Medio P200x RFID*/
+
/* an infrared receiver for user access control with IR tags */
#define FTDI_PIEGROUP_PID0xF208 /* Product Id */
Thanks,
Johan
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v4] USB: ftdi_sio: add Tagsys RFID Reader IDs
Adding two more IDs to the ftdi_sio usb serial driver.
It now connects Tagsys RFID readers.
There might be more IDs out there for other Tagsys models.
Signed-off-by: Ulrich Hahn uh...@eanco.de
diff -urpN linux-3.13.1/drivers/usb/serial/ftdi_sio.c
linux-3.13.1+tagsys/drivers/usb/serial/ftdi_sio.c
--- linux-3.13.1/drivers/usb/serial/ftdi_sio.c 2014-01-29 14:06:37.0
+0100
+++ linux-3.13.1+tagsys/drivers/usb/serial/ftdi_sio.c 2014-02-02
11:57:08.625914749 +0100
@@ -192,6 +192,8 @@ static struct usb_device_id id_table_com
{ USB_DEVICE(INTERBIOMETRICS_VID, INTERBIOMETRICS_IOBOARD_PID) },
{ USB_DEVICE(INTERBIOMETRICS_VID, INTERBIOMETRICS_MINI_IOBOARD_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_SPROG_II) },
+ { USB_DEVICE(FTDI_VID, FTDI_TAGSYS_LP101_PID) },
+ { USB_DEVICE(FTDI_VID, FTDI_TAGSYS_P200X_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_LENZ_LIUSB_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_XF_632_PID) },
{ USB_DEVICE(FTDI_VID, FTDI_XF_634_PID) },
diff -urpN linux-3.13.1/drivers/usb/serial/ftdi_sio_ids.h
linux-3.13.1+tagsys/drivers/usb/serial/ftdi_sio_ids.h
--- linux-3.13.1/drivers/usb/serial/ftdi_sio_ids.h 2014-01-29
14:06:37.0 +0100
+++ linux-3.13.1+tagsys/drivers/usb/serial/ftdi_sio_ids.h 2014-02-02
14:32:41.153358910 +0100
@@ -363,6 +363,12 @@
/* Sprog II (Andrew Crosland's SprogII DCC interface) */
#define FTDI_SPROG_II 0xF0C8
+/*
+ * Two of the Tagsys RFID Readers
+ */
+#define FTDI_TAGSYS_LP101_PID 0xF0E9 /* Tagsys L-P101 RFID*/
+#define FTDI_TAGSYS_P200X_PID 0xF0EE /* Tagsys Medio P200x RFID*/
+
/* an infrared receiver for user access control with IR tags */
#define FTDI_PIEGROUP_PID 0xF208 /* Product Id */
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 14/16] drm/nouveau/fb: add GK20A support
On Sun, Feb 2, 2014 at 8:58 AM, Lucas Stach d...@lynxeye.de wrote: Am Samstag, den 01.02.2014, 18:28 -0500 schrieb Ilia Mirkin: On Sat, Feb 1, 2014 at 8:40 AM, Lucas Stach d...@lynxeye.de wrote: Am Samstag, den 01.02.2014, 12:16 +0900 schrieb Alexandre Courbot: Add a clumsy-but-working FB support for GK20A. This chip only uses system memory, so we allocate a big chunk using CMA and let the existing memory managers work on it. A better future design would be to allocate objects directly from system memory without having to suffer from the limitations of a large, contiguous pool. I don't know if Tegra124 is similar to 114 in this regard [hint: get the TRM out :)], but if you go for a dedicated VRAM allocator, wouldn't it make sense to take a chunk of the MMIO overlaid memory for this when possible, rather than carving this out of CPU accessible mem? This is probably a stupid question... what do you need VRAM for anyways? In _theory_ it's an abstraction to talk about memory that's not accessible by the CPU. This is obviously not the case here, and presumably the GPU can access all the memory in the system, so it can be all treated as GART memory... AFAIK all accesses are behind the in-GPU MMU, so contiguous physical memory isn't an issue either. In practice, I suspect nouveau automatically sticks certain things into vram (gpuobj's), but it should be feasible to make them optionally use GART memory when VRAM is not available. I haven't really looked at the details though, perhaps that's a major undertaking. -ilia If it's similar to the Tegar114 there actually is memory that isn't accessible from the CPU. About 2GB of the address space is overlaid with MMIO for the devices, so in a 4GB system you potentially have 2GB of RAM that's only visible for the devices. But yes in general nouveau should just fall back to a GART placement if VRAM isn't available. With the limited time I spent studying it, it seems to me that Nouveau has a strong dependency on VRAM. For gpuobjects indeed (that one could be workarounded with a new instmem driver I suppose), and also for TTM: objects placed in TTM_PL_VRAM are handled by the VRAM manager, which requires a nouveau_ram instance in the FB. Actually the FB also seems to assume the presence of a dedicated video RAM. So while I agree that getting rid of VRAM altogether would be the most logical solution, I have not found a way to do so for the moment. T124's GPU actually sees the same physical address space as the CPU, so memory management should be simplified thanks to that (you could enable the SMMU and make things more interesting/complex, but for now it seems untimely to even consider doing so). Actually even the concept of a GART is not needed here: all your memory management needs could be fulfilled by getting pages with alloc_page() and arranging them using the GMMU. No GART, no BAR (at least for the purpose of mapping objects for CPU access), no PRAMIN. I really wonder how that picture would fit within Nouveau, and it is quite likely that there is an elegant solution to this problem already that my lack of understanding of Nouveau prevents me from seeing. That's why your thoughts on this matter would be greatly appreciated. Thanks, Alex. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Why is syscall auditing on with no rules?
Andy Lutomirski l...@amacapital.net writes: On a stock Fedora installation: $ sudo auditctl -l No rules I noticed the same recently on a recent opensuse. kauditd is running, even though I uninstalled all audit related userland long before. I'm sure the evil make syscalls slow flag is set too. -Andi -- a...@linux.intel.com -- Speaking for myself only -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/10] fs: Introduce new flag(FALLOC_FL_COLLAPSE_RANGE) for fallocate
On Sun, Feb 02, 2014 at 02:41:34PM +0900, Namjae Jeon wrote: The semantics of this flag are following: 1) It collapses the range lying between offset and length by removing any data blocks which are present in this range and than updates all the logical offsets of extents beyond offset + len to nullify the hole created by removing blocks. In short, it does not leave a hole. 2) It should be used exclusively. No other fallocate flag in combination. 3) Offset and length supplied to fallocate should be fs block size aligned in case of xfs and ext4. 4) Collaspe range does not work beyond i_size. What if the file is mmaped at the time somebody issues this command? Seems to me we should drop pagecache pages that overlap with the removed blocks. If the removed range is not a multiple of PAGE_SIZE, then we should also drop any pagecache pages after the removed range. -- Matthew Wilcox Intel Open Source Technology Centre Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND 0/10] fs: Introduce new flag(FALLOC_FL_COLLAPSE_RANGE) for fallocate
On Sun, Feb 02, 2014 at 08:16:24AM -0700, Matthew Wilcox wrote: On Sun, Feb 02, 2014 at 02:41:34PM +0900, Namjae Jeon wrote: The semantics of this flag are following: 1) It collapses the range lying between offset and length by removing any data blocks which are present in this range and than updates all the logical offsets of extents beyond offset + len to nullify the hole created by removing blocks. In short, it does not leave a hole. 2) It should be used exclusively. No other fallocate flag in combination. 3) Offset and length supplied to fallocate should be fs block size aligned in case of xfs and ext4. 4) Collaspe range does not work beyond i_size. What if the file is mmaped at the time somebody issues this command? Seems to me we should drop pagecache pages that overlap with the removed blocks. If the removed range is not a multiple of PAGE_SIZE, then we should also drop any pagecache pages after the removed range. Oops, forgot to add and if it is a multiple of page size, then we need to update the offsets of any pages after the removed page. We should probably start easy though; just drop all pages that overlap the beginning of the affected range to the end of the file. At some later point, if there's demand, we can add the optimisation to adjust the offsets of pages still in the cache. -- Matthew Wilcox Intel Open Source Technology Centre Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/4] Add 32 bit VDSO time support for 32 bit kernel
Sorry, I did not know/realize, and unfortunately he is the grammatical default in English, although the singular they is fortunately making a comeback. I stand corrected, through. Please acer my apologies. -hpa On February 2, 2014 3:20:25 AM PST, Stefani Seibold stef...@seibold.net wrote: -#define VDSO_HIGH_BASE 0xe000U /* CONFIG_COMPAT_VDSO address */ +#define VDSO_HIGH_BASE 0xc000U /* CONFIG_COMPAT_VDSO address */ This is odd. Can you explain it? He needs 3 pages instead of 1 after his changes. Not every kernel hackers a male. By definition i am femal, so please replace he and his with she and her. - Stefani -- Sent from my mobile phone. Please pardon brevity and lack of formatting. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/8] cleanup __vdso_gettimeofday
Even reading the timezone via gettimeofday is the uncommon case... never mind
only the timezome. Whatever you do please don't slow down the common case.
On February 2, 2014 3:27:13 AM PST, stef...@seibold.net wrote:
From: Stefani Seibold stef...@seibold.net
This patch do a little cleanup for the __vdso_gettimeofday() function.
It kick out an unneeded ret local variable and makes the code faster if
only the timezone is needed.
Signed-off-by: Stefani Seibold stef...@seibold.net
---
arch/x86/vdso/vclock_gettime.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/x86/vdso/vclock_gettime.c
b/arch/x86/vdso/vclock_gettime.c
index 743f277..bf969a0 100644
--- a/arch/x86/vdso/vclock_gettime.c
+++ b/arch/x86/vdso/vclock_gettime.c
@@ -259,13 +259,12 @@ int clock_gettime(clockid_t, struct timespec *)
notrace int __vdso_gettimeofday(struct timeval *tv, struct timezone
*tz)
{
- long ret = VCLOCK_NONE;
-
if (likely(tv != NULL)) {
BUILD_BUG_ON(offsetof(struct timeval, tv_usec) !=
offsetof(struct timespec, tv_nsec) ||
sizeof(*tv) != sizeof(struct timespec));
- ret = do_realtime((struct timespec *)tv);
+ if (do_realtime((struct timespec *)tv) == VCLOCK_NONE)
+ return vdso_fallback_gtod(tv, tz);
tv-tv_usec /= 1000;
}
if (unlikely(tz != NULL)) {
@@ -274,8 +273,6 @@ notrace int __vdso_gettimeofday(struct timeval *tv,
struct timezone *tz)
tz-tz_dsttime = gtod-sys_tz.tz_dsttime;
}
- if (ret == VCLOCK_NONE)
- return vdso_fallback_gtod(tv, tz);
return 0;
}
int gettimeofday(struct timeval *, struct timezone *)
--
Sent from my mobile phone. Please pardon brevity and lack of formatting.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 6/8] cleanup __vdso_gettimeofday
On Sun, 2014-02-02 at 07:43 -0800, H. Peter Anvin wrote:
Even reading the timezone via gettimeofday is the uncommon case... never mind
only the timezome. Whatever you do please don't slow down the common case.
On February 2, 2014 3:27:13 AM PST, stef...@seibold.net wrote:
From: Stefani Seibold stef...@seibold.net
This patch do a little cleanup for the __vdso_gettimeofday() function.
It kick out an unneeded ret local variable and makes the code faster if
only the timezone is needed.
Signed-off-by: Stefani Seibold stef...@seibold.net
---
arch/x86/vdso/vclock_gettime.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/x86/vdso/vclock_gettime.c
b/arch/x86/vdso/vclock_gettime.c
index 743f277..bf969a0 100644
--- a/arch/x86/vdso/vclock_gettime.c
+++ b/arch/x86/vdso/vclock_gettime.c
@@ -259,13 +259,12 @@ int clock_gettime(clockid_t, struct timespec *)
notrace int __vdso_gettimeofday(struct timeval *tv, struct timezone
*tz)
{
-long ret = VCLOCK_NONE;
-
if (likely(tv != NULL)) {
BUILD_BUG_ON(offsetof(struct timeval, tv_usec) !=
offsetof(struct timespec, tv_nsec) ||
sizeof(*tv) != sizeof(struct timespec));
-ret = do_realtime((struct timespec *)tv);
+if (do_realtime((struct timespec *)tv) == VCLOCK_NONE)
+return vdso_fallback_gtod(tv, tz);
tv-tv_usec /= 1000;
}
if (unlikely(tz != NULL)) {
@@ -274,8 +273,6 @@ notrace int __vdso_gettimeofday(struct timeval *tv,
struct timezone *tz)
tz-tz_dsttime = gtod-sys_tz.tz_dsttime;
}
-if (ret == VCLOCK_NONE)
-return vdso_fallback_gtod(tv, tz);
return 0;
}
int gettimeofday(struct timeval *, struct timezone *)
I don't see where i slow down the common case! It is exactly the same
behaviour as the original code. Only in the uncommon case where the tv
== NULL and tz != NULL is a bit faster.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Is it ok for deferrable timer wakeup the idle cpu?
Hi Frederic, On 01/31/2014 10:00 PM, Frederic Weisbecker wrote: On Wed, Jan 29, 2014 at 10:57:59AM +0530, Preeti Murthy wrote: Hi, On Thu, Jan 23, 2014 at 11:22 AM, Viresh Kumar viresh.ku...@linaro.org wrote: Hi Guys, So the first question is why cpufreq needs it and is it really stupid? Yes, it is stupid but that's how its implemented since a long time. It does so to get data about the load on CPUs, so that freq can be scaled up/down. Though there is a solution in discussion currently, which will take inputs from scheduler and so these background timers would go away. But we need to wait until that time. Now, why do we need that for every cpu, while that for a single cpu might be enough? The answer is cpuidle here: What if the cpu responsible for running timer goes to sleep? Who will evaluate the load then? And if we make this timer run on one cpu in non-deferrable mode then that cpu would be waken up again and again from idle. So, it was decided to have a per-cpu deferrable timer. Though to improve efficiency, once it is fired on any cpu, timer for all other CPUs are rescheduled, so that they don't fire before 5ms (sampling time).. How about simplifying this design by doing the below? 1. Since anyway cpufreq governors monitor load on the cpu once every 5ms, *tie it with tick_sched_timer*, which also gets deferred when the cpu enters nohz_idle. 2. To overcome the problem of running this job of monitoring the load on every cpu, have the *time keeping* cpu do it for you. The time keeping cpu has the property that if it has to go to idle, it will do so and let the next cpu that runs the periodic timer become the time keeper. Hence no cpu is prevented from entering nohz_idle and the cpu that is busy and first executes periodic timer will take over as the time keeper. The result would be: 1. One cpu at any point in time will be monitoring cpu load, at every sched tick as long as its busy. If it goes to sleep, then it gives up this duty and enters idle. The next cpu that runs the periodic timer becomes the cpu to monitor the load and will continue to do so as long as its busy. Hence we do not miss monitoring the cpu load. Well that's basically what an unbound deferrable timer does. It's deferrable so it's doesn't prevent from entering dynticks idle mode and it's not affine to any particular CPU so it's going to be tied to a buzy CPU according to the scheduler (see get_nohz_timer_target()). 2. This will avoid an additional timer for cpufreq. That doesn't look like a problem. 3. It avoids sending IPIs each time this timer gets modified since there is just one CPU doing the monitoring. If we fix the initial issue properly, we shouldn't need to send an IPI anymore. That's right. I am sorry I missed that we were completely avoiding IPIs in case of deferrable timers. 4. The downside to this could be that we are stretching the functions of the periodic timer into the power management domain which does not seem like the right thing to do. Indeed, that's what I'm worried about. The tick has grown into a Big Kernel Timer where any subsystem can hook into for any kind of periodic event. This is why it was not easy to implement full dynticks, and it's not even complete yet due to the complicated dependencies involved. I see your point. Yes point 4 is a bad idea. Having said the above, the fix that Viresh has proposed along with the nohz_full condition that Frederick added looks to solve this problem. In any case I believe we want Viresh patch since there are other users of deferrable timers that can profit from this. So I'm queueing it. Yeah it solves the problem reported. But on a different note, I was wondering if we could avoid running this timer on every CPU by having a model similar to the time-keeping CPU. Like the cpu-frequency tracking CPU which moves dynamically and serves its purpose at all points in time as long as there are busy CPUs. This will help avoid IPIs to busy CPUs indicating that they modify their timers to delay their respective updates of frequency, each time one of the CPUs does it on their behalf. Viresh? But your below point is very true, we will need to see if cpu frequency can use statistics about cpu load from the scheduler and avoid having to re-calculate it. Let me take a look at this. But just a thought on if there is scope to improve this part of the cpufreq code. What do you all think? I fear I don't know the problem well enough to display any serious advice. It depends what kind of measurement is needed. For example, isn't there some loads statistics that are already available from the scheduler that you could reuse? The scheduler alone takes gazillions of different loads and power statistics taken in interesting path such as the tick or sched switches. Aren't there some read-only metrics that could be interesting? Thanks Regards Preeti U
rtl8821ae.
On Sun, Feb 02, 2014 at 03:41:27AM -0800, scan-ad...@coverity.com wrote: Please find the latest report on new defect(s) introduced to Linux found with Coverity Scan. Defect(s) Reported-by: Coverity Scan Showing 20 of 83 defect(s) Ugh, this is even worse than the usual realtek drivers. (With the exception of rtl8188eu) All 83 of those new defects came from this new driver, and while there's a bunch of who cares type things in there, there's a load of stuff that needs fixing a lot more urgently than CodingStyle issues or anything else in the TODO for that driver. A bigger problem though, is what is the plan for these realtek drivers ? They've been in staging forever. rtl8187se has been there for _five_ years with no indication it's ever getting promoted to first class status. The git logs are littered mostly with CodingStyle cleanups, sparse cleanups and such, meanwhile for five years they've had out of bounds reads, overflows, and such for this whole time. Even worse, when one of the drivers gets fixes for actual problems like this, they never make it back to Realtek, who clone the same old shitty driver they shipped last time, and reintroduce new variants of the same damn bugs, and then we import the new turd into staging and start all over again. I get the whole a shit driver is better than no driver, but there's no discernable effort to ever improve this pile, just to keep adding to it. Dave -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCHv2] usb: move hub init and LED blink work to power efficient workqueue
On Sat, 1 Feb 2014, Zoran Markovic wrote: From: Shaibal Dutta shaibal.du...@broadcom.com Allow the scheduler to select the best CPU to handle hub initalization and LED blinking work. This extends idle residency times on idle CPUs and conserves power. This functionality is enabled when CONFIG_WQ_POWER_EFFICIENT is selected. Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Cc: Alan Stern st...@rowland.harvard.edu Cc: Sarah Sharp sarah.a.sh...@linux.intel.com Cc: Xenia Ragiadakou burzalod...@gmail.com Cc: Julius Werner jwer...@chromium.org Cc: Krzysztof Mazur krzys...@podlesie.net Cc: Matthias Beyer m...@beyermatthias.de Cc: Dan Williams dan.j.willi...@intel.com Cc: Mathias Nyman mathias.ny...@linux.intel.com Cc: Thomas Pugliese thomas.pugli...@gmail.com Signed-off-by: Shaibal Dutta shaibal.du...@broadcom.com [zoran.marko...@linaro.org: Rebased to latest kernel. Added commit message. Changed reference from system to power efficient workqueue for LEDs in check_highspeed() and hub_port_connect_change().] Signed-off-by: Zoran Markovic zoran.marko...@linaro.org Acked-off-by: Alan Stern st...@rowland.harvard.edu Is there some reason why schedule_delayed_work() doesn't use the power-efficient work queue by default? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 02/23] drm/i2c: tda998x: check more I/O errors
On Sat, Jan 25, 2014 at 06:14:45PM +0100, Jean-Francois Moine wrote:
This patch adds more error checking inn I2C I/O functions.
In case of I/O error, this permits to avoid writing in bad controller
pages, a bad chipset detection or looping when getting the EDID.
I've just looked at this again, and spotted something:
-static uint8_t
+static int
reg_read(struct tda998x_priv *priv, uint16_t reg)
{
uint8_t val = 0;
- reg_read_range(priv, reg, val, sizeof(val));
+ int ret;
+
+ ret = reg_read_range(priv, reg, val, sizeof(val));
+ if (ret 0)
+ return ret;
So yes, this can return negative numbers.
@@ -1158,8 +1184,11 @@ tda998x_encoder_init(struct i2c_client *client,
tda998x_reset(priv);
/* read version: */
- priv-rev = reg_read(priv, REG_VERSION_LSB) |
- reg_read(priv, REG_VERSION_MSB) 8;
+ ret = reg_read(priv, REG_VERSION_LSB) |
+ (reg_read(priv, REG_VERSION_MSB) 8);
+ if (ret 0)
+ goto fail;
+ priv-rev = ret;
Two issues here:
1. The additional parens are /really/ not required.
2. What if reg_read(priv, REG_VERSION_MSB) returns a negative number?
If we're going to the extent of attempting to make the read/write
functions return errors, we should at least handle errors generated
by them properly, otherwise it's pointless making them return errors.
ret = reg_read(priv, REG_VERSION_LSB);
if (ret 0)
goto fail;
priv-rev = ret;
ret = reg_read(priv, REG_VERSION_MSB);
if (ret 0)
goto fail;
priv-rev |= ret 8;
If you want it to look slightly nicer:
int rev_lo, rev_hi;
rev_lo = reg_read(priv, REG_VERSION_LSB);
rev_hi = reg_read(priv, REG_VERSION_MSB);
if (rev_lo 0 || rev_hi 0) {
ret = rev_lo 0 ? rev_lo : rev_hi;
goto fail;
}
priv-rev = rev_lo | rev_hi 8;
I'm happy to commit such a change after this patch to clean it up, or if
you want to regenerate your patch 2 and post /just/ that incorporating
this change.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was up to 13.2Mbit.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 09/23] drm/i2c: tda998x: don't read write-only registers
On Sat, Jan 25, 2014 at 06:14:42PM +0100, Jean-Francois Moine wrote: This patch takes care of the write-only registers of the tda998x. The registers SOFTRESET, TBG_CNTRL_0 and TBG_CNTRL_1 have all bits cleared after reset, so, they may be fully re-written. The register MAT_CONTRL is set to MAT_CONTRL_MAT_BP | MAT_CONTRL_MAT_SC(1) after reset, so, it may be fully set again to this value. I said in v3 of this patch, which seems to remain unaddressed: /* must be last register set: */ - reg_clear(priv, REG_TBG_CNTRL_0, TBG_CNTRL_0_SYNC_ONCE); + reg_write(priv, REG_TBG_CNTRL_0, 0); Register changes which have a potential effect shouldn't be part of a patch which is really only trying to avoid reading from write only registers. This could be a potential functional change - and it's probably one which Rob Clark should at least be made aware of. As I commented last time, when you're changing register values in an otherwise innocuous patch, you should comment about them in the patch description. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/8] memcg, slab: never try to merge memcg caches
Suppose we are creating memcg cache A that could be merged with cache B
of the same memcg. Since any memcg cache has the same parameters as its
parent cache, parent caches PA and PB of memcg caches A and B must be
mergeable too. That means PA was merged with PB on creation or vice
versa, i.e. PA = PB. From that it follows that A = B, and we couldn't
even try to create cache B, because it already exists - a contradiction.
So let's remove unused code responsible for merging memcg caches.
Signed-off-by: Vladimir Davydov vdavy...@parallels.com
---
mm/slab.h| 21 -
mm/slab_common.c |8 +---
mm/slub.c| 19 +--
3 files changed, 18 insertions(+), 30 deletions(-)
diff --git a/mm/slab.h b/mm/slab.h
index 8184a7cde272..3045316b7c9d 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -55,12 +55,12 @@ extern void create_boot_cache(struct kmem_cache *, const
char *name,
struct mem_cgroup;
#ifdef CONFIG_SLUB
struct kmem_cache *
-__kmem_cache_alias(struct mem_cgroup *memcg, const char *name, size_t size,
- size_t align, unsigned long flags, void (*ctor)(void *));
+__kmem_cache_alias(const char *name, size_t size, size_t align,
+ unsigned long flags, void (*ctor)(void *));
#else
static inline struct kmem_cache *
-__kmem_cache_alias(struct mem_cgroup *memcg, const char *name, size_t size,
- size_t align, unsigned long flags, void (*ctor)(void *))
+__kmem_cache_alias(const char *name, size_t size, size_t align,
+ unsigned long flags, void (*ctor)(void *))
{ return NULL; }
#endif
@@ -119,13 +119,6 @@ static inline bool is_root_cache(struct kmem_cache *s)
return !s-memcg_params || s-memcg_params-is_root_cache;
}
-static inline bool cache_match_memcg(struct kmem_cache *cachep,
-struct mem_cgroup *memcg)
-{
- return (is_root_cache(cachep) !memcg) ||
- (cachep-memcg_params-memcg == memcg);
-}
-
static inline void memcg_bind_pages(struct kmem_cache *s, int order)
{
if (!is_root_cache(s))
@@ -204,12 +197,6 @@ static inline bool is_root_cache(struct kmem_cache *s)
return true;
}
-static inline bool cache_match_memcg(struct kmem_cache *cachep,
-struct mem_cgroup *memcg)
-{
- return true;
-}
-
static inline void memcg_bind_pages(struct kmem_cache *s, int order)
{
}
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 152d9b118b7a..a75834bb966d 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -200,9 +200,11 @@ kmem_cache_create_memcg(struct mem_cgroup *memcg, const
char *name, size_t size,
*/
flags = CACHE_CREATE_MASK;
- s = __kmem_cache_alias(memcg, name, size, align, flags, ctor);
- if (s)
- goto out_unlock;
+ if (!memcg) {
+ s = __kmem_cache_alias(name, size, align, flags, ctor);
+ if (s)
+ goto out_unlock;
+ }
err = -ENOMEM;
s = kmem_cache_zalloc(kmem_cache, GFP_KERNEL);
diff --git a/mm/slub.c b/mm/slub.c
index 2b1a6970e46f..962abfdfde06 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3686,9 +3686,8 @@ static int slab_unmergeable(struct kmem_cache *s)
return 0;
}
-static struct kmem_cache *find_mergeable(struct mem_cgroup *memcg, size_t size,
- size_t align, unsigned long flags, const char *name,
- void (*ctor)(void *))
+static struct kmem_cache *find_mergeable(size_t size, size_t align,
+ unsigned long flags, const char *name, void (*ctor)(void *))
{
struct kmem_cache *s;
@@ -3707,11 +3706,14 @@ static struct kmem_cache *find_mergeable(struct
mem_cgroup *memcg, size_t size,
if (slab_unmergeable(s))
continue;
+ if (!is_root_cache(s))
+ continue;
+
if (size s-size)
continue;
if ((flags SLUB_MERGE_SAME) != (s-flags SLUB_MERGE_SAME))
- continue;
+ continue;
/*
* Check if alignment is compatible.
* Courtesy of Adrian Drzewiecki
@@ -3722,21 +3724,18 @@ static struct kmem_cache *find_mergeable(struct
mem_cgroup *memcg, size_t size,
if (s-size - size = sizeof(void *))
continue;
- if (!cache_match_memcg(s, memcg))
- continue;
-
return s;
}
return NULL;
}
struct kmem_cache *
-__kmem_cache_alias(struct mem_cgroup *memcg, const char *name, size_t size,
- size_t align, unsigned long flags, void (*ctor)(void *))
+__kmem_cache_alias(const char *name, size_t size, size_t align,
+ unsigned long flags, void (*ctor)(void *))
{
struct kmem_cache *s;
- s = find_mergeable(memcg,
[PATCH 2/8] memcg, slab: remove cgroup name from memcg cache names
The cgroup name is not informative at all in case the cgroup hierarchy
is not flat. Besides, we can always find the memcg a particular cache
belongs to by its kmemcg id, which is now exported via memory.kmem.id
cgroup fs file for each memcg.
So let's remove the cgroup name part from kmem caches names - it will
greatly simplify the call paths and make the code look clearer.
Signed-off-by: Vladimir Davydov vdavy...@parallels.com
---
mm/memcontrol.c | 63 +-
mm/slab_common.c |6 +-
2 files changed, 20 insertions(+), 49 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 91d242707404..3351c5b5486d 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3405,44 +3405,6 @@ void mem_cgroup_destroy_cache(struct kmem_cache *cachep)
schedule_work(cachep-memcg_params-destroy);
}
-static struct kmem_cache *memcg_create_kmem_cache(struct mem_cgroup *memcg,
- struct kmem_cache *s)
-{
- struct kmem_cache *new = NULL;
- static char *tmp_name = NULL;
- static DEFINE_MUTEX(mutex); /* protects tmp_name */
-
- BUG_ON(!memcg_can_account_kmem(memcg));
-
- mutex_lock(mutex);
- /*
-* kmem_cache_create_memcg duplicates the given name and
-* cgroup_name for this name requires RCU context.
-* This static temporary buffer is used to prevent from
-* pointless shortliving allocation.
-*/
- if (!tmp_name) {
- tmp_name = kmalloc(PATH_MAX, GFP_KERNEL);
- if (!tmp_name)
- goto out;
- }
-
- rcu_read_lock();
- snprintf(tmp_name, PATH_MAX, %s(%d:%s), s-name,
-memcg_cache_id(memcg), cgroup_name(memcg-css.cgroup));
- rcu_read_unlock();
-
- new = kmem_cache_create_memcg(memcg, tmp_name, s-object_size, s-align,
- (s-flags ~SLAB_PANIC), s-ctor, s);
- if (new)
- new-allocflags |= __GFP_KMEMCG;
- else
- new = s;
-out:
- mutex_unlock(mutex);
- return new;
-}
-
void kmem_cache_destroy_memcg_children(struct kmem_cache *s)
{
struct kmem_cache *c;
@@ -3489,12 +3451,6 @@ void kmem_cache_destroy_memcg_children(struct kmem_cache
*s)
mutex_unlock(activate_kmem_mutex);
}
-struct create_work {
- struct mem_cgroup *memcg;
- struct kmem_cache *cachep;
- struct work_struct work;
-};
-
static void mem_cgroup_destroy_all_caches(struct mem_cgroup *memcg)
{
struct kmem_cache *cachep;
@@ -3512,13 +3468,24 @@ static void mem_cgroup_destroy_all_caches(struct
mem_cgroup *memcg)
mutex_unlock(memcg-slab_caches_mutex);
}
+struct create_work {
+ struct mem_cgroup *memcg;
+ struct kmem_cache *cachep;
+ struct work_struct work;
+};
+
static void memcg_create_cache_work_func(struct work_struct *w)
{
- struct create_work *cw;
+ struct create_work *cw = container_of(w, struct create_work, work);
+ struct mem_cgroup *memcg = cw-memcg;
+ struct kmem_cache *s = cw-cachep;
+ struct kmem_cache *new;
- cw = container_of(w, struct create_work, work);
- memcg_create_kmem_cache(cw-memcg, cw-cachep);
- css_put(cw-memcg-css);
+ new = kmem_cache_create_memcg(memcg, s-name, s-object_size, s-align,
+ (s-flags ~SLAB_PANIC), s-ctor, s);
+ if (new)
+ new-allocflags |= __GFP_KMEMCG;
+ css_put(memcg-css);
kfree(cw);
}
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 1ec3c619ba04..152d9b118b7a 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -213,7 +213,11 @@ kmem_cache_create_memcg(struct mem_cgroup *memcg, const
char *name, size_t size,
s-align = calculate_alignment(flags, align, size);
s-ctor = ctor;
- s-name = kstrdup(name, GFP_KERNEL);
+ if (!memcg)
+ s-name = kstrdup(name, GFP_KERNEL);
+ else
+ s-name = kasprintf(GFP_KERNEL, %s:%d,
+ name, memcg_cache_id(memcg));
if (!s-name)
goto out_free_cache;
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 5/8] slub: adjust memcg caches when creating cache alias
Otherwise, kzalloc() called from a memcg won't clear the whole object.
Signed-off-by: Vladimir Davydov vdavy...@parallels.com
---
mm/slub.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/mm/slub.c b/mm/slub.c
index 962abfdfde06..a33d88afb61d 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3737,7 +3737,11 @@ __kmem_cache_alias(const char *name, size_t size, size_t
align,
s = find_mergeable(size, align, flags, name, ctor);
if (s) {
+ int i;
+ struct kmem_cache *c;
+
s-refcount++;
+
/*
* Adjust the object sizes so that we clear
* the complete object on kzalloc.
@@ -3745,6 +3749,16 @@ __kmem_cache_alias(const char *name, size_t size, size_t
align,
s-object_size = max(s-object_size, (int)size);
s-inuse = max_t(int, s-inuse, ALIGN(size, sizeof(void *)));
+ BUG_ON(!is_root_cache(s));
+ for_each_memcg_cache_index(i) {
+ c = cache_from_memcg_idx(s, i);
+ if (!c)
+ continue;
+ c-object_size = s-object_size;
+ c-inuse = max_t(int, c-inuse,
+ALIGN(size, sizeof(void *)));
+ }
+
if (sysfs_slab_alias(s, name)) {
s-refcount--;
s = NULL;
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 6/8] slub: rework sysfs layout for memcg caches
Currently, we try to arrange sysfs entries for memcg caches in the same
manner as for global caches. Apart from turning /sys/kernel/slab into a
mess when there are a lot of kmem-active memcgs created, it actually
does not work properly - we won't create more than one link to a memcg
cache in case its parent is merged with another cache. For instance, if
A is a root cache merged with another root cache B, we will have the
following sysfs setup:
X
A - X
B - X
where X is some unique id (see create_unique_id()). Now if memcgs M and
N start to allocate from cache A (or B, which is the same), we will get:
X
X:M
X:N
A - X
B - X
A:M - X:M
A:N - X:N
Since B is an alias for A, we won't get entries B:M and B:N, which is
confusing.
It is more logical to have entries for memcg caches under the
corresponding root cache's sysfs directory. This would allow us to keep
sysfs layout clean, and avoid such inconsistencies like one described
above.
This patch does the trick. It creates a cgroup kset in each root cache
kobject to keep its children caches there.
Signed-off-by: Vladimir Davydov vdavy...@parallels.com
---
include/linux/slub_def.h |3 ++
mm/slub.c| 88 +-
2 files changed, 74 insertions(+), 17 deletions(-)
diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h
index f56bfa9e4526..f2f7398848cf 100644
--- a/include/linux/slub_def.h
+++ b/include/linux/slub_def.h
@@ -87,6 +87,9 @@ struct kmem_cache {
#ifdef CONFIG_MEMCG_KMEM
struct memcg_cache_params *memcg_params;
int max_attr_size; /* for propagation, maximum size of a stored attr */
+#ifdef CONFIG_SYSFS
+ struct kset *memcg_kset;
+#endif
#endif
#ifdef CONFIG_NUMA
diff --git a/mm/slub.c b/mm/slub.c
index a33d88afb61d..c3cf0129eda5 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -5125,6 +5125,44 @@ static const struct kset_uevent_ops slab_uevent_ops = {
static struct kset *slab_kset;
+#ifdef CONFIG_MEMCG_KMEM
+static inline struct kset *cache_kset(struct kmem_cache *s)
+{
+ if (is_root_cache(s))
+ return slab_kset;
+ return s-memcg_params-root_cache-memcg_kset;
+}
+
+static int create_cache_memcg_kset(struct kmem_cache *s)
+{
+ if (!is_root_cache(s))
+ return 0;
+ s-memcg_kset = kset_create_and_add(cgroup, NULL, s-kobj);
+ if (!s-memcg_kset)
+ return -ENOMEM;
+ return 0;
+}
+
+static void destroy_cache_memcg_kset(struct kmem_cache *s)
+{
+ kset_unregister(s-memcg_kset);
+}
+#else
+static inline struct kset *cache_kset(struct kmem_cache *s)
+{
+ return slab_kset;
+}
+
+static inline int create_cache_memcg_kset(struct kmem_cache *s)
+{
+ return 0;
+}
+
+static inline void destroy_cache_memcg_kset(struct kmem_cache *s)
+{
+}
+#endif /* CONFIG_MEMCG_KMEM */
+
#define ID_STR_LENGTH 64
/* Create a unique string id for a slab cache:
@@ -5136,7 +5174,8 @@ static char *create_unique_id(struct kmem_cache *s)
char *name = kmalloc(ID_STR_LENGTH, GFP_KERNEL);
char *p = name;
- BUG_ON(!name);
+ if (!name)
+ return NULL;
*p++ = ':';
/*
@@ -5160,8 +5199,7 @@ static char *create_unique_id(struct kmem_cache *s)
#ifdef CONFIG_MEMCG_KMEM
if (!is_root_cache(s))
- p += sprintf(p, -%08d,
- memcg_cache_id(s-memcg_params-memcg));
+ p += sprintf(p, :%d, memcg_cache_id(s-memcg_params-memcg));
#endif
BUG_ON(p name + ID_STR_LENGTH - 1);
@@ -5180,7 +5218,8 @@ static int sysfs_slab_add(struct kmem_cache *s)
* This is typically the case for debug situations. In that
* case we can catch duplicate names easily.
*/
- sysfs_remove_link(slab_kset-kobj, s-name);
+ if (is_root_cache(s))
+ sysfs_remove_link(slab_kset-kobj, s-name);
name = s-name;
} else {
/*
@@ -5188,28 +5227,40 @@ static int sysfs_slab_add(struct kmem_cache *s)
* for the symlinks.
*/
name = create_unique_id(s);
+ if (!name)
+ return -ENOMEM;
}
- s-kobj.kset = slab_kset;
+ s-kobj.kset = cache_kset(s);
err = kobject_init_and_add(s-kobj, slab_ktype, NULL, name);
- if (err) {
- kobject_put(s-kobj);
- return err;
- }
+ if (err)
+ goto out_put_kobj;
+
+ err = create_cache_memcg_kset(s);
+ if (err)
+ goto out_del_kobj;
err = sysfs_create_group(s-kobj, slab_attr_group);
- if (err) {
- kobject_del(s-kobj);
- kobject_put(s-kobj);
- return err;
- }
+ if (err)
+ goto out_del_memcg_kset;
+
kobject_uevent(s-kobj, KOBJ_ADD);
- if (!unmergeable) {
+
[PATCH 4/8] memcg, slab: separate memcg vs root cache creation paths
Memcg-awareness turned kmem_cache_create() into a dirty interweaving of
memcg-only and except-for-memcg calls. To clean this up, let's create a
separate function handling memcg caches creation. Although this will
result in the two functions having several hunks of practically the same
code, I guess this is the case when readability fully covers the cost of
code duplication.
Signed-off-by: Vladimir Davydov vdavy...@parallels.com
---
include/linux/memcontrol.h |8 +--
include/linux/slab.h |9 ++-
mm/memcontrol.c| 16 ++
mm/slab_common.c | 133 ++--
4 files changed, 92 insertions(+), 74 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index abd0113b6620..87b8c614798f 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -497,8 +497,8 @@ void __memcg_kmem_commit_charge(struct page *page,
void __memcg_kmem_uncharge_pages(struct page *page, int order);
int memcg_cache_id(struct mem_cgroup *memcg);
-int memcg_alloc_cache_params(struct mem_cgroup *memcg, struct kmem_cache *s,
-struct kmem_cache *root_cache);
+int memcg_alloc_cache_params(struct kmem_cache *s,
+ struct mem_cgroup *memcg, struct kmem_cache *root_cache);
void memcg_free_cache_params(struct kmem_cache *s);
void memcg_register_cache(struct kmem_cache *s);
void memcg_unregister_cache(struct kmem_cache *s);
@@ -641,8 +641,8 @@ static inline int memcg_cache_id(struct mem_cgroup *memcg)
return -1;
}
-static inline int memcg_alloc_cache_params(struct mem_cgroup *memcg,
- struct kmem_cache *s, struct kmem_cache *root_cache)
+static inline int memcg_alloc_cache_params(struct kmem_cache *s,
+ struct mem_cgroup *memcg, struct kmem_cache *root_cache)
{
return 0;
}
diff --git a/include/linux/slab.h b/include/linux/slab.h
index a060142aa5f5..b8a4bad71f57 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -113,11 +113,10 @@ void __init kmem_cache_init(void);
int slab_is_available(void);
struct kmem_cache *kmem_cache_create(const char *, size_t, size_t,
- unsigned long,
- void (*)(void *));
-struct kmem_cache *
-kmem_cache_create_memcg(struct mem_cgroup *, const char *, size_t, size_t,
- unsigned long, void (*)(void *), struct kmem_cache *);
+unsigned long, void (*)(void *));
+#ifdef CONFIG_MEMCG_KMEM
+int kmem_cache_create_memcg(struct mem_cgroup *, struct kmem_cache *);
+#endif
void kmem_cache_destroy(struct kmem_cache *);
int kmem_cache_shrink(struct kmem_cache *);
void kmem_cache_free(struct kmem_cache *, void *);
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 3351c5b5486d..d69c427e106b 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3201,8 +3201,8 @@ int memcg_update_cache_size(struct kmem_cache *s, int
num_groups)
return 0;
}
-int memcg_alloc_cache_params(struct mem_cgroup *memcg, struct kmem_cache *s,
-struct kmem_cache *root_cache)
+int memcg_alloc_cache_params(struct kmem_cache *s,
+ struct mem_cgroup *memcg, struct kmem_cache *root_cache)
{
size_t size;
@@ -3477,15 +3477,9 @@ struct create_work {
static void memcg_create_cache_work_func(struct work_struct *w)
{
struct create_work *cw = container_of(w, struct create_work, work);
- struct mem_cgroup *memcg = cw-memcg;
- struct kmem_cache *s = cw-cachep;
- struct kmem_cache *new;
-
- new = kmem_cache_create_memcg(memcg, s-name, s-object_size, s-align,
- (s-flags ~SLAB_PANIC), s-ctor, s);
- if (new)
- new-allocflags |= __GFP_KMEMCG;
- css_put(memcg-css);
+
+ kmem_cache_create_memcg(cw-memcg, cw-cachep);
+ css_put(cw-memcg-css);
kfree(cw);
}
diff --git a/mm/slab_common.c b/mm/slab_common.c
index a75834bb966d..ade86bcddab9 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -29,8 +29,7 @@ DEFINE_MUTEX(slab_mutex);
struct kmem_cache *kmem_cache;
#ifdef CONFIG_DEBUG_VM
-static int kmem_cache_sanity_check(struct mem_cgroup *memcg, const char *name,
- size_t size)
+static int kmem_cache_sanity_check(const char *name, size_t size)
{
struct kmem_cache *s = NULL;
@@ -57,13 +56,7 @@ static int kmem_cache_sanity_check(struct mem_cgroup *memcg,
const char *name,
}
#if !defined(CONFIG_SLUB) || !defined(CONFIG_SLUB_DEBUG_ON)
- /*
-* For simplicity, we won't check this in the list of memcg
-* caches. We have control over memcg naming, and if there
-* aren't duplicates in the global list, there won't be any
-* duplicates in the memcg lists as well.
-*/
- if (!memcg !strcmp(s-name, name)) {
+
[PATCH 1/8] memcg: export kmemcg cache id via cgroup fs
Per-memcg kmem caches are named as follows:
global-cache-name(cgroup-kmem-id:cgroup-name)
where cgroup-kmem-id is the unique id of the memcg the cache belongs
to, cgroup-name is the relative name of the memcg on the cgroup fs.
Cache names are exposed to userspace for debugging purposes (e.g. via
sysfs in case of slub or via dmesg).
Using relative names makes it impossible in general (in case the cgroup
hierarchy is not flat) to find out which memcg a particular cache
belongs to, because cgroup-kmem-id is not known to the user. Since
using absolute cgroup names would be an overkill, let's fix this by
exporting the id of kmem-active memcg via cgroup fs file
memory.kmem.id.
Signed-off-by: Vladimir Davydov vdavy...@parallels.com
---
mm/memcontrol.c | 12
1 file changed, 12 insertions(+)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 53385cd4e6f0..91d242707404 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3113,6 +3113,14 @@ int memcg_cache_id(struct mem_cgroup *memcg)
return memcg ? memcg-kmemcg_id : -1;
}
+static s64 mem_cgroup_cache_id_read(struct cgroup_subsys_state *css,
+ struct cftype *cft)
+{
+ struct mem_cgroup *memcg = mem_cgroup_from_css(css);
+
+ return memcg_can_account_kmem(memcg) ? memcg_cache_id(memcg) : -1;
+}
+
static size_t memcg_caches_array_size(int num_groups)
{
ssize_t size;
@@ -6301,6 +6309,10 @@ static struct cftype mem_cgroup_files[] = {
#endif
#ifdef CONFIG_MEMCG_KMEM
{
+ .name = kmem.id,
+ .read_s64 = mem_cgroup_cache_id_read,
+ },
+ {
.name = kmem.limit_in_bytes,
.private = MEMFILE_PRIVATE(_KMEM, RES_LIMIT),
.write_string = mem_cgroup_write,
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 7/8] memcg, slab: unregister cache from memcg before starting to destroy it
Currently, memcg_unregister_cache(), which deletes the cache being
destroyed from the memcg_slab_caches list, is called after
__kmem_cache_shutdown() (see kmem_cache_destroy()), which starts to
destroy the cache. As a result, one can access a partially destroyed
cache while traversing a memcg_slab_caches list, which can have deadly
consequences (for instance, cache_show() called for each cache on a
memcg_slab_caches list from mem_cgroup_slabinfo_read() will dereference
pointers to already freed data).
To fix this, let's move memcg_unregister_cache() before the cache
destruction process beginning, issuing memcg_register_cache() on
failure.
Signed-off-by: Vladimir Davydov vdavy...@parallels.com
---
mm/memcontrol.c | 12 ++--
mm/slab_common.c |3 ++-
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index d69c427e106b..69e8726aae4f 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3224,6 +3224,7 @@ int memcg_alloc_cache_params(struct kmem_cache *s,
s-memcg_params-root_cache = root_cache;
INIT_WORK(s-memcg_params-destroy,
kmem_cache_destroy_work_func);
+ css_get(memcg-css);
} else
s-memcg_params-is_root_cache = true;
@@ -3232,6 +3233,10 @@ int memcg_alloc_cache_params(struct kmem_cache *s,
void memcg_free_cache_params(struct kmem_cache *s)
{
+ if (!s-memcg_params)
+ return;
+ if (!s-memcg_params-is_root_cache)
+ css_put(s-memcg_params-memcg-css);
kfree(s-memcg_params);
}
@@ -3254,9 +3259,6 @@ void memcg_register_cache(struct kmem_cache *s)
memcg = s-memcg_params-memcg;
id = memcg_cache_id(memcg);
- css_get(memcg-css);
-
-
/*
* Since readers won't lock (see cache_from_memcg_idx()), we need a
* barrier here to ensure nobody will see the kmem_cache partially
@@ -3305,10 +3307,8 @@ void memcg_unregister_cache(struct kmem_cache *s)
* after removing it from the memcg_slab_caches list, otherwise we can
* fail to convert memcg_params_to_cache() while traversing the list.
*/
- VM_BUG_ON(!root-memcg_params-memcg_caches[id]);
+ VM_BUG_ON(root-memcg_params-memcg_caches[id] != s);
root-memcg_params-memcg_caches[id] = NULL;
-
- css_put(memcg-css);
}
/*
diff --git a/mm/slab_common.c b/mm/slab_common.c
index ade86bcddab9..ea1075e65271 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -312,9 +312,9 @@ void kmem_cache_destroy(struct kmem_cache *s)
s-refcount--;
if (!s-refcount) {
list_del(s-list);
+ memcg_unregister_cache(s);
if (!__kmem_cache_shutdown(s)) {
- memcg_unregister_cache(s);
mutex_unlock(slab_mutex);
if (s-flags SLAB_DESTROY_BY_RCU)
rcu_barrier();
@@ -324,6 +324,7 @@ void kmem_cache_destroy(struct kmem_cache *s)
kmem_cache_free(kmem_cache, s);
} else {
list_add(s-list, slab_caches);
+ memcg_register_cache(s);
mutex_unlock(slab_mutex);
printk(KERN_ERR kmem_cache_destroy %s: Slab cache
still has objects\n,
s-name);
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/8] memcg-vs-slab related fixes, improvements, cleanups
Hi, This patch set mostly cleanups memcg slab caches creation/destruction paths fixing a couple of bugs in the meanwhile. However, it does introduce some functional changes. First, it changes the memcg caches naming convention (see patch 2). Second, it reworks sysfs layout for memcg slub caches (see patch 6). Comments are appreciated. Thanks. Vladimir Davydov (8): memcg: export kmemcg cache id via cgroup fs memcg, slab: remove cgroup name from memcg cache names memcg, slab: never try to merge memcg caches memcg, slab: separate memcg vs root cache creation paths slub: adjust memcg caches when creating cache alias slub: rework sysfs layout for memcg caches memcg, slab: unregister cache from memcg before starting to destroy it memcg, slab: do not destroy children caches if parent has aliases include/linux/memcontrol.h | 13 +-- include/linux/slab.h |9 +- include/linux/slub_def.h |3 + mm/memcontrol.c| 85 +++ mm/slab.h | 36 mm/slab_common.c | 194 mm/slub.c | 121 +-- 7 files changed, 277 insertions(+), 184 deletions(-) -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 8/8] memcg, slab: do not destroy children caches if parent has aliases
Currently we destroy children caches at the very beginning of
kmem_cache_destroy(). This is wrong, because the root cache will not
necessarily be destroyed in the end - if it has aliases (refcount 0),
kmem_cache_destroy() will simply decrement its refcount and return. In
this case, at best we will get a bunch of warnings in dmesg, like this
one:
kmem_cache_destroy kmalloc-32:0: Slab cache still has objects
CPU: 1 PID: 7139 Comm: modprobe Tainted: GB W3.13.0+ #117
Hardware name:
88007d7a6368 880039b07e48 8168cc2e 88007d7a6d68
88007d7a6300 880039b07e68 81175e9f
88007d7a6300 880039b07e98 811b67c7 88003e803c00
Call Trace:
[8168cc2e] dump_stack+0x49/0x5b
[81175e9f] kmem_cache_destroy+0xdf/0xf0
[811b67c7] kmem_cache_destroy_memcg_children+0x97/0xc0
[81175dcf] kmem_cache_destroy+0xf/0xf0
[a0130b21] xfs_mru_cache_uninit+0x21/0x30 [xfs]
[a01893ea] exit_xfs_fs+0x2e/0xc44 [xfs]
[810eeb58] SyS_delete_module+0x198/0x1f0
[816994f9] system_call_fastpath+0x16/0x1b
At worst - if kmem_cache_destroy() will race with an allocation from a
memcg cache - the kernel will panic.
This patch fixes this by moving children caches destruction after the
check if the cache has aliases. Plus, it forbids destroying a root cache
if it still has children caches, because each children cache keeps a
reference to its parent.
Signed-off-by: Vladimir Davydov vdavy...@parallels.com
---
include/linux/memcontrol.h |5 ---
mm/memcontrol.c|2 +-
mm/slab.h | 17 --
mm/slab_common.c | 74 +---
4 files changed, 65 insertions(+), 33 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index 87b8c614798f..69f6b0f84cb4 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -510,7 +510,6 @@ struct kmem_cache *
__memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp);
void mem_cgroup_destroy_cache(struct kmem_cache *cachep);
-void kmem_cache_destroy_memcg_children(struct kmem_cache *s);
/**
* memcg_kmem_newpage_charge: verify if a new kmem allocation is allowed.
@@ -664,10 +663,6 @@ memcg_kmem_get_cache(struct kmem_cache *cachep, gfp_t gfp)
{
return cachep;
}
-
-static inline void kmem_cache_destroy_memcg_children(struct kmem_cache *s)
-{
-}
#endif /* CONFIG_MEMCG_KMEM */
#endif /* _LINUX_MEMCONTROL_H */
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 69e8726aae4f..c3f9afaeef3e 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -3405,7 +3405,7 @@ void mem_cgroup_destroy_cache(struct kmem_cache *cachep)
schedule_work(cachep-memcg_params-destroy);
}
-void kmem_cache_destroy_memcg_children(struct kmem_cache *s)
+void __kmem_cache_destroy_memcg_children(struct kmem_cache *s)
{
struct kmem_cache *c;
int i;
diff --git a/mm/slab.h b/mm/slab.h
index 3045316b7c9d..b5ad968020a3 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -191,7 +191,16 @@ static inline struct kmem_cache *memcg_root_cache(struct
kmem_cache *s)
return s;
return s-memcg_params-root_cache;
}
-#else
+
+extern void __kmem_cache_destroy_memcg_children(struct kmem_cache *s);
+
+static inline void kmem_cache_destroy_memcg_children(struct kmem_cache *s)
+{
+ mutex_unlock(slab_mutex);
+ __kmem_cache_destroy_memcg_children(s);
+ mutex_lock(slab_mutex);
+}
+#else /* !CONFIG_MEMCG_KMEM */
static inline bool is_root_cache(struct kmem_cache *s)
{
return true;
@@ -226,7 +235,11 @@ static inline struct kmem_cache *memcg_root_cache(struct
kmem_cache *s)
{
return s;
}
-#endif
+
+static inline void kmem_cache_destroy_memcg_children(struct kmem_cache *s)
+{
+}
+#endif /* CONFIG_MEMCG_KMEM */
static inline struct kmem_cache *cache_from_obj(struct kmem_cache *s, void *x)
{
diff --git a/mm/slab_common.c b/mm/slab_common.c
index ea1075e65271..f83e0cf939a4 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -302,38 +302,62 @@ out_free_cache:
}
#endif /* CONFIG_MEMCG_KMEM */
-void kmem_cache_destroy(struct kmem_cache *s)
+static bool cache_has_children(struct kmem_cache *s)
{
- /* Destroy all the children caches if we aren't a memcg cache */
- kmem_cache_destroy_memcg_children(s);
+ int i;
+
+ if (!is_root_cache(s))
+ return false;
+ for_each_memcg_cache_index(i) {
+ if (cache_from_memcg_idx(s, i))
+ return true;
+ }
+ return false;
+}
+void kmem_cache_destroy(struct kmem_cache *s)
+{
get_online_cpus();
mutex_lock(slab_mutex);
+
s-refcount--;
- if (!s-refcount) {
- list_del(s-list);
- memcg_unregister_cache(s);
-
- if (!__kmem_cache_shutdown(s)) {
-
Re: [PATCH 6/8] cleanup __vdso_gettimeofday
On Sun, Feb 2, 2014 at 3:27 AM, stef...@seibold.net wrote:
From: Stefani Seibold stef...@seibold.net
This patch do a little cleanup for the __vdso_gettimeofday() function.
It kick out an unneeded ret local variable and makes the code faster if
only the timezone is needed.
Signed-off-by: Stefani Seibold stef...@seibold.net
Acked-by: Andy Lutomirski l...@amacapital.net
---
arch/x86/vdso/vclock_gettime.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/arch/x86/vdso/vclock_gettime.c b/arch/x86/vdso/vclock_gettime.c
index 743f277..bf969a0 100644
--- a/arch/x86/vdso/vclock_gettime.c
+++ b/arch/x86/vdso/vclock_gettime.c
@@ -259,13 +259,12 @@ int clock_gettime(clockid_t, struct timespec *)
notrace int __vdso_gettimeofday(struct timeval *tv, struct timezone *tz)
{
- long ret = VCLOCK_NONE;
-
if (likely(tv != NULL)) {
BUILD_BUG_ON(offsetof(struct timeval, tv_usec) !=
offsetof(struct timespec, tv_nsec) ||
sizeof(*tv) != sizeof(struct timespec));
- ret = do_realtime((struct timespec *)tv);
+ if (do_realtime((struct timespec *)tv) == VCLOCK_NONE)
+ return vdso_fallback_gtod(tv, tz);
tv-tv_usec /= 1000;
}
if (unlikely(tz != NULL)) {
@@ -274,8 +273,6 @@ notrace int __vdso_gettimeofday(struct timeval *tv,
struct timezone *tz)
tz-tz_dsttime = gtod-sys_tz.tz_dsttime;
}
- if (ret == VCLOCK_NONE)
- return vdso_fallback_gtod(tv, tz);
return 0;
}
int gettimeofday(struct timeval *, struct timezone *)
--
1.8.5.3
--
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/8] vclock_gettime.c __vdso_clock_gettime cleanup
On Sun, Feb 2, 2014 at 3:27 AM, stef...@seibold.net wrote:
From: Stefani Seibold stef...@seibold.net
This patch is a small code cleanup for the __vdso_clock_gettime() function.
It removes the unneeded return values from do_monotonic_coarse() and
do_realtime_coarse() and add a fallback label for doing the kernel
gettimeofday() system call.
This is possibly worth benchmarking, but it looks harmless, and
possibly even beneficial, to me. (If the optimizer is in a good mood,
it might do a nicer job with do_monotonic now.)
--Andy
Signed-off-by: Stefani Seibold stef...@seibold.net
---
arch/x86/vdso/vclock_gettime.c | 27 ++-
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/arch/x86/vdso/vclock_gettime.c b/arch/x86/vdso/vclock_gettime.c
index bbc8065..fd074dd 100644
--- a/arch/x86/vdso/vclock_gettime.c
+++ b/arch/x86/vdso/vclock_gettime.c
@@ -209,7 +209,7 @@ notrace static int do_monotonic(struct timespec *ts)
return mode;
}
-notrace static int do_realtime_coarse(struct timespec *ts)
+notrace static void do_realtime_coarse(struct timespec *ts)
{
unsigned long seq;
do {
@@ -217,10 +217,9 @@ notrace static int do_realtime_coarse(struct timespec
*ts)
ts-tv_sec = gtod-wall_time_coarse.tv_sec;
ts-tv_nsec = gtod-wall_time_coarse.tv_nsec;
} while (unlikely(read_seqcount_retry(gtod-seq, seq)));
- return 0;
}
-notrace static int do_monotonic_coarse(struct timespec *ts)
+notrace static void do_monotonic_coarse(struct timespec *ts)
{
unsigned long seq;
do {
@@ -228,30 +227,32 @@ notrace static int do_monotonic_coarse(struct timespec
*ts)
ts-tv_sec = gtod-monotonic_time_coarse.tv_sec;
ts-tv_nsec = gtod-monotonic_time_coarse.tv_nsec;
} while (unlikely(read_seqcount_retry(gtod-seq, seq)));
-
- return 0;
}
notrace int __vdso_clock_gettime(clockid_t clock, struct timespec *ts)
{
- int ret = VCLOCK_NONE;
-
switch (clock) {
case CLOCK_REALTIME:
- ret = do_realtime(ts);
+ if (do_realtime(ts) == VCLOCK_NONE)
+ goto fallback;
break;
case CLOCK_MONOTONIC:
- ret = do_monotonic(ts);
+ if (do_monotonic(ts) == VCLOCK_NONE)
+ goto fallback;
break;
case CLOCK_REALTIME_COARSE:
- return do_realtime_coarse(ts);
+ do_realtime_coarse(ts);
+ break;
case CLOCK_MONOTONIC_COARSE:
- return do_monotonic_coarse(ts);
+ do_monotonic_coarse(ts);
+ break;
+ default:
+ goto fallback;
}
- if (ret == VCLOCK_NONE)
- return vdso_fallback_gettime(clock, ts);
return 0;
+fallback:
+ return vdso_fallback_gettime(clock, ts);
}
int clock_gettime(clockid_t, struct timespec *)
__attribute__((weak, alias(__vdso_clock_gettime)));
--
1.8.5.3
--
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/8] revamp vclock_gettime.c
On Sun, Feb 2, 2014 at 3:27 AM, stef...@seibold.net wrote:
From: Stefani Seibold stef...@seibold.net
This intermediate patch revamps the vclock_gettime.c by moving some functions
around. It is only for spliting purpose, to make whole the 32 bit vdso timer
patch easier to review.
Signed-off-by: Stefani Seibold stef...@seibold.net
Acked-by: Andy Lutomirski l...@amacapital.net
---
arch/x86/vdso/vclock_gettime.c | 85
+-
1 file changed, 42 insertions(+), 43 deletions(-)
diff --git a/arch/x86/vdso/vclock_gettime.c b/arch/x86/vdso/vclock_gettime.c
index eb5d7a5..bbc8065 100644
--- a/arch/x86/vdso/vclock_gettime.c
+++ b/arch/x86/vdso/vclock_gettime.c
@@ -26,41 +26,26 @@
#define gtod (VVAR(vsyscall_gtod_data))
-notrace static cycle_t vread_tsc(void)
+static notrace cycle_t vread_hpet(void)
{
- cycle_t ret;
- u64 last;
-
- /*
-* Empirically, a fence (of type that depends on the CPU)
-* before rdtsc is enough to ensure that rdtsc is ordered
-* with respect to loads. The various CPU manuals are unclear
-* as to whether rdtsc can be reordered with later loads,
-* but no one has ever seen it happen.
-*/
- rdtsc_barrier();
- ret = (cycle_t)vget_cycles();
-
- last = VVAR(vsyscall_gtod_data).clock.cycle_last;
-
- if (likely(ret = last))
- return ret;
+ return readl((const void __iomem *)fix_to_virt(VSYSCALL_HPET) +
HPET_COUNTER);
+}
- /*
-* GCC likes to generate cmov here, but this branch is extremely
-* predictable (it's just a funciton of time and the likely is
-* very likely) and there's a data dependence, so force GCC
-* to generate a branch instead. I don't barrier() because
-* we don't actually need a barrier, and if this function
-* ever gets inlined it will generate worse code.
-*/
- asm volatile ();
- return last;
+notrace static long vdso_fallback_gettime(long clock, struct timespec *ts)
+{
+ long ret;
+ asm(syscall : =a (ret) :
+ 0 (__NR_clock_gettime), D (clock), S (ts) : memory);
+ return ret;
}
-static notrace cycle_t vread_hpet(void)
+notrace static long vdso_fallback_gtod(struct timeval *tv, struct timezone
*tz)
{
- return readl((const void __iomem *)fix_to_virt(VSYSCALL_HPET) +
HPET_COUNTER);
+ long ret;
+
+ asm(syscall : =a (ret) :
+ 0 (__NR_gettimeofday), D (tv), S (tz) : memory);
+ return ret;
}
#ifdef CONFIG_PARAVIRT_CLOCK
@@ -133,23 +118,37 @@ static notrace cycle_t vread_pvclock(int *mode)
}
#endif
-notrace static long vdso_fallback_gettime(long clock, struct timespec *ts)
+notrace static cycle_t vread_tsc(void)
{
- long ret;
- asm(syscall : =a (ret) :
- 0 (__NR_clock_gettime),D (clock), S (ts) : memory);
- return ret;
-}
+ cycle_t ret;
+ u64 last;
-notrace static long vdso_fallback_gtod(struct timeval *tv, struct timezone
*tz)
-{
- long ret;
+ /*
+* Empirically, a fence (of type that depends on the CPU)
+* before rdtsc is enough to ensure that rdtsc is ordered
+* with respect to loads. The various CPU manuals are unclear
+* as to whether rdtsc can be reordered with later loads,
+* but no one has ever seen it happen.
+*/
+ rdtsc_barrier();
+ ret = (cycle_t)vget_cycles();
- asm(syscall : =a (ret) :
- 0 (__NR_gettimeofday), D (tv), S (tz) : memory);
- return ret;
-}
+ last = VVAR(vsyscall_gtod_data).clock.cycle_last;
+ if (likely(ret = last))
+ return ret;
+
+ /*
+* GCC likes to generate cmov here, but this branch is extremely
+* predictable (it's just a funciton of time and the likely is
+* very likely) and there's a data dependence, so force GCC
+* to generate a branch instead. I don't barrier() because
+* we don't actually need a barrier, and if this function
+* ever gets inlined it will generate worse code.
+*/
+ asm volatile ();
+ return last;
+}
notrace static inline u64 vgetsns(int *mode)
{
--
1.8.5.3
--
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/8] replace VVAR(vsyscall_gtod_data) by gtod macro
On Sun, Feb 2, 2014 at 3:27 AM, stef...@seibold.net wrote:
From: Stefani Seibold stef...@seibold.net
There a currently more than 30 users of the gtod macro, so replace the
last VVAR(vsyscall_gtod_data) by gtod macro.
Acked-by: Andy Lutomirski l...@amacapital.net
Signed-off-by: Stefani Seibold stef...@seibold.net
---
arch/x86/vdso/vclock_gettime.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/vdso/vclock_gettime.c b/arch/x86/vdso/vclock_gettime.c
index fd074dd..743f277 100644
--- a/arch/x86/vdso/vclock_gettime.c
+++ b/arch/x86/vdso/vclock_gettime.c
@@ -109,7 +109,7 @@ static notrace cycle_t vread_pvclock(int *mode)
*mode = VCLOCK_NONE;
/* refer to tsc.c read_tsc() comment for rationale */
- last = VVAR(vsyscall_gtod_data).clock.cycle_last;
+ last = gtod-clock.cycle_last;
if (likely(ret = last))
return ret;
@@ -133,7 +133,7 @@ notrace static cycle_t vread_tsc(void)
rdtsc_barrier();
ret = (cycle_t)vget_cycles();
- last = VVAR(vsyscall_gtod_data).clock.cycle_last;
+ last = gtod-clock.cycle_last;
if (likely(ret = last))
return ret;
@@ -288,7 +288,7 @@ int gettimeofday(struct timeval *, struct timezone *)
notrace time_t __vdso_time(time_t *t)
{
/* This is atomic on x86_64 so we don't need any locks. */
- time_t result = ACCESS_ONCE(VVAR(vsyscall_gtod_data).wall_time_sec);
+ time_t result = ACCESS_ONCE(gtod-wall_time_sec);
if (t)
*t = result;
--
1.8.5.3
--
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 7/8] Add 32 bit VDSO time support for 32 bit kernel
On Sun, Feb 2, 2014 at 3:27 AM, stef...@seibold.net wrote: From: Stefani Seibold stef...@seibold.net This patch add the time support for 32 bit a VDSO to a 32 bit kernel. [...] Can you address the review comments from last time around? For example, this still seems to have redundant vvar and hpet mappings, it doesn't use the VVAR macro, it moves the 32-bit compat vDSO, etc. Also, the usual convention is to use [PATCH v2 3/8], etc, as subjects, to keep separate versions separate. --Andy -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 1/6] ACPI / hotplug: Fix theoretical race in acpi_hotplug_notify_cb()
On Sunday, February 02, 2014 01:54:02 AM Rafael J. Wysocki wrote:
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
There is a slight possibility for the ACPI device object pointed to
by adev in acpi_hotplug_notify_cb() to become invalid between the
acpi_bus_get_device() that it comes from and the subsequent get_device().
Namely, if acpi_scan_drop_device() runs concurrently with respect to
acpi_hotplug_notify_cb() and acpi_device_del_list is not empty,
acpi_device_del_work_fn() may delete the device object in question
without waiting for the ACPI events workqueue to drain, which very
well may happen right after a successful execution of
acpi_bus_get_device() in acpi_hotplug_notify_cb().
To prevent that from happening, run acpi_bus_get_device() and the
subsequent get_device() in acpi_hotplug_notify_cb() under
acpi_device_del_lock, so that the deletion of the given device
object cannot be queued up by acpi_scan_drop_device() between the
two.
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
drivers/acpi/scan.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
Index: linux-pm/drivers/acpi/scan.c
===
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -41,6 +41,8 @@ static DEFINE_MUTEX(acpi_scan_lock);
static LIST_HEAD(acpi_scan_handlers_list);
DEFINE_MUTEX(acpi_device_lock);
LIST_HEAD(acpi_wakeup_device_list);
+static LIST_HEAD(acpi_device_del_list);
+static DEFINE_MUTEX(acpi_device_del_lock);
struct acpi_device_bus_id{
char bus_id[15];
@@ -488,9 +490,6 @@ static void acpi_hotplug_notify_cb(acpi_
struct acpi_device *adev;
acpi_status status;
- if (acpi_bus_get_device(handle, adev))
- goto err_out;
-
switch (type) {
case ACPI_NOTIFY_BUS_CHECK:
acpi_handle_debug(handle, ACPI_NOTIFY_BUS_CHECK event\n);
@@ -512,7 +511,13 @@ static void acpi_hotplug_notify_cb(acpi_
/* non-hotplug event; possibly handled by other handler */
return;
}
+ mutex_lock(acpi_device_del_lock);
+ if (acpi_bus_get_device(handle, adev)) {
+ mutex_unlock(acpi_device_del_lock);
+ goto err_out;
+ }
get_device(adev-dev);
+ mutex_unlock(acpi_device_del_lock);
status = acpi_hotplug_execute(acpi_device_hotplug, adev, type);
if (ACPI_SUCCESS(status))
return;
Well, that would have been good if it hand't been broken. :-(
acpi_scan_drop_device() which acquires acpi_device_del_lock is called under
the ACPICA's namespace mutex and acpi_bus_get_device() above acquires that
mutex, so this leads to a classical ABBA deadlock scenario. Bummer.
And I haven't been able to convince myself that what we're doing in
acpi_hotplug_notify_cb() is actually safe without any locking. Not to
mention acpi_bus_notify() for that matter. Moreover, the *only* safe
way to do that I'm seeing at the moment is to call the get_device()
under the ACPICA's namespace mutex, before it is released in
acpi_get_data().
Of course, ACPICA will need to be modified slightly for that to be
possible (sorry, Bob), but at least that *should* work, so I have a
new version of this patchset doing just that. I'll send it out
shortly.
Thanks!
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Do we really need curr_target in signal_struct ?
On Sat, Feb 1, 2014 at 10:51 PM, Oleg Nesterov o...@redhat.com wrote: On 02/01, Rakib Mullick wrote: On Thu, Jan 30, 2014 at 1:02 PM, Rakib Mullick rakib.mull...@gmail.com wrote: On Thu, Jan 30, 2014 at 12:32 AM, Oleg Nesterov o...@redhat.com wrote: On 01/29, Rakib Mullick wrote: Are you thinking that , since things are not broken, then we shouldn't try to do anything? Hmm. No. I am thinking that, since you misunderstood the purpose of -curr_target, I should probably try to argue with your patch which blindly removes this optimization ? Since the optimization (usages of -curr_target) isn't perfect, so there's chance of being misunderstood. This optimization is misleading too (I think), cause curr_target don't have anything to do with wants_signal() can't understand... curr_target is obviously connected to wants_signal() ? No. I meant, curr_target doesn't makes sure that it really wants the signal, it's likely choice. As I already said it caches the last wants_signal(t) thread? Yes, you said. But, gets messed up at exit path, not useful everytime. If fails, p gets checked twice. and -curr_target is used only for this optimization and to get this optimization needs to maintain it properly, and this maintenance does have cost and if we don't get benefited too much, then it doesn't worth it (my pov). but you need to prove this somehow. Right, I'll try, I'll do it as per my understanding and everyone might not agree. I took a look and found that using while_each_thread() can make things better than current. Why? using while_each_thread() we can start thread traversing from p, which is a likely pick and also gets away from redundant checking of p. What do you think? The patch is technically wrong, a group-wide signal doesn't check all threads after this change. If group is empty, we don't need to check other than t. And I do not understand why complete_signal() still updates -curr_target. Didn't want to remove it blindly :-/ Plus thread_group_empty() doesn't buy too much if we use while_each_thread(). But this doesn't really matter, easy to fix. Rakib. It is not that I like -curr_target very much. But let me repeat, if you want to remove this optimization you need to explain why it doesn't make sense. You claim that this can make things better without any argument. Well, I had other way of looking at it and didn't find any real usages of -curr_target and got confused though the code comment in curr_target. As for me - I simply do not know. This logic is very old, I am not even sure that the current usage of -curr_signal matches the original intent. But it can obviously help in some cases, and you need to explain why we do not care. Actually, this is exactly what i wanted to know. What is the purpose of -curr_signal, *do we really need it?* If I knew or seen any reason I'd have never asked this question. You guys knows this domain better than me, that's why I asked. Git log didn't help much, neither it's documentation . As a result, I asked and thought about cleaning it up, (as i rarely do if it meets my capability of course), so appended a sort of rough patch. So I won't argue if you submit the technically correct patch, but you need to convince Andrew or Ingo to take it. I think the right change in complete_signal() is something like below. It can be even simpler and use the single do/while loop, but then we need to check group inside that loop. With the change below -curr_target can be simply removed. I'll try to make points to convince Andrew or Ingo, I'll try to show up my points, and the rest will be upon them. And here's what i think about -curr_target removal (as per my understanding): a) -curr_target is only might be useful in group wide case. b) Usually signals are sent particularly to a thread so -curr_signal doesn't makes sense. c) If -curr_signal pointed thread is killed, curr_signal points to the next thread, but nothing can make sure that it's a right pick. d) also current in implementation p is checked twice. e) One use case of -curr_signal is, it always points to the lastly created thread, as it's a better candidate for want_signal cause newly created thread don't usually have any signal pending. We can acheive the same without -curr_signal by traversing thread group from the lastly created thread. So, this is what I think. Let me know if these reason's looks reasonable to you, cause before Ingo or Andrew taking it, it requires your ack. Thanks, Rakib. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 4/7] ACPI / hotplug / PCI: Consolidate ACPIPHP with ACPI core hotplug
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
The ACPI-based PCI hotplug (ACPIPHP) code currently attaches its
hotplug context objects directly to ACPI namespace nodes representing
hotplug devices. However, after recent changes causing struct
acpi_device to be created for every namespace node representing a
device (regardless of its status), that is not necessary any more.
Moreover, it's vulnerable to a theoretical issue that the ACPI
handle passed in the context between handle_hotplug_event() and
hotplug_event_work() may become invalid in the meantime (as a
result of a concurrent table unload).
In principle, this issue might be addressed by adding a non-empty
release handler for ACPIPHP hotplug context objects analogous to
acpi_scan_drop_device(), but that would duplicate the code in that
function and in acpi_device_del_work_fn(). For this reason, it's
better to modify ACPIPHP to attach its device hotplug contexts to
struct device objects representing hotplug devices and make it
use acpi_hotplug_notify_cb() as its notify handler. At the same
time, acpi_device_hotplug() can be modified to dispatch the new
.hp.event() callback pointing to acpiphp_hotplug_event() from ACPI
device objects associated with PCI devices and use the generic
ACPI device hotplug code for device objects with scan handlers
attached to them.
This allows the existing code duplication between ACPIPHP and the
ACPI core to be reduced too and makes further ACPI-based device
hotplug consolidation possible.
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
drivers/acpi/scan.c| 106 +---
drivers/pci/hotplug/acpiphp.h |9 +-
drivers/pci/hotplug/acpiphp_glue.c | 136 +++--
include/acpi/acpi_bus.h| 22 +
4 files changed, 136 insertions(+), 137 deletions(-)
Index: linux-pm/drivers/acpi/scan.c
===
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -450,43 +450,61 @@ static int acpi_scan_bus_check(struct ac
return 0;
}
+static int acpi_generic_hotplug_event(struct acpi_device *adev, u32 type)
+{
+ switch (type) {
+ case ACPI_NOTIFY_BUS_CHECK:
+ return acpi_scan_bus_check(adev);
+ case ACPI_NOTIFY_DEVICE_CHECK:
+ return acpi_scan_device_check(adev);
+ case ACPI_NOTIFY_EJECT_REQUEST:
+ case ACPI_OST_EC_OSPM_EJECT:
+ return acpi_scan_hot_remove(adev);
+ }
+ return -EINVAL;
+}
+
static void acpi_device_hotplug(void *data, u32 src)
{
u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
struct acpi_device *adev = data;
- int error;
+ int error = -ENODEV;
lock_device_hotplug();
mutex_lock(acpi_scan_lock);
/*
* The device object's ACPI handle cannot become invalid as long as we
-* are holding acpi_scan_lock, but it may have become invalid before
+* are holding acpi_scan_lock, but it might have become invalid before
* that lock was acquired.
*/
if (adev-handle == INVALID_ACPI_HANDLE)
- goto out;
+ goto err_out;
- switch (src) {
- case ACPI_NOTIFY_BUS_CHECK:
- error = acpi_scan_bus_check(adev);
- break;
- case ACPI_NOTIFY_DEVICE_CHECK:
- error = acpi_scan_device_check(adev);
- break;
- case ACPI_NOTIFY_EJECT_REQUEST:
- case ACPI_OST_EC_OSPM_EJECT:
- error = acpi_scan_hot_remove(adev);
- break;
- default:
- error = -EINVAL;
- break;
+ if (adev-handler) {
+ error = acpi_generic_hotplug_event(adev, src);
+ } else {
+ int (*event)(struct acpi_device *, u32);
+
+ acpi_lock_hp_context();
+ event = adev-hp ? adev-hp-event : NULL;
+ acpi_unlock_hp_context();
+ /*
+* There may be additional notify handlers for device objects
+* without the .event() callback, so ignore them here.
+*/
+ if (event)
+ error = event(adev, src);
+ else
+ goto out;
}
if (!error)
ost_code = ACPI_OST_SC_SUCCESS;
- out:
+ err_out:
acpi_evaluate_hotplug_ost(adev-handle, src, ost_code, NULL);
+
+ out:
acpi_bus_put_acpi_device(adev);
mutex_unlock(acpi_scan_lock);
unlock_device_hotplug();
@@ -494,8 +512,8 @@ static void acpi_device_hotplug(void *da
static void acpi_hotplug_notify_cb(acpi_handle handle, u32 type, void *data)
{
- u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
struct acpi_scan_handler *handler = data;
+ u32 ost_code = ACPI_OST_SC_SUCCESS;
struct acpi_device *adev;
acpi_status status;
[PATCH v3 7/7] ACPI / hotplug / PCI: Hotplug notifications from acpi_bus_notify()
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
Since acpi_bus_notify() is executed on all notifications for all
devices anyway, make it execute acpi_device_hotplug() for all
hotplug events instead of installing notify handlers pointing to
the same function for all hotplug devices.
This change reduces both the size and complexity of ACPI-based device
hotplug code. Moreover, since acpi_device_hotplug() only does
significant things for devices that have either an ACPI scan handler,
or a hotplug context with .eject() defined, and those devices
had notify handlers pointing to acpi_hotplug_notify_cb() installed
before anyway, this modification shouldn't change functionality.
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
drivers/acpi/bus.c | 60 +-
drivers/acpi/internal.h|1
drivers/acpi/scan.c| 100 -
drivers/pci/hotplug/acpiphp.h |1
drivers/pci/hotplug/acpiphp_glue.c | 16 ++---
include/acpi/acpi_bus.h|2
6 files changed, 46 insertions(+), 134 deletions(-)
Index: linux-pm/drivers/acpi/bus.c
===
--- linux-pm.orig/drivers/acpi/bus.c
+++ linux-pm/drivers/acpi/bus.c
@@ -340,62 +340,76 @@ static void acpi_bus_osc_support(void)
*/
static void acpi_bus_notify(acpi_handle handle, u32 type, void *data)
{
- struct acpi_device *device = NULL;
+ struct acpi_device *adev;
struct acpi_driver *driver;
-
- ACPI_DEBUG_PRINT((ACPI_DB_INFO, Notification %#02x to handle %p\n,
- type, handle));
+ acpi_status status;
+ u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
switch (type) {
-
case ACPI_NOTIFY_BUS_CHECK:
- /* TBD */
+ acpi_handle_debug(handle, ACPI_NOTIFY_BUS_CHECK event\n);
break;
case ACPI_NOTIFY_DEVICE_CHECK:
- /* TBD */
+ acpi_handle_debug(handle, ACPI_NOTIFY_DEVICE_CHECK event\n);
break;
case ACPI_NOTIFY_DEVICE_WAKE:
- /* TBD */
+ acpi_handle_debug(handle, ACPI_NOTIFY_DEVICE_WAKE event\n);
break;
case ACPI_NOTIFY_EJECT_REQUEST:
- /* TBD */
+ acpi_handle_debug(handle, ACPI_NOTIFY_EJECT_REQUEST event\n);
break;
case ACPI_NOTIFY_DEVICE_CHECK_LIGHT:
+ acpi_handle_debug(handle, ACPI_NOTIFY_DEVICE_CHECK_LIGHT
event\n);
/* TBD: Exactly what does 'light' mean? */
break;
case ACPI_NOTIFY_FREQUENCY_MISMATCH:
- /* TBD */
+ acpi_handle_err(handle, Device cannot be configured due
+ to a frequency mismatch\n);
break;
case ACPI_NOTIFY_BUS_MODE_MISMATCH:
- /* TBD */
+ acpi_handle_err(handle, Device cannot be configured due
+ to a bus mode mismatch\n);
break;
case ACPI_NOTIFY_POWER_FAULT:
- /* TBD */
+ acpi_handle_err(handle, Device has suffered a power fault\n);
break;
default:
- ACPI_DEBUG_PRINT((ACPI_DB_INFO,
- Received unknown/unsupported notification
[%08x]\n,
- type));
- break;
+ acpi_handle_warn(handle, Unsupported event type 0x%x\n, type);
+ ost_code = ACPI_OST_SC_UNRECOGNIZED_NOTIFY;
+ goto err;
}
- acpi_bus_get_acpi_device(handle, device);
- if (device) {
- driver = device-driver;
- if (driver driver-ops.notify
- (driver-flags ACPI_DRIVER_ALL_NOTIFY_EVENTS))
- driver-ops.notify(device, type);
+ if (acpi_bus_get_acpi_device(handle, adev))
+ goto err;
- acpi_bus_put_acpi_device(device);
+ driver = adev-driver;
+ if (driver driver-ops.notify
+ (driver-flags ACPI_DRIVER_ALL_NOTIFY_EVENTS))
+ driver-ops.notify(adev, type);
+
+ switch (type) {
+ case ACPI_NOTIFY_BUS_CHECK:
+ case ACPI_NOTIFY_DEVICE_CHECK:
+ case ACPI_NOTIFY_EJECT_REQUEST:
+ status = acpi_hotplug_execute(acpi_device_hotplug, adev, type);
+ if (ACPI_SUCCESS(status))
+ return;
+ default:
+ break;
}
+ acpi_bus_put_acpi_device(adev);
+ return;
+
+ err:
+ acpi_evaluate_hotplug_ost(handle, type, ost_code, NULL);
}
/* --
Index: linux-pm/drivers/acpi/internal.h
===
--- linux-pm.orig/drivers/acpi/internal.h
+++
[PATCH v3 3/7] ACPI / hotplug / PCI: Define hotplug context lock in the core
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
Subsequent changes will require the ACPI core to acquire the lock
protecting the ACPIPHP hotplug contexts, so move the definition of
the lock to the core and change its name to be more generic.
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
drivers/acpi/scan.c| 11 +
drivers/pci/hotplug/acpiphp_glue.c | 41 ++---
include/acpi/acpi_bus.h|2 +
3 files changed, 33 insertions(+), 21 deletions(-)
Index: linux-pm/drivers/acpi/scan.c
===
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -41,6 +41,7 @@ static DEFINE_MUTEX(acpi_scan_lock);
static LIST_HEAD(acpi_scan_handlers_list);
DEFINE_MUTEX(acpi_device_lock);
LIST_HEAD(acpi_wakeup_device_list);
+static DEFINE_MUTEX(acpi_hp_context_lock);
struct acpi_device_bus_id{
char bus_id[15];
@@ -60,6 +61,16 @@ void acpi_scan_lock_release(void)
}
EXPORT_SYMBOL_GPL(acpi_scan_lock_release);
+void acpi_lock_hp_context(void)
+{
+ mutex_lock(acpi_hp_context_lock);
+}
+
+void acpi_unlock_hp_context(void)
+{
+ mutex_unlock(acpi_hp_context_lock);
+}
+
int acpi_scan_add_handler(struct acpi_scan_handler *handler)
{
if (!handler || !handler-attach)
Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c
===
--- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c
+++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c
@@ -58,7 +58,6 @@
static LIST_HEAD(bridge_list);
static DEFINE_MUTEX(bridge_mutex);
-static DEFINE_MUTEX(acpiphp_context_lock);
static void handle_hotplug_event(acpi_handle handle, u32 type, void *data);
static void acpiphp_sanitize_bus(struct pci_bus *bus);
@@ -75,7 +74,7 @@ static void acpiphp_context_handler(acpi
* acpiphp_init_context - Create hotplug context and grab a reference to it.
* @adev: ACPI device object to create the context for.
*
- * Call under acpiphp_context_lock.
+ * Call under acpi_hp_context_lock.
*/
static struct acpiphp_context *acpiphp_init_context(struct acpi_device *adev)
{
@@ -100,7 +99,7 @@ static struct acpiphp_context *acpiphp_i
* acpiphp_get_context - Get hotplug context and grab a reference to it.
* @handle: ACPI object handle to get the context for.
*
- * Call under acpiphp_context_lock.
+ * Call under acpi_hp_context_lock.
*/
static struct acpiphp_context *acpiphp_get_context(acpi_handle handle)
{
@@ -122,7 +121,7 @@ static struct acpiphp_context *acpiphp_g
*
* The context object is removed if there are no more references to it.
*
- * Call under acpiphp_context_lock.
+ * Call under acpi_hp_context_lock.
*/
static void acpiphp_put_context(struct acpiphp_context *context)
{
@@ -151,7 +150,7 @@ static void free_bridge(struct kref *kre
struct acpiphp_slot *slot, *next;
struct acpiphp_func *func, *tmp;
- mutex_lock(acpiphp_context_lock);
+ acpi_lock_hp_context();
bridge = container_of(kref, struct acpiphp_bridge, ref);
@@ -175,7 +174,7 @@ static void free_bridge(struct kref *kre
pci_dev_put(bridge-pci_dev);
kfree(bridge);
- mutex_unlock(acpiphp_context_lock);
+ acpi_unlock_hp_context();
}
/*
@@ -291,17 +290,17 @@ static acpi_status register_slot(acpi_ha
device = (adr 16) 0x;
function = adr 0x;
- mutex_lock(acpiphp_context_lock);
+ acpi_lock_hp_context();
context = acpiphp_init_context(adev);
if (!context) {
- mutex_unlock(acpiphp_context_lock);
+ acpi_unlock_hp_context();
acpi_handle_err(handle, No hotplug context\n);
return AE_NOT_EXIST;
}
newfunc = context-func;
newfunc-function = function;
newfunc-parent = bridge;
- mutex_unlock(acpiphp_context_lock);
+ acpi_unlock_hp_context();
if (acpi_has_method(handle, _EJ0))
newfunc-flags = FUNC_HAS_EJ0;
@@ -319,9 +318,9 @@ static acpi_status register_slot(acpi_ha
slot = kzalloc(sizeof(struct acpiphp_slot), GFP_KERNEL);
if (!slot) {
- mutex_lock(acpiphp_context_lock);
+ acpi_lock_hp_context();
acpiphp_put_context(context);
- mutex_unlock(acpiphp_context_lock);
+ acpi_unlock_hp_context();
return AE_NO_MEMORY;
}
@@ -396,7 +395,7 @@ static struct acpiphp_bridge *acpiphp_ha
struct acpiphp_context *context;
struct acpiphp_bridge *bridge = NULL;
- mutex_lock(acpiphp_context_lock);
+ acpi_lock_hp_context();
context = acpiphp_get_context(handle);
if (context) {
bridge = context-bridge;
@@ -405,7 +404,7 @@ static struct acpiphp_bridge *acpiphp_ha
acpiphp_put_context(context);
}
-
[PATCH v3 0/7] ACPI / hotplug / PCI: Consolidation of ACPIPHP with ACPI core device hotplug
On Sunday, February 02, 2014 01:52:26 AM Rafael J. Wysocki wrote: On Wednesday, January 29, 2014 12:57:06 AM Rafael J. Wysocki wrote: On Tuesday, January 28, 2014 11:10:30 PM Rafael J. Wysocki wrote: Hi All, It looks like there's time for more adventurous stuff. :-) The following series is on top of the one I sent on Sunday: https://lkml.org/lkml/2014/1/26/191 The final outcome of the patches below is that all ACPI hotplug notifications for PCI devices and for core system things like CPU, memory, PCI roots etc., will be dispatched from acpi_bus_notify() and it is not necessary to install a separate hotplug notify handler for each device any more. [1/5] Attach ACPIPHP hotplug contexts to struct acpi_device objects. [2/5] Introduce wrappers for installing and removing hotplug notify handlers (those wrappers go away later on, but they are useful for separating changes). [3/5] Consolidate ACPI hotplug signaling for PCI and ACPI core. [4/5] Simplify notify handle registration wrapper. [5/5] Dispatch ACPI hotplug notifications for core devices and PCI from acpi_bus_notify(). Unfortunately, I realized that patches [3-5/5] were buggy. The bugs were kind of subtle and might not be easy to reproduce, but they were bugs anyway. :-) A respin of the whole series follows. After the Mika's testing it turned out that they were more buggy than I had though. Oh well. The following patchset is a reworked version of the previous one. Functionality-wise the final result should be very similar, but not exactly the same. [1/6] Fix a theoretical race condition in acpi_hotplug_notify_cb(). [2/6] Move the hotplug context lock definition to the ACPI core (from ACPIPHP). [3/6] Consolidate ACPI hotplug signaling for PCI and ACPI core (this is a combination of patches [1-3/5] from the previous series). [4/6] Rework the handling of eject requests in the ACPI core. [5/6] Simplify a routine for installing hotplug notify handlers. [6/6] Dispatch ACPI hotplug notifications for core devices and PCI from acpi_bus_notify(). This is on top of https://lkml.org/lkml/2014/2/1/123 which in turn is on top of the current mainline. For the adventurous all this stuff is on the test-next branch of linux-pm.git. As stated in the message at http://marc.info/?l=linux-acpim=139135963030012w=4 , patch [1/6] was actaully wrong and the whole patchset had to be reworked for that reason. What follows is an entirely new version: [1/7] Add a new function to ACPICA allowing a callback to be executed under the namespace mutex after calling acpi_ns_get_attached_data(). [2/7] Use the new ACPICA's function to fix a couple of potential races related to ACPI notifies. [3/7] Same as [2/6] above. [4/7] Same as [3/6] above, rebased. [5/7] Same as [4/6] above. [6/7] Same as [5/6] above. [7/7] Dispatch ACPI hotplug notifications for core devices and PCI from acpi_bus_notify(). This actually is different from [6/6] above, although it serves the same purpose. Thanks! -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 1/7] ACPICA: Introduce acpi_get_data_full() and rework acpi_get_data()
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
Introduce a new function, acpi_get_data_full(), working in analogy
with acpi_get_data() except that it can execute a callback provided
as its 4th argument right after acpi_ns_get_attached_data() has
returned a success.
That will allow Linux to reference count the object pointed to by
*data before the namespace mutex is released so as to ensure that it
will not be freed going forward until the reference to it acquired
by acpi_get_data_full() is dropped.
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
drivers/acpi/acpica/nsxfeval.c | 33 ++---
include/acpi/acpixf.h |4
2 files changed, 34 insertions(+), 3 deletions(-)
Index: linux-pm/drivers/acpi/acpica/nsxfeval.c
===
--- linux-pm.orig/drivers/acpi/acpica/nsxfeval.c
+++ linux-pm/drivers/acpi/acpica/nsxfeval.c
@@ -923,19 +923,22 @@ ACPI_EXPORT_SYMBOL(acpi_detach_data)
/***
*
- * FUNCTION:acpi_get_data
+ * FUNCTION:acpi_get_data_full
*
* PARAMETERS: obj_handle - Namespace node
* handler - Handler used in call to attach_data
* data- Where the data is returned
+ * callback- function to execute before returning
*
* RETURN: Status
*
- * DESCRIPTION: Retrieve data that was previously attached to a namespace node.
+ * DESCRIPTION: Retrieve data that was previously attached to a namespace node
+ * and execute a callback before returning.
*
**/
acpi_status
-acpi_get_data(acpi_handle obj_handle, acpi_object_handler handler, void **data)
+acpi_get_data_full(acpi_handle obj_handle, acpi_object_handler handler,
+ void **data, void (*callback)(void *))
{
struct acpi_namespace_node *node;
acpi_status status;
@@ -960,10 +963,34 @@ acpi_get_data(acpi_handle obj_handle, ac
}
status = acpi_ns_get_attached_data(node, handler, data);
+ if (ACPI_SUCCESS(status) callback) {
+ callback(*data);
+ }
unlock_and_exit:
(void)acpi_ut_release_mutex(ACPI_MTX_NAMESPACE);
return (status);
}
+ACPI_EXPORT_SYMBOL(acpi_get_data_full)
+
+/***
+ *
+ * FUNCTION:acpi_get_data
+ *
+ * PARAMETERS: obj_handle - Namespace node
+ * handler - Handler used in call to attach_data
+ * data- Where the data is returned
+ *
+ * RETURN: Status
+ *
+ * DESCRIPTION: Retrieve data that was previously attached to a namespace node.
+ *
+
**/
+acpi_status
+acpi_get_data(acpi_handle obj_handle, acpi_object_handler handler, void **data)
+{
+ return acpi_get_data_full(obj_handle, handler, data, NULL);
+}
+
ACPI_EXPORT_SYMBOL(acpi_get_data)
Index: linux-pm/include/acpi/acpixf.h
===
--- linux-pm.orig/include/acpi/acpixf.h
+++ linux-pm/include/acpi/acpixf.h
@@ -230,6 +230,10 @@ acpi_attach_data(acpi_handle object, acp
acpi_status acpi_detach_data(acpi_handle object, acpi_object_handler handler);
acpi_status
+acpi_get_data_full(acpi_handle object, acpi_object_handler handler, void
**data,
+ void (*callback)(void *));
+
+acpi_status
acpi_get_data(acpi_handle object, acpi_object_handler handler, void **data);
acpi_status
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 5/7] ACPI / hotplug / PCI: Rework the handling of eject requests
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
To avoid the need to install a hotplug notify handler for each ACPI
namespace node representing a device and having a matching scan
handler, move the check whether or not the ejection of the given
device is enabled through its scan handler from acpi_hotplug_notify_cb()
to acpi_generic_hotplug_event(). Also, move the execution of
ACPI_OST_SC_EJECT_IN_PROGRESS _OST to acpi_generic_hotplug_event(),
because in acpi_hotplug_notify_cb() or in acpi_eject_store() we really
don't know whether or not the eject is going to be in progress (for
example, acpi_hotplug_execute() may still fail without queuing up the
work item).
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
drivers/acpi/scan.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
Index: linux-pm/drivers/acpi/scan.c
===
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -459,6 +459,12 @@ static int acpi_generic_hotplug_event(st
return acpi_scan_device_check(adev);
case ACPI_NOTIFY_EJECT_REQUEST:
case ACPI_OST_EC_OSPM_EJECT:
+ if (adev-handler !adev-handler-hotplug.enabled) {
+ dev_info(adev-dev, Eject disabled\n);
+ return -EPERM;
+ }
+ acpi_evaluate_hotplug_ost(adev-handle,
ACPI_NOTIFY_EJECT_REQUEST,
+ ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
return acpi_scan_hot_remove(adev);
}
return -EINVAL;
@@ -483,6 +489,10 @@ static void acpi_device_hotplug(void *da
if (adev-handler) {
error = acpi_generic_hotplug_event(adev, src);
+ if (error == -EPERM) {
+ ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
+ goto err_out;
+ }
} else {
int (*event)(struct acpi_device *, u32);
@@ -512,7 +522,6 @@ static void acpi_device_hotplug(void *da
static void acpi_hotplug_notify_cb(acpi_handle handle, u32 type, void *data)
{
- struct acpi_scan_handler *handler = data;
u32 ost_code = ACPI_OST_SC_SUCCESS;
struct acpi_device *adev;
acpi_status status;
@@ -528,13 +537,6 @@ static void acpi_hotplug_notify_cb(acpi_
case ACPI_NOTIFY_EJECT_REQUEST:
acpi_handle_debug(handle, ACPI_NOTIFY_EJECT_REQUEST event\n);
- if (handler !handler-hotplug.enabled) {
- acpi_handle_err(handle, Eject disabled\n);
- ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
- goto out;
- }
- acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST,
- ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
break;
case ACPI_NOTIFY_DEVICE_WAKE:
@@ -631,8 +633,6 @@ acpi_eject_store(struct device *d, struc
if (ACPI_FAILURE(status) || !acpi_device-flags.ejectable)
return -ENODEV;
- acpi_evaluate_hotplug_ost(acpi_device-handle, ACPI_OST_EC_OSPM_EJECT,
- ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
get_device(acpi_device-dev);
status = acpi_hotplug_execute(acpi_device_hotplug, acpi_device,
ACPI_OST_EC_OSPM_EJECT);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 6/7] ACPI / hotplug / PCI: Simplify acpi_install_hotplug_notify_handler()
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
Since acpi_hotplug_notify_cb() does not use its data argument any
more, the second argument of acpi_install_hotplug_notify_handler()
can be dropped, so do that and update its callers accordingly.
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
drivers/acpi/scan.c|6 +++---
drivers/pci/hotplug/acpiphp_glue.c |2 +-
include/acpi/acpi_bus.h|2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
Index: linux-pm/drivers/acpi/scan.c
===
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -576,10 +576,10 @@ static void acpi_hotplug_notify_cb(acpi_
acpi_evaluate_hotplug_ost(handle, type, ost_code, NULL);
}
-void acpi_install_hotplug_notify_handler(acpi_handle handle, void *data)
+void acpi_install_hotplug_notify_handler(acpi_handle handle)
{
acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
- acpi_hotplug_notify_cb, data);
+ acpi_hotplug_notify_cb, NULL);
}
void acpi_remove_hotplug_notify_handler(acpi_handle handle)
@@ -2044,7 +2044,7 @@ static void acpi_scan_init_hotplug(acpi_
list_for_each_entry(hwid, pnp.ids, list) {
handler = acpi_scan_match_handler(hwid-id, NULL);
if (handler) {
- acpi_install_hotplug_notify_handler(handle, handler);
+ acpi_install_hotplug_notify_handler(handle);
break;
}
}
Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c
===
--- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c
+++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c
@@ -362,7 +362,7 @@ static acpi_status register_slot(acpi_ha
/* install notify handler */
if (!(newfunc-flags FUNC_HAS_DCK))
- acpi_install_hotplug_notify_handler(handle, NULL);
+ acpi_install_hotplug_notify_handler(handle);
return AE_OK;
}
Index: linux-pm/include/acpi/acpi_bus.h
===
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -443,7 +443,7 @@ static inline bool acpi_device_enumerate
typedef void (*acpi_hp_callback)(void *data, u32 src);
acpi_status acpi_hotplug_execute(acpi_hp_callback func, void *data, u32 src);
-void acpi_install_hotplug_notify_handler(acpi_handle handle, void *data);
+void acpi_install_hotplug_notify_handler(acpi_handle handle);
void acpi_remove_hotplug_notify_handler(acpi_handle handle);
/**
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3 2/7] ACPI / hotplug: Fix potential races in notify handlers
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
There is a slight possibility for the ACPI device object pointed to
by adev in acpi_hotplug_notify_cb() to become invalid between the
acpi_bus_get_device() that it comes from and the subsequent dereference
of that pointer under get_device(). Namely, if acpi_scan_drop_device()
runs in parallel with acpi_hotplug_notify_cb(), acpi_device_del_work_fn()
queued up by it may delete the device object in question right after
a successful execution of acpi_bus_get_device() in acpi_bus_notify().
An analogous problem is present in acpi_bus_notify() where the device
pointer coming from acpi_bus_get_device() may become invalid before
it subsequent dereference in the if block.
To prevent that from happening, introduce a new function,
acpi_bus_get_acpi_device(), working analogously to acpi_bus_get_device()
except that it will grab a reference to the ACPI device object returned
by it and it will do that under the ACPICA's namespace mutex. Then,
make both acpi_hotplug_notify_cb() and acpi_bus_notify() use
acpi_bus_get_acpi_device() instead of acpi_bus_get_device() so as to
ensure that the pointers used by them will not become stale at one
point.
In addition to that, introduce acpi_bus_put_acpi_device() as a wrapper
around put_device() to be used along with acpi_bus_get_acpi_device()
and make the (new) users of the latter use acpi_bus_put_acpi_device()
too.
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
drivers/acpi/bus.c |4 +++-
drivers/acpi/internal.h |2 ++
drivers/acpi/scan.c | 38 ++
3 files changed, 35 insertions(+), 9 deletions(-)
Index: linux-pm/drivers/acpi/scan.c
===
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -476,7 +476,7 @@ static void acpi_device_hotplug(void *da
out:
acpi_evaluate_hotplug_ost(adev-handle, src, ost_code, NULL);
- put_device(adev-dev);
+ acpi_bus_put_acpi_device(adev);
mutex_unlock(acpi_scan_lock);
unlock_device_hotplug();
}
@@ -488,9 +488,6 @@ static void acpi_hotplug_notify_cb(acpi_
struct acpi_device *adev;
acpi_status status;
- if (acpi_bus_get_device(handle, adev))
- goto err_out;
-
switch (type) {
case ACPI_NOTIFY_BUS_CHECK:
acpi_handle_debug(handle, ACPI_NOTIFY_BUS_CHECK event\n);
@@ -512,12 +509,14 @@ static void acpi_hotplug_notify_cb(acpi_
/* non-hotplug event; possibly handled by other handler */
return;
}
- get_device(adev-dev);
+ if (acpi_bus_get_acpi_device(handle, adev))
+ goto err_out;
+
status = acpi_hotplug_execute(acpi_device_hotplug, adev, type);
if (ACPI_SUCCESS(status))
return;
- put_device(adev-dev);
+ acpi_bus_put_acpi_device(adev);
err_out:
acpi_evaluate_hotplug_ost(handle, type, ost_code, NULL);
@@ -1112,14 +,16 @@ static void acpi_scan_drop_device(acpi_h
mutex_unlock(acpi_device_del_lock);
}
-int acpi_bus_get_device(acpi_handle handle, struct acpi_device **device)
+static int acpi_get_device_data(acpi_handle handle, struct acpi_device
**device,
+ void (*callback)(void *))
{
acpi_status status;
if (!device)
return -EINVAL;
- status = acpi_get_data(handle, acpi_scan_drop_device, (void **)device);
+ status = acpi_get_data_full(handle, acpi_scan_drop_device,
+ (void **)device, callback);
if (ACPI_FAILURE(status) || !*device) {
ACPI_DEBUG_PRINT((ACPI_DB_INFO, No context for object [%p]\n,
handle));
@@ -1127,8 +1128,29 @@ int acpi_bus_get_device(acpi_handle hand
}
return 0;
}
+
+int acpi_bus_get_device(acpi_handle handle, struct acpi_device **device)
+{
+ return acpi_get_device_data(handle, device, NULL);
+}
EXPORT_SYMBOL(acpi_bus_get_device);
+static void get_acpi_device(void *dev)
+{
+ if (dev)
+ get_device(((struct acpi_device *)dev)-dev);
+}
+
+int acpi_bus_get_acpi_device(acpi_handle handle, struct acpi_device **adev_p)
+{
+ return acpi_get_device_data(handle, adev_p, get_acpi_device);
+}
+
+void acpi_bus_put_acpi_device(struct acpi_device *adev)
+{
+ put_device(adev-dev);
+}
+
int acpi_device_add(struct acpi_device *device,
void (*release)(struct device *))
{
Index: linux-pm/drivers/acpi/internal.h
===
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -81,6 +81,8 @@ bool acpi_scan_is_offline(struct acpi_de
#define ACPI_STA_DEFAULT (ACPI_STA_DEVICE_PRESENT | ACPI_STA_DEVICE_ENABLED | \
ACPI_STA_DEVICE_UI |
[PATCH TRIVIAL] mm: vmscan: shrink_slab: rename max_pass - freeable
The name `max_pass' is misleading, because this variable actually keeps
the estimate number of freeable objects, not the maximal number of
objects we can scan in this pass, which can be twice that. Rename it to
reflect its actual meaning.
Signed-off-by: Vladimir Davydov vdavy...@parallels.com
---
mm/vmscan.c | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index a9c74b409681..fa3385865fc6 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -224,15 +224,15 @@ shrink_slab_node(struct shrink_control *shrinkctl, struct
shrinker *shrinker,
unsigned long freed = 0;
unsigned long long delta;
long total_scan;
- long max_pass;
+ long freeable;
long nr;
long new_nr;
int nid = shrinkctl-nid;
long batch_size = shrinker-batch ? shrinker-batch
: SHRINK_BATCH;
- max_pass = shrinker-count_objects(shrinker, shrinkctl);
- if (max_pass == 0)
+ freeable = shrinker-count_objects(shrinker, shrinkctl);
+ if (freeable == 0)
return 0;
/*
@@ -244,14 +244,14 @@ shrink_slab_node(struct shrink_control *shrinkctl, struct
shrinker *shrinker,
total_scan = nr;
delta = (4 * nr_pages_scanned) / shrinker-seeks;
- delta *= max_pass;
+ delta *= freeable;
do_div(delta, lru_pages + 1);
total_scan += delta;
if (total_scan 0) {
printk(KERN_ERR
shrink_slab: %pF negative objects to delete nr=%ld\n,
shrinker-scan_objects, total_scan);
- total_scan = max_pass;
+ total_scan = freeable;
}
/*
@@ -260,26 +260,26 @@ shrink_slab_node(struct shrink_control *shrinkctl, struct
shrinker *shrinker,
* shrinkers to return -1 all the time. This results in a large
* nr being built up so when a shrink that can do some work
* comes along it empties the entire cache due to nr
-* max_pass. This is bad for sustaining a working set in
+* freeable. This is bad for sustaining a working set in
* memory.
*
* Hence only allow the shrinker to scan the entire cache when
* a large delta change is calculated directly.
*/
- if (delta max_pass / 4)
- total_scan = min(total_scan, max_pass / 2);
+ if (delta freeable / 4)
+ total_scan = min(total_scan, freeable / 2);
/*
* Avoid risking looping forever due to too large nr value:
* never try to free more than twice the estimate number of
* freeable entries.
*/
- if (total_scan max_pass * 2)
- total_scan = max_pass * 2;
+ if (total_scan freeable * 2)
+ total_scan = freeable * 2;
trace_mm_shrink_slab_start(shrinker, shrinkctl, nr,
nr_pages_scanned, lru_pages,
- max_pass, delta, total_scan);
+ freeable, delta, total_scan);
/*
* Normally, we should not scan less than batch_size objects in one
@@ -292,12 +292,12 @@ shrink_slab_node(struct shrink_control *shrinkctl, struct
shrinker *shrinker,
*
* We detect the tight on memory situations by looking at the total
* number of objects we want to scan (total_scan). If it is greater
-* than the total number of objects on slab (max_pass), we must be
+* than the total number of objects on slab (freeable), we must be
* scanning at high prio and therefore should try to reclaim as much as
* possible.
*/
while (total_scan = batch_size ||
- total_scan = max_pass) {
+ total_scan = freeable) {
unsigned long ret;
unsigned long nr_to_scan = min(batch_size, total_scan);
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 02/23] drm/i2c: tda998x: check more I/O errors
On Sun, 2 Feb 2014 16:20:58 +
Russell King - ARM Linux li...@arm.linux.org.uk wrote:
On Sat, Jan 25, 2014 at 06:14:45PM +0100, Jean-Francois Moine wrote:
This patch adds more error checking inn I2C I/O functions.
In case of I/O error, this permits to avoid writing in bad controller
pages, a bad chipset detection or looping when getting the EDID.
I've just looked at this again, and spotted something:
-static uint8_t
+static int
reg_read(struct tda998x_priv *priv, uint16_t reg)
{
uint8_t val = 0;
- reg_read_range(priv, reg, val, sizeof(val));
+ int ret;
+
+ ret = reg_read_range(priv, reg, val, sizeof(val));
+ if (ret 0)
+ return ret;
So yes, this can return negative numbers.
@@ -1158,8 +1184,11 @@ tda998x_encoder_init(struct i2c_client *client,
tda998x_reset(priv);
/* read version: */
- priv-rev = reg_read(priv, REG_VERSION_LSB) |
- reg_read(priv, REG_VERSION_MSB) 8;
+ ret = reg_read(priv, REG_VERSION_LSB) |
+ (reg_read(priv, REG_VERSION_MSB) 8);
+ if (ret 0)
+ goto fail;
+ priv-rev = ret;
Two issues here:
1. The additional parens are /really/ not required.
2. What if reg_read(priv, REG_VERSION_MSB) returns a negative number?
If we're going to the extent of attempting to make the read/write
functions return errors, we should at least handle errors generated
by them properly, otherwise it's pointless making them return errors.
ret = reg_read(priv, REG_VERSION_LSB);
if (ret 0)
goto fail;
priv-rev = ret;
ret = reg_read(priv, REG_VERSION_MSB);
if (ret 0)
goto fail;
priv-rev |= ret 8;
If you want it to look slightly nicer:
int rev_lo, rev_hi;
rev_lo = reg_read(priv, REG_VERSION_LSB);
rev_hi = reg_read(priv, REG_VERSION_MSB);
if (rev_lo 0 || rev_hi 0) {
ret = rev_lo 0 ? rev_lo : rev_hi;
goto fail;
}
priv-rev = rev_lo | rev_hi 8;
I'm happy to commit such a change after this patch to clean it up, or if
you want to regenerate your patch 2 and post /just/ that incorporating
this change.
I think that my code works correctly: when there is an error, the
result of reg_read() is minus the error code, and this error code is
always lower than 8388607 (0x7f). Then, reg_read() 8 will always
be negative.
Otherwise, I may redo a patch about the useless parenthesis.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 09/23] drm/i2c: tda998x: don't read write-only registers
On Sun, 2 Feb 2014 16:23:09 + Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Sat, Jan 25, 2014 at 06:14:42PM +0100, Jean-Francois Moine wrote: This patch takes care of the write-only registers of the tda998x. The registers SOFTRESET, TBG_CNTRL_0 and TBG_CNTRL_1 have all bits cleared after reset, so, they may be fully re-written. The register MAT_CONTRL is set to MAT_CONTRL_MAT_BP | MAT_CONTRL_MAT_SC(1) after reset, so, it may be fully set again to this value. I said in v3 of this patch, which seems to remain unaddressed: /* must be last register set: */ - reg_clear(priv, REG_TBG_CNTRL_0, TBG_CNTRL_0_SYNC_ONCE); + reg_write(priv, REG_TBG_CNTRL_0, 0); Register changes which have a potential effect shouldn't be part of a patch which is really only trying to avoid reading from write only registers. This could be a potential functional change - and it's probably one which Rob Clark should at least be made aware of. As I commented last time, when you're changing register values in an otherwise innocuous patch, you should comment about them in the patch description. According to the tda9983b documentation, the register TBG_CNTRL_0 is set to 0 at reset time. I think that it is the same for all the tda998x family. In the other hand, this register is supposed to be write only, so reading it may return any value, and the reg_clear() function may set any other bits. Then, clearing one bit is less secure than clearing the full register. -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 02/23] drm/i2c: tda998x: check more I/O errors
On Sun, Feb 02, 2014 at 06:30:00PM +0100, Jean-Francois Moine wrote:
On Sun, 2 Feb 2014 16:20:58 +
Russell King - ARM Linux li...@arm.linux.org.uk wrote:
On Sat, Jan 25, 2014 at 06:14:45PM +0100, Jean-Francois Moine wrote:
This patch adds more error checking inn I2C I/O functions.
In case of I/O error, this permits to avoid writing in bad controller
pages, a bad chipset detection or looping when getting the EDID.
I've just looked at this again, and spotted something:
-static uint8_t
+static int
reg_read(struct tda998x_priv *priv, uint16_t reg)
{
uint8_t val = 0;
- reg_read_range(priv, reg, val, sizeof(val));
+ int ret;
+
+ ret = reg_read_range(priv, reg, val, sizeof(val));
+ if (ret 0)
+ return ret;
So yes, this can return negative numbers.
@@ -1158,8 +1184,11 @@ tda998x_encoder_init(struct i2c_client *client,
tda998x_reset(priv);
/* read version: */
- priv-rev = reg_read(priv, REG_VERSION_LSB) |
- reg_read(priv, REG_VERSION_MSB) 8;
+ ret = reg_read(priv, REG_VERSION_LSB) |
+ (reg_read(priv, REG_VERSION_MSB) 8);
+ if (ret 0)
+ goto fail;
+ priv-rev = ret;
Two issues here:
1. The additional parens are /really/ not required.
2. What if reg_read(priv, REG_VERSION_MSB) returns a negative number?
If we're going to the extent of attempting to make the read/write
functions return errors, we should at least handle errors generated
by them properly, otherwise it's pointless making them return errors.
ret = reg_read(priv, REG_VERSION_LSB);
if (ret 0)
goto fail;
priv-rev = ret;
ret = reg_read(priv, REG_VERSION_MSB);
if (ret 0)
goto fail;
priv-rev |= ret 8;
If you want it to look slightly nicer:
int rev_lo, rev_hi;
rev_lo = reg_read(priv, REG_VERSION_LSB);
rev_hi = reg_read(priv, REG_VERSION_MSB);
if (rev_lo 0 || rev_hi 0) {
ret = rev_lo 0 ? rev_lo : rev_hi;
goto fail;
}
priv-rev = rev_lo | rev_hi 8;
I'm happy to commit such a change after this patch to clean it up, or if
you want to regenerate your patch 2 and post /just/ that incorporating
this change.
I think that my code works correctly: when there is an error, the
result of reg_read() is minus the error code, and this error code is
always lower than 8388607 (0x7f). Then, reg_read() 8 will always
be negative.
The issue I'm pointing out is not whether it will be interpreted as an
error or not, it's whether the value is a correct error code. If we
don't care about the reason why an error occurs, we might as well just
return -1.
I've added my own patch which adjusts it as per above to my tree anyway,
so I'm not that worried about this.
--
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was up to 13.2Mbit.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 09/23] drm/i2c: tda998x: don't read write-only registers
On Sun, Feb 02, 2014 at 06:45:12PM +0100, Jean-Francois Moine wrote: On Sun, 2 Feb 2014 16:23:09 + Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Sat, Jan 25, 2014 at 06:14:42PM +0100, Jean-Francois Moine wrote: This patch takes care of the write-only registers of the tda998x. The registers SOFTRESET, TBG_CNTRL_0 and TBG_CNTRL_1 have all bits cleared after reset, so, they may be fully re-written. The register MAT_CONTRL is set to MAT_CONTRL_MAT_BP | MAT_CONTRL_MAT_SC(1) after reset, so, it may be fully set again to this value. I said in v3 of this patch, which seems to remain unaddressed: /* must be last register set: */ - reg_clear(priv, REG_TBG_CNTRL_0, TBG_CNTRL_0_SYNC_ONCE); + reg_write(priv, REG_TBG_CNTRL_0, 0); Register changes which have a potential effect shouldn't be part of a patch which is really only trying to avoid reading from write only registers. This could be a potential functional change - and it's probably one which Rob Clark should at least be made aware of. As I commented last time, when you're changing register values in an otherwise innocuous patch, you should comment about them in the patch description. According to the tda9983b documentation, the register TBG_CNTRL_0 is set to 0 at reset time. I think that it is the same for all the tda998x family. In the other hand, this register is supposed to be write only, so reading it may return any value, and the reg_clear() function may set any other bits. Then, clearing one bit is less secure than clearing the full register. Okay, in that case I'm happy with this patch. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: rtl8821ae.
On Sun, Feb 02, 2014 at 11:05:12AM -0500, Dave Jones wrote: On Sun, Feb 02, 2014 at 03:41:27AM -0800, scan-ad...@coverity.com wrote: Please find the latest report on new defect(s) introduced to Linux found with Coverity Scan. Defect(s) Reported-by: Coverity Scan Showing 20 of 83 defect(s) Ugh, this is even worse than the usual realtek drivers. (With the exception of rtl8188eu) All 83 of those new defects came from this new driver, and while there's a bunch of who cares type things in there, there's a load of stuff that needs fixing a lot more urgently than CodingStyle issues or anything else in the TODO for that driver. A bigger problem though, is what is the plan for these realtek drivers ? They've been in staging forever. rtl8187se has been there for _five_ years with no indication it's ever getting promoted to first class status. This new driver will be moved to drivers/net/wireless for 3.15, Larry has a real port of it to the proper apis and the like, cleaning up these types of issues already. It didn't make 3.14, which is why I added the staging version for now (i.e. I want the hardware I have to work...) The git logs are littered mostly with CodingStyle cleanups, sparse cleanups and such, meanwhile for five years they've had out of bounds reads, overflows, and such for this whole time. Even worse, when one of the drivers gets fixes for actual problems like this, they never make it back to Realtek, who clone the same old shitty driver they shipped last time, and reintroduce new variants of the same damn bugs, and then we import the new turd into staging and start all over again. I get the whole a shit driver is better than no driver, but there's no discernable effort to ever improve this pile, just to keep adding to it. Larry is the one who could answer for the remaining realtek drivers, and the changes don't flow back issues. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 00/23]
On Sun, Feb 02, 2014 at 12:43:58PM +, Russell King - ARM Linux wrote: On Wed, Jan 29, 2014 at 10:01:22AM +0100, Jean-Francois Moine wrote: This patch set contains various extensions to the tda998x driver: - simplify the i2c read/write - code cleanup and fix some small errors - use global constants - don't read write-only registers - add DT support - use IRQ for connection status and EDID read I discussed these patches with Rob Clark recently, and the conclusion we came to is that I'll merge them into a git tree, test them, and once I'm happy I'll send a pull request as appropriate. I'll go through them later today. Those patches which have been re- posted without any change for the last few times (the first few) I'll take into my git tree today so you don't have to keep re-posting them (more importantly, I won't have to keep on looking at them either.) Okay, out of your patches, I would like to queue up the following for submission into -rc kernels: drm/i2c: tda998x: fix bad value in the AIF drm/i2c: tda998x: check the CEC device creation drm/i2c: tda998x: free the CEC device on encoder_destroy drm/i2c: tda998x: force the page register at startup time drm/i2c: tda998x: set the PLL division factor in range 0..3 drm/i2c: tda998x: fix the ENABLE_SPACE register since these fix real bugs. Moving them to the front of the queue isn't that big a deal (I've done it here in my git tree - yes, there were a few conflicts, but nothing serious.) I'll also consider these for -rc too: drm/i2c: tda998x: use HDMI constants drm/i2c: tda998x: add the active aspect in HDMI AVI frame drm/i2c: tda998x: change the frequence in the audio channel if we have reports of display devices affected by the info frame errors. As far as the last summary line goes, I'll change it to: Use ALSA IEC958 definitions and update audio frequency Note that in general, bug fixes should always be towards the beginning of a patch series, so that they can be applied independently of the remainder of the development, which also makes them easier to backport to stable kernels if that's deemed necessary. As for the remainder, I think moving the DT documentation patch immediately after the addition of DT code (or even just before it) is good form. So, in summary, I'm pretty happy with this again - and it's all been tested here with no apparant detrimental effects. All committed and queued up here: http://ftp.arm.linux.org.uk/cgit/linux-cubox.git/log/?h=tda998x-devel If you want to pull it back to check: git://ftp.arm.linux.org.uk/~rmk/linux-cubox.git tda998x-devel I'm proposing to send everything up to the tda998x-fixes tag next week for 3.13-rc kernels. Rob, do you want to provide any acks for this or are you happy? -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 00/23]
On Sun, 2 Feb 2014 12:43:58 + Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Wed, Jan 29, 2014 at 10:01:22AM +0100, Jean-Francois Moine wrote: This patch set contains various extensions to the tda998x driver: - simplify the i2c read/write - code cleanup and fix some small errors - use global constants - don't read write-only registers - add DT support - use IRQ for connection status and EDID read I discussed these patches with Rob Clark recently, and the conclusion we came to is that I'll merge them into a git tree, test them, and once I'm happy I'll send a pull request as appropriate. I'll go through them later today. Those patches which have been re- posted without any change for the last few times (the first few) I'll take into my git tree today so you don't have to keep re-posting them (more importantly, I won't have to keep on looking at them either.) Thanks. BTW, I found some problems with the patch 12 'add DT support' you already acked: - the .of_match_table is not needed because the i2c client is created by the i2c subsystem from the 'reg' in the DT, - on encoder_destroy(), the function drm_i2c_encoder_destroy() unregisters the i2c client, so, with a DT, a second encoder_init() would crash. Do you better like a new patch or a fix? -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Documentation cleanup, update 00-INDEX files in Documentation/
On Sun, 2014-02-02 at 01:04 -0600, Rob Landley wrote: Back before kernel.org did its epic barn door locking after the horses escaped, I used to use them to generate HTML indexes for kernel.org/doc/Documentation, but sometime after they took away my ability to rsync updates to that they replaced that directory with a raw git checkout. I'm still waiting for them to replace the HTML versions of the menuconfig help text and the htmldocs output with raw git checkouts too. I periodically poke them about it, ala https://bugzilla.kernel.org/show_bug.cgi?id=52721#c7 but the answer is always crickets chirping or you should upload web pages with git just like you browse the web with git, watch video with git, play skyrim with git, edit code with git, compile code with git... there is no other software tool. At which point I wander off for another 6 months. So I used to have a use for it, but since kernel.org got all paranoid I can't use it that way anymore. (That's why I haven't been updating these files when the patches themselves don't. It's on my todo list, but it's really hard to _care_ when they so obviously don't.) Has it not occurred to you that everyone else moved on from this. Sure there were annoying and painful adjustments that had to be made and processes that had to be completely rebuilt and sure it was hard to build a chain of trust to people in obscure places but everyone else did it. Business people are fond of dividing the world into Doers and Whiners. I don't entirely buy this because in my personal experience if something ends up being really hard, Doers like to whine about how hard it was to do in the pub afterwards ... I know I do, particularly over single malts. The essential truth, I think, is that people with the drive to do something use that drive to overcome obstacles in their path; people without it use the obstacles as an excuse for why something didn't get done. Fundamentally, Open Source is about Doers and thus, while there are many self help groups for Whiners Open Source lists like LKML aren't among them. James -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From Mrs. Tan Ruby
As you read this letter, please don't feel bad, because I believe everyone will die some day. My name is Mrs. Tan Ruby I have been suffering from ovarian cancer Disease and the doctor says that I have just few days to live on earth. I am From Singapore, but based in Burkina Faso, Africa since for over ten years now as a business woman dealing with cotton exportation, now that I am about to end the race like this, without any family members and no child. I have $5.3 Million US DOLLARS in Commercial Bank of Africa (CBA) which I Instructed the bank to give African union leaders to help sick people Around Africa. But my mind is not at rest because I am writing this now through the help of my nurse beside me here in my hospital room. I also have $2.7 Million US Dollars in another bank (ECOBANK) in Burkina Faso which I want you to claim from the bank and use it to help the less privilege People in your country, but you must assure me that you will take only 60% of the total money and give the remaining 40% to the Charity organization and orphanage home in your country for my heart to rest. This is my promise to God so that my spirit will rest in peace. Upon the receipt of your email that you are willing and capable to Execute my plan, i will instruct the bank management to make the Immediate transfer into your account. Please aways reply me on my private email.(mrs.tanrub...@gmail.com) Your friend, Mrs. Tan Ruby -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: rtl8821ae.
Hi [ CC'ing the relevant parties ] On Sunday 02 February 2014, Dave Jones wrote: On Sun, Feb 02, 2014 at 03:41:27AM -0800, scan-ad...@coverity.com wrote: Please find the latest report on new defect(s) introduced to Linux found with Coverity Scan. Defect(s) Reported-by: Coverity Scan Showing 20 of 83 defect(s) Ugh, this is even worse than the usual realtek drivers. (With the exception of rtl8188eu) All 83 of those new defects came from this new driver, and while there's a bunch of who cares type things in there, there's a load of stuff that needs fixing a lot more urgently than CodingStyle issues or anything else in the TODO for that driver. A bigger problem though, is what is the plan for these realtek drivers ? They've been in staging forever. rtl8187se has been there for _five_ years with no indication it's ever getting promoted to first class status. Actually rtl8187se (aka rtl8185b) seems to have gotten some attention recently: http://lkml.kernel.org/r/can8yu5pgkx9s9dewpfto_ztdr-+wdd5cx2jrv1zd1m1q0bp...@mail.gmail.com The git logs are littered mostly with CodingStyle cleanups, sparse cleanups and such, meanwhile for five years they've had out of bounds reads, overflows, and such for this whole time. Even worse, when one of the drivers gets fixes for actual problems like this, they never make it back to Realtek, who clone the same old shitty driver they shipped last time, and reintroduce new variants of the same damn bugs, and then we import the new turd into staging and start all over again. I get the whole a shit driver is better than no driver, but there's no discernable effort to ever improve this pile, just to keep adding to it. Dave I think there are mostly two major problems with these drivers, besides RealTek still working on a non-mac80211 codebase for USB based devices. The sheer number of slightly different RealTek drivers for similar chipsets, for which RealTek as forked off a dedicated driver each, rather than extending the existing ones. With the other, probably even larger, problem being that it isn't possible to port wireless drivers from non-mac80111 to mac80211 in a gradual fashion, it's always a parallel re-implementation. Just look at the recent history of staging wireless drivers: the successful ones: - csr -- /dev/null - otus -- ar9170 -- carl9170 - ( rt2870 rt3070 ) -- rt2800usb - rt2870 -- rt2800pci - [ at76c503a -- ] at76_usb -- at76c50x-usb [*] not in staging the pending ones - rtl8187se [ -- rtl8180 ] [*] hopefully soon - rtl8188eu -- ? - [ rtl8192du -- ? ] [*] not in staging, [1] - rtl8192e -- ? - rtl8192u -- ? - rtl8192su -- rtl8712 -- ? [ r92su[2] would add cfg80211 support, but it being a fullmac like re-implementation doesn't get it anywhere ] - rtl8821ae [ -- mac80211 port planned for 3.15[3]? ] these devices are, besides rtl8187se (802.11g) and rtl8821ae (802.11ac), all 802.11n compatible, but were quickly EOLed by the vendor, probably making it hard to get enough traction for a proper mac80211 port. Coincidentally these chipsets are also very popular, rtl8187se being the chipset of the early netbook craze, rtl8188eu pretty ubiquitous on embedded platforms, the others making the bulk of aftermarket USB devices. ancient hardware, probably not going anywhere: - vt6655 -- ? - vt6656 -- ? - wlags49_h2 -- ? - wlags49_h25 -- ? - wlan-ng -- ? This likely leaves staging wireless drivers to small corrections and bugfixes. In the hope that the devices will get enough traction that someone takes up the effort of doing a parallel re-implementation of a proper mac80211 based driver, using the staging source only as reference. Regards Stefan Lippers-Hollmann [1] https://github.com/lwfinger/rtl8192du [2] https://github.com/chunkeey/rtl8192su/tree/master/r92su [3] http://lkml.kernel.org/r/52e066a1.9010...@lwfinger.net http://lkml.kernel.org/r/52e7d9f6.5070...@lwfinger.net -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 00/23]
On Sun, Feb 02, 2014 at 07:06:06PM +0100, Jean-Francois Moine wrote: On Sun, 2 Feb 2014 12:43:58 + Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Wed, Jan 29, 2014 at 10:01:22AM +0100, Jean-Francois Moine wrote: This patch set contains various extensions to the tda998x driver: - simplify the i2c read/write - code cleanup and fix some small errors - use global constants - don't read write-only registers - add DT support - use IRQ for connection status and EDID read I discussed these patches with Rob Clark recently, and the conclusion we came to is that I'll merge them into a git tree, test them, and once I'm happy I'll send a pull request as appropriate. I'll go through them later today. Those patches which have been re- posted without any change for the last few times (the first few) I'll take into my git tree today so you don't have to keep re-posting them (more importantly, I won't have to keep on looking at them either.) Thanks. BTW, I found some problems with the patch 12 'add DT support' you already acked: - the .of_match_table is not needed because the i2c client is created by the i2c subsystem from the 'reg' in the DT, Okay - may it be a good idea to have someone knowledgable of I2C give it a review? - on encoder_destroy(), the function drm_i2c_encoder_destroy() unregisters the i2c client, so, with a DT, a second encoder_init() would crash. I think this is one of the down-sides of trying to bolt DT into this: the drm encoder slave support is not designed to cope with an i2c client device pre-created. In fact, I can't see how this stuff comes anywhere close to working in a DT setup: in such a scenario, you declare that there's a tda998x device in DT. I2C parses this, and creates an i2c_client itself for the tda998x. When the TDA998x driver initialises, it finds this i2c client and binds to it, calling tda998x_probe(), which does nothing. However, the only way to attach a slave encoder to a DRM device is via a call to drm_i2c_encoder_init(), which unconditionally calls i2c_new_device(). This creates a _new_ i2c_client structure, again unconditionally, for the tda998x. This must be bound by the I2C subsystem to a driver - hopefully the tda998x driver, which then calls it's encoder_init function. None of this will happen if DT has already created an i2c_client at the appropriate address, because DRMs i2c_new_device() will fail. My hypothesis is that you have other patches to I2C and/or DRM to sort this out which you haven't been posting with this series. So, could you please provide some hints as to how this works? -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 00/23]
On 02/02/2014 07:23 PM, Russell King - ARM Linux wrote: On Sun, Feb 02, 2014 at 07:06:06PM +0100, Jean-Francois Moine wrote: - on encoder_destroy(), the function drm_i2c_encoder_destroy() unregisters the i2c client, so, with a DT, a second encoder_init() would crash. I think this is one of the down-sides of trying to bolt DT into this: the drm encoder slave support is not designed to cope with an i2c client device pre-created. In fact, I can't see how this stuff comes anywhere close to working in a DT setup: in such a scenario, you declare that there's a tda998x device in DT. I2C parses this, and creates an i2c_client itself for the tda998x. When the TDA998x driver initialises, it finds this i2c client and binds to it, calling tda998x_probe(), which does nothing. However, the only way to attach a slave encoder to a DRM device is via a call to drm_i2c_encoder_init(), which unconditionally calls i2c_new_device(). This creates a _new_ i2c_client structure, again unconditionally, for the tda998x. This must be bound by the I2C subsystem to a driver - hopefully the tda998x driver, which then calls it's encoder_init function. None of this will happen if DT has already created an i2c_client at the appropriate address, because DRMs i2c_new_device() will fail. drm_i2c_encoder_init() could look at .of_node of the i2c_board_info. If it is there, do not try to i2c_new_device as it has already been registered by DT i2c auto-probing. Sebastian -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 00/23]
On Sun, 2 Feb 2014 18:23:49 + Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Sun, Feb 02, 2014 at 07:06:06PM +0100, Jean-Francois Moine wrote: On Sun, 2 Feb 2014 12:43:58 + Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Wed, Jan 29, 2014 at 10:01:22AM +0100, Jean-Francois Moine wrote: This patch set contains various extensions to the tda998x driver: - simplify the i2c read/write - code cleanup and fix some small errors - use global constants - don't read write-only registers - add DT support - use IRQ for connection status and EDID read I discussed these patches with Rob Clark recently, and the conclusion we came to is that I'll merge them into a git tree, test them, and once I'm happy I'll send a pull request as appropriate. I'll go through them later today. Those patches which have been re- posted without any change for the last few times (the first few) I'll take into my git tree today so you don't have to keep re-posting them (more importantly, I won't have to keep on looking at them either.) Thanks. BTW, I found some problems with the patch 12 'add DT support' you already acked: - the .of_match_table is not needed because the i2c client is created by the i2c subsystem from the 'reg' in the DT, Okay - may it be a good idea to have someone knowledgable of I2C give it a review? Sure! There is still a lot of magic in the DT. I used the tda998x in the DT for a long time without .of_match_table and it loaded correctly. I added it in the patch just because my other modules had it. A few days ago, when I moved the tda998x audio codec description in the DT as a subnode of the tda998x i2c, the codec module was not loaded. An of_platform_populate() of the subnodes was needed in the tda998x i2c driver. I could not find why... - on encoder_destroy(), the function drm_i2c_encoder_destroy() unregisters the i2c client, so, with a DT, a second encoder_init() would crash. I think this is one of the down-sides of trying to bolt DT into this: the drm encoder slave support is not designed to cope with an i2c client device pre-created. In fact, I can't see how this stuff comes anywhere close to working in a DT setup: in such a scenario, you declare that there's a tda998x device in DT. I2C parses this, and creates an i2c_client itself for the tda998x. When the TDA998x driver initialises, it finds this i2c client and binds to it, calling tda998x_probe(), which does nothing. However, the only way to attach a slave encoder to a DRM device is via a call to drm_i2c_encoder_init(), which unconditionally calls i2c_new_device(). This creates a _new_ i2c_client structure, again unconditionally, for the tda998x. This must be bound by the I2C subsystem to a driver - hopefully the tda998x driver, which then calls it's encoder_init function. None of this will happen if DT has already created an i2c_client at the appropriate address, because DRMs i2c_new_device() will fail. My hypothesis is that you have other patches to I2C and/or DRM to sort this out which you haven't been posting with this series. So, could you please provide some hints as to how this works? I explained how to use the tda998x in a DT context in a message to Jyri Sarha: http://lists.freedesktop.org/archives/dri-devel/2014-January/052936.html -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Xen-devel] [PATCH v6] xen/grant-table: Avoid m2p_override during mapping
On 02/02/14 11:29, Julien Grall wrote:
Hello,
This patch is breaking Linux compilation on ARM:
drivers/xen/grant-table.c: In function ‘__gnttab_map_refs’:
drivers/xen/grant-table.c:989:3: error: implicit declaration of function
‘FOREIGN_FRAME’ [-Werror=implicit-function-declaration]
if (unlikely(!set_phys_to_machine(pfn, FOREIGN_FRAME(mfn {
^
drivers/xen/grant-table.c: In function ‘__gnttab_unmap_refs’:
drivers/xen/grant-table.c:1054:3: error: implicit declaration of function
‘get_phys_to_machine’ [-Werror=implicit-function-declaration]
mfn = get_phys_to_machine(pfn);
^
drivers/xen/grant-table.c:1055:43: error: ‘FOREIGN_FRAME_BIT’ undeclared (first
use in this function)
if (mfn == INVALID_P2M_ENTRY || !(mfn FOREIGN_FRAME_BIT)) {
^
drivers/xen/grant-table.c:1055:43: note: each undeclared identifier is reported
only once for each function it appears in
drivers/xen/grant-table.c:1068:9: error: too many arguments to function
‘m2p_remove_override’
mfn);
^
In file included from include/xen/page.h:4:0,
from drivers/xen/grant-table.c:48:
/local/home/julien/works/midway/linux/arch/arm/include/asm/xen/page.h:106:19:
note: declared here
static inline int m2p_remove_override(struct page *page, bool clear_pte)
^
cc1: some warnings being treated as errors
Hi,
That's bad indeed. I think the best solution is to put those parts
behind an #ifdef x86. The ones moved from x86/p2m.c to grant-table.c.
David, Stefano, what do you think?
Zoli
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 00/23]
On Sun, 2 Feb 2014 18:04:34 + Russell King - ARM Linux li...@arm.linux.org.uk wrote: So, in summary, I'm pretty happy with this again - and it's all been tested here with no apparant detrimental effects. All committed and queued up here: http://ftp.arm.linux.org.uk/cgit/linux-cubox.git/log/?h=tda998x-devel If you want to pull it back to check: git://ftp.arm.linux.org.uk/~rmk/linux-cubox.git tda998x-devel I'm proposing to send everything up to the tda998x-fixes tag next week for 3.13-rc kernels. Everything is OK. Thank you. -- Ken ar c'hentañ | ** Breizh ha Linux atav! ** Jef | http://moinejf.free.fr/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC] compat: Get rid of (get|put)_compat_time(val|spec)
On Sat, Feb 1, 2014 at 7:03 PM, H. Peter Anvin h...@linux.intel.com wrote: Clean it up by favoring the full-service version; the limited version is replaced with double-underscore versions static to kernel/compat.c. Ack. Make it so. I assume you've tested this on x32 (and hopefully x86-32 on a 64-bit kernel), and that I will get this through the x86 git trees? Linus -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 00/16] drm/nouveau: initial support for GK20A (Tegra K1)
Hi Alexandre, On Fri, Jan 31, 2014 at 10:16 PM, Alexandre Courbot acour...@nvidia.com wrote: I guess my email address might surprise some of you, so let me anticipate some questions you might have. :P Yes, this work is endorsed by NVIDIA. Several other NVIDIAns (CC'd), including core GPU experts, have provided significant technical guidance and will continue their involvement. Special thanks go to Terje Bergstrom and Ken Adams for their invaluable GPU expertise, and Thierry Reding (at FOSDEM this weekend) for help with debugging and user-space testing. Let me also stress that although very exciting, this effort is still experimental, so I would like to make sure that nobody makes excessive expectations based on these few patches. The scope of this work is strictly limited to Tegra (although given the similarities desktop GPU support will certainly benefit from it indirectly), and we do not have any plan to work on user-space support. So do not uninstall that proprietary driver just yet. ;) With this being clarified, we are looking forward to getting your feedback and working with you guys to bring and improve Tegra K1 support into Nouveau! :) I've sent a couple of fairly trivial comments, as you saw, and I suspect that others with a better understanding of the guts will have more substantial architectural feedback, esp after the weekend/FOSDEM. However, since no one's said it already -- welcome to Nouveau! From the looks of it, you could bring up a full open-source stack with your patches (i.e. Xorg + nouveau DDX + mesa) and use PRIME to render stuff (assuming the actual display hw has an X ddx). Although I suspect that you're going to want to use your own drivers. Still a little curious if you've tried the open-source stack and whether it worked. [Not sure what the status is of render-node support is in mesa, but perhaps it's enough to try running piglit tests, if you can't get X going with the display HW.] -ilia -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 00/23]
On Sun, Feb 02, 2014 at 07:54:00PM +0100, Jean-Francois Moine wrote: I explained how to use the tda998x in a DT context in a message to Jyri Sarha: http://lists.freedesktop.org/archives/dri-devel/2014-January/052936.html Okay, so there's a bunch of changes required to the DRM slave support which aren't in place yet. In which case, it may be better to reorder the remaining patches such that the DT changes are at the very end - which means we can still benefit from the rest of the patches if the DT solution remains an open question. We do have another option now that my generic component support is in mainline (merged during this window), which should result in a much cleaner solution. If we convert TDA998x to a component, armada DRM to a component master (and same for other tda998x users) then we don't need the drm_encoder_slave stuff - all that goes away since it's no longer necessary. We also solve this problem as well - because we're then not messing around with working out if there's a DT node present: the TDA998x device must pre-exist. For non-DT setups, this can be done when the I2C bus is created - devices on it would be created using the standard mechanisms already present via the i2c_board_data array. For DT setups, the devices are created by parsing the I2C bus node in DT. Both cases result in a component being registered upon invocation of tda998x_probe(), and removal of the component when tda998x_remove() is called. The tda998x driver becomes a standard I2C driver. This is something I've been intending to look at now that the component stuff is in place - as I said previously when the questions around DT and Armada DRM were first posed, we need to solve these issues in a generic way first, rather than hacking around them. -- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was up to 13.2Mbit. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Make math_state_restore() save and restore the interrupt flag
On Sat, Feb 1, 2014 at 11:19 PM, Suresh Siddha sbsid...@gmail.com wrote: The real fix for Nate's problem will be coming from Linus, with a slightly modified option-b that Linus proposed. Linus, please let me know if you want me to spin it. I can do it sunday night. Please do it, since clearly I wasn't aware enough about the whole non-TS-checking FPU state details. Also, since this issue doesn't seem to be a recent regression, I'm not going to take this patch directly (even though I'm planning on doing -rc1 in a few hours), and expect that I'll get it through the normal channels (presumably together with the __kernel_fpu_end cleanups). Ok with everybody? Linus -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] parisc updates for v3.14
On Sun, Feb 2, 2014 at 3:15 AM, Helge Deller del...@gmx.de wrote: Anyway, the suggested untested patch below should fix the metag arch to cope which my changes to fs/exec.c ... -#define STACK_RND_MASK (0) +#define STACK_RND_MASK (-1) I don't think that works. That completely breaks randomize_stack_top(). So I'm not going to pull the parisc tree, this needs to be resolved sanely. In fact, I think that change to fs/exec.c is just completely broken: + /* add some more stack size for stack randomization */ + stack_base += STACK_RND_MASK + 1; and that +1 just doesn't make sense, and fundamentally breaks STACK_RND_MASK. It also seems to be entirely pointless, since the PAGE_ALIGN() that comes right afterwards will effectively do it anyway. So NAK on that whole fs/exec.c change. Afaik it's just wrong, and it's stupid. Linus -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: rtl8821ae.
On Sun, 2014-02-02 at 18:07 +, Stefan Lippers-Hollmann wrote: Hi [ CC'ing the relevant parties ] On Sunday 02 February 2014, Dave Jones wrote: On Sun, Feb 02, 2014 at 03:41:27AM -0800, scan-ad...@coverity.com wrote: Please find the latest report on new defect(s) introduced to Linux found with Coverity Scan. Defect(s) Reported-by: Coverity Scan Showing 20 of 83 defect(s) Ugh, this is even worse than the usual realtek drivers. (With the exception of rtl8188eu) All 83 of those new defects came from this new driver, and while there's a bunch of who cares type things in there, there's a load of stuff that needs fixing a lot more urgently than CodingStyle issues or anything else in the TODO for that driver. A bigger problem though, is what is the plan for these realtek drivers ? They've been in staging forever. rtl8187se has been there for _five_ years with no indication it's ever getting promoted to first class status. Actually rtl8187se (aka rtl8185b) seems to have gotten some attention recently: http://lkml.kernel.org/r/can8yu5pgkx9s9dewpfto_ztdr-+wdd5cx2jrv1zd1m1q0bp...@mail.gmail.com The git logs are littered mostly with CodingStyle cleanups, sparse cleanups and such, meanwhile for five years they've had out of bounds reads, overflows, and such for this whole time. Even worse, when one of the drivers gets fixes for actual problems like this, they never make it back to Realtek, who clone the same old shitty driver they shipped last time, and reintroduce new variants of the same damn bugs, and then we import the new turd into staging and start all over again. I get the whole a shit driver is better than no driver, but there's no discernable effort to ever improve this pile, just to keep adding to it. Dave I think there are mostly two major problems with these drivers, besides RealTek still working on a non-mac80211 codebase for USB based devices. The sheer number of slightly different RealTek drivers for similar chipsets, for which RealTek as forked off a dedicated driver each, rather than extending the existing ones. With the other, probably even larger, problem being that it isn't possible to port wireless drivers from non-mac80111 to mac80211 in a gradual fashion, it's always a parallel re-implementation. Just look at the recent history of staging wireless drivers: the successful ones: - csr -- /dev/null - otus -- ar9170 -- carl9170 - ( rt2870 rt3070 ) -- rt2800usb - rt2870 -- rt2800pci - [ at76c503a -- ] at76_usb -- at76c50x-usb [*] not in staging the pending ones - rtl8187se [ -- rtl8180 ] [*] hopefully soon - rtl8188eu -- ? - [ rtl8192du -- ? ] [*] not in staging, [1] - rtl8192e -- ? - rtl8192u -- ? - rtl8192su -- rtl8712 -- ? [ r92su[2] would add cfg80211 support, but it being a fullmac like re-implementation doesn't get it anywhere ] - rtl8821ae [ -- mac80211 port planned for 3.15[3]? ] these devices are, besides rtl8187se (802.11g) and rtl8821ae (802.11ac), all 802.11n compatible, but were quickly EOLed by the vendor, probably making it hard to get enough traction for a proper mac80211 port. Coincidentally these chipsets are also very popular, rtl8187se being the chipset of the early netbook craze, rtl8188eu pretty ubiquitous on embedded platforms, the others making the bulk of aftermarket USB devices. ancient hardware, probably not going anywhere: The below devices are still been sold new - vt6655 -- ? - vt6656 -- ? to mac80211 I have already done the conversion, just some minor things todo LED/ host implementation Should be ready to merge next + 3-4. I will update the TODO file shortly. - wlags49_h2 -- ? - wlags49_h25 -- ? - wlan-ng -- ? This likely leaves staging wireless drivers to small corrections and bugfixes. In the hope that the devices will get enough traction that someone takes up the effort of doing a parallel re-implementation of a proper mac80211 based driver, using the staging source only as reference. For my part, it is an educational exercise. However, I do wonder why I don't simply submit a new driver. There is very little of the staging code left. Regards Malcolm -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH v2 5/5] mutex: Give spinners a chance to spin_on_owner if need_resched() triggered while queued
On Fri, Jan 31, 2014 at 03:09:41PM +0100, Peter Zijlstra wrote:
+struct m_spinlock {
+ struct m_spinlock *next, *prev;
+ int locked; /* 1 if lock acquired */
+};
+
+/*
+ * Using a single mcs node per CPU is safe because mutex_lock() should not be
+ * called from interrupt context and we have preemption disabled over the mcs
+ * lock usage.
+ */
+static DEFINE_PER_CPU(struct m_spinlock, m_node);
+
+static bool m_spin_lock(struct m_spinlock **lock)
+{
+ struct m_spinlock *node = this_cpu_ptr(m_node);
+ struct m_spinlock *prev, *next;
+
+ node-locked = 0;
+ node-next = NULL;
+
+ node-prev = prev = xchg(lock, node);
+ if (likely(prev == NULL))
+ return true;
+
+ ACCESS_ONCE(prev-next) = node;
+
+ /*
+ * Normally @prev is untouchable after the above store; because at that
+ * moment unlock can proceed and wipe the node element from stack.
+ *
+ * However, since our nodes are static per-cpu storage, we're
+ * guaranteed their existence -- this allows us to apply
+ * cmpxchg in an attempt to undo our queueing.
+ */
+
+ while (!smp_load_acquire(node-locked)) {
+ if (need_resched())
+ goto unqueue;
+ arch_mutex_cpu_relax();
+ }
+ return true;
+
+unqueue:
+ /*
+ * Step - A -- stabilize @prev
+ *
+ * Undo our @prev-next assignment; this will make @prev's
+ * unlock()/cancel() wait for a next pointer since @lock points to us
+ * (or later).
+ */
+
+ for (;;) {
+ next = cmpxchg(prev-next, node, NULL); /* A - B,C */
+
+ /*
+ * Because the unlock() path retains @prev-next (for
+ * performance) we must check @node-locked after clearing
+ * @prev-next to see if we raced.
+ *
+ * Ordered by the cmpxchg() above and the conditional-store in
+ * unlock().
+ */
+ if (smp_load_acquire(node-locked)) {
+ /*
+ * OOPS, we were too late, we already got the lock. No
+ * harm done though; @prev is now unused an nobody
+ * cares we frobbed it.
+ */
+ return true;
+ }
+
+ if (next == node)
+ break;
+
+ /*
+ * @prev-next didn't point to us anymore, we didn't own the
+ * lock, so reload and try again.
+ *
+ * Because we observed the new @prev-next, the smp_wmb() at
+ * (C) ensures that we must now observe the new @node-prev.
+ */
+ prev = ACCESS_ONCE(node-prev);
+ }
+
+ /*
+ * Step - B -- stabilize @next
+ *
+ * Similar to unlock(), wait for @node-next or move @lock from @node
+ * back to @prev.
+ */
+
+ for (;;) {
+ if (*lock == node cmpxchg(lock, node, prev) == node) {
+ /*
+ * We were the last queued, we moved @lock back. @prev
+ * will now observe @lock and will complete its
+ * unlock()/cancel().
+ */
+ return false;
+ }
+
+ /*
+ * We must xchg() the @node-next value, because if we were to
+ * leave it in, a concurrent cancel() from @node-next might
+ * complete Step-A and think its @prev is still valid.
+ *
+ * If the concurrent cancel() wins the race, we'll wait for
+ * either @lock to point to us, through its Step-B, or wait for
+ * a new @node-next from its Step-C.
+ */
+ next = xchg(node-next, NULL); /* B - A */
+ if (next)
+ break;
+
+ arch_mutex_cpu_relax();
+ }
+
+ /*
+ * Step - C -- unlink
+ *
+ * @prev is stable because its still waiting for a new @prev-next
+ * pointer, @next is stable because our @node-next pointer is NULL and
+ * it will wait in Step-A.
+ */
+
+ ACCESS_ONCE(next-prev) = prev;
+
+ /*
+ * Ensure that @next-prev is written before we write @prev-next,
+ * this guarantees that when the cmpxchg at (A) fails we must
+ * observe the new prev value.
+ */
+ smp_wmb(); /* C - A */
OK, I've definitely stared at this code for too long :/
I don't think the above barrier is right -- or even required for that
matter. At this point I can't see anything wrong with the order of
either of these stores.
If the latter store hits first an unlock can can happen and release the
@next entry, which is fine as the Step-A loop can deal with that, if the
unlock doesn't happen, we'll simply wait for the first
