| 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
| ||
|
Message-Id: <FC536F44-E3A4-45E0-9B21-4C1CC256F94C@gmail.com>
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