- Linaro-mm-sig - lists.linaro.org

Re: [Linaro-mm-sig] [PATCH] dma-buf/resv: fix exclusive fence get

by Koenig, Christian

Hi Qiang, oh, good point. Yes it certainly should. Looks like I accidentally pushed it to the wrong branch. Thanks, Christian. Am 10.10.19 um 16:27 schrieb Qiang Yu: > Hi Chris, > > This fix has been pushed to drm-misc-next for a while. But Linux > 5.4-rc kernels still does not have this fix. > Should it be also pushed to drm-misc-fixes? > > Thanks, > Qiang > > > On Sun, Sep 22, 2019 at 8:50 PM Chris Wilson <chris(a)chris-wilson.co.uk> wrote: >> Quoting Chris Wilson (2019-09-22 13:17:19) >>> Quoting Qiang Yu (2019-09-22 08:49:00) >>>> This causes kernel crash when testing lima driver. >>>> >>>> Cc: Christian König <christian.koenig(a)amd.com> >>>> Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu a bit") >>>> Signed-off-by: Qiang Yu <yuq825(a)gmail.com> >>>> --- >>>> drivers/dma-buf/dma-resv.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c >>>> index 42a8f3f11681..709002515550 100644 >>>> --- a/drivers/dma-buf/dma-resv.c >>>> +++ b/drivers/dma-buf/dma-resv.c >>>> @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj, >>>> if (pfence_excl) >>>> *pfence_excl = fence_excl; >>>> else if (fence_excl) >>>> - shared[++shared_count] = fence_excl; >>>> + shared[shared_count++] = fence_excl; >>> Oops. >>> >>> Reviewed-by: Chris Wilson <chris(a)chris-wilson.co.uk> >> Applied, thanks for the fix. >> -Chris

6 years, 2 months

1
0
0 0

Re: [Linaro-mm-sig] [PATCH] dma-buf/resv: fix exclusive fence get

by Daniel Vetter

On Sun, Sep 22, 2019 at 2:08 PM Qiang Yu <yuq825(a)gmail.com> wrote: > > This causes kernel crash when testing lima driver. > > Cc: Christian König <christian.koenig(a)amd.com> > Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu a bit") > Signed-off-by: Qiang Yu <yuq825(a)gmail.com> Selftest for this would be lovely, now that the basic infrastructure is in place ... -Daniel > --- > drivers/dma-buf/dma-resv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > index 42a8f3f11681..709002515550 100644 > --- a/drivers/dma-buf/dma-resv.c > +++ b/drivers/dma-buf/dma-resv.c > @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj, > if (pfence_excl) > *pfence_excl = fence_excl; > else if (fence_excl) > - shared[++shared_count] = fence_excl; > + shared[shared_count++] = fence_excl; > > if (!shared_count) { > kfree(shared); > -- > 2.17.1 > > _______________________________________________ > dri-devel mailing list > dri-devel(a)lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch

6 years, 2 months

2
2
0 0

Re: [Linaro-mm-sig] [RFC PATCH] drm:- Add a modifier to denote 'protected' framebuffer

by Alex Deucher

