Re: [Intel-xe] [PATCH] drm/xe/irq: the irq handler local variable need not be static

[Jani Nikula <jani.nikula@intel.com>]

On Tue, 14 Mar 2023, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
> On Tue, Mar 14, 2023 at 12:32:59PM +0200, Jani Nikula wrote:
>>On Mon, 13 Mar 2023, Lucas De Marchi <lucas.demarchi@intel.com> wrote:
>>> On Thu, Mar 09, 2023 at 09:37:34AM -0800, Matt Roper wrote:
>>>>On Thu, Mar 09, 2023 at 02:21:33PM +0200, Jani Nikula wrote:
>>>>> It's just a local variable.
>>>>>
>>>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>>
>>>>Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>>>
>>>
>>> Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>>
>>> but the commit message should probably say 's/need/should/'. Setting it
>>> as static pretty much breaks setups with 2 devices like TGL + DG2.
>>
>>Mmmh, only if the probe races between xe_irq_install() calls for the two
>>devices. Can the same driver probe two devices in parallel?
>
> true, I don't think currently it will ever be in parallel since 
> we are tying the bind to the module load. We could stimulate
> a race by doing the bind separately, but even then the window for
> the race is just a few instructions.
>
> When I read this commit I was under the impression that whatever got set
> in this variable would end up being used later. Checking again, it's
> only used inside the xe_irq_install to call directly.
>
> So I'm fine with the current message and my r-b stands.

Thanks, pushed.

BR,
Jani.

>
> thanks
> Lucas De Marchi
>
>>
>>BR,
>>Jani.
>>
>>
>>>
>>> Lucas De Marchi
>>>
>>>>
>>>>> ---
>>>>>  drivers/gpu/drm/xe/xe_irq.c | 2 +-
>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
>>>>> index ae2f65c00fa6..529b42d9c9af 100644
>>>>> --- a/drivers/gpu/drm/xe/xe_irq.c
>>>>> +++ b/drivers/gpu/drm/xe/xe_irq.c
>>>>> @@ -529,7 +529,7 @@ static void irq_uninstall(struct drm_device *drm, void *arg)
>>>>>  int xe_irq_install(struct xe_device *xe)
>>>>>  {
>>>>>  	int irq = to_pci_dev(xe->drm.dev)->irq;
>>>>> -	static irq_handler_t irq_handler;
>>>>> +	irq_handler_t irq_handler;
>>>>>  	int err;
>>>>>
>>>>>  	irq_handler = xe_irq_handler(xe);
>>>>> --
>>>>> 2.39.1
>>>>>
>>>>
>>>>--
>>>>Matt Roper
>>>>Graphics Software Engineer
>>>>Linux GPU Platform Enablement
>>>>Intel Corporation
>>
>>-- 
>>Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center