Re: [PATCH v2 2/2] kallsyms: handle special absolute symbols

2014-03-12 Thread Rusty Russell
king them absolute symbols is the Right Thing, but requires fixes to
the relocs tool.  So for the moment, we add a --absolute-percpu option
which makes them absolute from a kallsyms perspective:

 $ egrep ' (gdt_|_(stext|_per_cpu_))' /proc/kallsyms # no KASLR
  A __per_cpu_start
 a000 A gdt_page
 00013040 A __per_cpu_end
 802001c8 T _stext
 8099b180 D __per_cpu_offset
 809a3000 D __per_cpu_load
 $ egrep ' (gdt_|_(stext|_per_cpu_))' /proc/kallsyms # With KASLR
  A __per_cpu_start
 a000 A gdt_page
 00013040 A __per_cpu_end
 89c001c8 T _stext
 8a39d180 D __per_cpu_offset
 8a3a5000 D __per_cpu_load

Signed-off-by: Rusty Russell 

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 3c6224728960..21013e739773 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -51,9 +51,14 @@ static struct addr_range text_ranges[] = {
 #define text_range_text (&text_ranges[0])
 #define text_range_inittext (&text_ranges[1])
 
+static struct addr_range percpu_range = {
+   "__per_cpu_start", "__per_cpu_end", -1ULL, 0
+};
+
 static struct sym_entry *table;
 static unsigned int table_size, table_cnt;
 static int all_symbols = 0;
+static int absolute_percpu = 0;
 static char symbol_prefix_char = '\0';
 static unsigned long long kernel_start_addr = 0;
 
@@ -166,6 +171,9 @@ static int read_symbol(FILE *in, struct sym_entry *s)
strcpy((char *)s->sym + 1, str);
s->sym[0] = stype;
 
+   /* Record if we've found __per_cpu_start/end. */
+   check_symbol_range(sym, s->addr, &percpu_range, 1);
+
return 0;
 }
 
@@ -656,6 +664,15 @@ static void sort_symbols(void)
qsort(table, table_cnt, sizeof(struct sym_entry), compare_symbols);
 }
 
+static void make_percpus_absolute(void)
+{
+   unsigned int i;
+
+   for (i = 0; i < table_cnt; i++)
+   if (symbol_in_range(&table[i], &percpu_range, 1))
+   table[i].sym[0] = 'A';
+}
+
 int main(int argc, char **argv)
 {
if (argc >= 2) {
@@ -663,6 +683,8 @@ int main(int argc, char **argv)
for (i = 1; i < argc; i++) {
if(strcmp(argv[i], "--all-symbols") == 0)
all_symbols = 1;
+   else if (strcmp(argv[i], "--absolute-percpu") == 0)
+   absolute_percpu = 1;
else if (strncmp(argv[i], "--symbol-prefix=", 16) == 0) 
{
char *p = &argv[i][16];
/* skip quote */
@@ -679,6 +701,8 @@ int main(int argc, char **argv)
usage();
 
read_map(stdin);
+   if (absolute_percpu)
+   make_percpus_absolute();
sort_symbols();
optimize_token_table();
write_src();
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 2dcb37736d84..86a4fe75f453 100644
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -86,6 +86,10 @@ kallsyms()
kallsymopt="${kallsymopt} --page-offset=$CONFIG_PAGE_OFFSET"
fi
 
+   if [ -n "${CONFIG_X86_64}" ]; then
+   kallsymopt="${kallsymopt} --absolute-percpu"
+   fi
+
local aflags="${KBUILD_AFLAGS} ${KBUILD_AFLAGS_KERNEL}   \
  ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS}"
 

--
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] module: LLVMLinux: Remove unused function warning from __param_check macro

2014-03-10 Thread Rusty Russell
beh...@converseincode.com writes:
> From: Mark Charlebois 
>
> This code makes a compile time type check that is optimized away. Clang
> complains that it generates an unused function.
>
> I believe GCC won't complain for a static inline fuction but would if it
> was just a static function.
>
> Adding the unused attribute to the function declaration removes the warning.
>
> This code works for both gcc and clang.
>
> Signed-off-by: Mark Charlebois 
> Signed-off-by: Behan Webster 

Please include the actual warning clang spits out.  That helps because
(1) I know what you're referring to, and
(2) it helps others if they are later googling for the error.

I don't have any huge objections to this patch (__always_unused) though.

Thanks,
Rusty.
--
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] Taint the kernel for unsafe module options

2014-03-06 Thread Rusty Russell
Daniel Vetter  writes:
> On Thu, Mar 06, 2014 at 11:19:54AM +1030, Rusty Russell wrote:
>> Daniel Vetter  writes:
>> > Users just love to set random piles of options since surely enabling
>> > all the experimental stuff helps. Later on we get bug reports because
>> > it all fell apart.
>> >
>> > Even more fun when it's labelled a regression when some change only
>> > just made the feature possible (e.g. stolen memory fixes suddenly
>> > making fbc possible).
>> >
>> > Make it clear that users are playing with fire here. In drm/i915 all
>> > these options follow the same pattern of using -1 as the per-machine
>> > default, and any other value being used for force the parameter.
>> >
>> > Adding a pile of cc's to solicit input and figure out whether this
>> > would be generally useful - this quick rfc is just for drm/i915.
>> 
>> If this is a good idea, you can write a macro module_param_unsafe_named
>> which is a general wrapper.
>
> For this to work I need to somehow store the safe default value somewhere.
> since with bools or strings there really isn't such a thing, even less
> than with integers where my fairly abitrary -1 choice is already
> restricting. But I don't have a good idea how to do that, since creating a
> local static struct in the macro to store the default + the pointer to the
> storage location feels a bit ugly.

I was thinking that if use the parameter, they get marked unsafe.  If
they use it to set it to the default, Don't Do That.

>> > -module_param_named(modeset, i915.modeset, int, 0400);
>> 
>> Wait, WTF?  Why do you prefix i915 here manually?  That means that
>> the commandline parameter would be "i915.i915.modeset=" and the
>> module parameter would be "i915.modeset="...
>
> Nope, this is the named macro. The name of the param is the first
> parameter to the macro "modeset", "i915.modeset" is just the variable
> it'll get stored in. We've specifically switched to the _named version to
> avoid ugly i915.i915* paramters ;-)

Oh, oops, my bad reading!  I'll shut up now :)

Thanks,
Rusty.
--
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 2/2] kallsyms: handle special absolute symbols

2014-03-06 Thread Rusty Russell
Kees Cook  writes:
> This forces the entire per_cpu range to be reported as absolute without
> losing their linker symbol types. Without this, the variables are
> incorrectly shown as relocated under kASLR.

I like these patches, thanks!

This one's a bit broken, since the zero-based __per_cpu_start/end thing
is an x86-64-ism.  You really do want them relocated on other
platforms, so I think you'll need do make this conditional via
a --per-cpu-absolute flag to kallsyms (which x86-64 would set).

Dumb Q: why don't we actually present these symbols as absolute in
/proc/kallsyms?  Seems like it would be clearer...

Thanks,
Rusty.
--
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] tracing/module: Replace include of tracepoint.h with jump_label.h in module.h

2014-03-06 Thread Rusty Russell
Steven Rostedt  writes:
> Rusty,
>
> Can you give an Acked-by for this patch so that I can include it in my
> 3.15 queue. I have some patches there that need to be added before this
> change can go in.

Acked-by: Rusty Russell 

Thanks,
Rusty.

>
> Thanks,
>
> -- Steve
>
>
> 
> There's nothing in the module.h header that requires tracepoint.h to be
> included, and there may be cases that tracepoint.h may need to include
> module.h, which will cause recursive header issues.
>
> But module.h requires seeing HAVE_JUMP_LABEL which is set in
> jump_label.h which it just coincidentally gets from tracepoint.h.
>
> Cc: Rusty Russell 
> Signed-off-by: Steven Rostedt 
> ---
>  include/linux/module.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index eaf60ff..5a50539 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -15,7 +15,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  #include 
>  
>  #include 
> -- 
> 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: [PATCH] [RFC] Taint the kernel for unsafe module options

2014-03-05 Thread Rusty Russell
Daniel Vetter  writes:
> Users just love to set random piles of options since surely enabling
> all the experimental stuff helps. Later on we get bug reports because
> it all fell apart.
>
> Even more fun when it's labelled a regression when some change only
> just made the feature possible (e.g. stolen memory fixes suddenly
> making fbc possible).
>
> Make it clear that users are playing with fire here. In drm/i915 all
> these options follow the same pattern of using -1 as the per-machine
> default, and any other value being used for force the parameter.
>
> Adding a pile of cc's to solicit input and figure out whether this
> would be generally useful - this quick rfc is just for drm/i915.

If this is a good idea, you can write a macro module_param_unsafe_named
which is a general wrapper.

> -module_param_named(modeset, i915.modeset, int, 0400);

Wait, WTF?  Why do you prefix i915 here manually?  That means that
the commandline parameter would be "i915.i915.modeset=" and the
module parameter would be "i915.modeset="...

Confused,
Rusty.
--
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: [PATCHv3 1/2] mm: introduce vm_ops->map_pages()

2014-03-04 Thread Rusty Russell
Andrew Morton  writes:
> On Thu, 27 Feb 2014 21:53:46 +0200 "Kirill A. Shutemov" 
>  wrote:
>> +
>> +void do_set_pte(struct vm_area_struct *vma, unsigned long address,
>> +struct page *page, pte_t *pte, bool write, bool anon);
>>  #endif
>>  
>>  /*
>
> lguest made a dubious naming decision:
>
> drivers/lguest/page_tables.c:890: error: conflicting types for 'do_set_pte'
> include/linux/mm.h:593: note: previous declaration of 'do_set_pte' was here
>
> I'll rename lguest's do_set_pte() to do_guest_set_pte() as a
> preparatory patch.

s/do_/ if you don't mind; if we're going to prefix it, we don't need the
extra verb.

Thanks,
Rusty.
--
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] tracepoint: Do not waste memory on mods with no tracepoints

2014-02-26 Thread Rusty Russell
Steven Rostedt  writes:
> No reason to allocate tp_module structures for modules that have no
> tracepoints. This just wastes memory.
>
> Fixes: b75ef8b44b1c "Tracepoint: Dissociate from module mutex"
> Cc: sta...@vger.kernel.org # 3.2+

Really?  CC:stable?  To save an insignificant amount of memory?

The definition of stable seems to be shifting away from "fixes for
problems with significant effects".  I obviously missed the memo.

Rusty.



> Cc: Mathieu Desnoyers 
> Signed-off-by: Steven Rostedt 
> ---
>  kernel/tracepoint.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
> index 29f2654..0d4ef26 100644
> --- a/kernel/tracepoint.c
> +++ b/kernel/tracepoint.c
> @@ -636,6 +636,9 @@ static int tracepoint_module_coming(struct module *mod)
>   struct tp_module *tp_mod, *iter;
>   int ret = 0;
>  
> + if (!mod->num_tracepoints)
> + return 0;
> +
>   /*
>* We skip modules that taint the kernel, especially those with 
> different
>* module headers (for forced load), to make sure we don't cause a 
> crash.
> @@ -679,6 +682,9 @@ static int tracepoint_module_going(struct module *mod)
>  {
>   struct tp_module *pos;
>  
> + if (!mod->num_tracepoints)
> + return 0;
> +
>   mutex_lock(&tracepoints_mutex);
>   tracepoint_update_probe_range(mod->tracepoints_ptrs,
>   mod->tracepoints_ptrs + mod->num_tracepoints);
> -- 
> 1.8.1.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: [RFC PATCH] Fix: module signature vs tracepoints: add new TAINT_UNSIGNED_MODULE

2014-02-25 Thread Rusty Russell
Steven Rostedt  writes:
> On Mon, 24 Feb 2014 16:55:36 + (UTC)
> Mathieu Desnoyers  wrote:
>
>> - Original Message -
>> > From: "Steven Rostedt" 
>> > To: "Mathieu Desnoyers" 
>> > Cc: "Ingo Molnar" , linux-kernel@vger.kernel.org, "Ingo 
>> > Molnar" , "Thomas
>> > Gleixner" , "Rusty Russell" , 
>> > "David Howells" ,
>> > "Greg Kroah-Hartman" 
>> > Sent: Monday, February 24, 2014 10:54:54 AM
>> > Subject: Re: [RFC PATCH] Fix: module signature vs tracepoints: add new 
>> > TAINT_UNSIGNED_MODULE
>> > 
>> [...]
>> 
>> (keeping discussion for later, as I'm busy at a client site)
>>  
>> > For now, I'm going to push this in, and also mark it for stable.
>> 
>> Which patch or patches do you plan to pull, and which is marked for stable ?
>
> The one that I replied to. I can't pull the module patch unless I get
> an ack from Rusty.

Ah, I applied it in my tree.  Happy for you to take it though; here's
the variant with an Acked-by from me instead of Signed-off-by if you
want it:

===
From: Mathieu Desnoyers 
Subject: Fix: module signature vs tracepoints: add new TAINT_UNSIGNED_MODULE

Users have reported being unable to trace non-signed modules loaded
within a kernel supporting module signature.

This is caused by tracepoint.c:tracepoint_module_coming() refusing to
take into account tracepoints sitting within force-loaded modules
(TAINT_FORCED_MODULE). The reason for this check, in the first place, is
that a force-loaded module may have a struct module incompatible with
the layout expected by the kernel, and can thus cause a kernel crash
upon forced load of that module on a kernel with CONFIG_TRACEPOINTS=y.

Tracepoints, however, specifically accept TAINT_OOT_MODULE and
TAINT_CRAP, since those modules do not lead to the "very likely system
crash" issue cited above for force-loaded modules.

With kernels having CONFIG_MODULE_SIG=y (signed modules), a non-signed
module is tainted re-using the TAINT_FORCED_MODULE taint flag.
Unfortunately, this means that Tracepoints treat that module as a
force-loaded module, and thus silently refuse to consider any tracepoint
within this module.

Since an unsigned module does not fit within the "very likely system
crash" category of tainting, add a new TAINT_UNSIGNED_MODULE taint flag
to specifically address this taint behavior, and accept those modules
within Tracepoints. This flag is assigned to the letter 'N', since all
the more obvious ones (e.g. 'S') were already taken.

Signed-off-by: Mathieu Desnoyers 
Nacked-by: Ingo Molnar 
CC: Steven Rostedt 
CC: Thomas Gleixner 
CC: David Howells 
CC: Greg Kroah-Hartman 
Acked-by: Rusty Russell 
---
 include/linux/kernel.h| 1 +
 include/trace/events/module.h | 3 ++-
 kernel/module.c   | 4 +++-
 kernel/panic.c| 1 +
 kernel/tracepoint.c   | 5 +++--
 5 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 196d1ea86df0..471090093c67 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -469,6 +469,7 @@ extern enum system_states {
 #define TAINT_CRAP 10
 #define TAINT_FIRMWARE_WORKAROUND  11
 #define TAINT_OOT_MODULE   12
+#define TAINT_UNSIGNED_MODULE  13
 
 extern const char hex_asc[];
 #define hex_asc_lo(x)  hex_asc[((x) & 0x0f)]
diff --git a/include/trace/events/module.h b/include/trace/events/module.h
index 161932737416..1788a02557f4 100644
--- a/include/trace/events/module.h
+++ b/include/trace/events/module.h
@@ -23,7 +23,8 @@ struct module;
 #define show_module_flags(flags) __print_flags(flags, "",  \
{ (1UL << TAINT_PROPRIETARY_MODULE),"P" },  \
{ (1UL << TAINT_FORCED_MODULE), "F" },  \
-   { (1UL << TAINT_CRAP),  "C" })
+   { (1UL << TAINT_CRAP),  "C" },  \
+   { (1UL << TAINT_UNSIGNED_MODULE),   "N" })
 
 TRACE_EVENT(module_load,
 
diff --git a/kernel/module.c b/kernel/module.c
index efa1e6031950..eadf1f1651fb 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1013,6 +1013,8 @@ static size_t module_flags_taint(struct module *mod, char 
*buf)
buf[l++] = 'F';
if (mod->taints & (1 << TAINT_CRAP))
buf[l++] = 'C';
+   if (mod->taints & (1 << TAINT_UNSIGNED_MODULE))
+   buf[l++] = 'N';
/*
 * TAINT_FORCED_RMMOD: could be added.
 * TAINT_UNSAFE_SMP, TAINT_MACHINE_CHECK, TAINT_BAD_PAGE don't
@@ -3214,7 +3216,7 @@ static int lo

Re: [RFC PATCH] Fix: module signature vs tracepoints: add new TAINT_UNSIGNED_MODULE

2014-02-25 Thread Rusty Russell
Johannes Berg  writes:
> Going on a tangent here - our use case is using backported upstream
> kernel modules (https://backports.wiki.kernel.org/) for delivering a
> driver to people who decided that they absolutely need to run with some
> random kernel (e.g. 3.10) but we don't yet support all the driver
> features they want/need in the kernel they picked.

Ah, a user!  See, that's not the "I forgot to sign my modules" case the
others were complaining about.

> We push our code upstream as soon as we can and typically only diverge
> from upstream by a few patches, so saying things like "crap" or "felony
> law breaker" about out-of-tree modules in general makes me furious.

Appreciated and understood.

I have applied Mathieu's patch to my pending tree, with Ingo's Nack
recorded.

Thanks,
Rusty.
--
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] tools: Unify export.h

2014-02-25 Thread Rusty Russell
Borislav Petkov  writes:
> On Tue, Feb 25, 2014 at 12:09:23PM +1030, Rusty Russell wrote:
>> Should we get more ambitious and start a fake-kernel/ directory where
>> we can put userspace equivs/stubs of kernel functionality?
>
> Dunno - people like to do that now, it seems.
>
> In any case, cleanups and unifications like that would definitely help
> if we decide to do that because, in this particular example, we have
> only one export.h to go and fish out.

Agreed.

Acked-by: Rusty Russell 

Cheers,
Rusty
--
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] tools: Unify export.h

2014-02-24 Thread Rusty Russell
Borislav Petkov  writes:
> From: Borislav Petkov 
>
> So tools/ has been growing three, at a different stage of their
> development export.h headers and so we should unite into one. Add
> tools/include/ to the include path of virtio and liblockdep to pick the
> shared header now.

Should we get more ambitious and start a fake-kernel/ directory where
we can put userspace equivs/stubs of kernel functionality?

Cheers,
Rusty.
--
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] kallsyms: fix absolute addresses for kASLR

2014-02-24 Thread Rusty Russell
Kees Cook  writes:
> From: Andy Honig 
>
> Currently symbols that are absolute addresses are incorrectly
> displayed in /proc/kallsyms if the kernel is loaded with kASLR.
>
> The problem was that the scripts/kallsyms.c file which generates
> the array of symbol names and addresses uses an relocatable value
> for all symbols, even absolute symbols.  This patch fixes that.

Hi Andy, Kees,

This is not a good patch.  See the commit where this was
introduced:

[PATCH] relocatable kernel: Fix kallsyms on avr32 after relocatable kernel 
changes

o On some platforms like avr32, section init comes before .text and
  not necessarily a symbol's relative position w.r.t _text is positive.
  In such cases assembler detects the overflow and emits warning. This
  patch fixes it.

Did you just break avr32?

And absolute symbols are supposed to be handled in the other branch:

for (i = 0; i < table_cnt; i++) {
if (toupper(table[i].sym[0]) != 'A') {
if (_text <= table[i].addr)
printf("\tPTR\t_text + %#llx\n",
table[i].addr - _text);
else
printf("\tPTR\t_text - %#llx\n",
_text - table[i].addr);
} else {
printf("\tPTR\t%#llx\n", table[i].addr);
}
}

__per_cpu_start is not an absolute symbol anyway.

You need to fix this properly.
Rusty.

> Several kallsyms output in different boot states for comparison:
>
> $ egrep '_(stext|_per_cpu_(start|end))' /root/kallsyms.nokaslr
>  D __per_cpu_start
> 00014280 D __per_cpu_end
> 810001c8 T _stext
> $ egrep '_(stext|_per_cpu_(start|end))' /root/kallsyms.kaslr1
> 1f20 D __per_cpu_start
> 1f214280 D __per_cpu_end
> a02001c8 T _stext
> $ egrep '_(stext|_per_cpu_(start|end))' /root/kallsyms.kaslr2
> 0d40 D __per_cpu_start
> 0d414280 D __per_cpu_end
> 8e4001c8 T _stext
> $ egrep '_(stext|_per_cpu_(start|end))' /root/kallsyms.kaslr-fixed
>  D __per_cpu_start
> 00014280 D __per_cpu_end
> adc001c8 T _stext
>
> Signed-off-by: Andy Honig 
> Signed-off-by: Kees Cook 
> ---
>  scripts/kallsyms.c |3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> index 10085de886fe..276e84b8a8e5 100644
> --- a/scripts/kallsyms.c
> +++ b/scripts/kallsyms.c
> @@ -330,8 +330,7 @@ static void write_src(void)
>   printf("\tPTR\t_text + %#llx\n",
>   table[i].addr - _text);
>   else
> - printf("\tPTR\t_text - %#llx\n",
> - _text - table[i].addr);
> + printf("\tPTR\t%#llx\n", table[i].addr);
>   } else {
>   printf("\tPTR\t%#llx\n", table[i].addr);
>   }
--
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 net V2] virtio-net: alloc big buffers also when guest can receive UFO

2014-02-23 Thread Rusty Russell
"Michael S. Tsirkin"  writes:
> On Fri, Feb 21, 2014 at 01:08:04PM +0800, Jason Wang wrote:
>> We should alloc big buffers also when guest can receive UFO
>> pakcets to let the big packets be fit into guest rx buffer.
>
> s/pakcets/packets/
>
> s/be fit/fit/
>
> otherwise:
>
> Acked-by: Michael S. Tsirkin 

Indeed.

Acked-by: Rusty Russell 

Thanks,
Rusty.
--
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] There should have a warn/log when out-of-tree module is loaded

2014-02-23 Thread Rusty Russell
Xudong Zhang  writes:
> I can not get any hint when I found that my kernel is tainted (out of
> tree module is loaded). And I checked all loaded and loadable modules
> but it showed that all of them are intree modules.
> I think we need a warn when out of tree module is loaded.
>
> Signed-off-by: Xudong Zhang 

This would be an escalation.

This taint only has an effect on oops messages, and
/proc/sys/kernel/tainted.

Cheers,
Rusty.
--
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] virtio: Use pci_enable_msix_exact() instead of pci_enable_msix()

2014-02-23 Thread Rusty Russell
"Michael S. Tsirkin"  writes:
> On Fri, Feb 21, 2014 at 06:01:28PM +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()  or pci_enable_msi_exact()
>> and pci_enable_msix_range() or pci_enable_msix_exact()
>> interfaces.
>
>
> Acked-by: Michael S. Tsirkin 

Thanks, applied.

Cheers,
Rusty.
--
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] Fix: module signature vs tracepoints: add new TAINT_UNSIGNED_MODULE

2014-02-20 Thread Rusty Russell
Steven Rostedt  writes:
> I need to clean out my email box. This email was hidden in between a
> pile of other crap email.
>
> On Fri, 14 Feb 2014 11:21:19 +1030
> Rusty Russell  wrote:
>
>> Steven Rostedt  writes:
>> > On Thu, 13 Feb 2014 13:54:42 +1030
>> > Rusty Russell  wrote:
>> >
>> >
>> >> I'm ambivalent towards out-of-tree modules, so not tempted unless I see
>> >> a bug report indicating a concrete problem.  Then we can discuss...
>> >
>> > As I replied in another email, this is a concrete problem, and affects
>> > in-tree kernel modules.
>> >
>> > If you have the following in your .config:
>> >
>> > CONFIG_MODULE_SIG=y
>> > # CONFIG_MODULE_SIG_FORCE is not set
>> > # CONFIG_MODULE_SIG_ALL is not set
>> 
>> This means you've set the "I will arrange my own module signing" config
>> option:
>> 
>>Sign all modules during make modules_install. Without this option,
>>modules must be signed manually, using the scripts/sign-file tool.
>> 
>> comment "Do not forget to sign required modules with scripts/sign-file"
>>  depends on MODULE_SIG_FORCE && !MODULE_SIG_ALL
>> 
>> Then you didn't do that.  You broke it, you get to keep both pieces.
>
> In this case we should fail the module load all together, and require
> insmod to add the --force flag to load it. Why the hell are we setting
> a FORCED_MODULE flag when no module was forced

If this mistake of creating unsigned modules is common, then it would be
friendly to do something about it, yes.

Perhaps we should append UNSIGNED to vermagic, and then strip that out
when we sign the module?  That would have this effect.

Cheers,
Rusty.
--
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: [tip:x86/asmlinkage] lto: Handle LTO common symbols in module loader

2014-02-20 Thread Rusty Russell
tip-bot for Joe Mario  writes:
> Commit-ID:  80375980f1608f43b47abc2671456b23ec68c434
> Gitweb: http://git.kernel.org/tip/80375980f1608f43b47abc2671456b23ec68c434
> Author: Joe Mario 
> AuthorDate: Sat, 8 Feb 2014 09:01:09 +0100
> Committer:  H. Peter Anvin 
> CommitDate: Thu, 13 Feb 2014 20:24:50 -0800
>
> lto: Handle LTO common symbols in module loader
>
> Here is the workaround I made for having the kernel not reject modules
> built with -flto.  The clean solution would be to get the compiler to not
> emit the symbol.  Or if it has to emit the symbol, then emit it as
> initialized data but put it into a comdat/linkonce section.

Gah, as I said, fix the damn comment!

>   case SHN_COMMON:
> + /* Ignore common symbols */
> + if (!strncmp(name, "__gnu_lto", 9))
> + break;
> +
>   /* We compiled with -fno-common.  These are not

/* Ignore common symbols */ is so bad, it's not even wrong.

