Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751848AbcCLGTQ (ORCPT ); Sat, 12 Mar 2016 01:19:16 -0500 Received: from mail-wm0-f41.google.com ([74.125.82.41]:33330 "EHLO mail-wm0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751195AbcCLGTG (ORCPT ); Sat, 12 Mar 2016 01:19:06 -0500 MIME-Version: 1.0 In-Reply-To: References: <1456859137-13646-1-git-send-email-pandit.parav@gmail.com> <1456859137-13646-2-git-send-email-pandit.parav@gmail.com> <20160302173949.GG29826@mtj.duckdns.org> <20160305125215.GC3567@htj.duckdns.org> Date: Sat, 12 Mar 2016 11:49:03 +0530 Message-ID: Subject: Re: [PATCHv9 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: 2266 Lines: 60 Hi Tejun, On Sat, Mar 5, 2016 at 10:50 PM, Parav Pandit wrote: > Hi Tejun, > > On Sat, Mar 5, 2016 at 6:22 PM, Tejun Heo wrote: >> Hello, Parav. >> >> On Sat, Mar 05, 2016 at 04:45:09PM +0530, Parav Pandit wrote: >>> Design that remains same from v6 to v10. >>> * spin lock is still fine grained at cgroup level instead of one >>> global shared lock among all cgroups. >>> In future it can be optimized further to do per cpu or using >>> single lock if required. >>> * file type enums are still present for max and current, as >>> read/write call to those files is already taken care by common >>> functions with required if/else. >>> * Resource limit setting is as it is, because number of devices are >>> in range of 1 to 4 count in most use cases (as explained in >>> documentation), and its not hot path. >> >> 1 and 2 are not okay. > For (1) shall I have one spin lock that is uses across multiple > hierarchy and multiple cgroup. > Essentially one global lock among all cgroup. During hierarchical > charging, continue to use same lock it at each level. > Would that work in this first release? > I am waiting for your reply. Shall one lock for all cgroup is ok with you? > Can you please review the code for (2), I cannot think of any further > helper functions that I can write. > For both the file types, all the code is already common. > file types are used only to find out whether to reference max variable > or usage variable in structure. > Which can also be made as array, but I do not want to lose the code > readability for that little gain. > What exactly is the issue in current implementation? You just > mentioned that "its not good sign". > Its readable, simple and serves the purpose, what am I missing? > If this is ok. I will keep the code as it is, because it uses common helper functions for max and current files. >> 3 is fine but resource [un]charging is not hot path? > charge/uncharge is hot path from cgroup perspective. > Considering 1 to 4 devices in system rpool list would grow upto 4 > entry deep at each cgroup level. > I believe this is good enough to start with. O complexity wise its > O(N). where N is number of devices in system. > > >> >> Thanks. >> >> -- >> tejun