Re: [PATCH] prime_self_import: Assure no pending requests before object counting

[Ben Widawsky <ben@bwidawsk.net>]

On Fri, Nov 01, 2013 at 05:08:17PM +0100, Daniel Vetter wrote:
> On Fri, Nov 01, 2013 at 12:53:42PM +0000, oscar.mateo@intel.com wrote:
> > From: Oscar Mateo <oscar.mateo@intel.com>
> > 
> > We don't want a previously used object to be freed in the middle of a
> > before/after object counting operation (or we would get a "-1 objects
> > leaked" message). We have seen this happening, e.g., when a context
> > from a previous run dies, but its backing object is alive waiting for
> > a retire_work to kick in.
> > 
> > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> > Cc: Ben Widawsky <ben@bwidawsk.net>
> 
> Nice catch. Should we do this in general as part of our gem_quiescent_gpu
> helper? All i-g-t testcase are written under the assumption that they
> completel own the gpu and that the gtt is completely empty besides the few
> driver-allocated and pinned objects. So trying really hard to get rid of
> any residual stuff sounds like a good idea.

I was going to address this in the other mail thread.... in any case, I
think not. I believe a separate helper is the way to go, and we should
only call it when we absolutely want to.

Though it's not the intention, I've seen many tests fail because of
previous state, and I don't want to miss out on those in the future. It
would also slow down the run unnecessarily further.

> 
> Then we could just add a call to gem_quiescent_gpu to the get_object_count
> function.
> 
> Care to rework the patch a bit and make sure it all still works?
> 
> Thanks, Daniel
> 
> > ---
> >  tests/prime_self_import.c |   29 +++++++++++++++++++++++++++--
> >  1 file changed, 27 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/prime_self_import.c b/tests/prime_self_import.c
> > index 481a809..e48abd1 100644
> > --- a/tests/prime_self_import.c
> > +++ b/tests/prime_self_import.c
> > @@ -211,6 +211,25 @@ static void test_with_one_bo(void)
> >  	check_bo(fd2, handle_import1, fd2, handle_import1);
> >  }
> >  
> > +static void retire_requests(void)
> > +{
> > +	char fname[FILENAME_MAX];
> > +	int drop_caches_fd;
> > +	const char *data = "0x4";
> > +
> > +	snprintf(fname, FILENAME_MAX, "%s/%i/%s",
> > +		 "/sys/kernel/debug/dri", drm_get_card(),
> > +		 "i915_gem_drop_caches");
> > +
> > +	drop_caches_fd = open(fname, O_WRONLY);
> > +
> > +	if (drop_caches_fd >= 0)
> > +	{
> > +		write(drop_caches_fd, data, strlen(data) + 1);
> > +		close(drop_caches_fd);
> > +	}
> > +}
> > +
> >  static int get_object_count(void)
> >  {
> >  	FILE *file;
> > @@ -252,10 +271,13 @@ static void test_reimport_close_race(void)
> >  	pthread_t *threads;
> >  	int r, i, num_threads;
> >  	int fds[2];
> > -	int obj_count = get_object_count();
> > +	int obj_count;
> >  	void *status;
> >  	uint32_t handle;
> >  
> > +	retire_requests();
> > +	obj_count = get_object_count();
> > +
> >  	num_threads = sysconf(_SC_NPROCESSORS_ONLN);
> >  
> >  	threads = calloc(num_threads, sizeof(pthread_t));
> > @@ -330,9 +352,12 @@ static void test_export_close_race(void)
> >  	pthread_t *threads;
> >  	int r, i, num_threads;
> >  	int fd;
> > -	int obj_count = get_object_count();
> > +	int obj_count;
> >  	void *status;
> >  
> > +	retire_requests();
> > +	obj_count = get_object_count();
> > +
> >  	num_threads = sysconf(_SC_NPROCESSORS_ONLN);
> >  
> >  	threads = calloc(num_threads, sizeof(pthread_t));
> > -- 
> > 1.7.9.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center