Re: [PATCH 3/3] tests/gem_userptr_benchmark: Benchmarking userptr surfaces and impact

Volkin, Bradley D
2014-04-23T15:25:35Z

[snip]

On Wed, Apr 23, 2014 at 06:28:54AM -0700, Tvrtko Ursulin wrote:
> On 04/18/2014 12:18 AM, Volkin, Bradley D wrote:
> > On Wed, Mar 19, 2014 at 04:13:06AM -0700, Tvrtko Ursulin wrote:
> >> +static void **handle_ptr_map;
> >> +static unsigned int num_handle_ptr_map;
> >
> > I'd prefer that we explicitly initialize at least num_handle_ptr_map.
>
> To zero, why?

Partly because I just like explicitly initialized variables and
partly because I forgot that static variables are *guaranteed* to
be initialized to zero vs just-happen-to-be-zero :)

You can leave it as is or change it, I'm fine either way.

Thanks,
Brad

>
> >> +
> >> +static void add_handle_ptr(uint32_t handle, void *ptr)
> >> +{
> >> + if (handle >= num_handle_ptr_map) {
> >> + handle_ptr_map = realloc(handle_ptr_map,
> >> + (handle + 1000) * sizeof(void*));
> >> + num_handle_ptr_map = handle + 1000;
> >> + }
> >> +
> >> + handle_ptr_map[handle] = ptr;
> >> +}
> >> +
> >> +static void *get_handle_ptr(uint32_t handle)
> >> +{
> >> + return handle_ptr_map[handle];
> >> +}
> >> +
> >> +static void free_handle_ptr(uint32_t handle)
> >> +{
> >> + igt_assert(handle < num_handle_ptr_map);
> >> + igt_assert(handle_ptr_map[handle]);
> >> +
> >> + free(handle_ptr_map[handle]);
> >> + handle_ptr_map[handle] = NULL;
> >> +}
> >> +
> >> +static uint32_t create_userptr_bo(int fd, int size)
> >> +{
> >> + void *ptr;
> >> + uint32_t handle;
> >> + int ret;
> >> +
> >> + ret = posix_memalign(&ptr, PAGE_SIZE, size);
> >> + igt_assert(ret == 0);
> >> +
> >> + ret = gem_userptr(fd, (uint32_t *)ptr, size, 0, &handle);
> >> + igt_assert(ret == 0);
> >> + add_handle_ptr(handle, ptr);
> >> +
> >> + return handle;
> >> +}
> >> +
> >> +static void free_userptr_bo(int fd, uint32_t handle)
> >> +{
> >> + gem_close(fd, handle);
> >> + free_handle_ptr(handle);
> >> +}
> >> +
> >> +static int has_userptr(int fd)
> >> +{
> >> + uint32_t handle = 0;
> >> + void *ptr;
> >> + uint32_t oldflags;
> >> + int ret;
> >> +
> >> + assert(posix_memalign(&ptr, PAGE_SIZE, PAGE_SIZE) == 0);
> >> + oldflags = userptr_flags;
> >> + gem_userptr_test_unsynchronized();
> >> + ret = gem_userptr(fd, ptr, PAGE_SIZE, 0, &handle);
> >> + userptr_flags = oldflags;
> >> + if (ret != 0) {
> >> + free(ptr);
> >> + return 0;
> >> + }
> >> +
> >> + gem_close(fd, handle);
> >> + free(ptr);
> >> +
> >> + return handle != 0;
> >> +}
> >> +
> >> +static const unsigned int nr_bos[] = {0, 1, 10, 100, 1000};
> >> +static const unsigned int test_duration_sec = 3;
> >> +
> >> +static volatile unsigned int run_test;
> >> +
> >> +static void alarm_handler(int sig)
> >> +{
> >> + assert(run_test == 1);
> >> + run_test = 0;
> >> +}
> >> +
> >> +static void start_test(unsigned int duration)
> >> +{
> >> + run_test = 1;
> >> + if (duration == 0)
> >> + duration = test_duration_sec;
> >> + signal(SIGALRM, alarm_handler);
> >> + alarm(duration);
> >> +}
> >> +
> >> +static void exchange_ptr(void *array, unsigned i, unsigned j)
> >> +{
> >> + void **arr, *tmp;
> >> + arr = (void **)array;
> >> +
> >> + tmp = arr[i];
> >> + arr[i] = arr[j];
> >> + arr[j] = tmp;
> >> +}
> >> +
> >> +static void test_malloc_free(int random)
> >> +{
> >> + unsigned long iter = 0;
> >> + unsigned int i, tot = 1000;
> >> + void *ptr[tot];
> >> +
> >> + start_test(test_duration_sec);
> >> +
> >> + while (run_test) {
> >> + for (i = 0; i < tot; i++) {
> >> + ptr[i] = malloc(1000);
> >> + assert(ptr[i]);
> >> + }
> >> + if (random)
> >> + igt_permute_array(ptr, tot, exchange_ptr);
> >> + for (i = 0; i < tot; i++)
> >> + free(ptr[i]);
> >> + iter++;
> >> + }
> >> +
> >> + printf("%8lu iter/s\n", iter / test_duration_sec);
> >> +}
> >> +
> >> +static void test_malloc_realloc_free(int random)
> >> +{
> >> + unsigned long iter = 0;
> >> + unsigned int i, tot = 1000;
> >> + void *ptr[tot];
> >> +
> >> + start_test(test_duration_sec);
> >> +
> >> + while (run_test) {
> >> + for (i = 0; i < tot; i++) {
> >> + ptr[i] = malloc(1000);
> >> + assert(ptr[i]);
> >> + }
> >> + if (random)
> >> + igt_permute_array(ptr, tot, exchange_ptr);
> >> + for (i = 0; i < tot; i++) {
> >> + ptr[i] = realloc(ptr[i], 2000);
> >> + assert(ptr[i]);
> >> + }
> >> + if (random)
> >> + igt_permute_array(ptr, tot, exchange_ptr);
> >> + for (i = 0; i < tot; i++)
> >> + free(ptr[i]);
> >> + iter++;
> >> + }
> >> +
> >> + printf("%8lu iter/s\n", iter / test_duration_sec);
> >> +}
> >> +
> >> +static void test_mmap_unmap(int random)
> >> +{
> >> + unsigned long iter = 0;
> >> + unsigned int i, tot = 1000;
> >> + void *ptr[tot];
> >> +
> >> + start_test(test_duration_sec);
> >> +
> >> + while (run_test) {
> >> + for (i = 0; i < tot; i++) {
> >> + ptr[i] = mmap(NULL, 1000, PROT_READ | PROT_WRITE,
> >> + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> >> + assert(ptr[i] != MAP_FAILED);
> >> + }
> >> + if (random)
> >> + igt_permute_array(ptr, tot, exchange_ptr);
> >> + for (i = 0; i < tot; i++)
> >> + munmap(ptr[i], 1000);
> >> + iter++;
> >> + }
> >> +
> >> + printf("%8lu iter/s\n", iter / test_duration_sec);
> >> +}
> >> +
> >> +static void test_ptr_read(void *ptr)
> >> +{
> >> + unsigned long iter = 0;
> >> + volatile unsigned long *p;
> >> + unsigned long i, loops;
> >> + register unsigned long v;
> >> +
> >> + loops = sizeof(linear) / sizeof(unsigned long) / 4;
> >> +
> >> + start_test(test_duration_sec);
> >> +
> >> + while (run_test) {
> >> + p = (unsigned long *)ptr;
> >> + for (i = 0; i < loops; i++) {
> >> + v = *p++;
> >> + v = *p++;
> >> + v = *p++;
> >> + v = *p++;
> >> + }
> >> + iter++;
> >> + }
> >> +
> >> + printf("%8lu MB/s\n", iter / test_duration_sec * sizeof(linear) / 1000000);
> >> +}
> >> +
> >> +static void test_ptr_write(void *ptr)
> >> +{
> >> + unsigned long iter = 0;
> >> + volatile unsigned long *p;
> >> + register unsigned long i, loops;
> >> +
> >> + loops = sizeof(linear) / sizeof(unsigned long) / 4;
> >> +
> >> + start_test(test_duration_sec);
> >> +
> >> + while (run_test) {
> >> + p = (unsigned long *)ptr;
> >> + for (i = 0; i < loops; i++) {
> >> + *p++ = i;
> >> + *p++ = i;
> >> + *p++ = i;
> >> + *p++ = i;
> >> + }
> >> + iter++;
> >> + }
> >> +
> >> + printf("%8lu MB/s\n", iter / test_duration_sec * sizeof(linear) / 1000000);
> >> +}
> >> +
> >> +static void test_impact(int fd)
> >> +{
> >> + unsigned int total = sizeof(nr_bos) / sizeof(nr_bos[0]);
> >> + unsigned int subtest, i;
> >> + uint32_t handles[nr_bos[total-1]];
> >> + void *ptr;
> >> + char buffer[sizeof(linear)];
> >> +
> >> + for (subtest = 0; subtest < total; subtest++) {
> >> + for (i = 0; i < nr_bos[subtest]; i++)
> >> + handles[i] = create_userptr_bo(fd, sizeof(linear));
> >> +
> >> + if (nr_bos[subtest] > 0)
> >> + ptr = get_handle_ptr(handles[0]);
> >> + else
> >> + ptr = buffer;
> >> +
> >> + printf("ptr-read, %5u bos = ", nr_bos[subtest]);
> >> + test_ptr_read(ptr);
> >> +
> >> + printf("ptr-write %5u bos = ", nr_bos[subtest]);
> >> + test_ptr_write(ptr);
> >> +
> >> + printf("malloc-free, %5u bos = ", nr_bos[subtest]);
> >> + test_malloc_free(0);
> >> + printf("malloc-free-random %5u bos = ", nr_bos[subtest]);
> >> + test_malloc_free(1);
> >> +
> >> + printf("malloc-realloc-free, %5u bos = ", nr_bos[subtest]);
> >> + test_malloc_realloc_free(0);
> >> + printf("malloc-realloc-free-random, %5u bos = ", nr_bos[subtest]);
> >> + test_malloc_realloc_free(1);
> >> +
> >> + printf("mmap-unmap, %5u bos = ", nr_bos[subtest]);
> >> + test_mmap_unmap(0);
> >> + printf("mmap-unmap-random, %5u bos = ", nr_bos[subtest]);
> >> + test_mmap_unmap(1);
> >> +
> >> + for (i = 0; i < nr_bos[subtest]; i++)
> >> + free_userptr_bo(fd, handles[i]);
> >> + }
> >> +}
> >> +
> >> +static void test_single(int fd)
> >> +{
> >> + char *ptr, *bo_ptr;
> >> + uint32_t handle = 0;
> >> + unsigned long iter = 0;
> >> + int ret;
> >> + unsigned long map_size = sizeof(linear) + PAGE_SIZE - 1;
> >> +
> >> + ptr = mmap(NULL, map_size, PROT_READ | PROT_WRITE,
> >> + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> >> + assert(ptr != MAP_FAILED);
> >> +
> >> + bo_ptr = (char *)(((unsigned long)ptr + (PAGE_SIZE - 1))
> >> + & ~(PAGE_SIZE - 1));
> >
> > You might add an ALIGN macro in this file or a suitable header. A couple of
> > .c files in lib/ individually define one already.
>
> OK, will add that in the patch series.
>
> Thanks for the review!
>
> Regards,
>
> Tvrtko
>