All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC v4 0/2] drm/i915/selftests: Use safe userspace memory for mappings
@ 2026-04-08  8:30 Krzysztof Karas
  2026-04-08  8:30 ` [PATCH v4 1/2] drm/i915/selftests: Prevent userspace mapping invalidation Krzysztof Karas
  2026-04-08  8:30 ` [PATCH v4 2/2] drm/i915/selftests: Run vma tests only if current->mm is present Krzysztof Karas
  0 siblings, 2 replies; 7+ messages in thread
From: Krzysztof Karas @ 2026-04-08  8:30 UTC (permalink / raw)
  To: intel-gfx
  Cc: Andi Shyti, Sebastian Brzezinka, Krzysztof Niemiec,
	Janusz Krzysztofik, Krzysztof Karas

Test-with: 20260330102400.1157658-1-krzysztof.karas@intel.com

Currently, i915 selftests use unknown process's address space to
perform mappings to userspace memory. This is problematic,
because there is no control over lifetime of the memory of such
task, so the test would occasionally borrow memory scheduled for
or in the middle of a cleanup causing SIGBUS errors.

Utilize user-provided PID of running userspace process to
perfofm mapping in a safe environment.

v4:
 * Rearrange the code in patches. (Janusz)

Krzysztof Karas (2):
  drm/i915/selftests: Prevent userspace mapping invalidation
  drm/i915/selftests: Run vma tests only if current->mm is present

 .../drm/i915/gem/selftests/i915_gem_mman.c    | 14 +++--
 drivers/gpu/drm/i915/i915_selftest.h          |  1 +
 .../gpu/drm/i915/selftests/i915_selftest.c    | 51 +++++++++++++++++++
 3 files changed, 58 insertions(+), 8 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v4 1/2] drm/i915/selftests: Prevent userspace mapping invalidation
  2026-04-08  8:30 [RFC v4 0/2] drm/i915/selftests: Use safe userspace memory for mappings Krzysztof Karas
@ 2026-04-08  8:30 ` Krzysztof Karas
  2026-04-08  9:20   ` Janusz Krzysztofik
  2026-04-09 11:09   ` Andi Shyti
  2026-04-08  8:30 ` [PATCH v4 2/2] drm/i915/selftests: Run vma tests only if current->mm is present Krzysztof Karas
  1 sibling, 2 replies; 7+ messages in thread
From: Krzysztof Karas @ 2026-04-08  8:30 UTC (permalink / raw)
  To: intel-gfx
  Cc: Andi Shyti, Sebastian Brzezinka, Krzysztof Niemiec,
	Janusz Krzysztofik, Krzysztof Karas

Migration testing in i915 assumes current task's address space
to allocate new userspace mapping and uses it without
registering real user for that address space in mm_struct.
On single NUMA node setups PCI probe executes in the same
context as userspace process calling the test (i915_selftest
from IGT), but when multiple nodes are available, the PCI code
puts probe into a kernel workqueue. This switches execution in
a kworker, which does not have its own address space in
userspace and must borrow such memory from another process, so
"current->active_mm" is unknown at the start of the test.

It was observed that mm->mm_users would occasionally be 0
or drop to 0 during the test due to short delay between
scheduling and executing work in forked process, which reaped
userspace mappings, further leading to failures upon reading
from userland memory.

Prevent this by adding a PID parameter to a trusted task, so its
mm struct may be used if needed.

Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14204
Fixes: 34b1c1c71d37 ("i915/selftest/igt_mmap: let mmap tests run in kthread")
Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
---
v2 (Janusz):
 * Reword and shorten commit message to be more precise.
 * Reorder variable declarations to follow upside down christmas
 tree style.

v3 (Andi):
 * Prevent PID and mm leaks.
 * Remove a flag and use mm pointer to determine whether to
 release references to the memory.

v4:
 * Revert !current->mm check. (Janusz, Sebastian)
 * Drop refernce to mm sooner. (Janusz)
 * Ensure kthread_use_mm did its job. (Janusz)

 drivers/gpu/drm/i915/i915_selftest.h          |  1 +
 .../gpu/drm/i915/selftests/i915_selftest.c    | 51 +++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_selftest.h b/drivers/gpu/drm/i915/i915_selftest.h
