From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933098AbcBYObS (ORCPT ); Thu, 25 Feb 2016 09:31:18 -0500 Received: from mail-db3on0099.outbound.protection.outlook.com ([157.55.234.99]:49021 "EHLO emea01-db3-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932337AbcBYOa6 (ORCPT ); Thu, 25 Feb 2016 09:30:58 -0500 Authentication-Results: spf=pass (sender IP is 193.47.165.134) smtp.mailfrom=mellanox.com; vger.kernel.org; dkim=none (message not signed) header.d=none;vger.kernel.org; dmarc=pass action=none header.from=mellanox.com; Subject: Re: [PATCHv6 1/3] rdmacg: Added rdma cgroup controller To: Parav Pandit References: <1455966006-13774-1-git-send-email-pandit.parav@gmail.com> <1455966006-13774-2-git-send-email-pandit.parav@gmail.com> <56CDAC7A.6030206@mellanox.com> <56CEED81.7010507@mellanox.com> CC: , , , , Tejun Heo , , Johannes Weiner , Doug Ledford , Liran Liss , "Hefty, Sean" , Jason Gunthorpe , Jonathan Corbet , , , Or Gerlitz , Matan Barak , , , From: Haggai Eran Message-ID: <56CF1015.4040408@mellanox.com> Date: Thu, 25 Feb 2016 16:30:45 +0200 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.0.52.254] X-EOPAttributedMessage: 0 X-Microsoft-Exchange-Diagnostics: 1;AM1FFO11OLC002;1:2PgBFDuCIFdJeCoVvjUS4LocbLhx9315UfoHhkgzqwiVqV6jnhi2TVf6bgdrKZBpet/yzEYPg81NzjOBVzJE/fsYhCV26m9ntWPywYxliWt+nkdhO4Tqp/3Drl8w/9+ug8S5TEGUVTlSMNCMxA+BZlcBRCKfUl30ucZANyra6P6u4fqpEQcZkSMdQBaoAvJRmpWLcBoE3W1WGEuJviRxoberf3whMdH8mOwRBj1cdW/CES1rWlRRSsxd3u9zEpDSeoe3CS1+KLq9KIiJlxs5bZWeYkTLI+iatptHLll2nOJVOUEZaz8na/6EVShuJ64pt9MuiRFholJ0hfFK5w2UK50gFu+g5qFNEhRGupvbcKVr1uthBjMleXQJIH7geZ1ZkEmUcKdJtC1OKQoZ1lkNNTRIZ7VR11aEDTo8U7x/p4cusy8U7cKA8PrR0NyaMtvZ3vN4G8Be6dUB4JqxX+Bstw== X-Forefront-Antispam-Report: CIP:193.47.165.134;CTRY:IL;IPV:NLI;EFV:NLI;SFV:NSPM;SFS:(10009020)(6009001)(2980300002)(438002)(479174004)(24454002)(377454003)(199003)(189002)(47776003)(77096005)(106466001)(86362001)(4001350100001)(65956001)(189998001)(110136002)(5004730100002)(36756003)(33656002)(65816999)(23676002)(3846002)(87936001)(50986999)(76176999)(54356999)(230700001)(92566002)(15975445007)(80316001)(1096002)(1220700001)(2950100001)(19580395003)(19580405001)(6116002)(4326007)(5008740100001)(11100500001)(50466002)(6806005)(2906002)(586003)(3940600001);DIR:OUT;SFP:1101;SCL:1;SRVR:AM2PR05MB1027;H:mtlcas13.mtl.com;FPR:;SPF:Pass;MLV:sfv;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: 1;AM2PR05MB1027;2:Vl9x0iohgKwPR8DEy3+CBM10duA6tO/GVcRzx9DTibIOnNDHyOMnZOB9cjcd6yTlvNBuj/gLqj30n9OfNxcbZF1V3NJ4/jKnRExXvUJTsINhY4jIcYJVX3yMEV60G+X7ONnSWY0kl0Z0m7MkNAzGZw==;3:3Om0RXdcGqqsGxMJdOBaYcHbwr4kQeGsAXoD2rmO+JgLK+p+hUBW3YhYCOhDr6Q6eQBc3VftFpHTGP6PWGtaFN/cSzTFfIGEPs940LAU7gUTqvtjM8+zN4CKUk4lEzgAYtCpN/ejt2jnpZFjZvvUVrql4FLrxYZZ6DxmY/W25qwIyOD0Go+sO8GgNhiMxFcZamHg33URhYIYItSqP5zhkdtKA9VtQENG/ZBbZVVWh9PgEHpQU6NVG8T7pwY2KQ78KVGPlsX5k0R/kcHR0y1cZw==;25:n392+GN5jQXIhyiCKFWmpMVJXwVsOXLpqZ+jY2MlS7V0X/nVnC7HV6t/Iw/RLfdptf+AXCLxdEKFWWH4ZQ/EMR3kdmhsyqYAkYHdnFbt0Sn2XbRcW8sUmpqhmoMsAY99mVrj2tttBpxBdCeOrdCziYl3FA8YKEx++vnO5WQbqxUYA0oMcfrjcQdIDd73aP5s6StIfSOVJqwqbblGYU77Jwq091QjTiMxoiM4EmqyQdRabH8nFqFUvor66EnTFREY8U1oFJxxibBosF/+UktA+Rqm3H9GR/PMkNUHVEwBnJLbeYEW44U9KhNkp+LvXlmih2WGneD+BfGq8cNY69mIjQ== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(8251501001);SRVR:AM2PR05MB1027; X-MS-Office365-Filtering-Correlation-Id: 63493028-4500-4be6-a3cf-08d33df0440c X-Microsoft-Exchange-Diagnostics: 1;AM2PR05MB1027;20:sAO7LVa3+HDk46YOnRM5kcvKzpvYux/Blx7MML0Y5zwaXeSg+hkpkXkGVgA+czIDmS1Kl8WA7bX5QBkajFMjFbD8bKo78S4cnKKxr4Qxi/Uk3MiCNnO882m1kNLKaS5aC2mFUbrq9Gq543CJt+V19c/FKssJf5Jfd7ZzUFhzdzYXRuhSl85cTBZDjMUF27ZBQWh7pydlAfroCEcE4m3/gj9780K33IkROTBzbo8LitXh37r5YZ6AJ0UX8LPu5N74Zacoli8n2dS0RGlEvnuCgABy3mnWNRTof6f9QYwShZqXDIX4udeCMqfbf3gbi2UyEi40z6e8QcrX/fVq7SNEBLnmYoas2OfqzLIl1XUJTtW4fmjQS0zsYyV2DNi0nDotIkF2rRFQyY1zi8pNSCVUqn0I5mQlX4hB6bwwCAC+NDnWO4cdNS4pRiiTpUDu7Ing7LalVm7kgujusSIgGSjJeEl1wM8EupgN/B3NcucnGOe5ICO20sL4TI+rRlolpsYl X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(5005006)(13017025)(13023025)(13024025)(13018025)(13015025)(8121501046)(3002001)(10201501046);SRVR:AM2PR05MB1027;BCL:0;PCL:0;RULEID:;SRVR:AM2PR05MB1027; X-Microsoft-Exchange-Diagnostics: 1;AM2PR05MB1027;4:jk8apid4ve9hqi9nrTNd7dNJelwynHE+Mhdnb/I21EkGV8PhxJtEEDn5dJLhFRhqUDqE427dXwnjD5IGoxoCTORC4FsZG7UXSV1Zkrq/p5+VybdOgvzBBLo+ND/MwbpGtePQYFIx9YavakTdBU8k3xSJr9FSIfeX47byrEsj0RGt35Z10kMO2B5ynYJq7J/mNz4iRijI+ZPGFj3bt+hpWgl7caDw7p3odZtuRhSNupheKlZwUmVhZsEcPodtQ9pwhRE1TUVAjdcquYaOLWdY8NzBYDgU0XDIVcpMWs7l5xJr+uagBBXzY2tTdtdykECWn9hMPwqHZ8q7HcoElqIig2ZYwRuQN06c6BIrReIaibVOI7shqHxvpnVA2ptHhlk65QJCLTqIkJfF7m3rYZXbHNi1qrtlwqCG/RT0dd1Rau4pzM5gIWGmRllFvs6c70zh5kNbyatUvZ+AmohSIonleA== X-Forefront-PRVS: 08635C03D4 X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtBTTJQUjA1TUIxMDI3OzIzOmtSaTN2Y1lrYk84NEIxcCtoY2ZiT3ZlY0hx?= =?utf-8?B?c1E0bk00b2tIbTNtc3A0eDBXRnZ4MnNWeWhFaWh2cm1xRktVbWY4c2RJTnlz?= =?utf-8?B?SXZvT3RjNFlZSkFsV0VsbTBKTHRTK1RKTUphRUR3TzI0UjZ1NW1XZ2RLckEv?= =?utf-8?B?UXF3c3RzcVZPZUZDMmxiUnRsUWx4cHNlczJNZHJ1Qit4R20zakNaaVQ4b3pB?= =?utf-8?B?cmdxRlNqaGZQZHpyaXJEeGZWRG9qQi8zTnIwU1N0TnNIRnZIOXJKSk9uRXRw?= =?utf-8?B?VWd1UCtJMjZEMnY1cktWNjJqblZhRXhHcWMxck5wbyt6RW1iL1F2WUFMQ2JO?= =?utf-8?B?a3pZTC9ycE1KMlRvR2FJSy9GK1VFUDlxR1c3QVl0TVdCTHU3akJEMy9PS3Zz?= =?utf-8?B?YTBsd0hNNFE0S3pWMkZMMk1lRjYySVdSbjZIZFQ2ZlJRU3Rrd1BBSDdhVnpZ?= =?utf-8?B?aUlLQ2oyWG5pZ2xKRGMxd3AydTMzaTBqZlZRRThBT2NJNUdUUXFMREtVcDMv?= =?utf-8?B?WER6RVh4OUFESm9NQ2VmeU04ZHNVd2E5VkpDbkJvblUwazl0Zkp5TWx3d2R1?= =?utf-8?B?QkdjZmQ5L05OakYrb1h6Q3lZMzcxTU5CZFJoaXJ0czNnRmNGNytTU2Rna2t3?= =?utf-8?B?MHltOFlQQmdmZ0o1NDZFMi9iSHkxc05lMUxZNWhweC96c3BCSFF0YlF5b2FN?= =?utf-8?B?aG5qOEMzYzlaNnQxREdDbHcyT2dLR0FTb3VJQW5uS3ArRTdWZDA1aks1WDY0?= =?utf-8?B?L0R4eTZzNWwyL3ZMWVJLRmtnUGNFZWUzenFkMDdUYUR1NE40dnJrZkpJODNl?= =?utf-8?B?bnhRbWNuZHltQnFZemtUN05POGNTMlQyM1czbG0yWTVHNFIrRlYrNWZEVHpk?= =?utf-8?B?T0F3clRiT3k0TExDbGxJaXo3WVFlVTVlRFJUYWxiOVJuYmhUZG1qQklKK00v?= =?utf-8?B?Mlh4Ni8vdVQwbEp2RWRnek9zQkhENWc0eHVyZndYakJySitVdDNjbVRDL0pE?= =?utf-8?B?d3pMeGp5by91MEpqNDZFMG5taG9VMnl5MVl3ejRoOHhoemdjaWRKejFxSnZZ?= =?utf-8?B?Yi9KSWhJMGNFd3plZ3hUMDVBVG42bmtObFpjZU95T2RmZHdJcG9LRFhvalN1?= =?utf-8?B?b3RkbTNpN1l1amhFTlR1SXMyWjNvdDZZS0tZdy8ydUd6aVpLdmxicEw1NENE?= =?utf-8?B?MmxtWGVWWC9Eait0L2pLNnplYTFCekxKR3RDTHR5SjJwNThDZzZvUTk2T0FU?= =?utf-8?B?K3Y1cnNkOHpDa0VQNjAvYzhYVWZKdUozVWFlUVhHUnBqSTRSdFNyUXY0OFRu?= =?utf-8?B?ZlVtRnJMWllXa01xVGVxM2FaNkRTWHExMW53WVc5SWFFdFVaeU94eDU3N1FT?= =?utf-8?B?a0pWZFB6SHUwYjlpY3NIV2pkRTcrUy9xRDYzaThXSEhXY24yTnJoT3B1R2tU?= =?utf-8?B?UjZJa1JPREtIdmo0eWNoRitsV213MXd2YmU5OHpINDVFVGtxbTlhY0dROW4w?= =?utf-8?Q?RNQbqkHNRvT72MbRCs47S2wNdWKusK2N1YmsFoQIJdvVJ4?= X-Microsoft-Exchange-Diagnostics: 1;AM2PR05MB1027;5:aUPBcH1shr4JUBTwIApdSpfV9qLV201kPQ4mqmhF6jBUMWz4/vekvarmk+lKOEuHNJTDZsRxEl9i+p5/5GcsuQ6Nwp7bvXn5CBz27WBQ5wp8u89cnshCT41cQkIQ0+FDCWLSmZOLoaf4stxEikL5uQ==;24:D3hnIW2065SK93nrCUDhMaU9DOr1YdADYJplVOFyK/eDjZa0bVlTZKBSDN4WRSYO9xnjW6LRFr+UcBK8YMW4umKM2+gWUB5hr4718UGzbcE= SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Feb 2016 14:30:53.0780 (UTC) X-MS-Exchange-CrossTenant-Id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=a652971c-7d2e-4d9b-a6a4-d149256f461b;Ip=[193.47.165.134];Helo=[mtlcas13.mtl.com] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM2PR05MB1027 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 25/02/2016 15:34, Parav Pandit wrote: > On Thu, Feb 25, 2016 at 5:33 PM, Haggai Eran wrote: >>>>> +retry: >>>>> + spin_lock(&cg->rpool_list_lock); >>>>> + rpool = find_cg_rpool_locked(cg, device); >>>>> + if (!rpool) { >>>>> + spin_unlock(&cg->rpool_list_lock); >>>>> + ret = alloc_cg_rpool(cg, device); >>>>> + if (ret) >>>>> + goto err; >>>>> + else >>>>> + goto retry; >>>> Instead of retrying after allocation of a new rpool, why not just return the >>>> newly allocated rpool (or the existing one) from alloc_cg_rpool? >>> >>> It can be done, but locking semantics just becomes difficult to >>> review/maintain with that where alloc_cg_rpool will unlock and lock >>> conditionally later on. >> Maybe I'm missing something, but couldn't you simply lock rpool_list_lock >> inside alloc_cg_rpool()? It already does that around its call to >> find_cg_rpool_locked() and the insertion to cg_list. > > No. ref_count and usage counters are updated at level where lock is > taken in charge_cg_resource(). > If I move locking rpool_list_lock inside alloc_cg_rpool, unlocking > will continue outside, alloc_cg_rpool() when its found or allocated. > As you acknowledged in below comment that this makes confusing to > lock/unlock from different context, I think current implementation > achieves both. > (a) take lock from single context > (b) keep functionality of find and alloc in two separate individual functions Okay, fair enough. >> I thought that was about functions that only locked the lock, called the >> find function, and released the lock. What I'm suggesting is to have one >> function that does "lock + find + allocate if needed + unlock", > > I had similar function in past which does, > "lock + find + allocate if needed + + inc_ref_cnt + unlock", (get_cg_rpool) > update usage_counter atomically, because other thread/process might update too. > check atomic_dec_cnt - on reaching zero, "lock + del_entry + unlock + free". > > Tejun asked to simplify this to, > > "lock + find + allocate if needed + inc_ref_cnt_without_atomic" + unlock". > which I did in this patch v6. Okay. >> and another >> function that does (under caller's lock) "check ref count + check max count + >> release rpool". > This can be done. Have one dumb basic question for thiat. > Can we call kfree() with spin_lock held? All these years I tend to > avoid doing so. > I think so. This is an old link but I think it still applies: https://lkml.org/lkml/2004/11/21/130