On Thu, Aug 27, 2020 at 09:31:27AM -0400, Laura Abbott wrote:
> On 8/27/20 8:36 AM, Greg Kroah-Hartman wrote:
> > The ION android code has long been marked to be removed, now that we
> > dma-buf support merged into the real part of the kernel.
> >
> > It was thought that we could wait to remove the ion kernel at a later
> > time, but as the out-of-tree Android fork of the ion code has diverged
> > quite a bit, and any Android device using the ion interface uses that
> > forked version and not this in-tree version, the in-tree copy of the
> > code is abandonded and not used by anyone.
> >
> > Combine this abandoned codebase with the need to make changes to it in
> > order to keep the kernel building properly, which then causes merge
> > issues when merging those changes into the out-of-tree Android code, and
> > you end up with two different groups of people (the in-kernel-tree
> > developers, and the Android kernel developers) who are both annoyed at
> > the current situation. Because of this problem, just drop the in-kernel
> > copy of the ion code now, as it's not used, and is only causing problems
> > for everyone involved.
> >
> > Cc: "Arve Hjønnevåg" <arve(a)android.com>
> > Cc: "Christian König" <christian.koenig(a)amd.com>
> > Cc: Christian Brauner <christian(a)brauner.io>
> > Cc: Christoph Hellwig <hch(a)infradead.org>
> > Cc: Hridya Valsaraju <hridya(a)google.com>
> > Cc: Joel Fernandes <joel(a)joelfernandes.org>
> > Cc: John Stultz <john.stultz(a)linaro.org>
> > Cc: Laura Abbott <laura(a)labbott.name>
> > Cc: Martijn Coenen <maco(a)android.com>
> > Cc: Shuah Khan <shuah(a)kernel.org>
> > Cc: Sumit Semwal <sumit.semwal(a)linaro.org>
> > Cc: Suren Baghdasaryan <surenb(a)google.com>
> > Cc: Todd Kjos <tkjos(a)android.com>
> > Cc: devel(a)driverdev.osuosl.org
> > Cc: dri-devel(a)lists.freedesktop.org
> > Cc: linaro-mm-sig(a)lists.linaro.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
>
> We discussed this at the Android MC on Monday and the plan was to
> remove it after the next LTS release.
I know it was discussed, my point is that it is actually causing
problems now (with developers who want to change the internal kernel api
hitting issues, and newbies trying to clean up code in ways that isn't
exactly optimal wasting maintainer cycles), and that anyone who uses
this code, is not actually using this version of the code. Everyone who
relies on ion right now, is using the version that is in the Android
common kernel tree, which has diverged from this in-kernel way quite a
bit now for the reason that we didn't want to take any of those new
features in the in-kernel version.
So this is a problem that we have caused by just wanting to wait, no one
is using this code, combined with it causing problems for the upstream
developers.
There is nothing "magic" about the last kernel of the year that requires
this code to sit here until then. At that point in time, all users
will, again, be using the forked Android kernel version, and if we
delete this now here, that fork can remain just fine, with the added
benifit of it reducing developer workloads here in-kernel.
So why wait?
thanks,
greg k-h
On Fri, Aug 21, 2020 at 2:14 AM Kunihiko Hayashi
<hayashi.kunihiko(a)socionext.com> wrote:
>
> On 2020/08/01 4:38, John Stultz wrote:
> > On Fri, Jul 31, 2020 at 2:32 AM Kunihiko Hayashi
> > <hayashi.kunihiko(a)socionext.com> wrote:
> >> On 2020/07/29 4:17, John Stultz wrote:
> >>> Do you have a upstream driver that you plan to make use this new call?
> >>
> >> Unfortunately I don't have an upstream driver using this call.
> >>
> >> This call is called from dma-buf heaps "importer" or "customer",
> >> and I only made an example (do nothing) importer driver
> >> to test the call.
> >>
> >>> We want to have in-tree users of code added.
> >>
> >> I think this is a generic way to use non-default CMA heaps, however,
> >> we need in-tree "importer" drivers to want to use non-default CMA heaps.
> >> I don't find it from now.
> >>
> >
> > Yea, I and again, I do agree this is functionality that will be
> > needed. But we'll need to wait for a user (camera driver, etc which
> > would utilize the reserved cma region) before we can merge it
> > upstream. :( Do let me know if you have an out of tree driver that
> > would make use of it, and we can see what can be done to help upstream
> > things.
>
> Sorry for late.
> Before I prepare or find a user driver as "importer",
> I think something is different in this patch.
>
> This patch makes it possible to treat non-default CMA connected to
> "importer" device with memory-region as dma-buf heaps.
>
> However, the allocated memory from this dma-buf heaps can be used
> for "any" devices, and the "importer" can treat memories from other
> dma-buf heaps.
>
> So, the "importer" and the non-default CMA aren't directly related,
> and I think an "exporter" for the non-default CMA should be enabled.
>
> In paticular, the kernel initializer (as an "exporter") calls
> dma_heap_add_cma() for all CMAs defined in Devicetree, and
> the device files associated with each CMA appear under "/dev/dma_heap/".
> For example:
>
> /dev/dma_heap/linux,cma@10000000
> /dev/dma_heap/linux,cma@11000000
> /dev/dma_heap/linux,cma@12000000
> ...
>
> All of these device files can be fairly allocated to any "importer" device.
>
> Actually I think that the kernel should executes dma_heap_add_cma()
> for ALL defined reserved-memory nodes.
>
> If this idea hasn't been discussed yet and this is reasonable,
> I'll prepare RFC patches.
So yes! An earlier version of the CMA heap I submitted did add all CMA
regions as accessible heaps as you propose here.
However, the concern was that in some cases, those regions are device
specific reserved memory that the driver is probably expecting to have
exclusive access. To allow (sufficiently privileged, or misconfigured)
userland to be able to allocate out of that seemed like it might cause
trouble, and instead we should have CMA regions explicitly exported.
There was some proposal to add a dt property to the reserved memory
section (similar to linux,cma-default) and use that to do the
exporting, but other discussions seemed to prefer having drivers
export it explicitly in a fashion very similar to what your earlier
patch does. The only trouble is we just need an upstream driver to add
such a call in the series before we can merge it.
thanks
-john
These patch series to introduce a new dma heap, chunk heap.
That heap is needed for special HW that requires bulk allocation of
fixed high order pages. For example, 64MB dma-buf pages are made up
to fixed order-4 pages * 1024.
The chunk heap uses alloc_pages_bulk to allocate high order page.
https://lore.kernel.org/linux-mm/20200814173131.2803002-1-minchan@kernel.org
The chunk heap is registered by device tree with alignment and memory node
of contiguous memory allocator(CMA). Alignment defines chunk page size.
For example, alignment 0x1_0000 means chunk page size is 64KB.
The phandle to memory node indicates contiguous memory allocator(CMA).
If device node doesn't have cma, the registration of chunk heap fails.
The patchset includes the following:
- export dma-heap API to register kernel module dma heap.
- add chunk heap implementation.
- document of device tree to register chunk heap
Hyesoo Yu (3):
dma-buf: add missing EXPORT_SYMBOL_GPL() for dma heaps
dma-buf: heaps: add chunk heap to dmabuf heaps
dma-heap: Devicetree binding for chunk heap
.../devicetree/bindings/dma-buf/chunk_heap.yaml | 46 +++++
drivers/dma-buf/dma-heap.c | 2 +
drivers/dma-buf/heaps/Kconfig | 9 +
drivers/dma-buf/heaps/Makefile | 1 +
drivers/dma-buf/heaps/chunk_heap.c | 222 +++++++++++++++++++++
drivers/dma-buf/heaps/heap-helpers.c | 2 +
6 files changed, 282 insertions(+)
create mode 100644 Documentation/devicetree/bindings/dma-buf/chunk_heap.yaml
create mode 100644 drivers/dma-buf/heaps/chunk_heap.c
--
2.7.4
Fix W=1 compile warnings (invalid kerneldoc):
drivers/dma-buf/dma-buf.c:328: warning: Function parameter or member 'dmabuf' not described in 'dma_buf_set_name'
Signed-off-by: Krzysztof Kozlowski <krzk(a)kernel.org>
---
drivers/dma-buf/dma-buf.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 1699a8e309ef..58564d82a3a2 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -316,9 +316,9 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll)
* name of the dma-buf if the same piece of memory is used for multiple
* purpose between different devices.
*
- * @dmabuf [in] dmabuf buffer that will be renamed.
- * @buf: [in] A piece of userspace memory that contains the name of
- * the dma-buf.
+ * @dmabuf: [in] dmabuf buffer that will be renamed.
+ * @buf: [in] A piece of userspace memory that contains the name of
+ * the dma-buf.
*
* Returns 0 on success. If the dma-buf buffer is already attached to
* devices, return -EBUSY.
--
2.17.1
This patchset implements the current proposal for virtio cross-device
resource sharing [1]. It will be used to import virtio resources into
the virtio-video driver currently under discussion [2]. The patch
under consideration to add support in the virtio-video driver is [3].
It uses the APIs from v3 of this series, but the changes to update it
are relatively minor.
This patchset adds a new flavor of dma-bufs that supports querying the
underlying virtio object UUID, as well as adding support for exporting
resources from virtgpu.
[1] https://markmail.org/thread/2ypjt5cfeu3m6lxu
[2] https://markmail.org/thread/p5d3k566srtdtute
[3] https://markmail.org/thread/j4xlqaaim266qpks
v6 -> v7 changes:
- Fix most strict checkpatch comments
David Stevens (3):
virtio: add dma-buf support for exported objects
virtio-gpu: add VIRTIO_GPU_F_RESOURCE_UUID feature
drm/virtio: Support virtgpu exported resources
drivers/gpu/drm/virtio/virtgpu_drv.c | 3 +
drivers/gpu/drm/virtio/virtgpu_drv.h | 21 ++++++
drivers/gpu/drm/virtio/virtgpu_kms.c | 4 ++
drivers/gpu/drm/virtio/virtgpu_prime.c | 96 +++++++++++++++++++++++++-
drivers/gpu/drm/virtio/virtgpu_vq.c | 55 +++++++++++++++
drivers/virtio/Makefile | 2 +-
drivers/virtio/virtio.c | 6 ++
drivers/virtio/virtio_dma_buf.c | 85 +++++++++++++++++++++++
include/linux/virtio.h | 1 +
include/linux/virtio_dma_buf.h | 37 ++++++++++
include/uapi/linux/virtio_gpu.h | 19 +++++
11 files changed, 325 insertions(+), 4 deletions(-)
create mode 100644 drivers/virtio/virtio_dma_buf.c
create mode 100644 include/linux/virtio_dma_buf.h
--
2.28.0.220.ged08abb693-goog
This patchset implements the current proposal for virtio cross-device
resource sharing [1]. It will be used to import virtio resources into
the virtio-video driver currently under discussion [2]. The patch
under consideration to add support in the virtio-video driver is [3].
It uses the APIs from v3 of this series, but the changes to update it
are relatively minor.
This patchset adds a new flavor of dma-bufs that supports querying the
underlying virtio object UUID, as well as adding support for exporting
resources from virtgpu.
[1] https://markmail.org/thread/2ypjt5cfeu3m6lxu
[2] https://markmail.org/thread/p5d3k566srtdtute
[3] https://markmail.org/thread/j4xlqaaim266qpks
v5 -> v6 changes:
- Fix >80 character comment
David Stevens (3):
virtio: add dma-buf support for exported objects
virtio-gpu: add VIRTIO_GPU_F_RESOURCE_UUID feature
drm/virtio: Support virtgpu exported resources
drivers/gpu/drm/virtio/virtgpu_drv.c | 3 +
drivers/gpu/drm/virtio/virtgpu_drv.h | 20 ++++++
drivers/gpu/drm/virtio/virtgpu_kms.c | 4 ++
drivers/gpu/drm/virtio/virtgpu_prime.c | 96 +++++++++++++++++++++++++-
drivers/gpu/drm/virtio/virtgpu_vq.c | 55 +++++++++++++++
drivers/virtio/Makefile | 2 +-
drivers/virtio/virtio.c | 6 ++
drivers/virtio/virtio_dma_buf.c | 82 ++++++++++++++++++++++
include/linux/virtio.h | 1 +
include/linux/virtio_dma_buf.h | 37 ++++++++++
include/uapi/linux/virtio_gpu.h | 19 +++++
11 files changed, 321 insertions(+), 4 deletions(-)
create mode 100644 drivers/virtio/virtio_dma_buf.c
create mode 100644 include/linux/virtio_dma_buf.h
--
2.28.0.220.ged08abb693-goog
Unless there are any remaining objections to these patches, what are
the next steps towards getting these merged? Sorry, I'm not familiar
with the workflow for contributing patches to Linux.
Thanks,
David
On Tue, Jun 9, 2020 at 6:53 PM Michael S. Tsirkin <mst(a)redhat.com> wrote:
>
> On Tue, Jun 09, 2020 at 10:25:15AM +0900, David Stevens wrote:
> > This patchset implements the current proposal for virtio cross-device
> > resource sharing [1]. It will be used to import virtio resources into
> > the virtio-video driver currently under discussion [2]. The patch
> > under consideration to add support in the virtio-video driver is [3].
> > It uses the APIs from v3 of this series, but the changes to update it
> > are relatively minor.
> >
> > This patchset adds a new flavor of dma-bufs that supports querying the
> > underlying virtio object UUID, as well as adding support for exporting
> > resources from virtgpu.
>
> Gerd, David, if possible, please test this in configuration with
> virtual VTD enabled but with iommu_platform=off
> to make sure we didn't break this config.
>
>
> Besides that, for virtio parts:
>
> Acked-by: Michael S. Tsirkin <mst(a)redhat.com>
>
>
> > [1] https://markmail.org/thread/2ypjt5cfeu3m6lxu
> > [2] https://markmail.org/thread/p5d3k566srtdtute
> > [3] https://markmail.org/thread/j4xlqaaim266qpks
> >
> > v4 -> v5 changes:
> > - Remove virtio_dma_buf_export_info.
> >
> > David Stevens (3):
> > virtio: add dma-buf support for exported objects
> > virtio-gpu: add VIRTIO_GPU_F_RESOURCE_UUID feature
> > drm/virtio: Support virtgpu exported resources
> >
> > drivers/gpu/drm/virtio/virtgpu_drv.c | 3 +
> > drivers/gpu/drm/virtio/virtgpu_drv.h | 20 ++++++
> > drivers/gpu/drm/virtio/virtgpu_kms.c | 4 ++
> > drivers/gpu/drm/virtio/virtgpu_prime.c | 96 +++++++++++++++++++++++++-
> > drivers/gpu/drm/virtio/virtgpu_vq.c | 55 +++++++++++++++
> > drivers/virtio/Makefile | 2 +-
> > drivers/virtio/virtio.c | 6 ++
> > drivers/virtio/virtio_dma_buf.c | 82 ++++++++++++++++++++++
> > include/linux/virtio.h | 1 +
> > include/linux/virtio_dma_buf.h | 37 ++++++++++
> > include/uapi/linux/virtio_gpu.h | 19 +++++
> > 11 files changed, 321 insertions(+), 4 deletions(-)
> > create mode 100644 drivers/virtio/virtio_dma_buf.c
> > create mode 100644 include/linux/virtio_dma_buf.h
> >
> > --
> > 2.27.0.278.ge193c7cf3a9-goog
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe(a)lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help(a)lists.oasis-open.org
>
This is an RFC because I'm still trying to grok the correct behavior.
Consider a dma_fence_array created two two fence and signal_on_any is true.
A reference to dma_fence_array is taken for each waiting fence.
When the client calls dma_fence_wait() only one of the fences is signaled.
The client returns successfully from the wait and puts it's reference to
the array fence but the array fence still remains because of the remaining
un-signaled fence.
Now consider that the unsignaled fence is signaled while the timeline is being
destroyed much later. The timeline destroy calls dma_fence_signal_locked(). The
following sequence occurs:
1) dma_fence_array_cb_func is called
2) array->num_pending is 0 (because it was set to 1 due to signal_on_any) so the
callback function calls dma_fence_put() instead of triggering the irq work
3) The array fence is released which in turn puts the lingering fence which is
then released
4) deadlock with the timeline
I think that we can fix this with the attached patch. Once the fence is
signaled signaling it again in the irq worker shouldn't hurt anything. The only
gotcha might be how the error is propagated - I wasn't quite sure the intent of
clearing it only after getting to the irq worker.
Signed-off-by: Jordan Crouse <jcrouse(a)codeaurora.org>
---
drivers/dma-buf/dma-fence-array.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/dma-buf/dma-fence-array.c b/drivers/dma-buf/dma-fence-array.c
index d3fbd950be94..b8829b024255 100644
--- a/drivers/dma-buf/dma-fence-array.c
+++ b/drivers/dma-buf/dma-fence-array.c
@@ -46,8 +46,6 @@ static void irq_dma_fence_array_work(struct irq_work *wrk)
{
struct dma_fence_array *array = container_of(wrk, typeof(*array), work);
- dma_fence_array_clear_pending_error(array);
-
dma_fence_signal(&array->base);
dma_fence_put(&array->base);
}
@@ -61,10 +59,10 @@ static void dma_fence_array_cb_func(struct dma_fence *f,
dma_fence_array_set_pending_error(array, f->error);
- if (atomic_dec_and_test(&array->num_pending))
- irq_work_queue(&array->work);
- else
- dma_fence_put(&array->base);
+ if (!atomic_dec_and_test(&array->num_pending))
+ dma_fence_array_set_pending_error(array, f->error);
+
+ irq_work_queue(&array->work);
}
static bool dma_fence_array_enable_signaling(struct dma_fence *fence)
--
2.25.1
With commit 72b6ede73623 ("dma-buf.rst: Document why indefinite fences are
a bad idea"), document generation warns:
Documentation/driver-api/dma-buf.rst:182: \
WARNING: Title underline too short.
Repair length of title underline to remove warning.
Fixes: 72b6ede73623 ("dma-buf.rst: Document why indefinite fences are a bad idea")
Signed-off-by: Lukas Bulwahn <lukas.bulwahn(a)gmail.com>
---
Daniel, please pick this minor non-urgent fix to your new documentation.
Documentation/driver-api/dma-buf.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/driver-api/dma-buf.rst b/Documentation/driver-api/dma-buf.rst
index 100bfd227265..13ea0cc0a3fa 100644
--- a/Documentation/driver-api/dma-buf.rst
+++ b/Documentation/driver-api/dma-buf.rst
@@ -179,7 +179,7 @@ DMA Fence uABI/Sync File
:internal:
Indefinite DMA Fences
-~~~~~~~~~~~~~~~~~~~~
+~~~~~~~~~~~~~~~~~~~~~
At various times &dma_fence with an indefinite time until dma_fence_wait()
finishes have been proposed. Examples include:
--
2.17.1