thr3ads.net - Linux Virtualization - [PATCH 5/6] kernel: better document the use_mm/unuse

If this information is useful, please help other people find it:
Share via:

Christoph Hellwig

2020-Apr-04 09:40 UTC

improve use_mm / unuse_mm

Hi all,

this series improves the use_mm / unuse_mm interface by better
documenting the assumptions, and my taking the set_fs manipulations
spread over the callers into the core API.

Christoph Hellwig

2020-Apr-04 09:40 UTC

head link

[PATCH 1/6] amdgpu: a NULL ->mm does not mean a thread is a kthread

Use the proper API instead.

Fixes: 70539bd795002 ("drm/amd: Update MEC HQD loading code for KFD")
Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 13feb313e9b3..4db143c19dcc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -190,7 +190,7 @@ uint8_t amdgpu_amdkfd_get_xgmi_hops_count(struct kgd_dev
*dst, struct kgd_dev *s
 			pagefault_disable();				\
 			if ((mmptr) == current->mm) {			\
 				valid = !get_user((dst), (wptr));	\
-			} else if (current->mm == NULL) {		\
+			} else if (current->flags & PF_KTHREAD) {	\
 				use_mm(mmptr);				\
 				valid = !get_user((dst), (wptr));	\
 				unuse_mm(mmptr);			\
-- 
2.25.1

Christoph Hellwig

2020-Apr-04 09:40 UTC

head link

[PATCH 2/6] i915/gvt/kvm: a NULL ->mm does not mean a thread is a kthread

Use the proper API instead.

Fixes: f440c8a572d7 ("drm/i915/gvt/kvmgt: read/write GPA via KVM API")
Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 074c4efb58eb..5848400620b4 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -2037,7 +2037,7 @@ static int kvmgt_rw_gpa(unsigned long handle, unsigned
long gpa,
 	struct kvmgt_guest_info *info;
 	struct kvm *kvm;
 	int idx, ret;
-	bool kthread = current->mm == NULL;
+	bool kthread = (current->flags & PF_KTHREAD);
 
 	if (!handle_valid(handle))
 		return -ESRCH;
-- 
2.25.1

Christoph Hellwig

2020-Apr-04 09:40 UTC

head link

[PATCH 3/6] i915/gvt: remove unused xen bits

No Xen support anywhere here.  Remove a dead declaration and an unused
include.

Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 drivers/gpu/drm/i915/gvt/gvt.c       | 1 -
 drivers/gpu/drm/i915/gvt/hypercall.h | 2 --
 2 files changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
index 9e1787867894..c7c561237883 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.c
+++ b/drivers/gpu/drm/i915/gvt/gvt.c
@@ -31,7 +31,6 @@
  */
 
 #include <linux/types.h>
-#include <xen/xen.h>
 #include <linux/kthread.h>
 
 #include "i915_drv.h"
diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h
b/drivers/gpu/drm/i915/gvt/hypercall.h
index b17c4a1599cd..b79da5124f83 100644
--- a/drivers/gpu/drm/i915/gvt/hypercall.h
+++ b/drivers/gpu/drm/i915/gvt/hypercall.h
@@ -79,6 +79,4 @@ struct intel_gvt_mpt {
 	bool (*is_valid_gfn)(unsigned long handle, unsigned long gfn);
 };
 
-extern struct intel_gvt_mpt xengt_mpt;
-
 #endif /* _GVT_HYPERCALL_H_ */
-- 
2.25.1

Christoph Hellwig

2020-Apr-04 09:40 UTC

head link

[PATCH 4/6] kernel: move use_mm/unuse_mm to kthread.c

These helpers are only for use with kernel threads, and I will tie them
more into the kthread infrastructure going forward.  Also move the
prototypes to kthread.h - mmu_context.h was a little weird to start with
as it otherwise contains very low-level MM bits.

Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    |  1 +
 .../drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c   |  1 -
 .../drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c    |  1 -
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c |  2 -
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c |  2 -
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c |  2 -
 drivers/gpu/drm/i915/gvt/kvmgt.c              |  2 +-
 drivers/usb/gadget/function/f_fs.c            |  2 +-
 drivers/usb/gadget/legacy/inode.c             |  2 +-
 drivers/vhost/vhost.c                         |  1 -
 fs/aio.c                                      |  1 -
 fs/io-wq.c                                    |  1 -
 fs/io_uring.c                                 |  1 -
 include/linux/kthread.h                       |  5 ++
 include/linux/mmu_context.h                   |  5 --
 kernel/kthread.c                              | 56 ++++++++++++++++
 mm/Makefile                                   |  2 +-
 mm/mmu_context.c                              | 64 -------------------
 18 files changed, 66 insertions(+), 85 deletions(-)
 delete mode 100644 mm/mmu_context.c

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 4db143c19dcc..bce5e93fefc8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -27,6 +27,7 @@
 
 #include <linux/types.h>
 #include <linux/mm.h>
+#include <linux/kthread.h>
 #include <linux/workqueue.h>
 #include <kgd_kfd_interface.h>
 #include <drm/ttm/ttm_execbuf_util.h>
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
index 6529caca88fe..35d4a5ab0228 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
@@ -22,7 +22,6 @@
 #include <linux/module.h>
 #include <linux/fdtable.h>
 #include <linux/uaccess.h>
-#include <linux/mmu_context.h>
 #include <linux/firmware.h>
 #include "amdgpu.h"
 #include "amdgpu_amdkfd.h"
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
index 4ec6d0c03201..b1655054b919 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
@@ -19,7 +19,6 @@
  * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
  * OTHER DEALINGS IN THE SOFTWARE.
  */
-#include <linux/mmu_context.h>
 #include "amdgpu.h"
 #include "amdgpu_amdkfd.h"
 #include "gc/gc_10_1_0_offset.h"
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
index 0b7e78748540..7d01420c0c85 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
@@ -20,8 +20,6 @@
  * OTHER DEALINGS IN THE SOFTWARE.
  */
 
-#include <linux/mmu_context.h>
-
 #include "amdgpu.h"
 #include "amdgpu_amdkfd.h"
 #include "cikd.h"
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
index ccd635b812b5..635cd1a26bed 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
@@ -20,8 +20,6 @@
  * OTHER DEALINGS IN THE SOFTWARE.
  */
 
-#include <linux/mmu_context.h>
-
 #include "amdgpu.h"
 #include "amdgpu_amdkfd.h"
 #include "gfx_v8_0.h"
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
index df841c2ac5e7..c7fd0c47b254 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
@@ -19,8 +19,6 @@
  * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
  * OTHER DEALINGS IN THE SOFTWARE.
  */
-#include <linux/mmu_context.h>
-
 #include "amdgpu.h"
 #include "amdgpu_amdkfd.h"
 #include "gc/gc_9_0_offset.h"
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 5848400620b4..dee01c371bf5 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -31,7 +31,7 @@
 #include <linux/init.h>
 #include <linux/device.h>
 #include <linux/mm.h>
-#include <linux/mmu_context.h>
+#include <linux/kthread.h>
 #include <linux/sched/mm.h>
 #include <linux/types.h>
 #include <linux/list.h>
diff --git a/drivers/usb/gadget/function/f_fs.c
b/drivers/usb/gadget/function/f_fs.c
index c81023b195c3..c57b1b2507c6 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -32,7 +32,7 @@
 #include <linux/usb/functionfs.h>
 
 #include <linux/aio.h>
-#include <linux/mmu_context.h>
+#include <linux/kthread.h>
 #include <linux/poll.h>
 #include <linux/eventfd.h>
 
diff --git a/drivers/usb/gadget/legacy/inode.c
b/drivers/usb/gadget/legacy/inode.c
index aa0de9e35afa..8b5233888bf8 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -21,7 +21,7 @@
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/poll.h>
-#include <linux/mmu_context.h>
+#include <linux/kthread.h>
 #include <linux/aio.h>
 #include <linux/uio.h>
 #include <linux/refcount.h>
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index f44340b41494..4e9ce54869af 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -14,7 +14,6 @@
 #include <linux/vhost.h>
 #include <linux/uio.h>
 #include <linux/mm.h>
-#include <linux/mmu_context.h>
 #include <linux/miscdevice.h>
 #include <linux/mutex.h>
 #include <linux/poll.h>
diff --git a/fs/aio.c b/fs/aio.c
index 5f3d3d814928..328829f0343b 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -27,7 +27,6 @@
 #include <linux/file.h>
 #include <linux/mm.h>
 #include <linux/mman.h>
-#include <linux/mmu_context.h>
 #include <linux/percpu.h>
 #include <linux/slab.h>
 #include <linux/timer.h>
diff --git a/fs/io-wq.c b/fs/io-wq.c
index cc5cf2209fb0..c49c2bdbafb5 100644
--- a/fs/io-wq.c
+++ b/fs/io-wq.c
@@ -10,7 +10,6 @@
 #include <linux/errno.h>
 #include <linux/sched/signal.h>
 #include <linux/mm.h>
-#include <linux/mmu_context.h>
 #include <linux/sched/mm.h>
 #include <linux/percpu.h>
 #include <linux/slab.h>
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 358f97be9c7b..27a4ecb724ca 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -55,7 +55,6 @@
 #include <linux/fdtable.h>
 #include <linux/mm.h>
 #include <linux/mman.h>
-#include <linux/mmu_context.h>
 #include <linux/percpu.h>
 #include <linux/slab.h>
 #include <linux/kthread.h>
diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index 8bbcaad7ef0f..c2d40c9672d6 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -5,6 +5,8 @@
 #include <linux/err.h>
 #include <linux/sched.h>
 
+struct mm_struct;
+
 __printf(4, 5)
 struct task_struct *kthread_create_on_node(int (*threadfn)(void *data),
 					   void *data,
@@ -198,6 +200,9 @@ bool kthread_cancel_delayed_work_sync(struct
kthread_delayed_work *work);
 
 void kthread_destroy_worker(struct kthread_worker *worker);
 
+void use_mm(struct mm_struct *mm);
+void unuse_mm(struct mm_struct *mm);
+
 struct cgroup_subsys_state;
 
 #ifdef CONFIG_BLK_CGROUP
diff --git a/include/linux/mmu_context.h b/include/linux/mmu_context.h
index d9a543a9e1cc..c51a84132d7c 100644
--- a/include/linux/mmu_context.h
+++ b/include/linux/mmu_context.h
@@ -4,11 +4,6 @@
 
 #include <asm/mmu_context.h>
 
-struct mm_struct;
-
-void use_mm(struct mm_struct *mm);
-void unuse_mm(struct mm_struct *mm);
-
 /* Architectures that care about IRQ state in switch_mm can override this. */
 #ifndef switch_mm_irqs_off
 # define switch_mm_irqs_off switch_mm
diff --git a/kernel/kthread.c b/kernel/kthread.c
index bfbfa481be3a..ce4610316377 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -1,13 +1,17 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /* Kernel thread helper functions.
  *   Copyright (C) 2004 IBM Corporation, Rusty Russell.
+ *   Copyright (C) 2009 Red Hat, Inc.
  *
  * Creation is done via kthreadd, so that we get a clean environment
  * even if we're invoked from userspace (think modprobe, hotplug cpu,
  * etc.).
  */
 #include <uapi/linux/sched/types.h>
+#include <linux/mm.h>
+#include <linux/mmu_context.h>
 #include <linux/sched.h>
+#include <linux/sched/mm.h>
 #include <linux/sched/task.h>
 #include <linux/kthread.h>
 #include <linux/completion.h>
@@ -25,6 +29,7 @@
 #include <linux/numa.h>
 #include <trace/events/sched.h>
 
+
 static DEFINE_SPINLOCK(kthread_create_lock);
 static LIST_HEAD(kthread_create_list);
 struct task_struct *kthreadd_task;
@@ -1203,6 +1208,57 @@ void kthread_destroy_worker(struct kthread_worker
*worker)
 }
 EXPORT_SYMBOL(kthread_destroy_worker);
 
+/*
+ * use_mm
+ *	Makes the calling kernel thread take on the specified
+ *	mm context.
+ *	(Note: this routine is intended to be called only
+ *	from a kernel thread context)
+ */
+void use_mm(struct mm_struct *mm)
+{
+	struct mm_struct *active_mm;
+	struct task_struct *tsk = current;
+
+	task_lock(tsk);
+	active_mm = tsk->active_mm;
+	if (active_mm != mm) {
+		mmgrab(mm);
+		tsk->active_mm = mm;
+	}
+	tsk->mm = mm;
+	switch_mm(active_mm, mm, tsk);
+	task_unlock(tsk);
+#ifdef finish_arch_post_lock_switch
+	finish_arch_post_lock_switch();
+#endif
+
+	if (active_mm != mm)
+		mmdrop(active_mm);
+}
+EXPORT_SYMBOL_GPL(use_mm);
+
+/*
+ * unuse_mm
+ *	Reverses the effect of use_mm, i.e. releases the
+ *	specified mm context which was earlier taken on
+ *	by the calling kernel thread
+ *	(Note: this routine is intended to be called only
+ *	from a kernel thread context)
+ */
+void unuse_mm(struct mm_struct *mm)
+{
+	struct task_struct *tsk = current;
+
+	task_lock(tsk);
+	sync_mm_rss(mm);
+	tsk->mm = NULL;
+	/* active_mm is still 'mm' */
+	enter_lazy_tlb(mm, tsk);
+	task_unlock(tsk);
+}
+EXPORT_SYMBOL_GPL(unuse_mm);
+
 #ifdef CONFIG_BLK_CGROUP
 /**
  * kthread_associate_blkcg - associate blkcg to current kthread
diff --git a/mm/Makefile b/mm/Makefile
index dbc8346d16ca..0af4ee81aed2 100644
--- a/mm/Makefile
+++ b/mm/Makefile
@@ -41,7 +41,7 @@ obj-y			:= filemap.o mempool.o oom_kill.o fadvise.o \
 			   maccess.o page-writeback.o \
 			   readahead.o swap.o truncate.o vmscan.o shmem.o \
 			   util.o mmzone.o vmstat.o backing-dev.o \
-			   mm_init.o mmu_context.o percpu.o slab_common.o \
+			   mm_init.o percpu.o slab_common.o \
 			   compaction.o vmacache.o \
 			   interval_tree.o list_lru.o workingset.o \
 			   debug.o gup.o $(mmu-y)
diff --git a/mm/mmu_context.c b/mm/mmu_context.c
deleted file mode 100644
index 3e612ae748e9..000000000000
--- a/mm/mmu_context.c
+++ /dev/null
@@ -1,64 +0,0 @@
-/* Copyright (C) 2009 Red Hat, Inc.
- *
- * See ../COPYING for licensing terms.
- */
-
-#include <linux/mm.h>
-#include <linux/sched.h>
-#include <linux/sched/mm.h>
-#include <linux/sched/task.h>
-#include <linux/mmu_context.h>
-#include <linux/export.h>
-
-#include <asm/mmu_context.h>
-
-/*
- * use_mm
- *	Makes the calling kernel thread take on the specified
- *	mm context.
- *	(Note: this routine is intended to be called only
- *	from a kernel thread context)
- */
-void use_mm(struct mm_struct *mm)
-{
-	struct mm_struct *active_mm;
-	struct task_struct *tsk = current;
-
-	task_lock(tsk);
-	active_mm = tsk->active_mm;
-	if (active_mm != mm) {
-		mmgrab(mm);
-		tsk->active_mm = mm;
-	}
-	tsk->mm = mm;
-	switch_mm(active_mm, mm, tsk);
-	task_unlock(tsk);
-#ifdef finish_arch_post_lock_switch
-	finish_arch_post_lock_switch();
-#endif
-
-	if (active_mm != mm)
-		mmdrop(active_mm);
-}
-EXPORT_SYMBOL_GPL(use_mm);
-
-/*
- * unuse_mm
- *	Reverses the effect of use_mm, i.e. releases the
- *	specified mm context which was earlier taken on
- *	by the calling kernel thread
- *	(Note: this routine is intended to be called only
- *	from a kernel thread context)
- */
-void unuse_mm(struct mm_struct *mm)
-{
-	struct task_struct *tsk = current;
-
-	task_lock(tsk);
-	sync_mm_rss(mm);
-	tsk->mm = NULL;
-	/* active_mm is still 'mm' */
-	enter_lazy_tlb(mm, tsk);
-	task_unlock(tsk);
-}
-EXPORT_SYMBOL_GPL(unuse_mm);
-- 
2.25.1

Christoph Hellwig

2020-Apr-04 09:41 UTC

head link

[PATCH 5/6] kernel: better document the use_mm/unuse_mm API contract

Switch the function documentation to kerneldoc comments, and add
WARN_ON_ONCE asserts that the calling thread is a kernel thread and
does not have ->mm set (or has ->mm set in the case of unuse_mm).

Also give the functions a kthread_ prefix to better document the
use case.

Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h |  4 +--
 drivers/gpu/drm/i915/gvt/kvmgt.c           |  4 +--
 drivers/usb/gadget/function/f_fs.c         |  4 +--
 drivers/usb/gadget/legacy/inode.c          |  4 +--
 drivers/vhost/vhost.c                      |  4 +--
 fs/io-wq.c                                 |  6 ++--
 fs/io_uring.c                              |  6 ++--
 include/linux/kthread.h                    |  4 +--
 kernel/kthread.c                           | 33 +++++++++++-----------
 mm/oom_kill.c                              |  6 ++--
 mm/vmacache.c                              |  4 +--
 11 files changed, 39 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index bce5e93fefc8..63db84e09408 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -192,9 +192,9 @@ uint8_t amdgpu_amdkfd_get_xgmi_hops_count(struct kgd_dev
*dst, struct kgd_dev *s
 			if ((mmptr) == current->mm) {			\
 				valid = !get_user((dst), (wptr));	\
 			} else if (current->flags & PF_KTHREAD) {	\
-				use_mm(mmptr);				\
+				kthread_use_mm(mmptr);			\
 				valid = !get_user((dst), (wptr));	\
-				unuse_mm(mmptr);			\
+				kthread_unuse_mm(mmptr);		\
 			}						\
 			pagefault_enable();				\
 		}							\
diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index dee01c371bf5..92e9b340dbc2 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -2048,7 +2048,7 @@ static int kvmgt_rw_gpa(unsigned long handle, unsigned
long gpa,
 	if (kthread) {
 		if (!mmget_not_zero(kvm->mm))
 			return -EFAULT;
-		use_mm(kvm->mm);
+		kthread_use_mm(kvm->mm);
 	}
 
 	idx = srcu_read_lock(&kvm->srcu);
@@ -2057,7 +2057,7 @@ static int kvmgt_rw_gpa(unsigned long handle, unsigned
long gpa,
 	srcu_read_unlock(&kvm->srcu, idx);
 
 	if (kthread) {
-		unuse_mm(kvm->mm);
+		kthread_unuse_mm(kvm->mm);
 		mmput(kvm->mm);
 	}
 
diff --git a/drivers/usb/gadget/function/f_fs.c
b/drivers/usb/gadget/function/f_fs.c
index c57b1b2507c6..d9e48bd7c692 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -827,9 +827,9 @@ static void ffs_user_copy_worker(struct work_struct *work)
 		mm_segment_t oldfs = get_fs();
 
 		set_fs(USER_DS);
-		use_mm(io_data->mm);
+		kthread_use_mm(io_data->mm);
 		ret = ffs_copy_to_iter(io_data->buf, ret, &io_data->data);
-		unuse_mm(io_data->mm);
+		kthread_unuse_mm(io_data->mm);
 		set_fs(oldfs);
 	}
 
diff --git a/drivers/usb/gadget/legacy/inode.c
b/drivers/usb/gadget/legacy/inode.c
index 8b5233888bf8..a05552bc2ff8 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -462,9 +462,9 @@ static void ep_user_copy_worker(struct work_struct *work)
 	struct kiocb *iocb = priv->iocb;
 	size_t ret;
 
-	use_mm(mm);
+	kthread_use_mm(mm);
 	ret = copy_to_iter(priv->buf, priv->actual, &priv->to);
-	unuse_mm(mm);
+	kthread_unuse_mm(mm);
 	if (!ret)
 		ret = -EFAULT;
 
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 4e9ce54869af..1787d426a956 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -336,7 +336,7 @@ static int vhost_worker(void *data)
 	mm_segment_t oldfs = get_fs();
 
 	set_fs(USER_DS);
-	use_mm(dev->mm);
+	kthread_use_mm(dev->mm);
 
 	for (;;) {
 		/* mb paired w/ kthread_stop */
@@ -364,7 +364,7 @@ static int vhost_worker(void *data)
 				schedule();
 		}
 	}
-	unuse_mm(dev->mm);
+	kthread_unuse_mm(dev->mm);
 	set_fs(oldfs);
 	return 0;
 }
diff --git a/fs/io-wq.c b/fs/io-wq.c
index c49c2bdbafb5..83c2868eff2a 100644
--- a/fs/io-wq.c
+++ b/fs/io-wq.c
@@ -169,7 +169,7 @@ static bool __io_worker_unuse(struct io_wqe *wqe, struct
io_worker *worker)
 		}
 		__set_current_state(TASK_RUNNING);
 		set_fs(KERNEL_DS);
-		unuse_mm(worker->mm);
+		kthread_unuse_mm(worker->mm);
 		mmput(worker->mm);
 		worker->mm = NULL;
 	}
