| Message ID | 20210610091800.1833-2-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/7] dma-buf: some dma_fence_chain improvements | expand |
On Thu, Jun 10, 2021 at 11:17:54AM +0200, Christian König wrote: > The callback and the irq work are never used at the same > time. Putting them into an union saves us 24 bytes and > makes the structure only 120 bytes in size. Yeah pushing below 128 bytes makes sense. > > Signed-off-by: Christian König <christian.koenig@amd.com> > --- > drivers/dma-buf/dma-fence-chain.c | 2 +- > include/linux/dma-fence-chain.h | 8 +++++--- > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c > index 7d129e68ac70..1b4cb3e5cec9 100644 > --- a/drivers/dma-buf/dma-fence-chain.c > +++ b/drivers/dma-buf/dma-fence-chain.c > @@ -137,6 +137,7 @@ static void dma_fence_chain_cb(struct dma_fence *f, struct dma_fence_cb *cb) > struct dma_fence_chain *chain; > > chain = container_of(cb, typeof(*chain), cb); > + init_irq_work(&chain->work, dma_fence_chain_irq_work); > irq_work_queue(&chain->work); > dma_fence_put(f); > } > @@ -239,7 +240,6 @@ void dma_fence_chain_init(struct dma_fence_chain *chain, > rcu_assign_pointer(chain->prev, prev); > chain->fence = fence; > chain->prev_seqno = 0; > - init_irq_work(&chain->work, dma_fence_chain_irq_work); > > /* Try to reuse the context of the previous chain node. */ > if (prev_chain && __dma_fence_is_later(seqno, prev->seqno, prev->ops)) { > diff --git a/include/linux/dma-fence-chain.h b/include/linux/dma-fence-chain.h > index 10462a029da2..9d6a062be640 100644 > --- a/include/linux/dma-fence-chain.h > +++ b/include/linux/dma-fence-chain.h > @@ -25,12 +25,14 @@ > */ > struct dma_fence_chain { > struct dma_fence base; > - spinlock_t lock; > struct dma_fence __rcu *prev; > u64 prev_seqno; > struct dma_fence *fence; > - struct dma_fence_cb cb; > - struct irq_work work; Can you pls pull the kerneldoc inline here for these too and extend the comments that @work is only used from the callback, at which point we don't need @cb anymore? For union lifetime tricks we really should document this in the datastructure docs. With that: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> I also think it'd be good to specify this clearly in the kerneldoc for dma_fence_add_callback() with something like: "Note that the registered @cb structure is no longer in use by the signalling code by the time @func is called, and can therefore be used again. This is useful when @func launches a work item because it allows us to put both the struct dma_fence_cb and the work struct (e.g. struct work_struct) into a union to save space." Feel free to includ this in this patch here or do a separate one. Cheers, Daniel > + union { > + struct dma_fence_cb cb; > + struct irq_work work; > + }; > + spinlock_t lock; > }; > > extern const struct dma_fence_ops dma_fence_chain_ops; > -- > 2.25.1 >
diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c index 7d129e68ac70..1b4cb3e5cec9 100644 --- a/drivers/dma-buf/dma-fence-chain.c +++ b/drivers/dma-buf/dma-fence-chain.c @@ -137,6 +137,7 @@ static void dma_fence_chain_cb(struct dma_fence *f, struct dma_fence_cb *cb) struct dma_fence_chain *chain; chain = container_of(cb, typeof(*chain), cb); + init_irq_work(&chain->work, dma_fence_chain_irq_work); irq_work_queue(&chain->work); dma_fence_put(f); } @@ -239,7 +240,6 @@ void dma_fence_chain_init(struct dma_fence_chain *chain, rcu_assign_pointer(chain->prev, prev); chain->fence = fence; chain->prev_seqno = 0; - init_irq_work(&chain->work, dma_fence_chain_irq_work); /* Try to reuse the context of the previous chain node. */ if (prev_chain && __dma_fence_is_later(seqno, prev->seqno, prev->ops)) { diff --git a/include/linux/dma-fence-chain.h b/include/linux/dma-fence-chain.h index 10462a029da2..9d6a062be640 100644 --- a/include/linux/dma-fence-chain.h +++ b/include/linux/dma-fence-chain.h @@ -25,12 +25,14 @@ */ struct dma_fence_chain { struct dma_fence base; - spinlock_t lock; struct dma_fence __rcu *prev; u64 prev_seqno; struct dma_fence *fence; - struct dma_fence_cb cb; - struct irq_work work; + union { + struct dma_fence_cb cb; + struct irq_work work; + }; + spinlock_t lock; }; extern const struct dma_fence_ops dma_fence_chain_ops;
The callback and the irq work are never used at the same time. Putting them into an union saves us 24 bytes and makes the structure only 120 bytes in size. Signed-off-by: Christian König <christian.koenig@amd.com> --- drivers/dma-buf/dma-fence-chain.c | 2 +- include/linux/dma-fence-chain.h | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-)