From: Daniel Vetter <daniel@ffwll.ch>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
Dave Airlie <airlied@gmail.com>
Cc: Song Liu <songliubraving@fb.com>,
DRI Development <dri-devel@lists.freedesktop.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Kenny Ho <Kenny.Ho@amd.com>,
"open list:CONTROL GROUP \(CGROUP\)" <cgroups@vger.kernel.org>,
Brian Welty <brian.welty@intel.com>,
John Fastabend <john.fastabend@gmail.com>,
Alexei Starovoitov <ast@kernel.org>,
amd-gfx list <amd-gfx@lists.freedesktop.org>,
Linux-Fsdevel <linux-fsdevel@vger.kernel.org>,
Kenny Ho <y2kenny@gmail.com>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Network Development <netdev@vger.kernel.org>,
KP Singh <kpsingh@chromium.org>, Yonghong Song <yhs@fb.com>,
bpf <bpf@vger.kernel.org>, Andrii Nakryiko <andriin@fb.com>,
Martin KaFai Lau <kafai@fb.com>,
Alex Deucher <alexander.deucher@amd.com>
Subject: Re: [RFC] Add BPF_PROG_TYPE_CGROUP_IOCTL
Date: Mon, 1 Feb 2021 15:49:25 +0100 [thread overview]
Message-ID: <YBgU9Vu0BGV8kCxD@phenom.ffwll.local> (raw)
In-Reply-To: <20201103232805.6uq4zg3gdvw2iiki@ast-mbp.dhcp.thefacebook.com>
Adding gpu folks.
On Tue, Nov 03, 2020 at 03:28:05PM -0800, Alexei Starovoitov wrote:
> On Tue, Nov 03, 2020 at 05:57:47PM -0500, Kenny Ho wrote:
> > On Tue, Nov 3, 2020 at 4:04 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Tue, Nov 03, 2020 at 02:19:22PM -0500, Kenny Ho wrote:
> > > > On Tue, Nov 3, 2020 at 12:43 AM Alexei Starovoitov
> > > > <alexei.starovoitov@gmail.com> wrote:
> > > > > On Mon, Nov 2, 2020 at 9:39 PM Kenny Ho <y2kenny@gmail.com> wrote:
> > >
> > > Sounds like either bpf_lsm needs to be made aware of cgv2 (which would
> > > be a great thing to have regardless) or cgroup-bpf needs a drm/gpu specific hook.
> > > I think generic ioctl hook is too broad for this use case.
> > > I suspect drm/gpu internal state would be easier to access inside
> > > bpf program if the hook is next to gpu/drm. At ioctl level there is 'file'.
> > > It's probably too abstract for the things you want to do.
> > > Like how VRAM/shader/etc can be accessed through file?
> > > Probably possible through a bunch of lookups and dereferences, but
> > > if the hook is custom to GPU that info is likely readily available.
> > > Then such cgroup-bpf check would be suitable in execution paths where
> > > ioctl-based hook would be too slow.
> > Just to clarify, when you say drm specific hook, did you mean just a
> > unique attach_type or a unique prog_type+attach_type combination? (I
> > am still a bit fuzzy on when a new prog type is needed vs a new attach
> > type. I think prog type is associated with a unique type of context
> > that the bpf prog will get but I could be missing some nuances.)
> >
> > When I was thinking of doing an ioctl wide hook, the file would be the
> > device file and the thinking was to have a helper function provided by
> > device drivers to further disambiguate. For our (AMD's) driver, we
> > have a bunch of ioctls for set/get/create/destroy
> > (https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c#L1763)
> > so the bpf prog can make the decision after the disambiguation. For
> > example, we have an ioctl called "kfd_ioctl_set_cu_mask." You can
>
> Thanks for the pointer.
> That's one monster ioctl. So much copy_from_user.
> BPF prog would need to be sleepable to able to examine the args in such depth.
> After quick glance at the code I would put a new hook into
> kfd_ioctl() right before
> retcode = func(filep, process, kdata);
> At this point kdata is already copied from user space
> and usize, that is cmd specific, is known.
> So bpf prog wouldn't need to copy that data again.
> That will save one copy.
> To drill into details of kfd_ioctl_set_cu_mask() the prog would
> need to be sleepable to do second copy_from_user of cu_mask.
> At least it's not that big.
> Yes, the attachment point will be amd driver specific,
> but the program doesn't need to be.
> It can be generic tracing prog that is agumented to use BTF.
> Something like writeable tracepoint with BTF support would do.
> So on the bpf side there will be minimal amount of changes.
> And in the driver you'll add one or few writeable tracepoints
> and the result of the tracepoint will gate
> retcode = func(filep, process, kdata);
> call in kfd_ioctl().
> The writeable tracepoint would need to be cgroup-bpf based.
> So that's the only tricky part. BPF infra doesn't have
> cgroup+tracepoint scheme. It's probably going to be useful
> in other cases like this. See trace_nbd_send_request.
Yeah I think this proposal doesn't work:
- inspecting ioctl arguments that need copying outside of the
driver/subsystem doing that copying is fundamentally racy
- there's been a pile of cgroups proposal to manage gpus at the drm
subsystem level, some by Kenny, and frankly this at least looks a bit
like a quick hack to sidestep the consensus process for that.
So once we push this into drivers it's not going to be a bpf hook anymore
I think.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next parent reply other threads:[~2021-02-01 14:49 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20201007152355.2446741-1-Kenny.Ho@amd.com>
[not found] ` <CAOWid-d=a1Q3R92s7GrzxWhXx7_dc8NQvQg7i7RYTVv3+jHxkQ@mail.gmail.com>
[not found] ` <20201103053244.khibmr66p7lhv7ge@ast-mbp.dhcp.thefacebook.com>
[not found] ` <CAOWid-eQSPru0nm8+Xo3r6C0pJGq+5r8mzM8BL2dgNn2c9mt2Q@mail.gmail.com>
[not found] ` <CAADnVQKuoZDB-Xga5STHdGSxvSP=B6jQ40kLdpL1u+J98bv65A@mail.gmail.com>
[not found] ` <CAOWid-czZphRz6Y-H3OcObKCH=bLLC3=bOZaSB-6YBE56+Qzrg@mail.gmail.com>
[not found] ` <20201103210418.q7hddyl7rvdplike@ast-mbp.dhcp.thefacebook.com>
[not found] ` <CAOWid-djQ_NRfCbOTnZQ-A8Pr7jMP7KuZEJDSsvzWkdw7qc=yA@mail.gmail.com>
[not found] ` <20201103232805.6uq4zg3gdvw2iiki@ast-mbp.dhcp.thefacebook.com>
2021-02-01 14:49 ` Daniel Vetter [this message]
2021-02-01 16:47 ` [RFC] Add BPF_PROG_TYPE_CGROUP_IOCTL Kenny Ho
2021-02-01 16:51 ` Kenny Ho
2021-02-03 11:09 ` Daniel Vetter
2021-02-03 19:01 ` Kenny Ho
2021-02-05 13:49 ` Daniel Vetter
2021-05-07 2:06 ` Kenny Ho
2021-05-07 8:59 ` Daniel Vetter
2021-05-07 15:33 ` Kenny Ho
2021-05-07 16:13 ` Daniel Vetter
2021-05-07 16:19 ` Alex Deucher
2021-05-07 16:26 ` Daniel Vetter
2021-05-07 16:31 ` Alex Deucher
2021-05-07 16:50 ` Alex Deucher
2021-05-07 16:54 ` Daniel Vetter
2021-05-07 17:04 ` Kenny Ho
2021-05-07 19:33 ` Tejun Heo
2021-05-07 19:55 ` Alex Deucher
2021-05-07 20:59 ` Tejun Heo
2021-05-07 22:30 ` Alex Deucher
2021-05-07 23:45 ` Tejun Heo
2021-05-11 15:48 ` Alex Deucher
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YBgU9Vu0BGV8kCxD@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=Kenny.Ho@amd.com \
--cc=airlied@gmail.com \
--cc=alexander.deucher@amd.com \
--cc=alexei.starovoitov@gmail.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=andriin@fb.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=brian.welty@intel.com \
--cc=cgroups@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=dri-devel@lists.freedesktop.org \
--cc=john.fastabend@gmail.com \
--cc=kafai@fb.com \
--cc=kpsingh@chromium.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=songliubraving@fb.com \
--cc=viro@zeniv.linux.org.uk \
--cc=y2kenny@gmail.com \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).