* [PATCH 0/8] Use correctly the Xen memory terminologies in Linux
From: Julien Grall @ 2015-07-28 15:02 UTC (permalink / raw)
To: xen-devel
Cc: ian.campbell, stefano.stabellini, linux-kernel, Julien Grall,
Boris Ostrovsky, David Vrabel, Dmitry Torokhov,
Greg Kroah-Hartman, H. Peter Anvin, Ingo Molnar,
James E.J. Bottomley, Jean-Christophe Plagniol-Villard,
Jiri Slaby, Juergen Gross, Konrad Rzeszutek Wilk, linux-api,
linux-arm-kernel, linux-fbdev, linux-input, linuxppc-dev,
linux-scsi, netdev, Roger Pau Monné
Hi all,
This patch series aims to use the memory terminologies described in
include/linux/mm.h [1] for Linux xen code.
Linux is using mistakenly MFN when GFN is meant, I suspect this is because the
first support of Xen was for PV. This has brought some misimplementation
of memory helpers on ARM and make the developper confused about the expected
behavior.
For instance, with pfn_to_mfn, we expect to get a MFN based on the name.
Although, if we look at the implementation on x86, it's returning a GFN.
Most of the callers are also using it this way.
The first 2 patches of this series is ARM related in order to remove
PV specific helpers which should not be used and fixing the implementation of
pfn_to_mfn.
The rest of the series is here rename most of the usage in the common code
of MFN to GFN. I also took the opportunity to replace most of the call to
pfn_to_gfn in the common code by page_to_gfn avoid construction such
as pfn_to_gfn(page_to_pfn(...).
Note the one xen-blkfront will be dropped by 64K series [2], I can include it
if necessary.
This series is based on Linux 4.2-rc4. A branch with all the patches
can be found here:
git://xenbits.xen.org/people/julieng/linux-arm.git branch page-renaming-v1
Sincerely yours,
[1] Xen tree: e758ed14f390342513405dd766e874934573e6cb
[2] https://lkml.org/lkml/2015/7/9/628
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "James E.J. Bottomley" <JBottomley@odin.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: linux-api@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-fbdev@vger.kernel.org
Cc: linux-input@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-scsi@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: x86@kernel.org
Julien Grall (8):
arm/xen: Remove helpers which are PV specific
xen: Make clear that swiotlb and biomerge are dealing with DMA address
arm/xen: implement correctly pfn_to_mfn
xen: Use the correctly the Xen memory terminologies
xen/tmem: Use page_to_gfn rather than pfn_to_gfn
video/xen-fbfront: Further s/MFN/GFN clean-up
hvc/xen: Further s/MFN/GFN clean-up
xen/privcmd: Further s/MFN/GFN/ clean-up
arch/arm/include/asm/xen/page.h | 44 +++++++++++++++---------------
arch/arm/xen/enlighten.c | 18 ++++++-------
arch/arm/xen/mm.c | 4 +--
arch/x86/include/asm/xen/page.h | 34 +++++++++++++----------
arch/x86/xen/enlighten.c | 4 +--
arch/x86/xen/mmu.c | 48 ++++++++++++++++-----------------
arch/x86/xen/p2m.c | 32 +++++++++++-----------
arch/x86/xen/setup.c | 12 ++++-----
arch/x86/xen/smp.c | 4 +--
arch/x86/xen/suspend.c | 8 +++---
drivers/block/xen-blkfront.c | 6 ++---
drivers/input/misc/xen-kbdfront.c | 4 +--
drivers/net/xen-netback/netback.c | 4 +--
drivers/net/xen-netfront.c | 8 +++---
drivers/scsi/xen-scsifront.c | 8 +++---
drivers/tty/hvc/hvc_xen.c | 18 +++++--------
drivers/video/fbdev/xen-fbfront.c | 20 +++++++-------
drivers/xen/balloon.c | 2 +-
drivers/xen/biomerge.c | 6 ++---
drivers/xen/events/events_base.c | 2 +-
drivers/xen/events/events_fifo.c | 4 +--
drivers/xen/gntalloc.c | 3 ++-
drivers/xen/manage.c | 2 +-
drivers/xen/privcmd.c | 44 +++++++++++++++---------------
drivers/xen/swiotlb-xen.c | 16 +++++------
drivers/xen/tmem.c | 21 +++++----------
drivers/xen/xenbus/xenbus_client.c | 2 +-
drivers/xen/xenbus/xenbus_dev_backend.c | 2 +-
drivers/xen/xenbus/xenbus_probe.c | 8 +++---
drivers/xen/xlate_mmu.c | 18 ++++++-------
include/uapi/xen/privcmd.h | 4 +++
include/xen/page.h | 4 +--
include/xen/xen-ops.h | 10 +++----
33 files changed, 210 insertions(+), 214 deletions(-)
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [RFC PATCH 14/14] kthread_worker: Add set_kthread_worker_scheduler*()
From: Petr Mladek @ 2015-07-28 14:39 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra
Cc: Steven Rostedt, Paul E. McKenney, Josh Triplett, Thomas Gleixner,
Linus Torvalds, Jiri Kosina, Borislav Petkov, Michal Hocko,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Vlastimil Babka,
live-patching-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Petr Mladek
In-Reply-To: <1438094371-8326-1-git-send-email-pmladek-IBi9RG/b67k@public.gmane.org>
The kthread worker API will be used for kthreads that need to modify
the scheduling policy.
This patch adds a function that allows to make it easily, safe way,
and hides implementation details. It might even help to get rid
of an init work.
It uses @sched_priority as a parameter instead of struct sched_param.
The structure has been there already in the initial kernel git commit
(April 2005) and always included only one member: sched_priority.
So, it rather looks like an overkill that is better to hide.
Signed-off-by: Petr Mladek <pmladek-IBi9RG/b67k@public.gmane.org>
---
include/linux/kthread.h | 5 +++
kernel/kthread.c | 59 ++++++++++++++++++++++++++++++++++++
kernel/rcu/tree.c | 10 +++---
kernel/trace/ring_buffer_benchmark.c | 11 +++----
4 files changed, 72 insertions(+), 13 deletions(-)
diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index b75847e1a4c9..d503dc16613c 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -144,6 +144,11 @@ int create_kthread_worker_on_node(struct kthread_worker *worker,
void set_kthread_worker_user_nice(struct kthread_worker *worker, long nice);
+int set_kthread_worker_scheduler(struct kthread_worker *worker,
+ int policy, int sched_priority);
+int set_kthread_worker_scheduler_nocheck(struct kthread_worker *worker,
+ int policy, int sched_priority);
+
bool queue_kthread_work(struct kthread_worker *worker,
struct kthread_work *work);
void flush_kthread_work(struct kthread_work *work);
diff --git a/kernel/kthread.c b/kernel/kthread.c
index ab2e235b6144..4ab31b914676 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -662,6 +662,65 @@ void set_kthread_worker_user_nice(struct kthread_worker *worker, long nice)
}
EXPORT_SYMBOL(set_kthread_worker_user_nice);
+static int
+__set_kthread_worker_scheduler(struct kthread_worker *worker,
+ int policy, int sched_priority, bool check)
+{
+ struct task_struct *task = worker->task;
+ const struct sched_param sp = {
+ .sched_priority = sched_priority
+ };
+ int ret;
+
+ WARN_ON(!task);
+
+ if (check)
+ ret = sched_setscheduler(task, policy, &sp);
+ else
+ ret = sched_setscheduler_nocheck(task, policy, &sp);
+
+ return ret;
+}
+
+/**
+ * set_kthread_worker_scheduler - change the scheduling policy and/or RT
+ * priority of a kthread worker.
+ * @worker: target kthread_worker
+ * @policy: new policy
+ * @sched_priority: new RT priority
+ *
+ * Return: 0 on success. An error code otherwise.
+ */
+int set_kthread_worker_scheduler(struct kthread_worker *worker,
+ int policy, int sched_priority)
+{
+ return __set_kthread_worker_scheduler(worker, policy, sched_priority,
+ true);
+}
+EXPORT_SYMBOL(set_kthread_worker_scheduler);
+
+/**
+ * set_kthread_worker_scheduler_nocheck - change the scheduling policy and/or RT
+ * priority of a kthread worker.
+ * @worker: target kthread_worker
+ * @policy: new policy
+ * @sched_priority: new RT priority
+ *
+ * Just like set_kthread_worker_sheduler(), only don't bother checking
+ * if the current context has permission. For example, this is needed
+ * in stop_machine(): we create temporary high priority worker threads,
+ * but our caller might not have that capability.
+ *
+ * Return: 0 on success. An error code otherwise.
+ */
+int set_kthread_worker_scheduler_nocheck(struct kthread_worker *worker,
+ int policy, int sched_priority)
+{
+ return __set_kthread_worker_scheduler(worker, policy, sched_priority,
+ false);
+}
+EXPORT_SYMBOL(set_kthread_worker_scheduler_nocheck);
+
/* insert @work before @pos in @worker */
static void insert_kthread_work(struct kthread_worker *worker,
struct kthread_work *work,
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 3a286f3b8b3c..d882464c71d7 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -3916,7 +3916,6 @@ static int __init rcu_spawn_gp_kthread(void)
int kthread_prio_in = kthread_prio;
struct rcu_node *rnp;
struct rcu_state *rsp;
- struct sched_param sp;
int ret;
/* Force priority into range. */
@@ -3940,11 +3939,10 @@ static int __init rcu_spawn_gp_kthread(void)
BUG_ON(ret);
rnp = rcu_get_root(rsp);
raw_spin_lock_irqsave(&rnp->lock, flags);
- if (kthread_prio) {
- sp.sched_priority = kthread_prio;
- sched_setscheduler_nocheck(rsp->gp_worker.task,
- SCHED_FIFO, &sp);
- }
+ if (kthread_prio)
+ set_kthread_worker_scheduler_nocheck(&rsp->gp_worker,
+ SCHED_FIFO,
+ kthread_prio);
queue_kthread_work(&rsp->gp_worker, &rsp->gp_init_work);
raw_spin_unlock_irqrestore(&rnp->lock, flags);
}
diff --git a/kernel/trace/ring_buffer_benchmark.c b/kernel/trace/ring_buffer_benchmark.c
index 73e4c7f11a2c..89028165bb22 100644
--- a/kernel/trace/ring_buffer_benchmark.c
+++ b/kernel/trace/ring_buffer_benchmark.c
@@ -469,13 +469,10 @@ static int __init ring_buffer_benchmark_init(void)
set_user_nice(consumer, consumer_nice);
}
- if (producer_fifo >= 0) {
- struct sched_param param = {
- .sched_priority = producer_fifo
- };
- sched_setscheduler(rb_producer_worker.task,
- SCHED_FIFO, ¶m);
- } else
+ if (producer_fifo >= 0)
+ set_kthread_worker_scheduler(&rb_producer_worker,
+ SCHED_FIFO, producer_fifo);
+ else
set_kthread_worker_user_nice(&rb_producer_worker,
producer_nice);
--
1.8.5.6
^ permalink raw reply related
* [RFC PATCH 13/14] kthread_worker: Add set_kthread_worker_user_nice()
From: Petr Mladek @ 2015-07-28 14:39 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra
Cc: Steven Rostedt, Paul E. McKenney, Josh Triplett, Thomas Gleixner,
Linus Torvalds, Jiri Kosina, Borislav Petkov, Michal Hocko,
linux-mm, Vlastimil Babka, live-patching, linux-api, linux-kernel,
Petr Mladek
In-Reply-To: <1438094371-8326-1-git-send-email-pmladek@suse.com>
kthread worker API will be used for kthreads that need to modify
the scheduling priority.
This patch adds a function that allows to make it easily, safe way,
and hides implementation details. It might even help to get rid
of an init work.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/kthread.h | 2 ++
kernel/kthread.c | 14 ++++++++++++++
kernel/trace/ring_buffer_benchmark.c | 3 ++-
mm/huge_memory.c | 10 +---------
4 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index d916b024e986..b75847e1a4c9 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -142,6 +142,8 @@ int create_kthread_worker_on_node(struct kthread_worker *worker,
#define create_kthread_worker(worker, flags, namefmt, arg...) \
create_kthread_worker_on_node(worker, flags, -1, namefmt, ##arg)
+void set_kthread_worker_user_nice(struct kthread_worker *worker, long nice);
+
bool queue_kthread_work(struct kthread_worker *worker,
struct kthread_work *work);
void flush_kthread_work(struct kthread_work *work);
diff --git a/kernel/kthread.c b/kernel/kthread.c
index d02509e17f7e..ab2e235b6144 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -648,6 +648,20 @@ int create_kthread_worker_on_node(struct kthread_worker *worker,
}
EXPORT_SYMBOL(create_kthread_worker_on_node);
+/*
+ * set_kthread_worker_user_nice - set scheduling priority for the kthread worker
+ * @worker: target kthread_worker
+ * @nice: niceness value
+ */
+void set_kthread_worker_user_nice(struct kthread_worker *worker, long nice)
+{
+ struct task_struct *task = worker->task;
+
+ WARN_ON(!task);
+ set_user_nice(task, nice);
+}
+EXPORT_SYMBOL(set_kthread_worker_user_nice);
+
/* insert @work before @pos in @worker */
static void insert_kthread_work(struct kthread_worker *worker,
struct kthread_work *work,
diff --git a/kernel/trace/ring_buffer_benchmark.c b/kernel/trace/ring_buffer_benchmark.c
index 5036d284885c..73e4c7f11a2c 100644
--- a/kernel/trace/ring_buffer_benchmark.c
+++ b/kernel/trace/ring_buffer_benchmark.c
@@ -476,7 +476,8 @@ static int __init ring_buffer_benchmark_init(void)
sched_setscheduler(rb_producer_worker.task,
SCHED_FIFO, ¶m);
} else
- set_user_nice(rb_producer_worker.task, producer_nice);
+ set_kthread_worker_user_nice(&rb_producer_worker,
+ producer_nice);
return 0;
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 51a514161f2b..1d5f990c55ab 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -55,12 +55,10 @@ static unsigned int khugepaged_scan_sleep_millisecs __read_mostly = 10000;
/* during fragmentation poll the hugepage allocator once every minute */
static unsigned int khugepaged_alloc_sleep_millisecs __read_mostly = 60000;
-static void khugepaged_init_func(struct kthread_work *dummy);
static void khugepaged_do_scan_func(struct kthread_work *dummy);
static void khugepaged_wait_func(struct kthread_work *dummy);
static void khugepaged_cleanup_func(struct kthread_work *dummy);
static DEFINE_KTHREAD_WORKER(khugepaged_worker);
-static DEFINE_KTHREAD_WORK(khugepaged_init_work, khugepaged_init_func);
static DEFINE_KTHREAD_WORK(khugepaged_do_scan_work, khugepaged_do_scan_func);
static DEFINE_KTHREAD_WORK(khugepaged_wait_work, khugepaged_wait_func);
static DEFINE_KTHREAD_WORK(khugepaged_cleanup_work, khugepaged_cleanup_func);
@@ -167,8 +165,7 @@ static int start_stop_khugepaged(void)
goto out;
}
- queue_kthread_work(&khugepaged_worker,
- &khugepaged_init_work);
+ set_kthread_worker_user_nice(&khugepaged_worker, MAX_NICE);
if (list_empty(&khugepaged_scan.mm_head))
queue_kthread_work(&khugepaged_worker,
@@ -2803,11 +2800,6 @@ static int khugepaged_wait_event(void)
!khugepaged_enabled());
}
-static void khugepaged_init_func(struct kthread_work *dummy)
-{
- set_user_nice(current, MAX_NICE);
-}
-
static void khugepaged_do_scan_func(struct kthread_work *dummy)
{
struct page *hpage = NULL;
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [RFC PATCH 12/14] kthread_worker: Better support freezable kthread workers
From: Petr Mladek @ 2015-07-28 14:39 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra
Cc: Steven Rostedt, Paul E. McKenney, Josh Triplett, Thomas Gleixner,
Linus Torvalds, Jiri Kosina, Borislav Petkov, Michal Hocko,
linux-mm, Vlastimil Babka, live-patching, linux-api, linux-kernel,
Petr Mladek
In-Reply-To: <1438094371-8326-1-git-send-email-pmladek@suse.com>
This patch allows to make kthread worker freezable via a new @flags
parameter. It will allow to avoid an init work in some kthreads.
It currently does not affect the function of kthread_worker_fn()
but it might help to do some optimization or fixes eventually.
I currently do not know about any other use for the @flags
parameter but I believe that we will want more flags
in the future.
Finally, I hope that it will not cause confusion with @flags member
in struct kthread. Well, I guess that we will want to rework the
basic kthreads implementation once all kthreads are converted into
kthread workers or workqueues. It is possible that we will merge
the two structures.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/kthread.h | 13 +++++++++----
kernel/kthread.c | 8 +++++++-
kernel/rcu/tree.c | 3 ++-
kernel/trace/ring_buffer_benchmark.c | 2 +-
mm/huge_memory.c | 2 +-
5 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index 02d3cc9ad923..d916b024e986 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -63,7 +63,12 @@ extern int tsk_fork_get_node(struct task_struct *tsk);
struct kthread_work;
typedef void (*kthread_work_func_t)(struct kthread_work *work);
+enum {
+ KTW_FREEZABLE = 1 << 2, /* freeze during suspend */
+};
+
struct kthread_worker {
+ unsigned int flags;
spinlock_t lock;
struct list_head work_list;
struct task_struct *task;
@@ -129,13 +134,13 @@ static inline bool kthread_worker_created(struct kthread_worker *worker)
int kthread_worker_fn(void *worker_ptr);
-__printf(3, 4)
+__printf(4, 5)
int create_kthread_worker_on_node(struct kthread_worker *worker,
- int node,
+ unsigned int flags, int node,
const char namefmt[], ...);
-#define create_kthread_worker(worker, namefmt, arg...) \
- create_kthread_worker_on_node(worker, -1, namefmt, ##arg)
+#define create_kthread_worker(worker, flags, namefmt, arg...) \
+ create_kthread_worker_on_node(worker, flags, -1, namefmt, ##arg)
bool queue_kthread_work(struct kthread_worker *worker,
struct kthread_work *work);
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 053c9dfa58ac..d02509e17f7e 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -535,6 +535,7 @@ void __init_kthread_worker(struct kthread_worker *worker,
const char *name,
struct lock_class_key *key)
{
+ worker->flags = 0;
spin_lock_init(&worker->lock);
lockdep_set_class_and_name(&worker->lock, key, name);
INIT_LIST_HEAD(&worker->work_list);
@@ -569,6 +570,10 @@ int kthread_worker_fn(void *worker_ptr)
*/
WARN_ON(worker->task && worker->task != current);
worker->task = current;
+
+ if (worker->flags & KTW_FREEZABLE)
+ set_freezable();
+
repeat:
set_current_state(TASK_INTERRUPTIBLE); /* mb paired w/ kthread_stop */
@@ -611,7 +616,7 @@ EXPORT_SYMBOL_GPL(kthread_worker_fn);
* in @node, to get NUMA affinity for kthread stack, or else give -1.
*/
int create_kthread_worker_on_node(struct kthread_worker *worker,
- int node,
+ unsigned int flags, int node,
const char namefmt[], ...)
{
struct task_struct *task;
@@ -633,6 +638,7 @@ int create_kthread_worker_on_node(struct kthread_worker *worker,
set_bit(KTHREAD_IS_WORKER, &kthread->flags);
spin_lock_irq(&worker->lock);
+ worker->flags = flags;
worker->task = task;
spin_unlock_irq(&worker->lock);
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 475bd59509ed..3a286f3b8b3c 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -3935,7 +3935,8 @@ static int __init rcu_spawn_gp_kthread(void)
init_kthread_worker(&rsp->gp_worker);
init_kthread_work(&rsp->gp_init_work, rcu_gp_kthread_init_func);
init_kthread_work(&rsp->gp_work, rcu_gp_kthread_func);
- ret = create_kthread_worker(&rsp->gp_worker, "%s", rsp->name);
+ ret = create_kthread_worker(&rsp->gp_worker, 0,
+ "%s", rsp->name);
BUG_ON(ret);
rnp = rcu_get_root(rsp);
raw_spin_lock_irqsave(&rnp->lock, flags);
diff --git a/kernel/trace/ring_buffer_benchmark.c b/kernel/trace/ring_buffer_benchmark.c
index 86514babe07f..5036d284885c 100644
--- a/kernel/trace/ring_buffer_benchmark.c
+++ b/kernel/trace/ring_buffer_benchmark.c
@@ -450,7 +450,7 @@ static int __init ring_buffer_benchmark_init(void)
goto out_fail;
}
- ret = create_kthread_worker(&rb_producer_worker, "rb_producer");
+ ret = create_kthread_worker(&rb_producer_worker, 0, "rb_producer");
if (ret)
goto out_kill;
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 55733735a487..51a514161f2b 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -159,6 +159,7 @@ static int start_stop_khugepaged(void)
goto out;
err = create_kthread_worker(&khugepaged_worker,
+ KTW_FREEZABLE,
"khugepaged");
if (unlikely(err)) {
@@ -2804,7 +2805,6 @@ static int khugepaged_wait_event(void)
static void khugepaged_init_func(struct kthread_work *dummy)
{
- set_freezable();
set_user_nice(current, MAX_NICE);
}
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [RFC PATCH 11/14] ring_buffer: Use kthread worker API for the producer kthread in the benchmark
From: Petr Mladek @ 2015-07-28 14:39 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra
Cc: Steven Rostedt, Paul E. McKenney, Josh Triplett, Thomas Gleixner,
Linus Torvalds, Jiri Kosina, Borislav Petkov, Michal Hocko,
linux-mm, Vlastimil Babka, live-patching, linux-api, linux-kernel,
Petr Mladek
In-Reply-To: <1438094371-8326-1-git-send-email-pmladek@suse.com>
Kthreads are currently implemented as an infinite loop. Each
has its own variant of checks for terminating, freezing,
awakening. In many cases it is unclear to say in which state
it is and sometimes it is done a wrong way.
The plan is to convert kthreads into kthread_worker or workqueues
API. It allows to split the functionality into separate operations.
It helps to make a better structure. Also it defines a clean state
where no locks are taken, IRQs blocked, the kthread might sleep
or even be safely migrated.
The kthread worker API is useful when we want to have a dedicated
single thread for the work. It helps to make sure that it is
available when needed. Also it allows a better control, e.g.
define a scheduling priority.
This patch converts the ring buffer benchmark producer into a kthread
worker because it modifies the scheduling priority and policy.
Also, it is a benchmark. It makes CPU very busy. It will most likely
run only limited time. IMHO, it does not make sense to mess the system
workqueues with it.
The thread is split into two independent works. It might look more
complicated but it helped me to find a race in the sleeping part
that was fixed separately.
kthread_should_stop() could not longer be used inside the works
because it defines the life of the worker and it needs to stay
usable until all works are done. Instead, we add @test_end
global variable. It is set during normal termination in compare
with @test_error.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
kernel/trace/ring_buffer_benchmark.c | 81 ++++++++++++++++++++++--------------
1 file changed, 49 insertions(+), 32 deletions(-)
diff --git a/kernel/trace/ring_buffer_benchmark.c b/kernel/trace/ring_buffer_benchmark.c
index 10e0ec9b797f..86514babe07f 100644
--- a/kernel/trace/ring_buffer_benchmark.c
+++ b/kernel/trace/ring_buffer_benchmark.c
@@ -26,9 +26,14 @@ static int wakeup_interval = 100;
static int reader_finish;
static DECLARE_COMPLETION(read_start);
static DECLARE_COMPLETION(read_done);
-
static struct ring_buffer *buffer;
-static struct task_struct *producer;
+
+static void rb_producer_hammer_func(struct kthread_work *dummy);
+static void rb_producer_sleep_func(struct kthread_work *dummy);
+static DEFINE_KTHREAD_WORKER(rb_producer_worker);
+static DEFINE_KTHREAD_WORK(rb_producer_hammer_work, rb_producer_hammer_func);
+static DEFINE_KTHREAD_WORK(rb_producer_sleep_work, rb_producer_sleep_func);
+
static struct task_struct *consumer;
static unsigned long read;
@@ -61,6 +66,7 @@ MODULE_PARM_DESC(consumer_fifo, "fifo prio for consumer");
static int read_events;
static int test_error;
+static int test_end;
#define TEST_ERROR() \
do { \
@@ -77,7 +83,11 @@ enum event_status {
static bool break_test(void)
{
- return test_error || kthread_should_stop();
+ /*
+ * FIXME: The test for kthread_should_stop() will get obsoleted
+ * once the consumer is too converted into the kthread worker API.
+ */
+ return test_error || test_end || kthread_should_stop();
}
static enum event_status read_event(int cpu)
@@ -387,34 +397,40 @@ static int ring_buffer_consumer_thread(void *arg)
return 0;
}
-static int ring_buffer_producer_thread(void *arg)
+static void rb_producer_hammer_func(struct kthread_work *dummy)
{
- while (!break_test()) {
- ring_buffer_reset(buffer);
+ if (break_test())
+ return;
- if (consumer) {
- wake_up_process(consumer);
- wait_for_completion(&read_start);
- }
+ ring_buffer_reset(buffer);
- ring_buffer_producer();
- if (break_test())
- goto out_kill;
+ if (consumer) {
+ wake_up_process(consumer);
+ wait_for_completion(&read_start);
+ }
- trace_printk("Sleeping for 10 secs\n");
- set_current_state(TASK_INTERRUPTIBLE);
- if (break_test()) {
- __set_current_state(TASK_RUNNING);
- goto out_kill;
- }
- schedule_timeout(HZ * SLEEP_TIME);
+ ring_buffer_producer();
+
+ if (break_test())
+ return;
+
+ queue_kthread_work(&rb_producer_worker, &rb_producer_sleep_work);
+}
+
+static void rb_producer_sleep_func(struct kthread_work *dummy)
+{
+ trace_printk("Sleeping for 10 secs\n");
+ set_current_state(TASK_INTERRUPTIBLE);
+ if (break_test()) {
+ set_current_state(TASK_RUNNING);
+ return;
}
+ schedule_timeout(HZ * SLEEP_TIME);
-out_kill:
- if (!kthread_should_stop())
- wait_to_die();
+ if (break_test())
+ return;
- return 0;
+ queue_kthread_work(&rb_producer_worker, &rb_producer_hammer_work);
}
static int __init ring_buffer_benchmark_init(void)
@@ -434,13 +450,12 @@ static int __init ring_buffer_benchmark_init(void)
goto out_fail;
}
- producer = kthread_run(ring_buffer_producer_thread,
- NULL, "rb_producer");
- ret = PTR_ERR(producer);
-
- if (IS_ERR(producer))
+ ret = create_kthread_worker(&rb_producer_worker, "rb_producer");
+ if (ret)
goto out_kill;
+ queue_kthread_work(&rb_producer_worker, &rb_producer_hammer_work);
+
/*
* Run them as low-prio background tasks by default:
*/
@@ -458,9 +473,10 @@ static int __init ring_buffer_benchmark_init(void)
struct sched_param param = {
.sched_priority = producer_fifo
};
- sched_setscheduler(producer, SCHED_FIFO, ¶m);
+ sched_setscheduler(rb_producer_worker.task,
+ SCHED_FIFO, ¶m);
} else
- set_user_nice(producer, producer_nice);
+ set_user_nice(rb_producer_worker.task, producer_nice);
return 0;
@@ -475,7 +491,8 @@ static int __init ring_buffer_benchmark_init(void)
static void __exit ring_buffer_benchmark_exit(void)
{
- kthread_stop(producer);
+ test_end = 1;
+ wakeup_and_destroy_kthread_worker(&rb_producer_worker);
if (consumer)
kthread_stop(consumer);
ring_buffer_free(buffer);
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [RFC PATCH 10/14] ring_buffer: Fix more races when terminating the producer in the benchmark
From: Petr Mladek @ 2015-07-28 14:39 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra
Cc: Steven Rostedt, Paul E. McKenney, Josh Triplett, Thomas Gleixner,
Linus Torvalds, Jiri Kosina, Borislav Petkov, Michal Hocko,
linux-mm, Vlastimil Babka, live-patching, linux-api, linux-kernel,
Petr Mladek
In-Reply-To: <1438094371-8326-1-git-send-email-pmladek@suse.com>
The commit b44754d8262d3aab8 ("ring_buffer: Allow to exit the ring
buffer benchmark immediately") added a hack into ring_buffer_producer()
that set @kill_test when kthread_should_stop() returned true. It improved
the situation a lot. It stopped the kthread in most cases because
the producer spent most of the time in the patched while cycle.
But there are still few possible races when kthread_should_stop()
is set outside of the cycle. Then we do not set @kill_test and
some other checks pass.
This patch adds a better fix. It renames @test_kill/TEST_KILL() into
a better descriptive @test_error/TEST_ERROR(). Also it introduces
break_test() function that checks for both @test_error and
kthread_should_stop(). Finally, the new function is used
on many locations when the check for @test_error is not enough.
Also it adds a missing check into ring_buffer_producer_thread()
between setting TASK_INTERRUPTIBLE and calling schedule_timeout().
Otherwise, we might miss a wakeup from kthread_stop().
Finally, it adds the same check also into ring_buffer_consumer()
between setting TASK_INTERRUPTIBLE and calling schedule_timeout().
Well, I added this one just for paranoid reasons. If we are here
the producer should have been destroyed before and it should have
set @reader_finish. But better be safe.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
kernel/trace/ring_buffer_benchmark.c | 65 ++++++++++++++++++++----------------
1 file changed, 37 insertions(+), 28 deletions(-)
diff --git a/kernel/trace/ring_buffer_benchmark.c b/kernel/trace/ring_buffer_benchmark.c
index ccb1a0b95f64..10e0ec9b797f 100644
--- a/kernel/trace/ring_buffer_benchmark.c
+++ b/kernel/trace/ring_buffer_benchmark.c
@@ -60,12 +60,12 @@ MODULE_PARM_DESC(consumer_fifo, "fifo prio for consumer");
static int read_events;
-static int kill_test;
+static int test_error;
-#define KILL_TEST() \
+#define TEST_ERROR() \
do { \
- if (!kill_test) { \
- kill_test = 1; \
+ if (!test_error) { \
+ test_error = 1; \
WARN_ON(1); \
} \
} while (0)
@@ -75,6 +75,11 @@ enum event_status {
EVENT_DROPPED,
};
+static bool break_test(void)
+{
+ return test_error || kthread_should_stop();
+}
+
static enum event_status read_event(int cpu)
{
struct ring_buffer_event *event;
@@ -87,7 +92,7 @@ static enum event_status read_event(int cpu)
entry = ring_buffer_event_data(event);
if (*entry != cpu) {
- KILL_TEST();
+ TEST_ERROR();
return EVENT_DROPPED;
}
@@ -115,10 +120,13 @@ static enum event_status read_page(int cpu)
rpage = bpage;
/* The commit may have missed event flags set, clear them */
commit = local_read(&rpage->commit) & 0xfffff;
- for (i = 0; i < commit && !kill_test; i += inc) {
+ for (i = 0; i < commit ; i += inc) {
+
+ if (break_test())
+ break;
if (i >= (PAGE_SIZE - offsetof(struct rb_page, data))) {
- KILL_TEST();
+ TEST_ERROR();
break;
}
@@ -128,7 +136,7 @@ static enum event_status read_page(int cpu)
case RINGBUF_TYPE_PADDING:
/* failed writes may be discarded events */
if (!event->time_delta)
- KILL_TEST();
+ TEST_ERROR();
inc = event->array[0] + 4;
break;
case RINGBUF_TYPE_TIME_EXTEND:
@@ -137,12 +145,12 @@ static enum event_status read_page(int cpu)
case 0:
entry = ring_buffer_event_data(event);
if (*entry != cpu) {
- KILL_TEST();
+ TEST_ERROR();
break;
}
read++;
if (!event->array[0]) {
- KILL_TEST();
+ TEST_ERROR();
break;
}
inc = event->array[0] + 4;
@@ -150,17 +158,17 @@ static enum event_status read_page(int cpu)
default:
entry = ring_buffer_event_data(event);
if (*entry != cpu) {
- KILL_TEST();
+ TEST_ERROR();
break;
}
read++;
inc = ((event->type_len + 1) * 4);
}
- if (kill_test)
+ if (test_error)
break;
if (inc <= 0) {
- KILL_TEST();
+ TEST_ERROR();
break;
}
}
@@ -178,7 +186,7 @@ static void ring_buffer_consumer(void)
read_events ^= 1;
read = 0;
- while (!reader_finish && !kill_test) {
+ while (!reader_finish && !break_test()) {
int found;
do {
@@ -193,17 +201,18 @@ static void ring_buffer_consumer(void)
else
stat = read_page(cpu);
- if (kill_test)
+ if (break_test())
break;
if (stat == EVENT_FOUND)
found = 1;
}
- } while (found && !kill_test);
+ } while (found && !break_test());
set_current_state(TASK_INTERRUPTIBLE);
- if (reader_finish)
+ if (reader_finish || break_test()) {
+ __set_current_state(TASK_RUNNING);
break;
-
+ }
schedule();
}
reader_finish = 0;
@@ -263,10 +272,7 @@ static void ring_buffer_producer(void)
if (cnt % wakeup_interval)
cond_resched();
#endif
- if (kthread_should_stop())
- kill_test = 1;
-
- } while (ktime_before(end_time, timeout) && !kill_test);
+ } while (ktime_before(end_time, timeout) && !break_test());
trace_printk("End ring buffer hammer\n");
if (consumer) {
@@ -282,7 +288,7 @@ static void ring_buffer_producer(void)
entries = ring_buffer_entries(buffer);
overruns = ring_buffer_overruns(buffer);
- if (kill_test && !kthread_should_stop())
+ if (test_error)
trace_printk("ERROR!\n");
if (!disable_reader) {
@@ -363,15 +369,14 @@ static void wait_to_die(void)
static int ring_buffer_consumer_thread(void *arg)
{
- while (!kthread_should_stop() && !kill_test) {
+ while (!break_test()) {
complete(&read_start);
ring_buffer_consumer();
set_current_state(TASK_INTERRUPTIBLE);
- if (kthread_should_stop() || kill_test)
+ if (break_test())
break;
-
schedule();
}
__set_current_state(TASK_RUNNING);
@@ -384,7 +389,7 @@ static int ring_buffer_consumer_thread(void *arg)
static int ring_buffer_producer_thread(void *arg)
{
- while (!kthread_should_stop() && !kill_test) {
+ while (!break_test()) {
ring_buffer_reset(buffer);
if (consumer) {
@@ -393,11 +398,15 @@ static int ring_buffer_producer_thread(void *arg)
}
ring_buffer_producer();
- if (kill_test)
+ if (break_test())
goto out_kill;
trace_printk("Sleeping for 10 secs\n");
set_current_state(TASK_INTERRUPTIBLE);
+ if (break_test()) {
+ __set_current_state(TASK_RUNNING);
+ goto out_kill;
+ }
schedule_timeout(HZ * SLEEP_TIME);
}
--
1.8.5.6
^ permalink raw reply related
* [RFC PATCH 09/14] ring_buffer: Initialize completions statically in the benchmark
From: Petr Mladek @ 2015-07-28 14:39 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra
Cc: Steven Rostedt, Paul E. McKenney, Josh Triplett, Thomas Gleixner,
Linus Torvalds, Jiri Kosina, Borislav Petkov, Michal Hocko,
linux-mm, Vlastimil Babka, live-patching, linux-api, linux-kernel,
Petr Mladek
In-Reply-To: <1438094371-8326-1-git-send-email-pmladek@suse.com>
It looks strange to initialize the completions repeatedly.
This patch uses static initialization. It simplifies the code
and even helps to get rid of two memory barriers.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
kernel/trace/ring_buffer_benchmark.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/kernel/trace/ring_buffer_benchmark.c b/kernel/trace/ring_buffer_benchmark.c
index a1503a027ee2..ccb1a0b95f64 100644
--- a/kernel/trace/ring_buffer_benchmark.c
+++ b/kernel/trace/ring_buffer_benchmark.c
@@ -24,8 +24,8 @@ struct rb_page {
static int wakeup_interval = 100;
static int reader_finish;
-static struct completion read_start;
-static struct completion read_done;
+static DECLARE_COMPLETION(read_start);
+static DECLARE_COMPLETION(read_done);
static struct ring_buffer *buffer;
static struct task_struct *producer;
@@ -270,11 +270,6 @@ static void ring_buffer_producer(void)
trace_printk("End ring buffer hammer\n");
if (consumer) {
- /* Init both completions here to avoid races */
- init_completion(&read_start);
- init_completion(&read_done);
- /* the completions must be visible before the finish var */
- smp_wmb();
reader_finish = 1;
/* finish var visible before waking up the consumer */
smp_wmb();
@@ -389,13 +384,10 @@ static int ring_buffer_consumer_thread(void *arg)
static int ring_buffer_producer_thread(void *arg)
{
- init_completion(&read_start);
-
while (!kthread_should_stop() && !kill_test) {
ring_buffer_reset(buffer);
if (consumer) {
- smp_wmb();
wake_up_process(consumer);
wait_for_completion(&read_start);
}
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [RFC PATCH 08/14] rcu: Convert RCU gp kthreads into kthread worker API
From: Petr Mladek @ 2015-07-28 14:39 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra
Cc: Steven Rostedt, Paul E. McKenney, Josh Triplett, Thomas Gleixner,
Linus Torvalds, Jiri Kosina, Borislav Petkov, Michal Hocko,
linux-mm, Vlastimil Babka, live-patching, linux-api, linux-kernel,
Petr Mladek
In-Reply-To: <1438094371-8326-1-git-send-email-pmladek@suse.com>
Kthreads are currently implemented as an infinite loop. Each
has its own variant of checks for terminating, freezing,
awakening. In many cases it is unclear to say in which state
it is and sometimes it is done a wrong way.
The plan is to convert kthreads into kthread_worker or workqueues
API. It allows to split the functionality into separate operations.
It helps to make a better structure. Also it defines a clean state
where no locks are taken, IRQs blocked, the kthread might sleep
or even be safely migrated.
The kthread worker API is useful when we want to have a dedicated
single thread for the work. It helps to make sure that it is
available when needed. Also it allows a better control, e.g.
define a scheduling priority.
This patch converts RCU gp threads into the kthread worker API.
They modify the scheduling, have their own logic to bind the process.
They provide functions that are critical for the system to work
and thus deserve a dedicated kthread. In fact, they most likely
could not be implemented using workqueues because workqueues
are implemented using RCU.
The conversion is rather straightforward. It moves the code from
the main cycle into a single work because they should be done
together.
Note that we would like to provide more helper functions in
the kthread worker API and hide access to worker.task in the
long term. But it is not completely solved in this RFC.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
kernel/rcu/tree.c | 175 +++++++++++++++++++++++++++++-------------------------
kernel/rcu/tree.h | 4 +-
2 files changed, 96 insertions(+), 83 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 65137bc28b2b..475bd59509ed 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -485,7 +485,7 @@ void show_rcu_gp_kthreads(void)
for_each_rcu_flavor(rsp) {
pr_info("%s: wait state: %d ->state: %#lx\n",
- rsp->name, rsp->gp_state, rsp->gp_kthread->state);
+ rsp->name, rsp->gp_state, rsp->gp_worker.task->state);
/* sched_show_task(rsp->gp_kthread); */
}
}
@@ -1586,9 +1586,9 @@ static int rcu_future_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
*/
static void rcu_gp_kthread_wake(struct rcu_state *rsp)
{
- if (current == rsp->gp_kthread ||
+ if (current == rsp->gp_worker.task ||
!READ_ONCE(rsp->gp_flags) ||
- !rsp->gp_kthread)
+ !rsp->gp_worker.task)
return;
wake_up(&rsp->gp_wq);
}
@@ -2017,101 +2017,109 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
raw_spin_unlock_irq(&rnp->lock);
}
+static void rcu_gp_kthread_init_func(struct kthread_work *work)
+{
+ struct rcu_state *rsp = container_of(work, struct rcu_state,
+ gp_init_work);
+
+ rcu_bind_gp_kthread();
+
+ queue_kthread_work(&rsp->gp_worker, &rsp->gp_work);
+}
+
/*
- * Body of kthread that handles grace periods.
+ * Main work of kthread that handles grace periods.
*/
-static int __noreturn rcu_gp_kthread(void *arg)
+static void rcu_gp_kthread_func(struct kthread_work *work)
{
int fqs_state;
int gf;
unsigned long j;
int ret;
- struct rcu_state *rsp = arg;
+ struct rcu_state *rsp = container_of(work, struct rcu_state, gp_work);
struct rcu_node *rnp = rcu_get_root(rsp);
- rcu_bind_gp_kthread();
+ /* Handle grace-period start. */
for (;;) {
+ trace_rcu_grace_period(rsp->name,
+ READ_ONCE(rsp->gpnum),
+ TPS("reqwait"));
+ rsp->gp_state = RCU_GP_WAIT_GPS;
+ wait_event_interruptible(rsp->gp_wq,
+ READ_ONCE(rsp->gp_flags) &
+ RCU_GP_FLAG_INIT);
+ /* Locking provides needed memory barrier. */
+ if (rcu_gp_init(rsp))
+ break;
+ cond_resched_rcu_qs();
+ WRITE_ONCE(rsp->gp_activity, jiffies);
+ WARN_ON(signal_pending(current));
+ trace_rcu_grace_period(rsp->name,
+ READ_ONCE(rsp->gpnum),
+ TPS("reqwaitsig"));
+ }
- /* Handle grace-period start. */
- for (;;) {
+ /* Handle quiescent-state forcing. */
+ fqs_state = RCU_SAVE_DYNTICK;
+ j = jiffies_till_first_fqs;
+ if (j > HZ) {
+ j = HZ;
+ jiffies_till_first_fqs = HZ;
+ }
+ ret = 0;
+ for (;;) {
+ if (!ret)
+ rsp->jiffies_force_qs = jiffies + j;
+ trace_rcu_grace_period(rsp->name,
+ READ_ONCE(rsp->gpnum),
+ TPS("fqswait"));
+ rsp->gp_state = RCU_GP_WAIT_FQS;
+ ret = wait_event_interruptible_timeout(rsp->gp_wq,
+ ((gf = READ_ONCE(rsp->gp_flags)) &
+ RCU_GP_FLAG_FQS) ||
+ (!READ_ONCE(rnp->qsmask) &&
+ !rcu_preempt_blocked_readers_cgp(rnp)),
+ j);
+ /* Locking provides needed memory barriers. */
+ /* If grace period done, leave loop. */
+ if (!READ_ONCE(rnp->qsmask) &&
+ !rcu_preempt_blocked_readers_cgp(rnp))
+ break;
+ /* If time for quiescent-state forcing, do it. */
+ if (ULONG_CMP_GE(jiffies, rsp->jiffies_force_qs) ||
+ (gf & RCU_GP_FLAG_FQS)) {
trace_rcu_grace_period(rsp->name,
READ_ONCE(rsp->gpnum),
- TPS("reqwait"));
- rsp->gp_state = RCU_GP_WAIT_GPS;
- wait_event_interruptible(rsp->gp_wq,
- READ_ONCE(rsp->gp_flags) &
- RCU_GP_FLAG_INIT);
- /* Locking provides needed memory barrier. */
- if (rcu_gp_init(rsp))
- break;
+ TPS("fqsstart"));
+ fqs_state = rcu_gp_fqs(rsp, fqs_state);
+ trace_rcu_grace_period(rsp->name,
+ READ_ONCE(rsp->gpnum),
+ TPS("fqsend"));
+ cond_resched_rcu_qs();
+ WRITE_ONCE(rsp->gp_activity, jiffies);
+ } else {
+ /* Deal with stray signal. */
cond_resched_rcu_qs();
WRITE_ONCE(rsp->gp_activity, jiffies);
WARN_ON(signal_pending(current));
trace_rcu_grace_period(rsp->name,
READ_ONCE(rsp->gpnum),
- TPS("reqwaitsig"));
+ TPS("fqswaitsig"));
}
-
- /* Handle quiescent-state forcing. */
- fqs_state = RCU_SAVE_DYNTICK;
- j = jiffies_till_first_fqs;
+ j = jiffies_till_next_fqs;
if (j > HZ) {
j = HZ;
- jiffies_till_first_fqs = HZ;
+ jiffies_till_next_fqs = HZ;
+ } else if (j < 1) {
+ j = 1;
+ jiffies_till_next_fqs = 1;
}
- ret = 0;
- for (;;) {
- if (!ret)
- rsp->jiffies_force_qs = jiffies + j;
- trace_rcu_grace_period(rsp->name,
- READ_ONCE(rsp->gpnum),
- TPS("fqswait"));
- rsp->gp_state = RCU_GP_WAIT_FQS;
- ret = wait_event_interruptible_timeout(rsp->gp_wq,
- ((gf = READ_ONCE(rsp->gp_flags)) &
- RCU_GP_FLAG_FQS) ||
- (!READ_ONCE(rnp->qsmask) &&
- !rcu_preempt_blocked_readers_cgp(rnp)),
- j);
- /* Locking provides needed memory barriers. */
- /* If grace period done, leave loop. */
- if (!READ_ONCE(rnp->qsmask) &&
- !rcu_preempt_blocked_readers_cgp(rnp))
- break;
- /* If time for quiescent-state forcing, do it. */
- if (ULONG_CMP_GE(jiffies, rsp->jiffies_force_qs) ||
- (gf & RCU_GP_FLAG_FQS)) {
- trace_rcu_grace_period(rsp->name,
- READ_ONCE(rsp->gpnum),
- TPS("fqsstart"));
- fqs_state = rcu_gp_fqs(rsp, fqs_state);
- trace_rcu_grace_period(rsp->name,
- READ_ONCE(rsp->gpnum),
- TPS("fqsend"));
- cond_resched_rcu_qs();
- WRITE_ONCE(rsp->gp_activity, jiffies);
- } else {
- /* Deal with stray signal. */
- cond_resched_rcu_qs();
- WRITE_ONCE(rsp->gp_activity, jiffies);
- WARN_ON(signal_pending(current));
- trace_rcu_grace_period(rsp->name,
- READ_ONCE(rsp->gpnum),
- TPS("fqswaitsig"));
- }
- j = jiffies_till_next_fqs;
- if (j > HZ) {
- j = HZ;
- jiffies_till_next_fqs = HZ;
- } else if (j < 1) {
- j = 1;
- jiffies_till_next_fqs = 1;
- }
- }
-
- /* Handle grace-period end. */
- rcu_gp_cleanup(rsp);
}
+
+ /* Handle grace-period end. */
+ rcu_gp_cleanup(rsp);
+
+ queue_kthread_work(&rsp->gp_worker, &rsp->gp_work);
}
/*
@@ -2129,7 +2137,7 @@ static bool
rcu_start_gp_advanced(struct rcu_state *rsp, struct rcu_node *rnp,
struct rcu_data *rdp)
{
- if (!rsp->gp_kthread || !cpu_needs_another_gp(rsp, rdp)) {
+ if (!rsp->gp_worker.task || !cpu_needs_another_gp(rsp, rdp)) {
/*
* Either we have not yet spawned the grace-period
* task, this CPU does not need another grace period,
@@ -3909,7 +3917,7 @@ static int __init rcu_spawn_gp_kthread(void)
struct rcu_node *rnp;
struct rcu_state *rsp;
struct sched_param sp;
- struct task_struct *t;
+ int ret;
/* Force priority into range. */
if (IS_ENABLED(CONFIG_RCU_BOOST) && kthread_prio < 1)
@@ -3924,16 +3932,19 @@ static int __init rcu_spawn_gp_kthread(void)
rcu_scheduler_fully_active = 1;
for_each_rcu_flavor(rsp) {
- t = kthread_create(rcu_gp_kthread, rsp, "%s", rsp->name);
- BUG_ON(IS_ERR(t));
+ init_kthread_worker(&rsp->gp_worker);
+ init_kthread_work(&rsp->gp_init_work, rcu_gp_kthread_init_func);
+ init_kthread_work(&rsp->gp_work, rcu_gp_kthread_func);
+ ret = create_kthread_worker(&rsp->gp_worker, "%s", rsp->name);
+ BUG_ON(ret);
rnp = rcu_get_root(rsp);
raw_spin_lock_irqsave(&rnp->lock, flags);
- rsp->gp_kthread = t;
if (kthread_prio) {
sp.sched_priority = kthread_prio;
- sched_setscheduler_nocheck(t, SCHED_FIFO, &sp);
+ sched_setscheduler_nocheck(rsp->gp_worker.task,
+ SCHED_FIFO, &sp);
}
- wake_up_process(t);
+ queue_kthread_work(&rsp->gp_worker, &rsp->gp_init_work);
raw_spin_unlock_irqrestore(&rnp->lock, flags);
}
rcu_spawn_nocb_kthreads();
diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index 4adb7ca0bf47..2f318d406a53 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -457,7 +457,9 @@ struct rcu_state {
u8 boost; /* Subject to priority boost. */
unsigned long gpnum; /* Current gp number. */
unsigned long completed; /* # of last completed gp. */
- struct task_struct *gp_kthread; /* Task for grace periods. */
+ struct kthread_worker gp_worker; /* Worker for grace periods */
+ struct kthread_work gp_init_work; /* Init work for handling gp */
+ struct kthread_work gp_work; /* Main work for handling gp */
wait_queue_head_t gp_wq; /* Where GP task waits. */
short gp_flags; /* Commands for GP task. */
short gp_state; /* GP kthread sleep state. */
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [RFC PATCH 07/14] mm/huge_page: Convert khugepaged() into kthread worker API
From: Petr Mladek @ 2015-07-28 14:39 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra
Cc: Steven Rostedt, Paul E. McKenney, Josh Triplett, Thomas Gleixner,
Linus Torvalds, Jiri Kosina, Borislav Petkov, Michal Hocko,
linux-mm, Vlastimil Babka, live-patching, linux-api, linux-kernel,
Petr Mladek
In-Reply-To: <1438094371-8326-1-git-send-email-pmladek@suse.com>
Kthreads are currently implemented as an infinite loop. Each
has its own variant of checks for terminating, freezing,
awakening. In many cases it is unclear to say in which state
it is and sometimes it is done a wrong way.
The plan is to convert kthreads into kthread_worker or workqueues
API. It allows to split the functionality into separate operations.
It helps to make a better structure. Also it defines a clean state
where no locks are taken, IRQs blocked, the kthread might sleep
or even be safely migrated.
The kthread worker API is useful when we want to have a dedicated
single thread for the work. It helps to make sure that it is
available when needed. Also it allows a better control, e.g.
define a scheduling priority.
This patch converts khugepaged() in kthread worker API
because it modifies the scheduling.
It keeps the functionality except that we do not wakeup
the worker when it is already created and someone
calls start() once again.
Note that we could not longer check for kthread_should_stop()
in the works. The kthread used by the worker has to stay alive
until all queued works are finished. Instead, we use the existing
check khugepaged_enabled() that returns false when we are going down.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
mm/huge_memory.c | 91 +++++++++++++++++++++++++++++++++++---------------------
1 file changed, 57 insertions(+), 34 deletions(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index c107094f79ba..55733735a487 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -54,7 +54,17 @@ static unsigned int khugepaged_full_scans;
static unsigned int khugepaged_scan_sleep_millisecs __read_mostly = 10000;
/* during fragmentation poll the hugepage allocator once every minute */
static unsigned int khugepaged_alloc_sleep_millisecs __read_mostly = 60000;
-static struct task_struct *khugepaged_thread __read_mostly;
+
+static void khugepaged_init_func(struct kthread_work *dummy);
+static void khugepaged_do_scan_func(struct kthread_work *dummy);
+static void khugepaged_wait_func(struct kthread_work *dummy);
+static void khugepaged_cleanup_func(struct kthread_work *dummy);
+static DEFINE_KTHREAD_WORKER(khugepaged_worker);
+static DEFINE_KTHREAD_WORK(khugepaged_init_work, khugepaged_init_func);
+static DEFINE_KTHREAD_WORK(khugepaged_do_scan_work, khugepaged_do_scan_func);
+static DEFINE_KTHREAD_WORK(khugepaged_wait_work, khugepaged_wait_func);
+static DEFINE_KTHREAD_WORK(khugepaged_cleanup_work, khugepaged_cleanup_func);
+
static DEFINE_MUTEX(khugepaged_mutex);
static DEFINE_SPINLOCK(khugepaged_mm_lock);
static DECLARE_WAIT_QUEUE_HEAD(khugepaged_wait);
@@ -65,7 +75,6 @@ static DECLARE_WAIT_QUEUE_HEAD(khugepaged_wait);
*/
static unsigned int khugepaged_max_ptes_none __read_mostly = HPAGE_PMD_NR-1;
-static int khugepaged(void *none);
static int khugepaged_slab_init(void);
static void khugepaged_slab_exit(void);
@@ -146,25 +155,34 @@ static int start_stop_khugepaged(void)
{
int err = 0;
if (khugepaged_enabled()) {
- if (!khugepaged_thread)
- khugepaged_thread = kthread_run(khugepaged, NULL,
- "khugepaged");
- if (unlikely(IS_ERR(khugepaged_thread))) {
- pr_err("khugepaged: kthread_run(khugepaged) failed\n");
- err = PTR_ERR(khugepaged_thread);
- khugepaged_thread = NULL;
- goto fail;
+ if (kthread_worker_created(&khugepaged_worker))
+ goto out;
+
+ err = create_kthread_worker(&khugepaged_worker,
+ "khugepaged");
+
+ if (unlikely(err)) {
+ pr_err("khugepaged: failed to create kthread worker\n");
+ goto out;
}
- if (!list_empty(&khugepaged_scan.mm_head))
- wake_up_interruptible(&khugepaged_wait);
+ queue_kthread_work(&khugepaged_worker,
+ &khugepaged_init_work);
+
+ if (list_empty(&khugepaged_scan.mm_head))
+ queue_kthread_work(&khugepaged_worker,
+ &khugepaged_wait_work);
+ else
+ queue_kthread_work(&khugepaged_worker,
+ &khugepaged_do_scan_work);
set_recommended_min_free_kbytes();
- } else if (khugepaged_thread) {
- kthread_stop(khugepaged_thread);
- khugepaged_thread = NULL;
+ } else if (kthread_worker_created(&khugepaged_worker)) {
+ queue_kthread_work(&khugepaged_worker,
+ &khugepaged_cleanup_work);
+ wakeup_and_destroy_kthread_worker(&khugepaged_worker);
}
-fail:
+out:
return err;
}
@@ -2780,11 +2798,17 @@ static int khugepaged_has_work(void)
static int khugepaged_wait_event(void)
{
- return !list_empty(&khugepaged_scan.mm_head) ||
- kthread_should_stop();
+ return (!list_empty(&khugepaged_scan.mm_head) ||
+ !khugepaged_enabled());
+}
+
+static void khugepaged_init_func(struct kthread_work *dummy)
+{
+ set_freezable();
+ set_user_nice(current, MAX_NICE);
}
-static void khugepaged_do_scan(void)
+static void khugepaged_do_scan_func(struct kthread_work *dummy)
{
struct page *hpage = NULL;
unsigned int progress = 0, pass_through_head = 0;
@@ -2799,7 +2823,7 @@ static void khugepaged_do_scan(void)
cond_resched();
- if (unlikely(kthread_should_stop() || try_to_freeze()))
+ if (unlikely(!khugepaged_enabled() || try_to_freeze()))
break;
spin_lock(&khugepaged_mm_lock);
@@ -2816,43 +2840,42 @@ static void khugepaged_do_scan(void)
if (!IS_ERR_OR_NULL(hpage))
put_page(hpage);
+
+ if (khugepaged_enabled())
+ queue_kthread_work(&khugepaged_worker, &khugepaged_wait_work);
}
-static void khugepaged_wait_work(void)
+static void khugepaged_wait_func(struct kthread_work *dummy)
{
if (khugepaged_has_work()) {
if (!khugepaged_scan_sleep_millisecs)
- return;
+ goto out;
wait_event_freezable_timeout(khugepaged_wait,
- kthread_should_stop(),
+ !khugepaged_enabled(),
msecs_to_jiffies(khugepaged_scan_sleep_millisecs));
- return;
+ goto out;
}
if (khugepaged_enabled())
wait_event_freezable(khugepaged_wait, khugepaged_wait_event());
+
+out:
+ if (khugepaged_enabled())
+ queue_kthread_work(&khugepaged_worker,
+ &khugepaged_do_scan_work);
}
-static int khugepaged(void *none)
+static void khugepaged_cleanup_func(struct kthread_work *dummy)
{
struct mm_slot *mm_slot;
- set_freezable();
- set_user_nice(current, MAX_NICE);
-
- while (!kthread_should_stop()) {
- khugepaged_do_scan();
- khugepaged_wait_work();
- }
-
spin_lock(&khugepaged_mm_lock);
mm_slot = khugepaged_scan.mm_slot;
khugepaged_scan.mm_slot = NULL;
if (mm_slot)
collect_mm_slot(mm_slot);
spin_unlock(&khugepaged_mm_lock);
- return 0;
}
static void __split_huge_zero_page_pmd(struct vm_area_struct *vma,
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [RFC PATCH 06/14] kthread: Add kthread_worker_created()
From: Petr Mladek @ 2015-07-28 14:39 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra
Cc: Steven Rostedt, Paul E. McKenney, Josh Triplett, Thomas Gleixner,
Linus Torvalds, Jiri Kosina, Borislav Petkov, Michal Hocko,
linux-mm, Vlastimil Babka, live-patching, linux-api, linux-kernel,
Petr Mladek
In-Reply-To: <1438094371-8326-1-git-send-email-pmladek@suse.com>
I would like to make cleaner kthread worker API and hide the definition
of struct kthread_worker. It will prevent any custom hacks and make
the API more secure.
This patch provides an API to check if the worker has been created
and hides the implementation details.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/kthread.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index 24d72bac27db..02d3cc9ad923 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -122,6 +122,11 @@ extern void __init_kthread_worker(struct kthread_worker *worker,
(work)->func = (fn); \
} while (0)
+static inline bool kthread_worker_created(struct kthread_worker *worker)
+{
+ return (worker && worker->task);
+}
+
int kthread_worker_fn(void *worker_ptr);
__printf(3, 4)
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [RFC PATCH 05/14] kthread: Add wakeup_and_destroy_kthread_worker()
From: Petr Mladek @ 2015-07-28 14:39 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra
Cc: Steven Rostedt, Paul E. McKenney, Josh Triplett, Thomas Gleixner,
Linus Torvalds, Jiri Kosina, Borislav Petkov, Michal Hocko,
linux-mm, Vlastimil Babka, live-patching, linux-api, linux-kernel,
Petr Mladek
In-Reply-To: <1438094371-8326-1-git-send-email-pmladek@suse.com>
Most kthreads are sleeping lots of time. They do some job either
in regular intervals or when there is an event. Many of them combine
the two approaches.
The job is either a "single" operation, e.g. check and make a huge page.
Or the kthread is serving several requests, e.g. handling several NFS
callbacks.
Anyway, the single thread could process only one request at a time
and there might be more pending requests. Some kthreads use a more
complex algorithms to prioritize the pending work, e.g. a red-black
tree used by dmcrypt_write().
I want to say that only some kthreads can be solved the "ideal" way
when a work is queued when it is needed. Instead, many kthreads will
use self-queuing works that will monitor the state and wait for
the job inside the work. It means that we will need to wakeup
the currently processing job when the worker is going to be
destroyed. This is where this function will be useful.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/kthread.h | 1 +
kernel/kthread.c | 25 +++++++++++++++++++++++++
2 files changed, 26 insertions(+)
diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index a0b811c95c75..24d72bac27db 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -138,5 +138,6 @@ void flush_kthread_work(struct kthread_work *work);
void flush_kthread_worker(struct kthread_worker *worker);
void destroy_kthread_worker(struct kthread_worker *worker);
+void wakeup_and_destroy_kthread_worker(struct kthread_worker *worker);
#endif /* _LINUX_KTHREAD_H */
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 4f6b20710eb3..053c9dfa58ac 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -875,3 +875,28 @@ void destroy_kthread_worker(struct kthread_worker *worker)
WARN_ON(kthread_stop(task));
}
EXPORT_SYMBOL(destroy_kthread_worker);
+
+/**
+ * wakeup_and_destroy_kthread_worker - wake up and destroy a kthread worker
+ * @worker: worker to be destroyed
+ *
+ * Wakeup potentially sleeping work and destroy the @worker. All users should
+ * be aware that they should not produce more work anymore. It is especially
+ * useful for self-queuing works that are waiting for some job inside the work.
+ * They are supposed to wake up, check the situation, and stop re-queuing.
+ */
+void wakeup_and_destroy_kthread_worker(struct kthread_worker *worker)
+{
+ struct task_struct *task = worker->task;
+
+ if (WARN_ON(!task))
+ return;
+
+ spin_lock_irq(&worker->lock);
+ if (worker->current_work)
+ wake_up_process(worker->task);
+ spin_unlock_irq(&worker->lock);
+
+ destroy_kthread_worker(worker);
+}
+EXPORT_SYMBOL(wakeup_and_destroy_kthread_worker);
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [RFC PATCH 04/14] kthread: Add destroy_kthread_worker()
From: Petr Mladek @ 2015-07-28 14:39 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra
Cc: Steven Rostedt, Paul E. McKenney, Josh Triplett, Thomas Gleixner,
Linus Torvalds, Jiri Kosina, Borislav Petkov, Michal Hocko,
linux-mm, Vlastimil Babka, live-patching, linux-api, linux-kernel,
Petr Mladek
In-Reply-To: <1438094371-8326-1-git-send-email-pmladek@suse.com>
The current kthread worker users call flush() and stop() explicitly.
The new function will make it easier and will do it better.
Note that flush() does not guarantee that the queue is empty. drain()
is more safe. It returns when the queue is empty. Also is causes
that queue() ignores unexpected works and warns about it.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/kthread.h | 2 ++
kernel/kthread.c | 20 ++++++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index 974d70193907..a0b811c95c75 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -137,4 +137,6 @@ bool queue_kthread_work(struct kthread_worker *worker,
void flush_kthread_work(struct kthread_work *work);
void flush_kthread_worker(struct kthread_worker *worker);
+void destroy_kthread_worker(struct kthread_worker *worker);
+
#endif /* _LINUX_KTHREAD_H */
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 872f17e383c4..4f6b20710eb3 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -855,3 +855,23 @@ void drain_kthread_worker(struct kthread_worker *worker)
spin_unlock_irq(&worker->lock);
}
EXPORT_SYMBOL(drain_kthread_worker);
+
+/**
+ * destroy_kthread_worker - destroy a kthread worker
+ * @worker: worker to be destroyed
+ *
+ * Destroy @worker. It should be idle when this is called.
+ */
+void destroy_kthread_worker(struct kthread_worker *worker)
+{
+ struct task_struct *task;
+
+ task = worker->task;
+ if (WARN_ON(!task))
+ return;
+
+ drain_kthread_worker(worker);
+
+ WARN_ON(kthread_stop(task));
+}
+EXPORT_SYMBOL(destroy_kthread_worker);
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [RFC PATCH 03/14] kthread: Add drain_kthread_worker()
From: Petr Mladek @ 2015-07-28 14:39 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra
Cc: Steven Rostedt, Paul E. McKenney, Josh Triplett, Thomas Gleixner,
Linus Torvalds, Jiri Kosina, Borislav Petkov, Michal Hocko,
linux-mm, Vlastimil Babka, live-patching, linux-api, linux-kernel,
Petr Mladek
In-Reply-To: <1438094371-8326-1-git-send-email-pmladek@suse.com>
flush_kthread_worker() returns when the currently queued works are proceed.
But some other works might have been queued in the meantime.
This patch adds drain_kthread_work() that is inspired by drain_workqueue().
It returns when the queue is completely empty. Also it affects the behavior
of queue_kthread_work(). Only currently running work is allowed to queue
another work when the draining is in progress. A warning is printed when
some work is being queued from other context or when the draining takes
too long.
Note that drain() will typically be called when the queue should stay
empty, e.g. when the worker is going to be destroyed. In this case,
the caller should block all users from producing more work. This is
why the warning is printed. But some more works might be needed
to proceed the already existing works. This is why re-queuing
is allowed.
Callers also have to block existing works from an infinite re-queuing.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/kthread.h | 1 +
kernel/kthread.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 117 insertions(+), 5 deletions(-)
diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index fc8a7d253c40..974d70193907 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -68,6 +68,7 @@ struct kthread_worker {
struct list_head work_list;
struct task_struct *task;
struct kthread_work *current_work;
+ int nr_drainers;
};
struct kthread_work {
diff --git a/kernel/kthread.c b/kernel/kthread.c
index fe9421728f76..872f17e383c4 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -51,6 +51,7 @@ enum KTHREAD_BITS {
KTHREAD_SHOULD_STOP,
KTHREAD_SHOULD_PARK,
KTHREAD_IS_PARKED,
+ KTHREAD_IS_WORKER,
};
#define __to_kthread(vfork) \
@@ -538,6 +539,7 @@ void __init_kthread_worker(struct kthread_worker *worker,
lockdep_set_class_and_name(&worker->lock, key, name);
INIT_LIST_HEAD(&worker->work_list);
worker->task = NULL;
+ worker->nr_drainers = 0;
}
EXPORT_SYMBOL_GPL(__init_kthread_worker);
@@ -613,6 +615,7 @@ int create_kthread_worker_on_node(struct kthread_worker *worker,
const char namefmt[], ...)
{
struct task_struct *task;
+ struct kthread *kthread;
va_list args;
if (worker->task)
@@ -626,6 +629,9 @@ int create_kthread_worker_on_node(struct kthread_worker *worker,
if (IS_ERR(task))
return PTR_ERR(task);
+ kthread = to_kthread(task);
+ set_bit(KTHREAD_IS_WORKER, &kthread->flags);
+
spin_lock_irq(&worker->lock);
worker->task = task;
spin_unlock_irq(&worker->lock);
@@ -649,6 +655,56 @@ static void insert_kthread_work(struct kthread_worker *worker,
wake_up_process(worker->task);
}
+/*
+ * Queue @work without the check for drainers.
+ * Must be called under @worker->lock.
+ */
+static bool __queue_kthread_work(struct kthread_worker *worker,
+ struct kthread_work *work)
+{
+ lockdep_assert_held(&worker->lock);
+
+ if (list_empty(&work->node)) {
+ insert_kthread_work(worker, work, &worker->work_list);
+ return true;
+ }
+
+ return false;
+}
+
+/* return struct kthread_worker if %current is a kthread worker */
+static struct kthread_worker *current_kthread_worker(void)
+{
+ struct kthread *k;
+
+ if (!(current->flags & PF_KTHREAD))
+ goto fail;
+
+ k = to_kthread(current);
+ if (test_bit(KTHREAD_IS_WORKER, &k->flags))
+ return k->data;
+
+fail:
+ return NULL;
+}
+
+
+/*
+ * Test whether @work is being queued from another work
+ * executing on the same kthread.
+ */
+static bool is_chained_work(struct kthread_worker *worker)
+{
+ struct kthread_worker *current_worker;
+
+ current_worker = current_kthread_worker();
+ /*
+ * Return %true if I'm a kthread worker executing a work item on
+ * the given @worker.
+ */
+ return current_worker && current_worker == worker;
+}
+
/**
* queue_kthread_work - queue a kthread_work
* @worker: target kthread_worker
@@ -665,10 +721,14 @@ bool queue_kthread_work(struct kthread_worker *worker,
unsigned long flags;
spin_lock_irqsave(&worker->lock, flags);
- if (list_empty(&work->node)) {
- insert_kthread_work(worker, work, &worker->work_list);
- ret = true;
- }
+
+ /* if draining, only works from the same kthread worker are allowed */
+ if (unlikely(worker->nr_drainers) &&
+ WARN_ON_ONCE(!is_chained_work(worker)))
+ goto fail;
+
+ ret = __queue_kthread_work(worker, work);
+fail:
spin_unlock_irqrestore(&worker->lock, flags);
return ret;
}
@@ -740,7 +800,58 @@ void flush_kthread_worker(struct kthread_worker *worker)
COMPLETION_INITIALIZER_ONSTACK(fwork.done),
};
- queue_kthread_work(worker, &fwork.work);
+ /* flush() is and can be used when draining */
+ spin_lock_irq(&worker->lock);
+ __queue_kthread_work(worker, &fwork.work);
+ spin_unlock_irq(&worker->lock);
+
wait_for_completion(&fwork.done);
}
EXPORT_SYMBOL_GPL(flush_kthread_worker);
+
+/**
+ * drain_kthread_worker - drain a kthread worker
+ * @worker: worker to be drained
+ *
+ * Wait until there is none work queued for the given kthread worker.
+ * Only currently running work on @worker can queue further work items
+ * on it. @worker is flushed repeatedly until it becomes empty.
+ * The number of flushing is determined by the depth of chaining
+ * and should be relatively short. Whine if it takes too long.
+ *
+ * The caller is responsible for blocking all existing works
+ * from an infinite re-queuing!
+ *
+ * Also the caller is responsible for blocking all the kthread
+ * worker users from queuing any new work. It is especially
+ * important if the queue has to stay empty once this function
+ * finishes.
+ */
+void drain_kthread_worker(struct kthread_worker *worker)
+{
+ int flush_cnt = 0;
+
+ spin_lock_irq(&worker->lock);
+ worker->nr_drainers++;
+
+ while (!list_empty(&worker->work_list)) {
+ /*
+ * Unlock, so we could move forward. Note that queuing
+ * is limited by @nr_drainers > 0.
+ */
+ spin_unlock_irq(&worker->lock);
+
+ flush_kthread_worker(worker);
+
+ if (++flush_cnt == 10 ||
+ (flush_cnt % 100 == 0 && flush_cnt <= 1000))
+ pr_warn("kthread worker %s: drain_kthread_worker() isn't complete after %u tries\n",
+ worker->task->comm, flush_cnt);
+
+ spin_lock_irq(&worker->lock);
+ }
+
+ worker->nr_drainers--;
+ spin_unlock_irq(&worker->lock);
+}
+EXPORT_SYMBOL(drain_kthread_worker);
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [RFC PATCH 02/14] kthread: Add create_kthread_worker*()
From: Petr Mladek @ 2015-07-28 14:39 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra
Cc: Steven Rostedt, Paul E. McKenney, Josh Triplett, Thomas Gleixner,
Linus Torvalds, Jiri Kosina, Borislav Petkov, Michal Hocko,
linux-mm, Vlastimil Babka, live-patching, linux-api, linux-kernel,
Petr Mladek
In-Reply-To: <1438094371-8326-1-git-send-email-pmladek@suse.com>
Kthread workers are currently created using the classic kthread API,
namely kthread_run(). kthread_worker_fn() is passed as the @threadfn
parameter.
This patch defines create_kthread_worker_on_node() and
create_kthread_worker() functions that hide implementation details.
It enforces using kthread_worker_fn() for the main thread. But I doubt
that there are any plans to create any alternative. In fact, I think
that we do not want any alternative main thread because it would be
hard to support consistency with the rest of the kthread worker API.
The naming is inspired by the workqueues API like the reset of the
kthread worker API.
This patch does _not_ convert existing kthread workers. The kthread worker
API need more improvements first, e.g. a function to destroy the worker.
We should not need to access @worker->task and other struct kthread_worker
members directly.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
include/linux/kthread.h | 8 ++++++++
kernel/kthread.c | 43 ++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 50 insertions(+), 1 deletion(-)
diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index 13d55206ccf6..fc8a7d253c40 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -123,6 +123,14 @@ extern void __init_kthread_worker(struct kthread_worker *worker,
int kthread_worker_fn(void *worker_ptr);
+__printf(3, 4)
+int create_kthread_worker_on_node(struct kthread_worker *worker,
+ int node,
+ const char namefmt[], ...);
+
+#define create_kthread_worker(worker, namefmt, arg...) \
+ create_kthread_worker_on_node(worker, -1, namefmt, ##arg)
+
bool queue_kthread_work(struct kthread_worker *worker,
struct kthread_work *work);
void flush_kthread_work(struct kthread_work *work);
diff --git a/kernel/kthread.c b/kernel/kthread.c
index fca7cd124512..fe9421728f76 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -561,7 +561,11 @@ int kthread_worker_fn(void *worker_ptr)
struct kthread_worker *worker = worker_ptr;
struct kthread_work *work;
- WARN_ON(worker->task);
+ /*
+ * FIXME: Update the check and remove the assignment when all kthread
+ * worker users are created using create_kthread_worker*() functions.
+ */
+ WARN_ON(worker->task && worker->task != current);
worker->task = current;
repeat:
set_current_state(TASK_INTERRUPTIBLE); /* mb paired w/ kthread_stop */
@@ -595,6 +599,43 @@ repeat:
}
EXPORT_SYMBOL_GPL(kthread_worker_fn);
+/**
+ * create_kthread_worker_on_node - create a kthread worker.
+ * @worker: initialized kthread worker struct.
+ * @node: memory node number.
+ * @namefmt: printf-style name for the kthread worker (task).
+ *
+ * If the worker is going to be bound on a particular CPU, give its node
+ * in @node, to get NUMA affinity for kthread stack, or else give -1.
+ */
+int create_kthread_worker_on_node(struct kthread_worker *worker,
+ int node,
+ const char namefmt[], ...)
+{
+ struct task_struct *task;
+ va_list args;
+
+ if (worker->task)
+ return -EINVAL;
+
+ va_start(args, namefmt);
+ task = __kthread_create_on_node(kthread_worker_fn, worker, node,
+ namefmt, args);
+ va_end(args);
+
+ if (IS_ERR(task))
+ return PTR_ERR(task);
+
+ spin_lock_irq(&worker->lock);
+ worker->task = task;
+ spin_unlock_irq(&worker->lock);
+
+ wake_up_process(task);
+
+ return 0;
+}
+EXPORT_SYMBOL(create_kthread_worker_on_node);
+
/* insert @work before @pos in @worker */
static void insert_kthread_work(struct kthread_worker *worker,
struct kthread_work *work,
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [RFC PATCH 01/14] kthread: Allow to call __kthread_create_on_node() with va_list args
From: Petr Mladek @ 2015-07-28 14:39 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra
Cc: Steven Rostedt, Paul E. McKenney, Josh Triplett, Thomas Gleixner,
Linus Torvalds, Jiri Kosina, Borislav Petkov, Michal Hocko,
linux-mm, Vlastimil Babka, live-patching, linux-api, linux-kernel,
Petr Mladek
In-Reply-To: <1438094371-8326-1-git-send-email-pmladek@suse.com>
kthread_create_on_node() implements a bunch of logic to create
the kthread. It is already called by kthread_create_on_cpu().
We are going to add a new API that will allow to standardize kthreads
and define safe points for termination, freezing, parking, and even
signal handling. It will want to call kthread_create_on_node()
with va_list args.
This patch does only a refactoring and does not modify the existing
behavior.
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
kernel/kthread.c | 71 +++++++++++++++++++++++++++++++++-----------------------
1 file changed, 42 insertions(+), 29 deletions(-)
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 10e489c448fe..fca7cd124512 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -242,32 +242,10 @@ static void create_kthread(struct kthread_create_info *create)
}
}
-/**
- * kthread_create_on_node - create a kthread.
- * @threadfn: the function to run until signal_pending(current).
- * @data: data ptr for @threadfn.
- * @node: memory node number.
- * @namefmt: printf-style name for the thread.
- *
- * Description: This helper function creates and names a kernel
- * thread. The thread will be stopped: use wake_up_process() to start
- * it. See also kthread_run().
- *
- * If thread is going to be bound on a particular cpu, give its node
- * in @node, to get NUMA affinity for kthread stack, or else give -1.
- * When woken, the thread will run @threadfn() with @data as its
- * argument. @threadfn() can either call do_exit() directly if it is a
- * standalone thread for which no one will call kthread_stop(), or
- * return when 'kthread_should_stop()' is true (which means
- * kthread_stop() has been called). The return value should be zero
- * or a negative error number; it will be passed to kthread_stop().
- *
- * Returns a task_struct or ERR_PTR(-ENOMEM) or ERR_PTR(-EINTR).
- */
-struct task_struct *kthread_create_on_node(int (*threadfn)(void *data),
- void *data, int node,
- const char namefmt[],
- ...)
+static struct task_struct *__kthread_create_on_node(int (*threadfn)(void *data),
+ void *data, int node,
+ const char namefmt[],
+ va_list args)
{
DECLARE_COMPLETION_ONSTACK(done);
struct task_struct *task;
@@ -308,11 +286,8 @@ struct task_struct *kthread_create_on_node(int (*threadfn)(void *data),
task = create->result;
if (!IS_ERR(task)) {
static const struct sched_param param = { .sched_priority = 0 };
- va_list args;
- va_start(args, namefmt);
vsnprintf(task->comm, sizeof(task->comm), namefmt, args);
- va_end(args);
/*
* root may have changed our (kthreadd's) priority or CPU mask.
* The kernel thread should not inherit these properties.
@@ -323,6 +298,44 @@ struct task_struct *kthread_create_on_node(int (*threadfn)(void *data),
kfree(create);
return task;
}
+
+
+/**
+ * kthread_create_on_node - create a kthread.
+ * @threadfn: the function to run until signal_pending(current).
+ * @data: data ptr for @threadfn.
+ * @node: memory node number.
+ * @namefmt: printf-style name for the thread.
+ *
+ * Description: This helper function creates and names a kernel
+ * thread. The thread will be stopped: use wake_up_process() to start
+ * it. See also kthread_run().
+ *
+ * If thread is going to be bound on a particular cpu, give its node
+ * in @node, to get NUMA affinity for kthread stack, or else give -1.
+ * When woken, the thread will run @threadfn() with @data as its
+ * argument. @threadfn() can either call do_exit() directly if it is a
+ * standalone thread for which no one will call kthread_stop(), or
+ * return when 'kthread_should_stop()' is true (which means
+ * kthread_stop() has been called). The return value should be zero
+ * or a negative error number; it will be passed to kthread_stop().
+ *
+ * Returns a task_struct or ERR_PTR(-ENOMEM) or ERR_PTR(-EINTR).
+ */
+struct task_struct *kthread_create_on_node(int (*threadfn)(void *data),
+ void *data, int node,
+ const char namefmt[],
+ ...)
+{
+ struct task_struct *task;
+ va_list args;
+
+ va_start(args, namefmt);
+ task = __kthread_create_on_node(threadfn, data, node, namefmt, args);
+ va_end(args);
+
+ return task;
+}
EXPORT_SYMBOL(kthread_create_on_node);
static void __kthread_bind(struct task_struct *p, unsigned int cpu, long state)
--
1.8.5.6
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related
* [RFC PATCH 00/14] kthread: Use kthread worker API more widely
From: Petr Mladek @ 2015-07-28 14:39 UTC (permalink / raw)
To: Andrew Morton, Oleg Nesterov, Tejun Heo, Ingo Molnar,
Peter Zijlstra
Cc: Steven Rostedt, Paul E. McKenney, Josh Triplett, Thomas Gleixner,
Linus Torvalds, Jiri Kosina, Borislav Petkov, Michal Hocko,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Vlastimil Babka,
live-patching-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Petr Mladek
Kthreads are currently implemented as an infinite loop. Each
has its own variant of checks for terminating, freezing,
awakening. Sometimes, it is hard to say if they are done
correctly. They are also harder to maintain if there is
a generic problem found in the area.
I have proposed a so-called kthread iterant API to improve the situation,
see https://lkml.org/lkml/2015/6/5/555. The RFC opened and/or answered
several questions.
This RFC is reaction on Tejun's suggestion to use the existing kthread
worker API instead of a new one, see https://lkml.org/lkml/2015/6/9/77.
I wanted to give it a try.
Structure of this patch set:
----------------------------
1st..6th patches: improve the existing kthread worker API
7th, 8th, 11th patches: converts three kthreads into the new API,
namely: RCU gp kthreas, khugepaged, my favorite ring buffer
benchmark
12th..14th patches: show how we could further improve the API
9th, 10th patches: do some further clean up of the ring buffer
benchmark; they allow easier conversion into the new API;
but they might be applied independently
TODO:
-----
If people like the kthread worker API, it will need more love.
The following ideas come to my mind:
+ allow to pass void *data via struct kthread_work;
+ hide struct kthread_worker in kthread.c and make the API
more safe
+ allow to cancel work
I have tested this patches against today's Linux tree, aka 4.2.0-rc4+.
Petr Mladek (14):
kthread: Allow to call __kthread_create_on_node() with va_list args
kthread: Add create_kthread_worker*()
kthread: Add drain_kthread_worker()
kthread: Add destroy_kthread_worker()
kthread: Add wakeup_and_destroy_kthread_worker()
kthread: Add kthread_worker_created()
mm/huge_page: Convert khugepaged() into kthread worker API
rcu: Convert RCU gp kthreads into kthread worker API
ring_buffer: Initialize completions statically in the benchmark
ring_buffer: Fix more races when terminating the producer in the
benchmark
ring_buffer: Use kthread worker API for the producer kthread in the
benchmark
kthread_worker: Better support freezable kthread workers
kthread_worker: Add set_kthread_worker_user_nice()
kthread_worker: Add set_kthread_worker_scheduler*()
include/linux/kthread.h | 29 +++
kernel/kthread.c | 359 +++++++++++++++++++++++++++++++----
kernel/rcu/tree.c | 182 +++++++++---------
kernel/rcu/tree.h | 4 +-
kernel/trace/ring_buffer_benchmark.c | 150 ++++++++-------
mm/huge_memory.c | 83 ++++----
6 files changed, 584 insertions(+), 223 deletions(-)
--
1.8.5.6
^ permalink raw reply
* Re: [PATCH RFC 0/1] Document how to add a new syscall
From: Peter Zijlstra @ 2015-07-28 14:19 UTC (permalink / raw)
To: Shuah Khan
Cc: David Drysdale, linux-api-u79uwXL29TY76Z2rM5mHXA, Michael Kerrisk,
Andrew Morton, Arnd Bergmann, Jonathan Corbet, Andrea Arcangeli,
Thomas Gleixner, Eric B Munson, Ingo Molnar, H. Peter Anvin,
Oleg Nesterov, Linus Torvalds, Greg Kroah-Hartman,
Andy Lutomirski, Al Viro, Rusty Russell, Vivek Goyal,
Alexei Starovoitov, David Herrmann, Theodore Ts'o, Kees Cook,
Miklos Szeredi, Milosz Tanski
In-Reply-To: <55B78AB4.2010807-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
On Tue, Jul 28, 2015 at 07:59:16AM -0600, Shuah Khan wrote:
> On 07/28/2015 05:41 AM, David Drysdale wrote:
> > Given that I've gotten some of the details wrong in the past (and I've
> > seen others do likewise), I thought it might be helpful to collate the
> > best practices for adding a new system call to the kernel.
> >
> > Apologies for the wide circulation -- I've tried to include folk who've
> > recently added or proposed a system call, as they're most likely to
> > have opinions on:
> > - whether this a useful addition to Documentation/
> > - whether the details of the advice are correct and complete.
> >
> > Shuah, is there anything more that should be added for the Testing
> > section in particular?
> >
>
> David,
>
> Looks good. You could add a one liner on in some cases, it might be
> necessary to install headers before test compiles. Other than that,
> the information looks good.
For perf we still use the syscall() wrapper and we have hardcoded
(fallback) syscall numbers in there in case they've not yet reached
unistd.h.
People update their kernels far more often than their userspace headers.
^ permalink raw reply
* Re: [PATCH RFC 1/1] Documentation: describe how to add a system call
From: Eric B Munson @ 2015-07-28 14:14 UTC (permalink / raw)
To: David Drysdale
Cc: linux-api, Michael Kerrisk, Andrew Morton, Arnd Bergmann,
Shuah Khan, Jonathan Corbet, Andrea Arcangeli, Thomas Gleixner,
Ingo Molnar, H. Peter Anvin, Oleg Nesterov, Linus Torvalds,
Greg Kroah-Hartman, Andy Lutomirski, Al Viro, Rusty Russell,
Peter Zijlstra, Vivek Goyal, Alexei Starovoitov, David Herrmann,
Theodore Ts'o, Kees Cook, Miklos Szeredi,
Milosz Tanski <milos>
In-Reply-To: <1438083663-24814-2-git-send-email-drysdale@google.com>
[-- Attachment #1: Type: text/plain, Size: 449 bytes --]
On Tue, 28 Jul 2015, David Drysdale wrote:
> Add a document describing the process of adding a new system call,
> including the need for a flags argument for future compatibility, and
> covering 32-bit/64-bit concerns (albeit in an x86-centric way).
>
> Signed-off-by: David Drysdale <drysdale@google.com>
> Reviewed-by: Michael Kerrisk <mtk.manpages@gmail.com>
This is really helpful
Reviewed-by: Eric B Munson <emunson@akamai.com>
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH RFC 0/1] Document how to add a new syscall
From: Shuah Khan @ 2015-07-28 13:59 UTC (permalink / raw)
To: David Drysdale, linux-api-u79uwXL29TY76Z2rM5mHXA, Michael Kerrisk,
Andrew Morton, Arnd Bergmann, Jonathan Corbet
Cc: Andrea Arcangeli, Thomas Gleixner, Eric B Munson, Ingo Molnar,
H. Peter Anvin, Oleg Nesterov, Linus Torvalds, Greg Kroah-Hartman,
Andy Lutomirski, Al Viro, Rusty Russell, Peter Zijlstra,
Vivek Goyal, Alexei Starovoitov, David Herrmann,
Theodore Ts'o, Kees Cook, Miklos Szeredi, Milosz Tanski,
Fam Zheng, Josh Triplett, Mathieu Desnoyers,
linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-kernel-fy+rA21nqHI
In-Reply-To: <1438083663-24814-1-git-send-email-drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
On 07/28/2015 05:41 AM, David Drysdale wrote:
> Given that I've gotten some of the details wrong in the past (and I've
> seen others do likewise), I thought it might be helpful to collate the
> best practices for adding a new system call to the kernel.
>
> Apologies for the wide circulation -- I've tried to include folk who've
> recently added or proposed a system call, as they're most likely to
> have opinions on:
> - whether this a useful addition to Documentation/
> - whether the details of the advice are correct and complete.
>
> Shuah, is there anything more that should be added for the Testing
> section in particular?
>
David,
Looks good. You could add a one liner on in some cases, it might be
necessary to install headers before test compiles. Other than that,
the information looks good.
thanks,
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978
^ permalink raw reply
* Re: [PATCH V5 0/7] Allow user to request memory to be locked on page fault
From: Eric B Munson @ 2015-07-28 13:49 UTC (permalink / raw)
To: Michal Hocko
Cc: Vlastimil Babka, Andrew Morton, Shuah Khan, Michael Kerrisk,
Jonathan Corbet, Ralf Baechle, linux-alpha, linux-kernel,
linux-mips, linux-parisc, linuxppc-dev, sparclinux, linux-xtensa,
linux-mm, linux-arch, linux-api
In-Reply-To: <20150728111725.GG24972@dhcp22.suse.cz>
[-- Attachment #1: Type: text/plain, Size: 2976 bytes --]
On Tue, 28 Jul 2015, Michal Hocko wrote:
> [I am sorry but I didn't get to this sooner.]
>
> On Mon 27-07-15 10:54:09, Eric B Munson wrote:
> > Now that VM_LOCKONFAULT is a modifier to VM_LOCKED and
> > cannot be specified independentally, it might make more sense to mirror
> > that relationship to userspace. Which would lead to soemthing like the
> > following:
>
> A modifier makes more sense.
>
> > To lock and populate a region:
> > mlock2(start, len, 0);
> >
> > To lock on fault a region:
> > mlock2(start, len, MLOCK_ONFAULT);
> >
> > If LOCKONFAULT is seen as a modifier to mlock, then having the flags
> > argument as 0 mean do mlock classic makes more sense to me.
> >
> > To mlock current on fault only:
> > mlockall(MCL_CURRENT | MCL_ONFAULT);
> >
> > To mlock future on fault only:
> > mlockall(MCL_FUTURE | MCL_ONFAULT);
> >
> > To lock everything on fault:
> > mlockall(MCL_CURRENT | MCL_FUTURE | MCL_ONFAULT);
>
> Makes sense to me. The only remaining and still tricky part would be
> the munlock{all}(flags) behavior. What should munlock(MLOCK_ONFAULT)
> do? Keep locked and poppulate the range or simply ignore the flag an
> just unlock?
>
> I can see some sense to allow munlockall(MCL_FUTURE[|MLOCK_ONFAULT]),
> munlockall(MCL_CURRENT) resp. munlockall(MCL_CURRENT|MCL_FUTURE) but
> other combinations sound weird to me.
>
> Anyway munlock with flags opens new doors of trickiness.
In the current revision there are no new munlock[all] system calls
introduced. munlockall() unconditionally cleared both MCL_CURRENT and
MCL_FUTURE before the set and now unconditionally clears all three.
munlock() does the same for VM_LOCK and VM_LOCKONFAULT. If the user
wants to adjust mlockall flags today, they need to call mlockall a
second time with the new flags, this remains true for mlockall after
this set and the same behavior is mirrored in mlock2. The only
remaining question I have is should we have 2 new mlockall flags so that
the caller can explicitly set VM_LOCKONFAULT in the mm->def_flags vs
locking all current VMAs on fault. I ask because if the user wants to
lock all current VMAs the old way, but all future VMAs on fault they
have to call mlockall() twice:
mlockall(MCL_CURRENT);
mlockall(MCL_CURRENT | MCL_FUTURE | MCL_ONFAULT);
This has the side effect of converting all the current VMAs to
VM_LOCKONFAULT, but because they were all made present and locked in the
first call, this should not matter in most cases. The catch is that,
like mmap(MAP_LOCKED), mlockall() does not communicate if mm_populate()
fails. This has been true of mlockall() from the beginning so I don't
know if it needs more than an entry in the man page to clarify (which I
will add when I add documentation for MCL_ONFAULT). In a much less
likely corner case, it is not possible in the current setup to request
all current VMAs be VM_LOCKONFAULT and all future be VM_LOCKED.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH RFC 0/1] UAPI,x86: export syscall numbers for all x86 archs
From: Paul Moore @ 2015-07-28 12:20 UTC (permalink / raw)
To: David Drysdale
Cc: x86-DgEjT+Ai2ygdnm+yROfE0A, Thomas Gleixner, Ingo Molnar,
Andy Lutomirski, H. Peter Anvin, Michael Kerrisk, Kees Cook,
Eric Paris, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1438070731-17764-1-git-send-email-drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
On Tue, Jul 28, 2015 at 4:05 AM, David Drysdale <drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> wrote:
> A while ago I was trying to build a seccomp-bpf filter program that would
> survive a change of x86 architecture. This was complicated for all sorts of
> reasons, but one of the problems was that the different syscall numbers aren't
> all available at the same time -- hence this patch.
Or just use libseccomp and let it take care of all the different ABI
specific warts for you. The library handles the undefined syscalls
you describe, but also multiplexed syscalls (e.g. socket related
syscalls on x86) and proper invalid arch/ABI filtering (you are
filtering x32 correctly on x86-64 right?).
* https://github.com/seccomp/libseccomp
--
paul moore
www.paul-moore.com
^ permalink raw reply
* [PATCH RFC 1/1] Documentation: describe how to add a system call
From: David Drysdale @ 2015-07-28 11:41 UTC (permalink / raw)
To: linux-api-u79uwXL29TY76Z2rM5mHXA, Michael Kerrisk, Andrew Morton,
Arnd Bergmann, Shuah Khan, Jonathan Corbet
Cc: Andrea Arcangeli, Thomas Gleixner, Eric B Munson, Ingo Molnar,
H. Peter Anvin, Oleg Nesterov, Linus Torvalds, Greg Kroah-Hartman,
Andy Lutomirski, Al Viro, Rusty Russell, Peter Zijlstra,
Vivek Goyal, Alexei Starovoitov, David Herrmann,
Theodore Ts'o, Kees Cook, Miklos Szeredi, Milosz Tanski,
Fam Zheng, Josh Triplett, Mathieu Desnoyers,
linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-kernel-fy+rA21nqHI
In-Reply-To: <1438083663-24814-1-git-send-email-drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Add a document describing the process of adding a new system call,
including the need for a flags argument for future compatibility, and
covering 32-bit/64-bit concerns (albeit in an x86-centric way).
Signed-off-by: David Drysdale <drysdale-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Michael Kerrisk <mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
Documentation/adding-syscalls.txt | 454 ++++++++++++++++++++++++++++++++++++++
1 file changed, 454 insertions(+)
create mode 100644 Documentation/adding-syscalls.txt
diff --git a/Documentation/adding-syscalls.txt b/Documentation/adding-syscalls.txt
new file mode 100644
index 000000000000..5f52edda8951
--- /dev/null
+++ b/Documentation/adding-syscalls.txt
@@ -0,0 +1,454 @@
+Adding a New System Call
+========================
+
+This document describes what's involved in adding a new system call to the
+Linux kernel, over and above the normal submission advice in
+Documentation/SubmittingPatches.
+
+
+System Call Alternatives
+------------------------
+
+The first thing to consider when adding a new system call is whether one of
+the alternatives might be suitable instead. Although system calls are the
+most traditional and most obvious interaction points between userspace and the
+kernel, there are other possibilities -- choose what fits best for your
+interface.
+
+ - If the operations involved can be made to look like a filesystem-like
+ object, it may make more sense to create a new filesystem or device. This
+ also makes it easier to encapsulate the new functionality in a kernel module
+ rather than requiring it to be built into the main kernel.
+ - If the new functionality involves operations where the kernel notifies
+ userspace that something has happened, then returning a new file
+ descriptor for the relevant object allows userspace to use
+ poll/select/epoll to receive that notification.
+ - However, operations that don't map to read(2)/write(2)-like operations
+ have to be implemented as ioctl(2) requests, which can lead to a
+ somewhat opaque API.
+ - If you're just exposing runtime system information, a new node in sysfs
+ (see Documentation/filesystems/sysfs.txt) or the /proc filesystem may be
+ more appropriate. However, access to these mechanisms requires that the
+ relevant filesystem is mounted, which might not always be the case (e.g.
+ in a namespaced/sandboxed/chrooted environment).
+ - If the operation is specific to a particular file or file descriptor, then
+ an additional fcntl(2) command option may be more appropriate. However,
+ fcntl(2) is a multiplexing system call that hides a lot of complexity, so
+ this option is best for when the new function is closely analogous to
+ existing fcntl(2) functionality, or the new functionality is very simple
+ (for example, getting/setting a simple flag related to a file descriptor).
+ - If the operation is specific to a particular task or process, then an
+ additional prctl(2) command option may be more appropriate. As with
+ fcntl(2), this system call is a complicated multiplexor so is best reserved
+ for near-analogs of existing prctl() commands or getting/setting a simple
+ flag related to a process.
+
+
+Designing the API
+-----------------
+
+A new system call forms part of the API of the kernel, and has to be supported
+indefinitely. As such, it's a very good idea to explicitly discuss the
+interface on the kernel mailing list, and to plan for future extensions of the
+interface. In particular:
+
+ **Include a flags argument for every new system call**
+
+The syscall table is littered with historical examples where this wasn't done,
+together with the corresponding follow-up system calls (eventfd/eventfd2,
+dup2/dup3, inotify_init/inotify_init1, pipe/pipe2, renameat/renameat2), so
+learn from the history of the kernel and include a flags argument from the
+start.
+
+Also, to make sure that userspace programs can safely use flags between kernel
+versions, check whether the flags value holds any unknown flags, and reject the
+sycall (with EINVAL) if it does:
+
+ if (flags & ~(THING_FLAG1 | THING_FLAG2 | THING_FLAG3))
+ return -EINVAL;
+
+If your new xyzzy(2) system call returns a new file descriptor, then the flags
+argument should include a value that is equivalent to setting O_CLOEXEC on the
+new FD. This makes it possible for userspace to close the timing window
+between xyzzy() and calling fcntl(fd, F_SETFD, FD_CLOEXEC), where an
+unexpected fork() and execve() in another thread could leak a descriptor to
+the exec'ed program. (However, resist the temptation to re-use the actual value
+of the O_CLOEXEC constant, as it is architecture-specific and is part of a
+numbering space of O_* flags that is fairly full.)
+
+If your new xyzzy(2) system call involves a filename argument:
+
+ int sys_xyzzy(const char __user *path, ..., unsigned int flags);
+
+you should also consider whether an xyzzyat(2) version is more appropriate:
+
+ int sys_xyzzyat(int dfd, const char __user *path, ..., unsigned int flags);
+
+This allows more flexibility for how userspace specifies the file in question;
+in particular it allows userspace to request the functionality for an
+already-opened file descriptor using the AT_EMPTY_PATH flag, effectively giving
+an fxyzzy(3) operation for free:
+
+ - xyzzyat(AT_FDCWD, path, ..., 0) is equivalent to xyzzy(path,...)
+ - xyzzyat(fd, "", ..., AT_EMPTY_PATH) is equivalent to fxyzzy(fd, ...)
+
+(For more details on the rationale of the *at() calls, see the openat(2) man
+page; for an example of AT_EMPTY_PATH, see the statat(2) man page.)
+
+If your new xyzzy(2) system call involves a parameter describing an offset
+within a file, make its type loff_t so that 64-bit offsets can be supported
+even on 32-bit architectures.
+
+If your new xyzzy(2) system call involves administrative functionality, it
+needs to be governed by the appropriate Linux capability bit, as described in
+the capabilities(7) man page.
+
+ - If there is an existing capability that governs related functionality, then
+ use that. However, avoid combining lots of only vaguely related functions
+ together under the same bit, as this goes against capabilities' purpose of
+ splitting the power of root. In particular, avoid adding new uses of the
+ already overly-general CAP_SYS_ADMIN capability.
+ - If there is no related capability, then consider adding a new capability
+ bit -- but bear in mind that the numbering space is limited, and each new
+ bit needs to be understood and administered by sysadmins.
+
+Finally, be aware that some non-x86 architectures have an easier time if
+system call parameters that are explicitly 64-bit fall on odd-numbered
+arguments (i.e. parameter 1, 3, 5), to allow use of contiguous pairs of 32-bit
+registers.
+
+
+Proposing the API
+-----------------
+
+To make new system calls easy to review, it's best to divide up the patchset
+into separate chunks. These should include at least the following items as
+distinct commits (each of which is described further below):
+
+ - The core implementation of the system call together with prototypes, generic
+ numbering and fallback stub implementation.
+ - Wiring up of the new system call for one particular architecture, usually
+ x86 (including all of x86_64, x86_32 and x32).
+ - A demonstration of the use of the new system call in userspace via a
+ selftest.
+ - A draft man-page for the new system call.
+
+Changes to the kernel's API should always be cc'ed to linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+
+
+Generic System Call Implementation
+----------------------------------
+
+The main entry point for your new xyzzy(2) system call will be called
+sys_xyzzy(), but you add this entry point with the appropriate
+SYSCALL_DEFINEn() macro rather than explicitly. The 'n' indicates the number
+of arguments to the system call, and the macro takes the system call name
+followed by the (type, name) pairs for the parameters as arguments. Using
+this macro allows metadata about the new system call to be made available for
+other tools.
+
+The new entry point also needs a corresponding function prototype, in
+include/linux/syscalls.h, marked as asmlinkage to match the way that system
+calls are invoked:
+
+ asmlinkage long sys_xyzzy(...);
+
+Some architectures (e.g. x86) have their own architecture-specific syscall
+tables, but several other architectures share a generic syscall table. Add your
+new system call to the generic list by adding an entry to the list in
+include/uapi/asm-generic/unistd.h:
+
+ #define __NR_xyzzy 292
+ __SYSCALL(__NR_xyzzy, sys_xyzzy)
+
+Also update the __NR_syscalls count to reflect the additional system call, and
+note that if multiple new system calls are added in the same merge window,
+your new syscall number may get adjusted to resolve conflicts.
+
+The file kernel/sys_ni.c provides a fallback stub implementation of each system
+call, returning -ENOSYS. Add your new system call here too:
+
+ cond_syscall(sys_xyzzy);
+
+To summarize, you need a commit that includes:
+
+ - SYSCALL_DEFINEn(xyzzy, ...) for the entry point
+ - corresponding prototype in include/linux/syscalls.h
+ - generic table entry in include/uapi/asm-generic/unistd.h
+ - fallback stub in kernel/sys_ni.c
+
+
+x86 System Call Implementation
+------------------------------
+
+To wire up your new system call for x86 platforms, you need to update the
+master syscall tables. Assuming your new system call isn't special in some
+way (see below), this involves a "common" entry (for x86_64 and x32) in
+arch/x86/entry/syscalls/syscall_64.tbl:
+
+ 333 common xyzzy sys_xyzzy
+
+and an "i386" entry in arch/x86/entry/syscalls/syscall_32.tbl:
+
+ 380 i386 xyzzy sys_xyzzy
+
+Again, these numbers are liable to be changed if there are conflicts in the
+relevant merge window.
+
+
+Compatibility System Calls (Generic)
+------------------------------------
+
+For most system calls the same 64-bit implementation can be invoked even when
+the userspace program is itself 32-bit; even if the system call's parameters
+include an explicit pointer, this is handled transparently.
+
+However, there are a couple of situations where a compatibility layer is
+needed to cope with size differences between 32-bit and 64-bit.
+
+The first is if the 64-bit kernel also supports 32-bit userspace programs, and
+so needs to parse areas of (__user) memory that could hold either 32-bit or
+64-bit values. In particular, this is needed whenever a system call argument
+is:
+
+ - a pointer to a pointer
+ - a pointer to a struct containing a pointer (e.g. struct iovec __user *)
+ - a pointer to a varying sized integral type (time_t, off_t, long, ...)
+ - a pointer to a struct containing a varying sized integral type.
+
+The second situation that requires a compatibility layer is if one of the
+system call's arguments has a type that is explicitly 64-bit even on a 32-bit
+architecture, for example loff_t or __u64. In this case, a value that arrives
+at a 64-bit kernel from a 32-bit application will be split into two 32-bit
+values, which then need to be re-assembled in the compatibility layer.
+
+(Note that a system call argument that's a pointer to an explicit 64-bit type
+does *not* need a compatibility layer; for example, splice(2)'s arguments of
+type loff_t __user * do not trigger the need for a compat_ system call.)
+
+The compatibility version of the system call is called compat_sys_xyzzy(), and
+is added with the COMPAT_SYSCALL_DEFINEn() macro, analogously to
+SYSCALL_DEFINEn. This version of the implementation runs as part of a 64-bit
+kernel, but expects to receive 32-bit parameter values and does whatever is
+needed to deal with them. (Typically, the compat_sys_ version converts the
+values to 64-bit versions and either calls on to the sys_ version, or both of
+them call a common inner implementation function.)
+
+The compat entry point also needs a corresponding function prototype, in
+include/linux/compat.h, marked as asmlinkage to match the way that system
+calls are invoked:
+
+ asmlinkage long compat_sys_xyzzy(...);
+
+If the system call involves a structure that is laid out differently on 32-bit
+and 64-bit systems, say struct xyzzy_args, then the include/linux/compat.h
+header file should also include a compat version of the structure (struct
+compat_xyzzy_args) where each variable-size field has the appropriate compat_
+type that corresponds to the type in struct xyzzy_args. The
+compat_sys_xyzzy() routine can then use this compat_ structure to parse the
+arguments from a 32-bit invocation.
+
+For example, if there are fields:
+
+ struct xyzzy_args {
+ const char __user *ptr;
+ __kernel_long_t varying_val;
+ u64 fixed_val;
+ /* ... */
+ };
+
+in struct xyzzy_args, then struct compat_xyzzy_args would have:
+
+ struct compat_xyzzy_args {
+ compat_uptr_t ptr;
+ compat_long_t varying_val;
+ u64 fixed_val;
+ /* ... */
+ };
+
+The generic system call list also needs adjusting to allow for the compat
+version; the entry in include/uapi/asm-generic/unistd.h should use
+__SC_COMP rather than __SYSCALL:
+
+ #define __NR_xyzzy 292
+ __SC_COMP(__NR_xyzzy, sys_xyzzy, compat_sys_xyzzy)
+
+To summarize, you need:
+
+ - a COMPAT_SYSCALL_DEFINEn(xyzzy, ...) for the compat entry point
+ - corresponding prototype in include/linux/compat.h
+ - (if needed) 32-bit mapping struct in include/linux/compat.h
+ - instance of __SC_COMP not __SYSCALL in include/uapi/asm-generic/unistd.h
+
+
+Compatibility System Calls (x86)
+--------------------------------
+
+To wire up the x86 architecture of a system call with a compatibility version,
+the entries in the syscall tables need to be adjusted.
+
+First, the entry in arch/x86/entry/syscalls/syscall_32.tbl gets an extra
+column to indicate that a 32-bit userspace program running on a 64-bit kernel
+should hit the compat entry point:
+
+ 380 i386 xyzzy sys_xyzzy compat_sys_xyzzy
+
+Second, you need to figure out what should happen for the x32 ABI version of
+the new system call. There's a choice here: the layout of the arguments
+should either match the 64-bit version or the 32-bit version.
+
+If there's a pointer-to-a-pointer involved, the decision is easy: x32 is
+ILP32, so the layout should match the 32-bit version, and the entry in
+arch/x86/entry/syscalls/syscall_64.tbl is split so that x32 programs hit the
+compatibility wrapper:
+
+ 333 64 xyzzy sys_xyzzy
+ ...
+ 555 x32 xyzzy compat_sys_xyzzy
+
+If no pointers are involved, then it is preferable to re-use the 64-bit system
+call for the x32 ABI (and consequently the entry in
+arch/x86/entry/syscalls/syscall_64.tbl is unchanged).
+
+In either case, you should check that the types involved in your argument
+layout do indeed map exactly from x32 (-mx32) to either the 32-bit (-m32) or
+64-bit (-m64) equivalents.
+
+
+System Calls Returning Elsewhere
+--------------------------------
+
+For most system calls, once the system call is complete the user program
+continues exactly where it left off -- at the next instruction, with the same
+stack and registers as before the system call, and with the same virtual
+memory space.
+
+However, a few system calls do things differently. They might return to a
+different location (rt_sigreturn) or change the memory space (fork/vfork/clone)
+or even architecture (execve/execveat) of the program.
+
+To allow for this, the kernel implementation of the system call may need to
+save and restore additional registers to the kernel stack, allowing complete
+control of where and how execution continues after the system call.
+
+This is arch-specific, but typically involves defining assembly entry points
+that save/restore additional registers and invoke the real system call entry
+point.
+
+For x86_64, this is implemented as a stub_xyzzy entry point in
+arch/x86/entry/entry_64.S, and the entry in the syscall table
+(arch/x86/entry/syscalls/syscall_64.tbl) is adjusted to match:
+
+ 333 common xyzzy stub_xyzzy
+
+The equivalent for 32-bit programs running on a 64-bit kernel is normally
+called stub32_xyzzy and implemented in arch/x86/entry/entry_64_compat.S,
+with the corresponding syscall table adjustment in
+arch/x86/entry/syscalls/syscall_32.tbl:
+
+ 380 i386 xyzzy sys_xyzzy stub32_xyzzy
+
+If the system call needs a compatibility layer (as in the previous section)
+then the stub32_ version needs to call on to the compat_sys_ version of the
+system call rather than the native 64-bit version. Also, if the x32 ABI
+implementation is not common with the x86_64 version, then its syscall
+table will also need to invoke a stub that calls on to the compat_sys_
+version.
+
+For completeness, it's also nice to set up a mapping so that user-mode Linux
+still works -- its syscall table will reference stub_xyzzy, but the UML build
+doesn't include arch/x86/entry/entry_64.S implementation (because UML
+simulates registers etc). Fixing this is as simple as adding a #define to
+arch/x86/um/sys_call_table_64.c:
+
+ #define stub_xyzzy sys_xyzzy
+
+
+Other Details
+-------------
+
+Most of the kernel treats system calls in a generic way, but there is the
+occasional exception that may need updating for your particular system call.
+
+The audit subsystem is one such special case; it includes (arch-specific)
+functions that classify some special types of system call -- specifically
+file open (open/openat), program execution (execve/exeveat) or socket
+multiplexor (socketcall) operations. If your new system call is analogous to
+one of these, then the audit system should be updated.
+
+More generally, if there is an existing system call that is analogous to your
+new system call, it's worth doing a kernel-wide grep for the existing system
+call to check there are no other special cases.
+
+
+Testing
+-------
+
+A new system call should obviously be tested; it is also useful to provide
+reviewers with a demonstration of how user space programs will use the system
+call. A good way to combine these aims is to include a simple self-test
+program in a new directory under tools/testing/selftests/.
+
+Make sure the selftest runs successfully on all supported architectures. For
+example, check that it works when compiled as an x86_64 (-m64), x86_32 (-m32)
+and x32 (-mx32) ABI program.
+
+
+Man Page
+--------
+
+All new system calls should come with a complete man page, ideally using groff
+markup, but plain text will do. If groff is used, it's helpful to include a
+pre-rendered ASCII version of the man page in the cover email for the
+patchset, for the convenience of reviewers.
+
+The man page should be cc'ed to linux-man-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+
+
+References and Sources
+----------------------
+
+ - LWN article from Michael Kerrisk on use of flags argument in system calls:
+ https://lwn.net/Articles/585415/
+ - LWN article from Michael Kerrisk on how to handle unknown flags in a system
+ call: https://lwn.net/Articles/588444/
+ - LWN article from Jake Edge describing constraints on 64-bit system call
+ arguments: https://lwn.net/Articles/311630/
+ - Pair of LWN articles from David Drysdale that describe the system call
+ implementation paths in detail for v3.14:
+ - https://lwn.net/Articles/604287/
+ - https://lwn.net/Articles/604515/
+ - Architecture-specific requirements for system calls are discussed in the
+ syscall(2) man-page:
+ http://man7.org/linux/man-pages/man2/syscall.2.html#NOTES
+ - Collated emails from Linus Torvalds discussing the problems with ioctl():
+ http://yarchive.net/comp/linux/ioctl.html
+ - "How to not invent kernel interfaces", Arnd Bergmann,
+ http://www.ukuug.org/events/linux2007/2007/papers/Bergmann.pdf
+ - LWN article from Michael Kerrisk on avoiding new uses of CAP_SYS_ADMIN:
+ https://lwn.net/Articles/486306/
+
+ - Recommendation from Andrew Morton that all related information for a new
+ system call should come in the same email thread:
+ https://lkml.org/lkml/2014/7/24/641
+ - Recommendation from Michael Kerrisk that a new system call should come with
+ a man page: https://lkml.org/lkml/2014/6/13/309
+ - Suggestion from Thomas Gleixner that x86 wire-up should be in a separate
+ commit: https://lkml.org/lkml/2014/11/19/254
+ - Suggestion from Greg Kroah-Hartman that it's good for new system calls to
+ come with a man-page & selftest: https://lkml.org/lkml/2014/3/19/710
+ - Discussion from Michael Kerrisk of new system call vs. prctl(2) extension:
+ https://lkml.org/lkml/2014/6/3/411
+ - Numbering oddities arising from (re-)use of O_* numbering space flags:
+ - commit 75069f2b5bfb ("vfs: renumber FMODE_NONOTIFY and add to uniqueness
+ check")
+ - commit 12ed2e36c98a ("fanotify: FMODE_NONOTIFY and __O_SYNC in sparc
+ conflict")
+ - commit bb458c644a59 ("Safer ABI for O_TMPFILE")
+ - Discussion from Matthew Wilcox about restrictions on 64-bit arguments:
+ https://lkml.org/lkml/2008/12/12/187
+ - Recommendation from Greg Kroah-Hartman that unknown flags should be
+ policed: https://lkml.org/lkml/2014/7/17/577
+ - Recommendation from Linus Torvalds that x32 system calls should prefer
+ compatibility with 64-bit versions rather than 32-bit versions:
+ https://lkml.org/lkml/2011/8/31/244
--
2.4.3.573.g4eafbef
^ permalink raw reply related
* [PATCH RFC 0/1] Document how to add a new syscall
From: David Drysdale @ 2015-07-28 11:41 UTC (permalink / raw)
To: linux-api, Michael Kerrisk, Andrew Morton, Arnd Bergmann,
Shuah Khan, Jonathan Corbet
Cc: Andrea Arcangeli, Thomas Gleixner, Eric B Munson, Ingo Molnar,
H. Peter Anvin, Oleg Nesterov, Linus Torvalds, Greg Kroah-Hartman,
Andy Lutomirski, Al Viro, Rusty Russell, Peter Zijlstra,
Vivek Goyal, Alexei Starovoitov, David Herrmann,
Theodore Ts'o, Kees Cook, Miklos Szeredi, Milosz Tanski,
Fam Zheng, Josh Triplett, Mathieu Desnoyers, linux-doc,
linux-kernel
Given that I've gotten some of the details wrong in the past (and I've
seen others do likewise), I thought it might be helpful to collate the
best practices for adding a new system call to the kernel.
Apologies for the wide circulation -- I've tried to include folk who've
recently added or proposed a system call, as they're most likely to
have opinions on:
- whether this a useful addition to Documentation/
- whether the details of the advice are correct and complete.
Shuah, is there anything more that should be added for the Testing
section in particular?
Thanks,
David
(With thanks to Andrew Morton for looking over an initial draft, and to
Michael Kerrisk for suggesting several clarifications and additions.)
David Drysdale (1):
Documentation: describe how to add a system call
Documentation/adding-syscalls.txt | 454 ++++++++++++++++++++++++++++++++++++++
1 file changed, 454 insertions(+)
create mode 100644 Documentation/adding-syscalls.txt
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply
* Re: [PATCH V5 0/7] Allow user to request memory to be locked on page fault
From: Vlastimil Babka @ 2015-07-28 11:23 UTC (permalink / raw)
To: Michal Hocko, Eric B Munson
Cc: Andrew Morton, Shuah Khan, Michael Kerrisk, Jonathan Corbet,
Ralf Baechle, linux-alpha, linux-kernel, linux-mips, linux-parisc,
linuxppc-dev, sparclinux, linux-xtensa, linux-mm, linux-arch,
linux-api
In-Reply-To: <20150728111725.GG24972@dhcp22.suse.cz>
On 07/28/2015 01:17 PM, Michal Hocko wrote:
> [I am sorry but I didn't get to this sooner.]
>
> On Mon 27-07-15 10:54:09, Eric B Munson wrote:
>> Now that VM_LOCKONFAULT is a modifier to VM_LOCKED and
>> cannot be specified independentally, it might make more sense to mirror
>> that relationship to userspace. Which would lead to soemthing like the
>> following:
>
> A modifier makes more sense.
>
>> To lock and populate a region:
>> mlock2(start, len, 0);
>>
>> To lock on fault a region:
>> mlock2(start, len, MLOCK_ONFAULT);
>>
>> If LOCKONFAULT is seen as a modifier to mlock, then having the flags
>> argument as 0 mean do mlock classic makes more sense to me.
>>
>> To mlock current on fault only:
>> mlockall(MCL_CURRENT | MCL_ONFAULT);
>>
>> To mlock future on fault only:
>> mlockall(MCL_FUTURE | MCL_ONFAULT);
>>
>> To lock everything on fault:
>> mlockall(MCL_CURRENT | MCL_FUTURE | MCL_ONFAULT);
>
> Makes sense to me. The only remaining and still tricky part would be
> the munlock{all}(flags) behavior. What should munlock(MLOCK_ONFAULT)
> do? Keep locked and poppulate the range or simply ignore the flag an
> just unlock?
munlock(all) already lost both MLOCK_LOCKED and MLOCK_ONFAULT flags in
this revision, so I suppose in the next revision it will also not accept
MLOCK_ONFAULT, and will just munlock whatever was mlocked in either mode.
> I can see some sense to allow munlockall(MCL_FUTURE[|MLOCK_ONFAULT]),
> munlockall(MCL_CURRENT) resp. munlockall(MCL_CURRENT|MCL_FUTURE) but
> other combinations sound weird to me.
The effect of munlockall(MCL_FUTURE|MLOCK_ONFAULT), which you probably
intended for converting the onfault to full prepopulation for future
mappings, can be achieved by calling mlockall(MCL_FUTURE) (without
MLOCK_ONFAULT).
> Anyway munlock with flags opens new doors of trickiness.
^ permalink raw reply
* Re: [PATCH V5 0/7] Allow user to request memory to be locked on page fault
From: Michal Hocko @ 2015-07-28 11:17 UTC (permalink / raw)
To: Eric B Munson
Cc: Vlastimil Babka, Andrew Morton, Shuah Khan, Michael Kerrisk,
Jonathan Corbet, Ralf Baechle, linux-alpha, linux-kernel,
linux-mips, linux-parisc, linuxppc-dev, sparclinux, linux-xtensa,
linux-mm, linux-arch, linux-api
In-Reply-To: <20150727145409.GB21664@akamai.com>
[I am sorry but I didn't get to this sooner.]
On Mon 27-07-15 10:54:09, Eric B Munson wrote:
> Now that VM_LOCKONFAULT is a modifier to VM_LOCKED and
> cannot be specified independentally, it might make more sense to mirror
> that relationship to userspace. Which would lead to soemthing like the
> following:
A modifier makes more sense.
> To lock and populate a region:
> mlock2(start, len, 0);
>
> To lock on fault a region:
> mlock2(start, len, MLOCK_ONFAULT);
>
> If LOCKONFAULT is seen as a modifier to mlock, then having the flags
> argument as 0 mean do mlock classic makes more sense to me.
>
> To mlock current on fault only:
> mlockall(MCL_CURRENT | MCL_ONFAULT);
>
> To mlock future on fault only:
> mlockall(MCL_FUTURE | MCL_ONFAULT);
>
> To lock everything on fault:
> mlockall(MCL_CURRENT | MCL_FUTURE | MCL_ONFAULT);
Makes sense to me. The only remaining and still tricky part would be
the munlock{all}(flags) behavior. What should munlock(MLOCK_ONFAULT)
do? Keep locked and poppulate the range or simply ignore the flag an
just unlock?
I can see some sense to allow munlockall(MCL_FUTURE[|MLOCK_ONFAULT]),
munlockall(MCL_CURRENT) resp. munlockall(MCL_CURRENT|MCL_FUTURE) but
other combinations sound weird to me.
Anyway munlock with flags opens new doors of trickiness.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox