Page MenuHomeFreeBSD

Add DTYPE/KF_TYPE and procstat support for dma-buf
Needs ReviewPublic

Authored by val_packett.cool on Oct 2 2020, 11:23 AM.
Tags
None
Referenced Files
Unknown Object (File)
Nov 13 2024, 2:24 PM
Unknown Object (File)
Oct 3 2024, 9:23 AM
Unknown Object (File)
Oct 1 2024, 11:28 AM
Unknown Object (File)
Sep 27 2024, 10:13 PM
Unknown Object (File)
Sep 24 2024, 9:30 AM
Unknown Object (File)
Sep 18 2024, 10:42 AM
Unknown Object (File)
Sep 16 2024, 12:01 AM
Unknown Object (File)
Sep 8 2024, 5:29 PM
Subscribers

Details

Reviewers
kib
manu
Group Reviewers
Contributor Reviews (src)
linuxkpi
Summary

The dmabuf implementation in drm-kmod currently uses an ad-hoc DTYPE of 100 for dma-bufs with an XXX comment (and does nothing in dma_buf_fill_kinfo):
https://github.com/freebsd/drm-kmod/blob/2b7442a5bc2a7209908a6697ac60bb81ad10b9a0/drivers/dma-buf/dma-buf.c#L88-L89

Defining a proper DTYPE_DMABUF and KF_TYPE_DMABUF would let procstat -f show us which file descriptors in a process are dma-bufs, which can be quite useful for debugging (e.g. recently I needed to see what kind of fd was failing to pass in Firefox).

Current drm side patch: https://reviews.freebsd.org/P431

TODO: edit procstat manpage, maybe display the size and device from kinfo in procstat

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

I think this should be handled differently.
Do we ever have a plan to merge DMABUF into base ? What are the plans there ?

If not, we should reserve some number of DTYPE_/KF_TYPE_ values for use in third-party code. May be, even go as far as to allow to request them at runtime. procstat(1)/kinfo_file should unify handling of them, e.g. displaying some opaque string filled by fo_fill_kinfo().

In D26642#593456, @kib wrote:

I think this should be handled differently.
Do we ever have a plan to merge DMABUF into base ? What are the plans there ?

Yes I do plan to commit something based on https://github.com/evadot/freebsd/blob/drm_base_v5.6-20200718/sys/dev/drm/drmkpi/drmkpi_dma_buf.c
This is NetBSD code that @mmel ported and that I've updated to match Linux 5.6
It is not complete yet and hasn't been tested with i915 or amdgpu though.
I don't have any timeframe for when this will happen in head.

If not, we should reserve some number of DTYPE_/KF_TYPE_ values for use in third-party code. May be, even go as far as to allow to request them at runtime. procstat(1)/kinfo_file should unify handling of them, e.g. displaying some opaque string filled by fo_fill_kinfo().

In D26642#593458, @manu wrote:
In D26642#593456, @kib wrote:

I think this should be handled differently.
Do we ever have a plan to merge DMABUF into base ? What are the plans there ?

Yes I do plan to commit something based on https://github.com/evadot/freebsd/blob/drm_base_v5.6-20200718/sys/dev/drm/drmkpi/drmkpi_dma_buf.c
This is NetBSD code that @mmel ported and that I've updated to match Linux 5.6
It is not complete yet and hasn't been tested with i915 or amdgpu though.
I don't have any timeframe for when this will happen in head.

Ok, let me reformulate my first question. Would the proposed diff useful for your WIP, or should instead we wait until the WIP is finished, and then allocate KTYPE and decide what to do with procstat ?

If not, we should reserve some number of DTYPE_/KF_TYPE_ values for use in third-party code. May be, even go as far as to allow to request them at runtime. procstat(1)/kinfo_file should unify handling of them, e.g. displaying some opaque string filled by fo_fill_kinfo().

In D26642#593460, @kib wrote:
In D26642#593458, @manu wrote:
In D26642#593456, @kib wrote:

I think this should be handled differently.
Do we ever have a plan to merge DMABUF into base ? What are the plans there ?

Yes I do plan to commit something based on https://github.com/evadot/freebsd/blob/drm_base_v5.6-20200718/sys/dev/drm/drmkpi/drmkpi_dma_buf.c
This is NetBSD code that @mmel ported and that I've updated to match Linux 5.6
It is not complete yet and hasn't been tested with i915 or amdgpu though.
I don't have any timeframe for when this will happen in head.

Ok, let me reformulate my first question. Would the proposed diff useful for your WIP, or should instead we wait until the WIP is finished, and then allocate KTYPE and decide what to do with procstat ?

I will be useful for both my WIP and the current external code.
So we could start by doing something for external DTYPE and then switch to an internal one but I'm not sure it's worth doing that.

If not, we should reserve some number of DTYPE_/KF_TYPE_ values for use in third-party code. May be, even go as far as to allow to request them at runtime. procstat(1)/kinfo_file should unify handling of them, e.g. displaying some opaque string filled by fo_fill_kinfo().

In D26642#593461, @manu wrote:

I will be useful for both my WIP and the current external code.
So we could start by doing something for external DTYPE and then switch to an internal one but I'm not sure it's worth doing that.

If it will be useful for WIP I do not see a reason to do the external trip.
Please check that this diff is fine for your purposes. I can handle it if you want.

In D26642#593463, @kib wrote:
In D26642#593461, @manu wrote:

I will be useful for both my WIP and the current external code.
So we could start by doing something for external DTYPE and then switch to an internal one but I'm not sure it's worth doing that.

If it will be useful for WIP I do not see a reason to do the external trip.
Please check that this diff is fine for your purposes. I can handle it if you want.

I've quickly looked when Greg told me he had that but I'll do a proper review this weekend.
Thanks.

sys/sys/user.h
443

Why these spares are needed ?

sys/sys/user.h
443

They are there on everything else, I just copied that.

sys/sys/user.h
443

For everything else there were ABI compat issues that were solved by making the gap.

  • Rebase on top of landed eventfd :)
  • Remove padding fields