* [PATCH 1/6] amdgpu: a NULL ->mm does not mean a thread is a kthread
2020-04-04 9:40 improve use_mm / unuse_mm Christoph Hellwig
@ 2020-04-04 9:40 ` Christoph Hellwig
2020-04-06 16:07 ` Felix Kuehling
2020-04-04 9:40 ` [PATCH 2/6] i915/gvt/kvm: " Christoph Hellwig
` (4 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Christoph Hellwig @ 2020-04-04 9:40 UTC (permalink / raw)
To: Linus Torvalds, Andrew Morton
Cc: Al Viro, Felix Kuehling, Alex Deucher, Zhenyu Wang, Zhi Wang,
Felipe Balbi, Michael S. Tsirkin, Jason Wang, Jens Axboe,
linux-kernel, amd-gfx, intel-gvt-dev, intel-gfx, linux-usb,
virtualization, linux-fsdevel, io-uring, linux-mm
Use the proper API instead.
Fixes: 70539bd795002 ("drm/amd: Update MEC HQD loading code for KFD")
Signed-off-by: Christoph Hellwig <[email protected]>
---
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 1/6] amdgpu: a NULL ->mm does not mean a thread is a kthread
2020-04-04 9:40 ` [PATCH 1/6] amdgpu: a NULL ->mm does not mean a thread is a kthread Christoph Hellwig
@ 2020-04-06 16:07 ` Felix Kuehling
0 siblings, 0 replies; 21+ messages in thread
From: Felix Kuehling @ 2020-04-06 16:07 UTC (permalink / raw)
To: Christoph Hellwig, Linus Torvalds, Andrew Morton
Cc: Al Viro, Alex Deucher, Zhenyu Wang, Zhi Wang, Felipe Balbi,
Michael S. Tsirkin, Jason Wang, Jens Axboe, linux-kernel, amd-gfx,
intel-gvt-dev, intel-gfx, linux-usb, virtualization,
linux-fsdevel, io-uring, linux-mm
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 <[email protected]>
Reviewed-by: Felix Kuehling <[email protected]>
> ---
> 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); \
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/6] i915/gvt/kvm: a NULL ->mm does not mean a thread is a kthread
2020-04-04 9:40 improve use_mm / unuse_mm Christoph Hellwig
2020-04-04 9:40 ` [PATCH 1/6] amdgpu: a NULL ->mm does not mean a thread is a kthread Christoph Hellwig
@ 2020-04-04 9:40 ` Christoph Hellwig
2020-04-04 10:05 ` Sergei Shtylyov
2020-04-07 3:08 ` Yan Zhao
2020-04-04 9:40 ` [PATCH 3/6] i915/gvt: remove unused xen bits Christoph Hellwig
` (3 subsequent siblings)
5 siblings, 2 replies; 21+ messages in thread
From: Christoph Hellwig @ 2020-04-04 9:40 UTC (permalink / raw)
To: Linus Torvalds, Andrew Morton
Cc: Al Viro, Felix Kuehling, Alex Deucher, Zhenyu Wang, Zhi Wang,
Felipe Balbi, Michael S. Tsirkin, Jason Wang, Jens Axboe,
linux-kernel, amd-gfx, intel-gvt-dev, intel-gfx, linux-usb,
virtualization, linux-fsdevel, io-uring, linux-mm
Use the proper API instead.
Fixes: f440c8a572d7 ("drm/i915/gvt/kvmgt: read/write GPA via KVM API")
Signed-off-by: Christoph Hellwig <[email protected]>
---
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 2/6] i915/gvt/kvm: a NULL ->mm does not mean a thread is a kthread
2020-04-04 9:40 ` [PATCH 2/6] i915/gvt/kvm: " Christoph Hellwig
@ 2020-04-04 10:05 ` Sergei Shtylyov
2020-04-07 3:08 ` Yan Zhao
1 sibling, 0 replies; 21+ messages in thread
From: Sergei Shtylyov @ 2020-04-04 10:05 UTC (permalink / raw)
To: Christoph Hellwig, Linus Torvalds, Andrew Morton
Cc: Al Viro, Felix Kuehling, Alex Deucher, Zhenyu Wang, Zhi Wang,
Felipe Balbi, Michael S. Tsirkin, Jason Wang, Jens Axboe,
linux-kernel, amd-gfx, intel-gvt-dev, intel-gfx, linux-usb,
virtualization, linux-fsdevel, io-uring, linux-mm
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 <[email protected]>
> ---
> 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
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 2/6] i915/gvt/kvm: a NULL ->mm does not mean a thread is a kthread
2020-04-04 9:40 ` [PATCH 2/6] i915/gvt/kvm: " Christoph Hellwig
2020-04-04 10:05 ` Sergei Shtylyov
@ 2020-04-07 3:08 ` Yan Zhao
2020-04-13 13:27 ` Christoph Hellwig
1 sibling, 1 reply; 21+ messages in thread
From: Yan Zhao @ 2020-04-07 3:08 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Linus Torvalds, Andrew Morton, Jens Axboe, Felipe Balbi, amd-gfx,
Michael S. Tsirkin, Felix Kuehling, linux-usb, io-uring,
linux-kernel, Zhenyu Wang, virtualization, linux-mm,
linux-fsdevel, Al Viro, intel-gfx, Alex Deucher, intel-gvt-dev,
Jason Wang, Zhi Wang
On Sat, Apr 04, 2020 at 11:40:57AM +0200, 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 <[email protected]>
> ---
> 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
>
hi
we were removing this code. see
https://lore.kernel.org/kvm/[email protected]/
The implementation of vfio_dma_rw() has been in vfio next tree.
https://github.com/awilliam/linux-vfio/commit/8d46c0cca5f4dc0538173d62cd36b1119b5105bc
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?
Thanks
Yan
> _______________________________________________
> intel-gvt-dev mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 2/6] i915/gvt/kvm: a NULL ->mm does not mean a thread is a kthread
2020-04-07 3:08 ` Yan Zhao
@ 2020-04-13 13:27 ` Christoph Hellwig
2020-04-14 0:04 ` Yan Zhao
0 siblings, 1 reply; 21+ messages in thread
From: Christoph Hellwig @ 2020-04-13 13:27 UTC (permalink / raw)
To: Yan Zhao
Cc: Christoph Hellwig, Linus Torvalds, Andrew Morton, Jens Axboe,
Felipe Balbi, amd-gfx, Michael S. Tsirkin, Felix Kuehling,
linux-usb, io-uring, linux-kernel, Zhenyu Wang, virtualization,
linux-mm, linux-fsdevel, Al Viro, intel-gfx, Alex Deucher,
intel-gvt-dev, Jason Wang, Zhi Wang
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/[email protected]/
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/6] i915/gvt/kvm: a NULL ->mm does not mean a thread is a kthread
2020-04-13 13:27 ` Christoph Hellwig
@ 2020-04-14 0:04 ` Yan Zhao
2020-04-14 7:00 ` Christoph Hellwig
0 siblings, 1 reply; 21+ messages in thread
From: Yan Zhao @ 2020-04-14 0:04 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, Felipe Balbi, Michael S. Tsirkin, Jason Wang,
intel-gvt-dev, Felix Kuehling, linux-usb, linux-kernel, amd-gfx,
io-uring, linux-mm, Zhenyu Wang, intel-gfx, linux-fsdevel,
Alex Deucher, Andrew Morton, virtualization, Linus Torvalds,
Zhi Wang, Al Viro
On Mon, Apr 13, 2020 at 03:27:30PM +0200, Christoph Hellwig wrote:
> 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/[email protected]/
>
> 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.
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?
Thanks
Yan
> _______________________________________________
> intel-gvt-dev mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/6] i915/gvt/kvm: a NULL ->mm does not mean a thread is a kthread
2020-04-14 0:04 ` Yan Zhao
@ 2020-04-14 7:00 ` Christoph Hellwig
2020-04-14 7:03 ` Yan Zhao
0 siblings, 1 reply; 21+ messages in thread
From: Christoph Hellwig @ 2020-04-14 7:00 UTC (permalink / raw)
To: Yan Zhao
Cc: Christoph Hellwig, Jens Axboe, Felipe Balbi, Michael S. Tsirkin,
Jason Wang, intel-gvt-dev, Felix Kuehling, linux-usb,
linux-kernel, amd-gfx, io-uring, linux-mm, Zhenyu Wang, intel-gfx,
linux-fsdevel, Alex Deucher, Andrew Morton, virtualization,
Linus Torvalds, Zhi Wang, Al Viro
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.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/6] i915/gvt/kvm: a NULL ->mm does not mean a thread is a kthread
2020-04-14 7:00 ` Christoph Hellwig
@ 2020-04-14 7:03 ` Yan Zhao
0 siblings, 0 replies; 21+ messages in thread
From: Yan Zhao @ 2020-04-14 7:03 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, Felipe Balbi, Michael S. Tsirkin, Jason Wang,
intel-gvt-dev, Felix Kuehling, linux-usb, linux-kernel, amd-gfx,
io-uring, linux-mm, Zhenyu Wang, intel-gfx, linux-fsdevel,
Alex Deucher, Andrew Morton, virtualization, Linus Torvalds,
Zhi Wang, Al Viro
On Tue, Apr 14, 2020 at 09:00:13AM +0200, Christoph Hellwig wrote:
> 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.
yes, agree.
I was explaining why we just use "current->mm == NULL"
(not "current->flag & PF_KTHREAD") as a criteria to call use_mm()
in vfio_dma_rw(), which you might ask us when you take that part into your
series. :)
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/6] i915/gvt: remove unused xen bits
2020-04-04 9:40 improve use_mm / unuse_mm Christoph Hellwig
2020-04-04 9:40 ` [PATCH 1/6] amdgpu: a NULL ->mm does not mean a thread is a kthread Christoph Hellwig
2020-04-04 9:40 ` [PATCH 2/6] i915/gvt/kvm: " Christoph Hellwig
@ 2020-04-04 9:40 ` Christoph Hellwig
2020-04-08 1:44 ` Zhenyu Wang
2020-04-04 9:40 ` [PATCH 4/6] kernel: move use_mm/unuse_mm to kthread.c Christoph Hellwig
` (2 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Christoph Hellwig @ 2020-04-04 9:40 UTC (permalink / raw)
To: Linus Torvalds, Andrew Morton
Cc: Al Viro, Felix Kuehling, Alex Deucher, Zhenyu Wang, Zhi Wang,
Felipe Balbi, Michael S. Tsirkin, Jason Wang, Jens Axboe,
linux-kernel, amd-gfx, intel-gvt-dev, intel-gfx, linux-usb,
virtualization, linux-fsdevel, io-uring, linux-mm
No Xen support anywhere here. Remove a dead declaration and an unused
include.
Signed-off-by: Christoph Hellwig <[email protected]>
---
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 3/6] i915/gvt: remove unused xen bits
2020-04-04 9:40 ` [PATCH 3/6] i915/gvt: remove unused xen bits Christoph Hellwig
@ 2020-04-08 1:44 ` Zhenyu Wang
2020-04-13 13:08 ` Christoph Hellwig
0 siblings, 1 reply; 21+ messages in thread
From: Zhenyu Wang @ 2020-04-08 1:44 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Linus Torvalds, Andrew Morton, Jens Axboe, Felipe Balbi, amd-gfx,
Michael S. Tsirkin, Felix Kuehling, linux-usb, io-uring,
linux-kernel, Zhenyu Wang, virtualization, linux-mm,
linux-fsdevel, Al Viro, intel-gfx, Alex Deucher, intel-gvt-dev,
Jason Wang, Zhi Wang
[-- Attachment #1: Type: text/plain, Size: 1528 bytes --]
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 <[email protected]>
> ---
We'll keep that off-tree.
Acked-by: Zhenyu Wang <[email protected]>
Thanks
> 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
>
> _______________________________________________
> intel-gvt-dev mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
--
Open Source Technology Center, Intel ltd.
$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 3/6] i915/gvt: remove unused xen bits
2020-04-08 1:44 ` Zhenyu Wang
@ 2020-04-13 13:08 ` Christoph Hellwig
2020-04-14 3:04 ` Zhenyu Wang
0 siblings, 1 reply; 21+ messages in thread
From: Christoph Hellwig @ 2020-04-13 13:08 UTC (permalink / raw)
To: Zhenyu Wang
Cc: Christoph Hellwig, Linus Torvalds, Andrew Morton, Jens Axboe,
Felipe Balbi, amd-gfx, Michael S. Tsirkin, Felix Kuehling,
linux-usb, io-uring, linux-kernel, virtualization, linux-mm,
linux-fsdevel, Al Viro, intel-gfx, Alex Deucher, intel-gvt-dev,
Jason Wang, Zhi Wang
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 <[email protected]>
> > ---
>
> We'll keep that off-tree.
>
> Acked-by: Zhenyu Wang <[email protected]>
Can you pick this up through the i915 tree?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 3/6] i915/gvt: remove unused xen bits
2020-04-13 13:08 ` Christoph Hellwig
@ 2020-04-14 3:04 ` Zhenyu Wang
0 siblings, 0 replies; 21+ messages in thread
From: Zhenyu Wang @ 2020-04-14 3:04 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Linus Torvalds, Andrew Morton, Jens Axboe, Felipe Balbi, amd-gfx,
Michael S. Tsirkin, Felix Kuehling, linux-usb, io-uring,
linux-kernel, virtualization, linux-mm, linux-fsdevel, Al Viro,
intel-gfx, Alex Deucher, intel-gvt-dev, Jason Wang, Zhi Wang
[-- Attachment #1: Type: text/plain, Size: 634 bytes --]
On 2020.04.13 15:08:06 +0200, Christoph Hellwig wrote:
> 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 <[email protected]>
> > > ---
> >
> > We'll keep that off-tree.
> >
> > Acked-by: Zhenyu Wang <[email protected]>
>
> Can you pick this up through the i915 tree?
Yes, I'll pick this.
Thanks
--
Open Source Technology Center, Intel ltd.
$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 4/6] kernel: move use_mm/unuse_mm to kthread.c
2020-04-04 9:40 improve use_mm / unuse_mm Christoph Hellwig
` (2 preceding siblings ...)
2020-04-04 9:40 ` [PATCH 3/6] i915/gvt: remove unused xen bits Christoph Hellwig
@ 2020-04-04 9:40 ` Christoph Hellwig
2020-04-06 16:09 ` Felix Kuehling
2020-04-04 9:41 ` [PATCH 5/6] kernel: better document the use_mm/unuse_mm API contract Christoph Hellwig
2020-04-04 9:41 ` [PATCH 6/6] kernel: set USER_DS in kthread_use_mm Christoph Hellwig
5 siblings, 1 reply; 21+ messages in thread
From: Christoph Hellwig @ 2020-04-04 9:40 UTC (permalink / raw)
To: Linus Torvalds, Andrew Morton
Cc: Al Viro, Felix Kuehling, Alex Deucher, Zhenyu Wang, Zhi Wang,
Felipe Balbi, Michael S. Tsirkin, Jason Wang, Jens Axboe,
linux-kernel, amd-gfx, intel-gvt-dev, intel-gfx, linux-usb,
virtualization, linux-fsdevel, io-uring, linux-mm
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 <[email protected]>
---
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 4/6] kernel: move use_mm/unuse_mm to kthread.c
2020-04-04 9:40 ` [PATCH 4/6] kernel: move use_mm/unuse_mm to kthread.c Christoph Hellwig
@ 2020-04-06 16:09 ` Felix Kuehling
0 siblings, 0 replies; 21+ messages in thread
From: Felix Kuehling @ 2020-04-06 16:09 UTC (permalink / raw)
To: Christoph Hellwig, Linus Torvalds, Andrew Morton
Cc: Al Viro, Alex Deucher, Zhenyu Wang, Zhi Wang, Felipe Balbi,
Michael S. Tsirkin, Jason Wang, Jens Axboe, linux-kernel, amd-gfx,
intel-gvt-dev, intel-gfx, linux-usb, virtualization,
linux-fsdevel, io-uring, linux-mm
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 <[email protected]>
Acked-by: Felix Kuehling <[email protected]>
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);
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 5/6] kernel: better document the use_mm/unuse_mm API contract
2020-04-04 9:40 improve use_mm / unuse_mm Christoph Hellwig
` (3 preceding siblings ...)
2020-04-04 9:40 ` [PATCH 4/6] kernel: move use_mm/unuse_mm to kthread.c Christoph Hellwig
@ 2020-04-04 9:41 ` Christoph Hellwig
2020-04-04 13:07 ` [Intel-gfx] " Pavel Begunkov
2020-04-06 16:10 ` Felix Kuehling
2020-04-04 9:41 ` [PATCH 6/6] kernel: set USER_DS in kthread_use_mm Christoph Hellwig
5 siblings, 2 replies; 21+ messages in thread
From: Christoph Hellwig @ 2020-04-04 9:41 UTC (permalink / raw)
To: Linus Torvalds, Andrew Morton
Cc: Al Viro, Felix Kuehling, Alex Deucher, Zhenyu Wang, Zhi Wang,
Felipe Balbi, Michael S. Tsirkin, Jason Wang, Jens Axboe,
linux-kernel, amd-gfx, intel-gvt-dev, intel-gfx, linux-usb,
virtualization, linux-fsdevel, io-uring, linux-mm
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 <[email protected]>
---
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Intel-gfx] [PATCH 5/6] kernel: better document the use_mm/unuse_mm API contract
2020-04-04 9:41 ` [PATCH 5/6] kernel: better document the use_mm/unuse_mm API contract Christoph Hellwig
@ 2020-04-04 13:07 ` Pavel Begunkov
2020-04-06 16:10 ` Felix Kuehling
1 sibling, 0 replies; 21+ messages in thread
From: Pavel Begunkov @ 2020-04-04 13:07 UTC (permalink / raw)
To: Christoph Hellwig, Linus Torvalds, Andrew Morton
Cc: Jens Axboe, Felipe Balbi, amd-gfx, Michael S. Tsirkin,
Felix Kuehling, linux-usb, io-uring, linux-kernel, virtualization,
linux-mm, linux-fsdevel, Al Viro, intel-gfx, Alex Deucher,
intel-gvt-dev, Jason Wang
On 04/04/2020 12:41, Christoph Hellwig wrote:
> 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.
>
io_uring and io-wq bits LGTM.
> --- a/include/linux/kthread.h
> +++ b/include/linux/kthread.h
...
> -/*
> - * 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()
s/kthread_use_mm/kthread_unuse_mm/
for the first one
> + * @mm: address space to operate on
> */
> -void unuse_mm(struct mm_struct *mm)
> +void kthread_unuse_mm(struct mm_struct *mm)
> {
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 5/6] kernel: better document the use_mm/unuse_mm API contract
2020-04-04 9:41 ` [PATCH 5/6] kernel: better document the use_mm/unuse_mm API contract Christoph Hellwig
2020-04-04 13:07 ` [Intel-gfx] " Pavel Begunkov
@ 2020-04-06 16:10 ` Felix Kuehling
1 sibling, 0 replies; 21+ messages in thread
From: Felix Kuehling @ 2020-04-06 16:10 UTC (permalink / raw)
To: Christoph Hellwig, Linus Torvalds, Andrew Morton
Cc: Al Viro, Alex Deucher, Zhenyu Wang, Zhi Wang, Felipe Balbi,
Michael S. Tsirkin, Jason Wang, Jens Axboe, linux-kernel, amd-gfx,
intel-gvt-dev, intel-gfx, linux-usb, virtualization,
linux-fsdevel, io-uring, linux-mm
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 <[email protected]>
Acked-by: Felix Kuehling <[email protected]>
> ---
> 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)
> {
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 6/6] kernel: set USER_DS in kthread_use_mm
2020-04-04 9:40 improve use_mm / unuse_mm Christoph Hellwig
` (4 preceding siblings ...)
2020-04-04 9:41 ` [PATCH 5/6] kernel: better document the use_mm/unuse_mm API contract Christoph Hellwig
@ 2020-04-04 9:41 ` Christoph Hellwig
2020-04-06 21:49 ` Michael S. Tsirkin
5 siblings, 1 reply; 21+ messages in thread
From: Christoph Hellwig @ 2020-04-04 9:41 UTC (permalink / raw)
To: Linus Torvalds, Andrew Morton
Cc: Al Viro, Felix Kuehling, Alex Deucher, Zhenyu Wang, Zhi Wang,
Felipe Balbi, Michael S. Tsirkin, Jason Wang, Jens Axboe,
linux-kernel, amd-gfx, intel-gvt-dev, intel-gfx, linux-usb,
virtualization, linux-fsdevel, io-uring, linux-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 <[email protected]>
---
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 6/6] kernel: set USER_DS in kthread_use_mm
2020-04-04 9:41 ` [PATCH 6/6] kernel: set USER_DS in kthread_use_mm Christoph Hellwig
@ 2020-04-06 21:49 ` Michael S. Tsirkin
0 siblings, 0 replies; 21+ messages in thread
From: Michael S. Tsirkin @ 2020-04-06 21:49 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Linus Torvalds, Andrew Morton, Al Viro, Felix Kuehling,
Alex Deucher, Zhenyu Wang, Zhi Wang, Felipe Balbi, Jason Wang,
Jens Axboe, linux-kernel, amd-gfx, intel-gvt-dev, intel-gfx,
linux-usb, virtualization, linux-fsdevel, io-uring, linux-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 <[email protected]>
I'm ok with vhost bits:
Acked-by: Michael S. Tsirkin <[email protected]>
> ---
> 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
^ permalink raw reply [flat|nested] 21+ messages in thread