Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760690AbcCELPP (ORCPT ); Sat, 5 Mar 2016 06:15:15 -0500 Received: from mail-wm0-f44.google.com ([74.125.82.44]:34625 "EHLO mail-wm0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760102AbcCELPL (ORCPT ); Sat, 5 Mar 2016 06:15:11 -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> Date: Sat, 5 Mar 2016 16:45:09 +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 , raindel@mellanox.com, 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: 2493 Lines: 58 Hi Tejun, I would like to submit patch v10. Can you please confirm that you are ok (or not) with the current design and below changes should be good enough? I am ok if you directly want to jump to review v10 too. Changes from v9: * Included documentation of resources in v2.txt and v1.txt * Fixed issue of race condition of process migration during charging stage. * Fixed comments and code to adhere to CodingStyle. * Simplified and removed support to charge/uncharge multiple resource. * Fixed removed refcnt with usage_num that tracks how many resources are unused to trigger freeing the object. * Removed inline for query_limit and other function as its not necessary. 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. Parav On Thu, Mar 3, 2016 at 8:19 AM, Parav Pandit wrote: > Hi Tejun, Haggai, > > On Thu, Mar 3, 2016 at 1:28 AM, Parav Pandit wrote: >>>> + rpool->refcnt--; >>>> + if (rpool->refcnt == 0 && rpool->num_max_cnt == pool_info->table_len) { >>> >>> If the caller charges 2 and then uncharges 1 two times, the refcnt >>> underflows? Why not just track how many usages are zero? >>> >> This is certainly must fix bug. Changed refcnt to usage_sum and changed to do >> usage_sum -= num during uncharging >> and >> usage_sum += num during charing. > > This is not sufficient as css_get() and put are done only once per > call, which leads to similar problem as of refcnt. > As I think more, I realised that this particular test is missing that > resulted in this related bug, I realize that we don't have use case to > have "num" field from the IB stack side. > For bulk free IB stack will have to keep track of different or same > rdmacg returned values to call uncharge() with right number of > resources, all of that complexity just doesn't make sense and not > required. > So as first step to further simplify this, I am removing "num" input > field from charge and uncharge API.