Re: [Intel-xe] [PATCH 4/5] drm/xe: Combine destroy_cb and destroy_work in xe_vma into union

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

On Fri, Jul 14, 2023 at 03:53:40AM +0000, Matthew Brost wrote:
> On Thu, Jul 13, 2023 at 04:23:43PM -0400, Rodrigo Vivi wrote:
> > On Tue, Jul 11, 2023 at 02:27:47PM -0700, Matthew Brost wrote:
> > > The callback kicks the worker thus mutually exclusive execution,
> > > combining saves a bit of space in xe_vma.
> > 
> > could you please open up a bit on why they are multually exclusive?!
> > 
> 
> Not sure how else to word this. The callback function (below) queues the
> worker, the callback argument at this point is safe to clobber.
> 
> 1047 static void vma_destroy_cb(struct dma_fence *fence,
> 1048                            struct dma_fence_cb *cb)
> 1049 {
> 1050         struct xe_vma *vma = container_of(cb, struct xe_vma, destroy_cb);
> 1051
> 1052         INIT_WORK(&vma->destroy_work, vma_destroy_work_func);
> 1053         queue_work(system_unbound_wq, &vma->destroy_work);
> 1054 }

oh, indeed... I should had checked that.
for a moment I wondered about scenarios where the callback would
be called more than once, but that is really not possible and
we would have other errors anyway.

no change needed in the original patch or commit msg.
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> Matt
> 
> > > 
> > > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > > ---
> > >  drivers/gpu/drm/xe/xe_vm_types.h | 11 ++++++-----
> > >  1 file changed, 6 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_vm_types.h b/drivers/gpu/drm/xe/xe_vm_types.h
> > > index 2a8691a48c55..30beae541aca 100644
> > > --- a/drivers/gpu/drm/xe/xe_vm_types.h
> > > +++ b/drivers/gpu/drm/xe/xe_vm_types.h
> > > @@ -51,11 +51,12 @@ struct xe_vma {
> > >  		struct list_head destroy;
> > >  	} combined_links;
> > >  
> > > -	/** @destroy_cb: callback to destroy VMA when unbind job is done */
> > > -	struct dma_fence_cb destroy_cb;
> > > -
> > > -	/** @destroy_work: worker to destroy this BO */
> > > -	struct work_struct destroy_work;
> > > +	union {
> > > +		/** @destroy_cb: callback to destroy VMA when unbind job is done */
> > > +		struct dma_fence_cb destroy_cb;
> > > +		/** @destroy_work: worker to destroy this BO */
> > > +		struct work_struct destroy_work;
> > > +	};
> > >  
> > >  	/** @userptr: user pointer state */
> > >  	struct {
> > > -- 
> > > 2.34.1
> > >