[PATCH 1/2] drm/i915: set O_LARGEFILE in __create_shmem()

[Andi Shyti]

Hi Taotao,

...

> >> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> >> index e3d188455f67..2b53aad915f5 100644
> >> --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> >> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> >> @@ -514,6 +514,11 @@ static int __create_shmem(struct drm_i915_private *i915,
> >>  	if (IS_ERR(filp))
> >>  		return PTR_ERR(filp);
> >>  
> >> +	/*
> >> +	 * Prevent -EFBIG by allowing large writes beyond MAX_NON_LFS on shmem
> >> +	 * objects by setting O_LARGEFILE.
> >> +	 */
> >> +	filp->f_flags |= O_LARGEFILE;
> >
> > I don't have anything against this, but is it really fixing the
> > issue? I thought that O_LARGEFILE is ignored in 64 bit machines,
> > while here the failure is happening in 64 bit machines.
> 
> As mentioned in the commit body, without O_LARGEFILE, file->f_op->write_iter
> calls generic_write_check_limits(), which enforces the 2GB (MAX_NON_LFS) limit
> and causes -EFBIG on large writes.
> 
> On 64-bit systems O_LARGEFILE is still set when opening files (e.g. via open()),
> so we also need to set it here for shmem objects created inside the kernel.
> 
> However, on older 32-bit systems, setting O_LARGEFILE unconditionally may be risky.
> Previously I did not check this, but to reduce the risk a safer approach is to wrap
> it in a check, for example:
> 
> +	if (force_o_largefile())
> +		filp->f_flags |= O_LARGEFILE;

Ack!

> > Besides, where do you see in the LKP logs the -EFBIG error
> > message?
> >
> 
> Due to the previous return order in shmem_pwrite(), this -EFBIG was being overwritten
> by -EIO on short writes. This issue will be fixed in PATCH 2/2.

Yes, correct :-)

Thanks,
Andi