@@ -416,7 +416,7 @@ static struct io_wq_work *io_get_next_work(struct io_wqe
*wqe)
 static void io_wq_switch_mm(struct io_worker *worker, struct io_wq_work *work)
 {
 	if (worker->mm) {
-		unuse_mm(worker->mm);
+		kthread_unuse_mm(worker->mm);
 		mmput(worker->mm);
 		worker->mm = NULL;
 	}
@@ -425,7 +425,7 @@ static void io_wq_switch_mm(struct io_worker *worker, struct
io_wq_work *work)
 		return;
 	}
 	if (mmget_not_zero(work->mm)) {
-		use_mm(work->mm);
+		kthread_use_mm(work->mm);
 		if (!worker->mm)
 			set_fs(USER_DS);
 		worker->mm = work->mm;
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 27a4ecb724ca..367406381044 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -5839,7 +5839,7 @@ static int io_submit_sqes(struct io_ring_ctx *ctx,
unsigned int nr,
 				err = -EFAULT;
 				goto fail_req;
 			}
-			use_mm(ctx->sqo_mm);
+			kthread_use_mm(ctx->sqo_mm);
 			*mm = ctx->sqo_mm;
 		}
 
@@ -5911,7 +5911,7 @@ static int io_sq_thread(void *data)
 			 * may sleep.
 			 */
 			if (cur_mm) {
-				unuse_mm(cur_mm);
+				kthread_unuse_mm(cur_mm);
 				mmput(cur_mm);
 				cur_mm = NULL;
 			}
