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

[Christian König <christian.koenig@xxxxxxx>]

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@xxxxxxx>
> > ---
> >   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