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>
