Re: [PATCH] fence: Use smp_mb__before_atomic()

From: Maarten Lankhorst
Date: Thu Jun 05 2014 - 08:00:25 EST


op 05-06-14 13:51, Rob Clark schreef:
On Wed, Jun 4, 2014 at 1:49 PM, Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
On Wed, Jun 04, 2014 at 03:28:33PM +0200, Thierry Reding wrote:
On Wed, Jun 04, 2014 at 04:57:07PM +0530, Sumit Semwal wrote:
Hi Greg,


On 30 May 2014 21:38, Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
On Fri, May 30, 2014 at 10:15:05AM +0200, Thierry Reding wrote:
On Wed, May 28, 2014 at 01:51:45PM -0700, Greg Kroah-Hartman wrote:
On Wed, May 28, 2014 at 04:26:32PM +0200, Thierry Reding wrote:
From: Thierry Reding <treding@xxxxxxxxxx>

Commit febdbfe8a91c (arch: Prepare for smp_mb__{before,after}_atomic())
deprecated the smp_mb__{before,after}_{atomic,clear}_{dec,inc,bit}*()
functions in favour of the unified smp_mb__{before,after}_atomic().

Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
---
drivers/base/fence.c | 4 ++--
Where does this file come from? I've not seen it before, and it's not
in my tree.
I think it came in through Sumit's tree and it's only in linux-next I
believe.
Odd, linux-next is for merging things in Linus's next release.

And as I have never seen this code that will end up being my
responsibility to maintain, it seems strange that it will be merged in
the next kernel development cycle.

What broke down here with our review process that required something to
be merged without at least a cc: to me?
This is a new file added by Maarten's patches [1], that got reviewed
on dri-devel and other mailing lists. Since it was quite closely
associated with dma-buf, I figured I should take it through the
dma-buf tree.

I am sorry I didn't notice that you weren't CC'ed on these patches -
Sincere apologies, since I should've noticed that during the patch
review process - I would take part of the blame here as well :(

I do realize now that atleast on my part, I should've asked you before
taking it through the dma-buf tree - I will make sure things like this
don't happen again through me.

May I request you to help us handle this - would it help if we add
Maarten as the maintainer for this file? Any other suggestions?
Perhaps something like the following would help?

diff --git a/MAINTAINERS b/MAINTAINERS
index fb39c9c3f0c1..d582f54adec8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2867,7 +2867,9 @@ L: linux-media@xxxxxxxxxxxxxxx
L: dri-devel@xxxxxxxxxxxxxxxxxxxxx
L: linaro-mm-sig@xxxxxxxxxxxxxxxx
F: drivers/base/dma-buf*
+F: drivers/base/fence.c
F: include/linux/dma-buf*
+F: include/linux/fence.h
F: Documentation/dma-buf-sharing.txt
T: git git://git.linaro.org/people/sumitsemwal/linux-dma-buf.git
@@ -2936,6 +2938,8 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
S: Supported
F: Documentation/kobject.txt
F: drivers/base/
+X: drivers/base/dma-buf*
+X: drivers/base/fence.c
F: fs/sysfs/
F: fs/debugfs/
F: include/linux/kobj*

That removes Greg from the list generated by get_maintainer.pl for
anything that touches the DMA-BUF files.
That doesn't really work for most people, I'll still be "responsible"
for the code.

Thinking about it, perhaps moving DMA-BUF into its own subdirectory
would be an option too, to make the separation more obvious.
That might be best for some of this.

But again, why is the fence.c code needed at all anyway? I'm not sold
on that.
Fence serves as a way to synchronize between (for example) multiple
asynchronous gpu's. There is definitely a need for this. Otherwise
performance for optimus/prime type setups is going to suck.

I thought we had added something under Documentation/ about it, but I
can't find it now (although possibly looking at the wrong trees)..
there is at least a bit of a description in the commit msg:

https://lkml.org/lkml/2014/2/24/602

I don't think the question about whether we need something like fence
to augment dma-buf is really in doubt. Maybe it should live somewhere
else, I'm not sure. But it makes sense for it to live wherever
dma-buf does, as they are intended to work together.

Additionally we already have a user for this in the kernel: the GPU drivers that use TTM.

In my tree I have a branch that converts TTM fences to this fence mechanism, roughly
preserving the same functionality. As soon as the fence code is merged I'll move TTM over to the new code.

Additionally I added a series on top that allows drm drivers to get additional fences through
dma-buf, allowing nouveau and i915 to work on the same buffer object,
with both drivers being aware that the other driver is performing work on it.

~Maarten

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/