radeon VS amdgpu: _afmt_init() behavior if kzalloc fails

[Christian König]

Hi guys,

yeah, agree totally. When we fail to allocate anything during module or 
device initialization we should cleanup properly and abort loading the 
module.

On the other hand the only time I actually run into such a problem was 
while constantly loading and unloading the radeon module to find bugs.

BTW: Was pretty fruitful to do this, we should try this with amdgpu at 
some time as well.

Christian.

Am 12.08.2016 um 18:17 schrieb StDenis, Tom:
>
> Hi Alex,
>
>
> I'm unsure of what patch you have specifically.  Are you suggesting to 
> modify the radeon side to mimic the behaviour?  Or to revert the 
> amdgpu changes?  I  think reverting the amdgpu side won't go over 
> well.  It's churn and the current approach is arguably better for a 
> number of reasons namely it's better defined behaviour and generally 
> speaking if during module init a kmalloc of 100 bytes fails something 
> bad is happening and you want to abort init anyways (so failing to 
> load just because part of DCE fails is probably a good thing).
>
>
> Tom
>
>
>
>
>
> ------------------------------------------------------------------------
> *From:* Alexandre Demers <alexandre.f.demers at gmail.com>
> *Sent:* Friday, August 12, 2016 12:11
> *To:* StDenis, Tom; Freedesktop - AMD-gfx
> *Cc:* Alexander Deucher
> *Subject:* Re: radeon VS amdgpu: _afmt_init() behavior if kzalloc fails
> Thanks for the quick answer Tom. I was thinking mostly as you do, but 
> before sending the patch to the list, I wanted to be sure it was worth 
> it.
>
> I'll send the patch later then, unless someone says otherwise.
>
> Cheers,
> Alexandre
>
> On Fri, 12 Aug 2016 at 11:50 StDenis, Tom <Tom.StDenis at amd.com 
> <mailto:Tom.StDenis at amd.com>> wrote:
>
>     Hi,
>
>
>     I wrote those changes back in March as part of a cleanup sweep. 
>     The idea being was that if some had failed the state of the driver
>     would be unknown so it was cleaner to simply abort allocating any
>     memory and set all of the pointers to NULL.
>
>
>     Generally speaking if you fail to kmalloc a few bytes you've got
>     bigger problems to worry about than your audio not working ideally.
>
>
>     Tom
>
>
>
>     ------------------------------------------------------------------------
>     *From:* amd-gfx <amd-gfx-bounces at lists.freedesktop.org
>     <mailto:amd-gfx-bounces at lists.freedesktop.org>> on behalf of
>     Alexandre Demers <alexandre.f.demers at gmail.com
>     <mailto:alexandre.f.demers at gmail.com>>
>     *Sent:* Friday, August 12, 2016 11:43
>     *To:* Freedesktop - AMD-gfx
>     *Cc:* Alexander Deucher
>     *Subject:* radeon VS amdgpu: _afmt_init() behavior if kzalloc fails
>     Hi,
>
>     While comparing radeon's radeon_afmt_init() to
>     dce_vX_0_afmt_init() on the amdgpu's side, I saw that on the latter:
>     1- when kzalloc fails to allocate mem for all afmt, an ENOMEM is
>     returned
>     2- all previously allocated afmt are freed
>
>     So on the amdgpu's side, it is an "all or none allocated"
>     situation, while on the radeon's side, some afmt may be allocated
>     and initialized.
>
>     Moreover, returning an ENOMEM prevents any other function calls in
>     dce_vX_0_sw_init() on the amdgpu's side, while it continues on the
>     radeon's side.
>
>     What is the expected behavior here? Should we rewind the memory
>     allocation as it is done under amdgpu when we can't allocate
>     memory for all afmt or is it OK to do as it is done on radeon?
>     Should we stop any remaining steps in radeon_modeset_init() if it
>     fails (thus, returning a ENOMEM from radeon_afmt_init())?
>
>     The patch is already ready if needed, I could send it later from
>     home if the amdgpu's behavior is the one that we are looking for.
>
>     Cheers,
>     Alexandre Demers
>
>
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20160812/752116a9/attachment.html>