Cheers,
Rusty.
--
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] virtio: Use pci_enable_msix_range() instead of pci_enable_msix()

2014-02-19 Thread Rusty Russell
Alexander Gordeev  writes:
> As result 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 
> Cc: Rusty Russell 
> Cc: "Michael S. Tsirkin" 
> Cc: virtio-...@lists.oasis-open.org
> Cc: virtualizat...@lists.linux-foundation.org
> Cc: linux-...@vger.kernel.org
> ---
>  drivers/virtio/virtio_pci.c |8 +++-
>  1 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
> index a416f9b..dea042c 100644
> --- a/drivers/virtio/virtio_pci.c
> +++ b/drivers/virtio/virtio_pci.c
> @@ -333,11 +333,9 @@ static int vp_request_msix_vectors(struct virtio_device 
> *vdev, int nvectors,
>   for (i = 0; i < nvectors; ++i)
>   vp_dev->msix_entries[i].entry = i;
>  
> - /* pci_enable_msix returns positive if we can't get this many. */
> - err = pci_enable_msix(vp_dev->pci_dev, vp_dev->msix_entries, nvectors);
> - if (err > 0)
> - err = -ENOSPC;
> - if (err)
> + err = pci_enable_msix_range(vp_dev->pci_dev,
> + vp_dev->msix_entries, nvectors, nvectors);
> + if (err < 0)
>   goto error;
>   vp_dev->msix_enabled = 1;

I've put this in my pending queue, but in case someone else wants to
take it:

Acked-by: Rusty Russell 

Cheers,
Rusty.

--
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] Fix: module signature vs tracepoints: add new TAINT_UNSIGNED_MODULE

2014-02-16 Thread Rusty Russell
Steven Rostedt  writes:
> On Thu, 13 Feb 2014 13:54:42 +1030
> Rusty Russell  wrote:
>
>
>> I'm ambivalent towards out-of-tree modules, so not tempted unless I see
>> a bug report indicating a concrete problem.  Then we can discuss...
>
> As I replied in another email, this is a concrete problem, and affects
> in-tree kernel modules.
>
> If you have the following in your .config:
>
> CONFIG_MODULE_SIG=y
> # CONFIG_MODULE_SIG_FORCE is not set
> # CONFIG_MODULE_SIG_ALL is not set

This means you've set the "I will arrange my own module signing" config
option:

  Sign all modules during make modules_install. Without this option,
  modules must be signed manually, using the scripts/sign-file tool.

comment "Do not forget to sign required modules with scripts/sign-file"
depends on MODULE_SIG_FORCE && !MODULE_SIG_ALL

Then you didn't do that.  You broke it, you get to keep both pieces.

Again: is there an actual valid use case?
Rusty.
--
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] Fix: module signature vs tracepoints: add new TAINT_UNSIGNED_MODULE

2014-02-12 Thread Rusty Russell
Steven Rostedt  writes:
> On Tue, 11 Feb 2014 08:27:38 +0100
> Ingo Molnar  wrote:
>
>> 
>> * Mathieu Desnoyers  wrote:
>> 
>> > Users have reported being unable to trace non-signed modules loaded 
>> > within a kernel supporting module signature.
>> 
>> External modules should strive to get out of the 'crap' and
>> 'felony law breaker' categories and we should not make it
>> easier for them to linger in a broken state.
>> 
>> Nacked-by: Ingo Molnar 

Well, distributions which sign their modules are sending a pretty strong
"go away" signal already.

I'm ambivalent towards out-of-tree modules, so not tempted unless I see
a bug report indicating a concrete problem.  Then we can discuss...

Cheers,
Rusty.
--
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 05/17] lto: Handle LTO common symbols in module loader

2014-02-12 Thread Rusty Russell
Andi Kleen  writes:
> From: Joe Mario 
>
> Here is the workaround I made for having the kernel not reject modules
> built with -flto.  The clean solution would be to get the compiler to not
> emit the symbol.  Or if it has to emit the symbol, then emit it as
> initialized data but put it into a comdat/linkonce section.
>
> Minor tweaks by AK over Joe's patch.

Patch is fine, but what's with the comment?

>   switch (sym[i].st_shndx) {
>   case SHN_COMMON:
> + /* Ignore common symbols */
> + if (!strncmp(name, "__gnu_lto", 9))
> + break;
> +

You mean, "/* Ignore symbols from -flto */"?

Other than that, I'm happy for this to go via some other tree:

Acked-by: Rusty Russell 

Thanks,
Rusty.
--
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 17/17] lto, module: Warn about modules that are not fully LTOed

2014-02-12 Thread Rusty Russell
Andi Kleen  writes:
> When __gnu_lto_* is present that means that the module hasn't run with
> LTO yet.

In practice, this means they didn't build their kernel properly, right?
It shouldn't break anything, but it seems really weird.  And how many
times will the prink fire on a single module?

Seems like a job for pr_warn?

Thanks,
Rusty.

> ---
>  kernel/module.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index b99e801..2052155 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -1949,8 +1949,11 @@ static int simplify_symbols(struct module *mod, const 
> struct load_info *info)
>   switch (sym[i].st_shndx) {
>   case SHN_COMMON:
>   /* Ignore common symbols */
> - if (!strncmp(name, "__gnu_lto", 9))
> + if (!strncmp(name, "__gnu_lto", 9)) {
> + printk("%s: module not link time optimized\n",
> +mod->name);
>   break;
> + }
>  
>   /* We compiled with -fno-common.  These are not
>  supposed to happen.  */
> -- 
> 1.8.5.2
--
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] Detect section mismatches in thumb relocations

2014-02-01 Thread Rusty Russell
David Long  writes:
> From: "David A. Long" 
>
> Add processing for normally encountered thumb relocation types so that
> section mismatches will be detected.
>
> Signed-off-by: David A. Long 

Happiest for this to go through an ARM tree, so:

Acked-by: Rusty Russell 

Cheers,
Rusty.

> ---
>  scripts/mod/modpost.c | 13 +
>  1 file changed, 13 insertions(+)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 1785576..9e6c996 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1498,6 +1498,16 @@ static int addend_386_rel(struct elf_info *elf, 
> Elf_Shdr *sechdr, Elf_Rela *r)
>  #define R_ARM_JUMP24 29
>  #endif
>  
> +#ifndef  R_ARM_THM_CALL
> +#define  R_ARM_THM_CALL  10
> +#endif
> +#ifndef  R_ARM_THM_JUMP24
> +#define  R_ARM_THM_JUMP2430
> +#endif
> +#ifndef  R_ARM_THM_JUMP19
> +#define  R_ARM_THM_JUMP1951
> +#endif
> +
>  static int addend_arm_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela 
> *r)
>  {
>   unsigned int r_typ = ELF_R_TYPE(r->r_info);
> @@ -1511,6 +1521,9 @@ static int addend_arm_rel(struct elf_info *elf, 
> Elf_Shdr *sechdr, Elf_Rela *r)
>   case R_ARM_PC24:
>   case R_ARM_CALL:
>   case R_ARM_JUMP24:
> + case R_ARM_THM_CALL:
> + case R_ARM_THM_JUMP24:
> + case R_ARM_THM_JUMP19:
>   /* From ARM ABI: ((S + A) | T) - P */
>   r->r_addend = (int)(long)(elf->hdr +
> sechdr->sh_offset +
> -- 
> 1.8.1.2
--
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] hw_random: cleanup in hwrng_register()

2014-02-01 Thread Rusty Russell
Dan Carpenter  writes:
> My static checker complains that:
>
>   drivers/char/hw_random/core.c:341 hwrng_register()
>   warn: we tested 'old_rng' before and it was 'false'
>
> The problem is that sometimes we test "if (!old_rng)" and sometimes we
> test "if (must_register_misc)".  The static checker knows they are
> equivalent but a human being reading the code could easily be confused.
>
> I have simplified the code by removing the "must_register_misc" variable
> and I have removed the redundant check on "if (!old_rng)".
>
> Signed-off-by: Dan Carpenter 

Yeah, clearer too.

Reviewed-by: Rusty Russell 

Thanks,
Rusty.

>
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index b9495a8c05c6..463382036a01 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -301,7 +301,6 @@ err_misc_dereg:
>  
>  int hwrng_register(struct hwrng *rng)
>  {
> - int must_register_misc;
>   int err = -EINVAL;
>   struct hwrng *old_rng, *tmp;
>  
> @@ -326,7 +325,6 @@ int hwrng_register(struct hwrng *rng)
>   goto out_unlock;
>   }
>  
> - must_register_misc = (current_rng == NULL);
>   old_rng = current_rng;
>   if (!old_rng) {
>   err = hwrng_init(rng);
> @@ -335,13 +333,11 @@ int hwrng_register(struct hwrng *rng)
>   current_rng = rng;
>   }
>   err = 0;
> - if (must_register_misc) {
> + if (!old_rng) {
>   err = register_miscdev();
>   if (err) {
> - if (!old_rng) {
> - hwrng_cleanup(rng);
> - current_rng = NULL;
> - }
> + hwrng_cleanup(rng);
> + current_rng = NULL;
>   goto out_unlock;
>   }
>   }
--
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 2/6] virtio_blk: don't crash, report error if virtqueue is broken.

2014-02-01 Thread Rusty Russell
Heinz Graalfs  writes:
> On 15/01/14 03:36, Rusty Russell wrote:
>> A bad implementation of virtio might cause us to mark the virtqueue
>> broken: we'll dev_err() in that case, and the device is useless, but
>> let's not BUG_ON().
>>
>> ENOMEM or ENOSPC implies the ring is full, and we should try again
>> later (-ENOMEM is documented to happen, but doesn't, as we fall
>> through to ENOSPC).
>>
>> EIO means it's broken.
>>
>> Signed-off-by: Rusty Russell 
>> ---
>>   drivers/block/virtio_blk.c | 10 --
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
>> index 6a680d4de7f1..704d6c814c17 100644
>> --- a/drivers/block/virtio_blk.c
>> +++ b/drivers/block/virtio_blk.c
>> @@ -158,6 +158,7 @@ static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx, 
>> struct request *req)
>>  unsigned long flags;
>>  unsigned int num;
>>  const bool last = (req->cmd_flags & REQ_END) != 0;
>> +int err;
>>
>>  BUG_ON(req->nr_phys_segments + 2 > vblk->sg_elems);
>>
>> @@ -198,11 +199,16 @@ static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx, 
>> struct request *req)
>>  }
>>
>>  spin_lock_irqsave(&vblk->vq_lock, flags);
>> -if (__virtblk_add_req(vblk->vq, vbr, vbr->sg, num) < 0) {
>> +err = __virtblk_add_req(vblk->vq, vbr, vbr->sg, num);
>> +if (err) {
>>  virtqueue_kick(vblk->vq);
>
> the kick might fail here after a request was successfully added.

It could, but it we're already in the error path, so we can't do
anything about it.

>>  spin_unlock_irqrestore(&vblk->vq_lock, flags);
>>  blk_mq_stop_hw_queue(hctx);
>> -return BLK_MQ_RQ_QUEUE_BUSY;
>> +/* Out of mem doesn't actually happen, since we fall back
>> + * to direct descriptors */
>> +if (err == -ENOMEM || err == -ENOSPC)
>> +return BLK_MQ_RQ_QUEUE_BUSY;
>> +return BLK_MQ_RQ_QUEUE_ERROR;
>>  }
>>
>>  if (last)
>>

Cheers,
Rusty.
--
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] module: allow multiple calls to MODULE_DEVICE_TABLE() per module

2014-01-28 Thread Rusty Russell
Tom Gundersen  writes:
> Commit 78551277e4df5: "Input: i8042 - add PNP modaliases" had a bug, where the
> second call to MODULE_DEVICE_TABLE() overrode the first resulting in not all
> the modaliases being exposed.
>
> This fixes the problem by including the name of the device_id table in the
> __mod_*_device_table alias, allowing us to export several device_id tables
> per module.

Thanks, applied.

I also put this on top:

module: remove MODULE_GENERIC_TABLE

MODULE_DEVICE_TABLE() calles MODULE_GENERIC_TABLE(); make it do the
work directly.  This also removes a wart introduced in the last patch,
where the alias is defined to be an unknown struct type "struct
type##__##name##_device_id" instead of "struct type##_device_id" (it's
an extern so GCC doesn't care, but it's wrong).

The other user of MODULE_GENERIC_TABLE (ISAPNP_CARD_TABLE) is unused,
so delete it.

Signed-off-by: Rusty Russell 

diff --git a/include/linux/isapnp.h b/include/linux/isapnp.h
index e2d28b026a8c..3c77bf9b1efd 100644
--- a/include/linux/isapnp.h
+++ b/include/linux/isapnp.h
@@ -56,10 +56,6 @@
 #define ISAPNP_DEVICE_ID(_va, _vb, _vc, _function) \
{ .vendor = ISAPNP_VENDOR(_va, _vb, _vc), .function = 
ISAPNP_FUNCTION(_function) }
 
