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

[Jani Nikula]

On Wed, 05 Oct 2022, Mauro Carvalho Chehab <mauro.chehab at linux.intel.com> wrote:
> On Wed, 05 Oct 2022 14:09:18 +0300
> Jani Nikula <jani.nikula at linux.intel.com> wrote:
>
>> On Wed, 05 Oct 2022, Mauro Carvalho Chehab <mauro.chehab at linux.intel.com> wrote:
>> > From: Chris Wilson <chris.p.wilson at intel.com>
>> >
>> > Close the device fd before we check for leaks during the atexit
>> > handlers.
>> >
>> > Signed-off-by: Chris Wilson <chris.p.wilson at intel.com>
>> > Reviewed-by: Andrzej Hajda <andrzej.hajda at intel.com>
>> > Reviewed-by: Nirmoy Das<nirmoy.das at intel.com>
>> > Signed-off-by: Mauro Carvalho Chehab <mchehab at 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.

Yeah, maybe "declare" in the name would be helpful.

I was just making an observation, really, and I'll defer the decisions
to Petri here.

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

Macros really are a blessing and a curse here...

BR,
Jani.


>
>
> Regards,
> Mauro

-- 
Jani Nikula, Intel Open Source Graphics Center