Re: [PATCH] fence: Use smp_mb__before_atomic()

[Rob Clark]

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.

BR,
-R


> greg k-h
> --
> 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/
--
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/