From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?q?Michal=20Koutn=C3=BD?= Subject: [PATCH v3] cgroup: Reorganize css_set_lock and kernfs path processing Date: Mon, 10 Oct 2022 10:29:18 +0200 Message-ID: <20221010082918.3821-1-mkoutny@suse.com> References: Mime-Version: 1.0 Content-Transfer-Encoding: base64 Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1665390562; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=U0+wxjXvZ2b1azfCMhNMsVGmzlC6W42MvNjIgBFeRTo=; b=AfLrtPQiwpodUPdDAwLKLscDoCuhxd2V1PvYuUJhPxSt2//G8QE10PRDG6xe6jDXqoo2kN r51ir81dTFUVCp9bfkqBw2y1j7SyrT3S7MdG7L4qVvs2sbB2N28Qzw6GX/aXJNGRfcY5XA 2ZnlE4DiCD9gRu9XEF8AibJd4tqdyvs= In-Reply-To: List-ID: Content-Type: text/plain; charset="macroman" To: tj@kernel.org Cc: cgroups@vger.kernel.org, dan.carpenter@oracle.com, hannes@cmpxchg.org, linux-kernel@vger.kernel.org, lizefan.x@bytedance.com, mkoutny@suse.com VGhlIGNvbW1pdCA3NGU0Yjk1NmViMWMgaW5jb3JyZWN0bHkgd3JhcHBlZCBrZXJuZnNfd2Fsa19h bmRfZ2V0CihtaWdodF9zbGVlcCkgdW5kZXIgY3NzX3NldF9sb2NrIChzcGlubG9jaykuIGNzc19z ZXRfbG9jayBpcyBuZWVkZWQgYnkKX19jc2V0X2Nncm91cF9mcm9tX3Jvb3QgdG8gZW5zdXJlIHN0 YWJsZSBjc2V0LT5jZ3JwX2xpbmtzIGJ1dCBub3QgZm9yCmtlcm5mc193YWxrX2FuZF9nZXQuCgpX ZSBvbmx5IG5lZWQgdG8gbWFrZSBzdXJlIHRoYXQgdGhlIHJldHVybmVkIHJvb3RfY2dycCB3b24n dCBiZSBmcmVlZAp1bmRlciB1cy4gVGhpcyBpcyBnaXZlbiBpbiB0aGUgY2FzZSBvZiBnbG9iYWwg cm9vdCBiZWNhdXNlIGl0IGlzIHN0YXRpYwooY2dycF9kZmxfcm9vdC5jZ3JwKS4gV2hlbiB0aGUg cm9vdF9jZ3JwIGlzIGxvd2VyIGluIHRoZSBoaWVyYXJjaHksIGl0CmlzIHBpbm5lZCBieSBjZ3Jv dXBfbnMtPnJvb3RfY3NldCAoYW5kIGBjdXJyZW50YCB0YXNrIGNhbm5vdCBzd2l0Y2gKbmFtZXNw YWNlIGFzeW5jaHJvbm91c2x5IHNvIG5zX3Byb3h5IHBpbnMgY2dyb3VwX25zKS4KCk5vdGUgdGhp cyByZWFzb25pbmcgd29uJ3QgaG9sZCBmb3Igcm9vdCBjZ3JvdXBzIGluIHYxIGhpZXJhcmNoaWVz LAp0aGVyZWZvcmUgY3JlYXRlIGEgc3BlY2lhbC1jYXNlZCBoZWxwZXIgZnVuY3Rpb24ganVzdCBm b3IgdGhlIGRlZmF1bHQKaGllcmFyY2h5LgoKRml4ZXM6IDc0ZTRiOTU2ZWIxYyAoImNncm91cDog SG9ub3IgY2FsbGVyJ3MgY2dyb3VwIE5TIHdoZW4gcmVzb2x2aW5nIHBhdGgiKQpSZXBvcnRlZC1i eTogRGFuIENhcnBlbnRlciA8ZGFuLmNhcnBlbnRlckBvcmFjbGUuY29tPgpTaWduZWQtb2ZmLWJ5 OiBNaWNoYWwgS291dG7DvSA8bWtvdXRueUBzdXNlLmNvbT4KLS0tCiBrZXJuZWwvY2dyb3VwL2Nn cm91cC5jIHwgNDEgKysrKysrKysrKysrKysrKysrKysrKysrKysrKy0tLS0tLS0tLS0tLS0KIDEg ZmlsZSBjaGFuZ2VkLCAyOCBpbnNlcnRpb25zKCspLCAxMyBkZWxldGlvbnMoLSkKClRlc3RlZCBv bmx5IHdpdGggdGVzdF9jb3JlIHNlbGZ0ZXN0cyAoaS5lLiB0aGUgcGF0aC9pZCByZXNvbHV0aW9u IG5vdApjaGVjZWQsIG9ubHkgdGhlIG1pZ3JhdGlvbiBjb2RlKS4KCmRpZmYgLS1naXQgYS9rZXJu ZWwvY2dyb3VwL2Nncm91cC5jIGIva2VybmVsL2Nncm91cC9jZ3JvdXAuYwppbmRleCBjMzdiODI2 NWMwYTMuLmE3ZWM5NmYyNjk5NyAxMDA2NDQKLS0tIGEva2VybmVsL2Nncm91cC9jZ3JvdXAuYwor KysgYi9rZXJuZWwvY2dyb3VwL2Nncm91cC5jCkBAIC0xMzkyLDYgKzEzOTIsOSBAQCBzdGF0aWMg dm9pZCBjZ3JvdXBfZGVzdHJveV9yb290KHN0cnVjdCBjZ3JvdXBfcm9vdCAqcm9vdCkKIAljZ3Jv dXBfZnJlZV9yb290KHJvb3QpOwogfQogCisvKgorICogUmV0dXJuZWQgY2dyb3VwIGlzIHdpdGhv dXQgcmVmY291bnQgYnV0IGl0J3MgdmFsaWQgYXMgbG9uZyBhcyBjc2V0IHBpbnMgaXQuCisgKi8K IHN0YXRpYyBpbmxpbmUgc3RydWN0IGNncm91cCAqX19jc2V0X2Nncm91cF9mcm9tX3Jvb3Qoc3Ry dWN0IGNzc19zZXQgKmNzZXQsCiAJCQkJCSAgICBzdHJ1Y3QgY2dyb3VwX3Jvb3QgKnJvb3QpCiB7 CkBAIC0xNDAzLDYgKzE0MDYsNyBAQCBzdGF0aWMgaW5saW5lIHN0cnVjdCBjZ3JvdXAgKl9fY3Nl dF9jZ3JvdXBfZnJvbV9yb290KHN0cnVjdCBjc3Nfc2V0ICpjc2V0LAogCQlyZXNfY2dyb3VwID0g Y3NldC0+ZGZsX2NncnA7CiAJfSBlbHNlIHsKIAkJc3RydWN0IGNncnBfY3NldF9saW5rICpsaW5r OworCQlsb2NrZGVwX2Fzc2VydF9oZWxkKCZjc3Nfc2V0X2xvY2spOwogCiAJCWxpc3RfZm9yX2Vh Y2hfZW50cnkobGluaywgJmNzZXQtPmNncnBfbGlua3MsIGNncnBfbGluaykgewogCQkJc3RydWN0 IGNncm91cCAqYyA9IGxpbmstPmNncnA7CkBAIC0xNDE0LDYgKzE0MTgsNyBAQCBzdGF0aWMgaW5s aW5lIHN0cnVjdCBjZ3JvdXAgKl9fY3NldF9jZ3JvdXBfZnJvbV9yb290KHN0cnVjdCBjc3Nfc2V0 ICpjc2V0LAogCQl9CiAJfQogCisJQlVHX09OKCFyZXNfY2dyb3VwKTsKIAlyZXR1cm4gcmVzX2Nn cm91cDsKIH0KIApAQCAtMTQzNiwyMyArMTQ0MSwzNyBAQCBjdXJyZW50X2NnbnNfY2dyb3VwX2Zy b21fcm9vdChzdHJ1Y3QgY2dyb3VwX3Jvb3QgKnJvb3QpCiAKIAlyY3VfcmVhZF91bmxvY2soKTsK IAotCUJVR19PTighcmVzKTsKIAlyZXR1cm4gcmVzOwogfQogCisvKgorICogbG9vayB1cCBjZ3Jv dXAgYXNzb2NpYXRlZCB3aXRoIGN1cnJlbnQgdGFzaydzIGNncm91cCBuYW1lc3BhY2Ugb24gdGhl CisgKiBkZWZhdWx0IGhpZXJhcmNoeQorICoKKyAqIE5vdGUgdGhpcyBkb2Vzbid0IG5lZWQgbG9j a3MgdW5saWtlIGdlbmVyaWMgY29sbGVhZ3Vlcy4gV2h5PworICogLSBJbnRlcm5hbCByY3VfcmVh ZF9sb2NrIGlzIHVubmVjZXNzYXJ5IGJlY2F1c2Ugd2UgZG9uJ3QgZGVyZWZlcmVuY2UgYW55IHJj dQorICogICBwb2ludGVycy4KKyAqIC0gY3NzX3NldF9sb2NrIGlzIG5vdCBuZWVkZWQgYmVjYXVz ZSB3ZSBqdXN0IHJlYWQgY3NldC0+ZGZsX2NncnAuCisgKiAtIEFzIGEgYm9udXMgcmV0dXJuZWQg Y2dycCBpcyBwaW5uZWQgd2l0aCB0aGUgY3VycmVudCBiZWNhdXNlIGl0IGNhbm5vdAorICogICBz d2l0Y2ggY2dyb3VwX25zIGFzeW5jaHJvbm91c2x5LgorICovCitzdGF0aWMgc3RydWN0IGNncm91 cCAqCitjdXJyZW50X2NnbnNfY2dyb3VwX2RmbCh2b2lkKQoreworCXN0cnVjdCBjc3Nfc2V0ICpj c2V0OworCisJY3NldCA9IGN1cnJlbnQtPm5zcHJveHktPmNncm91cF9ucy0+cm9vdF9jc2V0Owor CXJldHVybiBfX2NzZXRfY2dyb3VwX2Zyb21fcm9vdChjc2V0LCAmY2dycF9kZmxfcm9vdCk7Cit9 CisKIC8qIGxvb2sgdXAgY2dyb3VwIGFzc29jaWF0ZWQgd2l0aCBnaXZlbiBjc3Nfc2V0IG9uIHRo ZSBzcGVjaWZpZWQgaGllcmFyY2h5ICovCiBzdGF0aWMgc3RydWN0IGNncm91cCAqY3NldF9jZ3Jv dXBfZnJvbV9yb290KHN0cnVjdCBjc3Nfc2V0ICpjc2V0LAogCQkJCQkgICAgc3RydWN0IGNncm91 cF9yb290ICpyb290KQogewotCXN0cnVjdCBjZ3JvdXAgKnJlcyA9IE5VTEw7Ci0KIAlsb2NrZGVw X2Fzc2VydF9oZWxkKCZjZ3JvdXBfbXV0ZXgpOwogCWxvY2tkZXBfYXNzZXJ0X2hlbGQoJmNzc19z ZXRfbG9jayk7CiAKLQlyZXMgPSBfX2NzZXRfY2dyb3VwX2Zyb21fcm9vdChjc2V0LCByb290KTsK LQotCUJVR19PTighcmVzKTsKLQlyZXR1cm4gcmVzOworCXJldHVybiBfX2NzZXRfY2dyb3VwX2Zy b21fcm9vdChjc2V0LCByb290KTsKIH0KIAogLyoKQEAgLTYwODUsOSArNjEwNCw3IEBAIHN0cnVj dCBjZ3JvdXAgKmNncm91cF9nZXRfZnJvbV9pZCh1NjQgaWQpCiAJaWYgKCFjZ3JwKQogCQlyZXR1 cm4gRVJSX1BUUigtRU5PRU5UKTsKIAotCXNwaW5fbG9ja19pcnEoJmNzc19zZXRfbG9jayk7Ci0J cm9vdF9jZ3JwID0gY3VycmVudF9jZ25zX2Nncm91cF9mcm9tX3Jvb3QoJmNncnBfZGZsX3Jvb3Qp OwotCXNwaW5fdW5sb2NrX2lycSgmY3NzX3NldF9sb2NrKTsKKwlyb290X2NncnAgPSBjdXJyZW50 X2NnbnNfY2dyb3VwX2RmbCgpOwogCWlmICghY2dyb3VwX2lzX2Rlc2NlbmRhbnQoY2dycCwgcm9v dF9jZ3JwKSkgewogCQljZ3JvdXBfcHV0KGNncnApOwogCQlyZXR1cm4gRVJSX1BUUigtRU5PRU5U KTsKQEAgLTY2NzEsMTAgKzY2ODgsOCBAQCBzdHJ1Y3QgY2dyb3VwICpjZ3JvdXBfZ2V0X2Zyb21f cGF0aChjb25zdCBjaGFyICpwYXRoKQogCXN0cnVjdCBjZ3JvdXAgKmNncnAgPSBFUlJfUFRSKC1F Tk9FTlQpOwogCXN0cnVjdCBjZ3JvdXAgKnJvb3RfY2dycDsKIAotCXNwaW5fbG9ja19pcnEoJmNz c19zZXRfbG9jayk7Ci0Jcm9vdF9jZ3JwID0gY3VycmVudF9jZ25zX2Nncm91cF9mcm9tX3Jvb3Qo JmNncnBfZGZsX3Jvb3QpOworCXJvb3RfY2dycCA9IGN1cnJlbnRfY2duc19jZ3JvdXBfZGZsKCk7 CiAJa24gPSBrZXJuZnNfd2Fsa19hbmRfZ2V0KHJvb3RfY2dycC0+a24sIHBhdGgpOwotCXNwaW5f dW5sb2NrX2lycSgmY3NzX3NldF9sb2NrKTsKIAlpZiAoIWtuKQogCQlnb3RvIG91dDsKIAotLSAK Mi4zNy4zCgo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7F3A6C433F5 for ; Mon, 10 Oct 2022 08:29:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230082AbiJJI32 (ORCPT ); Mon, 10 Oct 2022 04:29:28 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34010 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230001AbiJJI3Y (ORCPT ); Mon, 10 Oct 2022 04:29:24 -0400 Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 91CC45FADA; Mon, 10 Oct 2022 01:29:23 -0700 (PDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 4CF1621941; Mon, 10 Oct 2022 08:29:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=susede1; t=1665390562; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=U0+wxjXvZ2b1azfCMhNMsVGmzlC6W42MvNjIgBFeRTo=; b=AfLrtPQiwpodUPdDAwLKLscDoCuhxd2V1PvYuUJhPxSt2//G8QE10PRDG6xe6jDXqoo2kN r51ir81dTFUVCp9bfkqBw2y1j7SyrT3S7MdG7L4qVvs2sbB2N28Qzw6GX/aXJNGRfcY5XA 2ZnlE4DiCD9gRu9XEF8AibJd4tqdyvs= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 1D94D13ACA; Mon, 10 Oct 2022 08:29:22 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id Gyn5BeLXQ2N/bwAAMHmgww (envelope-from ); Mon, 10 Oct 2022 08:29:22 +0000 From: =?UTF-8?q?Michal=20Koutn=C3=BD?= To: tj@kernel.org Cc: cgroups@vger.kernel.org, dan.carpenter@oracle.com, hannes@cmpxchg.org, linux-kernel@vger.kernel.org, lizefan.x@bytedance.com, mkoutny@suse.com Subject: [PATCH v3] cgroup: Reorganize css_set_lock and kernfs path processing Date: Mon, 10 Oct 2022 10:29:18 +0200 Message-Id: <20221010082918.3821-1-mkoutny@suse.com> X-Mailer: git-send-email 2.37.3 In-Reply-To: References: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org The commit 74e4b956eb1c incorrectly wrapped kernfs_walk_and_get (might_sleep) under css_set_lock (spinlock). css_set_lock is needed by __cset_cgroup_from_root to ensure stable cset->cgrp_links but not for kernfs_walk_and_get. We only need to make sure that the returned root_cgrp won't be freed under us. This is given in the case of global root because it is static (cgrp_dfl_root.cgrp). When the root_cgrp is lower in the hierarchy, it is pinned by cgroup_ns->root_cset (and `current` task cannot switch namespace asynchronously so ns_proxy pins cgroup_ns). Note this reasoning won't hold for root cgroups in v1 hierarchies, therefore create a special-cased helper function just for the default hierarchy. Fixes: 74e4b956eb1c ("cgroup: Honor caller's cgroup NS when resolving path") Reported-by: Dan Carpenter Signed-off-by: Michal Koutný --- kernel/cgroup/cgroup.c | 41 ++++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-) Tested only with test_core selftests (i.e. the path/id resolution not checed, only the migration code). diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c index c37b8265c0a3..a7ec96f26997 100644 --- a/kernel/cgroup/cgroup.c +++ b/kernel/cgroup/cgroup.c @@ -1392,6 +1392,9 @@ static void cgroup_destroy_root(struct cgroup_root *root) cgroup_free_root(root); } +/* + * Returned cgroup is without refcount but it's valid as long as cset pins it. + */ static inline struct cgroup *__cset_cgroup_from_root(struct css_set *cset, struct cgroup_root *root) { @@ -1403,6 +1406,7 @@ static inline struct cgroup *__cset_cgroup_from_root(struct css_set *cset, res_cgroup = cset->dfl_cgrp; } else { struct cgrp_cset_link *link; + lockdep_assert_held(&css_set_lock); list_for_each_entry(link, &cset->cgrp_links, cgrp_link) { struct cgroup *c = link->cgrp; @@ -1414,6 +1418,7 @@ static inline struct cgroup *__cset_cgroup_from_root(struct css_set *cset, } } + BUG_ON(!res_cgroup); return res_cgroup; } @@ -1436,23 +1441,37 @@ current_cgns_cgroup_from_root(struct cgroup_root *root) rcu_read_unlock(); - BUG_ON(!res); return res; } +/* + * look up cgroup associated with current task's cgroup namespace on the + * default hierarchy + * + * Note this doesn't need locks unlike generic colleagues. Why? + * - Internal rcu_read_lock is unnecessary because we don't dereference any rcu + * pointers. + * - css_set_lock is not needed because we just read cset->dfl_cgrp. + * - As a bonus returned cgrp is pinned with the current because it cannot + * switch cgroup_ns asynchronously. + */ +static struct cgroup * +current_cgns_cgroup_dfl(void) +{ + struct css_set *cset; + + cset = current->nsproxy->cgroup_ns->root_cset; + return __cset_cgroup_from_root(cset, &cgrp_dfl_root); +} + /* look up cgroup associated with given css_set on the specified hierarchy */ static struct cgroup *cset_cgroup_from_root(struct css_set *cset, struct cgroup_root *root) { - struct cgroup *res = NULL; - lockdep_assert_held(&cgroup_mutex); lockdep_assert_held(&css_set_lock); - res = __cset_cgroup_from_root(cset, root); - - BUG_ON(!res); - return res; + return __cset_cgroup_from_root(cset, root); } /* @@ -6085,9 +6104,7 @@ struct cgroup *cgroup_get_from_id(u64 id) if (!cgrp) return ERR_PTR(-ENOENT); - spin_lock_irq(&css_set_lock); - root_cgrp = current_cgns_cgroup_from_root(&cgrp_dfl_root); - spin_unlock_irq(&css_set_lock); + root_cgrp = current_cgns_cgroup_dfl(); if (!cgroup_is_descendant(cgrp, root_cgrp)) { cgroup_put(cgrp); return ERR_PTR(-ENOENT); @@ -6671,10 +6688,8 @@ struct cgroup *cgroup_get_from_path(const char *path) struct cgroup *cgrp = ERR_PTR(-ENOENT); struct cgroup *root_cgrp; - spin_lock_irq(&css_set_lock); - root_cgrp = current_cgns_cgroup_from_root(&cgrp_dfl_root); + root_cgrp = current_cgns_cgroup_dfl(); kn = kernfs_walk_and_get(root_cgrp->kn, path); - spin_unlock_irq(&css_set_lock); if (!kn) goto out; -- 2.37.3