index 72922028f4ba..e29ca298e7eb 100644
--- a/drivers/gpu/drm/i915/i915_selftest.h
+++ b/drivers/gpu/drm/i915/i915_selftest.h
@@ -35,6 +35,7 @@ struct i915_selftest {
 	unsigned long timeout_jiffies;
 	unsigned int timeout_ms;
 	unsigned int random_seed;
+	unsigned int userspace_pid;
 	char *filter;
 	int mock;
 	int live;
diff --git a/drivers/gpu/drm/i915/selftests/i915_selftest.c b/drivers/gpu/drm/i915/selftests/i915_selftest.c
index 8460f0a70d04..45fe750b799d 100644
--- a/drivers/gpu/drm/i915/selftests/i915_selftest.c
+++ b/drivers/gpu/drm/i915/selftests/i915_selftest.c
@@ -186,6 +186,8 @@ static int __run_selftests(const char *name,
 			   unsigned int count,
 			   void *data)
 {
+	int u_pid_nr = i915_selftest.userspace_pid;
+	struct mm_struct *mm = NULL;
 	int err = 0;
 
 	while (!i915_selftest.random_seed)
@@ -201,6 +203,50 @@ static int __run_selftests(const char *name,
 	pr_info(DRIVER_NAME ": Performing %s selftests with st_random_seed=0x%x st_timeout=%u\n",
 		name, i915_selftest.random_seed, i915_selftest.timeout_ms);
 
+	/**
+	 * If we are running in a kthread on a multi NUMA system and the user passed
+	 * a valid PID of a userspace task, then we may borrow its address space
+	 * to prepare a safe environment for the mmap selftests.
+	 */
+	if (!current->mm) {
+		struct pid *u_pid;
+		struct task_struct *task;
+
+		if (!u_pid_nr) {
+			pr_warn("No current->mm and no PID provided to safely borrow userspace memory from.\n"
+				"This may lead to switching off tests requiring that for mappings");
+			goto run_tests;
+		}
+
+		u_pid = find_get_pid(u_pid_nr);
+
+		if (!u_pid) {
+			pr_warn("Could not find PID: %d\n", u_pid_nr);
+			goto run_tests;
+		}
+
+		task = get_pid_task(u_pid, PIDTYPE_PID);
+		put_pid(u_pid);
+		if (!task) {
+			pr_warn("Could not find userspace task for PID: %d\n", u_pid_nr);
+			goto run_tests;
+		}
+
+		mm = get_task_mm(task);
+		put_task_struct(task);
+		if (!mm) {
+			pr_warn("Could not find address space of task with PID: %d\n", u_pid_nr);
+			goto run_tests;
+		}
+
+		kthread_use_mm(mm);
+		mmput_async(mm);
+		if (unlikely(!current->mm)) {
+			pr_warn("Could not set mm as current->mm\n");
+		}
+	}
+
+run_tests:
 	/* Tests are listed in order in i915_*_selftests.h */
 	for (; count--; st++) {
 		if (!st->enabled)
@@ -226,6 +272,9 @@ static int __run_selftests(const char *name,
 		 st->name, err))
 		err = -1;
 
+	if (mm)
+		kthread_unuse_mm(mm);
+
 	return err;
 }
 
@@ -507,6 +556,8 @@ void igt_hexdump(const void *buf, size_t len)
 module_param_named(st_random_seed, i915_selftest.random_seed, uint, 0400);
 module_param_named(st_timeout, i915_selftest.timeout_ms, uint, 0400);
 module_param_named(st_filter, i915_selftest.filter, charp, 0400);
+module_param_named(st_userspace_pid, i915_selftest.userspace_pid, uint, 0400);
+MODULE_PARM_DESC(st_userspace_pid, "For usage in tests that map userspace memory and require address space with controllable lifetime.");
 
 module_param_named_unsafe(mock_selftests, i915_selftest.mock, int, 0400);
 MODULE_PARM_DESC(mock_selftests, "Run selftests before loading, using mock hardware (0:disabled [default], 1:run tests then load driver, -1:run tests then leave dummy module)");
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v4 2/2] drm/i915/selftests: Run vma tests only if current->mm is present
  2026-04-08  8:30 [RFC v4 0/2] drm/i915/selftests: Use safe userspace memory for mappings Krzysztof Karas
  2026-04-08  8:30 ` [PATCH v4 1/2] drm/i915/selftests: Prevent userspace mapping invalidation Krzysztof Karas
@ 2026-04-08  8:30 ` Krzysztof Karas
  2026-04-08  9:25   ` Janusz Krzysztofik
  1 sibling, 1 reply; 7+ messages in thread
From: Krzysztof Karas @ 2026-04-08  8:30 UTC (permalink / raw)
  To: intel-gfx
  Cc: Andi Shyti, Sebastian Brzezinka, Krzysztof Niemiec,
	Janusz Krzysztofik, Krzysztof Karas

This set of tests require userspace memory to map objects, so
run them only if this that memory is available.

Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
---
 drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 9d454d0b46f2..0752e758b01b 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -1847,11 +1847,12 @@ static int igt_mmap_revoke(void *arg)
 int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
 {
 	int ret;
-	bool unuse_mm = false;
 	static const struct i915_subtest tests[] = {
 		SUBTEST(igt_partial_tiling),
 		SUBTEST(igt_smoke_tiling),
 		SUBTEST(igt_mmap_offset_exhaustion),
+	};
+	static const struct i915_subtest vma_tests[] = {
 		SUBTEST(igt_mmap),
 		SUBTEST(igt_mmap_migrate),
 		SUBTEST(igt_mmap_access),
@@ -1859,15 +1860,12 @@ int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
 		SUBTEST(igt_mmap_gpu),
 	};
 
-	if (!current->mm) {
-		kthread_use_mm(current->active_mm);
-		unuse_mm = true;
-	}
-
 	ret = i915_live_subtests(tests, i915);
+	if (ret)
+		return ret;
 
-	if (unuse_mm)
-		kthread_unuse_mm(current->active_mm);
+	if (current->mm)
+		ret = i915_live_subtests(vma_tests, i915);
 
 	return ret;
 }
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] drm/i915/selftests: Prevent userspace mapping invalidation
  2026-04-08  8:30 ` [PATCH v4 1/2] drm/i915/selftests: Prevent userspace mapping invalidation Krzysztof Karas
@ 2026-04-08  9:20   ` Janusz Krzysztofik
  2026-04-09  6:21     ` Krzysztof Karas
  2026-04-09 11:09   ` Andi Shyti
  1 sibling, 1 reply; 7+ messages in thread
From: Janusz Krzysztofik @ 2026-04-08  9:20 UTC (permalink / raw)
  To: Krzysztof Karas, intel-gfx
  Cc: Andi Shyti, Sebastian Brzezinka, Krzysztof Niemiec

Hi Krzysztof,

I found it not quite correct what I suggested before, see below.

On Wed, 2026-04-08 at 08:30 +0000, Krzysztof Karas wrote:
> Migration testing in i915 assumes current task's address space
> to allocate new userspace mapping and uses it without
> registering real user for that address space in mm_struct.
> On single NUMA node setups PCI probe executes in the same
> context as userspace process calling the test (i915_selftest
> from IGT), but when multiple nodes are available, the PCI code
> puts probe into a kernel workqueue. This switches execution in
> a kworker, which does not have its own address space in
> userspace and must borrow such memory from another process, so
> "current->active_mm" is unknown at the start of the test.
> 
> It was observed that mm->mm_users would occasionally be 0
> or drop to 0 during the test due to short delay between
> scheduling and executing work in forked process, which reaped
> userspace mappings, further leading to failures upon reading
> from userland memory.
> 
> Prevent this by adding a PID parameter to a trusted task, so its
> mm struct may be used if needed.
> 
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14204
> Fixes: 34b1c1c71d37 ("i915/selftest/igt_mmap: let mmap tests run in kthread")
> Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
> ---
> v2 (Janusz):
>  * Reword and shorten commit message to be more precise.
>  * Reorder variable declarations to follow upside down christmas
>  tree style.
> 
> v3 (Andi):
>  * Prevent PID and mm leaks.
>  * Remove a flag and use mm pointer to determine whether to
>  release references to the memory.
> 
> v4:
>  * Revert !current->mm check. (Janusz, Sebastian)
>  * Drop refernce to mm sooner. (Janusz)
>  * Ensure kthread_use_mm did its job. (Janusz)
> 
>  drivers/gpu/drm/i915/i915_selftest.h          |  1 +
>  .../gpu/drm/i915/selftests/i915_selftest.c    | 51 +++++++++++++++++++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_selftest.h b/drivers/gpu/drm/i915/i915_selftest.h
> index 72922028f4ba..e29ca298e7eb 100644
> --- a/drivers/gpu/drm/i915/i915_selftest.h
> +++ b/drivers/gpu/drm/i915/i915_selftest.h
> @@ -35,6 +35,7 @@ struct i915_selftest {
>  	unsigned long timeout_jiffies;
>  	unsigned int timeout_ms;
>  	unsigned int random_seed;
> +	unsigned int userspace_pid;
>  	char *filter;
>  	int mock;
>  	int live;
> diff --git a/drivers/gpu/drm/i915/selftests/i915_selftest.c b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> index 8460f0a70d04..45fe750b799d 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_selftest.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> @@ -186,6 +186,8 @@ static int __run_selftests(const char *name,
>  			   unsigned int count,
>  			   void *data)
>  {
> +	int u_pid_nr = i915_selftest.userspace_pid;
> +	struct mm_struct *mm = NULL;
>  	int err = 0;
>  
>  	while (!i915_selftest.random_seed)
> @@ -201,6 +203,50 @@ static int __run_selftests(const char *name,
>  	pr_info(DRIVER_NAME ": Performing %s selftests with st_random_seed=0x%x st_timeout=%u\n",
>  		name, i915_selftest.random_seed, i915_selftest.timeout_ms);
>  
> +	/**
> +	 * If we are running in a kthread on a multi NUMA system and the user passed
> +	 * a valid PID of a userspace task, then we may borrow its address space
> +	 * to prepare a safe environment for the mmap selftests.
> +	 */
> +	if (!current->mm) {
> +		struct pid *u_pid;
> +		struct task_struct *task;
> +
> +		if (!u_pid_nr) {
> +			pr_warn("No current->mm and no PID provided to safely borrow userspace memory from.\n"
> +				"This may lead to switching off tests requiring that for mappings");

Most selftests don't need current->mm, while the warning emitted from here 
will unnecessary trigger CI dmesg-warn result for any selftest.  I propose 
to decrease severity to INFO, and instead, add a similar warning to the 
second patch.

If a user provides a PID then the warnings below are OK.

Thanks,
Janusz

> +			goto run_tests;
> +		}
> +
> +		u_pid = find_get_pid(u_pid_nr);
> +
> +		if (!u_pid) {
> +			pr_warn("Could not find PID: %d\n", u_pid_nr);
> +			goto run_tests;
> +		}
> +
> +		task = get_pid_task(u_pid, PIDTYPE_PID);
> +		put_pid(u_pid);
> +		if (!task) {
> +			pr_warn("Could not find userspace task for PID: %d\n", u_pid_nr);
> +			goto run_tests;
> +		}
> +
> +		mm = get_task_mm(task);
> +		put_task_struct(task);
> +		if (!mm) {
> +			pr_warn("Could not find address space of task with PID: %d\n", u_pid_nr);
> +			goto run_tests;
> +		}
> +
> +		kthread_use_mm(mm);
> +		mmput_async(mm);
> +		if (unlikely(!current->mm)) {
> +			pr_warn("Could not set mm as current->mm\n");
> +		}
> +	}
> +
> +run_tests:
>  	/* Tests are listed in order in i915_*_selftests.h */
>  	for (; count--; st++) {
>  		if (!st->enabled)
> @@ -226,6 +272,9 @@ static int __run_selftests(const char *name,
>  		 st->name, err))
>  		err = -1;
>  
> +	if (mm)
> +		kthread_unuse_mm(mm);
> +
>  	return err;
>  }
>  
> @@ -507,6 +556,8 @@ void igt_hexdump(const void *buf, size_t len)
>  module_param_named(st_random_seed, i915_selftest.random_seed, uint, 0400);
>  module_param_named(st_timeout, i915_selftest.timeout_ms, uint, 0400);
>  module_param_named(st_filter, i915_selftest.filter, charp, 0400);
> +module_param_named(st_userspace_pid, i915_selftest.userspace_pid, uint, 0400);
> +MODULE_PARM_DESC(st_userspace_pid, "For usage in tests that map userspace memory and require address space with controllable lifetime.");
>  
>  module_param_named_unsafe(mock_selftests, i915_selftest.mock, int, 0400);
>  MODULE_PARM_DESC(mock_selftests, "Run selftests before loading, using mock hardware (0:disabled [default], 1:run tests then load driver, -1:run tests then leave dummy module)");

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 2/2] drm/i915/selftests: Run vma tests only if current->mm is present
  2026-04-08  8:30 ` [PATCH v4 2/2] drm/i915/selftests: Run vma tests only if current->mm is present Krzysztof Karas
@ 2026-04-08  9:25   ` Janusz Krzysztofik
  0 siblings, 0 replies; 7+ messages in thread
From: Janusz Krzysztofik @ 2026-04-08  9:25 UTC (permalink / raw)
  To: Krzysztof Karas, intel-gfx
  Cc: Andi Shyti, Sebastian Brzezinka, Krzysztof Niemiec

Hi Krzysztof,

As a continuation of my comment to patch 1/1, ...

On Wed, 2026-04-08 at 08:30 +0000, Krzysztof Karas wrote:
> This set of tests require userspace memory to map objects, so
> run them only if this that memory is available.
> 
> Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
> ---
>  drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> index 9d454d0b46f2..0752e758b01b 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> @@ -1847,11 +1847,12 @@ static int igt_mmap_revoke(void *arg)
>  int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
>  {
>  	int ret;
> -	bool unuse_mm = false;
>  	static const struct i915_subtest tests[] = {
>  		SUBTEST(igt_partial_tiling),
>  		SUBTEST(igt_smoke_tiling),
>  		SUBTEST(igt_mmap_offset_exhaustion),
> +	};
> +	static const struct i915_subtest vma_tests[] = {
>  		SUBTEST(igt_mmap),
>  		SUBTEST(igt_mmap_migrate),
>  		SUBTEST(igt_mmap_access),
> @@ -1859,15 +1860,12 @@ int i915_gem_mman_live_selftests(struct drm_i915_private *i915)
>  		SUBTEST(igt_mmap_gpu),
>  	};
>  
> -	if (!current->mm) {
> -		kthread_use_mm(current->active_mm);
> -		unuse_mm = true;
> -	}
> -
>  	ret = i915_live_subtests(tests, i915);
> +	if (ret)
> +		return ret;
>  
> -	if (unuse_mm)
> -		kthread_unuse_mm(current->active_mm);
> +	if (current->mm)

Since we have now a working method for providing current->mm, we should 
warn (and then trigger CI dmesg-warn result) if it is missing, I believe.

Thanks,
Janusz

> +		ret = i915_live_subtests(vma_tests, i915);
>  
>  	return ret;
>  }

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] drm/i915/selftests: Prevent userspace mapping invalidation
  2026-04-08  9:20   ` Janusz Krzysztofik
@ 2026-04-09  6:21     ` Krzysztof Karas
  0 siblings, 0 replies; 7+ messages in thread
From: Krzysztof Karas @ 2026-04-09  6:21 UTC (permalink / raw)
  To: Janusz Krzysztofik
  Cc: intel-gfx, Andi Shyti, Sebastian Brzezinka, Krzysztof Niemiec

Hi Janusz,

[...]

> > @@ -35,6 +35,7 @@ struct i915_selftest {
> >  	unsigned long timeout_jiffies;
> >  	unsigned int timeout_ms;
> >  	unsigned int random_seed;
> > +	unsigned int userspace_pid;
> >  	char *filter;
> >  	int mock;
> >  	int live;
> > diff --git a/drivers/gpu/drm/i915/selftests/i915_selftest.c b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> > index 8460f0a70d04..45fe750b799d 100644
> > --- a/drivers/gpu/drm/i915/selftests/i915_selftest.c
> > +++ b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> > @@ -186,6 +186,8 @@ static int __run_selftests(const char *name,
> >  			   unsigned int count,
> >  			   void *data)
> >  {
> > +	int u_pid_nr = i915_selftest.userspace_pid;
> > +	struct mm_struct *mm = NULL;
> >  	int err = 0;
> >  
> >  	while (!i915_selftest.random_seed)
> > @@ -201,6 +203,50 @@ static int __run_selftests(const char *name,
> >  	pr_info(DRIVER_NAME ": Performing %s selftests with st_random_seed=0x%x st_timeout=%u\n",
> >  		name, i915_selftest.random_seed, i915_selftest.timeout_ms);
> >  
> > +	/**
> > +	 * If we are running in a kthread on a multi NUMA system and the user passed
> > +	 * a valid PID of a userspace task, then we may borrow its address space
> > +	 * to prepare a safe environment for the mmap selftests.
> > +	 */
> > +	if (!current->mm) {
> > +		struct pid *u_pid;
> > +		struct task_struct *task;
> > +
> > +		if (!u_pid_nr) {
> > +			pr_warn("No current->mm and no PID provided to safely borrow userspace memory from.\n"
> > +				"This may lead to switching off tests requiring that for mappings");
> 
> Most selftests don't need current->mm, while the warning emitted from here 
> will unnecessary trigger CI dmesg-warn result for any selftest.  I propose 
> to decrease severity to INFO, and instead, add a similar warning to the 
> second patch.
Hmm, fair point, I did not think about that. I also already saw
the other mail under PATCH 2/2, so moving the warnings to the
tests that have the requirement makes sense to me.

[...]

-- 
Best Regards,
Krzysztof

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] drm/i915/selftests: Prevent userspace mapping invalidation
  2026-04-08  8:30 ` [PATCH v4 1/2] drm/i915/selftests: Prevent userspace mapping invalidation Krzysztof Karas
  2026-04-08  9:20   ` Janusz Krzysztofik
@ 2026-04-09 11:09   ` Andi Shyti
  1 sibling, 0 replies; 7+ messages in thread
From: Andi Shyti @ 2026-04-09 11:09 UTC (permalink / raw)
  To: Krzysztof Karas
  Cc: intel-gfx, Andi Shyti, Sebastian Brzezinka, Krzysztof Niemiec,
	Janusz Krzysztofik

Hi Krzysztof,

> +		if (!u_pid_nr) {
> +			pr_warn("No current->mm and no PID provided to safely borrow userspace memory from.\n"
> +				"This may lead to switching off tests requiring that for mappings");

This line is too long (please check with checkpatch). The message
is to long. We don't need to provide a full explanation. You can
actually just write something like "No usable userspace mm".

jkk

> +			goto run_tests;
> +		}
> +
> +		u_pid = find_get_pid(u_pid_nr);
> +
> +		if (!u_pid) {
> +			pr_warn("Could not find PID: %d\n", u_pid_nr);
> +			goto run_tests;
> +		}
> +
> +		task = get_pid_task(u_pid, PIDTYPE_PID);
> +		put_pid(u_pid);
> +		if (!task) {
> +			pr_warn("Could not find userspace task for PID: %d\n", u_pid_nr);
> +			goto run_tests;
> +		}
> +
> +		mm = get_task_mm(task);
> +		put_task_struct(task);
> +		if (!mm) {
> +			pr_warn("Could not find address space of task with PID: %d\n", u_pid_nr);
> +			goto run_tests;
> +		}
> +
> +		kthread_use_mm(mm);
> +		mmput_async(mm);
> +		if (unlikely(!current->mm)) {
> +			pr_warn("Could not set mm as current->mm\n");
> +		}

Please remove the two brackets here.

On a general note, I'm not a big fan of the goto's used for
jumping around the code, unless it's an exit point.

Perhaps this can be written as:

	if (!current->mm) {
		mm = a_new_function(...);
		if (mm) {
			kthread_use_mm(mm);
			...
		}
		...
	}

Thanks,
Andi

> +	}
> +

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-04-09 11:09 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-08  8:30 [RFC v4 0/2] drm/i915/selftests: Use safe userspace memory for mappings Krzysztof Karas
2026-04-08  8:30 ` [PATCH v4 1/2] drm/i915/selftests: Prevent userspace mapping invalidation Krzysztof Karas
2026-04-08  9:20   ` Janusz Krzysztofik
2026-04-09  6:21     ` Krzysztof Karas
2026-04-09 11:09   ` Andi Shyti
2026-04-08  8:30 ` [PATCH v4 2/2] drm/i915/selftests: Run vma tests only if current->mm is present Krzysztof Karas
2026-04-08  9:25   ` Janusz Krzysztofik

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.