@@ -5987,7 +5987,7 @@ static int io_sq_thread(void *data)
 
 	set_fs(old_fs);
 	if (cur_mm) {
-		unuse_mm(cur_mm);
+		kthread_unuse_mm(cur_mm);
 		mmput(cur_mm);
 	}
 	revert_creds(old_cred);
diff --git a/include/linux/kthread.h b/include/linux/kthread.h
index c2d40c9672d6..12258ea077cf 100644
--- a/include/linux/kthread.h
+++ b/include/linux/kthread.h
@@ -200,8 +200,8 @@ bool kthread_cancel_delayed_work_sync(struct
kthread_delayed_work *work);
 
 void kthread_destroy_worker(struct kthread_worker *worker);
 
-void use_mm(struct mm_struct *mm);
-void unuse_mm(struct mm_struct *mm);
+void kthread_use_mm(struct mm_struct *mm);
+void kthread_unuse_mm(struct mm_struct *mm);
 
 struct cgroup_subsys_state;
 
diff --git a/kernel/kthread.c b/kernel/kthread.c
index ce4610316377..316db17f6b4f 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -1208,18 +1208,18 @@ void kthread_destroy_worker(struct kthread_worker
*worker)
 }
 EXPORT_SYMBOL(kthread_destroy_worker);
 
-/*
- * use_mm
- *	Makes the calling kernel thread take on the specified
- *	mm context.
- *	(Note: this routine is intended to be called only
- *	from a kernel thread context)
+/**
+ * kthread_use_mm - make the calling kthread operate on an address space
+ * @mm: address space to operate on
  */
-void use_mm(struct mm_struct *mm)
+void kthread_use_mm(struct mm_struct *mm)
 {
 	struct mm_struct *active_mm;
 	struct task_struct *tsk = current;
 
+	WARN_ON_ONCE(!(tsk->flags & PF_KTHREAD));
+	WARN_ON_ONCE(tsk->mm);
+
 	task_lock(tsk);
 	active_mm = tsk->active_mm;
 	if (active_mm != mm) {
@@ -1236,20 +1236,19 @@ void use_mm(struct mm_struct *mm)
 	if (active_mm != mm)
 		mmdrop(active_mm);
 }
-EXPORT_SYMBOL_GPL(use_mm);
+EXPORT_SYMBOL_GPL(kthread_use_mm);
 
-/*
- * unuse_mm
- *	Reverses the effect of use_mm, i.e. releases the
- *	specified mm context which was earlier taken on
- *	by the calling kernel thread
- *	(Note: this routine is intended to be called only
- *	from a kernel thread context)
+/**
+ * kthread_use_mm - reverse the effect of kthread_use_mm()
+ * @mm: address space to operate on
  */
-void unuse_mm(struct mm_struct *mm)
+void kthread_unuse_mm(struct mm_struct *mm)
 {
 	struct task_struct *tsk = current;
 
+	WARN_ON_ONCE(!(tsk->flags & PF_KTHREAD));
+	WARN_ON_ONCE(!tsk->mm);
+
 	task_lock(tsk);
 	sync_mm_rss(mm);
 	tsk->mm = NULL;
@@ -1257,7 +1256,7 @@ void unuse_mm(struct mm_struct *mm)
 	enter_lazy_tlb(mm, tsk);
 	task_unlock(tsk);
 }
-EXPORT_SYMBOL_GPL(unuse_mm);
+EXPORT_SYMBOL_GPL(kthread_unuse_mm);
 
 #ifdef CONFIG_BLK_CGROUP
 /**
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index dfc357614e56..958d2972313f 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -126,7 +126,7 @@ static bool oom_cpuset_eligible(struct task_struct *tsk,
struct oom_control *oc)
 
 /*
  * The process p may have detached its own ->mm while exiting or through
- * use_mm(), but one or more of its subthreads may still have a valid
+ * kthread_use_mm(), but one or more of its subthreads may still have a valid
  * pointer.  Return p, or any of its subthreads with a valid ->mm, with
  * task_lock() held.
  */
@@ -919,8 +919,8 @@ static void __oom_kill_process(struct task_struct *victim,
const char *message)
 			continue;
 		}
 		/*
-		 * No use_mm() user needs to read from the userspace so we are
-		 * ok to reap it.
+		 * No kthead_use_mm() user needs to read from the userspace so
+		 * we are ok to reap it.
 		 */
 		if (unlikely(p->flags & PF_KTHREAD))
 			continue;
