Re: [PATCH 2/4] lib: Add igt_drop_caches_set()

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Nov 04, 2013 at 04:30:47PM +0000, oscar.mateo@intel.com wrote:
> From: Oscar Mateo <oscar.mateo@intel.com>
> 
> This is basically a "drop cache" interface to the igt_debugfs
> facilities. Also, update existing users.
> 
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  lib/igt_debugfs.c             |   28 ++++++++++++++++++++++++++++
>  lib/igt_debugfs.h             |   15 +++++++++++++++
>  tests/gem_persistent_relocs.c |   15 ++++-----------
>  tests/gem_reloc_vs_gpu.c      |   15 ++++-----------
>  4 files changed, 51 insertions(+), 22 deletions(-)
> 
> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> index 0319eff..fc274fd 100644
> --- a/lib/igt_debugfs.c
> +++ b/lib/igt_debugfs.c
> @@ -316,3 +316,31 @@ igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
>  
>  	*out_crcs = crcs;
>  }
> +
> +/*
> + * Drop caches
> + */
> +
> +int igt_drop_caches_set(uint64_t val)
> +{
> +	igt_debugfs_t debugfs;
> +	int fd;
> +	char data[19];
> +	size_t nbytes;
> +	int ret = -1;
> +
> +	sprintf(data, "0x%" PRIx64, val);
> +
> +	igt_debugfs_init(&debugfs);
> +	fd = igt_debugfs_open(&debugfs, "i915_gem_drop_caches", O_WRONLY);
> +
> +	if (fd >= 0)
> +	{
> +		nbytes = write(fd, data, strlen(data) + 1);
> +		if (nbytes == strlen(data) + 1)
> +			ret = 0;
> +		close(fd);
> +	}
> +
> +	return ret;
> +}

Just a quick style nitpick on igt helpers: If no one cares about the
return value then it's simpler to just sprinkle igt_asserts into the
helper and make the function never fail. I'll bikeshed that in a patch on
top of your series.
-Daniel

> diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
> index c2810ee..02f4afa 100644
> --- a/lib/igt_debugfs.h
> +++ b/lib/igt_debugfs.h
> @@ -79,4 +79,19 @@ void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc);
>  void igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
>  			   igt_crc_t **out_crcs);
>  
> +/*
> + * Drop caches
> + */
> +
> +#define DROP_UNBOUND 0x1
> +#define DROP_BOUND 0x2
> +#define DROP_RETIRE 0x4
> +#define DROP_ACTIVE 0x8
> +#define DROP_ALL (DROP_UNBOUND | \
> +		  DROP_BOUND | \
> +		  DROP_RETIRE | \
> +		  DROP_ACTIVE)
> +
> +int igt_drop_caches_set(uint64_t val);
> +
>  #endif /* __IGT_DEBUGFS_H__ */
> diff --git a/tests/gem_persistent_relocs.c b/tests/gem_persistent_relocs.c
> index 34d8492..863f464 100644
> --- a/tests/gem_persistent_relocs.c
> +++ b/tests/gem_persistent_relocs.c
> @@ -42,6 +42,7 @@
>  #include "intel_bufmgr.h"
>  #include "intel_batchbuffer.h"
>  #include "intel_gpu_tools.h"
> +#include "igt_debugfs.h"
>  
>  /*
>   * Testcase: Persistent relocations as used by uxa/libva
> @@ -287,21 +288,13 @@ static void do_forked_test(int fd, unsigned flags)
>  	struct igt_helper_process thrasher = {};
>  
>  	if (flags & (THRASH | THRASH_INACTIVE)) {
> -		char fname[FILENAME_MAX];
> -		int drop_caches_fd;
> -		const char *data = (flags & THRASH_INACTIVE) ? "0x7" : "0xf";
> -
> -		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);
> -		igt_require(drop_caches_fd >= 0);
> +		uint64_t val = (flags & THRASH_INACTIVE) ?
> +				(DROP_RETIRE | DROP_BOUND | DROP_UNBOUND) : DROP_ALL;
>  
>  		igt_fork_helper(&thrasher) {
>  			while (1) {
>  				usleep(1000);
> -				igt_assert(write(drop_caches_fd, data, strlen(data) + 1) == strlen(data) + 1);
> +				do_or_die(igt_drop_caches_set(val));
>  			}
>  		}
>  	}
> diff --git a/tests/gem_reloc_vs_gpu.c b/tests/gem_reloc_vs_gpu.c
> index bbfdc3a..9508b1c 100644
> --- a/tests/gem_reloc_vs_gpu.c
> +++ b/tests/gem_reloc_vs_gpu.c
> @@ -42,6 +42,7 @@
>  #include "intel_bufmgr.h"
>  #include "intel_batchbuffer.h"
>  #include "intel_gpu_tools.h"
> +#include "igt_debugfs.h"
>  
>  /*
>   * Testcase: Kernel relocations vs. gpu races
> @@ -282,21 +283,13 @@ static void do_forked_test(int fd, unsigned flags)
>  	struct igt_helper_process thrasher = {};
>  
>  	if (flags & (THRASH | THRASH_INACTIVE)) {
> -		char fname[FILENAME_MAX];
> -		int drop_caches_fd;
> -		const char *data = (flags & THRASH_INACTIVE) ? "0x7" : "0xf";
> -
> -		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);
> -		igt_require(drop_caches_fd >= 0);
> +		uint64_t val = (flags & THRASH_INACTIVE) ?
> +				(DROP_RETIRE | DROP_BOUND | DROP_UNBOUND) : DROP_ALL;
>  
>  		igt_fork_helper(&thrasher) {
>  			while (1) {
>  				usleep(1000);
> -				igt_assert(write(drop_caches_fd, data, strlen(data) + 1) == strlen(data) + 1);
> +				do_or_die(igt_drop_caches_set(val));
>  			}
>  		}
>  	}
> -- 
> 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