| Message ID | 20220309165222.2843651-7-tjmercier@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Proposal for a GPU cgroup controller | expand |
On Wed, Mar 9, 2022 at 8:52 AM T.J. Mercier <tjmercier@google.com> wrote: > > From: Hridya Valsaraju <hridya@google.com> > > This patch introduces a buffer flag BINDER_BUFFER_FLAG_SENDER_NO_NEED > that a process sending an fd array to another process over binder IPC > can set to relinquish ownership of the fds being sent for memory > accounting purposes. If the flag is found to be set during the fd array > translation and the fd is for a DMA-BUF, the buffer is uncharged from > the sender's cgroup and charged to the receiving process's cgroup > instead. > > It is up to the sending process to ensure that it closes the fds > regardless of whether the transfer failed or succeeded. > > Most graphics shared memory allocations in Android are done by the > graphics allocator HAL process. On requests from clients, the HAL process > allocates memory and sends the fds to the clients over binder IPC. > The graphics allocator HAL will not retain any references to the > buffers. When the HAL sets the BINDER_BUFFER_FLAG_SENDER_NO_NEED for fd > arrays holding DMA-BUF fds, the gpu cgroup controller will be able to > correctly charge the buffers to the client processes instead of the > graphics allocator HAL. > > Since this is a new feature exposed to userspace, the kernel and userspace > must be compatible for the accounting to work for transfers. In all cases > the allocation and transport of DMA buffers via binder will succeed, but > only when both the kernel supports, and userspace depends on this feature > will the transfer accounting work. The possible scenarios are detailed > below: > > 1. new kernel + old userspace > The kernel supports the feature but userspace does not use it. The old > userspace won't mount the new cgroup controller, accounting is not > performed, charge is not transferred. > > 2. old kernel + new userspace > The new cgroup controller is not supported by the kernel, accounting is > not performed, charge is not transferred. > > 3. old kernel + old userspace > Same as #2 > > 4. new kernel + new userspace > Cgroup is mounted, feature is supported and used. > > Signed-off-by: Hridya Valsaraju <hridya@google.com> > Signed-off-by: T.J. Mercier <tjmercier@google.com> Acked-by: Todd Kjos <tkjos@google.com> > > --- > v3 changes > Remove android from title per Todd Kjos. > > Use more common dual author commit message format per John Stultz. > > Include details on behavior for all combinations of kernel/userspace > versions in changelog (thanks Suren Baghdasaryan) per Greg Kroah-Hartman. > > v2 changes > Move dma-buf cgroup charge transfer from a dma_buf_op defined by every > heap to a single dma-buf function for all heaps per Daniel Vetter and > Christian König. > --- > drivers/android/binder.c | 26 ++++++++++++++++++++++++++ > include/uapi/linux/android/binder.h | 1 + > 2 files changed, 27 insertions(+) > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c > index 8351c5638880..f50d88ded188 100644 > --- a/drivers/android/binder.c > +++ b/drivers/android/binder.c > @@ -42,6 +42,7 @@ > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > +#include <linux/dma-buf.h> > #include <linux/fdtable.h> > #include <linux/file.h> > #include <linux/freezer.h> > @@ -2482,8 +2483,10 @@ static int binder_translate_fd_array(struct list_head *pf_head, > { > binder_size_t fdi, fd_buf_size; > binder_size_t fda_offset; > + bool transfer_gpu_charge = false; > const void __user *sender_ufda_base; > struct binder_proc *proc = thread->proc; > + struct binder_proc *target_proc = t->to_proc; > int ret; > > fd_buf_size = sizeof(u32) * fda->num_fds; > @@ -2521,8 +2524,15 @@ static int binder_translate_fd_array(struct list_head *pf_head, > if (ret) > return ret; > > + if (IS_ENABLED(CONFIG_CGROUP_GPU) && > + parent->flags & BINDER_BUFFER_FLAG_SENDER_NO_NEED) > + transfer_gpu_charge = true; > + > for (fdi = 0; fdi < fda->num_fds; fdi++) { > u32 fd; > + struct dma_buf *dmabuf; > + struct gpucg *gpucg; > + > binder_size_t offset = fda_offset + fdi * sizeof(fd); > binder_size_t sender_uoffset = fdi * sizeof(fd); > > @@ -2532,6 +2542,22 @@ static int binder_translate_fd_array(struct list_head *pf_head, > in_reply_to); > if (ret) > return ret > 0 ? -EINVAL : ret; > + > + if (!transfer_gpu_charge) > + continue; > + > + dmabuf = dma_buf_get(fd); > + if (IS_ERR(dmabuf)) > + continue; > + > + gpucg = gpucg_get(target_proc->tsk); > + ret = dma_buf_charge_transfer(dmabuf, gpucg); > + if (ret) { > + pr_warn("%d:%d Unable to transfer DMA-BUF fd charge to %d", > + proc->pid, thread->pid, target_proc->pid); > + gpucg_put(gpucg); > + } > + dma_buf_put(dmabuf); > } > return 0; > } > diff --git a/include/uapi/linux/android/binder.h b/include/uapi/linux/android/binder.h > index 3246f2c74696..169fd5069a1a 100644 > --- a/include/uapi/linux/android/binder.h > +++ b/include/uapi/linux/android/binder.h > @@ -137,6 +137,7 @@ struct binder_buffer_object { > > enum { > BINDER_BUFFER_FLAG_HAS_PARENT = 0x01, > + BINDER_BUFFER_FLAG_SENDER_NO_NEED = 0x02, > }; > > /* struct binder_fd_array_object - object describing an array of fds in a buffer > -- > 2.35.1.616.g0bdcbb4464-goog >
diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 8351c5638880..f50d88ded188 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -42,6 +42,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include <linux/dma-buf.h> #include <linux/fdtable.h> #include <linux/file.h> #include <linux/freezer.h> @@ -2482,8 +2483,10 @@ static int binder_translate_fd_array(struct list_head *pf_head, { binder_size_t fdi, fd_buf_size; binder_size_t fda_offset; + bool transfer_gpu_charge = false; const void __user *sender_ufda_base; struct binder_proc *proc = thread->proc; + struct binder_proc *target_proc = t->to_proc; int ret; fd_buf_size = sizeof(u32) * fda->num_fds; @@ -2521,8 +2524,15 @@ static int binder_translate_fd_array(struct list_head *pf_head, if (ret) return ret; + if (IS_ENABLED(CONFIG_CGROUP_GPU) && + parent->flags & BINDER_BUFFER_FLAG_SENDER_NO_NEED) + transfer_gpu_charge = true; + for (fdi = 0; fdi < fda->num_fds; fdi++) { u32 fd; + struct dma_buf *dmabuf; + struct gpucg *gpucg; + binder_size_t offset = fda_offset + fdi * sizeof(fd); binder_size_t sender_uoffset = fdi * sizeof(fd); @@ -2532,6 +2542,22 @@ static int binder_translate_fd_array(struct list_head *pf_head, in_reply_to); if (ret) return ret > 0 ? -EINVAL : ret; + + if (!transfer_gpu_charge) + continue; + + dmabuf = dma_buf_get(fd); + if (IS_ERR(dmabuf)) + continue; + + gpucg = gpucg_get(target_proc->tsk); + ret = dma_buf_charge_transfer(dmabuf, gpucg); + if (ret) { + pr_warn("%d:%d Unable to transfer DMA-BUF fd charge to %d", + proc->pid, thread->pid, target_proc->pid); + gpucg_put(gpucg); + } + dma_buf_put(dmabuf); } return 0; } diff --git a/include/uapi/linux/android/binder.h b/include/uapi/linux/android/binder.h index 3246f2c74696..169fd5069a1a 100644 --- a/include/uapi/linux/android/binder.h +++ b/include/uapi/linux/android/binder.h @@ -137,6 +137,7 @@ struct binder_buffer_object { enum { BINDER_BUFFER_FLAG_HAS_PARENT = 0x01, + BINDER_BUFFER_FLAG_SENDER_NO_NEED = 0x02, }; /* struct binder_fd_array_object - object describing an array of fds in a buffer