| Message ID | 20220506141009.18047-1-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/5] dma-buf: cleanup dma_fence_unwrap selftest v2 | expand |
I had to send this out once more. This time with the right mail addresses and a much simplified patch #3. Christian. Am 06.05.22 um 16:10 schrieb Christian König: > The selftests, fix the error handling, remove unused functions and stop > leaking memory in failed tests. > > v2: fix the memory leak correctly. > > Signed-off-by: Christian König <christian.koenig@amd.com> > --- > drivers/dma-buf/st-dma-fence-unwrap.c | 48 +++++++++++---------------- > 1 file changed, 19 insertions(+), 29 deletions(-) > > diff --git a/drivers/dma-buf/st-dma-fence-unwrap.c b/drivers/dma-buf/st-dma-fence-unwrap.c > index 039f016b57be..e20c5a7dcfe4 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; > } > @@ -59,7 +52,7 @@ static struct dma_fence *mock_array(unsigned int num_fences, ...) > > fences = kcalloc(num_fences, sizeof(*fences), GFP_KERNEL); > if (!fences) > - return NULL; > + goto error_put; > > va_start(valist, num_fences); > for (i = 0; i < num_fences; ++i) > @@ -70,13 +63,17 @@ static struct dma_fence *mock_array(unsigned int num_fences, ...) > dma_fence_context_alloc(1), > 1, false); > if (!array) > - goto cleanup; > + goto error_free; > return &array->base; > > -cleanup: > - for (i = 0; i < num_fences; ++i) > - dma_fence_put(fences[i]); > +error_free: > kfree(fences); > + > +error_put: > + va_start(valist, num_fences); > + for (i = 0; i < num_fences; ++i) > + dma_fence_put(va_arg(valist, typeof(*fences))); > + va_end(valist); > return NULL; > } > > @@ -113,7 +110,6 @@ static int sanitycheck(void *arg) > if (!chain) > return -ENOMEM; > > - dma_fence_signal(f); > dma_fence_put(chain); > return err; > } > @@ -154,10 +150,8 @@ static int unwrap_array(void *arg) > err = -EINVAL; > } > > - dma_fence_signal(f1); > - dma_fence_signal(f2); > dma_fence_put(array); > - return 0; > + return err; > } > > static int unwrap_chain(void *arg) > @@ -196,10 +190,8 @@ static int unwrap_chain(void *arg) > err = -EINVAL; > } > > - dma_fence_signal(f1); > - dma_fence_signal(f2); > dma_fence_put(chain); > - return 0; > + return err; > } > > static int unwrap_chain_array(void *arg) > @@ -242,10 +234,8 @@ static int unwrap_chain_array(void *arg) > err = -EINVAL; > } > > - dma_fence_signal(f1); > - dma_fence_signal(f2); > dma_fence_put(chain); > - return 0; > + return err; > } > > int dma_fence_unwrap(void)
Hey Daniel, a gentle ping on this here. Those patches come before my drm-exec work, so would be nice if we could get that reviewed first. Thanks, Christian. Am 06.05.22 um 16:11 schrieb Christian König: > I had to send this out once more. > > This time with the right mail addresses and a much simplified patch #3. > > Christian. > > Am 06.05.22 um 16:10 schrieb Christian König: >> The selftests, fix the error handling, remove unused functions and stop >> leaking memory in failed tests. >> >> v2: fix the memory leak correctly. >> >> Signed-off-by: Christian König <christian.koenig@amd.com> >> --- >> drivers/dma-buf/st-dma-fence-unwrap.c | 48 +++++++++++---------------- >> 1 file changed, 19 insertions(+), 29 deletions(-) >> >> diff --git a/drivers/dma-buf/st-dma-fence-unwrap.c >> b/drivers/dma-buf/st-dma-fence-unwrap.c >> index 039f016b57be..e20c5a7dcfe4 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; >> } >> @@ -59,7 +52,7 @@ static struct dma_fence *mock_array(unsigned int >> num_fences, ...) >> fences = kcalloc(num_fences, sizeof(*fences), GFP_KERNEL); >> if (!fences) >> - return NULL; >> + goto error_put; >> va_start(valist, num_fences); >> for (i = 0; i < num_fences; ++i) >> @@ -70,13 +63,17 @@ static struct dma_fence *mock_array(unsigned int >> num_fences, ...) >> dma_fence_context_alloc(1), >> 1, false); >> if (!array) >> - goto cleanup; >> + goto error_free; >> return &array->base; >> -cleanup: >> - for (i = 0; i < num_fences; ++i) >> - dma_fence_put(fences[i]); >> +error_free: >> kfree(fences); >> + >> +error_put: >> + va_start(valist, num_fences); >> + for (i = 0; i < num_fences; ++i) >> + dma_fence_put(va_arg(valist, typeof(*fences))); >> + va_end(valist); >> return NULL; >> } >> @@ -113,7 +110,6 @@ static int sanitycheck(void *arg) >> if (!chain) >> return -ENOMEM; >> - dma_fence_signal(f); >> dma_fence_put(chain); >> return err; >> } >> @@ -154,10 +150,8 @@ static int unwrap_array(void *arg) >> err = -EINVAL; >> } >> - dma_fence_signal(f1); >> - dma_fence_signal(f2); >> dma_fence_put(array); >> - return 0; >> + return err; >> } >> static int unwrap_chain(void *arg) >> @@ -196,10 +190,8 @@ static int unwrap_chain(void *arg) >> err = -EINVAL; >> } >> - dma_fence_signal(f1); >> - dma_fence_signal(f2); >> dma_fence_put(chain); >> - return 0; >> + return err; >> } >> static int unwrap_chain_array(void *arg) >> @@ -242,10 +234,8 @@ static int unwrap_chain_array(void *arg) >> err = -EINVAL; >> } >> - dma_fence_signal(f1); >> - dma_fence_signal(f2); >> dma_fence_put(chain); >> - return 0; >> + return err; >> } >> int dma_fence_unwrap(void) >
On Fri, May 06, 2022 at 04:10:05PM +0200, Christian König wrote: > The selftests, fix the error handling, remove unused functions and stop > leaking memory in failed tests. > > v2: fix the memory leak correctly. > > Signed-off-by: Christian König <christian.koenig@amd.com> I'm still a bit lost on all this (maybe add an explainer why you want to drop dma_fence_signal() - it's just not necessary for test functionality). But I think it's at least correct now. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> I've seen you've resent it to get intel-gfx-ci to look at it, so assuming that's all fine I think it's now all reviewed and ready for merging. -Daniel > --- > drivers/dma-buf/st-dma-fence-unwrap.c | 48 +++++++++++---------------- > 1 file changed, 19 insertions(+), 29 deletions(-) > > diff --git a/drivers/dma-buf/st-dma-fence-unwrap.c b/drivers/dma-buf/st-dma-fence-unwrap.c > index 039f016b57be..e20c5a7dcfe4 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; > } > @@ -59,7 +52,7 @@ static struct dma_fence *mock_array(unsigned int num_fences, ...) > > fences = kcalloc(num_fences, sizeof(*fences), GFP_KERNEL); > if (!fences) > - return NULL; > + goto error_put; > > va_start(valist, num_fences); > for (i = 0; i < num_fences; ++i) > @@ -70,13 +63,17 @@ static struct dma_fence *mock_array(unsigned int num_fences, ...) > dma_fence_context_alloc(1), > 1, false); > if (!array) > - goto cleanup; > + goto error_free; > return &array->base; > > -cleanup: > - for (i = 0; i < num_fences; ++i) > - dma_fence_put(fences[i]); > +error_free: > kfree(fences); > + > +error_put: > + va_start(valist, num_fences); > + for (i = 0; i < num_fences; ++i) > + dma_fence_put(va_arg(valist, typeof(*fences))); > + va_end(valist); > return NULL; > } > > @@ -113,7 +110,6 @@ static int sanitycheck(void *arg) > if (!chain) > return -ENOMEM; > > - dma_fence_signal(f); > dma_fence_put(chain); > return err; > } > @@ -154,10 +150,8 @@ static int unwrap_array(void *arg) > err = -EINVAL; > } > > - dma_fence_signal(f1); > - dma_fence_signal(f2); > dma_fence_put(array); > - return 0; > + return err; > } > > static int unwrap_chain(void *arg) > @@ -196,10 +190,8 @@ static int unwrap_chain(void *arg) > err = -EINVAL; > } > > - dma_fence_signal(f1); > - dma_fence_signal(f2); > dma_fence_put(chain); > - return 0; > + return err; > } > > static int unwrap_chain_array(void *arg) > @@ -242,10 +234,8 @@ static int unwrap_chain_array(void *arg) > err = -EINVAL; > } > > - dma_fence_signal(f1); > - dma_fence_signal(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..e20c5a7dcfe4 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; } @@ -59,7 +52,7 @@ static struct dma_fence *mock_array(unsigned int num_fences, ...) fences = kcalloc(num_fences, sizeof(*fences), GFP_KERNEL); if (!fences) - return NULL; + goto error_put; va_start(valist, num_fences); for (i = 0; i < num_fences; ++i) @@ -70,13 +63,17 @@ static struct dma_fence *mock_array(unsigned int num_fences, ...) dma_fence_context_alloc(1), 1, false); if (!array) - goto cleanup; + goto error_free; return &array->base; -cleanup: - for (i = 0; i < num_fences; ++i) - dma_fence_put(fences[i]); +error_free: kfree(fences); + +error_put: + va_start(valist, num_fences); + for (i = 0; i < num_fences; ++i) + dma_fence_put(va_arg(valist, typeof(*fences))); + va_end(valist); return NULL; } @@ -113,7 +110,6 @@ static int sanitycheck(void *arg) if (!chain) return -ENOMEM; - dma_fence_signal(f); dma_fence_put(chain); return err; } @@ -154,10 +150,8 @@ static int unwrap_array(void *arg) err = -EINVAL; } - dma_fence_signal(f1); - dma_fence_signal(f2); dma_fence_put(array); - return 0; + return err; } static int unwrap_chain(void *arg) @@ -196,10 +190,8 @@ static int unwrap_chain(void *arg) err = -EINVAL; } - dma_fence_signal(f1); - dma_fence_signal(f2); dma_fence_put(chain); - return 0; + return err; } static int unwrap_chain_array(void *arg) @@ -242,10 +234,8 @@ static int unwrap_chain_array(void *arg) err = -EINVAL; } - dma_fence_signal(f1); - dma_fence_signal(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. v2: fix the memory leak correctly. Signed-off-by: Christian König <christian.koenig@amd.com> --- drivers/dma-buf/st-dma-fence-unwrap.c | 48 +++++++++++---------------- 1 file changed, 19 insertions(+), 29 deletions(-)