[PATCH] drm/i915/selftests: Keep mock file open during unfaultable migrate with fill

[Krzysztof Karas]

Hi Krzysztof,

thanks for reviewing!

[...]
> To me, this has a bit of code smell. The double pointer needs to be passed
> to the helper function if we want to acquire the file inside it. However
> we can also just let the caller handle that acquisition, especially since
> we have it call fput() anyway. That way it's clear that it's the caller
> who actually has to manage the resource during its lifetime. This will
> also allow the helper function to take just a copy of the file pointer,
> getting rid of all the redundant dereferencing inside.
> 
> So IMO the usage should be more like:
> 
> <in caller function>
> 
> unsigned long addr;
> struct file *file;
> 
> file = acquire_in_some_manner();
> addr = __igt_mmap_offset(i915, offset, size, prot, flags, file);
> 
> /* do some operations */
> 
> fput(file);
> 
> </in caller function>
> 
> > +unsigned long igt_mmap_offset_get_file(struct drm_i915_private *i915,
> > +				       u64 offset,
> > +				       unsigned long size,
> > +				       unsigned long prot,
> > +				       unsigned long flags,
> > +				       struct file **file)
> > +{
> > +	return __igt_mmap_offset(i915, offset, size, prot, flags, file);
> > +}
> 
> Also given the above I'd reconsider this function. Since we're now
> expecting the caller to acquire the file anyway, we could probably just
> put the body of __igt_mmap_offset() into igt_mmap_offset_get_file() and
> call _get_file() from igt_mmap_offset().
> 
Great suggestions, haven't thought about it that way. I like the
last part especially - putting the get_file call inside
igt_mmap_offset().

Best Regards,
Krzysztof