Re: [GIT PULL] kdbus for 4.1-rc1

Andy Lutomirski
2015-04-23T16:46:47Z

On Thu, Apr 23, 2015 at 9:36 AM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> 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.

- There's still an open performance question. Namely: is kdbus performant?

- 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.

- 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.

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.

--Andy

>
> thanks,
>
> greg k-h

--
Andy Lutomirski
AMA Capital Management, LLC