Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751294AbbGaERH (ORCPT ); Fri, 31 Jul 2015 00:17:07 -0400 Received: from mail-pd0-f179.google.com ([209.85.192.179]:34460 "EHLO mail-pd0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750777AbbGaERG convert rfc822-to-8bit (ORCPT ); Fri, 31 Jul 2015 00:17:06 -0400 Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 8.2 \(2098\)) Subject: Re: [PATCH v2] kthread: Export kthread functions From: yalin wang In-Reply-To: <20150730120223.GA27430@hmsreliant.think-freely.org> Date: Fri, 31 Jul 2015 12:16:58 +0800 Cc: Thomas Gleixner , Andrew Morton , David Kershner , tj@kernel.org, laijs@cn.fujitsu.com, nacc@linux.vnet.ibm.com, mingo@redhat.com, open list , jes.sorensen@redhat.com, sparmaintainer@unisys.com Content-Transfer-Encoding: 8BIT Message-Id: References: <1437777920-31156-1-git-send-email-david.kershner@unisys.com> <1438099141-8614-1-git-send-email-david.kershner@unisys.com> <20150728142748.a756d7540ad5cdaf4c9efc9e@linux-foundation.org> <20150730120223.GA27430@hmsreliant.think-freely.org> To: Neil Horman X-Mailer: Apple Mail (2.2098) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 10991 Lines: 311 > On Jul 30, 2015, at 20:02, Neil Horman wrote: > > On Thu, Jul 30, 2015 at 11:48:17AM +0800, yalin wang wrote: >> >>> On Jul 29, 2015, at 18:34, Thomas Gleixner wrote: >>> >>> On Tue, 28 Jul 2015, Andrew Morton wrote: >>> >>>> On Tue, 28 Jul 2015 11:59:01 -0400 David Kershner wrote: >>>> >>>>> The s-Par visornic driver, currently in staging, processes a queue >>>>> being serviced by the an s-Par service partition. We can get a message >>>>> that something has happened with the Service Partition, when that >>>>> happens, we must not access the channel until we get a message that the >>>>> service partition is back again. >>>>> >>>>> The visornic driver has a thread for processing the channel, when we >>>>> get the message, we need to be able to park the thread and then resume >>>>> it when the problem clears. >>>>> >>>>> We can do this with kthread_park and unpark but they are not exported >>>>> from the kernel, this patch exports the needed functions. >>>>> >>>>> Signed-off-by: David Kershner >>>> >>>> Please accumulate the acked-by's and reviewed-by's in the changelog as >>>> they are received. I presently have >>>> >>>> Acked-by: Ingo Molnar >>>> Acked-by: Neil Horman >>>> Cc: Thomas Gleixner >>>> Cc: Richard Weinberger >>>> Cc: Tejun Heo >>>> >>>> >>>> I'll scoot this into mainline probably this week to make life simpler >>>> for the various trees. >>> >> i am curious why not make some tiny functions to be inline ? >> so that don’t need EXPORT_SYMOBLS , shrink the kernel size. >> Thanks > > Because exporting symbols isn't a big deal, and the compiler can decide when its > best to inline these functions. As it is, they aren't that small, if you expand > all their internals > > Neil > this is my test on arm64 arch, i think kthread_should_stop() kthread_should_park() kthread_data() should be inline : without inline: ffffffc0000d265c : ………. ffffffc0000d26a0: b9001a61 str w1, [x19,#24] ffffffc0000d26a4: 97fff260 bl ffffffc0000cf024 ffffffc0000d26a8: 53001c00 uxtb w0, w0 ffffffc0000d26ac: 35000c20 cbnz w0, ffffffc0000d2830 ffffffc0000d26b0: 97fff4aa bl ffffffc0000cf958 ffffffc0000d26b4: 53001c00 uxtb w0, w0 ffffffc0000d26b8: 34000320 cbz w0, ffffffc0000d271c ffffffc0000d26bc: f9400a60 ldr x0, [x19,#16] ffffffc0000d26c0: f900001f str xzr, [x0] ffffffc0000cf958 : ffffffc0000cf958: 910003e0 mov x0, sp ffffffc0000cf95c: 9272c400 and x0, x0, #0xffffffffffffc000 ffffffc0000cf960: f9400800 ldr x0, [x0,#16] ffffffc0000cf964: f9420400 ldr x0, [x0,#1032] ffffffc0000cf968: f85c8000 ldr x0, [x0,#-56] ffffffc0000cf96c: d3420800 ubfx x0, x0, #2, #1 ffffffc0000cf970: d65f03c0 ret if i mark kthread_should_park to be inline : ffffffc0000d19ec : ffffffc0000d19ec: a9bc7bfd stp x29, x30, [sp,#-64]! ffffffc0000d19f0: 910003fd mov x29, sp ffffffc0000d19f4: a90153f3 stp x19, x20, [sp,#16] ffffffc0000d19f8: aa0003f4 mov x20, x0 ffffffc0000d19fc: 910003e0 mov x0, sp ffffffc0000d1a00: a90363f7 stp x23, x24, [sp,#48] ffffffc0000d1a04: a9025bf5 stp x21, x22, [sp,#32] ffffffc0000d1a08: d2800035 mov x21, #0x1 // #1 ffffffc0000d1a0c: 9272c413 and x19, x0, #0xffffffffffffc000 ffffffc0000d1a10: f9400696 ldr x22, [x20,#8] ffffffc0000d1a14: 2a1503f7 mov w23, w21 ffffffc0000d1a18: 52800058 mov w24, #0x2 // #2 ffffffc0000d1a1c: f9400a60 ldr x0, [x19,#16] ffffffc0000d1a20: f9000015 str x21, [x0] ffffffc0000d1a24: d5033bbf dmb ish ffffffc0000d1a28: b9401a61 ldr w1, [x19,#24] ffffffc0000d1a2c: 11000421 add w1, w1, #0x1 ffffffc0000d1a30: b9001a61 str w1, [x19,#24] ffffffc0000d1a34: f9400a62 ldr x2, [x19,#16] ffffffc0000d1a38: f9420441 ldr x1, [x2,#1032] ffffffc0000d1a3c: f85c8020 ldr x0, [x1,#-56] ffffffc0000d1a40: 37080a60 tbnz w0, #1, ffffffc0000d1b8c ffffffc0000d1a44: f85c8020 ldr x0, [x1,#-56] // kthread_should_park line ffffffc0000d1a48: 36100300 tbz w0, #2, ffffffc0000d1aa8 // kthread_should_park line ffffffc0000d1a4c: f900005f str xzr, [x2] ffffffc0000d1a50: b9401a60 ldr w0, [x19,#24] it is optimised to 2 instructions , this is my patch, hope can be merged : diff --git a/include/linux/kthread.h b/include/linux/kthread.h diff --git a/include/linux/kthread.h b/include/linux/kthread.h index 3e6773e..9210030 100644 --- a/include/linux/kthread.h +++ b/include/linux/kthread.h @@ -4,6 +4,70 @@ #include #include +struct kthread { + unsigned long flags; + unsigned int cpu; + void *data; + struct completion parked; + struct completion exited; +}; + +enum KTHREAD_BITS { + KTHREAD_IS_PER_CPU = 0, + KTHREAD_SHOULD_STOP, + KTHREAD_SHOULD_PARK, + KTHREAD_IS_PARKED, +}; + +#define __to_kthread(vfork) \ + container_of(vfork, struct kthread, exited) + +static inline struct kthread *to_kthread(struct task_struct *k) +{ + return __to_kthread(k->vfork_done); +} + +/** + * kthread_should_stop - should this kthread return now? + * + * When someone calls kthread_stop() on your kthread, it will be woken + * and this will return true. You should then return, and your return + * value will be passed through to kthread_stop(). + */ +static inline bool kthread_should_stop(void) +{ + return test_bit(KTHREAD_SHOULD_STOP, &to_kthread(current)->flags); +} + +/** + * kthread_should_park - should this kthread park now? + * + * When someone calls kthread_park() on your kthread, it will be woken + * and this will return true. You should then do the necessary + * cleanup and call kthread_parkme() + * + * Similar to kthread_should_stop(), but this keeps the thread alive + * and in a park position. kthread_unpark() "restarts" the thread and + * calls the thread function again. + */ +static inline bool kthread_should_park(void) +{ + return test_bit(KTHREAD_SHOULD_PARK, &to_kthread(current)->flags); +} + +/** + * kthread_data - return data value specified on kthread creation + * @task: kthread task in question + * + * Return the data value specified when kthread @task was created. + * The caller is responsible for ensuring the validity of @task when + * calling this function. + */ +static inline void *kthread_data(struct task_struct *task) +{ + return to_kthread(task)->data; +} + __printf(4, 5) struct task_struct *kthread_create_on_node(int (*threadfn)(void *data), void *data, @@ -39,10 +103,7 @@ struct task_struct *kthread_create_on_cpu(int (*threadfn)(void *data), void kthread_bind(struct task_struct *k, unsigned int cpu); int kthread_stop(struct task_struct *k); -bool kthread_should_stop(void); -bool kthread_should_park(void); bool kthread_freezable_should_stop(bool *was_frozen); -void *kthread_data(struct task_struct *k); void *probe_kthread_data(struct task_struct *k); int kthread_park(struct task_struct *k); void kthread_unpark(struct task_struct *k); diff --git a/kernel/kthread.c b/kernel/kthread.c index baf3673..e036019 100644 --- a/kernel/kthread.c +++ b/kernel/kthread.c @@ -38,29 +38,6 @@ struct kthread_create_info struct list_head list; }; -struct kthread { - unsigned long flags; - unsigned int cpu; - void *data; - struct completion parked; - struct completion exited; -}; - -enum KTHREAD_BITS { - KTHREAD_IS_PER_CPU = 0, - KTHREAD_SHOULD_STOP, - KTHREAD_SHOULD_PARK, - KTHREAD_IS_PARKED, -}; - -#define __to_kthread(vfork) \ - container_of(vfork, struct kthread, exited) - -static inline struct kthread *to_kthread(struct task_struct *k) -{ - return __to_kthread(k->vfork_done); -} - static struct kthread *to_live_kthread(struct task_struct *k) { struct completion *vfork = ACCESS_ONCE(k->vfork_done); @@ -70,36 +47,6 @@ static struct kthread *to_live_kthread(struct task_struct *k) } /** ...skipping... - * When someone calls kthread_stop() on your kthread, it will be woken - * and this will return true. You should then return, and your return - * value will be passed through to kthread_stop(). - */ -bool kthread_should_stop(void) -{ - return test_bit(KTHREAD_SHOULD_STOP, &to_kthread(current)->flags); -} -EXPORT_SYMBOL(kthread_should_stop); - -/** - * kthread_should_park - should this kthread park now? - * - * When someone calls kthread_park() on your kthread, it will be woken - * and this will return true. You should then do the necessary - * cleanup and call kthread_parkme() - * - * Similar to kthread_should_stop(), but this keeps the thread alive - * and in a park position. kthread_unpark() "restarts" the thread and - * calls the thread function again. - */ -bool kthread_should_park(void) -{ - return test_bit(KTHREAD_SHOULD_PARK, &to_kthread(current)->flags); -} -EXPORT_SYMBOL_GPL(kthread_should_park); - -/** * kthread_freezable_should_stop - should this freezable kthread return now? * @was_frozen: optional out parameter, indicates whether %current was frozen * @@ -125,19 +72,6 @@ bool kthread_freezable_should_stop(bool *was_frozen) EXPORT_SYMBOL_GPL(kthread_freezable_should_stop); /** - * kthread_data - return data value specified on kthread creation - * @task: kthread task in question - * - * Return the data value specified when kthread @task was created. - * The caller is responsible for ensuring the validity of @task when - * calling this function. - */ -void *kthread_data(struct task_struct *task) -{ - return to_kthread(task)->data; -} - -/** * probe_kthread_data - speculative version of kthread_data() * @task: possible kthread task in question * --- (END) Thanks -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/