Re: [PATCH] drm/xe: Remove NULL check of lrc->bo in xe_lrc_snapshot_capture()

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Tue, Aug 20, 2024 at 11:43:12AM +0200, Nirmoy Das wrote:
> Hi Rodrigo,
> 
> On 8/16/2024 4:00 PM, Rodrigo Vivi wrote:
> > On Fri, Aug 16, 2024 at 01:33:55PM +0530, apoorva.singh@intel.com wrote:
> > > From: Apoorva Singh <apoorva.singh@intel.com>
> > > 
> > > - lrc->bo NULL check is not needed in xe_lrc_snapshot_capture() as
> > >    its already been taken care of in xe_lrc_init().
> > I'm afraid this is not a good reason.
> > This snapshot capture is coming from other places and apparently
> > with risks of paths where bo was already freed?!
> 
> 
> After taking another look, I just realized the lrc bo is always pinned and
> as far as I see, only place we unpin and free it,  is when we destroy the
> lrc.
> 
> Am I missing any condition when xe_lrc_snapshot_capture() can be called with
> lrc->bo freed ?

if you are confident we don't need it, let's kill...

Acked-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> 
> Regards,
> 
> Nirmoy
> 
> 
> > 
> > why is this check bothering you so much?
> > 
> > > Signed-off-by: Apoorva Singh <apoorva.singh@intel.com>
> > > ---
> > >   drivers/gpu/drm/xe/xe_lrc.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_lrc.c b/drivers/gpu/drm/xe/xe_lrc.c
> > > index 974a9cd8c379..aec7db39c061 100644
> > > --- a/drivers/gpu/drm/xe/xe_lrc.c
> > > +++ b/drivers/gpu/drm/xe/xe_lrc.c
> > > @@ -1649,7 +1649,7 @@ struct xe_lrc_snapshot *xe_lrc_snapshot_capture(struct xe_lrc *lrc)
> > >   	if (!snapshot)
> > >   		return NULL;
> > > -	if (lrc->bo && lrc->bo->vm)
> > > +	if (lrc->bo->vm)
> > >   		xe_vm_get(lrc->bo->vm);
> > >   	snapshot->context_desc = xe_lrc_ggtt_addr(lrc);
> > > -- 
> > > 2.34.1
> > >