diff --git a/mm/vmacache.c b/mm/vmacache.c
index cdc32a3b02fa..ceedbab82106 100644
--- a/mm/vmacache.c
+++ b/mm/vmacache.c
@@ -25,8 +25,8 @@
  * task's vmacache pertains to a different mm (ie, its own).  There is
  * nothing we can do here.
  *
- * Also handle the case where a kernel thread has adopted this mm via use_mm().
- * That kernel thread's vmacache is not applicable to this mm.
+ * Also handle the case where a kernel thread has adopted this mm via
+ * kthread_use_mm(). That kernel thread's vmacache is not applicable to
this mm.
  */
 static inline bool vmacache_valid_mm(struct mm_struct *mm)
 {
-- 
2.25.1

Christoph Hellwig

2020-Apr-04 09:41 UTC

head link

[PATCH 6/6] kernel: set USER_DS in kthread_use_mm

Some architectures like arm64 and s390 require USER_DS to be set for
kernel threads to access user address space, which is the whole purpose
of kthread_use_mm, but other like x86 don't.  That has lead to a huge
mess where some callers are fixed up once they are tested on said
architectures, while others linger around and yet other like io_uring
try to do "clever" optimizations for what usually is just a trivial
asignment to a member in the thread_struct for most architectures.

Make kthread_use_mm set USER_DS, and kthread_unuse_mm restore to the
previous value instead.

Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 drivers/usb/gadget/function/f_fs.c | 4 ----
 drivers/vhost/vhost.c              | 3 ---
 fs/io-wq.c                         | 8 ++------
 fs/io_uring.c                      | 4 ----
 kernel/kthread.c                   | 6 ++++++
 5 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c
b/drivers/usb/gadget/function/f_fs.c
index d9e48bd7c692..a1198f4c527c 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -824,13 +824,9 @@ static void ffs_user_copy_worker(struct work_struct *work)
 	bool kiocb_has_eventfd = io_data->kiocb->ki_flags & IOCB_EVENTFD;
 
 	if (io_data->read && ret > 0) {
-		mm_segment_t oldfs = get_fs();
-
-		set_fs(USER_DS);
 		kthread_use_mm(io_data->mm);
 		ret = ffs_copy_to_iter(io_data->buf, ret, &io_data->data);
 		kthread_unuse_mm(io_data->mm);
-		set_fs(oldfs);
 	}
 
 	io_data->kiocb->ki_complete(io_data->kiocb, ret, ret);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 1787d426a956..b5229ae01d3b 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -333,9 +333,7 @@ static int vhost_worker(void *data)
 	struct vhost_dev *dev = data;
 	struct vhost_work *work, *work_next;
 	struct llist_node *node;
-	mm_segment_t oldfs = get_fs();
 
-	set_fs(USER_DS);
 	kthread_use_mm(dev->mm);
 
 	for (;;) {
@@ -365,7 +363,6 @@ static int vhost_worker(void *data)
 		}
 	}
 	kthread_unuse_mm(dev->mm);
-	set_fs(oldfs);
 	return 0;
 }
 
diff --git a/fs/io-wq.c b/fs/io-wq.c
index 83c2868eff2a..75cc2f31816d 100644
--- a/fs/io-wq.c
+++ b/fs/io-wq.c
@@ -168,7 +168,6 @@ static bool __io_worker_unuse(struct io_wqe *wqe, struct
io_worker *worker)
 			dropped_lock = true;
 		}
 		__set_current_state(TASK_RUNNING);
-		set_fs(KERNEL_DS);
 		kthread_unuse_mm(worker->mm);
 		mmput(worker->mm);
 		worker->mm = NULL;
@@ -420,14 +419,11 @@ static void io_wq_switch_mm(struct io_worker *worker,
struct io_wq_work *work)
 		mmput(worker->mm);
 		worker->mm = NULL;
 	}
