diff mbox series

[1/5] dma-buf: cleanup dma_fence_unwrap selftest

Message ID 20220504122256.1654-1-christian.koenig@amd.com (mailing list archive)
State New, archived
Headers show
Series [1/5] dma-buf: cleanup dma_fence_unwrap selftest | expand

Commit Message

Christian König May 4, 2022, 12:22 p.m. UTC 
The selftests, fix the error handling, remove unused functions and stop
leaking memory in failed tests.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/dma-buf/st-dma-fence-unwrap.c | 40 +++++++++++----------------
 1 file changed, 16 insertions(+), 24 deletions(-)

Comments

Daniel Vetter May 5, 2022, 1:29 p.m. UTC | #1 
On Wed, May 04, 2022 at 02:22:52PM +0200, Christian König wrote:
> The selftests, fix the error handling, remove unused functions and stop
> leaking memory in failed tests.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/dma-buf/st-dma-fence-unwrap.c | 40 +++++++++++----------------
>  1 file changed, 16 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/dma-buf/st-dma-fence-unwrap.c b/drivers/dma-buf/st-dma-fence-unwrap.c
> index 039f016b57be..59628add93f5 100644
> --- a/drivers/dma-buf/st-dma-fence-unwrap.c
> +++ b/drivers/dma-buf/st-dma-fence-unwrap.c
> @@ -4,27 +4,19 @@
>   * Copyright (C) 2022 Advanced Micro Devices, Inc.
>   */
>  
> +#include <linux/dma-fence.h>
> +#include <linux/dma-fence-array.h>
> +#include <linux/dma-fence-chain.h>
>  #include <linux/dma-fence-unwrap.h>
> -#if 0
> -#include <linux/kernel.h>
> -#include <linux/kthread.h>
> -#include <linux/mm.h>
> -#include <linux/sched/signal.h>
> -#include <linux/slab.h>
> -#include <linux/spinlock.h>
> -#include <linux/random.h>
> -#endif
>  
>  #include "selftest.h"
>  
>  #define CHAIN_SZ (4 << 10)
>  
> -static inline struct mock_fence {
> +struct mock_fence {
>  	struct dma_fence base;
>  	spinlock_t lock;
> -} *to_mock_fence(struct dma_fence *f) {
> -	return container_of(f, struct mock_fence, base);
> -}
> +};
>  
>  static const char *mock_name(struct dma_fence *f)
>  {
> @@ -45,7 +37,8 @@ static struct dma_fence *mock_fence(void)
>  		return NULL;
>  
>  	spin_lock_init(&f->lock);
> -	dma_fence_init(&f->base, &mock_ops, &f->lock, 0, 0);
> +	dma_fence_init(&f->base, &mock_ops, &f->lock,
> +		       dma_fence_context_alloc(1), 1);
>  
>  	return &f->base;
>  }
> @@ -113,7 +106,6 @@ static int sanitycheck(void *arg)
>  	if (!chain)
>  		return -ENOMEM;
>  
> -	dma_fence_signal(f);
>  	dma_fence_put(chain);
>  	return err;
>  }
> @@ -154,10 +146,10 @@ static int unwrap_array(void *arg)
>  		err = -EINVAL;
>  	}
>  
> -	dma_fence_signal(f1);
> -	dma_fence_signal(f2);
> +	dma_fence_put(f1);
> +	dma_fence_put(f2);

I'm completely lost on why you add these _put() calls? The reference we
create all get transferred over to the container object, and that takes
care of releasing them.

The other bits with error handling and code cleanup all look good, and
dropping dma_fence_signal calls also makes sense. But this one I don't
get.
-Daniel

>  	dma_fence_put(array);
> -	return 0;
> +	return err;
>  }
>  
>  static int unwrap_chain(void *arg)
> @@ -196,10 +188,10 @@ static int unwrap_chain(void *arg)
>  		err = -EINVAL;
>  	}
>  
> -	dma_fence_signal(f1);
> -	dma_fence_signal(f2);
> +	dma_fence_put(f1);
> +	dma_fence_put(f2);
>  	dma_fence_put(chain);
> -	return 0;
> +	return err;
>  }
>  
>  static int unwrap_chain_array(void *arg)
> @@ -242,10 +234,10 @@ static int unwrap_chain_array(void *arg)
>  		err = -EINVAL;
>  	}
>  
> -	dma_fence_signal(f1);
> -	dma_fence_signal(f2);
> +	dma_fence_put(f1);
> +	dma_fence_put(f2);
>  	dma_fence_put(chain);
> -	return 0;
> +	return err;
>  }
>  
>  int dma_fence_unwrap(void)
> -- 
> 2.25.1
>
Christian König May 5, 2022, 5:46 p.m. UTC | #2 
Am 05.05.22 um 15:29 schrieb Daniel Vetter:
> On Wed, May 04, 2022 at 02:22:52PM +0200, Christian König wrote:
>> The selftests, fix the error handling, remove unused functions and stop
>> leaking memory in failed tests.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/dma-buf/st-dma-fence-unwrap.c | 40 +++++++++++----------------
>>   1 file changed, 16 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/dma-buf/st-dma-fence-unwrap.c b/drivers/dma-buf/st-dma-fence-unwrap.c
>> index 039f016b57be..59628add93f5 100644
>> --- a/drivers/dma-buf/st-dma-fence-unwrap.c
>> +++ b/drivers/dma-buf/st-dma-fence-unwrap.c
>> @@ -4,27 +4,19 @@
>>    * Copyright (C) 2022 Advanced Micro Devices, Inc.
>>    */
>>   
>> +#include <linux/dma-fence.h>
>> +#include <linux/dma-fence-array.h>
>> +#include <linux/dma-fence-chain.h>
>>   #include <linux/dma-fence-unwrap.h>
>> -#if 0
>> -#include <linux/kernel.h>
>> -#include <linux/kthread.h>
>> -#include <linux/mm.h>
>> -#include <linux/sched/signal.h>
>> -#include <linux/slab.h>
>> -#include <linux/spinlock.h>
>> -#include <linux/random.h>
>> -#endif
>>   
>>   #include "selftest.h"
>>   
>>   #define CHAIN_SZ (4 << 10)
>>   
>> -static inline struct mock_fence {
>> +struct mock_fence {
>>   	struct dma_fence base;
>>   	spinlock_t lock;
>> -} *to_mock_fence(struct dma_fence *f) {
>> -	return container_of(f, struct mock_fence, base);
>> -}
>> +};
>>   
>>   static const char *mock_name(struct dma_fence *f)
>>   {
>> @@ -45,7 +37,8 @@ static struct dma_fence *mock_fence(void)
>>   		return NULL;
>>   
>>   	spin_lock_init(&f->lock);
>> -	dma_fence_init(&f->base, &mock_ops, &f->lock, 0, 0);
>> +	dma_fence_init(&f->base, &mock_ops, &f->lock,
>> +		       dma_fence_context_alloc(1), 1);
>>   
>>   	return &f->base;
>>   }
>> @@ -113,7 +106,6 @@ static int sanitycheck(void *arg)
>>   	if (!chain)
>>   		return -ENOMEM;
>>   
>> -	dma_fence_signal(f);
>>   	dma_fence_put(chain);
>>   	return err;
>>   }
>> @@ -154,10 +146,10 @@ static int unwrap_array(void *arg)
>>   		err = -EINVAL;
>>   	}
>>   
>> -	dma_fence_signal(f1);
>> -	dma_fence_signal(f2);
>> +	dma_fence_put(f1);
>> +	dma_fence_put(f2);
> I'm completely lost on why you add these _put() calls?

Because my five year old had a nightmare and I had not enough caffeine 
in my blood stream on the next morning.

Fixed in the next round.

Thanks,
Christian.

>   The reference we
> create all get transferred over to the container object, and that takes
> care of releasing them.
>
> The other bits with error handling and code cleanup all look good, and
> dropping dma_fence_signal calls also makes sense. But this one I don't
> get.
> -Daniel
>
>>   	dma_fence_put(array);
>> -	return 0;
>> +	return err;
>>   }
>>   
>>   static int unwrap_chain(void *arg)
>> @@ -196,10 +188,10 @@ static int unwrap_chain(void *arg)
>>   		err = -EINVAL;
>>   	}
>>   
>> -	dma_fence_signal(f1);
>> -	dma_fence_signal(f2);
>> +	dma_fence_put(f1);
>> +	dma_fence_put(f2);
>>   	dma_fence_put(chain);
>> -	return 0;
>> +	return err;
>>   }
>>   
>>   static int unwrap_chain_array(void *arg)
>> @@ -242,10 +234,10 @@ static int unwrap_chain_array(void *arg)
>>   		err = -EINVAL;
>>   	}
>>   
>> -	dma_fence_signal(f1);
>> -	dma_fence_signal(f2);
>> +	dma_fence_put(f1);
>> +	dma_fence_put(f2);
>>   	dma_fence_put(chain);
>> -	return 0;
>> +	return err;
>>   }
>>   
>>   int dma_fence_unwrap(void)
>> -- 
>> 2.25.1
>>
diff mbox series

Patch

diff --git a/drivers/dma-buf/st-dma-fence-unwrap.c b/drivers/dma-buf/st-dma-fence-unwrap.c
index 039f016b57be..59628add93f5 100644
--- a/drivers/dma-buf/st-dma-fence-unwrap.c
+++ b/drivers/dma-buf/st-dma-fence-unwrap.c
@@ -4,27 +4,19 @@ 
  * Copyright (C) 2022 Advanced Micro Devices, Inc.
  */
 
+#include <linux/dma-fence.h>
+#include <linux/dma-fence-array.h>
+#include <linux/dma-fence-chain.h>
 #include <linux/dma-fence-unwrap.h>
-#if 0
-#include <linux/kernel.h>
-#include <linux/kthread.h>
-#include <linux/mm.h>
-#include <linux/sched/signal.h>
-#include <linux/slab.h>
-#include <linux/spinlock.h>
-#include <linux/random.h>
-#endif
 
 #include "selftest.h"
 
 #define CHAIN_SZ (4 << 10)
 
-static inline struct mock_fence {
+struct mock_fence {
 	struct dma_fence base;
 	spinlock_t lock;
-} *to_mock_fence(struct dma_fence *f) {
-	return container_of(f, struct mock_fence, base);
-}
+};
 
 static const char *mock_name(struct dma_fence *f)
 {
@@ -45,7 +37,8 @@  static struct dma_fence *mock_fence(void)
 		return NULL;
 
 	spin_lock_init(&f->lock);
-	dma_fence_init(&f->base, &mock_ops, &f->lock, 0, 0);
+	dma_fence_init(&f->base, &mock_ops, &f->lock,
+		       dma_fence_context_alloc(1), 1);
 
 	return &f->base;
 }
@@ -113,7 +106,6 @@  static int sanitycheck(void *arg)
 	if (!chain)
 		return -ENOMEM;
 
-	dma_fence_signal(f);
 	dma_fence_put(chain);
 	return err;
 }
@@ -154,10 +146,10 @@  static int unwrap_array(void *arg)
 		err = -EINVAL;
 	}
 
-	dma_fence_signal(f1);
-	dma_fence_signal(f2);
+	dma_fence_put(f1);
+	dma_fence_put(f2);
 	dma_fence_put(array);
-	return 0;
+	return err;
 }
 
 static int unwrap_chain(void *arg)
@@ -196,10 +188,10 @@  static int unwrap_chain(void *arg)
 		err = -EINVAL;
 	}
 
-	dma_fence_signal(f1);
-	dma_fence_signal(f2);
+	dma_fence_put(f1);
+	dma_fence_put(f2);
 	dma_fence_put(chain);
-	return 0;
+	return err;
 }
 
 static int unwrap_chain_array(void *arg)
@@ -242,10 +234,10 @@  static int unwrap_chain_array(void *arg)
 		err = -EINVAL;
 	}
 
-	dma_fence_signal(f1);
-	dma_fence_signal(f2);
+	dma_fence_put(f1);
+	dma_fence_put(f2);
 	dma_fence_put(chain);
-	return 0;
+	return err;
 }
 
 int dma_fence_unwrap(void)