Re: [PATCH libdrm 3/3] amdgpu: rework and remove amdgpu_get_auth()

["Christian König" <deathsimple@vodafone.de>]

Am 21.08.2017 um 14:31 schrieb Emil Velikov:
> Hi all,
>
> Can anyone skim through this patch?
>
> Thanks
> Emil
>
> On 22 January 2017 at 18:48, Emil Velikov <emil.l.velikov@gmail.com> wrote:
>> All one needs is to establish if dev->fd is the flink (primary/card)
>> node, rather than use DRM_IOCTL_GET_CLIENT to query the auth status.
>>
>> The latter is [somewhat] deprecated and incorrect. We need to know [and
>> store] the primary node FD, since we're going to use it [at a later
>> stage] for buffer import/export sharing.
>>
>> Cc: amd-gfx@lists.freedesktop.org
>> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
>> ---
>> Again not 100% sure but things look quite fishy as-is... The
>> conditionals might be off.
>>
>> Note: original code [and this one] do not consider if flink_fd is
>> already set, thus as we dup we'll leak it.
>> ---
>>   amdgpu/amdgpu_device.c | 43 ++-----------------------------------------
>>   1 file changed, 2 insertions(+), 41 deletions(-)
>>
>> diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c
>> index f4ede031..6f04d936 100644
>> --- a/amdgpu/amdgpu_device.c
>> +++ b/amdgpu/amdgpu_device.c
>> @@ -101,34 +101,6 @@ static int fd_compare(void *key1, void *key2)
>>          return result;
>>   }
>>
>> -/**
>> -* Get the authenticated form fd,
>> -*
>> -* \param   fd   - \c [in]  File descriptor for AMD GPU device
>> -* \param   auth - \c [out] Pointer to output the fd is authenticated or not
>> -*                          A render node fd, output auth = 0
>> -*                          A legacy fd, get the authenticated for compatibility root
>> -*
>> -* \return   0 on success\n
>> -*          >0 - AMD specific error code\n
>> -*          <0 - Negative POSIX Error code
>> -*/
>> -static int amdgpu_get_auth(int fd, int *auth)
>> -{
>> -       int r = 0;
>> -       drm_client_t client = {};
>> -
>> -       if (drmGetNodeTypeFromFd(fd) == DRM_NODE_RENDER)
>> -               *auth = 0;
>> -       else {
>> -               client.idx = 0;
>> -               r = drmIoctl(fd, DRM_IOCTL_GET_CLIENT, &client);
>> -               if (!r)
>> -                       *auth = client.auth;
>> -       }
>> -       return r;
>> -}
>> -
>>   static void amdgpu_device_free_internal(amdgpu_device_handle dev)
>>   {
>>          amdgpu_vamgr_deinit(dev->vamgr);
>> @@ -175,8 +147,6 @@ int amdgpu_device_initialize(int fd,
>>          struct amdgpu_device *dev;
>>          drmVersionPtr version;
>>          int r;
>> -       int flag_auth = 0;
>> -       int flag_authexist=0;
>>          uint32_t accel_working = 0;
>>          uint64_t start, max;
>>
>> @@ -185,19 +155,10 @@ int amdgpu_device_initialize(int fd,
>>          pthread_mutex_lock(&fd_mutex);
>>          if (!fd_tab)
>>                  fd_tab = util_hash_table_create(fd_hash, fd_compare);
>> -       r = amdgpu_get_auth(fd, &flag_auth);
>> -       if (r) {
>> -               pthread_mutex_unlock(&fd_mutex);
>> -               return r;
>> -       }
>>          dev = util_hash_table_get(fd_tab, UINT_TO_PTR(fd));
>>          if (dev) {
>> -               r = amdgpu_get_auth(dev->fd, &flag_authexist);
>> -               if (r) {
>> -                       pthread_mutex_unlock(&fd_mutex);
>> -                       return r;
>> -               }
>> -               if ((flag_auth) && (!flag_authexist)) {
>> +               if (drmGetNodeTypeFromFd(fd) == DRM_NODE_RENDER &&
>> +                   drmGetNodeTypeFromFd(dev->fd) == DRM_NODE_PRIMARY) {

First of all that looks inversed the logic. In other words we want to 
keep the primary as flink_fd, not the render node one.

Second you are relaxing the test here which I can't judge if it's a good 
idea or not.

Sorry that I can't help much more,
Christian.

>>                          dev->flink_fd = dup(fd);
>>                  }
>>                  *major_version = dev->major_version;
>> --
>> 2.11.0
>>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel