Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S966625AbbDWRQq (ORCPT ); Thu, 23 Apr 2015 13:16:46 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:60195 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030210AbbDWRQo (ORCPT ); Thu, 23 Apr 2015 13:16:44 -0400 Date: Thu, 23 Apr 2015 19:16:40 +0200 From: Greg Kroah-Hartman To: Andy Lutomirski Cc: Linus Torvalds , Andrew Morton , Arnd Bergmann , "Eric W. Biederman" , One Thousand Gnomes , Tom Gundersen , Jiri Kosina , "linux-kernel@vger.kernel.org" , Daniel Mack , David Herrmann , Djalal Harouni Subject: Re: [GIT PULL] kdbus for 4.1-rc1 Message-ID: <20150423171640.GA11227@kroah.com> References: <20150413190350.GA9485@kroah.com> <20150423130548.GA4253@kroah.com> <20150423163616.GA10874@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4285 Lines: 91 On Thu, Apr 23, 2015 at 09:46:22AM -0700, Andy Lutomirski wrote: > On Thu, Apr 23, 2015 at 9:36 AM, Greg Kroah-Hartman > wrote: > > On Thu, Apr 23, 2015 at 03:05:48PM +0200, Greg Kroah-Hartman wrote: > >> > >> Andy's concerns about the capability stuff has been hashed out in > >> multiple threads here. The kernel code isn't buggy as-designed or > >> implemented from what we can all tell, it's just that the new > >> functionality isn't liked by everyone, which is totally fair, but not a > >> reason to declare that the function isn't useful. > > > > Andy, did I capture your existing position correctly? If we drop the > > caps metadata, I'm guessing that you are ok with the code as you have > > reviewed it and tested it out. So should I just add a small patch that > > removes this for now? After that, we can discuss the addition of > > capabilities to the metadata as an add-on feature with a future patch > > and not hold up this larger merge request? > > No. I can fish out lists I've posted of what I personally dislike. > To repeat from my not-yet-awake memory, briefly: > > - starttime, cmdline, and possibly other pieces of metadata are also > problematic. I think starttime is especially bad because it both > breaks CRIU and is IMO completely unnecessary -- I sent out draft > "highpid" patches a while ago to give a much better alternative that > isn't racy and won't break CRIU. But cmdline is also IMO ridiculous. starttime was removed a while ago, are you sure you are looking at the latest code? cmdline has been discussed and it really helps with debugging. Decisions aren't being made based on it. > - There's still an open performance question. Namely: is kdbus performant? Yes, I thought that was already answered. Tizen posted some numbers with a much older version of the code, before David fixed a bunch of issues that he and you found, and that averaged between 25-50% faster. Details are in this presentation: http://download.tizen.org/misc/media/conference2014/slides/tdc2014-kdbus-in-tizen3.pdf The Tizen and GENIVI developers are off running numbers with the latest code, or so they told me through emails, but I don't know when/if that will ever happen, so I can't promise more than what is already here. > - The policy system still sucks. Now, if we give up on the idea of > anyone ever using it for anything other than dbus as it currently > works, maybe this isn't a real problem. As designed, it's for D-Bus, so there's not much I can suggest here, this isn't a "generic IPC" :) The binder developers at Samsung have stated that the implementation we have here works for their model as well, so I guess that is some kind of verification it's not entirely tied to D-Bus. They have plans on dropping the existing binder kernel code and using the kdbus code instead when it is merged. > - Someone should probably convince someone who understands memory > accounting that the pool mechanism accounts memory acceptably. I > don't know much about mm stuff, but I think it's subject to all kinds > of nasty latency and accounting abuses, some of which might even be > exploited by accident. Michal and David agree that this all works properly. I don't know of anyone else to ask about it, do you? > I haven't reviewed most of it. I've reviewed the metadata code (and > not recently) and the pool *docs*. > > Shouldn't the bulk of this code have actual review before it gets > merged? I've only reviewed some of it, and I didn't like what I found > in that small fraction, hence my objections to caps. I'd love more review, and we have been asking for it since last October. You provided a lot of it a while ago, and that helped immensely. I can't force anyone to read the code, I can only go on what people offer to do. We have 3 signed-off-bys on the main kdbus patches, and numerous other different developers have provided fixes / tweaks that are in this tree, so it's not like this is unread/unposted code here at all. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/