Re: kcmp implementation for mesa

Hi Ivan,

Looks good. Could you please put patch on reviews.freebsd.org?

Thx,
Michael

On Fri, 19 Jan 2024, 14:11 Rozhuk Ivan, <rozhuk.im@gmail.com> wrote:

> Hi!
>
>
> graphics/mesa-* uses SYS_kcmp [1] to compare two fds:
>
> int
> os_same_file_description(int fd1, int fd2)
> {
>    pid_t pid = getpid();
>
>    /* Same file descriptor trivially implies same file description */
>    if (fd1 == fd2)
>       return 0;
>
>    return syscall(SYS_kcmp, pid, pid, KCMP_FILE, fd1, fd2);
> }
>
> FreeBSD does not implemet this and we got in terminal:
> "amdgpu: os_same_file_description couldn't determine if two DRM fds
> reference the same file description."
>
>
> Mesa say:
> /* DRM file descriptors, file descriptions and buffer sharing.
>  *
>  * amdgpu_device_initialize first argument is a file descriptor (fd)
>  * representing a specific GPU.
>  * If a fd is duplicated using os_dupfd_cloexec,
>  * the file description will remain the same (os_same_file_description will
>  * return 0).
>  * But if the same device is re-opened, the fd and the file description
> will
>  * be different.
>  *
>  * amdgpu_screen_winsys's fd tracks the file description which was
>  * given to amdgpu_winsys_create. This is the fd used by the application
>  * using the driver and may be used in other ioctl (eg: drmModeAddFB)
>  *
>  * amdgpu_winsys's fd is the file description used to initialize the
>  * device handle in libdrm_amdgpu.
>  *
>  * The 2 fds can be different, even in systems with a single GPU, eg: if
>  * radv is initialized before radeonsi.
>  *
>  * This fd tracking is useful for buffer sharing because KMS/GEM handles
> are
>  * specific to a DRM file description, i.e. the same handle value may refer
>  * to different underlying BOs in different DRM file descriptions.
>  * As an example, if an app wants to use drmModeAddFB it'll need a KMS
> handle
>  * valid for its fd (== amdgpu_screen_winsys::fd).
>  * If both fds are identical, there's nothing to do: bo->u.real.kms_handle
>  * can be used directly (see amdgpu_bo_get_handle).
>  * If they're different, the BO has to be exported from the device fd as
>  * a dma-buf, then imported from the app fd as a KMS handle.
>  */
>
>
>
> I do few checks with dup() and os_dupfd_cloexec() and code show that fd
> equal.
>
> Does this implementation will do that mesa expects?
>
>
> #include <sys/user.h>
> #include <fcntl.h>
>
> int
> os_same_file_description(int fd1, int fd2) {
>         struct kinfo_file kif1, kif2;
>
>         /* Same file descriptor trivially implies same file description */
>         if (fd1 == fd2)
>                 return (0);
>
>         kif1.kf_structsize = sizeof(kif1);
>         kif2.kf_structsize = sizeof(kif2);
>         if (-1 == fcntl(fd1, F_KINFO, &kif1) ||
>             -1 == fcntl(fd2, F_KINFO, &kif2))
>                 return (-1);
>
>         if (kif1.kf_type != kif2.kf_type ||
>             0 != memcmp(&kif1.kf_path, &kif2.kf_path,
> sizeof(kif1.kf_path)))
>                 return (3);
>
>         switch (kif1.kf_type) {
>         case KF_TYPE_VNODE:
>                 if (0 == memcmp(&kif1.kf_un.kf_file, &kif2.kf_un.kf_file,
>                     sizeof(kif1.kf_un.kf_file)))
>                         return (0);
>                 return (3);
>         case KF_TYPE_SOCKET:
>                 if (0 == memcmp(&kif1.kf_un.kf_sock, &kif2.kf_un.kf_sock,
>                     sizeof(kif1.kf_un.kf_sock)))
>                         return (0);
>                 return (3);
>         case KF_TYPE_PIPE:
>                 if (0 == memcmp(&kif1.kf_un.kf_pipe, &kif2.kf_un.kf_pipe,
>                     sizeof(kif1.kf_un.kf_pipe)))
>                         return (0);
>                 return (3);
>         //case KF_TYPE_FIFO:
>         case KF_TYPE_KQUEUE:
>                 if (0 == memcmp(&kif1.kf_un.kf_kqueue,
> &kif2.kf_un.kf_kqueue,
>                     sizeof(kif1.kf_un.kf_kqueue)))
>                         return (0);
>                 return (3);
>         //case KF_TYPE_MQUEUE:
>         //case KF_TYPE_SHM:
>         case KF_TYPE_SEM:
>                 if (0 == memcmp(&kif1.kf_un.kf_sem, &kif2.kf_un.kf_sem,
>                     sizeof(kif1.kf_un.kf_sem)))
>                         return (0);
>                 return (3);
>         case KF_TYPE_PTS:
>                 if (0 == memcmp(&kif1.kf_un.kf_pts, &kif2.kf_un.kf_pts,
>                     sizeof(kif1.kf_un.kf_pts)))
>                         return (0);
>                 return (3);
>         case KF_TYPE_PROCDESC:
>                 if (0 == memcmp(&kif1.kf_un.kf_proc, &kif2.kf_un.kf_proc,
>                     sizeof(kif1.kf_un.kf_proc)))
>                         return (0);
>                 return (3);
>         //case KF_TYPE_DEV:
>         case KF_TYPE_EVENTFD:
>                 if (0 == memcmp(&kif1.kf_un.kf_eventfd,
> &kif2.kf_un.kf_eventfd,
>                     sizeof(kif1.kf_un.kf_eventfd)))
>                         return (0);
>                 return (3);
>         case KF_TYPE_TIMERFD:
>                 if (0 == memcmp(&kif1.kf_un.kf_timerfd,
> &kif2.kf_un.kf_timerfd,
>                     sizeof(kif1.kf_un.kf_timerfd)))
>                         return (0);
>                 return (3);
>         }
>         /* Otherwise we can't tell */
>         return (-1);
> }
>
>
> Refs:
> 1. https://man7.org/linux/man-pages/man2/kcmp.2.html
>
>
>