From patchwork Wed Mar 24 19:20:44 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Minchan Kim X-Patchwork-Id: 12162171 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-16.8 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9C964C433C1 for ; Wed, 24 Mar 2021 19:20:56 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 1771961984 for ; Wed, 24 Mar 2021 19:20:56 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1771961984 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 8A2946B02FB; Wed, 24 Mar 2021 15:20:55 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 82BB76B02FD; Wed, 24 Mar 2021 15:20:55 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 67F976B02FE; Wed, 24 Mar 2021 15:20:55 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0169.hostedemail.com [216.40.44.169]) by kanga.kvack.org (Postfix) with ESMTP id 498BA6B02FB for ; Wed, 24 Mar 2021 15:20:55 -0400 (EDT) Received: from smtpin39.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 03089582B for ; Wed, 24 Mar 2021 19:20:55 +0000 (UTC) X-FDA: 77955735270.39.74EF3D4 Received: from mail-pf1-f174.google.com (mail-pf1-f174.google.com [209.85.210.174]) by imf17.hostedemail.com (Postfix) with ESMTP id DEF3C4080F4D for ; Wed, 24 Mar 2021 19:20:48 +0000 (UTC) Received: by mail-pf1-f174.google.com with SMTP id q5so18100080pfh.10 for ; Wed, 24 Mar 2021 12:20:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=lvb7hlibIPwzR60x/yUMbLm1jxf0Ps+EL44ahkPgqgk=; b=sP+UjYtD0EYGlXpp6dl2cOxcX20AueElPepb+bh68mFOcQQWIC9ArIpI/5bWe3KZaq rnVfmYjeeRC9tXa59+w6AU80c0EfhHcteOZvRxe/NE4p4jWGIMxIjrleJZ5juPW4AH9q lUW+aHzbPPJuLrHwkIiJB3Q2qfJs64IPmGDD1Hr3oEPUro7bGS95HN8tnQw7b5GO58Ey QtaR16tnU9yKP907tmHy3mK7ofnbaGyWSdQ7vZfg+AA3PCTHThFIFKug8EmfFVnow5aB zgZFQpZho2QmU2rHa5Q2krToeM7iSI6andvc+nPJBNbvzs2dGyeN7x3mq9FcvPLQhayH eybA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:cc:subject:date:message-id :mime-version:content-transfer-encoding; bh=lvb7hlibIPwzR60x/yUMbLm1jxf0Ps+EL44ahkPgqgk=; b=AE33ez3F7RaY6yIGSUIqdrxIP2rCCuIG30aNg+GEOFAS2ROy7j64P/DNlSpP42wwxL ANDCvAOmY5TudFjtGs3Q/PW8iKR7UlAPg49yySq7XhPIBYdB7pa6EK5dfYZAHF1mgfTq /ZGUqNNvokNXMDaW7F5wL/FWMG5CL2EcCDGNTFCUuSZEidHu5OymXRyJ+eV/y0CN+2WS +Y5OVn3rOn/mTJyOtFq9n2ZE76nH+AXUwO3nhzyBHkVpGmX14RsYjsvNyXhFexVyuelP yjzjs9YTF32399KXIjLJMuOHQ/yl3Psrdlk41V62LNxQpIncD8QoC4alUXVWCAjdR6KK E2Mg== X-Gm-Message-State: AOAM530jbe1ETUkrEJLtP043BVjzpdjSIAYvTvd1OUH97eHUp5i1Rais mS4hxh+ZvwaGiRAJ1qbkRIg= X-Google-Smtp-Source: ABdhPJyl7WMk3WqPOcoEn2Z9BfcptKKY4nyp60428xzj+BuWTbxRuruwfPWC380+G8Rid0RvvyUZ4Q== X-Received: by 2002:a63:5343:: with SMTP id t3mr4354311pgl.136.1616613649042; Wed, 24 Mar 2021 12:20:49 -0700 (PDT) Received: from bbox-1.mtv.corp.google.com ([2620:15c:211:201:7dfa:1e53:536:7976]) by smtp.gmail.com with ESMTPSA id e20sm3337210pgm.1.2021.03.24.12.20.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 24 Mar 2021 12:20:48 -0700 (PDT) From: Minchan Kim To: Andrew Morton Cc: linux-mm , LKML , gregkh@linuxfoundation.org, surenb@google.com, joaodias@google.com, jhubbard@nvidia.com, willy@infradead.org, digetx@gmail.com, Minchan Kim Subject: [PATCH] mm: cma: fix corruption cma_sysfs_alloc_pages_count Date: Wed, 24 Mar 2021 12:20:44 -0700 Message-Id: <20210324192044.1505747-1-minchan@kernel.org> X-Mailer: git-send-email 2.31.0.291.g576ba9dcdaf-goog MIME-Version: 1.0 X-Stat-Signature: yizgjnk1rzhmyhi4j75cdn3ehsp1qt4x X-Rspamd-Server: rspam01 X-Rspamd-Queue-Id: DEF3C4080F4D Received-SPF: none (gmail.com>: No applicable sender policy available) receiver=imf17; identity=mailfrom; envelope-from=""; helo=mail-pf1-f174.google.com; client-ip=209.85.210.174 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1616613648-188775 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: struct cma_stat's lifespan for cma_sysfs is different with struct cma because kobject for sysfs requires dynamic object while CMA is static object[1]. When CMA is initialized, it couldn't use slab to allocate cma_stat since slab was not initialized yet. Thus, it allocates the dynamic object in subsys_initcall. However, the cma allocation can happens before subsys_initcall then, it goes crash. Dmitry reported[2]: .. [ 1.226190] [] (cma_sysfs_alloc_pages_count) from [] (cma_alloc+0x153/0x274) [ 1.226720] [] (cma_alloc) from [] (__alloc_from_contiguous+0x37/0x8c) [ 1.227272] [] (__alloc_from_contiguous) from [] (atomic_pool_init+0x7b/0x126) [ 1.233596] [] (atomic_pool_init) from [] (do_one_initcall+0x45/0x1e4) [ 1.234188] [] (do_one_initcall) from [] (kernel_init_freeable+0x157/0x1a6) [ 1.234741] [] (kernel_init_freeable) from [] (kernel_init+0xd/0xe0) [ 1.235289] [] (kernel_init) from [] (ret_from_fork+0x11/0x1c) This patch moves those statistic fields of cma_stat into struct cma and introduces cma_kobject wrapper to follow kobject's rule. At the same time, it fixes other routines based on suggestions[3][4]. [1] https://lore.kernel.org/linux-mm/YCOAmXqt6dZkCQYs@kroah.com/ [2] https://lore.kernel.org/linux-mm/fead70a2-4330-79ff-e79a-d8511eab1256@gmail.com/ [3] https://lore.kernel.org/linux-mm/20210323195050.2577017-1-minchan@kernel.org/ [4] https://lore.kernel.org/linux-mm/20210324010547.4134370-1-minchan@kernel.org/ Reported-by: Dmitry Osipenko Tested-by: Dmitry Osipenko Suggested-by: Dmitry Osipenko Suggested-by: John Hubbard Suggested-by: Matthew Wilcox Signed-off-by: Minchan Kim --- I belive it's worth to have separate patch rather than replacing original patch. It will also help to merge without conflict since we already filed other patch based on it. Strictly speaking, separating fix part and readbility part in this patch would be better but it's gray to separate them since most code in this patch was done while we were fixing the bug. Since we don't release it yet, I hope it will work. Otherwise, I can send a replacement patch inclucing all of changes happend until now with gathering SoB. mm/cma.c | 4 +-- mm/cma.h | 25 +++++++------- mm/cma_sysfs.c | 88 ++++++++++++++++++++++++++++---------------------- 3 files changed, 65 insertions(+), 52 deletions(-) diff --git a/mm/cma.c b/mm/cma.c index 90e27458ddb7..08c45157911a 100644 --- a/mm/cma.c +++ b/mm/cma.c @@ -509,11 +509,11 @@ struct page *cma_alloc(struct cma *cma, size_t count, unsigned int align, out: if (page) { count_vm_event(CMA_ALLOC_SUCCESS); - cma_sysfs_alloc_pages_count(cma, count); + cma_sysfs_account_success_pages(cma, count); } else { count_vm_event(CMA_ALLOC_FAIL); if (cma) - cma_sysfs_fail_pages_count(cma, count); + cma_sysfs_account_fail_pages(cma, count); } return page; diff --git a/mm/cma.h b/mm/cma.h index 95d1aa2d808a..37b9b7858c8e 100644 --- a/mm/cma.h +++ b/mm/cma.h @@ -5,12 +5,8 @@ #include #include -struct cma_stat { - spinlock_t lock; - /* the number of CMA page successful allocations */ - unsigned long nr_pages_succeeded; - /* the number of CMA page allocation failures */ - unsigned long nr_pages_failed; +struct cma_kobject { + struct cma *cma; struct kobject kobj; }; @@ -27,7 +23,12 @@ struct cma { #endif char name[CMA_MAX_NAME]; #ifdef CONFIG_CMA_SYSFS - struct cma_stat *stat; + /* the number of CMA page successful allocations */ + atomic64_t nr_pages_succeeded; + /* the number of CMA page allocation failures */ + atomic64_t nr_pages_failed; + /* kobject requires dynamic object */ + struct cma_kobject *cma_kobj; #endif }; @@ -40,10 +41,12 @@ static inline unsigned long cma_bitmap_maxno(struct cma *cma) } #ifdef CONFIG_CMA_SYSFS -void cma_sysfs_alloc_pages_count(struct cma *cma, size_t count); -void cma_sysfs_fail_pages_count(struct cma *cma, size_t count); +void cma_sysfs_account_success_pages(struct cma *cma, unsigned long nr_pages); +void cma_sysfs_account_fail_pages(struct cma *cma, unsigned long nr_pages); #else -static inline void cma_sysfs_alloc_pages_count(struct cma *cma, size_t count) {}; -static inline void cma_sysfs_fail_pages_count(struct cma *cma, size_t count) {}; +static inline void cma_sysfs_account_success_pages(struct cma *cma, + unsigned long nr_pages) {}; +static inline void cma_sysfs_account_fail_pages(struct cma *cma, + unsigned long nr_pages) {}; #endif #endif diff --git a/mm/cma_sysfs.c b/mm/cma_sysfs.c index 3134b2b3a96d..a670a80aad6f 100644 --- a/mm/cma_sysfs.c +++ b/mm/cma_sysfs.c @@ -11,50 +11,54 @@ #include "cma.h" -static struct cma_stat *cma_stats; - -void cma_sysfs_alloc_pages_count(struct cma *cma, size_t count) +void cma_sysfs_account_success_pages(struct cma *cma, unsigned long nr_pages) { - spin_lock(&cma->stat->lock); - cma->stat->nr_pages_succeeded += count; - spin_unlock(&cma->stat->lock); + atomic64_add(nr_pages, &cma->nr_pages_succeeded); } -void cma_sysfs_fail_pages_count(struct cma *cma, size_t count) +void cma_sysfs_account_fail_pages(struct cma *cma, unsigned long nr_pages) { - spin_lock(&cma->stat->lock); - cma->stat->nr_pages_failed += count; - spin_unlock(&cma->stat->lock); + atomic64_add(nr_pages, &cma->nr_pages_failed); } #define CMA_ATTR_RO(_name) \ static struct kobj_attribute _name##_attr = __ATTR_RO(_name) -static struct kobject *cma_kobj; +static inline struct cma *cma_from_kobj(struct kobject *kobj) +{ + struct cma_kobject *cma_kobj = container_of(kobj, struct cma_kobject, + kobj); + struct cma *cma = cma_kobj->cma; + + return cma; +} static ssize_t alloc_pages_success_show(struct kobject *kobj, - struct kobj_attribute *attr, char *buf) + struct kobj_attribute *attr, char *buf) { - struct cma_stat *stat = container_of(kobj, struct cma_stat, kobj); + struct cma *cma = cma_from_kobj(kobj); - return sysfs_emit(buf, "%lu\n", stat->nr_pages_succeeded); + return sysfs_emit(buf, "%llu\n", + atomic64_read(&cma->nr_pages_succeeded)); } CMA_ATTR_RO(alloc_pages_success); static ssize_t alloc_pages_fail_show(struct kobject *kobj, - struct kobj_attribute *attr, char *buf) + struct kobj_attribute *attr, char *buf) { - struct cma_stat *stat = container_of(kobj, struct cma_stat, kobj); + struct cma *cma = cma_from_kobj(kobj); - return sysfs_emit(buf, "%lu\n", stat->nr_pages_failed); + return sysfs_emit(buf, "%llu\n", atomic64_read(&cma->nr_pages_failed)); } CMA_ATTR_RO(alloc_pages_fail); static void cma_kobj_release(struct kobject *kobj) { - struct cma_stat *stat = container_of(kobj, struct cma_stat, kobj); + struct cma *cma = cma_from_kobj(kobj); + struct cma_kobject *cma_kobj = cma->cma_kobj; - kfree(stat); + kfree(cma_kobj); + cma->cma_kobj = NULL; } static struct attribute *cma_attrs[] = { @@ -67,44 +71,50 @@ ATTRIBUTE_GROUPS(cma); static struct kobj_type cma_ktype = { .release = cma_kobj_release, .sysfs_ops = &kobj_sysfs_ops, - .default_groups = cma_groups + .default_groups = cma_groups, }; static int __init cma_sysfs_init(void) { - int i = 0; + struct kobject *cma_kobj_root; + struct cma_kobject *cma_kobj; struct cma *cma; + unsigned int i; + int err; - cma_kobj = kobject_create_and_add("cma", mm_kobj); - if (!cma_kobj) + cma_kobj_root = kobject_create_and_add("cma", mm_kobj); + if (!cma_kobj_root) return -ENOMEM; - cma_stats = kmalloc_array(cma_area_count, sizeof(struct cma_stat), - GFP_KERNEL|__GFP_ZERO); - if (ZERO_OR_NULL_PTR(cma_stats)) - goto out; + for (i = 0; i < cma_area_count; i++) { + cma_kobj = kzalloc(sizeof(*cma_kobj), GFP_KERNEL); + if (!cma_kobj) { + err = -ENOMEM; + goto out; + } - do { cma = &cma_areas[i]; - cma->stat = &cma_stats[i]; - spin_lock_init(&cma->stat->lock); - if (kobject_init_and_add(&cma->stat->kobj, &cma_ktype, - cma_kobj, "%s", cma->name)) { - kobject_put(&cma->stat->kobj); + cma->cma_kobj = cma_kobj; + cma_kobj->cma = cma; + err = kobject_init_and_add(&cma_kobj->kobj, &cma_ktype, + cma_kobj_root, "%s", cma->name); + if (err) { + kobject_put(&cma_kobj->kobj); goto out; } - } while (++i < cma_area_count); + } return 0; out: while (--i >= 0) { cma = &cma_areas[i]; - kobject_put(&cma->stat->kobj); - } - kfree(cma_stats); - kobject_put(cma_kobj); + kobject_put(&cma->cma_kobj->kobj); + kfree(cma->cma_kobj); + cma->cma_kobj = NULL; + } + kobject_put(cma_kobj_root); - return -ENOMEM; + return err; } subsys_initcall(cma_sysfs_init);