lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Fri, 31 Jul 2015 12:16:58 +0800
From:	yalin wang <yalin.wang2010@...il.com>
To:	Neil Horman <nhorman@...hat.com>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	David Kershner <david.kershner@...sys.com>, tj@...nel.org,
	laijs@...fujitsu.com, nacc@...ux.vnet.ibm.com, mingo@...hat.com,
	open list <linux-kernel@...r.kernel.org>,
	jes.sorensen@...hat.com, sparmaintainer@...sys.com
Subject: Re: [PATCH v2] kthread: Export kthread functions


> On Jul 30, 2015, at 20:02, Neil Horman <nhorman@...hat.com> wrote:
> 
> On Thu, Jul 30, 2015 at 11:48:17AM +0800, yalin wang wrote:
>> 
>>> On Jul 29, 2015, at 18:34, Thomas Gleixner <tglx@...utronix.de> wrote:
>>> 
>>> On Tue, 28 Jul 2015, Andrew Morton wrote:
>>> 
>>>> On Tue, 28 Jul 2015 11:59:01 -0400 David Kershner <david.kershner@...sys.com> 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 <david.kershner@...sys.com>
>>>> 
>>>> Please accumulate the acked-by's and reviewed-by's in the changelog as
>>>> they are received.   I presently have
>>>> 
>>>> Acked-by: Ingo Molnar <mingo@...nel.org>
>>>> Acked-by: Neil Horman <nhorman@...driver.com>
>>>> Cc: Thomas Gleixner <tglx@...utronix.de>
>>>> Cc: Richard Weinberger <richard.weinberger@...il.com>
>>>> Cc: Tejun Heo <tj@...nel.org>
>>>> 
>>>> 
>>>> 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 <smpboot_thread_fn>:
                                 ……….
ffffffc0000d26a0:       b9001a61        str     w1, [x19,#24]
ffffffc0000d26a4:       97fff260        bl      ffffffc0000cf024 <kthread_should_stop>
ffffffc0000d26a8:       53001c00        uxtb    w0, w0
ffffffc0000d26ac:       35000c20        cbnz    w0, ffffffc0000d2830 <smpboot_thread_fn+0x1d4>
ffffffc0000d26b0:       97fff4aa        bl      ffffffc0000cf958 <kthread_should_park>
ffffffc0000d26b4:       53001c00        uxtb    w0, w0
ffffffc0000d26b8:       34000320        cbz     w0, ffffffc0000d271c <smpboot_thread_fn+0xc0>
ffffffc0000d26bc:       f9400a60        ldr     x0, [x19,#16]
ffffffc0000d26c0:       f900001f        str     xzr, [x0]


ffffffc0000cf958 <kthread_should_park>:
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 <smpboot_thread_fn>:
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 <smpboot_thread_fn+0x1a0>
ffffffc0000d1a44:       f85c8020        ldr     x0, [x1,#-56]    									// kthread_should_park  line
ffffffc0000d1a48:       36100300        tbz     w0, #2, ffffffc0000d1aa8 <smpboot_thread_fn+0xbc>   // 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 <linux/err.h>
 #include <linux/sched.h>
 
+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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