dma-buf: Shrink size of struct dma_fence

Submitted by Chris Wilson on Aug. 17, 2019, 11:39 a.m.

Details

Message ID 20190817113947.5868-1-chris@chris-wilson.co.uk
State Accepted
Commit 4fe3997a68f3300c73adc196ff33a952febe6974
Headers show
Series "dma-buf: Shrink size of struct dma_fence" ( rev: 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Chris Wilson Aug. 17, 2019, 11:39 a.m.
Rearrange the couple of 32-bit atomics hidden amongst the field of
pointers that unnecessarily caused the compiler to insert some padding,
shrinks the size of the base struct dma_fence from 80 to 72 bytes on
x86-64.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Christian König <christian.koenig@amd.com>
---
 include/linux/dma-fence.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 404aa748eda6..2ce4d877d33e 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -63,7 +63,7 @@  struct dma_fence_cb;
  * been completed, or never called at all.
  */
 struct dma_fence {
-	struct kref refcount;
+	spinlock_t *lock;
 	const struct dma_fence_ops *ops;
 	/* We clear the callback list on kref_put so that by the time we
 	 * release the fence it is unused. No one should be adding to the cb_list
@@ -73,11 +73,11 @@  struct dma_fence {
 		struct rcu_head rcu;
 		struct list_head cb_list;
 	};
-	spinlock_t *lock;
 	u64 context;
 	u64 seqno;
-	unsigned long flags;
 	ktime_t timestamp;
+	unsigned long flags;
+	struct kref refcount;
 	int error;
 };
 

Comments

Am 17.08.19 um 13:39 schrieb Chris Wilson:
> Rearrange the couple of 32-bit atomics hidden amongst the field of

> pointers that unnecessarily caused the compiler to insert some padding,

> shrinks the size of the base struct dma_fence from 80 to 72 bytes on

> x86-64.

>

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

> Cc: Christian König <christian.koenig@amd.com>


Reviewed-by: Christian König <christian.koenig@amd.com>


BTW: We could also put the timestamp in the union if we want.

E.g. the cb_list should only be used while the fence is unsignaled, the 
timestamp while it is signaled and the rcu while it is freed.

Would save another 8 bytes, bringing us down to 64.

Christian.

> ---

>   include/linux/dma-fence.h | 6 +++---

>   1 file changed, 3 insertions(+), 3 deletions(-)

>

> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h

> index 404aa748eda6..2ce4d877d33e 100644

> --- a/include/linux/dma-fence.h

> +++ b/include/linux/dma-fence.h

> @@ -63,7 +63,7 @@ struct dma_fence_cb;

>    * been completed, or never called at all.

>    */

>   struct dma_fence {

> -	struct kref refcount;

> +	spinlock_t *lock;

>   	const struct dma_fence_ops *ops;

>   	/* We clear the callback list on kref_put so that by the time we

>   	 * release the fence it is unused. No one should be adding to the cb_list

> @@ -73,11 +73,11 @@ struct dma_fence {

>   		struct rcu_head rcu;

>   		struct list_head cb_list;

>   	};

> -	spinlock_t *lock;

>   	u64 context;

>   	u64 seqno;

> -	unsigned long flags;

>   	ktime_t timestamp;

> +	unsigned long flags;

> +	struct kref refcount;

>   	int error;

>   };

>
Quoting Koenig, Christian (2019-08-17 12:42:48)
> Am 17.08.19 um 13:39 schrieb Chris Wilson:
> > Rearrange the couple of 32-bit atomics hidden amongst the field of
> > pointers that unnecessarily caused the compiler to insert some padding,
> > shrinks the size of the base struct dma_fence from 80 to 72 bytes on
> > x86-64.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Christian König <christian.koenig@amd.com>
> 
> Reviewed-by: Christian König <christian.koenig@amd.com>
> 
> BTW: We could also put the timestamp in the union if we want.
> 
> E.g. the cb_list should only be used while the fence is unsignaled, the 
> timestamp while it is signaled and the rcu while it is freed.
> 
> Would save another 8 bytes, bringing us down to 64.

I was looking at packing the error into the flags and shrinking that to
32b to fit inside the magical 64 bytes. You are right about the
timestamp being mutually exclusive with the cb_list. The only caveat
being that no reader would be allowed access to the timestamp unless
they hold a reference (which I think covers all current users).
-Chris