Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758618AbcDEOZS (ORCPT ); Tue, 5 Apr 2016 10:25:18 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:35008 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758568AbcDEOZO (ORCPT ); Tue, 5 Apr 2016 10:25:14 -0400 MIME-Version: 1.0 In-Reply-To: <20160405140107.GB7822@mtj.duckdns.org> References: <1458850962-16057-1-git-send-email-pandit.parav@gmail.com> <1458850962-16057-2-git-send-email-pandit.parav@gmail.com> <20160404193640.GA7822@mtj.duckdns.org> <20160405012504.GG24661@htj.duckdns.org> <20160405140107.GB7822@mtj.duckdns.org> Date: Tue, 5 Apr 2016 07:25:11 -0700 Message-ID: Subject: Re: [PATCHv10 1/3] rdmacg: Added rdma cgroup controller From: Parav Pandit To: Tejun Heo Cc: cgroups@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org, lizefan@huawei.com, Johannes Weiner , Doug Ledford , Liran Liss , "Hefty, Sean" , Jason Gunthorpe , Haggai Eran , Jonathan Corbet , james.l.morris@oracle.com, serge@hallyn.com, Or Gerlitz , Matan Barak , akpm@linux-foundation.org, linux-security-module@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2532 Lines: 59 On Tue, Apr 5, 2016 at 7:01 AM, Tejun Heo wrote: > Hello, Parav. > > On Mon, Apr 04, 2016 at 07:22:38PM -0700, Parav Pandit wrote: >> > Is it actually customary to have rdma core module updated more >> > frequently separate from the kernel? Out-of-tree modules being >> > updated separately happens quite a bit but this is an in-kernel >> > module, which usually is tightly coupled with the rest of the kernel. >> > >> Yes. >> rdma core module is compiled as kernel module. >> Its often updated for new features, fixes. >> So kernel version can be one but RDMA core module(s) get updated more >> frequently than kernel. > > As it is a fairly isolated domain, to certain extent, it could be okay > to let it go. At the same time, I know that these enterprise things > tend to go completely wayward and am worried about individual drivers > going crazy with custom attributes in a non-sensical way. If its crazy at driver level, I am sure it will be equally crazy for any end-user too. Therefore no user would use those crazy resources either. Intent is certainly not for the individual drivers as we agreed in past. IB stack maintainers would be reviewing new enum addition anyway, whether its verb or hw resource (unlikely). > The > interface this patch is proposing easily allows that and that at the > cost of internal engineering flexibility. I don't really want to be > caught up in a situation where we're forced to include broken usages > because that's what's already out in the wild. I personally would > much prefer the resources to be defined rigidly. Let's see how the > discussion with Christoph evolves. > Sure. I will wait for his comments. >> > I don't remember the details well but the code was vastly more complex >> > back then and the object lifetime management was muddy at best. If I >> > reviewed in a contradicting way, my apologies, but given the current >> > code, it'd be better to have objects creation upfront. >> >> Do you mean, >> try_charge() should >> lock() >> run loop to allocate in hierarchy, if not allocated. >> run loop again to charge. >> unlock() >> >> If so, I prefer to run the loop once. > > In the original review message, I mentioned creating an interface > which creates the hierarchy of objects as necessary and returns the > target pool with lock held, can you please give it a shot? Something > like the following. o.k. I will attempt. Looks doable. I am on travel so it will take few more days for me to turn around with below approach with tested code.