Hi Janusz,
[...]
> > +++ b/drivers/gpu/drm/i915/selftests/i915_selftest.c
> > @@ -181,11 +181,48 @@ __wait_gsc_huc_load_completed(struct drm_i915_private
> > *i915)
> > pr_warn(DRIVER_NAME "Timed out waiting for huc load via
> > GSC!\n");
> > }
> >
> > +static struct mm_struct *
> > +get_mm(int u_pid_nr)
> > +{
> > + struct pid *u_pid = find_get_pid(u_pid_nr);
>
> What happens here if the st_userspace_pid module parameter is not provided?
Hmm, good catch, let's be paranoid about the usage and make the
intent clear.
[...]
> > static int __run_selftests(const char *name,
> > struct selftest *st,
> > 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,14 +238,36 @@ 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) {
>
> I think this condition should also check for a valid u_pid_nr. To avoid
> ambiguity, maybe the i915_selftest.userspace_pid attribute should be
> initialized to a negative value by default (when not overwritten with the
> corresponding module parameter). There is no point in submitting any
> warnings from here if the module parameter is not provided, I believe.
I like the init to a negative value. I'll also guard agains PID
0, because that is not a value we are interested in, so anything
positive is acceptable (I am checking if this is a userspace
process later anyway).
Thanks for your review.
--
Best Regards,
Krzysztof
