On Thu, Jun 05, 2014 at 10:09:29PM +0530, Sumit Semwal wrote: > Hi Greg, > > On 5 June 2014 21:18, Greg Kroah-Hartman <gre...@linuxfoundation.org> wrote: > >> > 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. > > > > What's wrong with the 'sync' code in the drivers/staging/android/ > > directory? I thought that is what that codebase was supposed to be > > doing. > > > (Just a quick recap on sync v/s fences): > Well, there was a discussion on Sync v/s Fences at LPC 2013 [1], and > the agreement was that if (explicit) sync could be implemented on top > of (implicitly synchronized) fences, fences might provide more > flexibility. > Maarten implemented that (it is a part of this patch series), and > Android guys confirmed that it worked the same way as expected in the > Android system. That is a major reason for acceptance of fence as the > solution.
That's great to hear, again, it should have been conveyed somehow :) > >> 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 > > > > Ah, so no documenation, no discussion, and you want to just throw it in > > a directory I am responsible for? That sounds like a major rush job... > > > While I totally agree that it was an honest mistake to not have you > notified in CC for the patch series, for your other observations, > please allow me to answer one-by-one: > (1) No documentation: there is in-code documentation which is also > added to Documentation/DocBook/device-drivers.tmpl; the commit message > has a lot of description about it. Of course, it could do with its own > documentation as well. > (2) No discussion: the patch series is into v17 - v2 was posted in > July 2012, so it does seem like it has gone through a series of > reviews - I would admit that it has been mostly limited to the people > it seemed to matter the most for - the dri-devel and v4l communities. Limiting the discussion to the people involved is fine, and great, but really, for a core kernel function, you need to pull in _some_ core kernel people to at least go over the api. With just a quick glance, I already had api objections to how you were implementing things, those should be addressed at the least before the code is merged. > >> 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. > > > > Ok, then let's give it a proper review cycle, and notify everyone > > involved. It doesn't look like this happened at all. We don't add core > > primitives to the kernel without a lot of discussion and agreement. And > > we sure don't add them without telling the person who owns the directory > > (again, my pet peeve, I know...) > > > And again, I do apologize that we all missed the fact that you weren't > CC'ed onto the patch series. > > Still, even in the wake of above information, if you feel we are not > ready to take it for 3.16, I would drop it from my queue for 3.16 > (though there are quite a few people who've waited long for this to > land in). Given that I have yet to even be cc:ed on a patch, and the merge window is 1 week in, _and_ people are still discussing where to put the files, yes, it's too late for 3.16, sorry. Please feel free to respin and resend after 3.16-rc1 is out. There is no "deadline" here that is requiring code to ever be merged. We do things correctly, not rushed. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
