[PATCH 2/2] drm/cma-helper: Implement mmap as GEM CMA object functions

[Thomas Zimmermann]

Hi

Am 14.01.21 um 17:28 schrieb Kieran Bingham:
> Hi Thomas,
> 
> On 14/01/2021 15:15, Thomas Zimmermann wrote:
>>>>> On 23/11/2020 11:56, Thomas Zimmermann wrote:
>>>>>> The new GEM object function drm_gem_cma_mmap() sets the VMA flags
>>>>>> and offset as in the old implementation and immediately maps in the
>>>>>> buffer's memory pages.
>>>>>>
>>>>>> Changing CMA helpers to use the GEM object function allows for the
>>>>>> removal of the special implementations for mmap and gem_prime_mmap
>>>>>> callbacks. The regular functions drm_gem_mmap() and
>>>>>> drm_gem_prime_mmap()
>>>>>> are now used.
>>>>>
>>>>> I've encountered a memory leak regression in our Renesas R-Car DU
>>>>> tests,
>>>>> and git bisection has led me to this patch (as commit f5ca8eb6f9).
>>>>>
>>>>> Running the tests sequentially, while grepping /proc/meminfo for
>>>>> Cma, it
>>>>> is evident that CMA memory is not released, until exhausted and the
>>>>> allocations fail (seen in [0]) shown by the error report:
>>>>>
>>>>>>        self.fbs.append(pykms.DumbFramebuffer(self.card, mode.hdisplay,
>>>>>> mode.vdisplay, "XR24"))
>>>>>> ValueError: DRM_IOCTL_MODE_CREATE_DUMB failed: Cannot allocate memory
>>>>>
>>>>>
>>>>> Failing tests at f5ca8eb6f9 can be seen at [0], while the tests pass
>>>>> successfully [1] on the commit previous to that (bc2532ab7c2):
>>>>>
>>>>> Reverting f5ca8eb6f9 also produces a successful pass [2]
>>>>>
>>>>>     [0] https://paste.ubuntu.com/p/VjPGPgswxR/ # Failed at f5ca8eb6f9
>>>>>     [1] https://paste.ubuntu.com/p/78RRp2WpNR/ # Success at bc2532ab7c2
>>>>>     [2] https://paste.ubuntu.com/p/qJKjZZN2pt/ # Success with revert
>>>>>
>>>>>
>>>>> I don't believe we handle mmap specially in the RCar-DU driver, so I
>>>>> wonder if this issue has hit anyone else as well?
>>>>>
>>>>> Any ideas of a repair without a revert ? Or do we just need to submit a
>>>>> revert?
>>>>
>>>> I think we might not be setting the VMA ops and therefore not finalize
>>>> the BO correctly. Could you please apply the attched (quick-and-dirty)
>>>> patch and try again?
>>>
>>> Thanks for the quick response.
>>>
>>> I can confirm the quick-and-dirty patch resolves the issue:
>>>     https://paste.ubuntu.com/p/sKDp3dNvwV/
>>>
>>> You can add a
>>> Tested-by: Kieran Bingham <kieran.bingham+renesas at ideasonboard.com>
>>
>> Great! If you don't mind, I'd also add you in the Reported-by tag.
> 
> Certainly!
> 
>>>
>>> if it stays like that, but I suspect there might be a better place to
>>> initialise the ops rather than in the mmap call itself.
>>
>> I think that's the fix, basically. We could put such a line as a
>> fall-back somewhere into the DRM core code. But I don't know if this
>> really works with all drivers. Maybe there's one that requires vm_ops to
>> be NULL.
> 
> Ok, that's reaching beyond code I've explored, so I'll leave it to you.

Daniel asked for a fix in the DRM core, so I'll go with the alternative 
approach. If you have the time, I'd appreciate another test run.

Best regards
Thomas

> 
> 
>> Thanks for reporting this issue and testing quickly.
> 
> Thanks for fixing so quickly :-)
> 
> Regards
> 
> Kieran
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20210115/15cb3b9b/attachment.sig>