On Mon, Sep 30, 2019 at 8:57 AM Ayan Halder <Ayan.Halder(a)arm.com> wrote: > > On Mon, Sep 30, 2019 at 09:51:35AM +0000, Brian Starkey wrote: > > Hi, > > > > On Tue, Sep 17, 2019 at 07:36:45PM +0200, Daniel Vetter wrote: > > > On Tue, Sep 17, 2019 at 6:15 PM Neil Armstrong <narmstrong(a)baylibre.com> wrote: > > > > > > > > Hi, > > > > > > > > On 17/09/2019 18:07, Liviu Dudau wrote: > > > > > On Tue, Sep 17, 2019 at 02:53:01PM +0200, Daniel Vetter wrote: > > > > >> On Mon, Sep 09, 2019 at 01:42:53PM +0000, Ayan Halder wrote: > > > > >>> Add a modifier 'DRM_FORMAT_MOD_ARM_PROTECTED' which denotes that the framebuffer > > > > >>> is allocated in a protected system memory. > > > > >>> Essentially, we want to support EGL_EXT_protected_content in our komeda driver. > > > > >>> > > > > >>> Signed-off-by: Ayan Kumar Halder <ayan.halder(a)arm.com> > > > > >>> > > > > >>> /-- Note to reviewer > > > > >>> Komeda driver is capable of rendering DRM (Digital Rights Management) protected > > > > >>> content. The DRM content is stored in a framebuffer allocated in system memory > > > > >>> (which needs some special hardware signals for access). > > > > >>> > > > > >>> Let us ignore how the protected system memory is allocated and for the scope of > > > > >>> this discussion, we want to figure out the best way possible for the userspace > > > > >>> to communicate to the drm driver to turn the protected mode on (for accessing the > > > > >>> framebuffer with the DRM content) or off. > > > > >>> > > > > >>> The possible ways by which the userspace could achieve this is via:- > > > > >>> > > > > >>> 1. Modifiers :- This looks to me the best way by which the userspace can > > > > >>> communicate to the kernel to turn the protected mode on for the komeda driver > > > > >>> as it is going to access one of the protected framebuffers. The only problem is > > > > >>> that the current modifiers describe the tiling/compression format. However, it > > > > >>> does not hurt to extend the meaning of modifiers to denote other attributes of > > > > >>> the framebuffer as well. > > > > >>> > > > > >>> The other reason is that on Android, we get an info from Gralloc > > > > >>> (GRALLOC_USAGE_PROTECTED) which tells us that the buffer is protected. This can > > > > >>> be used to set up the modifier/s (AddFB2) during framebuffer creation. > > > > >> > > > > >> How does this mesh with other modifiers, like AFBC? That's where I see the > > > > >> issue here. > > > > > > > > > > AFBC modifiers are currently under Arm's namespace, the thought behind the DRM > > > > > modifiers would be to have it as a "generic" modifier. > > > > > > But if it's a generic flag, how do you combine that with other > > > modifiers? Like if you have a tiled buffer, but also encrypted? Or > > > afbc compressed, or whatever else. I'd expect for your hw encryption > > > is orthogonal to the buffer/tiling/compression format used? > > > > This bit doesn't overlap with any of the other AFBC modifiers, so as > > you say it'd be orthogonal, and could be set on AFBC buffers (if we > > went that route). > > > > > > > > > >>> 2. Framebuffer flags :- As of today, this can be one of the two values > > > > >>> ie (DRM_MODE_FB_INTERLACED/DRM_MODE_FB_MODIFIERS). Unlike modifiers, the drm > > > > >>> framebuffer flags are generic to the drm subsystem and ideally we should not > > > > >>> introduce any driver specific constraint/feature. > > > > >>> > > > > >>> 3. Connector property:- I could see the following properties used for DRM > > > > >>> protected content:- > > > > >>> DRM_MODE_CONTENT_PROTECTION_DESIRED / ENABLED :- "This property is used by > > > > >>> userspace to request the kernel protect future content communicated over > > > > >>> the link". Clearly, we are not concerned with the protection attributes of the > > > > >>> transmitter. So, we cannot use this property for our case. > > > > >>> > > > > >>> 4. DRM plane property:- Again, we want to communicate that the framebuffer(which > > > > >>> can be attached to any plane) is protected. So introducing a new plane property > > > > >>> does not help. > > > > >>> > > > > >>> 5. DRM crtc property:- For the same reason as above, introducing a new crtc > > > > >>> property does not help. > > > > >> > > > > >> 6. Just track this as part of buffer allocation, i.e. I think it does > > > > >> matter how you allocate these protected buffers. We could add a "is > > > > >> protected buffer" flag at the dma_buf level for this. > > > > I also like this approach. The protected-ness is a property of the > > allocation, so makes sense to store it with the allocation IMO. > > > > > > >> > > > > >> So yeah for this stuff here I think we do want the full userspace side, > > > > >> from allocator to rendering something into this protected buffers (no need > > > > >> to also have the entire "decode a protected bitstream part" imo, since > > > > >> that will freak people out). Unfortunately, in my experience, that kills > > > > >> it for upstream :-/ But also in my experience of looking into this for > > > > >> other gpu's, we really need to have the full picture here to make sure > > > > >> we're not screwing this up. > > > > > > > > > > Maybe Ayan could've been a bit clearer in his message, but the ask here is for ideas > > > > > on how userspace "communicates" (stores?) the fact that the buffers are protected to > > > > > the kernel driver. In our display processor we need to the the hardware that the > > > > > buffers are protected before it tries to fetch them so that it can 1) enable the > > > > > additional hardware signaling that sets the protection around the stream; and 2) read > > > > > the protected buffers in a special mode where there the magic happens. > > > > > > That was clear, but for the full picture we also need to know how > > > these buffers are produced and where they are allocated. One approach > > > would be to have a dma-buf heap that gives you encrypted buffers back. > > > With that we need to make sure that only encryption-aware drivers > > > allow such buffers to be imported, and the entire problem becomes a > > > kernel-internal one - aside from allocating the right kind of buffer > > > at the right place. > > > > > > > In our case, we'd be supporting a system like TZMP-1, there's a > > Linaro connect presentation on it here: > > https://connect.linaro.org/resources/hkg18/hkg18-408/ > > > > The simplest way to implement this is for firmware to set up a > > carveout which it tells linux is secure. A linux allocator (ion, gem, > > vb2, whatever) can allocate from this carveout, and tag the buffer as > > secure. > > > > In this kind of system, linux doesn't necessarily need to know > > anything about how buffers are protected, or what HW is capable of - > > it only needs to carry around the "is_protected" flag. > > > > Here, the TEE is ultimately responsible for deciding which HW gets > > access to a buffer. I don't see a benefit of having linux decide which > > drivers can or cannot import a buffer, because that decision should be > > handled by the TEE. > > > > For proving out the pipeline, IMO it doesn't matter whether the > > buffers are protected or not. For our DPU, all that matters is that if > > the buffer claims to be protected, we have to set our protected > > control bit. Nothing more. AFAIK it should work the same for other > > TZMP-1 implementations. > > > > > > > So yeah, we know we do want full userspace support, we're prodding the community on > > > > > answers on how to best let the kernel side know what userspace has done. > > > > > > > > Actually this is interesting for other multimedia SoCs implementing secure video decode > > > > paths where video buffers are allocated and managed by a trusted app. > > > > > > Yeah I expect there's more than just arm wanting this. I also wonder > > > how that interacts with the secure memory allocator that was bobbing > > > around on dri-devel for a while, but seems to not have gone anywhere. > > > That thing implemented my idea of "secure memory is only allocated by > > > a special entity". > > > -Daniel > > > > Like I said, for us all we need is a way to carry around a 1-bit > > "is_protected" flag with a buffer. Could other folks share what's > > needed for their systems so we can reason about something that works > > for all? > > To make things a bit more specific, we are thinking of the following > patch :- > > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h > index ec212cb27fdc..36f0813073a2 100644 > --- a/include/linux/dma-buf.h > +++ b/include/linux/dma-buf.h > @@ -279,6 +279,7 @@ struct dma_buf_ops { > * kernel module. > * @list_node: node for dma_buf accounting and debugging. > * @priv: exporter specific private data for this buffer object. > + * @is_protected: denotes that the buffer is > secure/protected/encrypted/trusted. > * @resv: reservation object linked to this dma-buf > * @poll: for userspace poll support > * @cb_excl: for userspace poll support > @@ -306,6 +307,7 @@ struct dma_buf { > struct module *owner; > struct list_head list_node; > void *priv; > + bool is_protected; > struct dma_resv *resv; > > /* poll support */ > > @all, @amdgpu-folks :- Is this something you can use of to denote > secure/protected/encrypted/trusted buffers ? I suppose. At the moment, we don't really have a need for it since we only our IPs support our encryption scheme and if we share buffers between we can get to the secure status when we look up the amdgpu buffer object internally in the kernel side. Still might be useful for cases where secure buffers get shared across drivers so we have a generic check for secure status. Alex > > The way 'is_protected' flag gets used to allocate > secure/protected/encrypted buffers will be vendor specific. > > Please comment to let us know if it looks useful to non Arm folks. > > > > Thanks! > > -Brian > > > > > > > > > > > > > Neil > > > > > > > > > > > > > > Best regards, > > > > > Liviu > > > > > > > > > > > > > > >> -Daniel > > > > >> > > > > >>> > > > > >>> --/ > > > > >>> > > > > >>> --- > > > > >>> include/uapi/drm/drm_fourcc.h | 9 +++++++++ > > > > >>> 1 file changed, 9 insertions(+) > > > > >>> > > > > >>> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h > > > > >>> index 3feeaa3f987a..38e5e81d11fe 100644 > > > > >>> --- a/include/uapi/drm/drm_fourcc.h > > > > >>> +++ b/include/uapi/drm/drm_fourcc.h > > > > >>> @@ -742,6 +742,15 @@ extern "C" { > > > > >>> */ > > > > >>> #define AFBC_FORMAT_MOD_BCH (1ULL << 11) > > > > >>> > > > > >>> +/* > > > > >>> + * Protected framebuffer > > > > >>> + * > > > > >>> + * The framebuffer is allocated in a protected system memory which can be accessed > > > > >>> + * via some special hardware signals from the dpu. This is used to support > > > > >>> + * 'GRALLOC_USAGE_PROTECTED' in our framebuffer for EGL_EXT_protected_content. > > > > >>> + */ > > > > >>> +#define DRM_FORMAT_MOD_ARM_PROTECTED fourcc_mod_code(ARM, (1ULL << 55)) > > > > >>> + > > > > >>> /* > > > > >>> * Allwinner tiled modifier > > > > >>> * > > > > >>> -- > > > > >>> 2.23.0 > > > > >>> > > > > >> > > > > >> -- > > > > >> Daniel Vetter > > > > >> Software Engineer, Intel Corporation > > > > >> http://blog.ffwll.ch > > > > > > > > > > > > > _______________________________________________ > > > > dri-devel mailing list > > > > dri-devel(a)lists.freedesktop.org > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > > > > > > > > > -- > > > Daniel Vetter > > > Software Engineer, Intel Corporation > > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch

6 years, 2 months

1
0
0 0

Re: [Linaro-mm-sig] [PATCH] ion_system_heap: support X86 archtecture

by Laura Abbott

On 9/29/19 3:28 AM, jun.zhang(a)intel.com wrote: > From: zhang jun <jun.zhang(a)intel.com> > > we see tons of warning like: > [ 45.846872] x86/PAT: NDK MediaCodec_:3753 map pfn RAM range req > write-combining for [mem 0x1e7a80000-0x1e7a87fff], got write-back > [ 45.848827] x86/PAT: .vorbis.decoder:4088 map pfn RAM range req > write-combining for [mem 0x1e7a58000-0x1e7a58fff], got write-back > [ 45.848875] x86/PAT: NDK MediaCodec_:3753 map pfn RAM range req > write-combining for [mem 0x1e7a48000-0x1e7a4ffff], got write-back > [ 45.849403] x86/PAT: .vorbis.decoder:4088 map pfn RAM range > req write-combining for [mem 0x1e7a70000-0x1e7a70fff], got write-back > > check the kernel Documentation/x86/pat.txt, it says: > A. Exporting pages to users with remap_pfn_range, io_remap_pfn_range, > vm_insert_pfn > Drivers wanting to export some pages to userspace do it by using > mmap interface and a combination of > 1) pgprot_noncached() > 2) io_remap_pfn_range() or remap_pfn_range() or vm_insert_pfn() > With PAT support, a new API pgprot_writecombine is being added. > So, drivers can continue to use the above sequence, with either > pgprot_noncached() or pgprot_writecombine() in step 1, followed by step 2. > > In addition, step 2 internally tracks the region as UC or WC in > memtype list in order to ensure no conflicting mapping. > > Note that this set of APIs only works with IO (non RAM) regions. > If driver ants to export a RAM region, it has to do set_memory_uc() or > set_memory_wc() as step 0 above and also track the usage of those pages > and use set_memory_wb() before the page is freed to free pool. > > the fix follow the pat document, do set_memory_wc() as step 0 and > use the set_memory_wb() before the page is freed. > All this work needs to be done on the new dma-buf heap rework and I don't think it makes sense to put it on the staging version https://lore.kernel.org/lkml/20190906184712.91980-1-john.stultz@linaro.org/ (I also continue to question the value of uncached buffers, especially on x86) > Signed-off-by: he, bo <bo.he(a)intel.com> > Signed-off-by: zhang jun <jun.zhang(a)intel.com> > Signed-off-by: Bai, Jie A <jie.a.bai(a)intel.com> > --- > drivers/staging/android/ion/ion_system_heap.c | 28 ++++++++++++++++++- > 1 file changed, 27 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c > index b83a1d16bd89..d298b8194820 100644 > --- a/drivers/staging/android/ion/ion_system_heap.c > +++ b/drivers/staging/android/ion/ion_system_heap.c > @@ -13,6 +13,7 @@ > #include <linux/scatterlist.h> > #include <linux/slab.h> > #include <linux/vmalloc.h> > +#include <asm/set_memory.h> > > #include "ion.h" > > @@ -134,6 +135,13 @@ static int ion_system_heap_allocate(struct ion_heap *heap, > sg = table->sgl; > list_for_each_entry_safe(page, tmp_page, &pages, lru) { > sg_set_page(sg, page, page_size(page), 0); > + > +#ifdef CONFIG_X86 > + if (!(buffer->flags & ION_FLAG_CACHED)) > + set_memory_wc((unsigned long)page_address(sg_page(sg)), > + PAGE_ALIGN(sg->length) >> PAGE_SHIFT); > +#endif > + > sg = sg_next(sg); > list_del(&page->lru); > } > @@ -162,8 +170,15 @@ static void ion_system_heap_free(struct ion_buffer *buffer) > if (!(buffer->private_flags & ION_PRIV_FLAG_SHRINKER_FREE)) > ion_heap_buffer_zero(buffer); > > - for_each_sg(table->sgl, sg, table->nents, i) > + for_each_sg(table->sgl, sg, table->nents, i) { > +#ifdef CONFIG_X86 > + if (!(buffer->flags & ION_FLAG_CACHED)) > + set_memory_wb((unsigned long)page_address(sg_page(sg)), > + PAGE_ALIGN(sg->length) >> PAGE_SHIFT); > +#endif > + > free_buffer_page(sys_heap, buffer, sg_page(sg)); > + } > sg_free_table(table); > kfree(table); > } > @@ -316,6 +331,12 @@ static int ion_system_contig_heap_allocate(struct ion_heap *heap, > > buffer->sg_table = table; > > +#ifdef CONFIG_X86 > + if (!(buffer->flags & ION_FLAG_CACHED)) > + set_memory_wc((unsigned long)page_address(page), > + PAGE_ALIGN(len) >> PAGE_SHIFT); > +#endif > + > return 0; > > free_table: > @@ -334,6 +355,11 @@ static void ion_system_contig_heap_free(struct ion_buffer *buffer) > unsigned long pages = PAGE_ALIGN(buffer->size) >> PAGE_SHIFT; > unsigned long i; > > +#ifdef CONFIG_X86 > + if (!(buffer->flags & ION_FLAG_CACHED)) > + set_memory_wb((unsigned long)page_address(page), pages); > +#endif > + > for (i = 0; i < pages; i++) > __free_page(page + i); > sg_free_table(table); >

