Re: [igt-dev] [PATCH i-g-t v4 04/77] i915/gem_linear_blits: Close device before exit

[Mauro Carvalho Chehab <mauro.chehab@linux.intel.com>]

On Wed, 05 Oct 2022 14:09:18 +0300
Jani Nikula <jani.nikula@linux.intel.com> wrote:

> On Wed, 05 Oct 2022, Mauro Carvalho Chehab <mauro.chehab@linux.intel.com> wrote:
> > From: Chris Wilson <chris.p.wilson@intel.com>
> >
> > Close the device fd before we check for leaks during the atexit
> > handlers.
> >
> > Signed-off-by: Chris Wilson <chris.p.wilson@intel.com>
> > Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
> > Reviewed-by: Nirmoy Das<nirmoy.das@intel.com>
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>
> > ---
> >  tests/i915/gem_linear_blits.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/i915/gem_linear_blits.c b/tests/i915/gem_linear_blits.c
> > index 1fd5b733c3ce..d02751be9232 100644
> > --- a/tests/i915/gem_linear_blits.c
> > +++ b/tests/i915/gem_linear_blits.c
> > @@ -47,6 +47,7 @@
> >  #include "i915/gem.h"
> >  #include "i915/gem_create.h"
> >  #include "igt.h"
> > +#include "igt_types.h"
> >  
> >  IGT_TEST_DESCRIPTION("Test doing many blits with a working set larger than the"
> >  		     " aperture size.");
> > @@ -236,7 +237,7 @@ igt_main
> >  	const int ncpus = sysconf(_SC_NPROCESSORS_ONLN);
> >  	uint64_t count = 0;
> >  	bool do_relocs;
> > -	int fd = -1;
> > +	igt_fd_t(fd);  
> 
> I guess igt has long since stopped being written in C, and started being
> written in an "igt dialect" of its own... but the constructs still kind
> of resemble C.
> 
> This doesn't. It looks like a function call but isn't. Looks odd that
> it's among the declarations... but it is a declaration.

True, but this is not different than what we have already at the Linux
Kernel with things like DECLARE_BITMAP():

   struct fsubdev {
        struct work_struct fw_work;
        struct work_struct pm_work;
        struct delayed_work ue_work;
        struct work_struct rescan_work;
        struct fdev *fdev;
        char __iomem *csr_base;
        int irq;
        bool fw_running;
        DECLARE_BITMAP(errors, NUM_SD_ERRORS);
        struct mbdb *mbdb;
...


and other similar variable declaration macros.

Maybe we could name it at patch 01/77 to:


	#define DECLARE_IGT_FD(x__) \
	       volatile int x__ cleanup_with(igt_cleanup_fd) = IGT_OUTER_SCOPE_INIT(-1)

in order to be more like the Kernel.

In a matter of fact, C is just a nicer and more portable way to work
macro-assembler... :-p


Regards,
Mauro