Date: Fri, 19 Jan 2024 21:34:31 +0300 From: Michael Zhilin <mizhka@gmail.com> To: Rozhuk Ivan <rozhuk.im@gmail.com> Cc: FreeBSD Hackers <freebsd-hackers@freebsd.org>, FreeBSD Mailing List <freebsd-ports@freebsd.org> Subject: Re: kcmp implementation for mesa Message-ID: <CAF19XBJ-5YMoayGoQv_zVxr%2BJKFRkXec9d5Az1oM6iCrpykbkQ@mail.gmail.com> In-Reply-To: <20240119131047.6975f574@rimwks.local> References: <20240119131047.6975f574@rimwks.local>
next in thread | previous in thread | raw e-mail | index | archive | help
--000000000000e07473060f50bb6c
Content-Type: text/plain; charset="UTF-8"
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
>
>
>
--000000000000e07473060f50bb6c
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
<div dir=3D"auto">Hi Ivan,=C2=A0<div dir=3D"auto"><br></div><div dir=3D"aut=
o">Looks good. Could you please put patch on <a href=3D"http://reviews.free=
bsd.org">reviews.freebsd.org</a>?=C2=A0</div><div dir=3D"auto"><br></div><d=
iv dir=3D"auto">Thx,=C2=A0</div><div dir=3D"auto">Michael</div></div><br><d=
iv class=3D"gmail_quote"><div dir=3D"ltr" class=3D"gmail_attr">On Fri, 19 J=
an 2024, 14:11 Rozhuk Ivan, <<a href=3D"mailto:rozhuk.im@gmail.com">rozh=
uk.im@gmail.com</a>> wrote:<br></div><blockquote class=3D"gmail_quote" s=
tyle=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi!<=
br>
<br>
<br>
graphics/mesa-* uses SYS_kcmp [1] to compare two fds:<br>
<br>
int<br>
os_same_file_description(int fd1, int fd2)<br>
{<br>
=C2=A0 =C2=A0pid_t pid =3D getpid();<br>
<br>
=C2=A0 =C2=A0/* Same file descriptor trivially implies same file descriptio=
n */<br>
=C2=A0 =C2=A0if (fd1 =3D=3D fd2)<br>
=C2=A0 =C2=A0 =C2=A0 return 0;<br>
<br>
=C2=A0 =C2=A0return syscall(SYS_kcmp, pid, pid, KCMP_FILE, fd1, fd2);<br>
}<br>
<br>
FreeBSD does not implemet this and we got in terminal:<br>
"amdgpu: os_same_file_description couldn't determine if two DRM fd=
s reference the same file description."<br>
<br>
<br>
Mesa say:<br>
/* DRM file descriptors, file descriptions and buffer sharing.<br>
=C2=A0*<br>
=C2=A0* amdgpu_device_initialize first argument is a file descriptor (fd)<b=
r>
=C2=A0* representing a specific GPU.<br>
=C2=A0* If a fd is duplicated using os_dupfd_cloexec,<br>
=C2=A0* the file description will remain the same (os_same_file_description=
will<br>
=C2=A0* return 0).<br>
=C2=A0* But if the same device is re-opened, the fd and the file descriptio=
n will<br>
=C2=A0* be different.<br>
=C2=A0*<br>
=C2=A0* amdgpu_screen_winsys's fd tracks the file description which was=
<br>
=C2=A0* given to amdgpu_winsys_create. This is the fd used by the applicati=
on<br>
=C2=A0* using the driver and may be used in other ioctl (eg: drmModeAddFB)<=
br>
=C2=A0*<br>
=C2=A0* amdgpu_winsys's fd is the file description used to initialize t=
he<br>
=C2=A0* device handle in libdrm_amdgpu.<br>
=C2=A0*<br>
=C2=A0* The 2 fds can be different, even in systems with a single GPU, eg: =
if<br>
=C2=A0* radv is initialized before radeonsi.<br>
=C2=A0*<br>
=C2=A0* This fd tracking is useful for buffer sharing because KMS/GEM handl=
es are<br>
=C2=A0* specific to a DRM file description, i.e. the same handle value may =
refer<br>
=C2=A0* to different underlying BOs in different DRM file descriptions.<br>
=C2=A0* As an example, if an app wants to use drmModeAddFB it'll need a=
KMS handle<br>
=C2=A0* valid for its fd (=3D=3D amdgpu_screen_winsys::fd).<br>
=C2=A0* If both fds are identical, there's nothing to do: bo->u.real=
.kms_handle<br>
=C2=A0* can be used directly (see amdgpu_bo_get_handle).<br>
=C2=A0* If they're different, the BO has to be exported from the device=
fd as<br>
=C2=A0* a dma-buf, then imported from the app fd as a KMS handle.<br>
=C2=A0*/<br>
<br>
<br>
<br>
I do few checks with dup() and os_dupfd_cloexec() and code show that fd equ=
al.<br>
<br>
Does this implementation will do that mesa expects?<br>
<br>
<br>
#include <sys/user.h><br>
#include <fcntl.h><br>
<br>
int<br>
os_same_file_description(int fd1, int fd2) {<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 struct kinfo_file kif1, kif2;<br>
<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Same file descriptor trivially implies same =
file description */<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (fd1 =3D=3D fd2)<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return (0);<br>
<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 kif1.kf_structsize =3D sizeof(kif1);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 kif2.kf_structsize =3D sizeof(kif2);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (-1 =3D=3D fcntl(fd1, F_KINFO, &kif1) ||=
<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 -1 =3D=3D fcntl(fd2, F_KINFO, &am=
p;kif2))<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return (-1);<br>
<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (kif1.kf_type !=3D kif2.kf_type ||<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 0 !=3D memcmp(&kif1.kf_path, =
&kif2.kf_path, sizeof(kif1.kf_path)))<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return (3);<br>
<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 switch (kif1.kf_type) {<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 case KF_TYPE_VNODE:<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (0 =3D=3D memcmp=
(&kif1.kf_un.kf_file, &kif2.kf_un.kf_file,<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sizeo=
f(kif1.kf_un.kf_file)))<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 return (0);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return (3);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 case KF_TYPE_SOCKET:<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (0 =3D=3D memcmp=
(&kif1.kf_un.kf_sock, &kif2.kf_un.kf_sock,<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sizeo=
f(kif1.kf_un.kf_sock)))<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 return (0);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return (3);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 case KF_TYPE_PIPE:<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (0 =3D=3D memcmp=
(&kif1.kf_un.kf_pipe, &kif2.kf_un.kf_pipe,<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sizeo=
f(kif1.kf_un.kf_pipe)))<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 return (0);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return (3);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 //case KF_TYPE_FIFO:<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 case KF_TYPE_KQUEUE:<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (0 =3D=3D memcmp=
(&kif1.kf_un.kf_kqueue, &kif2.kf_un.kf_kqueue,<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sizeo=
f(kif1.kf_un.kf_kqueue)))<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 return (0);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return (3);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 //case KF_TYPE_MQUEUE:<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 //case KF_TYPE_SHM:<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 case KF_TYPE_SEM:<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (0 =3D=3D memcmp=
(&kif1.kf_un.kf_sem, &kif2.kf_un.kf_sem,<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sizeo=
f(kif1.kf_un.kf_sem)))<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 return (0);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return (3);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 case KF_TYPE_PTS:<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (0 =3D=3D memcmp=
(&kif1.kf_un.kf_pts, &kif2.kf_un.kf_pts,<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sizeo=
f(kif1.kf_un.kf_pts)))<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 return (0);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return (3);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 case KF_TYPE_PROCDESC:<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (0 =3D=3D memcmp=
(&kif1.kf_un.kf_proc, &kif2.kf_un.kf_proc,<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sizeo=
f(kif1.kf_un.kf_proc)))<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 return (0);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return (3);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 //case KF_TYPE_DEV:<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 case KF_TYPE_EVENTFD:<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (0 =3D=3D memcmp=
(&kif1.kf_un.kf_eventfd, &kif2.kf_un.kf_eventfd,<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sizeo=
f(kif1.kf_un.kf_eventfd)))<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 return (0);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return (3);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 case KF_TYPE_TIMERFD:<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (0 =3D=3D memcmp=
(&kif1.kf_un.kf_timerfd, &kif2.kf_un.kf_timerfd,<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sizeo=
f(kif1.kf_un.kf_timerfd)))<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 return (0);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return (3);<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 }<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Otherwise we can't tell */<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0 return (-1);<br>
}<br>
<br>
<br>
Refs:<br>
1. <a href=3D"https://man7.org/linux/man-pages/man2/kcmp.2.html" rel=3D"nor=
eferrer noreferrer" target=3D"_blank">https://man7.org/linux/man-pages/man2=
/kcmp.2.html</a><br>
<br>
<br>
</blockquote></div>
--000000000000e07473060f50bb6c--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAF19XBJ-5YMoayGoQv_zVxr%2BJKFRkXec9d5Az1oM6iCrpykbkQ>