6 years, 2 months

1
0
0 0

Re: [Linaro-mm-sig] [PATCH] ion_system_heap: support X86 archtecture

by Greg KH

On Sun, Sep 29, 2019 at 03:28:41PM +0800, jun.zhang(a)intel.com wrote: > From: zhang jun <jun.zhang(a)intel.com> > > we see tons of warning like: > [ 45.846872] x86/PAT: NDK MediaCodec_:3753 map pfn RAM range req > write-combining for [mem 0x1e7a80000-0x1e7a87fff], got write-back > [ 45.848827] x86/PAT: .vorbis.decoder:4088 map pfn RAM range req > write-combining for [mem 0x1e7a58000-0x1e7a58fff], got write-back > [ 45.848875] x86/PAT: NDK MediaCodec_:3753 map pfn RAM range req > write-combining for [mem 0x1e7a48000-0x1e7a4ffff], got write-back > [ 45.849403] x86/PAT: .vorbis.decoder:4088 map pfn RAM range > req write-combining for [mem 0x1e7a70000-0x1e7a70fff], got write-back > > check the kernel Documentation/x86/pat.txt, it says: > A. Exporting pages to users with remap_pfn_range, io_remap_pfn_range, > vm_insert_pfn > Drivers wanting to export some pages to userspace do it by using > mmap interface and a combination of > 1) pgprot_noncached() > 2) io_remap_pfn_range() or remap_pfn_range() or vm_insert_pfn() > With PAT support, a new API pgprot_writecombine is being added. > So, drivers can continue to use the above sequence, with either > pgprot_noncached() or pgprot_writecombine() in step 1, followed by step 2. > > In addition, step 2 internally tracks the region as UC or WC in > memtype list in order to ensure no conflicting mapping. > > Note that this set of APIs only works with IO (non RAM) regions. > If driver ants to export a RAM region, it has to do set_memory_uc() or > set_memory_wc() as step 0 above and also track the usage of those pages > and use set_memory_wb() before the page is freed to free pool. > > the fix follow the pat document, do set_memory_wc() as step 0 and > use the set_memory_wb() before the page is freed. > > Signed-off-by: he, bo <bo.he(a)intel.com> > Signed-off-by: zhang jun <jun.zhang(a)intel.com> > Signed-off-by: Bai, Jie A <jie.a.bai(a)intel.com> > --- > drivers/staging/android/ion/ion_system_heap.c | 28 ++++++++++++++++++- > 1 file changed, 27 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/android/ion/ion_system_heap.c b/drivers/staging/android/ion/ion_system_heap.c > index b83a1d16bd89..d298b8194820 100644 > --- a/drivers/staging/android/ion/ion_system_heap.c > +++ b/drivers/staging/android/ion/ion_system_heap.c > @@ -13,6 +13,7 @@ > #include <linux/scatterlist.h> > #include <linux/slab.h> > #include <linux/vmalloc.h> > +#include <asm/set_memory.h> > > #include "ion.h" > > @@ -134,6 +135,13 @@ static int ion_system_heap_allocate(struct ion_heap *heap, > sg = table->sgl; > list_for_each_entry_safe(page, tmp_page, &pages, lru) { > sg_set_page(sg, page, page_size(page), 0); > + > +#ifdef CONFIG_X86 > + if (!(buffer->flags & ION_FLAG_CACHED)) > + set_memory_wc((unsigned long)page_address(sg_page(sg)), > + PAGE_ALIGN(sg->length) >> PAGE_SHIFT); > +#endif There is no way to do this without these #ifdefs? That feels odd, why can't you just always test for this? thanks, greg k-h

6 years, 2 months

1
0
0 0

Re: [Linaro-mm-sig] [PATCH] dma-buf/resv: fix exclusive fence get

by Chris Wilson

Quoting Qiang Yu (2019-09-22 08:49:00) > This causes kernel crash when testing lima driver. > > Cc: Christian König <christian.koenig(a)amd.com> > Fixes: b8c036dfc66f ("dma-buf: simplify reservation_object_get_fences_rcu a bit") > Signed-off-by: Qiang Yu <yuq825(a)gmail.com> > --- > drivers/dma-buf/dma-resv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c > index 42a8f3f11681..709002515550 100644 > --- a/drivers/dma-buf/dma-resv.c > +++ b/drivers/dma-buf/dma-resv.c > @@ -471,7 +471,7 @@ int dma_resv_get_fences_rcu(struct dma_resv *obj, > if (pfence_excl) > *pfence_excl = fence_excl; > else if (fence_excl) > - shared[++shared_count] = fence_excl; > + shared[shared_count++] = fence_excl; Oops. Reviewed-by: Chris Wilson <chris(a)chris-wilson.co.uk> -Chris

6 years, 2 months

1
1
0 0

Re: [Linaro-mm-sig] [PATCH] staging: android: ion: Replace strncpy() for stracpy()

by Dan Carpenter

On Sun, Sep 08, 2019 at 02:34:50PM +1000, Adam Zerella wrote: > Using strncpy() does not always terminate the destination string. > stracpy() is a alternative function that does, by using this new > function we will no longer need to insert a null separator. > > Signed-off-by: Adam Zerella <adam.zerella(a)gmail.com> > --- > drivers/staging/android/ion/ion.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c > index e6b1ca141b93..17901bd626be 100644 > --- a/drivers/staging/android/ion/ion.c > +++ b/drivers/staging/android/ion/ion.c > @@ -433,8 +433,7 @@ static int ion_query_heaps(struct ion_heap_query *query) > max_cnt = query->cnt; > > plist_for_each_entry(heap, &dev->heaps, node) { > - strncpy(hdata.name, heap->name, MAX_HEAP_NAME); > - hdata.name[sizeof(hdata.name) - 1] = '\0'; > + stracpy(hdata.name, heap->name, MAX_HEAP_NAME); stracpy() only takes two arguments. This doesn't compile. regards, dan carpenter

6 years, 3 months

1
0
0 0

Use dma_fence_array for implementing shared dma_resv fences

by Christian König

This is the new dma_fence_array based container for shared fences in the dma_resv object. Advantage of this approach is that you can grab a reference to the current set of shared fences at any time, which allows us to drop the sequence number increment and makes the whole RCU handling much more easier. Disadvantage is that RCU users now have to grab a reference instead of using the sequence counter. As far as I can see i915 was actually the only driver doing this. So we optimize for adding more fences instead of reading them now. Another behavior change worth noting is that the shared fences are now only visible after unlocking the dma_resv object or calling dma_resv_fences_commit() manually. Please review and/or comment, Christian.

6 years, 3 months

3
10
0 0

Re: [Linaro-mm-sig] [PATCH 08/10] dma-buf/resv: replace shared fence with new fences container

by Chris Wilson

Quoting Christian König (2019-08-21 13:31:45) > @@ -528,20 +352,9 @@ void dma_resv_prune_fences(struct dma_resv *obj) > dma_fence_put(fence); > } > > - list = dma_resv_get_list(obj); > - if (!list) > - return; > - > - for (i = 0; i < list->shared_count; ++i) { > - fence = rcu_dereference_protected(list->shared[i], > - dma_resv_held(obj)); > - > - if (!dma_fence_is_signaled(fence)) > - continue; > - > - RCU_INIT_POINTER(list->shared[i], dma_fence_get_stub()); > - dma_fence_put(fence); > - } > + fence = dma_resv_fences_deref(obj, &obj->readers); > + if (dma_fence_is_signaled(fence)) > + dma_resv_fences_set(obj, &obj->readers, NULL); Something to note is that a dma-fence-array is not automatically signaled and dma_fence_is_signaled() does not check the array. -Chris

6 years, 3 months

1
0
0 0

[RFC] replacing dma_resv API

by Christian König

Hi everyone, In previous discussion it surfaced that different drivers use the shared and explicit fences in the dma_resv object with different meanings. This is problematic when we share buffers between those drivers and requirements for implicit and explicit synchronization leaded to quite a number of workarounds related to this. So I started an effort to get all drivers back to a common understanding of what the fences in the dma_resv object mean and be able to use the object for different kind of workloads independent of the classic DRM command submission interface. The result is this patch set which modifies the dma_resv API to get away from a single explicit fence and multiple shared fences, towards a notation where we have explicit categories for writers, readers and others. To do this I came up with a new container called dma_resv_fences which can store both a single fence as well as multiple fences in a dma_fence_array. This turned out to actually be even be quite a bit simpler, since we don't need any complicated dance between RCU and sequence count protected updates any more. Instead we can just grab a reference to the dma_fence_array under RCU and so keep the current state of synchronization alive until we are done with it. This results in both a small performance improvement since we don't need so many barriers any more, as well as fewer lines of code in the actual implementation. Please review and/or comment, Christian.

6 years, 3 months

6
25
0 0

[PATCH 3/3] udmabuf: check that flags has no unsupported bits set

by Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel(a)redhat.com> Reported-by: Yann Droneaud <ydroneaud(a)opteya.com> --- drivers/dma-buf/udmabuf.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 6c3ec8fcef01..ca1364102b18 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -131,6 +131,9 @@ static long udmabuf_create(const struct udmabuf_create_list *head, int seals, ret = -EINVAL; u32 i, flags; + if (head->flags & ~UDMABUF_FLAGS_CLOEXEC) + return -EINVAL; + ubuf = kzalloc(sizeof(*ubuf), GFP_KERNEL); if (!ubuf) return -ENOMEM; -- 2.18.1

6 years, 3 months

1
0
0 0

[PATCH 2/3] udmabuf: check that __pad is zero

by Gerd Hoffmann

Reported-by: Yann Droneaud <ydroneaud(a)opteya.com> Signed-off-by: Gerd Hoffmann <kraxel(a)redhat.com> --- drivers/dma-buf/udmabuf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c index 9635897458a0..6c3ec8fcef01 100644 --- a/drivers/dma-buf/udmabuf.c +++ b/drivers/dma-buf/udmabuf.c @@ -137,6 +137,8 @@ static long udmabuf_create(const struct udmabuf_create_list *head, pglimit = (size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; for (i = 0; i < head->count; i++) { + if (list[i].__pad) + goto err; if (!IS_ALIGNED(list[i].offset, PAGE_SIZE)) goto err; if (!IS_ALIGNED(list[i].size, PAGE_SIZE)) -- 2.18.1

6 years, 3 months

1
0
0 0

[PATCH 1/3] udmabuf: add documentation

by Gerd Hoffmann

Signed-off-by: Gerd Hoffmann <kraxel(a)redhat.com> --- include/uapi/linux/udmabuf.h | 52 ++++++++++++++++++++++++++-- Documentation/driver-api/dma-buf.rst | 8 +++++ 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/include/uapi/linux/udmabuf.h b/include/uapi/linux/udmabuf.h index 46b6532ed855..9fe440abf2f9 100644 --- a/include/uapi/linux/udmabuf.h +++ b/include/uapi/linux/udmabuf.h @@ -5,8 +5,39 @@ #include <linux/types.h> #include <linux/ioctl.h> +/** + * DOC: udmabuf + * + * udmabuf is a device driver which allows userspace to create + * dmabufs. The memory used for these dmabufs must be backed by + * memfd. The memfd must have F_SEAL_SHRINK and it must not have + * F_SEAL_WRITE. + * + * The driver has two ioctls, one to create a dmabuf from a single + * memory block and one to create a dmabuf from a list of memory + * blocks. + * + * UDMABUF_CREATE - _IOW('u', 0x42, udmabuf_create) + * + * UDMABUF_CREATE_LIST - _IOW('u', 0x43, udmabuf_create_list) + */ + +#define UDMABUF_CREATE _IOW('u', 0x42, struct udmabuf_create) +#define UDMABUF_CREATE_LIST _IOW('u', 0x43, struct udmabuf_create_list) + #define UDMABUF_FLAGS_CLOEXEC 0x01 +/** + * struct udmabuf_create - create a dmabuf from a single memory block. + * + * @memfd: The file handle. + * @offset: Start of the buffer (from memfd start). + * Must be page aligned. + * @size: Size of the buffer. Must be rounded to page size. + * + * @flags: + * UDMABUF_FLAGS_CLOEXEC: set CLOEXEC flag for the dmabuf. + */ struct udmabuf_create { __u32 memfd; __u32 flags; @@ -14,6 +45,15 @@ struct udmabuf_create { __u64 size; }; +/** + * struct udmabuf_create_item - one memory block list item. + * + * @memfd: The file handle. + * @__pad: Padding field (unused). + * @offset: Start of the buffer (from memfd start). + * Must be page aligned. + * @size: Size of the buffer. Must be rounded to page size. + */ struct udmabuf_create_item { __u32 memfd; __u32 __pad; @@ -21,13 +61,19 @@ struct udmabuf_create_item { __u64 size; }; +/** + * struct udmabuf_create_list - create a dmabuf from a memory block list. + * + * @count: The number of list elements. + * @list: The memory block list + * + * @flags: + * UDMABUF_FLAGS_CLOEXEC: set CLOEXEC flag for the dmabuf. + */ struct udmabuf_create_list { __u32 flags; __u32 count; struct udmabuf_create_item list[]; }; -#define UDMABUF_CREATE _IOW('u', 0x42, struct udmabuf_create) -#define UDMABUF_CREATE_LIST _IOW('u', 0x43, struct udmabuf_create_list) - #endif /* _UAPI_LINUX_UDMABUF_H */ diff --git a/Documentation/driver-api/dma-buf.rst b/Documentation/driver-api/dma-buf.rst index b541e97c7ab1..1f62c30a14b0 100644 --- a/Documentation/driver-api/dma-buf.rst +++ b/Documentation/driver-api/dma-buf.rst @@ -166,3 +166,11 @@ DMA Fence uABI/Sync File .. kernel-doc:: include/linux/sync_file.h :internal: +Userspace DMA Buffer driver +~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. kernel-doc:: include/uapi/linux/udmabuf.h + :doc: udmabuf + +.. kernel-doc:: include/uapi/linux/udmabuf.h + :internal: -- 2.18.1

6 years, 3 months

1
0
0 0

Re: [Linaro-mm-sig] [PATCH 08/10] dma-buf/resv: replace shared fence with new fences container

by Chris Wilson

Quoting Christian König (2019-08-21 13:31:45) > @@ -117,17 +120,10 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data, > busy_check_writer(rcu_dereference(obj->base.resv->fence_excl)); > > /* Translate shared fences to READ set of engines */ > - list = rcu_dereference(obj->base.resv->fence); > - if (list) { > - unsigned int shared_count = list->shared_count, i; > - > - for (i = 0; i < shared_count; ++i) { > - struct dma_fence *fence = > - rcu_dereference(list->shared[i]); > - > - args->busy |= busy_check_reader(fence); > - } > - } > + readers = dma_resv_fences_get_rcu(&obj->base.resv->readers); > + dma_fence_array_for_each(fence, cursor, readers) > + args->busy |= busy_check_reader(fence); > + dma_fence_put(readers); That's underwhelming, the full-mb shows up in scaling tests (I'll test the impact of this series later). Something like, do { read = 0; fences = dma_resv_fences_get_deref(&obj->base.resv->readers); dma_fence_array_for_each(fence, cursor, fences) read |= busy_check_reader(fence); smp_rmb(); } while (dma_resv_fences_get_deref(obj->readers) != fences) do { fences = dma_resv_fences_get_deref(&obj->base.resv->fences); write = busy_check_writer(fences); smp_rmb(); } while (dma_resv_fences_get_deref(obj->writes) != fences) args->busy = write | read; Perhaps? -Chris

6 years, 3 months

2
3
0 0

Re: [Linaro-mm-sig] [PATCH 08/10] dma-buf/resv: replace shared fence with new fences container

by Chris Wilson

Quoting Christian König (2019-08-21 13:31:45) > @@ -528,20 +352,9 @@ void dma_resv_prune_fences(struct dma_resv *obj) > dma_fence_put(fence); > } > > - list = dma_resv_get_list(obj); > - if (!list) > - return; > - > - for (i = 0; i < list->shared_count; ++i) { > - fence = rcu_dereference_protected(list->shared[i], > - dma_resv_held(obj)); > - > - if (!dma_fence_is_signaled(fence)) > - continue; > - > - RCU_INIT_POINTER(list->shared[i], dma_fence_get_stub()); > - dma_fence_put(fence); > - } > + fence = dma_resv_fences_deref(obj, &obj->readers); > + if (dma_fence_is_signaled(fence)) > + dma_resv_fences_set(obj, &obj->readers, NULL); Needs fence==NULL safeguards. It's not as pruny as it was before. Or did you add some magic to fence-array? -Chris

6 years, 3 months

1
0
0 0

[PATCH 1/4] dma-buf: add reservation_object_fences helper

by Christian König

Add a new helper to get a consistent set of pointers from the reservation object. While at it group all access helpers together in the header file. v2: correctly return shared_count as well Signed-off-by: Christian König <christian.koenig(a)amd.com> --- drivers/dma-buf/dma-buf.c | 31 ++------- drivers/dma-buf/reservation.c | 82 ++++++++---------------- include/linux/reservation.h | 115 +++++++++++++++++++++------------- 3 files changed, 101 insertions(+), 127 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index f45bfb29ef96..67510f2be8bc 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -199,7 +199,7 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll) struct reservation_object_list *fobj; struct dma_fence *fence_excl; __poll_t events; - unsigned shared_count, seq; + unsigned shared_count; dmabuf = file->private_data; if (!dmabuf || !dmabuf->resv) @@ -213,21 +213,8 @@ static __poll_t dma_buf_poll(struct file *file, poll_table *poll) if (!events) return 0; -retry: - seq = read_seqcount_begin(&resv->seq); rcu_read_lock(); - - fobj = rcu_dereference(resv->fence); - if (fobj) - shared_count = fobj->shared_count; - else - shared_count = 0; - fence_excl = rcu_dereference(resv->fence_excl); - if (read_seqcount_retry(&resv->seq, seq)) { - rcu_read_unlock(); - goto retry; - } - + reservation_object_fences(resv, &fence_excl, &fobj, &shared_count); if (fence_excl && (!(events & EPOLLOUT) || shared_count == 0)) { struct dma_buf_poll_cb_t *dcb = &dmabuf->cb_excl; __poll_t pevents = EPOLLIN; @@ -1157,7 +1144,6 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused) struct reservation_object *robj; struct reservation_object_list *fobj; struct dma_fence *fence; - unsigned seq; int count = 0, attach_count, shared_count, i; size_t size = 0; @@ -1188,16 +1174,9 @@ static int dma_buf_debug_show(struct seq_file *s, void *unused) buf_obj->name ?: ""); robj = buf_obj->resv; - while (true) { - seq = read_seqcount_begin(&robj->seq); - rcu_read_lock(); - fobj = rcu_dereference(robj->fence); - shared_count = fobj ? fobj->shared_count : 0; - fence = rcu_dereference(robj->fence_excl); - if (!read_seqcount_retry(&robj->seq, seq)) - break; - rcu_read_unlock(); - } + rcu_read_lock(); + reservation_object_fences(robj, &fence, &fobj, &shared_count); + rcu_read_unlock(); if (fence) seq_printf(s, "\tExclusive fence: %s %s %ssignalled\n", diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index ad6775b32a73..8fcaddffd5d4 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -317,17 +317,15 @@ int reservation_object_copy_fences(struct reservation_object *dst, { struct reservation_object_list *src_list, *dst_list; struct dma_fence *old, *new; - unsigned i; + unsigned int i, shared_count; reservation_object_assert_held(dst); rcu_read_lock(); - src_list = rcu_dereference(src->fence); retry: - if (src_list) { - unsigned shared_count = src_list->shared_count; - + reservation_object_fences(src, &new, &src_list, &shared_count); + if (shared_count) { rcu_read_unlock(); dst_list = reservation_object_list_alloc(shared_count); @@ -335,14 +333,14 @@ int reservation_object_copy_fences(struct reservation_object *dst, return -ENOMEM; rcu_read_lock(); - src_list = rcu_dereference(src->fence); - if (!src_list || src_list->shared_count > shared_count) { + reservation_object_fences(src, &new, &src_list, &shared_count); + if (!src_list || shared_count > dst_list->shared_max) { kfree(dst_list); goto retry; } dst_list->shared_count = 0; - for (i = 0; i < src_list->shared_count; ++i) { + for (i = 0; i < shared_count; ++i) { struct dma_fence *fence; fence = rcu_dereference(src_list->shared[i]); @@ -352,7 +350,6 @@ int reservation_object_copy_fences(struct reservation_object *dst, if (!dma_fence_get_rcu(fence)) { reservation_object_list_free(dst_list); - src_list = rcu_dereference(src->fence); goto retry; } @@ -367,7 +364,10 @@ int reservation_object_copy_fences(struct reservation_object *dst, dst_list = NULL; } - new = dma_fence_get_rcu_safe(&src->fence_excl); + if (new && !dma_fence_get_rcu(new)) { + reservation_object_list_free(dst_list); + goto retry; + } rcu_read_unlock(); src_list = reservation_object_get_list(dst); @@ -413,19 +413,18 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj, do { struct reservation_object_list *fobj; - unsigned int i, seq; + unsigned int i; size_t sz = 0; - shared_count = i = 0; + i = 0; rcu_read_lock(); - seq = read_seqcount_begin(&obj->seq); + reservation_object_fences(obj, &fence_excl, &fobj, + &shared_count); - fence_excl = rcu_dereference(obj->fence_excl); if (fence_excl && !dma_fence_get_rcu(fence_excl)) goto unlock; - fobj = rcu_dereference(obj->fence); if (fobj) sz += sizeof(*shared) * fobj->shared_max; @@ -453,7 +452,6 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj, break; } shared = nshared; - shared_count = fobj ? fobj->shared_count : 0; for (i = 0; i < shared_count; ++i) { shared[i] = rcu_dereference(fobj->shared[i]); if (!dma_fence_get_rcu(shared[i])) @@ -461,7 +459,7 @@ int reservation_object_get_fences_rcu(struct reservation_object *obj, } } - if (i != shared_count || read_seqcount_retry(&obj->seq, seq)) { + if (i != shared_count) { while (i--) dma_fence_put(shared[i]); dma_fence_put(fence_excl); @@ -505,18 +503,17 @@ long reservation_object_wait_timeout_rcu(struct reservation_object *obj, bool wait_all, bool intr, unsigned long timeout) { + struct reservation_object_list *fobj; struct dma_fence *fence; - unsigned seq, shared_count; + unsigned shared_count; long ret = timeout ? timeout : 1; int i; retry: - shared_count = 0; - seq = read_seqcount_begin(&obj->seq); rcu_read_lock(); i = -1; - fence = rcu_dereference(obj->fence_excl); + reservation_object_fences(obj, &fence, &fobj, &shared_count); if (fence && !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) { if (!dma_fence_get_rcu(fence)) goto unlock_retry; @@ -531,12 +528,6 @@ long reservation_object_wait_timeout_rcu(struct reservation_object *obj, } if (wait_all) { - struct reservation_object_list *fobj = - rcu_dereference(obj->fence); - - if (fobj) - shared_count = fobj->shared_count; - for (i = 0; !fence && i < shared_count; ++i) { struct dma_fence *lfence = rcu_dereference(fobj->shared[i]); @@ -559,11 +550,6 @@ long reservation_object_wait_timeout_rcu(struct reservation_object *obj, rcu_read_unlock(); if (fence) { - if (read_seqcount_retry(&obj->seq, seq)) { - dma_fence_put(fence); - goto retry; - } - ret = dma_fence_wait_timeout(fence, intr, ret); dma_fence_put(fence); if (ret > 0 && wait_all && (i + 1 < shared_count)) @@ -608,24 +594,19 @@ reservation_object_test_signaled_single(struct dma_fence *passed_fence) bool reservation_object_test_signaled_rcu(struct reservation_object *obj, bool test_all) { - unsigned seq, shared_count; + struct reservation_object_list *fobj; + struct dma_fence *fence_excl; + unsigned shared_count; int ret; rcu_read_lock(); retry: ret = true; - shared_count = 0; - seq = read_seqcount_begin(&obj->seq); + reservation_object_fences(obj, &fence_excl, &fobj, &shared_count); if (test_all) { unsigned i; - struct reservation_object_list *fobj = - rcu_dereference(obj->fence); - - if (fobj) - shared_count = fobj->shared_count; - for (i = 0; i < shared_count; ++i) { struct dma_fence *fence = rcu_dereference(fobj->shared[i]); @@ -635,23 +616,12 @@ bool reservation_object_test_signaled_rcu(struct reservation_object *obj, else if (!ret) break; } - - if (read_seqcount_retry(&obj->seq, seq)) - goto retry; } - if (!shared_count) { - struct dma_fence *fence_excl = rcu_dereference(obj->fence_excl); - - if (fence_excl) { - ret = reservation_object_test_signaled_single( - fence_excl); - if (ret < 0) - goto retry; - - if (read_seqcount_retry(&obj->seq, seq)) - goto retry; - } + if (!shared_count && fence_excl) { + ret = reservation_object_test_signaled_single(fence_excl); + if (ret < 0) + goto retry; } rcu_read_unlock(); diff --git a/include/linux/reservation.h b/include/linux/reservation.h index 56b782fec49b..044a5cd4af50 100644 --- a/include/linux/reservation.h +++ b/include/linux/reservation.h @@ -81,6 +81,51 @@ struct reservation_object { #define reservation_object_assert_held(obj) \ lockdep_assert_held(&(obj)->lock.base) +/** + * reservation_object_get_excl - get the reservation object's + * exclusive fence, with update-side lock held + * @obj: the reservation object + * + * Returns the exclusive fence (if any). Does NOT take a + * reference. Writers must hold obj->lock, readers may only + * hold a RCU read side lock. + * + * RETURNS + * The exclusive fence or NULL + */ +static inline struct dma_fence * +reservation_object_get_excl(struct reservation_object *obj) +{ + return rcu_dereference_protected(obj->fence_excl, + reservation_object_held(obj)); +} + +/** + * reservation_object_get_excl_rcu - get the reservation object's + * exclusive fence, without lock held. + * @obj: the reservation object + * + * If there is an exclusive fence, this atomically increments it's + * reference count and returns it. + * + * RETURNS + * The exclusive fence or NULL if none + */ +static inline struct dma_fence * +reservation_object_get_excl_rcu(struct reservation_object *obj) +{ + struct dma_fence *fence; + + if (!rcu_access_pointer(obj->fence_excl)) + return NULL; + + rcu_read_lock(); + fence = dma_fence_get_rcu_safe(&obj->fence_excl); + rcu_read_unlock(); + + return fence; +} + /** * reservation_object_get_list - get the reservation object's * shared fence list, with update-side lock held @@ -96,6 +141,31 @@ reservation_object_get_list(struct reservation_object *obj) reservation_object_held(obj)); } +/** + * reservation_object_fences - read consistent fence pointers + * @obj: reservation object where we get the fences from + * @excl: pointer for the exclusive fence + * @list: pointer for the shared fence list + * + * Make sure we have a consisten exclusive fence and shared fence list. + * Must be called with rcu read side lock held. + */ +static inline void +reservation_object_fences(struct reservation_object *obj, + struct dma_fence **excl, + struct reservation_object_list **list, + u32 *shared_count) +{ + unsigned int seq; + + do { + seq = read_seqcount_begin(&obj->seq); + *excl = rcu_dereference(obj->fence_excl); + *list = rcu_dereference(obj->fence); + *shared_count = *list ? (*list)->shared_count : 0; + } while (read_seqcount_retry(&obj->seq, seq)); +} + /** * reservation_object_lock - lock the reservation object * @obj: the reservation object @@ -239,51 +309,6 @@ reservation_object_unlock(struct reservation_object *obj) ww_mutex_unlock(&obj->lock); } -/** - * reservation_object_get_excl - get the reservation object's - * exclusive fence, with update-side lock held - * @obj: the reservation object - * - * Returns the exclusive fence (if any). Does NOT take a - * reference. Writers must hold obj->lock, readers may only - * hold a RCU read side lock. - * - * RETURNS - * The exclusive fence or NULL - */ -static inline struct dma_fence * -reservation_object_get_excl(struct reservation_object *obj) -{ - return rcu_dereference_protected(obj->fence_excl, - reservation_object_held(obj)); -} - -/** - * reservation_object_get_excl_rcu - get the reservation object's - * exclusive fence, without lock held. - * @obj: the reservation object - * - * If there is an exclusive fence, this atomically increments it's - * reference count and returns it. - * - * RETURNS - * The exclusive fence or NULL if none - */ -static inline struct dma_fence * -reservation_object_get_excl_rcu(struct reservation_object *obj) -{ - struct dma_fence *fence; - - if (!rcu_access_pointer(obj->fence_excl)) - return NULL; - - rcu_read_lock(); - fence = dma_fence_get_rcu_safe(&obj->fence_excl); - rcu_read_unlock(); - - return fence; -} - void reservation_object_init(struct reservation_object *obj); void reservation_object_fini(struct reservation_object *obj); int reservation_object_reserve_shared(struct reservation_object *obj, -- 2.17.1

6 years, 4 months

4
19
0 0

[PATCH] dma-buf: make dma_fence structure a bit smaller v2

by Christian König

We clear the callback list on kref_put so that by the time we release the fence it is unused. No one should be adding to the cb_list that they don't themselves hold a reference for. This small change is actually making the structure 16% smaller. v2: add the comment to the code as well. Signed-off-by: Christian König <christian.koenig(a)amd.com> Reviewed-by: Chris Wilson <chris(a)chris-wilson.co.uk> --- include/linux/dma-fence.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 05d29dbc7e62..bea1d05cf51e 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -65,8 +65,14 @@ struct dma_fence_cb; struct dma_fence { struct kref refcount; const struct dma_fence_ops *ops; - struct rcu_head rcu; - struct list_head cb_list; + /* We clear the callback list on kref_put so that by the time we + * release the fence it is unused. No one should be adding to the cb_list + * that they don't themselves hold a reference for. + */ + union { + struct rcu_head rcu; + struct list_head cb_list; + }; spinlock_t *lock; u64 context; u64 seqno; -- 2.17.1

6 years, 4 months

2
2
0 0

[PATCH] dma-buf: make dma_fence structure a bit smaller

by Christian König

We clear the callback list on kref_put so that by the time we release the fence it is unused. No one should be adding to the cb_list that they don't themselves hold a reference for. This small change is actually making the structure 16% smaller. Signed-off-by: Christian König <christian.koenig(a)amd.com> Reviewed-by: Chris Wilson <chris(a)chris-wilson.co.uk> --- include/linux/dma-fence.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 05d29dbc7e62..3985c72cd0c2 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -65,8 +65,10 @@ struct dma_fence_cb; struct dma_fence { struct kref refcount; const struct dma_fence_ops *ops; - struct rcu_head rcu; - struct list_head cb_list; + union { + struct rcu_head rcu; + struct list_head cb_list; + }; spinlock_t *lock; u64 context; u64 seqno; -- 2.17.1

6 years, 4 months

1
0
0 0

[PATCH] dma-buf: make dma_fence structure a bit smaller

by Christian König

The ruc and cb_list are never used at the same time. This smal change is actually making the structure 16% smaller. Signed-off-by: Christian König <christian.koenig(a)amd.com> --- include/linux/dma-fence.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 05d29dbc7e62..3985c72cd0c2 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -65,8 +65,10 @@ struct dma_fence_cb; struct dma_fence { struct kref refcount; const struct dma_fence_ops *ops; - struct rcu_head rcu; - struct list_head cb_list; + union { + struct rcu_head rcu; + struct list_head cb_list; + }; spinlock_t *lock; u64 context; u64 seqno; -- 2.17.1

6 years, 4 months

3
2
0 0

[PATCH 1/8] dma-buf: fix busy wait for new shared fences

by Christian König

When reservation_object_add_shared_fence is replacing an old fence with a new one we should not drop the old one before the new one is in place. Otherwise other cores can busy wait for the new one to appear. Signed-off-by: Christian König <christian.koenig(a)amd.com> --- drivers/dma-buf/reservation.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c index c71b85c8c159..d59207ca72d2 100644 --- a/drivers/dma-buf/reservation.c +++ b/drivers/dma-buf/reservation.c @@ -196,6 +196,7 @@ void reservation_object_add_shared_fence(struct reservation_object *obj, struct dma_fence *fence) { struct reservation_object_list *fobj; + struct dma_fence *old; unsigned int i, count; dma_fence_get(fence); @@ -209,18 +210,16 @@ void reservation_object_add_shared_fence(struct reservation_object *obj, write_seqcount_begin(&obj->seq); for (i = 0; i < count; ++i) { - struct dma_fence *old_fence; - old_fence = rcu_dereference_protected(fobj->shared[i], - reservation_object_held(obj)); - if (old_fence->context == fence->context || - dma_fence_is_signaled(old_fence)) { - dma_fence_put(old_fence); + old = rcu_dereference_protected(fobj->shared[i], + reservation_object_held(obj)); + if (old->context == fence->context || + dma_fence_is_signaled(old)) goto replace; - } } BUG_ON(fobj->shared_count >= fobj->shared_max); + old = NULL; count++; replace: @@ -230,6 +229,7 @@ void reservation_object_add_shared_fence(struct reservation_object *obj, write_seqcount_end(&obj->seq); preempt_enable(); + dma_fence_put(old); } EXPORT_SYMBOL(reservation_object_add_shared_fence); -- 2.17.1

6 years, 4 months

3
20
0 0

[PATCH 1/5] drm/i915: stop pruning reservation object after wait

by Christian König

The reservation object should be capable of handling its internal memory management itself. And since we search for a free slot to add the fence from the beginning this is actually a waste of time and only minimal helpful. Drop it to allow removal of the seqno handling in the reservation object. This essentially reverts commit "drm/i915: Remove completed fences after a wait". Signed-off-by: Christian König <christian.koenig(a)amd.com> --- drivers/gpu/drm/i915/gem/i915_gem_wait.c | 27 ------------------------ 1 file changed, 27 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_wait.c b/drivers/gpu/drm/i915/gem/i915_gem_wait.c index 26ec6579b7cd..bb64ec6bef8e 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_wait.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_wait.c @@ -35,9 +35,7 @@ i915_gem_object_wait_reservation(struct reservation_object *resv, unsigned int flags, long timeout) { - unsigned int seq = __read_seqcount_begin(&resv->seq); struct dma_fence *excl; - bool prune_fences = false; if (flags & I915_WAIT_ALL) { struct dma_fence **shared; @@ -61,17 +59,6 @@ i915_gem_object_wait_reservation(struct reservation_object *resv, for (; i < count; i++) dma_fence_put(shared[i]); kfree(shared); - - /* - * If both shared fences and an exclusive fence exist, - * then by construction the shared fences must be later - * than the exclusive fence. If we successfully wait for - * all the shared fences, we know that the exclusive fence - * must all be signaled. If all the shared fences are - * signaled, we can prune the array and recover the - * floating references on the fences/requests. - */ - prune_fences = count && timeout >= 0; } else { excl = reservation_object_get_excl_rcu(resv); } @@ -80,20 +67,6 @@ i915_gem_object_wait_reservation(struct reservation_object *resv, timeout = i915_gem_object_wait_fence(excl, flags, timeout); dma_fence_put(excl); - - /* - * Opportunistically prune the fences iff we know they have *all* been - * signaled and that the reservation object has not been changed (i.e. - * no new fences have been added). - */ - if (prune_fences && !__read_seqcount_retry(&resv->seq, seq)) { - if (reservation_object_trylock(resv)) { - if (!__read_seqcount_retry(&resv->seq, seq)) - reservation_object_add_excl_fence(resv, NULL); - reservation_object_unlock(resv); - } - } - return timeout; } -- 2.17.1

6 years, 4 months

3
9
0 0

[PATCH] dma-buf: add more reservation object locking wrappers

by Christian König

Complete the abstraction of the ww_mutex inside the reservation object. This allows us to add more handling and debugging to the reservation object in the future. Signed-off-by: Christian König <christian.koenig(a)amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 6 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 6 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 3 +- drivers/gpu/drm/drm_gem.c | 14 ++--- drivers/gpu/drm/nouveau/nouveau_prime.c | 4 +- drivers/gpu/drm/radeon/radeon_object.c | 6 +-- drivers/gpu/drm/radeon/radeon_prime.c | 4 +- drivers/gpu/drm/ttm/ttm_bo.c | 10 ++-- drivers/gpu/drm/ttm/ttm_execbuf_util.c | 6 +-- drivers/gpu/drm/vmwgfx/vmwgfx_blit.c | 4 +- drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c | 4 +- drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 6 +-- include/drm/ttm/ttm_bo_driver.h | 6 +-- include/linux/reservation.h | 57 +++++++++++++++++++++ 16 files changed, 100 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 4b1e4b321999..f486c23fa24c 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -1725,7 +1725,7 @@ int amdgpu_cs_find_mapping(struct amdgpu_cs_parser *parser, *map = mapping; /* Double check that the BO is reserved by this CS */ - if (READ_ONCE((*bo)->tbo.resv->lock.ctx) != &parser->ticket) + if (reservation_object_locking_ctx((*bo)->tbo.resv) != &parser->ticket) return -EINVAL; if (!((*bo)->flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS)) { diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c index 6770eb3967a6..e1d2c410bc33 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c @@ -398,7 +398,7 @@ amdgpu_gem_prime_import_sg_table(struct drm_device *dev, bp.flags = 0; bp.type = ttm_bo_type_sg; bp.resv = resv; - ww_mutex_lock(&resv->lock, NULL); + reservation_object_lock(resv, NULL); ret = amdgpu_bo_create(adev, &bp, &bo); if (ret) goto error; @@ -410,11 +410,11 @@ amdgpu_gem_prime_import_sg_table(struct drm_device *dev, if (attach->dmabuf->ops != &amdgpu_dmabuf_ops) bo->prime_shared_count = 1; - ww_mutex_unlock(&resv->lock); + reservation_object_unlock(resv); return &bo->gem_base; error: - ww_mutex_unlock(&resv->lock); + reservation_object_unlock(resv); return ERR_PTR(ret); } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 87020628a66e..c31e96925892 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -553,7 +553,7 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev, fail_unreserve: if (!bp->resv) - ww_mutex_unlock(&bo->tbo.resv->lock); + reservation_object_unlock(bo->tbo.resv); amdgpu_bo_unref(&bo); return r; } @@ -1096,7 +1096,7 @@ int amdgpu_bo_set_tiling_flags(struct amdgpu_bo *bo, u64 tiling_flags) */ void amdgpu_bo_get_tiling_flags(struct amdgpu_bo *bo, u64 *tiling_flags) { - lockdep_assert_held(&bo->tbo.resv->lock.base); + reservation_object_assert_held(bo->tbo.resv); if (tiling_flags) *tiling_flags = bo->tiling_flags; @@ -1337,7 +1337,7 @@ int amdgpu_bo_sync_wait(struct amdgpu_bo *bo, void *owner, bool intr) u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo) { WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_SYSTEM); - WARN_ON_ONCE(!ww_mutex_is_locked(&bo->tbo.resv->lock) && + WARN_ON_ONCE(!reservation_object_is_locked(bo->tbo.resv) && !bo->pin_count && bo->tbo.type != ttm_bo_type_kernel); WARN_ON_ONCE(bo->tbo.mem.start == AMDGPU_BO_INVALID_OFFSET); WARN_ON_ONCE(bo->tbo.mem.mem_type == TTM_PL_VRAM && diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index c1baf3d879b7..e9df0b10a37e 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -2416,7 +2416,8 @@ void amdgpu_vm_bo_trace_cs(struct amdgpu_vm *vm, struct ww_acquire_ctx *ticket) struct amdgpu_bo *bo; bo = mapping->bo_va->base.bo; - if (READ_ONCE(bo->tbo.resv->lock.ctx) != ticket) + if (reservation_object_locking_ctx(bo->tbo.resv) != + ticket) continue; } diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 50de138c89e0..04c0307ca54f 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -1316,8 +1316,8 @@ drm_gem_lock_reservations(struct drm_gem_object **objs, int count, if (contended != -1) { struct drm_gem_object *obj = objs[contended]; - ret = ww_mutex_lock_slow_interruptible(&obj->resv->lock, - acquire_ctx); + ret = reservation_object_lock_slow_interruptible(obj->resv, + acquire_ctx); if (ret) { ww_acquire_done(acquire_ctx); return ret; @@ -1328,16 +1328,16 @@ drm_gem_lock_reservations(struct drm_gem_object **objs, int count, if (i == contended) continue; - ret = ww_mutex_lock_interruptible(&objs[i]->resv->lock, - acquire_ctx); + ret = reservation_object_lock_interruptible(objs[i]->resv, + acquire_ctx); if (ret) { int j; for (j = 0; j < i; j++) - ww_mutex_unlock(&objs[j]->resv->lock); + reservation_object_unlock(objs[j]->resv); if (contended != -1 && contended >= i) - ww_mutex_unlock(&objs[contended]->resv->lock); + reservation_object_unlock(objs[contended]->resv); if (ret == -EDEADLK) { contended = i; @@ -1362,7 +1362,7 @@ drm_gem_unlock_reservations(struct drm_gem_object **objs, int count, int i; for (i = 0; i < count; i++) - ww_mutex_unlock(&objs[i]->resv->lock); + reservation_object_unlock(objs[i]->resv); ww_acquire_fini(acquire_ctx); } diff --git a/drivers/gpu/drm/nouveau/nouveau_prime.c b/drivers/gpu/drm/nouveau/nouveau_prime.c index 1fefc93af1d7..38abdfa96af7 100644 --- a/drivers/gpu/drm/nouveau/nouveau_prime.c +++ b/drivers/gpu/drm/nouveau/nouveau_prime.c @@ -68,10 +68,10 @@ struct drm_gem_object *nouveau_gem_prime_import_sg_table(struct drm_device *dev, flags = TTM_PL_FLAG_TT; - ww_mutex_lock(&robj->lock, NULL); + reservation_object_lock(robj, NULL); ret = nouveau_bo_new(&drm->client, attach->dmabuf->size, 0, flags, 0, 0, sg, robj, &nvbo); - ww_mutex_unlock(&robj->lock); + reservation_object_unlock(robj); if (ret) return ERR_PTR(ret); diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c index 36683de0300b..3210b1e090f8 100644 --- a/drivers/gpu/drm/radeon/radeon_object.c +++ b/drivers/gpu/drm/radeon/radeon_object.c @@ -607,7 +607,7 @@ int radeon_bo_get_surface_reg(struct radeon_bo *bo) int steal; int i; - lockdep_assert_held(&bo->tbo.resv->lock.base); + reservation_object_assert_held(bo->tbo.resv); if (!bo->tiling_flags) return 0; @@ -733,7 +733,7 @@ void radeon_bo_get_tiling_flags(struct radeon_bo *bo, uint32_t *tiling_flags, uint32_t *pitch) { - lockdep_assert_held(&bo->tbo.resv->lock.base); + reservation_object_assert_held(bo->tbo.resv); if (tiling_flags) *tiling_flags = bo->tiling_flags; @@ -745,7 +745,7 @@ int radeon_bo_check_tiling(struct radeon_bo *bo, bool has_moved, bool force_drop) { if (!force_drop) - lockdep_assert_held(&bo->tbo.resv->lock.base); + reservation_object_assert_held(bo->tbo.resv); if (!(bo->tiling_flags & RADEON_TILING_SURFACE)) return 0; diff --git a/drivers/gpu/drm/radeon/radeon_prime.c b/drivers/gpu/drm/radeon/radeon_prime.c index 7110d403322c..f48bc87d0e4b 100644 --- a/drivers/gpu/drm/radeon/radeon_prime.c +++ b/drivers/gpu/drm/radeon/radeon_prime.c @@ -66,10 +66,10 @@ struct drm_gem_object *radeon_gem_prime_import_sg_table(struct drm_device *dev, struct radeon_bo *bo; int ret; - ww_mutex_lock(&resv->lock, NULL); + reservation_object_lock(resv, NULL); ret = radeon_bo_create(rdev, attach->dmabuf->size, PAGE_SIZE, false, RADEON_GEM_DOMAIN_GTT, 0, sg, resv, &bo); - ww_mutex_unlock(&resv->lock); + reservation_object_unlock(resv); if (ret) return ERR_PTR(ret); diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 58c403eda04e..40d3e547c78e 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -850,8 +850,8 @@ static int ttm_mem_evict_first(struct ttm_bo_device *bdev, if (!ttm_bo_evict_swapout_allowable(bo, ctx, &locked, &busy)) { - if (busy && !busy_bo && - bo->resv->lock.ctx != ticket) + if (busy && !busy_bo && ticket != + reservation_object_locking_ctx(bo->resv)) busy_bo = bo; continue; } @@ -957,8 +957,10 @@ static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo, { struct ttm_bo_device *bdev = bo->bdev; struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type]; + struct ww_acquire_ctx *ticket; int ret; + ticket = reservation_object_locking_ctx(bo->resv); do { ret = (*man->func->get_node)(man, bo, place, mem); if (unlikely(ret != 0)) @@ -966,7 +968,7 @@ static int ttm_bo_mem_force_space(struct ttm_buffer_object *bo, if (mem->mm_node) break; ret = ttm_mem_evict_first(bdev, mem->mem_type, place, ctx, - bo->resv->lock.ctx); + ticket); if (unlikely(ret != 0)) return ret; } while (1); @@ -1963,7 +1965,7 @@ int ttm_bo_wait_unreserved(struct ttm_buffer_object *bo) ret = mutex_lock_interruptible(&bo->wu_mutex); if (unlikely(ret != 0)) return -ERESTARTSYS; - if (!ww_mutex_is_locked(&bo->resv->lock)) + if (!reservation_object_is_locked(bo->resv)) goto out_unlock; ret = reservation_object_lock_interruptible(bo->resv, NULL); if (ret == -EINTR) diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c b/drivers/gpu/drm/ttm/ttm_execbuf_util.c index 957ec375a4ba..723fb583fdda 100644 --- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c +++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c @@ -144,10 +144,10 @@ int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket, if (ret == -EDEADLK) { if (intr) { - ret = ww_mutex_lock_slow_interruptible(&bo->resv->lock, - ticket); + ret = reservation_object_lock_slow_interruptible(bo->resv, + ticket); } else { - ww_mutex_lock_slow(&bo->resv->lock, ticket); + reservation_object_lock_slow(bo->resv, ticket); ret = 0; } } diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c index fc6673cde289..703786e3d579 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c @@ -459,9 +459,9 @@ int vmw_bo_cpu_blit(struct ttm_buffer_object *dst, /* Buffer objects need to be either pinned or reserved: */ if (!(dst->mem.placement & TTM_PL_FLAG_NO_EVICT)) - lockdep_assert_held(&dst->resv->lock.base); + reservation_object_assert_held(dst->resv); if (!(src->mem.placement & TTM_PL_FLAG_NO_EVICT)) - lockdep_assert_held(&src->resv->lock.base); + reservation_object_assert_held(src->resv); if (dst->ttm->state == tt_unpopulated) { ret = dst->ttm->bdev->driver->ttm_tt_populate(dst->ttm, &ctx); diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c index 5d5c2bce01f3..315da41a18b4 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c @@ -342,7 +342,7 @@ void vmw_bo_pin_reserved(struct vmw_buffer_object *vbo, bool pin) uint32_t old_mem_type = bo->mem.mem_type; int ret; - lockdep_assert_held(&bo->resv->lock.base); + reservation_object_assert_held(bo->resv); if (pin) { if (vbo->pin_count++ > 0) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c b/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c index b4f6e1217c9d..71e901bbed68 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c @@ -169,7 +169,7 @@ static int vmw_cotable_unscrub(struct vmw_resource *res) } *cmd; WARN_ON_ONCE(bo->mem.mem_type != VMW_PL_MOB); - lockdep_assert_held(&bo->resv->lock.base); + reservation_object_assert_held(bo->resv); cmd = VMW_FIFO_RESERVE(dev_priv, sizeof(*cmd)); if (!cmd) @@ -311,7 +311,7 @@ static int vmw_cotable_unbind(struct vmw_resource *res, return 0; WARN_ON_ONCE(bo->mem.mem_type != VMW_PL_MOB); - lockdep_assert_held(&bo->resv->lock.base); + reservation_object_assert_held(bo->resv); mutex_lock(&dev_priv->binding_mutex); if (!vcotbl->scrubbed) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c index 1d38a8b2f2ec..303d2c7d9ab3 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c @@ -402,14 +402,14 @@ void vmw_resource_unreserve(struct vmw_resource *res, if (switch_backup && new_backup != res->backup) { if (res->backup) { - lockdep_assert_held(&res->backup->base.resv->lock.base); + reservation_object_assert_held(res->backup->base.resv); list_del_init(&res->mob_head); vmw_bo_unreference(&res->backup); } if (new_backup) { res->backup = vmw_bo_reference(new_backup); - lockdep_assert_held(&new_backup->base.resv->lock.base); + reservation_object_assert_held(new_backup->base.resv); list_add_tail(&res->mob_head, &new_backup->res_list); } else { res->backup = NULL; @@ -691,7 +691,7 @@ void vmw_resource_unbind_list(struct vmw_buffer_object *vbo) .num_shared = 0 }; - lockdep_assert_held(&vbo->base.resv->lock.base); + reservation_object_assert_held(vbo->base.resv); list_for_each_entry_safe(res, next, &vbo->res_list, mob_head) { if (!res->func->unbind) continue; diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h index c9b8ba492f24..0e6a111bed0b 100644 --- a/include/drm/ttm/ttm_bo_driver.h +++ b/include/drm/ttm/ttm_bo_driver.h @@ -745,10 +745,10 @@ static inline int ttm_bo_reserve_slowpath(struct ttm_buffer_object *bo, WARN_ON(!kref_read(&bo->kref)); if (interruptible) - ret = ww_mutex_lock_slow_interruptible(&bo->resv->lock, - ticket); + ret = reservation_object_lock_slow_interruptible(bo->resv, + ticket); else - ww_mutex_lock_slow(&bo->resv->lock, ticket); + reservation_object_lock_slow(bo->resv, ticket); if (likely(ret == 0)) ttm_bo_del_sub_from_lru(bo); diff --git a/include/linux/reservation.h b/include/linux/reservation.h index f47e8196d039..d19de7a6af71 100644 --- a/include/linux/reservation.h +++ b/include/linux/reservation.h @@ -140,6 +140,38 @@ reservation_object_lock_interruptible(struct reservation_object *obj, return ww_mutex_lock_interruptible(&obj->lock, ctx); } +/** + * reservation_object_lock_slow - slowpath lock the reservation object + * @obj: the reservation object + * @ctx: the locking context + * + * Acquires the reservation object after a die case. This function + * will sleep until the lock becomes available. See reservation_object_lock() as + * well. + */ +static inline void +reservation_object_lock_slow(struct reservation_object *obj, + struct ww_acquire_ctx *ctx) +{ + ww_mutex_lock_slow(&obj->lock, ctx); +} + +/** + * reservation_object_lock_slow_interruptible - slowpath lock the reservation + * object, interruptible + * @obj: the reservation object + * @ctx: the locking context + * + * Acquires the reservation object interruptible after a die case. This function + * will sleep until the lock becomes available. See + * reservation_object_lock_interruptible() as well. + */ +static inline int +reservation_object_lock_slow_interruptible(struct reservation_object *obj, + struct ww_acquire_ctx *ctx) +{ + return ww_mutex_lock_slow_interruptible(&obj->lock, ctx); +} /** * reservation_object_trylock - trylock the reservation object @@ -161,6 +193,31 @@ reservation_object_trylock(struct reservation_object *obj) return ww_mutex_trylock(&obj->lock); } +/** + * reservation_object_is_locked - is the reservation object locked + * @obj: the reservation object + * + * Returns true if the mutex is locked, false if unlocked. + */ +static inline bool +reservation_object_is_locked(struct reservation_object *obj) +{ + return ww_mutex_is_locked(&obj->lock); +} + +/** + * reservation_object_locking_ctx - returns the context used to lock the object + * @obj: the reservation object + * + * Returns the context used to lock a reservation object or NULL if no context + * was used or the object is not locked at all. + */ +static inline struct ww_acquire_ctx * +reservation_object_locking_ctx(struct reservation_object *obj) +{ + return READ_ONCE(obj->lock.ctx); +} + /** * reservation_object_unlock - unlock the reservation object * @obj: the reservation object -- 2.17.1

6 years, 4 months

3
4
0 0

[PATCH 4.14 205/293] dma-buf: balance refcount inbalance

by Greg Kroah-Hartman

From: Jérôme Glisse <jglisse(a)redhat.com> commit 5e383a9798990c69fc759a4930de224bb497e62c upstream. The debugfs take reference on fence without dropping them. Signed-off-by: Jérôme Glisse <jglisse(a)redhat.com> Cc: Christian König <christian.koenig(a)amd.com> Cc: Daniel Vetter <daniel.vetter(a)ffwll.ch> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: linux-media(a)vger.kernel.org Cc: dri-devel(a)lists.freedesktop.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: Stéphane Marchesin <marcheu(a)chromium.org> Cc: stable(a)vger.kernel.org Reviewed-by: Christian König <christian.koenig(a)amd.com> Signed-off-by: Sumit Semwal <sumit.semwal(a)linaro.org> Link: https://patchwork.freedesktop.org/patch/msgid/20181206161840.6578-1-jglisse… Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- drivers/dma-buf/dma-buf.c | 1 + 1 file changed, 1 insertion(+) --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1115,6 +1115,7 @@ static int dma_buf_debug_show(struct seq fence->ops->get_driver_name(fence), fence->ops->get_timeline_name(fence), dma_fence_is_signaled(fence) ? "" : "un"); + dma_fence_put(fence); } rcu_read_unlock();

6 years, 4 months

1
0
0 0

[PATCH 4.19 36/50] dma-buf: balance refcount inbalance

by Greg Kroah-Hartman

From: Jérôme Glisse <jglisse(a)redhat.com> commit 5e383a9798990c69fc759a4930de224bb497e62c upstream. The debugfs take reference on fence without dropping them. Signed-off-by: Jérôme Glisse <jglisse(a)redhat.com> Cc: Christian König <christian.koenig(a)amd.com> Cc: Daniel Vetter <daniel.vetter(a)ffwll.ch> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: linux-media(a)vger.kernel.org Cc: dri-devel(a)lists.freedesktop.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: Stéphane Marchesin <marcheu(a)chromium.org> Cc: stable(a)vger.kernel.org Reviewed-by: Christian König <christian.koenig(a)amd.com> Signed-off-by: Sumit Semwal <sumit.semwal(a)linaro.org> Link: https://patchwork.freedesktop.org/patch/msgid/20181206161840.6578-1-jglisse… Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- drivers/dma-buf/dma-buf.c | 1 + 1 file changed, 1 insertion(+) --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1069,6 +1069,7 @@ static int dma_buf_debug_show(struct seq fence->ops->get_driver_name(fence), fence->ops->get_timeline_name(fence), dma_fence_is_signaled(fence) ? "" : "un"); + dma_fence_put(fence); } rcu_read_unlock();

6 years, 4 months

1
0
0 0

[PATCH 5.1 43/62] dma-buf: balance refcount inbalance

by Greg Kroah-Hartman

From: Jérôme Glisse <jglisse(a)redhat.com> commit 5e383a9798990c69fc759a4930de224bb497e62c upstream. The debugfs take reference on fence without dropping them. Signed-off-by: Jérôme Glisse <jglisse(a)redhat.com> Cc: Christian König <christian.koenig(a)amd.com> Cc: Daniel Vetter <daniel.vetter(a)ffwll.ch> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: linux-media(a)vger.kernel.org Cc: dri-devel(a)lists.freedesktop.org Cc: linaro-mm-sig(a)lists.linaro.org Cc: Stéphane Marchesin <marcheu(a)chromium.org> Cc: stable(a)vger.kernel.org Reviewed-by: Christian König <christian.koenig(a)amd.com> Signed-off-by: Sumit Semwal <sumit.semwal(a)linaro.org> Link: https://patchwork.freedesktop.org/patch/msgid/20181206161840.6578-1-jglisse… Signed-off-by: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org> --- drivers/dma-buf/dma-buf.c | 1 + 1 file changed, 1 insertion(+) --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1068,6 +1068,7 @@ static int dma_buf_debug_show(struct seq fence->ops->get_driver_name(fence), fence->ops->get_timeline_name(fence), dma_fence_is_signaled(fence) ? "" : "un"); + dma_fence_put(fence); } rcu_read_unlock();

6 years, 4 months

1
0
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig