| 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 |
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 >
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 --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)
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(-)