Re: [PATCH 1/8] drm/amd/amdgpu: Added asic_type as ACP DMA driver platform data

From: Mark Brown <broonie@kernel.org>
To: Alex Deucher <alexdeucher@gmail.com>
Cc: "alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	amd-gfx list <amd-gfx@lists.freedesktop.org>,
	rajeev kumar <rajeevkumar.linux@gmail.com>,
	Maling list - DRI developers <dri-devel@lists.freedesktop.org>,
	Vijendar Mukunda <Vijendar.Mukunda@amd.com>,
	Alex Deucher <alexander.deucher@amd.com>,
	perex@perex.cz
Subject: Re: [PATCH 1/8] drm/amd/amdgpu: Added asic_type as ACP DMA driver platform data
Date: Wed, 28 Jun 2017 18:54:06 +0100	[thread overview]
Message-ID: <20170628175406.5yykiv33xdcs6qnr@sirena.org.uk> (raw)
In-Reply-To: <CADnq5_MT+h6nOmXfVi42tH9G917wTRguBdMC02zmgRuH2dD-iA@mail.gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 1231 bytes --]

On Fri, Jun 23, 2017 at 01:05:37PM -0400, Alex Deucher wrote:
> On Fri, Jun 23, 2017 at 12:43 PM, Christian König

> > Have the painkillers jellyfied my brain or are you setting a pointer in the
> > amdgpu device structure to some variable on the stack?

> > If it's just for initialization then that's probably ok, but I would reset
> > the pointer at the end of the function.

> > Otherwise somebody might have the idea to dereference it later on and that
> > can only lead to trouble.

> I think it's ok because the hotplug of the audio device is triggered
> from this function, so the audio device should be probed by the time
> this variable goes out of scope.  That said, it would probably be

No, that's in no way safe.  There is no guarantee that probe is going to
happen on any particular schedule, if the other driver isn't registered
yet it'll need to wait for that to happen for example.  You will often
get away with it but that's at best going to be vulnerable to framework
changes (what if someone decides to kick off the probe on a separate
CPU in an effort to reduce boot time?).

> better to use the asic_type from the GPU driver instance directly
> rather than a stack variable.

Yes.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel