WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [PATCH RFC 7/8] s390/jump-label: add arch_jump_label_tra

To: Jeremy Fitzhardinge <jeremy@xxxxxxxx>
Subject: [Xen-devel] Re: [PATCH RFC 7/8] s390/jump-label: add arch_jump_label_transform_early()
From: Jan Glauber <jang@xxxxxxxxxxxxxxxxxx>
Date: Sat, 01 Oct 2011 13:22:17 +0200
Cc: Xen Devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, David Daney <david.daney@xxxxxxxxxx>, Jason Baron <jbaron@xxxxxxxxxx>, the arch/x86 maintainers <x86@xxxxxxxxxx>, Linux Kernel Mailing List <linux-kernel@xxxxxxxxxxxxxxx>, Steven Rostedt <rostedt@xxxxxxxxxxx>, Michael Ellerman <michael@xxxxxxxxxxxxxx>, Jeremy Fitzhardinge <jeremy.fitzhardinge@xxxxxxxxxx>, "David S. Miller" <davem@xxxxxxxxxxxxx>
Delivery-date: Tue, 04 Oct 2011 01:30:26 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4E85E867.8080809@xxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <cover.1317338254.git.jeremy.fitzhardinge@xxxxxxxxxx> <b1d87557dc3001031ee8d7f69ee97b90cecba6aa.1317338254.git.jeremy.fitzhardinge@xxxxxxxxxx> <1317394099.11297.54.camel@xxxxxxxxxxxxxxxxxxxxx> <4E85E867.8080809@xxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
On Fri, 2011-09-30 at 09:03 -0700, Jeremy Fitzhardinge wrote:
> On 09/30/2011 07:48 AM, Jan Glauber wrote:
> > On Thu, 2011-09-29 at 16:26 -0700, Jeremy Fitzhardinge wrote:
> >> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@xxxxxxxxxx>
> >>
> >> This allows jump-label entries to be modified early, in a pre-SMP
> >> environment.
> >>
> >> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@xxxxxxxxxx>
> >> Cc: Jan Glauber <jang@xxxxxxxxxxxxxxxxxx>
> > Hi Jeremy,
> >
> > Your patch looks fine, if you can fix the minor compiler warnings
> > below. Excluding stop_machine() on pre-SMP also looks safer too me.
> 
> Do you think there would be an actual problem, or are you just being
> cautious?

Just cautious since stop_machine() is quite a big hammer. Who knows how
many jump labels we might get? So without an early exit in
stop_machine() for pre-SMP we might waste performance there.

> It seems to me - in general - stop_machine could just be defined to be a
> no-op (ie, just directly calls the callback) until enough SMP is
> initialized for it to make sense, rather than having to make every user
> work around it (if there's a chance they might call it early).

Agreed.

> >   CC      arch/s390/kernel/jump_label.o
> > arch/s390/kernel/jump_label.c: In function ‘__jump_label_transform’:
> > arch/s390/kernel/jump_label.c:41:2: error: ‘rc’ undeclared (first use in 
> > this function)
> > arch/s390/kernel/jump_label.c:41:2: note: each undeclared identifier is 
> > reported only once for each function it appears in
> > arch/s390/kernel/jump_label.c:41:2: warning: passing argument 1 of 
> > ‘probe_kernel_write’ makes pointer from integer without a cast [enabled by 
> > default]
> > include/linux/uaccess.h:108:21: note: expected ‘void *’ but argument is of 
> > type ‘jump_label_t’
> > arch/s390/kernel/jump_label.c:28:19: warning: unused variable ‘args’ 
> > [-Wunused-variable]
> > make[2]: *** [arch/s390/kernel/jump_label.o] Error 1
> >
> >
> 
> Like so?

Yes, that compiles!

--Jan

> From 9572689d1e5e6f54a1936a1dca09a6920d1bce27 Mon Sep 17 00:00:00 2001
> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@xxxxxxxxxx>
> Date: Thu, 29 Sep 2011 10:58:46 -0700
> Subject: [PATCH] s390/jump-label: add arch_jump_label_transform_early()
> 
> This allows jump-label entries to be modified early, in a pre-SMP
> environment.
> 
> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@xxxxxxxxxx>
> Cc: Jan Glauber <jang@xxxxxxxxxxxxxxxxxx>
> 
> diff --git a/arch/s390/kernel/jump_label.c b/arch/s390/kernel/jump_label.c
> index 44cc06b..4fbe63b 100644
> --- a/arch/s390/kernel/jump_label.c
> +++ b/arch/s390/kernel/jump_label.c
> @@ -18,26 +18,15 @@ struct insn {
>  } __packed;
> 
>  struct insn_args {
> -     unsigned long *target;
> -     struct insn *insn;
> -     ssize_t size;
> +     struct jump_entry *entry;
> +     enum jump_label_type type;
>  };
> 
> -static int __arch_jump_label_transform(void *data)
> +static void __jump_label_transform(struct jump_entry *entry,
> +                                enum jump_label_type type)
>  {
> -     struct insn_args *args = data;
> -     int rc;
> -
> -     rc = probe_kernel_write(args->target, args->insn, args->size);
> -     WARN_ON_ONCE(rc < 0);
> -     return 0;
> -}
> -
> -void arch_jump_label_transform(struct jump_entry *entry,
> -                            enum jump_label_type type)
> -{
> -     struct insn_args args;
>       struct insn insn;
> +     int rc;
> 
>       if (type == JUMP_LABEL_ENABLE) {
>               /* brcl 15,offset */
> @@ -49,11 +38,33 @@ void arch_jump_label_transform(struct jump_entry *entry,
>               insn.offset = 0;
>       }
> 
> -     args.target = (void *) entry->code;
> -     args.insn = &insn;
> -     args.size = JUMP_LABEL_NOP_SIZE;
> +     rc = probe_kernel_write((void *)entry->code, &insn, 
> JUMP_LABEL_NOP_SIZE);
> +     WARN_ON_ONCE(rc < 0);
> +}
> 
> -     stop_machine(__arch_jump_label_transform, &args, NULL);
> +static int __sm_arch_jump_label_transform(void *data)
> +{
> +     struct insn_args *args = data;
> +
> +     __jump_label_transform(args->entry, args->type);
> +     return 0;
> +}
> +
> +void arch_jump_label_transform(struct jump_entry *entry,
> +                            enum jump_label_type type)
> +{
> +     struct insn_args args;
> +
> +     args.entry = entry;
> +     args.type = type;
> +
> +     stop_machine(__sm_arch_jump_label_transform, &args, NULL);
> +}
> +
> +void __init arch_jump_label_transform_early(struct jump_entry *entry,
> +                                         enum jump_label_type type)
> +{
> +     __jump_label_transform(entry, type);
>  }
> 
>  #endif
> 
>       J
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>