-	if (!work->mm) {
-		set_fs(KERNEL_DS);
+	if (!work->mm)
 		return;
-	}
+
 	if (mmget_not_zero(work->mm)) {
 		kthread_use_mm(work->mm);
-		if (!worker->mm)
-			set_fs(USER_DS);
 		worker->mm = work->mm;
 		/* hang on to this mm */
 		work->mm = NULL;
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 367406381044..c332a34e8b34 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -5871,15 +5871,12 @@ static int io_sq_thread(void *data)
 	struct io_ring_ctx *ctx = data;
 	struct mm_struct *cur_mm = NULL;
 	const struct cred *old_cred;
-	mm_segment_t old_fs;
 	DEFINE_WAIT(wait);
 	unsigned long timeout;
 	int ret = 0;
 
 	complete(&ctx->completions[1]);
 
-	old_fs = get_fs();
-	set_fs(USER_DS);
 	old_cred = override_creds(ctx->creds);
 
 	timeout = jiffies + ctx->sq_thread_idle;
@@ -5985,7 +5982,6 @@ static int io_sq_thread(void *data)
 	if (current->task_works)
 		task_work_run();
 
-	set_fs(old_fs);
 	if (cur_mm) {
 		kthread_unuse_mm(cur_mm);
 		mmput(cur_mm);
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 316db17f6b4f..9e27d01b6d78 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -52,6 +52,7 @@ struct kthread {
 	unsigned long flags;
 	unsigned int cpu;
 	void *data;
+	mm_segment_t oldfs;
 	struct completion parked;
 	struct completion exited;
 #ifdef CONFIG_BLK_CGROUP
@@ -1235,6 +1236,9 @@ void kthread_use_mm(struct mm_struct *mm)
 
 	if (active_mm != mm)
 		mmdrop(active_mm);
+
+	to_kthread(tsk)->oldfs = get_fs();
+	set_fs(USER_DS);
 }
 EXPORT_SYMBOL_GPL(kthread_use_mm);
 
@@ -1249,6 +1253,8 @@ void kthread_unuse_mm(struct mm_struct *mm)
 	WARN_ON_ONCE(!(tsk->flags & PF_KTHREAD));
 	WARN_ON_ONCE(!tsk->mm);
 
+	set_fs(to_kthread(tsk)->oldfs);
+
 	task_lock(tsk);
 	sync_mm_rss(mm);
 	tsk->mm = NULL;
-- 
2.25.1

Sergei Shtylyov

2020-Apr-04 10:05 UTC

head link

[PATCH 2/6] i915/gvt/kvm: a NULL ->mm does not mean a thread is a kthread

Hello!

On 04.04.2020 12:40, Christoph Hellwig wrote:
> Use the proper API instead.
> 
> Fixes: f440c8a572d7 ("drm/i915/gvt/kvmgt: read/write GPA via KVM
API")
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> ---
>   drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 074c4efb58eb..5848400620b4 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -2037,7 +2037,7 @@ static int kvmgt_rw_gpa(unsigned long handle,
unsigned long gpa,
>   	struct kvmgt_guest_info *info;
>   	struct kvm *kvm;
>   	int idx, ret;
> -	bool kthread = current->mm == NULL;
> +	bool kthread = (current->flags & PF_KTHREAD);
    Don't need the parens.

[...]

MBR, Sergei

Felix Kuehling

2020-Apr-06 16:07 UTC

head link

[PATCH 1/6] amdgpu: a NULL ->mm does not mean a thread is a kthread

Am 2020-04-04 um 5:40 a.m. schrieb Christoph Hellwig:> Use the proper API instead.
>
> Fixes: 70539bd795002 ("drm/amd: Update MEC HQD loading code for
KFD")
> Signed-off-by: Christoph Hellwig <hch at lst.de>
Reviewed-by: Felix Kuehling <Felix.Kuehling at amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index 13feb313e9b3..4db143c19dcc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -190,7 +190,7 @@ uint8_t amdgpu_amdkfd_get_xgmi_hops_count(struct
kgd_dev *dst, struct kgd_dev *s
>  			pagefault_disable();				\
>  			if ((mmptr) == current->mm) {			\
>  				valid = !get_user((dst), (wptr));	\
> -			} else if (current->mm == NULL) {		\
> +			} else if (current->flags & PF_KTHREAD) {	\
>  				use_mm(mmptr);				\
>  				valid = !get_user((dst), (wptr));	\
>  				unuse_mm(mmptr);			\

Felix Kuehling

2020-Apr-06 16:09 UTC

head link

[PATCH 4/6] kernel: move use_mm/unuse_mm to kthread.c

Am 2020-04-04 um 5:40 a.m. schrieb Christoph Hellwig:> These helpers are only for use with kernel threads, and I will tie them
> more into the kthread infrastructure going forward.  Also move the
> prototypes to kthread.h - mmu_context.h was a little weird to start with
> as it otherwise contains very low-level MM bits.
>
> Signed-off-by: Christoph Hellwig <hch at lst.de>
Acked-by: Felix Kuehling <Felix.Kuehling at amd.com>

Thanks for cleaning up the unnecessary includes in amdgpu.

Regards,
? Felix

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    |  1 +
>  .../drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c   |  1 -
>  .../drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c    |  1 -
>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c |  2 -
>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c |  2 -
>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c |  2 -
>  drivers/gpu/drm/i915/gvt/kvmgt.c              |  2 +-
>  drivers/usb/gadget/function/f_fs.c            |  2 +-
>  drivers/usb/gadget/legacy/inode.c             |  2 +-
>  drivers/vhost/vhost.c                         |  1 -
>  fs/aio.c                                      |  1 -
>  fs/io-wq.c                                    |  1 -
>  fs/io_uring.c                                 |  1 -
>  include/linux/kthread.h                       |  5 ++
>  include/linux/mmu_context.h                   |  5 --
>  kernel/kthread.c                              | 56 ++++++++++++++++
>  mm/Makefile                                   |  2 +-
>  mm/mmu_context.c                              | 64 -------------------
>  18 files changed, 66 insertions(+), 85 deletions(-)
>  delete mode 100644 mm/mmu_context.c
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index 4db143c19dcc..bce5e93fefc8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -27,6 +27,7 @@
>  
>  #include <linux/types.h>
>  #include <linux/mm.h>
> +#include <linux/kthread.h>
>  #include <linux/workqueue.h>
>  #include <kgd_kfd_interface.h>
>  #include <drm/ttm/ttm_execbuf_util.h>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> index 6529caca88fe..35d4a5ab0228 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> @@ -22,7 +22,6 @@
>  #include <linux/module.h>
>  #include <linux/fdtable.h>
>  #include <linux/uaccess.h>
> -#include <linux/mmu_context.h>
>  #include <linux/firmware.h>
>  #include "amdgpu.h"
>  #include "amdgpu_amdkfd.h"
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
> index 4ec6d0c03201..b1655054b919 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
> @@ -19,7 +19,6 @@
>   * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>   * OTHER DEALINGS IN THE SOFTWARE.
>   */
> -#include <linux/mmu_context.h>
>  #include "amdgpu.h"
>  #include "amdgpu_amdkfd.h"
>  #include "gc/gc_10_1_0_offset.h"
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
> index 0b7e78748540..7d01420c0c85 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
> @@ -20,8 +20,6 @@
>   * OTHER DEALINGS IN THE SOFTWARE.
>   */
>  
> -#include <linux/mmu_context.h>
> -
>  #include "amdgpu.h"
>  #include "amdgpu_amdkfd.h"
>  #include "cikd.h"
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
> index ccd635b812b5..635cd1a26bed 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
> @@ -20,8 +20,6 @@
>   * OTHER DEALINGS IN THE SOFTWARE.
>   */
>  
> -#include <linux/mmu_context.h>
> -
>  #include "amdgpu.h"
>  #include "amdgpu_amdkfd.h"
>  #include "gfx_v8_0.h"
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> index df841c2ac5e7..c7fd0c47b254 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> @@ -19,8 +19,6 @@
>   * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
>   * OTHER DEALINGS IN THE SOFTWARE.
>   */
> -#include <linux/mmu_context.h>
> -
>  #include "amdgpu.h"
>  #include "amdgpu_amdkfd.h"
>  #include "gc/gc_9_0_offset.h"
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index 5848400620b4..dee01c371bf5 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -31,7 +31,7 @@
>  #include <linux/init.h>
>  #include <linux/device.h>
>  #include <linux/mm.h>
> -#include <linux/mmu_context.h>
> +#include <linux/kthread.h>
>  #include <linux/sched/mm.h>
>  #include <linux/types.h>
>  #include <linux/list.h>
> diff --git a/drivers/usb/gadget/function/f_fs.c
b/drivers/usb/gadget/function/f_fs.c
> index c81023b195c3..c57b1b2507c6 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -32,7 +32,7 @@
>  #include <linux/usb/functionfs.h>
>  
>  #include <linux/aio.h>
> -#include <linux/mmu_context.h>
> +#include <linux/kthread.h>
>  #include <linux/poll.h>
>  #include <linux/eventfd.h>
>  
> diff --git a/drivers/usb/gadget/legacy/inode.c
b/drivers/usb/gadget/legacy/inode.c
> index aa0de9e35afa..8b5233888bf8 100644
> --- a/drivers/usb/gadget/legacy/inode.c
> +++ b/drivers/usb/gadget/legacy/inode.c
> @@ -21,7 +21,7 @@
>  #include <linux/sched.h>
>  #include <linux/slab.h>
>  #include <linux/poll.h>
> -#include <linux/mmu_context.h>
> +#include <linux/kthread.h>
>  #include <linux/aio.h>
>  #include <linux/uio.h>
>  #include <linux/refcount.h>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index f44340b41494..4e9ce54869af 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -14,7 +14,6 @@
>  #include <linux/vhost.h>
>  #include <linux/uio.h>
>  #include <linux/mm.h>
> -#include <linux/mmu_context.h>
>  #include <linux/miscdevice.h>
>  #include <linux/mutex.h>
>  #include <linux/poll.h>
> diff --git a/fs/aio.c b/fs/aio.c
> index 5f3d3d814928..328829f0343b 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -27,7 +27,6 @@
>  #include <linux/file.h>
>  #include <linux/mm.h>
>  #include <linux/mman.h>
> -#include <linux/mmu_context.h>
>  #include <linux/percpu.h>
>  #include <linux/slab.h>
>  #include <linux/timer.h>
> diff --git a/fs/io-wq.c b/fs/io-wq.c
> index cc5cf2209fb0..c49c2bdbafb5 100644
> --- a/fs/io-wq.c
> +++ b/fs/io-wq.c
> @@ -10,7 +10,6 @@
>  #include <linux/errno.h>
>  #include <linux/sched/signal.h>
>  #include <linux/mm.h>
> -#include <linux/mmu_context.h>
>  #include <linux/sched/mm.h>
>  #include <linux/percpu.h>
>  #include <linux/slab.h>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 358f97be9c7b..27a4ecb724ca 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -55,7 +55,6 @@
>  #include <linux/fdtable.h>
>  #include <linux/mm.h>
>  #include <linux/mman.h>
> -#include <linux/mmu_context.h>
>  #include <linux/percpu.h>
>  #include <linux/slab.h>
>  #include <linux/kthread.h>
> diff --git a/include/linux/kthread.h b/include/linux/kthread.h
> index 8bbcaad7ef0f..c2d40c9672d6 100644
> --- a/include/linux/kthread.h
> +++ b/include/linux/kthread.h
> @@ -5,6 +5,8 @@
>  #include <linux/err.h>
>  #include <linux/sched.h>
>  
> +struct mm_struct;
> +
>  __printf(4, 5)
>  struct task_struct *kthread_create_on_node(int (*threadfn)(void *data),
>  					   void *data,
> @@ -198,6 +200,9 @@ bool kthread_cancel_delayed_work_sync(struct
kthread_delayed_work *work);
>  
>  void kthread_destroy_worker(struct kthread_worker *worker);
>  
> +void use_mm(struct mm_struct *mm);
> +void unuse_mm(struct mm_struct *mm);
> +
>  struct cgroup_subsys_state;
>  
>  #ifdef CONFIG_BLK_CGROUP
> diff --git a/include/linux/mmu_context.h b/include/linux/mmu_context.h
> index d9a543a9e1cc..c51a84132d7c 100644
> --- a/include/linux/mmu_context.h
> +++ b/include/linux/mmu_context.h
> @@ -4,11 +4,6 @@
>  
>  #include <asm/mmu_context.h>
>  
> -struct mm_struct;
> -
> -void use_mm(struct mm_struct *mm);
> -void unuse_mm(struct mm_struct *mm);
> -
>  /* Architectures that care about IRQ state in switch_mm can override this.
*/
>  #ifndef switch_mm_irqs_off
>  # define switch_mm_irqs_off switch_mm
> diff --git a/kernel/kthread.c b/kernel/kthread.c
> index bfbfa481be3a..ce4610316377 100644
> --- a/kernel/kthread.c
> +++ b/kernel/kthread.c
> @@ -1,13 +1,17 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /* Kernel thread helper functions.
>   *   Copyright (C) 2004 IBM Corporation, Rusty Russell.
> + *   Copyright (C) 2009 Red Hat, Inc.
>   *
>   * Creation is done via kthreadd, so that we get a clean environment
>   * even if we're invoked from userspace (think modprobe, hotplug cpu,
>   * etc.).
>   */
>  #include <uapi/linux/sched/types.h>
> +#include <linux/mm.h>
> +#include <linux/mmu_context.h>
>  #include <linux/sched.h>
> +#include <linux/sched/mm.h>
>  #include <linux/sched/task.h>
>  #include <linux/kthread.h>
>  #include <linux/completion.h>
> @@ -25,6 +29,7 @@
>  #include <linux/numa.h>
>  #include <trace/events/sched.h>
>  
> +
>  static DEFINE_SPINLOCK(kthread_create_lock);
>  static LIST_HEAD(kthread_create_list);
>  struct task_struct *kthreadd_task;
> @@ -1203,6 +1208,57 @@ void kthread_destroy_worker(struct kthread_worker
*worker)
>  }
>  EXPORT_SYMBOL(kthread_destroy_worker);
>  
> +/*
> + * use_mm
> + *	Makes the calling kernel thread take on the specified
> + *	mm context.
> + *	(Note: this routine is intended to be called only
> + *	from a kernel thread context)
> + */
> +void use_mm(struct mm_struct *mm)
> +{
> +	struct mm_struct *active_mm;
> +	struct task_struct *tsk = current;
> +
> +	task_lock(tsk);
> +	active_mm = tsk->active_mm;
> +	if (active_mm != mm) {
> +		mmgrab(mm);
> +		tsk->active_mm = mm;
> +	}
> +	tsk->mm = mm;
> +	switch_mm(active_mm, mm, tsk);
> +	task_unlock(tsk);
> +#ifdef finish_arch_post_lock_switch
> +	finish_arch_post_lock_switch();
> +#endif
> +
> +	if (active_mm != mm)
> +		mmdrop(active_mm);
> +}
> +EXPORT_SYMBOL_GPL(use_mm);
> +
> +/*
> + * unuse_mm
> + *	Reverses the effect of use_mm, i.e. releases the
> + *	specified mm context which was earlier taken on
> + *	by the calling kernel thread
> + *	(Note: this routine is intended to be called only
> + *	from a kernel thread context)
> + */
> +void unuse_mm(struct mm_struct *mm)
> +{
> +	struct task_struct *tsk = current;
> +
> +	task_lock(tsk);
> +	sync_mm_rss(mm);
> +	tsk->mm = NULL;
> +	/* active_mm is still 'mm' */
> +	enter_lazy_tlb(mm, tsk);
> +	task_unlock(tsk);
> +}
> +EXPORT_SYMBOL_GPL(unuse_mm);
> +
>  #ifdef CONFIG_BLK_CGROUP
>  /**
>   * kthread_associate_blkcg - associate blkcg to current kthread
> diff --git a/mm/Makefile b/mm/Makefile
> index dbc8346d16ca..0af4ee81aed2 100644
> --- a/mm/Makefile
> +++ b/mm/Makefile
> @@ -41,7 +41,7 @@ obj-y			:= filemap.o mempool.o oom_kill.o fadvise.o \
>  			   maccess.o page-writeback.o \
>  			   readahead.o swap.o truncate.o vmscan.o shmem.o \
>  			   util.o mmzone.o vmstat.o backing-dev.o \
> -			   mm_init.o mmu_context.o percpu.o slab_common.o \
> +			   mm_init.o percpu.o slab_common.o \
>  			   compaction.o vmacache.o \
>  			   interval_tree.o list_lru.o workingset.o \
>  			   debug.o gup.o $(mmu-y)
> diff --git a/mm/mmu_context.c b/mm/mmu_context.c
> deleted file mode 100644
> index 3e612ae748e9..000000000000
> --- a/mm/mmu_context.c
> +++ /dev/null
> @@ -1,64 +0,0 @@
> -/* Copyright (C) 2009 Red Hat, Inc.
> - *
> - * See ../COPYING for licensing terms.
> - */
> -
> -#include <linux/mm.h>
> -#include <linux/sched.h>
> -#include <linux/sched/mm.h>
> -#include <linux/sched/task.h>
> -#include <linux/mmu_context.h>
> -#include <linux/export.h>
> -
> -#include <asm/mmu_context.h>
> -
> -/*
> - * use_mm
> - *	Makes the calling kernel thread take on the specified
> - *	mm context.
> - *	(Note: this routine is intended to be called only
> - *	from a kernel thread context)
> - */
> -void use_mm(struct mm_struct *mm)
> -{
> -	struct mm_struct *active_mm;
> -	struct task_struct *tsk = current;
> -
> -	task_lock(tsk);
> -	active_mm = tsk->active_mm;
> -	if (active_mm != mm) {
> -		mmgrab(mm);
> -		tsk->active_mm = mm;
> -	}
> -	tsk->mm = mm;
> -	switch_mm(active_mm, mm, tsk);
> -	task_unlock(tsk);
> -#ifdef finish_arch_post_lock_switch
> -	finish_arch_post_lock_switch();
> -#endif
> -
> -	if (active_mm != mm)
> -		mmdrop(active_mm);
> -}
> -EXPORT_SYMBOL_GPL(use_mm);
> -
> -/*
> - * unuse_mm
> - *	Reverses the effect of use_mm, i.e. releases the
> - *	specified mm context which was earlier taken on
> - *	by the calling kernel thread
> - *	(Note: this routine is intended to be called only
> - *	from a kernel thread context)
> - */
> -void unuse_mm(struct mm_struct *mm)
> -{
> -	struct task_struct *tsk = current;
> -
> -	task_lock(tsk);
> -	sync_mm_rss(mm);
> -	tsk->mm = NULL;
> -	/* active_mm is still 'mm' */
> -	enter_lazy_tlb(mm, tsk);
> -	task_unlock(tsk);
> -}
> -EXPORT_SYMBOL_GPL(unuse_mm);

Felix Kuehling

2020-Apr-06 16:10 UTC

head link

[PATCH 5/6] kernel: better document the use_mm/unuse_mm API contract

Am 2020-04-04 um 5:41 a.m. schrieb Christoph Hellwig:> Switch the function documentation to kerneldoc comments, and add
> WARN_ON_ONCE asserts that the calling thread is a kernel thread and
> does not have ->mm set (or has ->mm set in the case of unuse_mm).
>
> Also give the functions a kthread_ prefix to better document the
> use case.
>
> Signed-off-by: Christoph Hellwig <hch at lst.de>
Acked-by: Felix Kuehling <Felix.Kuehling at amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h |  4 +--
>  drivers/gpu/drm/i915/gvt/kvmgt.c           |  4 +--
>  drivers/usb/gadget/function/f_fs.c         |  4 +--
>  drivers/usb/gadget/legacy/inode.c          |  4 +--
>  drivers/vhost/vhost.c                      |  4 +--
>  fs/io-wq.c                                 |  6 ++--
>  fs/io_uring.c                              |  6 ++--
>  include/linux/kthread.h                    |  4 +--
>  kernel/kthread.c                           | 33 +++++++++++-----------
>  mm/oom_kill.c                              |  6 ++--
>  mm/vmacache.c                              |  4 +--
>  11 files changed, 39 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index bce5e93fefc8..63db84e09408 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -192,9 +192,9 @@ uint8_t amdgpu_amdkfd_get_xgmi_hops_count(struct
kgd_dev *dst, struct kgd_dev *s
>  			if ((mmptr) == current->mm) {			\
>  				valid = !get_user((dst), (wptr));	\
>  			} else if (current->flags & PF_KTHREAD) {	\
> -				use_mm(mmptr);				\
> +				kthread_use_mm(mmptr);			\
>  				valid = !get_user((dst), (wptr));	\
> -				unuse_mm(mmptr);			\
> +				kthread_unuse_mm(mmptr);		\
>  			}						\
>  			pagefault_enable();				\
>  		}							\
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index dee01c371bf5..92e9b340dbc2 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -2048,7 +2048,7 @@ static int kvmgt_rw_gpa(unsigned long handle,
unsigned long gpa,
>  	if (kthread) {
>  		if (!mmget_not_zero(kvm->mm))
>  			return -EFAULT;
> -		use_mm(kvm->mm);
> +		kthread_use_mm(kvm->mm);
>  	}
>  
>  	idx = srcu_read_lock(&kvm->srcu);
> @@ -2057,7 +2057,7 @@ static int kvmgt_rw_gpa(unsigned long handle,
unsigned long gpa,
>  	srcu_read_unlock(&kvm->srcu, idx);
>  
>  	if (kthread) {
> -		unuse_mm(kvm->mm);
> +		kthread_unuse_mm(kvm->mm);
>  		mmput(kvm->mm);
>  	}
>  
> diff --git a/drivers/usb/gadget/function/f_fs.c
b/drivers/usb/gadget/function/f_fs.c
> index c57b1b2507c6..d9e48bd7c692 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -827,9 +827,9 @@ static void ffs_user_copy_worker(struct work_struct
*work)
>  		mm_segment_t oldfs = get_fs();
>  
>  		set_fs(USER_DS);
> -		use_mm(io_data->mm);
> +		kthread_use_mm(io_data->mm);
>  		ret = ffs_copy_to_iter(io_data->buf, ret, &io_data->data);
> -		unuse_mm(io_data->mm);
> +		kthread_unuse_mm(io_data->mm);
>  		set_fs(oldfs);
>  	}
>  
> diff --git a/drivers/usb/gadget/legacy/inode.c
b/drivers/usb/gadget/legacy/inode.c
> index 8b5233888bf8..a05552bc2ff8 100644
> --- a/drivers/usb/gadget/legacy/inode.c
> +++ b/drivers/usb/gadget/legacy/inode.c
> @@ -462,9 +462,9 @@ static void ep_user_copy_worker(struct work_struct
*work)
>  	struct kiocb *iocb = priv->iocb;
>  	size_t ret;
>  
> -	use_mm(mm);
> +	kthread_use_mm(mm);
>  	ret = copy_to_iter(priv->buf, priv->actual, &priv->to);
> -	unuse_mm(mm);
> +	kthread_unuse_mm(mm);
>  	if (!ret)
>  		ret = -EFAULT;
>  
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 4e9ce54869af..1787d426a956 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -336,7 +336,7 @@ static int vhost_worker(void *data)
>  	mm_segment_t oldfs = get_fs();
>  
>  	set_fs(USER_DS);
> -	use_mm(dev->mm);
> +	kthread_use_mm(dev->mm);
>  
>  	for (;;) {
>  		/* mb paired w/ kthread_stop */
> @@ -364,7 +364,7 @@ static int vhost_worker(void *data)
>  				schedule();
>  		}
>  	}
> -	unuse_mm(dev->mm);
> +	kthread_unuse_mm(dev->mm);
>  	set_fs(oldfs);
>  	return 0;
>  }
> diff --git a/fs/io-wq.c b/fs/io-wq.c
> index c49c2bdbafb5..83c2868eff2a 100644
> --- a/fs/io-wq.c
> +++ b/fs/io-wq.c
> @@ -169,7 +169,7 @@ static bool __io_worker_unuse(struct io_wqe *wqe,
struct io_worker *worker)
>  		}
>  		__set_current_state(TASK_RUNNING);
>  		set_fs(KERNEL_DS);
> -		unuse_mm(worker->mm);
> +		kthread_unuse_mm(worker->mm);
>  		mmput(worker->mm);
>  		worker->mm = NULL;
>  	}
> @@ -416,7 +416,7 @@ static struct io_wq_work *io_get_next_work(struct
io_wqe *wqe)
>  static void io_wq_switch_mm(struct io_worker *worker, struct io_wq_work
*work)
>  {
>  	if (worker->mm) {
> -		unuse_mm(worker->mm);
> +		kthread_unuse_mm(worker->mm);
>  		mmput(worker->mm);
>  		worker->mm = NULL;
>  	}
> @@ -425,7 +425,7 @@ static void io_wq_switch_mm(struct io_worker *worker,
struct io_wq_work *work)
>  		return;
>  	}
>  	if (mmget_not_zero(work->mm)) {
> -		use_mm(work->mm);
> +		kthread_use_mm(work->mm);
>  		if (!worker->mm)
>  			set_fs(USER_DS);
>  		worker->mm = work->mm;
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 27a4ecb724ca..367406381044 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -5839,7 +5839,7 @@ static int io_submit_sqes(struct io_ring_ctx *ctx,
unsigned int nr,
>  				err = -EFAULT;
>  				goto fail_req;
>  			}
> -			use_mm(ctx->sqo_mm);
> +			kthread_use_mm(ctx->sqo_mm);
>  			*mm = ctx->sqo_mm;
>  		}
>  
> @@ -5911,7 +5911,7 @@ static int io_sq_thread(void *data)
>  			 * may sleep.
>  			 */
>  			if (cur_mm) {
> -				unuse_mm(cur_mm);
> +				kthread_unuse_mm(cur_mm);
>  				mmput(cur_mm);
>  				cur_mm = NULL;
>  			}
> @@ -5987,7 +5987,7 @@ static int io_sq_thread(void *data)
>  
>  	set_fs(old_fs);
>  	if (cur_mm) {
> -		unuse_mm(cur_mm);
> +		kthread_unuse_mm(cur_mm);
>  		mmput(cur_mm);
>  	}
>  	revert_creds(old_cred);
> diff --git a/include/linux/kthread.h b/include/linux/kthread.h
> index c2d40c9672d6..12258ea077cf 100644
> --- a/include/linux/kthread.h
> +++ b/include/linux/kthread.h
> @@ -200,8 +200,8 @@ bool kthread_cancel_delayed_work_sync(struct
kthread_delayed_work *work);
>  
>  void kthread_destroy_worker(struct kthread_worker *worker);
>  
> -void use_mm(struct mm_struct *mm);
> -void unuse_mm(struct mm_struct *mm);
> +void kthread_use_mm(struct mm_struct *mm);
> +void kthread_unuse_mm(struct mm_struct *mm);
>  
>  struct cgroup_subsys_state;
>  
> diff --git a/kernel/kthread.c b/kernel/kthread.c
> index ce4610316377..316db17f6b4f 100644
> --- a/kernel/kthread.c
> +++ b/kernel/kthread.c
> @@ -1208,18 +1208,18 @@ void kthread_destroy_worker(struct kthread_worker
*worker)
>  }
>  EXPORT_SYMBOL(kthread_destroy_worker);
>  
> -/*
> - * use_mm
> - *	Makes the calling kernel thread take on the specified
> - *	mm context.
> - *	(Note: this routine is intended to be called only
> - *	from a kernel thread context)
> +/**
> + * kthread_use_mm - make the calling kthread operate on an address space
> + * @mm: address space to operate on
>   */
> -void use_mm(struct mm_struct *mm)
> +void kthread_use_mm(struct mm_struct *mm)
>  {
>  	struct mm_struct *active_mm;
>  	struct task_struct *tsk = current;
>  
> +	WARN_ON_ONCE(!(tsk->flags & PF_KTHREAD));
> +	WARN_ON_ONCE(tsk->mm);
> +
>  	task_lock(tsk);
>  	active_mm = tsk->active_mm;
>  	if (active_mm != mm) {
> @@ -1236,20 +1236,19 @@ void use_mm(struct mm_struct *mm)
>  	if (active_mm != mm)
>  		mmdrop(active_mm);
>  }
> -EXPORT_SYMBOL_GPL(use_mm);
> +EXPORT_SYMBOL_GPL(kthread_use_mm);
>  
> -/*
> - * unuse_mm
> - *	Reverses the effect of use_mm, i.e. releases the
> - *	specified mm context which was earlier taken on
> - *	by the calling kernel thread
> - *	(Note: this routine is intended to be called only
> - *	from a kernel thread context)
> +/**
> + * kthread_use_mm - reverse the effect of kthread_use_mm()
> + * @mm: address space to operate on
>   */
> -void unuse_mm(struct mm_struct *mm)
> +void kthread_unuse_mm(struct mm_struct *mm)
>  {
>  	struct task_struct *tsk = current;
>  
> +	WARN_ON_ONCE(!(tsk->flags & PF_KTHREAD));
> +	WARN_ON_ONCE(!tsk->mm);
> +
>  	task_lock(tsk);
>  	sync_mm_rss(mm);
>  	tsk->mm = NULL;
> @@ -1257,7 +1256,7 @@ void unuse_mm(struct mm_struct *mm)
>  	enter_lazy_tlb(mm, tsk);
>  	task_unlock(tsk);
>  }
> -EXPORT_SYMBOL_GPL(unuse_mm);
> +EXPORT_SYMBOL_GPL(kthread_unuse_mm);
>  
>  #ifdef CONFIG_BLK_CGROUP
>  /**
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index dfc357614e56..958d2972313f 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -126,7 +126,7 @@ static bool oom_cpuset_eligible(struct task_struct
*tsk, struct oom_control *oc)
>  
>  /*
>   * The process p may have detached its own ->mm while exiting or
through
> - * use_mm(), but one or more of its subthreads may still have a valid
> + * kthread_use_mm(), but one or more of its subthreads may still have a
valid
>   * pointer.  Return p, or any of its subthreads with a valid ->mm, with
>   * task_lock() held.
>   */
> @@ -919,8 +919,8 @@ static void __oom_kill_process(struct task_struct
*victim, const char *message)
>  			continue;
>  		}
>  		/*
> -		 * No use_mm() user needs to read from the userspace so we are
> -		 * ok to reap it.
> +		 * No kthead_use_mm() user needs to read from the userspace so
> +		 * we are ok to reap it.
>  		 */
>  		if (unlikely(p->flags & PF_KTHREAD))
>  			continue;
> diff --git a/mm/vmacache.c b/mm/vmacache.c
> index cdc32a3b02fa..ceedbab82106 100644
> --- a/mm/vmacache.c
> +++ b/mm/vmacache.c
> @@ -25,8 +25,8 @@
>   * task's vmacache pertains to a different mm (ie, its own).  There is
>   * nothing we can do here.
>   *
> - * Also handle the case where a kernel thread has adopted this mm via
use_mm().
> - * That kernel thread's vmacache is not applicable to this mm.
> + * Also handle the case where a kernel thread has adopted this mm via
> + * kthread_use_mm(). That kernel thread's vmacache is not applicable
to this mm.
>   */
>  static inline bool vmacache_valid_mm(struct mm_struct *mm)
>  {

Michael S. Tsirkin

2020-Apr-06 21:49 UTC

head link

[PATCH 6/6] kernel: set USER_DS in kthread_use_mm

On Sat, Apr 04, 2020 at 11:41:01AM +0200, Christoph Hellwig
wrote:> Some architectures like arm64 and s390 require USER_DS to be set for
> kernel threads to access user address space, which is the whole purpose
> of kthread_use_mm, but other like x86 don't.  That has lead to a huge
> mess where some callers are fixed up once they are tested on said
> architectures, while others linger around and yet other like io_uring
> try to do "clever" optimizations for what usually is just a
trivial
> asignment to a member in the thread_struct for most architectures.
> 
> Make kthread_use_mm set USER_DS, and kthread_unuse_mm restore to the
> previous value instead.
> 
> Signed-off-by: Christoph Hellwig <hch at lst.de>
I'm ok with vhost bits:

Acked-by: Michael S. Tsirkin <mst at redhat.com>
> ---
>  drivers/usb/gadget/function/f_fs.c | 4 ----
>  drivers/vhost/vhost.c              | 3 ---
>  fs/io-wq.c                         | 8 ++------
>  fs/io_uring.c                      | 4 ----
>  kernel/kthread.c                   | 6 ++++++
>  5 files changed, 8 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_fs.c
b/drivers/usb/gadget/function/f_fs.c
> index d9e48bd7c692..a1198f4c527c 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -824,13 +824,9 @@ static void ffs_user_copy_worker(struct work_struct
*work)
>  	bool kiocb_has_eventfd = io_data->kiocb->ki_flags &
IOCB_EVENTFD;
>  
>  	if (io_data->read && ret > 0) {
> -		mm_segment_t oldfs = get_fs();
> -
> -		set_fs(USER_DS);
>  		kthread_use_mm(io_data->mm);
>  		ret = ffs_copy_to_iter(io_data->buf, ret, &io_data->data);
>  		kthread_unuse_mm(io_data->mm);
> -		set_fs(oldfs);
>  	}
>  
>  	io_data->kiocb->ki_complete(io_data->kiocb, ret, ret);
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 1787d426a956..b5229ae01d3b 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -333,9 +333,7 @@ static int vhost_worker(void *data)
>  	struct vhost_dev *dev = data;
>  	struct vhost_work *work, *work_next;
>  	struct llist_node *node;
> -	mm_segment_t oldfs = get_fs();
>  
> -	set_fs(USER_DS);
>  	kthread_use_mm(dev->mm);
>  
>  	for (;;) {
> @@ -365,7 +363,6 @@ static int vhost_worker(void *data)
>  		}
>  	}
>  	kthread_unuse_mm(dev->mm);
> -	set_fs(oldfs);
>  	return 0;
>  }
>  
> diff --git a/fs/io-wq.c b/fs/io-wq.c
> index 83c2868eff2a..75cc2f31816d 100644
> --- a/fs/io-wq.c
> +++ b/fs/io-wq.c
> @@ -168,7 +168,6 @@ static bool __io_worker_unuse(struct io_wqe *wqe,
struct io_worker *worker)
>  			dropped_lock = true;
>  		}
>  		__set_current_state(TASK_RUNNING);
> -		set_fs(KERNEL_DS);
>  		kthread_unuse_mm(worker->mm);
>  		mmput(worker->mm);
>  		worker->mm = NULL;
> @@ -420,14 +419,11 @@ static void io_wq_switch_mm(struct io_worker *worker,
struct io_wq_work *work)
>  		mmput(worker->mm);
>  		worker->mm = NULL;
>  	}
> -	if (!work->mm) {
> -		set_fs(KERNEL_DS);
> +	if (!work->mm)
>  		return;
> -	}
> +
>  	if (mmget_not_zero(work->mm)) {
>  		kthread_use_mm(work->mm);
> -		if (!worker->mm)
> -			set_fs(USER_DS);
>  		worker->mm = work->mm;
>  		/* hang on to this mm */
>  		work->mm = NULL;
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 367406381044..c332a34e8b34 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -5871,15 +5871,12 @@ static int io_sq_thread(void *data)
>  	struct io_ring_ctx *ctx = data;
>  	struct mm_struct *cur_mm = NULL;
>  	const struct cred *old_cred;
> -	mm_segment_t old_fs;
>  	DEFINE_WAIT(wait);
>  	unsigned long timeout;
>  	int ret = 0;
>  
>  	complete(&ctx->completions[1]);
>  
> -	old_fs = get_fs();
> -	set_fs(USER_DS);
>  	old_cred = override_creds(ctx->creds);
>  
>  	timeout = jiffies + ctx->sq_thread_idle;
> @@ -5985,7 +5982,6 @@ static int io_sq_thread(void *data)
>  	if (current->task_works)
>  		task_work_run();
>  
> -	set_fs(old_fs);
>  	if (cur_mm) {
>  		kthread_unuse_mm(cur_mm);
>  		mmput(cur_mm);
> diff --git a/kernel/kthread.c b/kernel/kthread.c
> index 316db17f6b4f..9e27d01b6d78 100644
> --- a/kernel/kthread.c
> +++ b/kernel/kthread.c
> @@ -52,6 +52,7 @@ struct kthread {
>  	unsigned long flags;
>  	unsigned int cpu;
>  	void *data;
> +	mm_segment_t oldfs;
>  	struct completion parked;
>  	struct completion exited;
>  #ifdef CONFIG_BLK_CGROUP
> @@ -1235,6 +1236,9 @@ void kthread_use_mm(struct mm_struct *mm)
>  
>  	if (active_mm != mm)
>  		mmdrop(active_mm);
> +
> +	to_kthread(tsk)->oldfs = get_fs();
> +	set_fs(USER_DS);
>  }
>  EXPORT_SYMBOL_GPL(kthread_use_mm);
>  
> @@ -1249,6 +1253,8 @@ void kthread_unuse_mm(struct mm_struct *mm)
>  	WARN_ON_ONCE(!(tsk->flags & PF_KTHREAD));
>  	WARN_ON_ONCE(!tsk->mm);
>  
> +	set_fs(to_kthread(tsk)->oldfs);
> +
>  	task_lock(tsk);
>  	sync_mm_rss(mm);
>  	tsk->mm = NULL;
> -- 
> 2.25.1

Christoph Hellwig

2020-Apr-13 13:08 UTC

head link

[PATCH 3/6] i915/gvt: remove unused xen bits

On Wed, Apr 08, 2020 at 09:44:37AM +0800, Zhenyu Wang
wrote:> On 2020.04.04 11:40:58 +0200, Christoph Hellwig wrote:
> > No Xen support anywhere here.  Remove a dead declaration and an unused
> > include.
> > 
> > Signed-off-by: Christoph Hellwig <hch at lst.de>
> > ---
> 
> We'll keep that off-tree.
> 
> Acked-by: Zhenyu Wang <zhenyuw at linux.intel.com>
Can you pick this up through the i915 tree?

Christoph Hellwig

2020-Apr-13 13:27 UTC

head link

[PATCH 2/6] i915/gvt/kvm: a NULL ->mm does not mean a thread is a kthread

On Mon, Apr 06, 2020 at 11:08:46PM -0400, Yan Zhao
wrote:> hi
> we were removing this code. see
> https://lore.kernel.org/kvm/20200313031109.7989-1-yan.y.zhao at intel.com/
This didn't make 5.7-rc1.
> The implementation of vfio_dma_rw() has been in vfio next tree.
>
https://github.com/awilliam/linux-vfio/commit/8d46c0cca5f4dc0538173d62cd36b1119b5105bc

This made 5.7-rc1, so I'll update the series to take it into account.

T> in vfio_dma_rw(),  we still use
> bool kthread = current->mm == NULL.
> because if current->mm != NULL and current->flags & PF_KTHREAD,
instead
> of calling use_mm(), we first check if (current->mm == mm) and allow
copy_to_user() if it's true.
> 
> Do you think it's all right?
I can't think of another way for a kernel thread to have a mm indeed.

Christoph Hellwig

2020-Apr-14 07:00 UTC

head link

[PATCH 2/6] i915/gvt/kvm: a NULL ->mm does not mean a thread is a kthread

On Mon, Apr 13, 2020 at 08:04:10PM -0400, Yan Zhao
wrote:> > I can't think of another way for a kernel thread to have a mm
indeed.
> for example, before calling to vfio_dma_rw(), a kernel thread has already
> called use_mm(), then its current->mm is not null, and it has flag
> PF_KTHREAD.
> in this case, we just want to allow the copy_to_user() directly if
> current->mm == mm, rather than call another use_mm() again.
> 
> do you think it makes sense?
I mean no other way than using use_mm.  That being said nesting
potentional use_mm callers sounds like a rather bad idea, and we
should avoid that.

Apparently Analagous Threads

Search for more apparently analagous threads

Linux Virtualization - Apr 2020 - [PATCH 5/6] kernel: better document the use_mm/unuse_mm API contract

improve use_mm / unuse_mm

[PATCH 1/6] amdgpu: a NULL ->mm does not mean a thread is a kthread

[PATCH 2/6] i915/gvt/kvm: a NULL ->mm does not mean a thread is a kthread

[PATCH 3/6] i915/gvt: remove unused xen bits

[PATCH 4/6] kernel: move use_mm/unuse_mm to kthread.c

[PATCH 5/6] kernel: better document the use_mm/unuse_mm API contract

[PATCH 6/6] kernel: set USER_DS in kthread_use_mm

[PATCH 2/6] i915/gvt/kvm: a NULL ->mm does not mean a thread is a kthread

[PATCH 1/6] amdgpu: a NULL ->mm does not mean a thread is a kthread

[PATCH 4/6] kernel: move use_mm/unuse_mm to kthread.c

[PATCH 5/6] kernel: better document the use_mm/unuse_mm API contract

[PATCH 6/6] kernel: set USER_DS in kthread_use_mm

[PATCH 3/6] i915/gvt: remove unused xen bits

[PATCH 2/6] i915/gvt/kvm: a NULL ->mm does not mean a thread is a kthread

[PATCH 2/6] i915/gvt/kvm: a NULL ->mm does not mean a thread is a kthread

Apparently Analagous Threads