-/* export used IDs outside module */
-#define ISAPNP_CARD_TABLE(name) \
-   MODULE_GENERIC_TABLE(isapnp_card, name)
-
 struct isapnp_card_id {
unsigned long driver_data;  /* data private to the driver */
unsigned short card_vendor, card_device;
diff --git a/include/linux/module.h b/include/linux/module.h
index ad18f6006f86..5686b37e11d6 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -82,15 +82,6 @@ void sort_extable(struct exception_table_entry *start,
 void sort_main_extable(void);
 void trim_init_extable(struct module *m);
 
-#ifdef MODULE
-#define MODULE_GENERIC_TABLE(gtype, name)  \
-extern const struct gtype##_id __mod_##gtype##_table   \
-  __attribute__ ((unused, alias(__stringify(name
-
-#else  /* !MODULE */
-#define MODULE_GENERIC_TABLE(gtype, name)
-#endif
-
 /* Generic info of form tag = "info" */
 #define MODULE_INFO(tag, info) __MODULE_INFO(tag, tag, info)
 
@@ -141,8 +132,14 @@ extern const struct gtype##_id __mod_##gtype##_table   
\
 /* What your module does. */
 #define MODULE_DESCRIPTION(_description) MODULE_INFO(description, _description)
 
-#define MODULE_DEVICE_TABLE(type, name)\
-  MODULE_GENERIC_TABLE(type##__##name##_device, name)
+#ifdef MODULE
+/* Creates an alias so file2alias.c can find device table. */
+#define MODULE_DEVICE_TABLE(type, name)
\
+  extern const struct type##_device_id __mod_##type##__##name##_device_table \
+  __attribute__ ((unused, alias(__stringify(name
+#else  /* !MODULE */
+#define MODULE_DEVICE_TABLE(type, name)
+#endif
 
 /* Version of form [:][-].
  * Or for CVS/RCS ID version, everything but the number is stripped.
--
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] module: allow multiple calls to MODULE_DEVICE_TABLE() per module

2014-01-27 Thread Rusty Russell
Greg Kroah-Hartman  writes:
> On Mon, Jan 27, 2014 at 08:09:55PM +0100, Tom Gundersen wrote:
>> Commit 78551277e4df5: "Input: i8042 - add PNP modaliases" had a bug, where 
>> the
>> second call to MODULE_DEVICE_TABLE() overrode the first resulting in not all
>> the modaliases being exposed.
>> 
>> This fixes the problem by including the name of the device_id table in the
>> __mod_*_device_table alias, allowing us to export several device_id tables
>> per module.
>> 
>> Suggested-by: Kay Sievers 
>> Cc: Dmitry Torokhov 
>> Cc: Greg Kroah-Hartman 
>> Cc: Rusty Russell 
>> ---
>>  include/linux/module.h   |  2 +-
>>  scripts/mod/file2alias.c | 14 +-
>>  2 files changed, 10 insertions(+), 6 deletions(-)
>
> Ah, very nice, I've wanted this for a while now, it would make a number
> of different drivers much smaller and simpler to add new device ids to
> (no multiple lists of ids, one for the module loader and one for the
> sub-driver that is in the single file.)

You never asked :(

I've applied, this, but I'm actually amazed this patch works.  C is
weird sometimes.  It changes declarations of the form:

extern const struct pci_device_id __mod_pci_device_id_table
__attribute__ ((unused, alias("e1000_pci_tbl"));

Into:

extern const struct pci__e1000_pci_tbl_device_id 
__mod_pci__e1000_pci_tbl_device_id_table
__attribute__ ((unused, alias("e1000_pci_tbl"));

Now, that's a completely unknown type, but gcc doesn't care because it's
just an extern declaration.  It does insert the alias, which is all we
care about.

We would normally use a special section for this, so it's mainly
historical.  Now we have DEFINE_PCI_DEVICE_TABLE etc, we should
use those to put it in a special section (eg. "pci_ids") and
grab that directly.

Volunteers welcome :)
Rusty.
--
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] module: allow multiple calls to MODULE_DEVICE_TABLE() per module

2014-01-27 Thread Rusty Russell
Tom Gundersen  writes:
> Commit 78551277e4df5: "Input: i8042 - add PNP modaliases" had a bug, where the
> second call to MODULE_DEVICE_TABLE() overrode the first resulting in not all
> the modaliases being exposed.

No Signed-off-by?

Thanks,
Rusty.

>
> This fixes the problem by including the name of the device_id table in the
> __mod_*_device_table alias, allowing us to export several device_id tables
> per module.
>
> Suggested-by: Kay Sievers 
> Cc: Dmitry Torokhov 
> Cc: Greg Kroah-Hartman 
> Cc: Rusty Russell 
> ---
>  include/linux/module.h   |  2 +-
>  scripts/mod/file2alias.c | 14 +-
>  2 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 15cd6b1..7732d76 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -143,7 +143,7 @@ extern const struct gtype##_id __mod_##gtype##_table  
> \
>  #define MODULE_DESCRIPTION(_description) MODULE_INFO(description, 
> _description)
>  
>  #define MODULE_DEVICE_TABLE(type,name)   \
> -  MODULE_GENERIC_TABLE(type##_device,name)
> +  MODULE_GENERIC_TABLE(type##__##name##_device,name)
>  
>  /* Version of form [:][-].
> Or for CVS/RCS ID version, everything but the number is stripped.
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index 2370863..6778381 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -42,7 +42,7 @@ typedef unsigned char   __u8;
>  
>  /* This array collects all instances that use the generic do_table */
>  struct devtable {
> - const char *device_id; /* name of table, __mod__device_table. */
> + const char *device_id; /* name of table, __mod___*_device_table. 
> */
>   unsigned long id_size;
>   void *function;
>  };
> @@ -146,7 +146,8 @@ static void device_id_check(const char *modname, const 
> char *device_id,
>  
>   if (size % id_size || size < id_size) {
>   fatal("%s: sizeof(struct %s_device_id)=%lu is not a modulo "
> -   "of the size of section __mod_%s_device_table=%lu.\n"
> +   "of the size of "
> +   "section __mod_%s___device_table=%lu.\n"
> "Fix definition of struct %s_device_id "
> "in mod_devicetable.h\n",
> modname, device_id, id_size, device_id, size, device_id);
> @@ -1206,7 +1207,7 @@ void handle_moddevtable(struct module *mod, struct 
> elf_info *info,
>  {
>   void *symval;
>   char *zeros = NULL;
> - const char *name;
> + const char *name, *identifier;
>   unsigned int namelen;
>  
>   /* We're looking for a section relative symbol */
> @@ -1217,7 +1218,7 @@ void handle_moddevtable(struct module *mod, struct 
> elf_info *info,
>   if (ELF_ST_TYPE(sym->st_info) != STT_OBJECT)
>   return;
>  
> - /* All our symbols are of form __mod_XXX_device_table. */
> + /* All our symbols are of form 
> __moddevice_table. */
>   name = strstr(symname, "__mod_");
>   if (!name)
>   return;
> @@ -1227,7 +1228,10 @@ void handle_moddevtable(struct module *mod, struct 
> elf_info *info,
>   return;
>   if (strcmp(name + namelen - strlen("_device_table"), "_device_table"))
>   return;
> - namelen -= strlen("_device_table");
> + identifier = strstr(name, "__");
> + if (!identifier)
> + return;
> + namelen = identifier - name;
>  
>   /* Handle all-NULL symbols allocated into .bss */
>   if (info->sechdrs[get_secindex(info, sym)].sh_type & SHT_NOBITS) {
> -- 
> 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: [PATCH] virtio_balloon: don't call virtio_has_feature() twice on init_vqs()

2014-01-27 Thread Rusty Russell
Leandro Dorileo  writes:
> Cchange init_vqs() to avoid calling twice the virtio_has_feature()
> - attempting to find out if VIRTIO_BALLOON_F_STATS_VQ feature was negotiated -
> consequently we prevent unnecessarily running the drivers' feature_table more
> than needed.
>
> Signed-off-by: Leandro Dorileo 

Hi Leandro,

This seems like a premature optimization.  The current code is
fairly clear, and there's no performance issue with init_vqs.

Am I missing something?
Rusty.

> ---
>  drivers/virtio/virtio_balloon.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 34bdaba..41771c1 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -320,19 +320,21 @@ static int init_vqs(struct virtio_balloon *vb)
>   vq_callback_t *callbacks[] = { balloon_ack, balloon_ack, stats_request 
> };
>   const char *names[] = { "inflate", "deflate", "stats" };
>   int err, nvqs;
> + bool stats;
>  
>   /*
>* We expect two virtqueues: inflate and deflate, and
>* optionally stat.
>*/
> - nvqs = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ) ? 3 : 2;
> + stats = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ);
> + nvqs = stats ? 3 : 2;
>   err = vb->vdev->config->find_vqs(vb->vdev, nvqs, vqs, callbacks, names);
>   if (err)
>   return err;
>  
>   vb->inflate_vq = vqs[0];
>   vb->deflate_vq = vqs[1];
> - if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) {
> + if (stats) {
>   struct scatterlist sg;
>   vb->stats_vq = vqs[2];
>  
> -- 
> 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: [RFC PATCH 1/1] module: Make wait module's refcount to zero procedure as async

2014-01-27 Thread Rusty Russell
Jan Engelhardt  writes:
> On Monday 2013-09-16 05:47, Rusty Russell wrote:
>>
>>Here's what I've got in my pending-rebases tree.
>>
>>@@ -842,6 +818,11 @@ SYSCALL_DEFINE2(delete_module, const char __user *, 
>>name_user,
>>  return -EFAULT;
>>  name[MODULE_NAME_LEN-1] = '\0';
>> 
>>+ if (!(flags & O_NONBLOCK)) {
>>+ printk(KERN_WARNING
>>+"waiting module removal not supported: please upgrade");
>>+ }
>>+
>
> This patch has come to my attention via the opensuse lists, where
> a poster reported about this new user-visible message (it appears
> in dmesg!) and rightfully asked: upgrade what component?
>
> It's probably the userspace module loading utility, but it _really_
> should say so. There's good room to read that message - a few months
> into the future - as "upgrade your kernel" instead ;)

True, but honestly I never expected to see it!

Please try to discover what is doing rmmod --wait?

Thanks,
Rusty.
--
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 2/2] module: use pr_cont

2014-01-27 Thread Rusty Russell
Jiri Slaby  writes:
> When dumping loaded modules, we print them one by one in separate
> printks. Let's use pr_cont as they are continuation prints.

Applied.

Thanks,
Rusty.
--
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: Deadlock between cpu_hotplug_begin and cpu_add_remove_lock

2014-01-23 Thread Rusty Russell
"Srivatsa S. Bhat"  writes:
> On 01/23/2014 07:59 AM, Rusty Russell wrote:
>> "Srivatsa S. Bhat"  writes:
>>> On 01/22/2014 02:00 PM, Srivatsa S. Bhat wrote:
>>>> Hi Paul,
>> 
>> I find an old patch for register_allcpu_notifier(), but the "bool
>> replay_history" should be eliminated (always true): it's too weird.
>> 
>
> Sorry, I didn't get this part. Why do you say that replay_history
> will always be true?

OK, let me start again and try to explain myself properly:

register_cpu_notifier is a bad API.  It's hard to get right because:
1) You need to loop over online (or present) cpus once before you call
   it.
2) You have to beware the race between the loop and registration, but
   much example code happens at boot time where it doesn't matter,
   so random author is likely to copy that and have a race.
3) You have two paths doing the same thing: the loop which is run on
   every machine (cpu hotplug or not), and the notifier callback which
   is run far less rarely.

What we actually *want* is a routine which will reliably call for every
current and future CPU, and then there are very few places which should
use the current register_cpu_notifier().

ie. halfway between register_cpu_notifier() (too racy) and
register_allcpu_notifier() (too simplified).

Let's call it register_cpu_callback / unregister_cpu_callback?

> By the way, I'm still tempted to try out the simpler-looking alternative
> idea of exporting cpu_maps_update_begin() and cpu_maps_update_done()
> and then mandating that the callers do:
>
> cpu_maps_update_begin();
> for_each_online_cpu(cpu) {
>   ...
> }
>
> __register_cpu_notifier(); // this doesn't take the add_remove_lock
> cpu_maps_update_done();

Sure, fix this one for -stable.  But let's create an idiom we can be
proud of for the longer term.

Thanks,
Rusty.
--
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: Deadlock between cpu_hotplug_begin and cpu_add_remove_lock

2014-01-22 Thread Rusty Russell
"Srivatsa S. Bhat"  writes:
> On 01/22/2014 02:00 PM, Srivatsa S. Bhat wrote:
>> Hi Paul,

I find an old patch for register_allcpu_notifier(), but the "bool
replay_history" should be eliminated (always true): it's too weird.

Then we should get rid of register_cpu_notifier, or at least hide it.

Thanks,
Rusty.
--
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] scripts/gcc-version.sh: handle CC="gcc -m32"

2014-01-21 Thread Rusty Russell
Rusty Russell  writes:
> Michal Marek  writes:
>>> gcc: warning: ‘-mcpu=’ is deprecated; use ‘-mtune=’ or ‘-march=’ instead
>>> gcc: warning: ‘-mcpu=’ is deprecated; use ‘-mtune=’ or ‘-march=’ instead
>>> kernel/bounds.c:1:0: error: CPU you selected does not support x86-64 
>>> instruction set
>>>  /*
>>>  ^
>>> kernel/bounds.c:1:0: warning: -mregparm is ignored in 64-bit mode [enabled 
>>> by default]
>>> make[1]: *** [kernel/bounds.s] Error 1
>>> make: *** [prepare0] Error 2

Sorry, ignore this report.

In case anyone else hits this: was resolved by installing more 32 bit headers
(Ubuntu's libc6-dev-i386 package, in this case).

Thanks,
Rusty.
--
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 31/73] module: relocate module_init from init.h to module.h

2014-01-21 Thread Rusty Russell
Paul Gortmaker  writes:
> Modular users will always be users of init functionality, but
> users of init functionality are not necessarily always modules.
>
> Hence any functionality like module_init and module_exit would
> be more at home in the module.h file.  And module.h should
> explicitly include init.h to make the dependency clear.
>
> We've already done all the legwork needed to ensure that this
> move does not cause any build regressions due to implicit
> header file include assumptions about where module_init lives.
>
> Cc: Rusty Russell 
> Signed-off-by: Paul Gortmaker 

Acked-by: Rusty Russell 

Want to delete the extraneous semicolons, for bonus points? :)

> +#define module_init(x)   __initcall(x);
...
> +#define module_exit(x)   __exitcall(x);

Cheers,
Rusty.
--
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.13-rc5] module: Add missing newline in printk call.

2014-01-20 Thread Rusty Russell
Tetsuo Handa  writes:
> Rusty, would you pick up this patch?
>
> This message was added in 3.13-rc1. Thus, should be fixed in 3.13.

Thanks, applied.  It's a bit trivial for a CC:stable though.

Cheers,
Rusty.

> Tetsuo Handa wrote:
>> From cc90e27d5cda227e7a0cbeb5de3cc1cbb1595dfa Mon Sep 17 00:00:00 2001
>> From: Tetsuo Handa 
>> Date: Mon, 23 Dec 2013 15:52:42 +0900
>> Subject: [PATCH] module: Add missing newline in printk call.
>> 
>> Add missing \n and also follow commit bddb12b3 "kernel/module.c: use 
>> pr_foo()".
>> 
>> Signed-off-by: Tetsuo Handa 
>> ---
>>  kernel/module.c |6 ++
>>  1 files changed, 2 insertions(+), 4 deletions(-)
>> 
>> diff --git a/kernel/module.c b/kernel/module.c
>> index f5a3b1e..d24fcf2 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -815,10 +815,8 @@ SYSCALL_DEFINE2(delete_module, const char __user *, 
>> name_user,
>>  return -EFAULT;
>>  name[MODULE_NAME_LEN-1] = '\0';
>>  
>> -if (!(flags & O_NONBLOCK)) {
>> -printk(KERN_WARNING
>> -   "waiting module removal not supported: please upgrade");
>> -}
>> +if (!(flags & O_NONBLOCK))
>> +pr_warn("waiting module removal not supported: please 
>> upgrade\n");
>>  
>>  if (mutex_lock_interruptible(&module_mutex) != 0)
>>  return -EINTR;
>> -- 
>> 1.7.1
>> 
--
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 3/4] virtio_balloon: add pressure notification via a new virtqueue

2014-01-19 Thread Rusty Russell
Luiz Capitulino  writes:
> On Fri, 17 Jan 2014 09:10:47 +1030
> Rusty Russell  wrote:
>
>> Luiz Capitulino  writes:
>> > From: Luiz capitulino 
>> >
>> > This commit adds support to a new virtqueue called message virtqueue.
>> 
>> OK, this needs a lot of thought (especially since reworking the virtio
>> balloon is on the TODO list for the OASIS virtio technical committee...)
>> 
>> But AFAICT this is a way of explicitly saying "no" to the host's target
>> (vs the implicit method of just not meeting the target).  I'm not sure
>> that gives enough information to the host.  On the other hand, I'm not
>> sure what information we *can* give.
>> 
>> Should we instead be counter-proposing a target?
>
> The problem is how to estimate a target value. I found it simpler
> to just try to obey what the host is asking for (and fail if not
> possible) than trying to make the guest negotiate with the host.

Understood, but we already do this the other way where the host tells
the guest how much memory to give up.

And is a guest expected to automatically inflate the balloon even if the
host doesn't want the memory, or wait to be asked?

> >> What does qemu do with this information?
> >
> > There are two possible scenarios:
> >
> >  1. The balloon driver is currently inflating when it gets under
> > pressure
> >
> > QEMU resets "num_pages" to the current balloon size. This
> > cancels the on-going inflate
> >
> >  2. The balloon driver is not inflating, eg. it's possibly sleeping
> >
> >QEMU issues a deflate
> >
> > But note that those scenarios are not supposed to be used with the
> > current device, they are part of the automatic ballooning feature.
> > I CC'ed you on the QEMU patch, you can find it here case you didn't
> > see it:
> >
> >  http://marc.info/?l=kvm&m=138988966315690&w=2

Yes, caught that after I replied; I should have checked first.

It seems like we are still figuring out how to do ballooning.  The
best approach in cases like this is to make it simple, so I don't hate
this.

But note that Daniel Kiper and I have been discussing a new virtio
balloon for draft 2 of the standard.  I'll CC you when I post that to
one of the OASIS virtio mailing lists.

Cheers,
Rusty.
--
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] MODSIGN: Fix including certificate twice when the signing_key.x509

2014-01-16 Thread Rusty Russell
Michal Marek  writes:
> On 15.1.2014 05:39, Rusty Russell wrote:
>> "Lee, Chun-Yi"  writes:
>>> From: Chun-Yi Lee 
>>> v2:
>>> Using '$(shell /bin/pwd)' instead of '$(shell pwd)' for more reliable
>>> between different shells
>> 
>> Hmm, that's not a great test for equality.  How about:
>> 
>>  ifneq ($(realpath .), $(realpath $(srctree)))
>> 
>> That should cover all the cases.
>
> make 3.80 does not have realpath :(.

OK, I guess the former is the best compromise then.

Thanks,
Rusty.
--
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 3/4] virtio_balloon: add pressure notification via a new virtqueue

2014-01-16 Thread Rusty Russell
Luiz Capitulino  writes:
> From: Luiz capitulino 
>
> This commit adds support to a new virtqueue called message virtqueue.

OK, this needs a lot of thought (especially since reworking the virtio
balloon is on the TODO list for the OASIS virtio technical committee...)

But AFAICT this is a way of explicitly saying "no" to the host's target
(vs the implicit method of just not meeting the target).  I'm not sure
that gives enough information to the host.  On the other hand, I'm not
sure what information we *can* give.

Should we instead be counter-proposing a target?

What does qemu do with this information?

Thanks,
Rusty.

> The message virtqueue can be used by guests to notify the host about
> important memory-related state changes in the guest. Currently, the
> only implemented notification is the "guest is under pressure" one,
> which informs the host that the guest is under memory pressure.
>
> This notification can be used to implement automatic memory ballooning
> in the host. For example, once learning that the guest is under pressure,
> the host could cancel an on-going inflate and/or start a deflate
> operation.
>
> Doing this through a virtqueue might seem like overkill, as all
> we're doing is to transfer an integer between guest and host. However,
> using a virtqueue offers the following advantages:
>
>  1. We can realibly synchronize host and guest. That is, the guest
> will only continue once the host acknowledges the notification.
> This is important, because if the guest gets under pressure while
> inflating the balloon, it has to stop to give the host a chance
> to reset "num_pages" (see next commit)
>
>  2. It's extensible. We may (or may not) want to tell the host
> which pressure level the guest finds itself in (ie. low,
> medium or critical)
>
> The lightweight alternative is to use a configuration space parameter.
> For this to work though, the guest would have to wait the for host
> to acknowloedge the receipt of a configuration change update. I could
> try this if the virtqueue is too overkill.
>
> Finally, the guest learns it's under pressure by registering a
> callback with the in-kernel vmpressure API.
>
> FIXMEs:
>
>  - vmpressure API is missing an de-registration routine
>  - not completely sure my changes in virtballoon_probe() are correct
>
> Signed-off-by: Luiz capitulino 
> ---
>  drivers/virtio/virtio_balloon.c | 93 
> +
>  include/uapi/linux/virtio_balloon.h |  1 +
>  2 files changed, 84 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 5c4a95b..1c3ee71 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -29,6 +29,9 @@
>  #include 
>  #include 
>  
> +#include 
> +#include 
> +
>  /*
>   * Balloon device works in 4K page units.  So each page is pointed to by
>   * multiple balloon pages.  All memory counters in this driver are in balloon
> @@ -37,10 +40,12 @@
>  #define VIRTIO_BALLOON_PAGES_PER_PAGE (unsigned)(PAGE_SIZE >> 
> VIRTIO_BALLOON_PFN_SHIFT)
>  #define VIRTIO_BALLOON_ARRAY_PFNS_MAX 256
>  
> +#define VIRTIO_BALLOON_MSG_PRESSURE 1
> +
>  struct virtio_balloon
>  {
>   struct virtio_device *vdev;
> - struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
> + struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *message_vq;
>  
>   /* Where the ballooning thread waits for config to change. */
>   wait_queue_head_t config_change;
> @@ -51,6 +56,8 @@ struct virtio_balloon
>   /* Waiting for host to ack the pages we released. */
>   wait_queue_head_t acked;
>  
> + wait_queue_head_t message_acked;
> +
>   /* Number of balloon pages we've told the Host we're not using. */
>   unsigned int num_pages;
>   /*
> @@ -71,6 +78,9 @@ struct virtio_balloon
>   /* Memory statistics */
>   int need_stats_update;
>   struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR];
> +
> + /* Message virtqueue */
> + atomic_t guest_pressure;
>  };
>  
>  static struct virtio_device_id id_table[] = {
> @@ -78,6 +88,41 @@ static struct virtio_device_id id_table[] = {
>   { 0 },
>  };
>  
> +static inline bool guest_under_pressure(const struct virtio_balloon *vb)
> +{
> + return atomic_read(&vb->guest_pressure) == 1;
> +}
> +
> +static void vmpressure_event_handler(void *data, int level)
> +{
> + struct virtio_balloon *vb = data;
> +
> + atomic_set(&vb->guest_pressure, 1);
> + wake_up(&vb->config_change);
> +}
> +
> +static void tell_host_pressure(struct virtio_balloon *vb)
> +{
> + const uint32_t msg = VIRTIO_BALLOON_MSG_PRESSURE;
> + struct scatterlist sg;
> + unsigned int len;
> + int err;
> +
> + sg_init_one(&sg, &msg, sizeof(msg));
> +
> + err = virtqueue_add_outbuf(vb->message_vq, &sg, 1, vb, GFP_KERNEL);
> + if (err < 0) {
> + printk(KERN_WARNING "virtio-balloon: failed to send host 
> message 

Re: [PATCH net-next RFC] virtio-net: drop rq->max and rq->num

2014-01-15 Thread Rusty Russell
Rusty Russell  writes:
> Jason Wang  writes:
>> It looks like there's no need for those two fields:
>>
>> - Unless there's a failure for the first refill try, rq->max should be always
>>   equal to the vring size.
>> - rq->num is only used to determine the condition that we need to do the 
>> refill,
>>   we could check vq->num_free instead.
>> - rq->num was required to be increased or decreased explicitly after each
>>   get/put which results a bad API.
>>
>> So this patch removes them both to make the code simpler.
>
> Nice.  These fields date from when the vq struct was opaque.
>
> Applied,
> Rusty.

Oops, this doesn't require any core virtio changes, so it's for DaveM:

Acked-by: Rusty Russell 

Thanks,
Rusty.

>> Cc: Rusty Russell 
>> Cc: Michael S. Tsirkin 
>> Signed-off-by: Jason Wang 
>> ---
>>  drivers/net/virtio_net.c | 16 +++-
>>  1 file changed, 3 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index c51a988..4e1bce3 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -72,9 +72,6 @@ struct receive_queue {
>>  
>>  struct napi_struct napi;
>>  
>> -/* Number of input buffers, and max we've ever had. */
>> -unsigned int num, max;
>> -
>>  /* Chain pages by the private ptr. */
>>  struct page *pages;
>>  
>> @@ -360,7 +357,6 @@ static struct sk_buff *receive_mergeable(struct 
>> net_device *dev,
>>  }
>>  
>>  page = virt_to_head_page(buf);
>> ---rq->num;
>>  
>>  num_skb_frags = skb_shinfo(curr_skb)->nr_frags;
>>  if (unlikely(num_skb_frags == MAX_SKB_FRAGS)) {
>> @@ -406,7 +402,6 @@ err_skb:
>>  }
>>  page = virt_to_head_page(buf);
>>  put_page(page);
>> ---rq->num;
>>  }
>>  err_buf:
>>  dev->stats.rx_dropped++;
>> @@ -628,10 +623,7 @@ static bool try_fill_recv(struct receive_queue *rq, 
>> gfp_t gfp)
>>  oom = err == -ENOMEM;
>>  if (err)
>>  break;
>> -++rq->num;
>>  } while (rq->vq->num_free);
>> -if (unlikely(rq->num > rq->max))
>> -rq->max = rq->num;
>>  if (unlikely(!virtqueue_kick(rq->vq)))
>>  return false;
>>  return !oom;
>> @@ -699,11 +691,10 @@ again:
>>  while (received < budget &&
>> (buf = virtqueue_get_buf(rq->vq, &len)) != NULL) {
>>  receive_buf(rq, buf, len);
>> ---rq->num;
>>  received++;
>>  }
>>  
>> -if (rq->num < rq->max / 2) {
>> +if (rq->vq->num_free > virtqueue_get_vring_size(rq->vq) / 2) {
>>  if (!try_fill_recv(rq, GFP_ATOMIC))
>>  schedule_delayed_work(&vi->refill, 0);
>>  }
>> @@ -1398,9 +1389,7 @@ static void free_unused_bufs(struct virtnet_info *vi)
>>  give_pages(&vi->rq[i], buf);
>>  else
>>  dev_kfree_skb(buf);
>> ---vi->rq[i].num;
>>  }
>> -BUG_ON(vi->rq[i].num != 0);
>>  }
>>  }
>>  
>> @@ -1671,7 +1660,8 @@ static int virtnet_probe(struct virtio_device *vdev)
>>  try_fill_recv(&vi->rq[i], GFP_KERNEL);
>>  
>>  /* If we didn't even get one input buffer, we're useless. */
>> -if (vi->rq[i].num == 0) {
>> +if (vi->rq[i].vq->num_free ==
>> +virtqueue_get_vring_size(vi->rq[i].vq)) {
>>  free_unused_bufs(vi);
>>  err = -ENOMEM;
>>  goto free_recv_bufs;
>> -- 
>> 1.8.3.2
> --
> 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/
--
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] scripts/gcc-version.sh: handle CC="gcc -m32"

2014-01-15 Thread Rusty Russell
Michal Marek  writes:
> On 3.1.2014 18:10, Michal Marek wrote:
>> On 2013-12-10 08:13, Rusty Russell wrote:
>>> Without it we get ugly warnings (though build still succeeds).
>>>
>>> $ make -j8 CC="gcc -m32"
>>> In file included from :0:0:
>>> /usr/include/stdc-predef.h:30:26: fatal error: bits/predefs.h: No such file 
>>> or directory
>>>  #include 
>>>   ^
>>> compilation terminated.
>>> In file included from :0:0:
>>> /usr/include/stdc-predef.h:30:26: fatal error: bits/predefs.h: No such file 
>>> or directory
>>>  #include 
>>>   ^
>>> compilation terminated.
>>> /home/rusty/devel/kernel/linux/scripts/gcc-version.sh: line 31: printf: #: 
>>> invalid number
>>> /home/rusty/devel/kernel/linux/scripts/gcc-version.sh: line 31: printf: #: 
>>> invalid number
>>> /bin/sh: 1: [: 0001: unexpected operator
>>>   CHK include/config/kernel.release
>>>   CHK include/generated/uapi/linux/version.h
>>> make[1]: Nothing to be done for `all'.
>>> ...
>>>
>>> Signed-off-by: Rusty Russell 
>>>
>>> diff --git a/scripts/gcc-version.sh b/scripts/gcc-version.sh
>>> index 7f2126df91f2..d48b0cbaf246 100644
>>> --- a/scripts/gcc-version.sh
>>> +++ b/scripts/gcc-version.sh
>>> @@ -14,7 +14,7 @@ if [ "$1" = "-p" ] ; then
>>> shift;
>>>  fi
>>>  
>>> -compiler="$*"
>>> +compiler="$1"
>> 
>> But this will break things like CC="ccache gcc". Your problem is that
>> you do not have the 32bit glibc headers, right?
>
> BTW, what is the usecase for CC="gcc -m32"? AFAICS, at least x86 and
> powerpc do this for you when building a 32bit kernel.

No, without -m32 I get:

$ make
make[1]: Nothing to be done for `all'.
make[1]: Nothing to be done for `relocs'.
  CHK include/config/kernel.release
  CHK include/generated/uapi/linux/version.h
  CHK include/generated/utsrelease.h
  CC  kernel/bounds.s
gcc: warning: ‘-mcpu=’ is deprecated; use ‘-mtune=’ or ‘-march=’ instead
gcc: warning: ‘-mcpu=’ is deprecated; use ‘-mtune=’ or ‘-march=’ instead
kernel/bounds.c:1:0: error: CPU you selected does not support x86-64 
instruction set
 /*
 ^
kernel/bounds.c:1:0: warning: -mregparm is ignored in 64-bit mode [enabled by 
default]
make[1]: *** [kernel/bounds.s] Error 1
make: *** [prepare0] Error 2

If that's *supposed* to work, then the bug is elsewhere.

Cheers,
Rusty.
--
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] MODSIGN: Fix including certificate twice when the signing_key.x509

2014-01-15 Thread Rusty Russell
Punting to David Howells...

Cheers,
Rusty.
"Lee, Chun-Yi"  writes:
> From: Chun-Yi Lee 
>
> This issue was found in devel-pekey branch on linux-modsign.git tree.
> The
> x509_certificate_list includes certificate twice when the
> signing_key.x509
> already exists.
> We can reproduce this issue by making kernel twice, the build log of
> second time looks like this:
>
> ...
>   CHK kernel/config_data.h
>   CERTS   kernel/x509_certificate_list
>   - Including cert /ramdisk/working/joey/linux-modsign/signing_key.x509
>   - Including cert signing_key.x509
> ...
>
> Actually the build path was the same with the srctree path when building
> kernel. It causes the size of bzImage increased by packaging
> certificates
> twice.
>
> v2:
> Using '$(shell /bin/pwd)' instead of '$(shell pwd)' for more reliable
> between different shells

Hmm, that's not a great test for equality.  How about:

 ifneq ($(realpath .), $(realpath $(srctree)))

That should cover all the cases.

Cheers,
Rusty.

>
> Cc: Rusty Russell 
> Cc: Josh Boyer 
> Cc: Randy Dunlap 
> Cc: Herbert Xu 
> Cc: "David S. Miller" 
> Cc: Michal Marek 
> Signed-off-by: Chun-Yi Lee 
> Signed-off-by: David Howells 
> ---
>  kernel/Makefile |5 -
>  1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/kernel/Makefile b/kernel/Makefile
> index bc010ee..582fa7a 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -136,7 +136,10 @@ $(obj)/timeconst.h: $(obj)/hz.bc $(src)/timeconst.bc 
> FORCE
>  #
>  
> ###
>  ifeq ($(CONFIG_SYSTEM_TRUSTED_KEYRING),y)
> -X509_CERTIFICATES-y := $(wildcard *.x509) $(wildcard $(srctree)/*.x509)
> +X509_CERTIFICATES-y := $(wildcard *.x509)
> +ifneq ($(shell /bin/pwd), $(srctree))
> +X509_CERTIFICATES-y += $(wildcard $(srctree)/*.x509)
> +endif
>  X509_CERTIFICATES-$(CONFIG_MODULE_SIG) += $(objtree)/signing_key.x509
>  X509_CERTIFICATES-raw := $(sort $(foreach CERT,$(X509_CERTIFICATES-y), \
>   $(or $(realpath $(CERT)),$(CERT
> -- 
> 1.6.4.2
--
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 net-next RFC] virtio-net: drop rq->max and rq->num

2014-01-15 Thread Rusty Russell
Jason Wang  writes:
> It looks like there's no need for those two fields:
>
> - Unless there's a failure for the first refill try, rq->max should be always
>   equal to the vring size.
> - rq->num is only used to determine the condition that we need to do the 
> refill,
>   we could check vq->num_free instead.
> - rq->num was required to be increased or decreased explicitly after each
>   get/put which results a bad API.
>
> So this patch removes them both to make the code simpler.

Nice.  These fields date from when the vq struct was opaque.

Applied,
Rusty.

> Cc: Rusty Russell 
> Cc: Michael S. Tsirkin 
> Signed-off-by: Jason Wang 
> ---
>  drivers/net/virtio_net.c | 16 +++-
>  1 file changed, 3 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index c51a988..4e1bce3 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -72,9 +72,6 @@ struct receive_queue {
>  
>   struct napi_struct napi;
>  
> - /* Number of input buffers, and max we've ever had. */
> - unsigned int num, max;
> -
>   /* Chain pages by the private ptr. */
>   struct page *pages;
>  
> @@ -360,7 +357,6 @@ static struct sk_buff *receive_mergeable(struct 
> net_device *dev,
>   }
>  
>   page = virt_to_head_page(buf);
> - --rq->num;
>  
>   num_skb_frags = skb_shinfo(curr_skb)->nr_frags;
>   if (unlikely(num_skb_frags == MAX_SKB_FRAGS)) {
> @@ -406,7 +402,6 @@ err_skb:
>   }
>   page = virt_to_head_page(buf);
>   put_page(page);
> - --rq->num;
>   }
>  err_buf:
>   dev->stats.rx_dropped++;
> @@ -628,10 +623,7 @@ static bool try_fill_recv(struct receive_queue *rq, 
> gfp_t gfp)
>   oom = err == -ENOMEM;
>   if (err)
>   break;
> - ++rq->num;
>   } while (rq->vq->num_free);
> - if (unlikely(rq->num > rq->max))
> - rq->max = rq->num;
>   if (unlikely(!virtqueue_kick(rq->vq)))
>   return false;
>   return !oom;
> @@ -699,11 +691,10 @@ again:
>   while (received < budget &&
>  (buf = virtqueue_get_buf(rq->vq, &len)) != NULL) {
>   receive_buf(rq, buf, len);
> - --rq->num;
>   received++;
>   }
>  
> - if (rq->num < rq->max / 2) {
> + if (rq->vq->num_free > virtqueue_get_vring_size(rq->vq) / 2) {
>   if (!try_fill_recv(rq, GFP_ATOMIC))
>   schedule_delayed_work(&vi->refill, 0);
>   }
> @@ -1398,9 +1389,7 @@ static void free_unused_bufs(struct virtnet_info *vi)
>   give_pages(&vi->rq[i], buf);
>   else
>   dev_kfree_skb(buf);
> - --vi->rq[i].num;
>   }
> - BUG_ON(vi->rq[i].num != 0);
>   }
>  }
>  
> @@ -1671,7 +1660,8 @@ static int virtnet_probe(struct virtio_device *vdev)
>   try_fill_recv(&vi->rq[i], GFP_KERNEL);
>  
>   /* If we didn't even get one input buffer, we're useless. */
> - if (vi->rq[i].num == 0) {
> + if (vi->rq[i].vq->num_free ==
> + virtqueue_get_vring_size(vi->rq[i].vq)) {
>   free_unused_bufs(vi);
>   err = -ENOMEM;
>   goto free_recv_bufs;
> -- 
> 1.8.3.2
--
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: powerpc: possible access beyond TASK_SIZE in start_thread

2014-01-15 Thread Rusty Russell
Mathieu Desnoyers  writes:
> Hi Rusty,
>
> I was looking at the diff between kernel v3.12 and recent master (after 
> 3.13-rc7),
> and noticed that in the following commit:

Cool!

I just moved the code, so any problem exists before this, too.

> commit 94af3abf995b17f6a008b00152c94841242ec6c7
> Author: Rusty Russell 
> Date:   Wed Nov 20 22:15:02 2013 +1100
>
> powerpc: ELF2 binaries launched directly.
>
> on powerpc, those lines appear in start_thread():
>
> +   /* start is a relocated pointer to the function
> +* descriptor for the elf _start routine.  The first
> +* entry in the function descriptor is the entry
> +* address of _start and the second entry is the TOC
> +* value we need to use.
> +*/
> +   __get_user(entry, (unsigned long __user *)start);
> +   __get_user(toc, (unsigned long __user *)start+1);
>
> Note the "__" before get_user(), which bypass any kind of validation on the
> addresses.
>
> Amongst the callers, if we look at fs/binfmt_elf.c:load_elf_binary(), we see:
>
> elf_entry = loc->elf_ex.e_entry;
> if (BAD_ADDR(elf_entry)) {
> force_sig(SIGSEGV, current);
> retval = -EINVAL;
> goto out_free_dentry;
> }
>
> and the elf_entry gets passed to start_thread().
>
> If we craft a binary with elf_entry address of
>
> TASK_SIZE - 1  (1 byte before TASK_SIZE), then I think we could make both
> __get_user() calls access data beyond TASK_SIZE, because elf_entry address
> is verified, but there is no validation on its range AFAIU. Is it expected ?
> Am I missing something ?

Yes, looks like we can read the first 15 bytes of kernel space.  That's
not likely to be interesting, but we should probably check anyway.

Thanks!
Rusty.
===
Subject: powerpc: don't allow entry point to be read from kernel.

Mathieu points out that while start is checked, it could be TASK_SIZE-1,
which means this code could read the first 15 bytes of kernel space.
That's text for now, but let's be paranoid.

start_thread() doesn't have a return value, so easiest to use get_user:
you'll get a jump to zero if it fails.

Reported-by: Mathieu Desnoyers 

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 4a96556fd2d4..b4ca298a5536 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1113,8 +1113,8 @@ void start_thread(struct pt_regs *regs, unsigned long 
start, unsigned long sp)
 * address of _start and the second entry is the TOC
 * value we need to use.
 */
-   __get_user(entry, (unsigned long __user *)start);
-   __get_user(toc, (unsigned long __user *)start+1);
+   get_user(entry, (unsigned long __user *)start);
+   get_user(toc, (unsigned long __user *)start+1);
 
/* Check whether the e_entry function descriptor entries
 * need to be relocated before we can use them.
--
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/6] virtio: fail adding buffer on broken queues.

2014-01-14 Thread Rusty Russell
Heinz points out that adding buffers to a broken virtqueue (which
should "never happen") still works.  Failing allows drivers to detect
and complain about broken devices.

Now drivers are robust, we can add this extra check.

Reported-by: Heinz Graalfs 
Signed-off-by: Rusty Russell 
---
 drivers/virtio/virtio_ring.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 28b5338fff71..b74033dca384 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -203,6 +203,11 @@ static inline int virtqueue_add(struct virtqueue *_vq,
 
BUG_ON(data == NULL);
 
+   if (unlikely(vq->broken)) {
+   END_USE(vq);
+   return -EIO;
+   }
+
 #ifdef DEBUG
{
ktime_t now = ktime_get();
@@ -309,7 +314,7 @@ add_head:
  * Caller must ensure we don't call this with other virtqueue operations
  * at the same time (except where noted).
  *
- * Returns zero or a negative error (ie. ENOSPC, ENOMEM).
+ * Returns zero or a negative error (ie. ENOSPC, ENOMEM, EIO).
  */
 int virtqueue_add_sgs(struct virtqueue *_vq,
  struct scatterlist *sgs[],
@@ -347,7 +352,7 @@ EXPORT_SYMBOL_GPL(virtqueue_add_sgs);
  * Caller must ensure we don't call this with other virtqueue operations
  * at the same time (except where noted).
  *
- * Returns zero or a negative error (ie. ENOSPC, ENOMEM).
+ * Returns zero or a negative error (ie. ENOSPC, ENOMEM, EIO).
  */
 int virtqueue_add_outbuf(struct virtqueue *vq,
 struct scatterlist sg[], unsigned int num,
@@ -369,7 +374,7 @@ EXPORT_SYMBOL_GPL(virtqueue_add_outbuf);
  * Caller must ensure we don't call this with other virtqueue operations
  * at the same time (except where noted).
  *
- * Returns zero or a negative error (ie. ENOSPC, ENOMEM).
+ * Returns zero or a negative error (ie. ENOSPC, ENOMEM, EIO).
  */
 int virtqueue_add_inbuf(struct virtqueue *vq,
struct scatterlist sg[], unsigned int num,
-- 
1.8.3.2

--
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] export: declare ksymtab symbols

2014-01-14 Thread Rusty Russell
Andi Kleen  writes:
>> Cc: Andi Kleen 
>> Signed-off-by: Johannes Berg 
>
> Acked-by: Andi Kleen 

Thanks, applied.

I added:

Fixes: e0f244c63fc9 ("asmlinkage, module: Make ksymtab [...] __visible")

Cheers,
Rusty.
--
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/6] virtio_blk: don't crash, report error if virtqueue is broken.

2014-01-14 Thread Rusty Russell
A bad implementation of virtio might cause us to mark the virtqueue
broken: we'll dev_err() in that case, and the device is useless, but
let's not BUG_ON().

ENOMEM or ENOSPC implies the ring is full, and we should try again
later (-ENOMEM is documented to happen, but doesn't, as we fall
through to ENOSPC).

EIO means it's broken.

Signed-off-by: Rusty Russell 
---
 drivers/block/virtio_blk.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 6a680d4de7f1..704d6c814c17 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -158,6 +158,7 @@ static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx, 
struct request *req)
unsigned long flags;
unsigned int num;
const bool last = (req->cmd_flags & REQ_END) != 0;
+   int err;
 
BUG_ON(req->nr_phys_segments + 2 > vblk->sg_elems);
 
@@ -198,11 +199,16 @@ static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx, 
struct request *req)
}
 
spin_lock_irqsave(&vblk->vq_lock, flags);
-   if (__virtblk_add_req(vblk->vq, vbr, vbr->sg, num) < 0) {
+   err = __virtblk_add_req(vblk->vq, vbr, vbr->sg, num);
+   if (err) {
virtqueue_kick(vblk->vq);
spin_unlock_irqrestore(&vblk->vq_lock, flags);
blk_mq_stop_hw_queue(hctx);
-   return BLK_MQ_RQ_QUEUE_BUSY;
+   /* Out of mem doesn't actually happen, since we fall back
+* to direct descriptors */
+   if (err == -ENOMEM || err == -ENOSPC)
+   return BLK_MQ_RQ_QUEUE_BUSY;
+   return BLK_MQ_RQ_QUEUE_ERROR;
}
 
if (last)
-- 
1.8.3.2

--
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 1/6] virtio_net: don't crash if virtqueue is broken.

2014-01-14 Thread Rusty Russell
A bad implementation of virtio might cause us to mark the virtqueue
broken: we'll dev_err() in that case, and the device is useless, but
let's not BUG_ON().

Signed-off-by: Rusty Russell 
---
 drivers/net/virtio_net.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 5d776447d9c3..0949688ae540 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -904,8 +904,7 @@ static bool virtnet_send_command(struct virtnet_info *vi, 
u8 class, u8 cmd,
sgs[out_num + in_num++] = &stat;
 
BUG_ON(out_num + in_num > ARRAY_SIZE(sgs));
-   BUG_ON(virtqueue_add_sgs(vi->cvq, sgs, out_num, in_num, vi, GFP_ATOMIC)
-  < 0);
+   virtqueue_add_sgs(vi->cvq, sgs, out_num, in_num, vi, GFP_ATOMIC);
 
if (unlikely(!virtqueue_kick(vi->cvq)))
return status == VIRTIO_NET_OK;
-- 
1.8.3.2

--
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/6] virtio: virtio_break_device() to mark all virtqueues broken.

2014-01-14 Thread Rusty Russell
Good for post-apocalyptic scenarios, like S/390 hotplug.

Signed-off-by: Rusty Russell 
---
 drivers/virtio/virtio_ring.c | 15 +++
 include/linux/virtio.h   |  2 ++
 2 files changed, 17 insertions(+)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index b74033dca384..a84350019f62 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -864,4 +864,19 @@ bool virtqueue_is_broken(struct virtqueue *_vq)
 }
 EXPORT_SYMBOL_GPL(virtqueue_is_broken);
 
+/*
+ * This should prevent the device from being used, allowing drivers to
+ * recover.  You may need to grab appropriate locks to flush.
+ */
+void virtio_break_device(struct virtio_device *dev)
+{
+   struct virtqueue *_vq;
+
+   list_for_each_entry(_vq, &dev->vqs, list) {
+   struct vring_virtqueue *vq = to_vvq(_vq);
+   vq->broken = true;
+   }
+}
+EXPORT_SYMBOL_GPL(virtio_break_device);
+
 MODULE_LICENSE("GPL");
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index e4abb84199be..b46671e28de2 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -106,6 +106,8 @@ static inline struct virtio_device *dev_to_virtio(struct 
device *_dev)
 int register_virtio_device(struct virtio_device *dev);
 void unregister_virtio_device(struct virtio_device *dev);
 
+void virtio_break_device(struct virtio_device *dev);
+
 /**
  * virtio_driver - operations for a virtio I/O driver
  * @driver: underlying device driver (populate name and owner).
-- 
1.8.3.2

--
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/6] virtio-rng: don't crash if virtqueue is broken.

2014-01-14 Thread Rusty Russell
A bad implementation of virtio might cause us to mark the virtqueue
broken: we'll dev_err() in that case, and the device is useless, but
let's not BUG().

Signed-off-by: Rusty Russell 
---
 drivers/char/hw_random/virtio-rng.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/char/hw_random/virtio-rng.c 
b/drivers/char/hw_random/virtio-rng.c
index c12398d1517c..2ce0e225e58c 100644
--- a/drivers/char/hw_random/virtio-rng.c
+++ b/drivers/char/hw_random/virtio-rng.c
@@ -47,8 +47,7 @@ static void register_buffer(u8 *buf, size_t size)
sg_init_one(&sg, buf, size);
 
/* There should always be room for one buffer. */
-   if (virtqueue_add_inbuf(vq, &sg, 1, buf, GFP_KERNEL) < 0)
-   BUG();
+   virtqueue_add_inbuf(vq, &sg, 1, buf, GFP_KERNEL);
 
virtqueue_kick(vq);
 }
-- 
1.8.3.2

--
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/6] virtio_balloon: don't crash if virtqueue is broken.

2014-01-14 Thread Rusty Russell
A bad implementation of virtio might cause us to mark the virtqueue
broken: we'll dev_err() in that case, and the device is useless, but
let's not BUG().

Signed-off-by: Rusty Russell 
---
 drivers/virtio/virtio_balloon.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 5c4a95b516cf..fc1fb27dca49 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -108,8 +108,7 @@ static void tell_host(struct virtio_balloon *vb, struct 
virtqueue *vq)
sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns);
 
/* We should always be able to add one buffer to an empty queue. */
-   if (virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL) < 0)
-   BUG();
+   virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
virtqueue_kick(vq);
 
/* When host has read buffer, this completes via balloon_ack */
@@ -258,8 +257,7 @@ static void stats_handle_request(struct virtio_balloon *vb)
if (!virtqueue_get_buf(vq, &len))
return;
sg_init_one(&sg, vb->stats, sizeof(vb->stats));
-   if (virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL) < 0)
-   BUG();
+   virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
virtqueue_kick(vq);
 }
 
@@ -338,7 +336,7 @@ static int init_vqs(struct virtio_balloon *vb)
 
/*
 * Prime this virtqueue with one buffer so the hypervisor can
-* use it to signal us later.
+* use it to signal us later (it can't be broken yet!).
 */
sg_init_one(&sg, vb->stats, sizeof vb->stats);
if (virtqueue_add_outbuf(vb->stats_vq, &sg, 1, vb, GFP_KERNEL)
-- 
1.8.3.2

--
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/


[PULL] fixes for virtio balloon and non-MMU kallsyms

2013-12-22 Thread Rusty Russell
The following changes since commit af91706d5ddecb4a9858cca9e90d463037cfd498:

  ima: store address of template_fmt_copy in a pointer before calling strsep 
(2013-11-30 13:09:53 +1100)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux.git 
tags/fixes-for-linus

for you to fetch changes up to 7122c3e9154b5d9a7422f68f02d8acf050fad2b0:

  scripts/link-vmlinux.sh: only filter kernel symbols for arm (2013-12-10 
16:49:19 +1030)


Refactoring broke the balloon driver, and fixing kallsyms on ARM broke
some (non-ARM) MMUless setups, so we're making that fix ARM-only for now.

Unfortunately, the ARM refactoring which broke kallsyms/perf was CC:stable,
so the fix (which broken non-ARM) was also CC:stable, so now the partial
reversion is also CC:stable...

Cheers,
Rusty.


Luiz Capitulino (1):
  virtio_balloon: update_balloon_size(): update correct field

Ming Lei (1):
  scripts/link-vmlinux.sh: only filter kernel symbols for arm

 drivers/virtio/virtio_balloon.c | 2 +-
 scripts/link-vmlinux.sh | 4 +++-
 2 files changed, 4 insertions(+), 2 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/


Re: [PATCH] module: fix coding style

2013-12-20 Thread Rusty Russell
Seunghun Lee  writes:
> Fix coding style of module.h

Thanks, applied.

Cheers,
Rusty.

>
> Signed-off-by: Seunghun Lee 
> ---
>  include/linux/module.h |   62 
> 
>  1 file changed, 31 insertions(+), 31 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 46e548f..eaf60ff 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -29,8 +29,7 @@
>  
>  #define MODULE_NAME_LEN MAX_PARAM_PREFIX_LEN
>  
> -struct modversion_info
> -{
> +struct modversion_info {
>   unsigned long crc;
>   char name[MODULE_NAME_LEN];
>  };
> @@ -84,12 +83,12 @@ void sort_main_extable(void);
>  void trim_init_extable(struct module *m);
>  
>  #ifdef MODULE
> -#define MODULE_GENERIC_TABLE(gtype,name) \
> +#define MODULE_GENERIC_TABLE(gtype, name)\
>  extern const struct gtype##_id __mod_##gtype##_table \
>__attribute__ ((unused, alias(__stringify(name
>  
>  #else  /* !MODULE */
> -#define MODULE_GENERIC_TABLE(gtype,name)
> +#define MODULE_GENERIC_TABLE(gtype, name)
>  #endif
>  
>  /* Generic info of form tag = "info" */
> @@ -126,7 +125,7 @@ extern const struct gtype##_id __mod_##gtype##_table  
> \
>   * is a GPL combined work.
>   *
>   * This exists for several reasons
> - * 1.So modinfo can show license info for users wanting to vet their 
> setup 
> + * 1.So modinfo can show license info for users wanting to vet their 
> setup
>   *   is free
>   * 2.So the community can ignore bug reports including proprietary 
> modules
>   * 3.So vendors can do likewise based on their own policies
> @@ -138,27 +137,29 @@ extern const struct gtype##_id __mod_##gtype##_table
> \
>   * authors use multiple MODULE_AUTHOR() statements/lines.
>   */
>  #define MODULE_AUTHOR(_author) MODULE_INFO(author, _author)
> -  
> +
>  /* What your module does. */
>  #define MODULE_DESCRIPTION(_description) MODULE_INFO(description, 
> _description)
>  
> -#define MODULE_DEVICE_TABLE(type,name)   \
> -  MODULE_GENERIC_TABLE(type##_device,name)
> +#define MODULE_DEVICE_TABLE(type, name)  \
> +  MODULE_GENERIC_TABLE(type##_device, name)
>  
>  /* Version of form [:][-].
> -   Or for CVS/RCS ID version, everything but the number is stripped.
> -  : A (small) unsigned integer which allows you to start versions
> -   anew. If not mentioned, it's zero.  eg. "2:1.0" is after
> -"1:2.0".
> -  : The  may contain only alphanumerics and the
> -   character `.'.  Ordered by numeric sort for numeric parts,
> -ascii sort for ascii parts (as per RPM or DEB algorithm).
> -  : Like , but inserted for local
> -   customizations, eg "rh3" or "rusty1".
> -
> -  Using this automatically adds a checksum of the .c files and the
> -  local headers in "srcversion".
> -*/
> + * Or for CVS/RCS ID version, everything but the number is stripped.
> + * : A (small) unsigned integer which allows you to start versions
> + * anew. If not mentioned, it's zero.  eg. "2:1.0" is after
> + * "1:2.0".
> +
> + * : The  may contain only alphanumerics and the
> + * character `.'.  Ordered by numeric sort for numeric parts,
> + * ascii sort for ascii parts (as per RPM or DEB algorithm).
> +
> + * : Like , but inserted for local
> + * customizations, eg "rh3" or "rusty1".
> +
> + * Using this automatically adds a checksum of the .c files and the
> + * local headers in "srcversion".
> + */
>  
>  #if defined(MODULE) || !defined(CONFIG_SYSFS)
>  #define MODULE_VERSION(_version) MODULE_INFO(version, _version)
> @@ -226,8 +227,7 @@ struct module_ref {
>   unsigned long decs;
>  } __attribute((aligned(2 * sizeof(unsigned long;
>  
> -struct module
> -{
> +struct module {
>   enum module_state state;
>  
>   /* Member of list of modules */
> @@ -480,8 +480,8 @@ static inline void module_put(struct module *module)
>  static inline void __module_get(struct module *module)
>  {
>  }
> -#define symbol_put(x) do { } while(0)
> -#define symbol_put_addr(p) do { } while(0)
> +#define symbol_put(x) do { } while (0)
> +#define symbol_put_addr(p) do { } while (0)
>  
>  #endif /* CONFIG_MODULE_UNLOAD */
>  int ref_module(struct module *a, struct module *b);
> @@ -507,8 +507,8 @@ int lookup_module_symbol_attrs(unsigned long addr, 
> unsigned long *size, unsigned
>  /* For extable.c to search modules' exception tables. */
>  const struct exception_table_entry *search_module_extables(unsigned long 
> addr);
>  
> -int register_module_notifier(struct notifier_block * nb);
> -int unregister_module_notifier(struct notifier_block * nb);
> +int register_module_notifier(struct notifier_block *nb);
> +int unregister_module_notifier(struct notifier_block *nb);
>  
>  extern void print_modules(void);
>  
> @@ -548,8 +548,8 @@ static inline bool is_module_text_address(unsigned long 
> addr)
>  
>  /* Get/put a kernel symbol (calls should 

Re: [PATCH] drivers: virtio: Mark function virtballoon_migratepage() as static in virtio_balloon.c

2013-12-16 Thread Rusty Russell
"Michael S. Tsirkin"  writes:
> On Mon, Dec 16, 2013 at 06:23:38AM -0800, Josh Triplett wrote:
>> On Mon, Dec 16, 2013 at 04:54:08PM +0530, Rashika Kheria wrote:
>> > Mark the function virtballoon_migratepage() as static in
>> > virtio_balloon.c because it is not used outside this file.
>> > 
>> > This eliminates the following warning in virtio_balloon.c:
>> > drivers/virtio/virtio_balloon.c:372:5: warning: no previous prototype for 
>> > ‘virtballoon_migratepage’ [-Wmissing-prototypes]
>> > 
>> > Signed-off-by: Rashika Kheria 
>> 
>> Reviewed-by: Josh Triplett 
>
> Acked-by: Michael S. Tsirkin 

Thanks, applied.

Cheers,
Rusty.

>
>> >  drivers/virtio/virtio_balloon.c |2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > 
>> > diff --git a/drivers/virtio/virtio_balloon.c 
>> > b/drivers/virtio/virtio_balloon.c
>> > index c444654..8c5943d 100644
>> > --- a/drivers/virtio/virtio_balloon.c
>> > +++ b/drivers/virtio/virtio_balloon.c
>> > @@ -369,7 +369,7 @@ static const struct address_space_operations 
>> > virtio_balloon_aops;
>> >   * This function preforms the balloon page migration task.
>> >   * Called through balloon_mapping->a_ops->migratepage
>> >   */
>> > -int virtballoon_migratepage(struct address_space *mapping,
>> > +static int virtballoon_migratepage(struct address_space *mapping,
>> >struct page *newpage, struct page *page, enum migrate_mode mode)
>> >  {
>> >struct balloon_dev_info *vb_dev_info = balloon_page_device(page);
>> > -- 
>> > 1.7.9.5
>> > 
--
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] fix printk output

2013-12-09 Thread Rusty Russell
Sergei Ianovich  writes:
> Signed-off-by: Sergei Ianovich 
> CC: Hannes Frederic Sowa 
> ---
>  Changes v1..v2
>  * 1-for-1 match between source and output lines
>  * clarify warning
>  * print tool name to avoid confusion with what to upgrade

Hmm, the copy here is gratuitous.  Using current->comm is safe, just
possibly ambigious if someone is changing the task name at the same time.

And we really want this one line anyway:

printk(KERN_WARNING
   "%s: waiting module removal not supported: please 
upgrade\n",
current->comm);

BTW, did you actually hit this?

Thanks,
Rusty.

>  kernel/module.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index f5a3b1e..0e627e7 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -816,8 +816,10 @@ SYSCALL_DEFINE2(delete_module, const char __user *, 
> name_user,
>   name[MODULE_NAME_LEN-1] = '\0';
>  
>   if (!(flags & O_NONBLOCK)) {
> + char tool[TASK_COMM_LEN];
>   printk(KERN_WARNING
> -"waiting module removal not supported: please upgrade");
> +"waiting module removal no longer supported\n"
> +"please upgrade %s\n", get_task_comm(tool, current));
>   }
>  
>   if (mutex_lock_interruptible(&module_mutex) != 0)
> -- 
> 1.8.4.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: kernel BUG at kernel/kallsyms.c:222!

2013-12-09 Thread Rusty Russell
Ming Lei  writes:

> Hi Axel,
>
> I am fine to resend it to RMK's patch system, but I am not sure
> if Russell would like to host it.
>
> Maybe it is better to push it via Rusty's tree since the change is only
> on scripts/, and it doesn't depend on the 1st one.
>
> Rusty, could you pick up the patch with title of
> "scripts/link-vmlinux.sh: only filter kernel symbols for arm"?

OK, and I added the correct Fixes: line so the stable people can track
it properly:

Cc: 
Cc: Rusty Russell 
Fixes: f6537f2f0eba4eba3354e48dbe3047db6d8b6254
Singed-off-by: Ming Lei 
Signed-off-by: Rusty Russell 

BTW, you misspelled Signed-off-by :)

Applied,
Rusty.


>
> Thanks,
> --
> Ming Lei
>
> On Mon, Dec 2, 2013 at 9:57 AM, Axel Lin  wrote:
>> 2013/11/13 Ming Lei :
>>> Hi Axel,
>>>
>>> On Wed, Nov 13, 2013 at 5:58 PM, Axel Lin  wrote:
>>>>
>>>> Hi Ming,
>>>> You missed "; then" in the end of if statement in your patch.
>>>>
>>>> So I got below error with your patch:
>>>> scripts/link-vmlinux.sh: line 87: syntax error near unexpected token `fi'
>>>> make: *** [vmlinuxclean] Error 2
>>>
>>> Thanks for your test, and I have fixed it and post a formal one for merge,
>>> and you can find these two patches on arm list, but forget to Cc you, sorry
>>> for that.
>>
>> Hi Ming,
>> Both your patch and Jonathan's patch are still not in linux-next.
>> Any chance to resend the patchs to RMK's patch system?
>>
>> Thanks,
>> Axel
>
>
>
> -- 
> Ming Lei
--
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] scripts/gcc-version.sh: handle CC="gcc -m32"

2013-12-09 Thread Rusty Russell
Without it we get ugly warnings (though build still succeeds).

$ make -j8 CC="gcc -m32"
In file included from :0:0:
/usr/include/stdc-predef.h:30:26: fatal error: bits/predefs.h: No such file or 
directory
 #include 
  ^
compilation terminated.
In file included from :0:0:
/usr/include/stdc-predef.h:30:26: fatal error: bits/predefs.h: No such file or 
directory
 #include 
  ^
compilation terminated.
/home/rusty/devel/kernel/linux/scripts/gcc-version.sh: line 31: printf: #: 
invalid number
/home/rusty/devel/kernel/linux/scripts/gcc-version.sh: line 31: printf: #: 
invalid number
/bin/sh: 1: [: 0001: unexpected operator
  CHK include/config/kernel.release
  CHK include/generated/uapi/linux/version.h
make[1]: Nothing to be done for `all'.
...

Signed-off-by: Rusty Russell 

diff --git a/scripts/gcc-version.sh b/scripts/gcc-version.sh
index 7f2126df91f2..d48b0cbaf246 100644
--- a/scripts/gcc-version.sh
+++ b/scripts/gcc-version.sh
@@ -14,7 +14,7 @@ if [ "$1" = "-p" ] ; then
shift;
 fi
 
-compiler="$*"
+compiler="$1"
 
 if [ ${#compiler} -eq 0 ]; then
echo "Error: No compiler specified."
--
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] Ignore generated file kernel/x509_certificate_list

2013-12-09 Thread Rusty Russell
$ git status
# On branch pending-rebases
# Untracked files:
#   (use "git add ..." to include in what will be committed)
#
#   kernel/x509_certificate_list
nothing added to commit but untracked files present (use "git add" to track)
$ 

Cc: David Howells 
Signed-off-by: Rusty Russell 

diff --git a/kernel/.gitignore b/kernel/.gitignore
index b3097bde4e9c..790d83c7d160 100644
--- a/kernel/.gitignore
+++ b/kernel/.gitignore
@@ -5,3 +5,4 @@ config_data.h
 config_data.gz
 timeconst.h
 hz.bc
+x509_certificate_list
--
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 1/2] virtio-net: determine type of bufs correctly

2013-12-06 Thread Rusty Russell
Jason Wang  writes:
> On 12/05/2013 10:36 PM, Andrey Vagin wrote:
>> free_unused_bufs must check vi->mergeable_rx_bufs before
>> vi->big_packets, because we use this sequence in other places.
>> Otherwise we allocate buffer of one type, then free it as another
>> type.
>>
>> general protection fault:  [#1] SMP
>> Dumping ftrace buffer:
>>(ftrace buffer empty)
>> Modules linked in: ip6table_filter ip6_tables iptable_filter ip_tables 
>> pcspkr virtio_balloon virtio_net(-) i2c_pii
>> CPU: 0 PID: 400 Comm: rmmod Not tainted 3.13.0-rc2+ #170
>> Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
>> task: 8800b6d2a210 ti: 8800aed32000 task.ti: 8800aed32000
>> RIP: 0010:[]  [] 
>> free_unused_bufs+0xc3/0x190 [virtio_net]
>> RSP: 0018:8800aed33dd8  EFLAGS: 00010202
>> RAX: 8800b1fe2c00 RBX: 8800b66a7240 RCX: 6b6b6b6b6b6b6b6b
>> RDX: 6b6b6b6b6b6b6b6b RSI: 8800b8419a68 RDI: 8800b66a1148
>> RBP: 8800aed33e00 R08: 0001 R09: 
>> R10:  R11: 0001 R12: 
>> R13: 8800b66a1148 R14:  R15: 77ff8000
>> FS:  7fc4f9c4e740() GS:8800bfa0() knlGS:
>> CS:  0010 DS:  ES:  CR0: 8005003b
>> CR2: 7f63f432f000 CR3: b6538000 CR4: 06f0
>> Stack:
>>  8800b66a7240 8800b66a7380 8800377bd3f0 
>>  023302f0 8800aed33e18 a00346e2 8800b66a7240
>>  8800aed33e38 a003474d 8800377bd388 8800377bd390
>> Call Trace:
>>  [] remove_vq_common+0x22/0x40 [virtio_net]
>>  [] virtnet_remove+0x4d/0x90 [virtio_net]
>>  [] virtio_dev_remove+0x23/0x80
>>  [] __device_release_driver+0x7f/0xf0
>>  [] driver_detach+0xc0/0xd0
>>  [] bus_remove_driver+0x58/0xd0
>>  [] driver_unregister+0x2c/0x50
>>  [] unregister_virtio_driver+0xe/0x10
>>  [] virtio_net_driver_exit+0x10/0x7be [virtio_net]
>>  [] SyS_delete_module+0x172/0x220
>>  [] ? trace_hardirqs_on+0xd/0x10
>>  [] ? __audit_syscall_entry+0x9c/0xf0
>>  [] system_call_fastpath+0x16/0x1b
>> Code: c0 74 55 0f 1f 44 00 00 80 7b 30 00 74 7a 48 8b 50 30 4c 89 e6 48 03 
>> 73 20 48 85 d2 0f 84 bb 00 00 00 66 0f
>> RIP  [] free_unused_bufs+0xc3/0x190 [virtio_net]
>>  RSP 
>> ---[ end trace edb570ea923cce9c ]---
>>
>> Fixes: 2613af0ed18a (virtio_net: migrate mergeable rx buffers to page frag 
>> allocators)
>> Cc: Michael Dalton 
>> Cc: Rusty Russell 
>> Cc: "Michael S. Tsirkin" 
>> Signed-off-by: Andrey Vagin 
>> ---
>>  drivers/net/virtio_net.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 916241d..930039a 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -1396,10 +1396,10 @@ static void free_unused_bufs(struct virtnet_info *vi)
>>  struct virtqueue *vq = vi->rq[i].vq;
>>  
>>  while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
>> -if (vi->big_packets)
>> -give_pages(&vi->rq[i], buf);
>> -    else if (vi->mergeable_rx_bufs)
>> +if (vi->mergeable_rx_bufs)
>>  put_page(virt_to_head_page(buf));
>> +else if (vi->big_packets)
>> +give_pages(&vi->rq[i], buf);
>>  else
>>  dev_kfree_skb(buf);
>>  --vi->rq[i].num;
>
> Acked-by: Jason Wang 

Acked-by: Rusty Russell 

Dave, please apply.

Thanks,
Rusty.
--
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 2/2] virtio: delete napi structures from netdev before releasing memory

2013-12-06 Thread Rusty Russell
Jason Wang  writes:
> On 12/05/2013 10:36 PM, Andrey Vagin wrote:
>> free_netdev calls netif_napi_del too, but it's too late, because napi
>> structures are placed on vi->rq. netif_napi_add() is called from
>> virtnet_alloc_queues.
>>
>> general protection fault:  [#1] SMP
>> Dumping ftrace buffer:
>>(ftrace buffer empty)
>> Modules linked in: ip6table_filter ip6_tables iptable_filter ip_tables 
>> virtio_balloon pcspkr virtio_net(-) i2c_pii
>> CPU: 1 PID: 347 Comm: rmmod Not tainted 3.13.0-rc2+ #171
>> Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
>> task: 8800b779c420 ti: 8800379e task.ti: 8800379e
>> RIP: 0010:[]  [] 
>> __list_del_entry+0x29/0xd0
>> RSP: 0018:8800379e1dd0  EFLAGS: 00010a83
>> RAX: 6b6b6b6b6b6b6b6b RBX: 8800379c2fd0 RCX: dead00200200
>> RDX: 6b6b6b6b6b6b6b6b RSI: 0001 RDI: 8800379c2fd0
>> RBP: 8800379e1dd0 R08: 0001 R09: 
>> R10:  R11: 0001 R12: 8800379c2f90
>> R13: 880037839160 R14:  R15: 013352f0
>> FS:  7f1400e34740() GS:8800bfb0() knlGS:
>> CS:  0010 DS:  ES:  CR0: 8005003b
>> CR2: 7f464124c763 CR3: b68cf000 CR4: 06e0
>> Stack:
>>  8800379e1df0 8155beab 6b6b6b6b6b6b6b2b 8800378391c0
>>  8800379e1e18 8156499b 880037839be0 880037839d20
>>  88003779d3f0 8800379e1e38 a003477c 88003779d388
>> Call Trace:
>>  [] netif_napi_del+0x1b/0x80
>>  [] free_netdev+0x8b/0x110
>>  [] virtnet_remove+0x7c/0x90 [virtio_net]
>>  [] virtio_dev_remove+0x23/0x80
>>  [] __device_release_driver+0x7f/0xf0
>>  [] driver_detach+0xc0/0xd0
>>  [] bus_remove_driver+0x58/0xd0
>>  [] driver_unregister+0x2c/0x50
>>  [] unregister_virtio_driver+0xe/0x10
>>  [] virtio_net_driver_exit+0x10/0x6ce [virtio_net]
>>  [] SyS_delete_module+0x172/0x220
>>  [] ? trace_hardirqs_on+0xd/0x10
>>  [] ? __audit_syscall_entry+0x9c/0xf0
>>  [] system_call_fastpath+0x16/0x1b
>> Code: 00 00 55 48 8b 17 48 b9 00 01 10 00 00 00 ad de 48 8b 47 08 48 89 e5 
>> 48 39 ca 74 29 48 b9 00 02 20 00 00 00
>> RIP  [] __list_del_entry+0x29/0xd0
>>  RSP 
>> ---[ end trace d5931cd3f87c9763 ]---
>>
>> Fixes: 986a4f4d452d (virtio_net: multiqueue support)
>> Cc: Rusty Russell 
>> Cc: "Michael S. Tsirkin" 
>> Signed-off-by: Andrey Vagin 
>> ---
>>  drivers/net/virtio_net.c | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 930039a..c293764 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -1367,6 +1367,11 @@ static void virtnet_config_changed(struct 
>> virtio_device *vdev)
>>  
>>  static void virtnet_free_queues(struct virtnet_info *vi)
>>  {
>> +int i;
>> +
>> +for (i = 0; i < vi->max_queue_pairs; i++)
>> +netif_napi_del(&vi->rq[i].napi);
>> +
>>  kfree(vi->rq);
>>  kfree(vi->sq);
>>  }
>
> Acked-by: Jason Wang 

Acked-by: Rusty Russell 

Needed since v3.8.  Dave, please apply.

Cheers,
Rusty.
--
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 v3 1/2] test: add minimal module for verification testing

2013-12-04 Thread Rusty Russell
Andrew Morton  writes:
> On Thu, 05 Dec 2013 13:12:17 +1030 Rusty Russell  
> wrote:
>
>> Kees Cook  writes:
>> > When doing module loading verification tests (for example, with module
>> > singing, or LSM hooks), it is very handy to have a module that can be
>> 
>> "module singing" sounds like a horrible idea!  Is the author even
>> musical?  I've only heard it said David Howls.
>
> You're such a killjoy.
>
> btw, git log | grep Singed

I had my ego singed off by Linus once, so I completely understand.

Cheers,
Rusty.
--
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] virtio_balloon: update_balloon_size(): update correct field

2013-12-04 Thread Rusty Russell
Luiz Capitulino  writes:
> According to the virtio spec, the device configuration field
> that should be updated after an inflation or deflation
> operation is the 'actual' field, not the 'num_pages' one.
>
> Commit 855e0c5288177bcb193f6f6316952d2490478e1c swapped them
> in update_balloon_size(). Fix it.
>
> Signed-off-by: Luiz Capitulino 

Damn, exactly right.  Good catch.

Applied,
Rusty.

>  drivers/virtio/virtio_balloon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index c444654..5c4a95b 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -285,7 +285,7 @@ static void update_balloon_size(struct virtio_balloon *vb)
>  {
>   __le32 actual = cpu_to_le32(vb->num_pages);
>  
> - virtio_cwrite(vb->vdev, struct virtio_balloon_config, num_pages,
> + virtio_cwrite(vb->vdev, struct virtio_balloon_config, actual,
> &actual);
>  }
>  
> -- 
> 1.8.1.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 v3 1/2] test: add minimal module for verification testing

2013-12-04 Thread Rusty Russell
Kees Cook  writes:
> When doing module loading verification tests (for example, with module
> singing, or LSM hooks), it is very handy to have a module that can be

"module singing" sounds like a horrible idea!  Is the author even
musical?  I've only heard it said David Howls.

But if my ack for the patch helps:

    Acked-by: Rusty Russell 

Cheers,
Rusty.


> built on all systems under test, isn't auto-loaded at boot, and has
> no device or similar dependencies. This creates the "test_module.ko"
> module for that purpose, which only reports its load and unload to printk.
>
> Signed-off-by: Kees Cook 
> ---
> v3:
>  - use KBUILD_MODNAME; Rusty Russell
> v2:
>  - use pr_warn, better comment, add headers explicitly, move to lib/; akpm.
> ---
>  lib/Kconfig.debug |   14 ++
>  lib/Makefile  |1 +
>  lib/test_module.c |   33 +
>  3 files changed, 48 insertions(+)
>  create mode 100644 lib/test_module.c
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index db25707aa41b..81882335c625 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -1578,6 +1578,20 @@ config DMA_API_DEBUG
> This option causes a performance degredation.  Use only if you want
> to debug device drivers. If unsure, say N.
>  
> +config TEST_MODULE
> + tristate "Test module loading with 'hello world' module"
> + default n
> + depends on m
> + help
> +   This builds the "test_module" module that emits "Hello, world"
> +   on printk when loaded. It is designed to be used for basic
> +   evaluation of the module loading subsystem (for example when
> +   validating module verification). It lacks any extra dependencies,
> +   and will not normally be loaded by the system unless explicitly
> +   requested by name.
> +
> +   If unsure, say N.
> +
>  source "samples/Kconfig"
>  
>  source "lib/Kconfig.kgdb"
> diff --git a/lib/Makefile b/lib/Makefile
> index a459c31e8c6b..b494b9af631c 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -31,6 +31,7 @@ obj-y += string_helpers.o
>  obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o
>  obj-y += kstrtox.o
>  obj-$(CONFIG_TEST_KSTRTOX) += test-kstrtox.o
> +obj-$(CONFIG_TEST_MODULE) += test_module.o
>  
>  ifeq ($(CONFIG_DEBUG_KOBJECT),y)
>  CFLAGS_kobject.o += -DDEBUG
> diff --git a/lib/test_module.c b/lib/test_module.c
> new file mode 100644
> index ..319b66f1ff61
> --- /dev/null
> +++ b/lib/test_module.c
> @@ -0,0 +1,33 @@
> +/*
> + * This module emits "Hello, world" on printk when loaded.
> + *
> + * It is designed to be used for basic evaluation of the module loading
> + * subsystem (for example when validating module signing/verification). It
> + * lacks any extra dependencies, and will not normally be loaded by the
> + * system unless explicitly requested by name.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include 
> +#include 
> +#include 
> +
> +static int __init test_module_init(void)
> +{
> + pr_warn("Hello, world\n");
> +
> + return 0;
> +}
> +
> +module_init(test_module_init);
> +
> +static void __exit test_module_exit(void)
> +{
> + pr_warn("Goodbye\n");
> +}
> +
> +module_exit(test_module_exit);
> +
> +MODULE_AUTHOR("Kees Cook ");
> +MODULE_LICENSE("GPL");
> -- 
> 1.7.9.5
--
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 1/2] test: add minimal module for verification testing

2013-12-03 Thread Rusty Russell
Kees Cook  writes:
> When doing module loading verification tests (for example, with module
> singing, or LSM hooks), it is very handy to have a module that can be
> built on all systems under test, isn't auto-loaded at boot, and has
> no device or similar dependencies. This creates the "test_module.ko"
> module for that purpose, which only reports its load and unload to printk.

Bikeshed time!

> +#define pr_fmt(fmt) "test_module: " fmt

Perhaps:
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

Cheers,
Rusty.
--
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: Some kernel module options not showing up in modinfo

2013-11-30 Thread Rusty Russell
Raphael Hertzog  writes:
> Hi,
>
> On Fri, 22 Nov 2013, Rusty Russell wrote:
>> Raphael Hertzog  writes:
>> > Hello,
>> >
>> > I noticed that some options are not visible in the modinfo output. For
>> > instance "modinfo lockd" reports this on my Debian sid system:
> [...]
>> > Multiple other options are defined by way of module_param_call(...) like
>> > nlm_grace_period, nlm_timeout, nlm_updport, nlm_tcpport.
>> 
>> Yes, the module_param() macro adds a type description.  The
>> module_param_call() does not.
>
> Couldn't they be displayed without any type or with a type "unknown" ?
>
> parm: nlm_grace_period
>
> or 
>
> parm: nlm_grace_period:unknown

The type thing is really a hack.  Ideally we should force documentation
on everyone who declares a module parameter with a new macro we can
transition to.

Of course, most descriptions would be incoherent, a-la Kbuild help
messages, so we'd need an enforcer...

Rusty.
--
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 v4] Add Documentation/module-signing.txt file

2013-11-19 Thread Rusty Russell
David Howells  writes:
> James Solner  wrote:
>
>> This patch adds the Documentation/module-signing.txt file that is
>> currently missing from the Documentation directory. The init/Kconfig
>> file references the Documentation/module-signing.txt file to explain
>> how kernel module signing works. This patch supplies this documentation. 
>> 
>> The initial version of this patch provided old documentation
>> that was a mixture of the old RHEL style GPG signing. 
>> 
>> Version 1: Updated the documentation to described the current
>>implementation using x509 certificate signing. 
>> 
>> Version 2: fixes grammar/spelling mistakes and removes
>>trailing whitespaces. 
>> 
>> Version 3, fixes grammar/spelling mistakes. 
>> 
>> Version 4: Include updates from David Howells and fixes for
>>spelling mistakes.
>> 
>> Signed-off-by: James Solner 
>
> Signed-off-by: David Howells 

?  I changed this to an Acked-by...

Cheers,
Rusty.
--
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] core kernel update

2013-11-19 Thread Rusty Russell
Ingo Molnar  writes:
> [*] Plus perhaps allow offstack to be configurable arbitrarily if 
> debugging is enabled [DEBUG_PER_CPU_MAP=y], to allow easy
> experiments/measurements?

Yes, it was good for i386 testing.

But now it seems that DEBUG_KERNEL is on for every distribution
(AFAICT), it's no longer an obscure option.  I'm happy to kill it.

Cheers,
Rusty.
--
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 11/17] kernel/param: Consolidate __{start,stop}___param[] in

2013-11-13 Thread Rusty Russell
Geert Uytterhoeven  writes:
> Consolidate the various external const and non-const declarations of
> __start___param[] and __stop___param in .
> This requires making a few struct kernel_param pointers in kernel/params.c
> const.
>
> Signed-off-by: Geert Uytterhoeven 
> Cc: Rusty Russell 

Acked-by: Rusty Russell 

Thanks!
Rusty.

> ---
>  include/linux/moduleparam.h |2 ++
>  init/main.c |2 --
>  kernel/params.c |7 +++
>  3 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> index c3eb102a9cc8..77ccfca0c4ba 100644
> --- a/include/linux/moduleparam.h
> +++ b/include/linux/moduleparam.h
> @@ -68,6 +68,8 @@ struct kernel_param {
>   };
>  };
>  
> +extern const struct kernel_param __start___param[], __stop___param[];
> +
>  /* Special one for strings we want to copy into */
>  struct kparam_string {
>   unsigned int maxlen;
> diff --git a/init/main.c b/init/main.c
> index 15ed159b681b..f2c4901f90d1 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -469,7 +469,6 @@ static void __init mm_init(void)
>  asmlinkage void __init start_kernel(void)
>  {
>   char * command_line;
> - extern const struct kernel_param __start___param[], __stop___param[];
>  
>   /*
>* Need to run as early as possible, to initialize the
> @@ -737,7 +736,6 @@ static char *initcall_level_names[] __initdata = {
>  
>  static void __init do_initcall_level(int level)
>  {
> - extern const struct kernel_param __start___param[], __stop___param[];
>   initcall_t *fn;
>  
>   strcpy(static_command_line, saved_command_line);
> diff --git a/kernel/params.c b/kernel/params.c
> index c00d5b502aa4..54e05ffb7d24 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -19,6 +19,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -506,8 +507,6 @@ EXPORT_SYMBOL(param_ops_string);
>  #define to_module_attr(n) container_of(n, struct module_attribute, attr)
>  #define to_module_kobject(n) container_of(n, struct module_kobject, kobj)
>  
> -extern struct kernel_param __start___param[], __stop___param[];
> -
>  struct param_attribute
>  {
>   struct module_attribute mattr;
> @@ -766,7 +765,7 @@ static struct module_kobject * __init 
> locate_module_kobject(const char *name)
>  }
>  
>  static void __init kernel_add_sysfs_param(const char *name,
> -   struct kernel_param *kparam,
> +   const struct kernel_param *kparam,
> unsigned int name_skip)
>  {
>   struct module_kobject *mk;
> @@ -801,7 +800,7 @@ static void __init kernel_add_sysfs_param(const char 
> *name,
>   */
>  static void __init param_sysfs_builtin(void)
>  {
> - struct kernel_param *kp;
> + const struct kernel_param *kp;
>   unsigned int name_len;
>   char modname[MODULE_NAME_LEN];
>  
> -- 
> 1.7.9.5
--
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/


[PULL v2] virtio-next

2013-11-10 Thread Rusty Russell
The following changes since commit d8524ae9d6f492a9c6db9f4d89c5f9b8782fa2d5:

  Merge branch 'drm-fixes' of git://people.freedesktop.org/~airlied/linux 
(2013-09-22 19:51:49 -0700)

are available in the git repository at:


  git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux.git 
tags/virtio-next-for-linus

for you to fetch changes up to 2bf4fd31394a3f875ea093ee8a209f30b378cbf3:

  virtio_scsi: verify if queue is broken after virtqueue_get_buf() (2013-11-11 
11:53:26 +1030)


Nothing really exciting: some groundwork for changing virtio endian, and
some robustness fixes for broken virtio devices, plus minor tweaks.

[vs last pull request: added the virtio-scsi broken vq escape patch, which
I somehow lost.]

Cheers,
Rusty.


Aaron Lu (1):
  virtio: pm: use CONFIG_PM_SLEEP instead of CONFIG_PM

Andi Kleen (1):
  x86, asmlinkage, lguest: Pass in globals into assembler statement

Heinz Graalfs (10):
  virtio_ring: change host notification API
  virtio_ring: let virtqueue_{kick()/notify()} return a bool
  virtio_net: verify if virtqueue_kick() succeeded
  virtio_test: verify if virtqueue_kick() succeeded
  virtio_ring: add new function virtqueue_is_broken()
  virtio_blk: verify if queue is broken after virtqueue_get_buf()
  virtio_console: verify if queue is broken after virtqueue_get_buf()
  virtio_net: verify if queue is broken after virtqueue_get_buf()
  virtio_ring: adapt to notify() returning bool
  virtio_scsi: verify if queue is broken after virtqueue_get_buf()

Marc Zyngier (1):
  virtio: mmio: fix signature checking for BE guests

Rusty Russell (4):
  virtio_ring: plug kmemleak false positive.
  virtio_config: introduce size-based accessors.
  virtio: use size-based config accessors.
  virtio_config: remove virtio_config_val

 drivers/block/virtio_blk.c |  83 -
 drivers/char/hw_random/virtio-rng.c|   4 +-
 drivers/char/virtio_console.c  |  25 +++--
 drivers/lguest/lguest_device.c |   3 +-
 drivers/lguest/x86/core.c  |   6 +-
 drivers/net/caif/caif_virtio.c |  23 ++---
 drivers/net/virtio_net.c   |  44 +
 drivers/remoteproc/remoteproc_virtio.c |   3 +-
 drivers/s390/kvm/kvm_virtio.c  |   8 +-
 drivers/s390/kvm/virtio_ccw.c  |   5 +-
 drivers/scsi/virtio_scsi.c |  19 ++--
 drivers/virtio/virtio_balloon.c|  14 ++-
 drivers/virtio/virtio_mmio.c   |   5 +-
 drivers/virtio/virtio_pci.c|   3 +-
 drivers/virtio/virtio_ring.c   |  34 +--
 include/linux/virtio.h |   6 +-
 include/linux/virtio_config.h  | 161 +++--
 include/linux/virtio_ring.h|   2 +-
 net/9p/trans_virtio.c  |   9 +-
 tools/virtio/virtio_test.c |   6 +-
 tools/virtio/vringh_test.c |  13 ++-
 21 files changed, 310 insertions(+), 166 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/


Re: kernel BUG at kernel/kallsyms.c:222!

2013-11-10 Thread Rusty Russell
Ming Lei  writes:
> Hi Axel,
>
> On Fri, Nov 8, 2013 at 12:20 PM, Axel Lin  wrote:
>>
>>
>> Hi Ming,
>>
>> I have patches on top of 3.12 to support gpl32700 SoC.
>> So you cannot find this platform on mainline kernel.
>
> OK, I see.
>
>> I havn't tried perf, below is my config for your reference:
>> (to make the config smaller, I grep out disabled config entries.)
>
> I doubt CONFIG_VMSPLIT_3G/CONFIG_PAGE_OFFSET isn't
> used at all in your unmerged patchset, also your link script should
> be different with in-tree arch/arm/kernel/vmlinux.lds.S, otherwise
> you may put all your kernel symbols after 0xC.
>
> Maybe you need to not define PAGE_OFFSET and let it be
> PHYS_OFFSET at default, also VMSPLIT_3G shouldn't have
> been there on non-MMU linux.

OK, unless there are other platforms which have this issue, I'll
leave it with you to hold this patch for merge with your SoC.

Cheers,
Rusty.
--
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: [trivial PATCH] module.h: Remove unnecessary semicolon

2013-11-07 Thread Rusty Russell
Joe Perches  writes:
> On Fri, 2013-11-08 at 09:41 +1030, Rusty Russell wrote:
>> Joe Perches  writes:
>> > That's a trust issue.
>> > I've done it.  It isn't necessary.
>> 
>> WTF?  Now you just said it's not necessary, I *know* I can't trust you.
>
> "It" in this case is the grep that I did
> prior to sending the patch.

Hi Joe,

Apologies for my off tone.  But I think we got tangled somewhere?

You said "It isn't necessary".

You maintain that grepping the source to find out if you'd broken
something "isn't necessary"?

git history shows me you've done lots of these cleanups.  I'm pretty
sure that's not what you meant.

But as maintainer, it's annoying that I had to check myself.  I don't
have personal experience in how diligent you are.  If you'd just
mentioned it, it would have saved me a few minutes and streamlined my
workflow immensely. 

Providing assurance makes me a happy maintainer.  So I modified it
because it sets a clear example for others.

I hope that clarifies,
Rusty.
--
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: [trivial PATCH] module.h: Remove unnecessary semicolon

2013-11-07 Thread Rusty Russell
Joe Perches  writes:
> On Thu, 2013-11-07 at 12:32 +1030, Rusty Russell wrote:
>> Joe Perches  writes:
>> > This semicolon isn't necessary, remove it.
>> >
>> > Signed-off-by: Joe Perches 
>> 
>> This is a terrible description.  Really bad.
>
> I'd've preferred no description.

Me too.

>> First, it just repeats the subject, with more words.
>
> Which others have demanded.

They're wrong.

>> Second, it gives me no indication that you've done a grep to make sure
>> noone is abusing the macro, so I can't apply it without doing that check
>> myself.
>
> That's a trust issue.
> I've done it.  It isn't necessary.

WTF?  Now you just said it's not necessary, I *know* I can't trust you.

> The other #define module_put_and_exit in a
> different #if #else already doesn't have one.

You didn't mention this, and even if you did, it's not sufficient.  Some
code only ever gets compiled as a module, so it'd never hit the
!CONFIG_MODULES case.

> Trust it or not, apply it or not.

Now I know when I receive a patch from you I have to check it carefully,
because you can't be bothered.

I've fixed your patch, you can find it below.

From: Joe Perches 
Subject: module.h: Remove unnecessary semicolon

[All 8 callers already have semicolons. -- RR]

Signed-off-by: Joe Perches 
Signed-off-by: Rusty Russell 
---
 include/linux/module.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index 15cd6b1b211e..46e548fd502a 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -451,7 +451,7 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const 
char *,
 
 extern void __module_put_and_exit(struct module *mod, long code)
__attribute__((noreturn));
-#define module_put_and_exit(code) __module_put_and_exit(THIS_MODULE, code);
+#define module_put_and_exit(code) __module_put_and_exit(THIS_MODULE, code)
 
 #ifdef CONFIG_MODULE_UNLOAD
 unsigned long module_refcount(struct module *mod);
--
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: kernel BUG at kernel/kallsyms.c:222!

2013-11-07 Thread Rusty Russell
Axel Lin  writes:
> 2013/11/7 Ming Lei :
>> Hi,
>>
>> On Thu, Nov 7, 2013 at 10:47 AM, Axel Lin  wrote:
>>>
>>> hi Ming,
>>> Seems CONFIG_PAGE_OFFSET is not configurabe in "make menuconfig".
>>> And I found CONFIG_PAGE_OFFSET=0xC000 for all below configs...
>>> $ make at91_dt_defconfig; grep  CONFIG_PAGE_OFFSET .config
>>> $ make ep93xx_defconfig; grep  CONFIG_PAGE_OFFSET .config
>>> $ make imx_v4_v5_defconfig; grep CONFIG_PAGE_OFFSET .config
>>> $ make mxs_defconfig; grep CONFIG_PAGE_OFFSET .config
>>> $ make omap2plus_defconfig; grep CONFIG_PAGE_OFFSET .config
>>> $ make s3c6400_defconfig; grep CONFIG_PAGE_OFFSET .config
>>> $ make at91x40_defconfig; grep CONFIG_PAGE_OFFSET .config
>>> ( at91x40_defconfig is also arm7tdmi )
>>
>> Firstly it can be configured via VMSPLIT_3GVMSPLIT_2G/VMSPLIT_1G.
>>
>> Secondly, configurable or not isn't the point, and maybe some uclinux
>> platforms do not use CONFIG_PAGE_OFFSET at all, but they should
>> set it as a reasonable value or at least be below than the start link
>> address of vmlinux.
>
> Hi Ming,
>
> I found in arch/arm/include/asm/memory.h:
> CONFIG_PAGE_OFFSET is not used if !CONFIG_MMU.
> So looks like setting CONFIG_PAGE_OFFSET to other value still won't work.

This seems like the simplest solution, but it may mean you still have
crap in /proc/kallsyms.

Does it work for you?

Thanks,
Rusty.


diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 32b10f53d0b4..c3bd3efec4cc 100644
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -82,7 +82,9 @@ kallsyms()
kallsymopt="${kallsymopt} --all-symbols"
fi
 
-   kallsymopt="${kallsymopt} --page-offset=$CONFIG_PAGE_OFFSET"
+   if [ -n "${CONFIG_MMU}" ]; then
+   kallsymopt="${kallsymopt} --page-offset=$CONFIG_PAGE_OFFSET"
+   fi
 
local aflags="${KBUILD_AFLAGS} ${KBUILD_AFLAGS_KERNEL}   \
  ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS}"
--
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/


[PULL] virtio-next

2013-11-07 Thread Rusty Russell
The following changes since commit d8524ae9d6f492a9c6db9f4d89c5f9b8782fa2d5:

  Merge branch 'drm-fixes' of git://people.freedesktop.org/~airlied/linux 
(2013-09-22 19:51:49 -0700)

are available in the git repository at:


  git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux.git 
tags/virtio-next-for-linus

for you to fetch changes up to cdd77e87eae52b7251acc5990207a1c4500a84ce:

  x86, asmlinkage, lguest: Pass in globals into assembler statement (2013-11-07 
12:13:05 +1030)


Nothing really exciting: some groundwork for changing virtio endian, and
some robustness fixes for broken virtio devices, plus minor tweaks.

Cheers,
Rusty.


Aaron Lu (1):
  virtio: pm: use CONFIG_PM_SLEEP instead of CONFIG_PM

Andi Kleen (1):
  x86, asmlinkage, lguest: Pass in globals into assembler statement

Heinz Graalfs (9):
  virtio_ring: change host notification API
  virtio_ring: let virtqueue_{kick()/notify()} return a bool
  virtio_net: verify if virtqueue_kick() succeeded
  virtio_test: verify if virtqueue_kick() succeeded
  virtio_ring: add new function virtqueue_is_broken()
  virtio_blk: verify if queue is broken after virtqueue_get_buf()
  virtio_console: verify if queue is broken after virtqueue_get_buf()
  virtio_net: verify if queue is broken after virtqueue_get_buf()
  virtio_ring: adapt to notify() returning bool

Marc Zyngier (1):
  virtio: mmio: fix signature checking for BE guests

Rusty Russell (4):
  virtio_ring: plug kmemleak false positive.
  virtio_config: introduce size-based accessors.
  virtio: use size-based config accessors.
  virtio_config: remove virtio_config_val

 drivers/block/virtio_blk.c |  83 -
 drivers/char/hw_random/virtio-rng.c|   4 +-
 drivers/char/virtio_console.c  |  25 +++--
 drivers/lguest/lguest_device.c |   3 +-
 drivers/lguest/x86/core.c  |   6 +-
 drivers/net/caif/caif_virtio.c |  23 ++---
 drivers/net/virtio_net.c   |  44 +
 drivers/remoteproc/remoteproc_virtio.c |   3 +-
 drivers/s390/kvm/kvm_virtio.c  |   8 +-
 drivers/s390/kvm/virtio_ccw.c  |   5 +-
 drivers/scsi/virtio_scsi.c |  16 ++--
 drivers/virtio/virtio_balloon.c|  14 ++-
 drivers/virtio/virtio_mmio.c   |   5 +-
 drivers/virtio/virtio_pci.c|   3 +-
 drivers/virtio/virtio_ring.c   |  34 +--
 include/linux/virtio.h |   6 +-
 include/linux/virtio_config.h  | 161 +++--
 include/linux/virtio_ring.h|   2 +-
 net/9p/trans_virtio.c  |   9 +-
 tools/virtio/virtio_test.c |   6 +-
 tools/virtio/vringh_test.c |  13 ++-
 21 files changed, 307 insertions(+), 166 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/


Re: [PATCH v3] Add Documentation/module-signing.txt file

2013-11-06 Thread Rusty Russell
Randy Dunlap  writes:
> On 11/06/13 10:53, James Solner wrote:
>> This patch adds the Documentation/module-signing.txt file that is
>> currently missing from the Documentation directory. The init/Kconfig
>> file references the Documentation/module-signing.txt file to explain
>> how kernel module signing works. This patch supplies this documentation. 
>> 
>> The initial version of this patch provided old documentation
>> that was a mixture of the old RHEL style GPG signing. 
>> Version 1 updated the documentation to described the current
>> implementation using x509 certificate signing. 
>> Version 2, fixes grammar/spelling mistakes and removes
>> trailing whitespaces. Version 3, fixes grammar/spelling mistakes. 
>> 
>> Signed-off-by: James Solner 
>> 
>
> Looks good to me.  I'll let David, Josh, Rusty, et al, comment
> on the real content.

Once David Acks, I'll take it.

Thanks,
Rusty.
--
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: [trivial PATCH] module.h: Remove unnecessary semicolon

2013-11-06 Thread Rusty Russell
Joe Perches  writes:
> This semicolon isn't necessary, remove it.
>
> Signed-off-by: Joe Perches 

This is a terrible description.  Really bad.

First, it just repeats the subject, with more words.

Second, it gives me no indication that you've done a grep to make sure
noone is abusing the macro, so I can't apply it without doing that check
myself.

Please try again.

Rusty.

> ---
>  include/linux/module.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 05f2447..d1ad477 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -454,7 +454,7 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, 
> const char *,
>  
>  extern void __module_put_and_exit(struct module *mod, long code)
>   __attribute__((noreturn));
> -#define module_put_and_exit(code) __module_put_and_exit(THIS_MODULE, code);
> +#define module_put_and_exit(code) __module_put_and_exit(THIS_MODULE, code)
>  
>  #ifdef CONFIG_MODULE_UNLOAD
>  unsigned long module_refcount(struct module *mod);
--
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: linux-next: build warnings after merge of the modules tree

2013-11-04 Thread Rusty Russell
Stephen Rothwell  writes:
> Hi Rusty,
>
> After merging the modules tree, today's linux-next build (x86_64
> allmodconfig) produced these warning:
>
> WARNING: vmlinux: 'pci_restore_msi_state' exported twice. Previous export was 
> in vmlinux
> WARNING: vmlinux: '__mod_zone_page_state' exported twice. Previous export was 
> in vmlinux
> WARNING: vmlinux: 'scsi_prep_return' exported twice. Previous export was in 
> vmlinux
> WARNING: vmlinux: 'hvc_poll' exported twice. Previous export was in vmlinux
> WARNING: vmlinux: 'nfs_clear_inode' exported twice. Previous export was in 
> vmlinux
>
> (just some samples ... it scrolled off my scrollback :-()
>
> Presumably caused by commit e0f244c63fc9 ("asmlinkage, module: Make
> ksymtab and kcrctab symbols and __this_module __visible").  (reverting
> that commit makes the warnings go away.)

This works for me feedback please!

Cheers,
Rusty.

modpost: fix bogus 'exported twice' warnings.

Andi's change in e0f244c63fc9 ("asmlinkage, module: Make ksymtab and
kcrctab symbols and __this_module __visible") make the crc appear
first in the symbol table.

modpost creates an entry when it sees the CRC, then when it sees the
actual symbol, it complains that it's seen it before.  The preloaded
flag already exists for the equivalent case where we loaded from
Module.symvers, so use that.

Signed-off-by: Rusty Russell 

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 9b873ac6ed7b..5c677a3e7487 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -164,7 +164,7 @@ struct symbol {
unsigned int vmlinux:1;/* 1 if symbol is defined in vmlinux */
unsigned int kernel:1; /* 1 if symbol is from kernel
*  (only for external modules) **/
-   unsigned int preloaded:1;  /* 1 if symbol from Module.symvers */
+   unsigned int preloaded:1;  /* 1 if symbol from Module.symvers, or crc */
enum export  export;   /* Type of export */
char name[0];
 };
@@ -332,8 +332,11 @@ static void sym_update_crc(const char *name, struct module 
*mod,
 {
struct symbol *s = find_symbol(name);
 
-   if (!s)
+   if (!s) {
s = new_symbol(name, mod, export);
+   /* Don't complain when we find it later. */
+   s->preloaded = 1;
+   }
s->crc = crc;
s->crc_valid = 1;
 }
--
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/


[PULL] symbol fix for ARM kallsyms

2013-11-01 Thread Rusty Russell
The following changes since commit 12aee278b50c4a94a93fa0b4d201ae35d792c696:

  Merge branch 'akpm' (fixes from Andrew Morton) (2013-10-30 14:27:10 -0700)

are available in the git repository at:


  git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux.git 
tags/fixes-for-linus

for you to fetch changes up to f6537f2f0eba4eba3354e48dbe3047db6d8b6254:

  scripts/kallsyms: filter symbols not in kernel address space (2013-11-02 
09:13:02 +1030)


Last minute perf unbreakage for ARM modules; spent a day in linux-next.


Ming Lei (1):
  scripts/kallsyms: filter symbols not in kernel address space

 scripts/kallsyms.c  | 12 +++-
 scripts/link-vmlinux.sh |  2 ++
 2 files changed, 13 insertions(+), 1 deletion(-)
--
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] scripts/kallsyms: filter symbols not in kernel address space

2013-10-31 Thread Rusty Russell
From: Ming Lei 

This patch uses CONFIG_PAGE_OFFSET to filter symbols which
are not in kernel address space because these symbols are
generally for generating code purpose and can't be run at
kernel mode, so we needn't keep them in /proc/kallsyms.

For example, on ARM there are some symbols which may be
linked in relocatable code section, then perf can't parse
symbols any more from /proc/kallsyms, this patch fixes the
problem (introduced b9b32bf70f2fb710b07c94e13afbc729afe221da)

Cc: Russell King 
Cc: linux-arm-ker...@lists.infradead.org
Cc: Michal Marek 
Signed-off-by: Ming Lei 
Signed-off-by: Rusty Russell 
Cc: sta...@kernel.org
---
Stephen, please run this on todays' linux-next, so I can push to Linus
asap.  Thanks...

 scripts/kallsyms.c  | 12 +++-
 scripts/link-vmlinux.sh |  2 ++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 487ac6f..9a11f9f 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -55,6 +55,7 @@ static struct sym_entry *table;
 static unsigned int table_size, table_cnt;
 static int all_symbols = 0;
 static char symbol_prefix_char = '\0';
+static unsigned long long kernel_start_addr = 0;
 
 int token_profit[0x1];
 
@@ -65,7 +66,10 @@ unsigned char best_table_len[256];
 
 static void usage(void)
 {
-   fprintf(stderr, "Usage: kallsyms [--all-symbols] 
[--symbol-prefix=] < in.map > out.S\n");
+   fprintf(stderr, "Usage: kallsyms [--all-symbols] "
+   "[--symbol-prefix=] "
+   "[--page-offset=] "
+   "< in.map > out.S\n");
exit(1);
 }
 
@@ -194,6 +198,9 @@ static int symbol_valid(struct sym_entry *s)
int i;
int offset = 1;
 
+   if (s->addr < kernel_start_addr)
+   return 0;
+
/* skip prefix char */
if (symbol_prefix_char && *(s->sym + 1) == symbol_prefix_char)
offset++;
@@ -646,6 +653,9 @@ int main(int argc, char **argv)
if ((*p == '"' && *(p+2) == '"') || (*p == '\'' 
&& *(p+2) == '\''))
p++;
symbol_prefix_char = *p;
+   } else if (strncmp(argv[i], "--page-offset=", 14) == 0) 
{
+   const char *p = &argv[i][14];
+   kernel_start_addr = strtoull(p, NULL, 16);
} else
usage();
}
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 0149949..32b10f5 100644
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -82,6 +82,8 @@ kallsyms()
kallsymopt="${kallsymopt} --all-symbols"
fi
 
+   kallsymopt="${kallsymopt} --page-offset=$CONFIG_PAGE_OFFSET"
+
local aflags="${KBUILD_AFLAGS} ${KBUILD_AFLAGS_KERNEL}   \
  ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS}"
 
--
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] kernel/kallsyms.c: only show legal kernel symbol

2013-10-31 Thread Rusty Russell
Ming Lei  writes:

> On Thu, Oct 31, 2013 at 11:14 AM, Rusty Russell  wrote:
>> Russell King - ARM Linux  writes:
>>
>> Kallsyms tends to fall between modules and scripts.  I assume it's not
>> urgent, so no cc:stable on this one.
>
> Rusty, thanks a lot.
>
> BTW, there is already other report on the problem:
>
>http://lkml.indiana.edu/hypermail/linux/kernel/1310.3/02772.html

OK, *that* references the commit which is the problem, when went into
v3.11 (b9b32bf70f2fb710b07c94e13afbc729afe221da)

Unfortunately, *that commit* was cc:stable, so this needs to be
cc:stable as well, not just >= 3.11.

Looking back on those patches, there's a mass of cc:stable on them.  The
descriptions are either misleading, or these patches prevent theoretical
attacks which means they shouldn't have been cc:stable.

Grr...
Rusty.

--
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 net-next 2/2] virtio-net: coalesce rx frags when possible during rx

2013-10-31 Thread Rusty Russell
Jason Wang  writes:
> Commit 2613af0ed18a11d5c566a81f9a6510b73180660a (virtio_net: migrate mergeable
> rx buffers to page frag allocators) try to increase the payload/truesize for
> MTU-sized traffic. But this will introduce the extra overhead for GSO packets
> received because of the frag list. This commit tries to reduce this issue by
> coalesce the possible rx frags when possible during rx. Test result shows the
> about 15% improvement on full size GSO packet receiving (and even better than
> commit 2613af0ed18a11d5c566a81f9a6510b73180660a).

I don't know about the other users of skb_add_rx_frag, but should
this coalesce-if-possible code be built into that?

Thanks,
Rusty.
--
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] kernel/kallsyms.c: only show legal kernel symbol

2013-10-30 Thread Rusty Russell
Russell King - ARM Linux  writes:
> On Mon, Oct 28, 2013 at 04:20:19PM +1030, Rusty Russell wrote:
>> Ming Lei  writes:
>> > On Mon, 28 Oct 2013 13:44:30 +1030
>> > Rusty Russell  wrote:
>> >
>> >> Ming Lei  writes:
>> >> 
>> >> I don't know...  It would be your job, as the person making the change,
>> >> to find all the users of kallsyms and prove that.
>> >> 
>> >> This is why it is easier not to include incorrect values in the kernel's
>> >> kallsyms in the first place.
>> >
>> > OK, thanks for your comment, and I figured out one way to do it in
>> > scripts/kallsyms.c, could you comment on below patch?
>> 
>> Looks great! Seems like we spent more time arguing than it took you to
>> code that up.
>> 
>> Acked-by: Rusty Russell 
>> 
>> Russell, this seems logical for you to take along with the changes which
>> caused the problem?
>
> The changes are already in mainline since a long time (back in July/August
> time).  Am I the right person to take stuff for scripts/ ?  Isn't that
> more kbuild territory?

Kallsyms tends to fall between modules and scripts.  I assume it's not
urgent, so no cc:stable on this one.

Applied,
Rusty.
--
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] kernel/kallsyms.c: only show legal kernel symbol

2013-10-27 Thread Rusty Russell
Ming Lei  writes:
> On Mon, 28 Oct 2013 13:44:30 +1030
> Rusty Russell  wrote:
>
>> Ming Lei  writes:
>> 
>> I don't know...  It would be your job, as the person making the change,
>> to find all the users of kallsyms and prove that.
>> 
>> This is why it is easier not to include incorrect values in the kernel's
>> kallsyms in the first place.
>
> OK, thanks for your comment, and I figured out one way to do it in
> scripts/kallsyms.c, could you comment on below patch?

Looks great! Seems like we spent more time arguing than it took you to
code that up.

Acked-by: Rusty Russell 

Russell, this seems logical for you to take along with the changes which
caused the problem?

Thanks,
Rusty.

> --
> From 4327534dedfa60d208ac3e23db7556c243e1c7dc Mon Sep 17 00:00:00 2001
> From: Ming Lei 
> Date: Mon, 28 Oct 2013 13:04:49 +0800
> Subject: [PATCH] scripts/kallsyms: filter symbols not in kernel address space
>
> This patch uses CONFIG_PAGE_OFFSET to filter symbols which
> are not in kernel address space because these symbols are
> generally for generating code purpose and can't be run at
> kernel mode, so we needn't keep them in /proc/kallsyms.
>
> For example, on ARM there are some symbols which may be
> linked in relocatable code section, then perf can't parse
> symbols any more from /proc/kallsyms, this patch fixes the
> problem.
>
> Cc: Russell King 
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: Rusty Russell 
> Cc: Michal Marek 
> Signed-off-by: Ming Lei 
> ---
>  scripts/kallsyms.c  |   12 +++-
>  scripts/link-vmlinux.sh |2 ++
>  2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> index 487ac6f..9a11f9f 100644
> --- a/scripts/kallsyms.c
> +++ b/scripts/kallsyms.c
> @@ -55,6 +55,7 @@ static struct sym_entry *table;
>  static unsigned int table_size, table_cnt;
>  static int all_symbols = 0;
>  static char symbol_prefix_char = '\0';
> +static unsigned long long kernel_start_addr = 0;
>  
>  int token_profit[0x1];
>  
> @@ -65,7 +66,10 @@ unsigned char best_table_len[256];
>  
>  static void usage(void)
>  {
> - fprintf(stderr, "Usage: kallsyms [--all-symbols] 
> [--symbol-prefix=] < in.map > out.S\n");
> + fprintf(stderr, "Usage: kallsyms [--all-symbols] "
> + "[--symbol-prefix=] "
> + "[--page-offset=] "
> + "< in.map > out.S\n");
>   exit(1);
>  }
>  
> @@ -194,6 +198,9 @@ static int symbol_valid(struct sym_entry *s)
>   int i;
>   int offset = 1;
>  
> + if (s->addr < kernel_start_addr)
> + return 0;
> +
>   /* skip prefix char */
>   if (symbol_prefix_char && *(s->sym + 1) == symbol_prefix_char)
>   offset++;
> @@ -646,6 +653,9 @@ int main(int argc, char **argv)
>   if ((*p == '"' && *(p+2) == '"') || (*p == '\'' 
> && *(p+2) == '\''))
>   p++;
>   symbol_prefix_char = *p;
> + } else if (strncmp(argv[i], "--page-offset=", 14) == 0) 
> {
> + const char *p = &argv[i][14];
> + kernel_start_addr = strtoull(p, NULL, 16);
>   } else
>   usage();
>   }
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index 0149949..32b10f5 100644
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -82,6 +82,8 @@ kallsyms()
>   kallsymopt="${kallsymopt} --all-symbols"
>   fi
>  
> + kallsymopt="${kallsymopt} --page-offset=$CONFIG_PAGE_OFFSET"
> +
>   local aflags="${KBUILD_AFLAGS} ${KBUILD_AFLAGS_KERNEL}   \
> ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS}"
>  
> -- 
> 1.7.9.5
>
>
>
>
> Thanks,
> -- 
> Ming Lei
--
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 2/2] virtio_blk: blk-mq support

2013-10-27 Thread Rusty Russell
Christoph Hellwig  writes:
> Switch virtio-blk from the dual support for old-style requests and bios
> to use the block-multiqueue.  For now pretend to have 4 issue queues
> as Jens pulled that out of his this hair and it worked.

Let's pretend I'm stupid.

We don't actually have multiple queues through to the host, but we're
pretending to, because it makes the block layer go faster?

Do I want to know *why* it's faster?  Or should I look the other way?

Thanks,
Rusty.

> Signed-off-by: Jens Axboe 
> Signed-off-by: Christoph Hellwig 
> ---
>  drivers/block/virtio_blk.c |  312 
> +---
>  1 file changed, 65 insertions(+), 247 deletions(-)
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index 5cdf88b..87f099d 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -11,12 +11,11 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  
>  #define PART_BITS 4
>  
> -static bool use_bio;
> -module_param(use_bio, bool, S_IRUGO);
> -
>  static int major;
>  static DEFINE_IDA(vd_index_ida);
>  
> @@ -26,13 +25,11 @@ struct virtio_blk
>  {
>   struct virtio_device *vdev;
>   struct virtqueue *vq;
> - wait_queue_head_t queue_wait;
> + spinlock_t vq_lock;
>  
>   /* The disk structure for the kernel. */
>   struct gendisk *disk;
>  
> - mempool_t *pool;
> -
>   /* Process context for config space updates */
>   struct work_struct config_work;
>  
> @@ -47,15 +44,11 @@ struct virtio_blk
>  
>   /* Ida index - used to track minor number allocations. */
>   int index;
> -
> - /* Scatterlist: can be too big for stack. */
> - struct scatterlist sg[/*sg_elems*/];
>  };
>  
>  struct virtblk_req
>  {
>   struct request *req;
> - struct bio *bio;
>   struct virtio_blk_outhdr out_hdr;
>   struct virtio_scsi_inhdr in_hdr;
>   struct work_struct work;
> @@ -84,22 +77,6 @@ static inline int virtblk_result(struct virtblk_req *vbr)
>   }
>  }
>  
> -static inline struct virtblk_req *virtblk_alloc_req(struct virtio_blk *vblk,
> - gfp_t gfp_mask)
> -{
> - struct virtblk_req *vbr;
> -
> - vbr = mempool_alloc(vblk->pool, gfp_mask);
> - if (!vbr)
> - return NULL;
> -
> - vbr->vblk = vblk;
> - if (use_bio)
> - sg_init_table(vbr->sg, vblk->sg_elems);
> -
> - return vbr;
> -}
> -
>  static int __virtblk_add_req(struct virtqueue *vq,
>struct virtblk_req *vbr,
>struct scatterlist *data_sg,
> @@ -143,83 +120,8 @@ static int __virtblk_add_req(struct virtqueue *vq,
>   return virtqueue_add_sgs(vq, sgs, num_out, num_in, vbr, GFP_ATOMIC);
>  }
>  
> -static void virtblk_add_req(struct virtblk_req *vbr, bool have_data)
> -{
> - struct virtio_blk *vblk = vbr->vblk;
> - DEFINE_WAIT(wait);
> - int ret;
> -
> - spin_lock_irq(vblk->disk->queue->queue_lock);
> - while (unlikely((ret = __virtblk_add_req(vblk->vq, vbr, vbr->sg,
> -  have_data)) < 0)) {
> - prepare_to_wait_exclusive(&vblk->queue_wait, &wait,
> -   TASK_UNINTERRUPTIBLE);
> -
> - spin_unlock_irq(vblk->disk->queue->queue_lock);
> - io_schedule();
> - spin_lock_irq(vblk->disk->queue->queue_lock);
> -
> - finish_wait(&vblk->queue_wait, &wait);
> - }
> -
> - virtqueue_kick(vblk->vq);
> - spin_unlock_irq(vblk->disk->queue->queue_lock);
> -}
> -
> -static void virtblk_bio_send_flush(struct virtblk_req *vbr)
> -{
> - vbr->flags |= VBLK_IS_FLUSH;
> - vbr->out_hdr.type = VIRTIO_BLK_T_FLUSH;
> - vbr->out_hdr.sector = 0;
> - vbr->out_hdr.ioprio = 0;
> -
> - virtblk_add_req(vbr, false);
> -}
> -
> -static void virtblk_bio_send_data(struct virtblk_req *vbr)
> -{
> - struct virtio_blk *vblk = vbr->vblk;
> - struct bio *bio = vbr->bio;
> - bool have_data;
> -
> - vbr->flags &= ~VBLK_IS_FLUSH;
> - vbr->out_hdr.type = 0;
> - vbr->out_hdr.sector = bio->bi_sector;
> - vbr->out_hdr.ioprio = bio_prio(bio);
> -
> - if (blk_bio_map_sg(vblk->disk->queue, bio, vbr->sg)) {
> - have_data = true;
> - if (bio->bi_rw & REQ_WRITE)
> - vbr->out_hdr.type |= VIRTIO_BLK_T_OUT;
> - else
> - vbr->out_hdr.type |= VIRTIO_BLK_T_IN;
> - } else
> - have_data = false;
> -
> - virtblk_add_req(vbr, have_data);
> -}
> -
> -static void virtblk_bio_send_data_work(struct work_struct *work)
> -{
> - struct virtblk_req *vbr;
> -
> - vbr = container_of(work, struct virtblk_req, work);
> -
> - virtblk_bio_send_data(vbr);
> -}
> -
> -static void virtblk_bio_send_flush_work(struct work_struct *work)
> -{
> - struct virtblk_req *vbr;
> -
> - vbr = container_of(work, struct virtblk

Re: [PATCH] lglock: Map to spinlock when !CONFIG_SMP

2013-10-27 Thread Rusty Russell
Josh Triplett  writes:
> When the system has only one CPU, lglock is effectively a spinlock; map
> it directly to spinlock to eliminate the indirection and duplicate code.
>
> In addition to removing overhead, this drops 1.6k of code with a defconfig
> modified to have !CONFIG_SMP, and 1.1k with a minimal config.
>
> Signed-off-by: Josh Triplett 

Nice.  Looks like a patch for Andrew Morton though.

Cheers,
Rusty.

> ---
>  include/linux/lglock.h | 16 
>  kernel/Makefile|  4 ++--
>  2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/lglock.h b/include/linux/lglock.h
> index 0d24e93..6561b1c 100644
> --- a/include/linux/lglock.h
> +++ b/include/linux/lglock.h
> @@ -35,6 +35,8 @@
>  #define DEFINE_BRLOCK(name)  DEFINE_LGLOCK(name)
>  #define DEFINE_STATIC_BRLOCK(name)   DEFINE_STATIC_LGLOCK(name)
>  
> +#ifdef CONFIG_SMP
> +
>  #ifdef CONFIG_DEBUG_LOCK_ALLOC
>  #define LOCKDEP_INIT_MAP lockdep_init_map
>  #else
> @@ -67,4 +69,18 @@ void lg_local_unlock_cpu(struct lglock *lg, int cpu);
>  void lg_global_lock(struct lglock *lg);
>  void lg_global_unlock(struct lglock *lg);
>  
> +#else
> +/* When !CONFIG_SMP, map lglock to spinlock */
> +#define lglock spinlock
> +#define DEFINE_LGLOCK(name) DEFINE_SPINLOCK(name)
> +#define DEFINE_STATIC_LGLOCK(name) static DEFINE_SPINLOCK(name)
> +#define lg_lock_init(lg, name) spin_lock_init(lg)
> +#define lg_local_lock spin_lock
> +#define lg_local_unlock spin_unlock
> +#define lg_local_lock_cpu(lg, cpu) spin_lock(lg)
> +#define lg_local_unlock_cpu(lg, cpu) spin_unlock(lg)
> +#define lg_global_lock spin_lock
> +#define lg_global_unlock spin_unlock
> +#endif
> +
>  #endif
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 1ce4755..84a89f7 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -10,7 +10,7 @@ obj-y = fork.o exec_domain.o panic.o \
>   kthread.o wait.o sys_ni.o posix-cpu-timers.o mutex.o \
>   hrtimer.o rwsem.o nsproxy.o srcu.o semaphore.o \
>   notifier.o ksysfs.o cred.o reboot.o \
> - async.o range.o groups.o lglock.o smpboot.o
> + async.o range.o groups.o smpboot.o
>  
>  ifdef CONFIG_FUNCTION_TRACER
>  # Do not trace debug files and internal ftrace files
> @@ -50,7 +50,7 @@ obj-$(CONFIG_SMP) += smp.o
>  ifneq ($(CONFIG_SMP),y)
>  obj-y += up.o
>  endif
> -obj-$(CONFIG_SMP) += spinlock.o
> +obj-$(CONFIG_SMP) += lglock.o spinlock.o
>  obj-$(CONFIG_DEBUG_SPINLOCK) += spinlock.o
>  obj-$(CONFIG_PROVE_LOCKING) += spinlock.o
>  obj-$(CONFIG_UID16) += uid16.o
> -- 
> 1.8.4.rc3
--
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] kernel/kallsyms.c: only show legal kernel symbol

2013-10-27 Thread Rusty Russell
Ming Lei  writes:
> On Fri, Oct 25, 2013 at 7:58 PM, Rusty Russell  wrote:
>>>
>>> Basically these symbols are only used to generate code, and in
>>> kernel mode, CPU won't run into the corresponding addresses
>>> because the generate code is copied to other address during booting,
>>> so I understand they won't appear in backtraces.
>>
>> An oops occurs when something went *wrong*.  We look up all kinds of
>> stuff.  Are you so sure that *none* of the callers will ever see these
>> strange symbols and produce a confusing result?
>
> Suppose that might happen, kernel should be smart enough to know
> that the address is not inside kernel address space and won't produce
> confusing result, right?

I don't know...  It would be your job, as the person making the change,
to find all the users of kallsyms and prove that.

This is why it is easier not to include incorrect values in the kernel's
kallsyms in the first place.

Hope that helps,
Rusty.
--
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] kernel/kallsyms.c: only show legal kernel symbol

2013-10-25 Thread Rusty Russell
Ming Lei  writes:
> On Fri, Oct 25, 2013 at 1:50 PM, Rusty Russell  wrote:
>> Ming Lei  writes:
>>> On Fri, Oct 25, 2013 at 7:08 AM, Rusty Russell  
>>> wrote:
>>>>
>>>> Sorry, I was imprecise.  I was referring to the kernel's kallsyms
>>>> tables produced by scripts/kallsyms.c.  This patch left them in the
>>>> the kallsyms tables and filtered them out from /proc/kallsyms.
>>>
>>> Yes, but it isn't easy to do it by script/kallsyms.c , and IMO, it should
>>> be correct to hide them for user space but keep them in kallsyms table.
>>
>> So they'll appear in backtraces?  And turn up randomly for other symbol
>> dereferences?
>>
>> I don't think you really want this!
>
> Basically these symbols are only used to generate code, and in
> kernel mode, CPU won't run into the corresponding addresses
> because the generate code is copied to other address during booting,
> so I understand they won't appear in backtraces.

An oops occurs when something went *wrong*.  We look up all kinds of
stuff.  Are you so sure that *none* of the callers will ever see these
strange symbols and produce a confusing result?

Cheers,
Rusty.



--
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] kernel/kallsyms.c: only show legal kernel symbol

2013-10-24 Thread Rusty Russell
Ming Lei  writes:
> On Fri, Oct 25, 2013 at 7:08 AM, Rusty Russell  wrote:
>>
>> Sorry, I was imprecise.  I was referring to the kernel's kallsyms
>> tables produced by scripts/kallsyms.c.  This patch left them in the
>> the kallsyms tables and filtered them out from /proc/kallsyms.
>
> Yes, but it isn't easy to do it by script/kallsyms.c , and IMO, it should
> be correct to hide them for user space but keep them in kallsyms table.

So they'll appear in backtraces?  And turn up randomly for other symbol
dereferences?

I don't think you really want this!

>> It's weird that cpu_v7_suspend_size appeared above, since kallsyms
>> should filter out 'A' symbols already.
>
> Sorry, the 'A' symbol is a mistake, but the others do exist in /proc/kallsyms.

Ah, OK.

Cheers,
Rusty.
--
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] kernel/kallsyms.c: only show legal kernel symbol

2013-10-24 Thread Rusty Russell
Russell King - ARM Linux  writes:
> On Thu, Oct 24, 2013 at 11:51:18AM +1030, Rusty Russell wrote:
>> Ming Lei  writes:
>> > Address of non-module kernel symbol should always be located
>> > from CONFIG_PAGE_OFFSET on, so only show these legal kernel
>> > symbols in /proc/kallsyms.
>> >
>> > On ARM, some symbols(see below) may drop in relocatable code, so
>> > perf can't parse kernel symbols any more from /proc/kallsyms, this
>> > patch fixes the problem.
>> >
>> > t __vectors_start
>> >0020 A cpu_v7_suspend_size
>> >1000 t __stubs_start
>> >1004 t vector_rst
>> >1020 t vector_irq
>> >10a0 t vector_dabt
>> >1120 t vector_pabt
>> >11a0 t vector_und
>> >1220 t vector_addrexcptn
>> >1224 t vector_fiq
>> >1224 T vector_fiq_offset
>> >
>> > The issue can be fixed in scripts/kallsyms.c too, but looks this
>> > approach is easier.
>> 
>> This fix looks hacky; if these symbols are not available, don't just
>> remove them from /proc/kallsyms, but don't put them in the kernel at
>> all.
>
> How do you "don't put them in the kernel at all" when they're used by
> the kernel internally as offsets?

Sorry, I was imprecise.  I was referring to the kernel's kallsyms
tables produced by scripts/kallsyms.c.  This patch left them in the
the kallsyms tables and filtered them out from /proc/kallsyms.

It's weird that cpu_v7_suspend_size appeared above, since kallsyms
should filter out 'A' symbols already.

> If you mean, just get rid of them, shall I just add these as magic
> numbers instead based on the values in this email?  Is that really a
> sane solution?
>
> No, we have to keep these symbols IMHO.

Can you make them absolute symbols?  That should Just Work for kallsyms.

Cheers,
Rusty.
--
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] kernel/kallsyms.c: only show legal kernel symbol

2013-10-23 Thread Rusty Russell
Ming Lei  writes:
> Address of non-module kernel symbol should always be located
> from CONFIG_PAGE_OFFSET on, so only show these legal kernel
> symbols in /proc/kallsyms.
>
> On ARM, some symbols(see below) may drop in relocatable code, so
> perf can't parse kernel symbols any more from /proc/kallsyms, this
> patch fixes the problem.
>
>    t __vectors_start
>   0020 A cpu_v7_suspend_size
>   1000 t __stubs_start
>   1004 t vector_rst
>   1020 t vector_irq
>   10a0 t vector_dabt
>   1120 t vector_pabt
>   11a0 t vector_und
>   1220 t vector_addrexcptn
>   1224 t vector_fiq
>   1224 T vector_fiq_offset
>
> The issue can be fixed in scripts/kallsyms.c too, but looks this
> approach is easier.

This fix looks hacky; if these symbols are not available, don't just
remove them from /proc/kallsyms, but don't put them in the kernel at
all.

That way, they won't interfere with other kallsyms uses (eg. backtrace).

Or, better, figure out a smart way of excluding them by knowing why
these symbol addresses are wrong.

Thanks,
Rusty.

> Cc: Russell King 
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: Rusty Russell 
> Cc: Chen Gang 
> Signed-off-by: Ming Lei 
> ---
>  kernel/kallsyms.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> index 3127ad5..184f271 100644
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c
> @@ -543,7 +543,7 @@ static int s_show(struct seq_file *m, void *p)
>   tolower(iter->type);
>   seq_printf(m, "%pK %c %s\t[%s]\n", (void *)iter->value,
>  type, iter->name, iter->module_name);
> - } else
> + } else if (iter->value >= CONFIG_PAGE_OFFSET)
>   seq_printf(m, "%pK %c %s\n", (void *)iter->value,
>  iter->type, iter->name);
>   return 0;
> -- 
> 1.7.9.5
--
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 1/7] x86, asmlinkage, lguest: Pass in globals into assembler statement

2013-10-22 Thread Rusty Russell
Andi Kleen  writes:
> From: Andi Kleen 
>
> Tell the compiler that the inline assembler statement
> references lguest_entry.
>
> This fixes compile problems with LTO where the variable
> and the assembler code may end up in different files.
>
> Cc: x...@kernel.org
> Cc: ru...@rustcorp.com.au
> Signed-off-by: Andi Kleen 

Great, thanks.  Might as well keep this with the others:

Acked-by: Rusty Russell 
Tested-by: Rusty Russell 

Cheers,
Rusty.

> ---
>  drivers/lguest/x86/core.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/lguest/x86/core.c b/drivers/lguest/x86/core.c
> index 5169239..922a1ac 100644
> --- a/drivers/lguest/x86/core.c
> +++ b/drivers/lguest/x86/core.c
> @@ -157,7 +157,7 @@ static void run_guest_once(struct lg_cpu *cpu, struct 
> lguest_pages *pages)
>* stack, then the address of this call.  This stack layout happens to
>* exactly match the stack layout created by an interrupt...
>*/
> - asm volatile("pushf; lcall *lguest_entry"
> + asm volatile("pushf; lcall *%4"
>/*
> * This is how we tell GCC that %eax ("a") and %ebx ("b")
> * are changed by this routine.  The "=" means output.
> @@ -169,7 +169,9 @@ static void run_guest_once(struct lg_cpu *cpu, struct 
> lguest_pages *pages)
> * physical address of the Guest's top-level page
> * directory.
> */
> -  : "0"(pages), 
> "1"(__pa(cpu->lg->pgdirs[cpu->cpu_pgd].pgdir))
> +  : "0"(pages), 
> +"1"(__pa(cpu->lg->pgdirs[cpu->cpu_pgd].pgdir)),
> +"m"(lguest_entry)
>/*
> * We tell gcc that all these registers could change,
> * which means we don't have to save and restore them in
> -- 
> 1.8.3.1
--
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 2/7] x86, asmlinkage, lguest: Fix C functions used by inline assembler

2013-10-22 Thread Rusty Russell
Andi Kleen  writes:
> From: Andi Kleen 
>
> - Make the C code used by the paravirt stubs visible
> - Since they have to be global now, give them a more unique
> name.
>
> Cc: ru...@rustcorp.com.au
> Cc: x...@kernel.org
> Signed-off-by: Andi Kleen 

Acked-by: Rusty Russell 

Cheers,
Rusty.

> ---
>  arch/x86/lguest/boot.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
> index bdf8532..ad1fb5f 100644
> --- a/arch/x86/lguest/boot.c
> +++ b/arch/x86/lguest/boot.c
> @@ -233,13 +233,13 @@ static void lguest_end_context_switch(struct 
> task_struct *next)
>   * flags word contains all kind of stuff, but in practice Linux only cares
>   * about the interrupt flag.  Our "save_flags()" just returns that.
>   */
> -static unsigned long save_fl(void)
> +asmlinkage unsigned long lguest_save_fl(void)
>  {
>   return lguest_data.irq_enabled;
>  }
>  
>  /* Interrupts go off... */
> -static void irq_disable(void)
> +asmlinkage void lguest_irq_disable(void)
>  {
>   lguest_data.irq_enabled = 0;
>  }
> @@ -253,8 +253,8 @@ static void irq_disable(void)
>   * PV_CALLEE_SAVE_REGS_THUNK(), which pushes %eax onto the stack, calls the
>   * C function, then restores it.
>   */
> -PV_CALLEE_SAVE_REGS_THUNK(save_fl);
> -PV_CALLEE_SAVE_REGS_THUNK(irq_disable);
> +PV_CALLEE_SAVE_REGS_THUNK(lguest_save_fl);
> +PV_CALLEE_SAVE_REGS_THUNK(lguest_irq_disable);
>  /*:*/
>  
>  /* These are in i386_head.S */
> @@ -1291,9 +1291,9 @@ __init void lguest_init(void)
>*/
>  
>   /* Interrupt-related operations */
> - pv_irq_ops.save_fl = PV_CALLEE_SAVE(save_fl);
> + pv_irq_ops.save_fl = PV_CALLEE_SAVE(lguest_save_fl);
>   pv_irq_ops.restore_fl = __PV_IS_CALLEE_SAVE(lg_restore_fl);
> - pv_irq_ops.irq_disable = PV_CALLEE_SAVE(irq_disable);
> + pv_irq_ops.irq_disable = PV_CALLEE_SAVE(lguest_irq_disable);
>   pv_irq_ops.irq_enable = __PV_IS_CALLEE_SAVE(lg_irq_enable);
>   pv_irq_ops.safe_halt = lguest_safe_halt;
>  
> -- 
> 1.8.3.1
--
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 07/11] asmlinkage, module: Make ksymtab and kcrctab symbols and __this_module __visible

2013-10-22 Thread Rusty Russell
Andi Kleen  writes:
> From: Andi Kleen 
>
> Make the ksymtab symbols for EXPORT_SYMBOL visible.
> This prevents the LTO compiler from adding a .NUMBER prefix,
> which avoids various problems in later export processing.

Applied, thanks.

Cheers,
Rusty.

> Cc: ru...@rustcorp.com.au
> Signed-off-by: Andi Kleen 
> ---
>  include/linux/export.h | 4 ++--
>  scripts/mod/modpost.c  | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/export.h b/include/linux/export.h
> index 412cd50..3f2793d 100644
> --- a/include/linux/export.h
> +++ b/include/linux/export.h
> @@ -43,7 +43,7 @@ extern struct module __this_module;
>  /* Mark the CRC weak since genksyms apparently decides not to
>   * generate a checksums for some symbols */
>  #define __CRC_SYMBOL(sym, sec)   \
> - extern void *__crc_##sym __attribute__((weak)); \
> + extern __visible void *__crc_##sym __attribute__((weak));   
> \
>   static const unsigned long __kcrctab_##sym  \
>   __used  \
>   __attribute__((section("___kcrctab" sec "+" #sym), unused)) \
> @@ -59,7 +59,7 @@ extern struct module __this_module;
>   static const char __kstrtab_##sym[] \
>   __attribute__((section("__ksymtab_strings"), aligned(1))) \
>   = VMLINUX_SYMBOL_STR(sym);  \
> - static const struct kernel_symbol __ksymtab_##sym   \
> + __visible const struct kernel_symbol __ksymtab_##sym\
>   __used  \
>   __attribute__((section("___ksymtab" sec "+" #sym), unused)) \
>   = { (unsigned long)&sym, __kstrtab_##sym }
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 8247979..0971bac 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1853,7 +1853,7 @@ static void add_header(struct buffer *b, struct module 
> *mod)
>   buf_printf(b, "\n");
>   buf_printf(b, "MODULE_INFO(vermagic, VERMAGIC_STRING);\n");
>   buf_printf(b, "\n");
> - buf_printf(b, "struct module __this_module\n");
> + buf_printf(b, "__visible struct module __this_module\n");
>   buf_printf(b, "__attribute__((section(\".gnu.linkonce.this_module\"))) 
> = {\n");
>   buf_printf(b, "\t.name = KBUILD_MODNAME,\n");
>   if (mod->has_init)
> -- 
> 1.8.3.1
--
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] init: fix in-place parameter modification regression

2013-10-20 Thread Rusty Russell
Krzysztof Mazur  writes:
> On Fri, Oct 18, 2013 at 02:20:38PM +1030, Rusty Russell wrote:
>> Back when there was almost no parameter parsing support, everyone got
>> used to keeping pointers into the original.  Making everyone kstrdup()
>> seems like gratuitous churn which is likely to make more bugs.
>> 
>> Your fix means __setup() gets treated specially, in that only it can
>> mangle the command line.  That makes sense.  But it introduces another
>> regression: normal parsing functions can't keep pointers, since that's
>> now __initdata.
>> 
>> There are two possible solutions:
>> (1) Audit all __setup to make sure they copy if they want to mangle.
>> There are about 750 of them, but many are trivial.
>> (2) alloc_bootmem() a third commandline for parsing.
>> 
>> Now, many functions of form __setup("XXX=") should be turned into
>> module_param anyway.
>> 
>> I suggest we do (2) for the moment, and start sweeping through cleaning
>> up __setup() in the longer term.
>> 
>
> Yes, the buffer cannot be __initdata. I'm sending an updated patch.
>
>
> However, keeping pointers to buffer, that will be reinitialized
> in next initcall parameters parsing pass, might cause some race
> conditions.

Thanks, applied.

Cheers,
Rusty.

> Thanks,
> Krzysiek
>
> -- >8 --
> Subject: [PATCH v2] init: fix in-place parameter modification regression
>
> Before commit 026cee0086fe1df4cf74691cf273062cc769617d
> ("params: _initcall-like kernel parameters") the __setup
> parameter parsing code could modify parameter in the
> static_command_line buffer and such modifications were kept. After
> that commit such modifications are destroyed during per-initcall level
> parameter parsing because the same static_command_line buffer is used
> and only parameters for appropriate initcall level are parsed.
>
> That change broke at least parsing "ubd" parameter in the ubd driver
> when the COW file is used.
>
> Now the separate buffer is used for per-initcall parameter parsing.
>
> Signed-off-by: Krzysztof Mazur 
> ---
>  init/main.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/init/main.c b/init/main.c
> index 63d3e8f..c093b5c 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -132,6 +132,8 @@ char __initdata boot_command_line[COMMAND_LINE_SIZE];
>  char *saved_command_line;
>  /* Command line for parameter parsing */
>  static char *static_command_line;
> +/* Command line for per-initcall parameter parsing */
> +static char *initcall_command_line;
>  
>  static char *execute_command;
>  static char *ramdisk_execute_command;
> @@ -348,6 +350,7 @@ static inline void smp_prepare_cpus(unsigned int maxcpus) 
> { }
>  static void __init setup_command_line(char *command_line)
>  {
>   saved_command_line = alloc_bootmem(strlen (boot_command_line)+1);
> + initcall_command_line = alloc_bootmem(strlen (boot_command_line)+1);
>   static_command_line = alloc_bootmem(strlen (command_line)+1);
>   strcpy (saved_command_line, boot_command_line);
>   strcpy (static_command_line, command_line);
> @@ -745,9 +748,9 @@ static void __init do_initcall_level(int level)
>   extern const struct kernel_param __start___param[], __stop___param[];
>   initcall_t *fn;
>  
> - strcpy(static_command_line, saved_command_line);
> + strcpy(initcall_command_line, saved_command_line);
>   parse_args(initcall_level_names[level],
> -static_command_line, __start___param,
> +initcall_command_line, __start___param,
>  __stop___param - __start___param,
>  level, level,
>  &repair_env_string);
> -- 
> 1.8.4.1.635.g6a5
--
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: module: Documentation/module-signing.txt?

2013-10-20 Thread Rusty Russell
Paul Bolle  writes:
> Rusty,
>
> Since v3.7 there's a link to Documentation/module-signing.txt in
> init/Kconfig. It was added by your commit 106a4ee258d1 ("module:
> signature checking hook"). It seems that this file was never added to
> the tree. Is it perhaps queued somewhere?

Well spotted.  David?

Thanks,
Rusty.
--
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 net V2 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready

2013-10-17 Thread Rusty Russell
David Miller  writes:
> From: Rusty Russell 
> Date: Fri, 18 Oct 2013 11:30:15 +1030
>
>> Asking people to express 'CC: stable' in words is error-prone; if Dave
>> wants to filter it, he's quite capable.
>
> Filtering it one time is one thing.
>
> Potentially acting on that filter 100 or so times a day...
>
> That's completely another.

I don't see the difference between reacting to:

> The patch were need for 3.10 and above.

And:

CC: sta...@kernel.org # 3.10+

Except the latter is the standard form which everyone else uses.

Do you want awk script to turn the latter into the former?  Would that
really help?

Confused,
Rusty.
--
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] init: fix in-place parameter modification regression

2013-10-17 Thread Rusty Russell
Krzysztof Mazur  writes:
> On Mon, Oct 14, 2013 at 12:34:02PM +0100, Pawel Moll wrote:
>> So, assuming that it is actually legal to modify static_command_line in
>> __setup()-s (and I must say I have rather mixed feelings about it ;-),
>
> I also have mixed feelings about that, but the parse_args() already
> does that, because some characters are replaced with '\0' to split
> command line into separate strings. The ubd driver does the same
> to split parameter into two strings.

Back when there was almost no parameter parsing support, everyone got
used to keeping pointers into the original.  Making everyone kstrdup()
seems like gratuitous churn which is likely to make more bugs.

Your fix means __setup() gets treated specially, in that only it can
mangle the command line.  That makes sense.  But it introduces another
regression: normal parsing functions can't keep pointers, since that's
now __initdata.

There are two possible solutions:
(1) Audit all __setup to make sure they copy if they want to mangle.
There are about 750 of them, but many are trivial.
(2) alloc_bootmem() a third commandline for parsing.

Now, many functions of form __setup("XXX=") should be turned into
module_param anyway.

I suggest we do (2) for the moment, and start sweeping through cleaning
up __setup() in the longer term.

Cheers,
Rusty.
--
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 net V2 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready

2013-10-17 Thread Rusty Russell
"Michael S. Tsirkin"  writes:
> On Thu, Oct 17, 2013 at 09:57:41AM +1030, Rusty Russell wrote:
>> Jason Wang  writes:
>> > We're trying to re-configure the affinity unconditionally in cpu hotplug
>> > callback. This may lead the issue during resuming from s3/s4 since
>> >
>> > - virt queues haven't been allocated at that time.
>> > - it's unnecessary since thaw method will re-configure the affinity.
>> >
>> > Fix this issue by checking the config_enable and do nothing is we're not 
>> > ready.
>> >
>> > The bug were introduced by commit 8de4b2f3ae90c8fc0f17eeaab87d5a951b66ee17
>> > (virtio-net: reset virtqueue affinity when doing cpu hotplug).
>> >
>> > Cc: Rusty Russell 
>> > Cc: Michael S. Tsirkin 
>> > Cc: Wanlong Gao 
>> > Acked-by: Michael S. Tsirkin 
>> > Reviewed-by: Wanlong Gao 
>> > Signed-off-by: Jason Wang 
>> > ---
>> > The patch is need for 3.8 and above.
>> 
>> Please put 'CC: sta...@kernel.org # 3.8+' in the commit.
>
> Not if this is going in through the net tree.

WTF?  Wow, there really *is* an FAQ: https://lwn.net/Articles/561669/

DaveM is the best maintainer I've ever known, but I abhor the idea that
every subsystem has its own incompatible variant on workflow and style.

Asking people to express 'CC: stable' in words is error-prone; if Dave
wants to filter it, he's quite capable.

Rusty.
--
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 net V2 1/2] virtio-net: don't respond to cpu hotplug notifier if we're not ready

2013-10-16 Thread Rusty Russell
Jason Wang  writes:
> We're trying to re-configure the affinity unconditionally in cpu hotplug
> callback. This may lead the issue during resuming from s3/s4 since
>
> - virt queues haven't been allocated at that time.
> - it's unnecessary since thaw method will re-configure the affinity.
>
> Fix this issue by checking the config_enable and do nothing is we're not 
> ready.
>
> The bug were introduced by commit 8de4b2f3ae90c8fc0f17eeaab87d5a951b66ee17
> (virtio-net: reset virtqueue affinity when doing cpu hotplug).
>
> Cc: Rusty Russell 
> Cc: Michael S. Tsirkin 
> Cc: Wanlong Gao 
> Acked-by: Michael S. Tsirkin 
> Reviewed-by: Wanlong Gao 
> Signed-off-by: Jason Wang 
> ---
> The patch is need for 3.8 and above.

Please put 'CC: sta...@kernel.org # 3.8+' in the commit.

(The specification of the stable line is poor, but that seems to be one
common method).

Cheers,
Rusty.
--
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/


<    2   3   4   5   6   7   8   9   10   11   >