[Mesa-dev] [PATCH mesa 2/4] vulkan: add VK_EXT_display_control [v8]

[Keith Packard]

Jason Ekstrand <jason at jlekstrand.net> writes:

>> +      if (!ret)
>> +         return VK_SUCCESS;
>> +
>> +      if (errno != ENOMEM) {
>
> This strikes me as a bit odd. What does ENOMEM mean if not "out of
> memory"?

ENOMEM means that the queue is full and that we should drain it and try
again; that's what the wait_for_event call is below.

The other-than-ENOMEM case is for some other failure, such as VT switch
or lease revoke. For RegisterDisplayEvent, there aren't any return
values other than VK_SUCCESS defined, and we're already assuming we can
use VK_OUT_OF_HOST_MEMORY for any function which allocates memory.

I think the correct value might be VK_ERROR_DEVICE_LOST or
VK_ERROR_OUT_OF_DATE_KHR as something "bad" has clearly happened? The
other place this is called is from QueuePresent, where either of those
error codes are allowed. I could convert that message to
VK_OUT_OF_HOST_MEMORY for RegisterDisplayEvent if you think that's a
good idea.

The sleep prevents an application from spinning at this failure,
allowing the user to gracefully terminate the application.

>
>> +         wsi_display_debug("queue vblank event %lu failed\n", 
>> fence->sequence);
>> +         struct timespec delay = {
>> +            .tv_sec = 0,
>> +            .tv_nsec = 100000000ull,
>> +         };
>> +         nanosleep(&delay, NULL);
>> +         return VK_ERROR_OUT_OF_HOST_MEMORY;
>
> Given your previous explanation, I think this is ok but I think it deserves 
> a comment.

Wilco.

I've added comments to this section to try and explain what's going on:

      if (!ret)
         return VK_SUCCESS;

      if (errno != ENOMEM) {

         /* Something unexpected happened. Pause for a moment so the
          * application doesn't just spin and then return a failure indication
          */

         wsi_display_debug("queue vblank event %lu failed\n", fence->sequence);
         struct timespec delay = {
            .tv_sec = 0,
            .tv_nsec = 100000000ull,
         };
         nanosleep(&delay, NULL);
         return VK_ERROR_OUT_OF_HOST_MEMORY;
      }

      /* The kernel event queue is full. Wait for some events to be
       * processed and try again
       */

      pthread_mutex_lock(&wsi->wait_mutex);
      ret = wsi_display_wait_for_event(wsi, wsi_rel_to_abs_time(100000000ull));
      pthread_mutex_unlock(&wsi->wait_mutex);

      if (ret) {
         wsi_display_debug("vblank queue full, event wait failed\n");
         return VK_ERROR_OUT_OF_HOST_MEMORY;
      }

-- 
-keith
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180621/eb31e674/attachment.sig>