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 X-Spam-Level: X-Spam-Status: No, score=-8.8 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,INCLUDES_CR_TRAILER,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED 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 5B4EDC433E0 for ; Wed, 24 Mar 2021 21:55:20 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id E0FA661A13 for ; Wed, 24 Mar 2021 21:55:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E0FA661A13 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 5CEFD6B0322; Wed, 24 Mar 2021 17:55:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 57E136B0323; Wed, 24 Mar 2021 17:55:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 3AB066B0324; Wed, 24 Mar 2021 17:55:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0001.hostedemail.com [216.40.44.1]) by kanga.kvack.org (Postfix) with ESMTP id 1EF8A6B0322 for ; Wed, 24 Mar 2021 17:55:19 -0400 (EDT) Received: from smtpin25.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id D36A5181B10E2 for ; Wed, 24 Mar 2021 21:55:18 +0000 (UTC) X-FDA: 77956124316.25.2BACF5F Received: from mail-pf1-f169.google.com (mail-pf1-f169.google.com [209.85.210.169]) by imf12.hostedemail.com (Postfix) with ESMTP id 7734C139 for ; Wed, 24 Mar 2021 21:55:17 +0000 (UTC) Received: by mail-pf1-f169.google.com with SMTP id l3so18510102pfc.7 for ; Wed, 24 Mar 2021 14:55:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to; bh=wXYRQi+4Ua6dJ+RlkFOgEs3wMt45QU3vsQsKu6v7/e0=; b=jID9UGQgQc9JY2nHQbuDNioFRtqow14sa55mHg8r21Vmr003XsHJpUqz9UniBwygKt y04vTqMukhaO8pc6SGZCfrW2CphWNbs76ZEyQEmrjS+q0+RM4sqD3gFfWvS8dl/BbmVe yZtqwE8LHr3qQGDLDBY9g8R/Xk9e/b+keQowEW3deQo6UX+7AqVisl0d462XlbFwgRi1 Kx1sIFupO/FRR35FycxL4kjEBInIm3guoDL3MPYJ9t/WmHRWuKlUVDC1g5R3UqZMmVRB TAaytiJTQ6cGIKkrruh5KoDa1G6h63RrBqGcSQtDSX4TsLmUHbX3ZPcYujE9g/6DTHPt QCeQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition :content-transfer-encoding:in-reply-to; bh=wXYRQi+4Ua6dJ+RlkFOgEs3wMt45QU3vsQsKu6v7/e0=; b=JWddSCA58jBJ3DGUIlXPZxjsTgnMlSTir7fokQCM6cpd3P+PqN8bozcj+EX8cETTmy KOL0ZOqYPzcsVGVtEoEa2K/QEi6YHkGy9/QAGrQ7wiVYajVsDOyStMBcTwDDb77iXqZj GL348bOcwzWR0nGmrKRavK+GadDkHoRu7xoSzsAKoJZdB7iNg0UcAda46oA0OT3aRpZY E882bWYWmArPiCMNmzsoiM3FJrkgQnVjclssd0nmtxxvVT4N/bfq5aoQEQdSBq3zuwn0 q5eGHrLyUqzfWcg1LpXj2brKBRiU9hpcTwxWzC0g3cGZ5GmRYujVRLFBiALVkS0nzq0h GzMg== X-Gm-Message-State: AOAM532KE2ZqjVdFzUUvYBvBw6or03Osj3I5DcHZJEh2Q/ePsCaYz698 ZJKC7VekkiSHcl03IBl7Bjk= X-Google-Smtp-Source: ABdhPJzWxUI8dDNb680lACSnDlagTQlM8wr2Dv/9QmxM0cwMKN/KP2NKF7wudnR5ihBdfJ7DMWV0ZQ== X-Received: by 2002:a17:903:1cc:b029:e6:f37a:2185 with SMTP id e12-20020a17090301ccb02900e6f37a2185mr5636836plh.67.1616622917204; Wed, 24 Mar 2021 14:55:17 -0700 (PDT) Received: from google.com ([2620:15c:211:201:7dfa:1e53:536:7976]) by smtp.gmail.com with ESMTPSA id l20sm3653490pfd.82.2021.03.24.14.55.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 24 Mar 2021 14:55:16 -0700 (PDT) Date: Wed, 24 Mar 2021 14:55:14 -0700 From: Minchan Kim To: Dmitry Osipenko Cc: Andrew Morton , linux-mm , LKML , gregkh@linuxfoundation.org, surenb@google.com, joaodias@google.com, jhubbard@nvidia.com, willy@infradead.org, Colin Ian King Subject: Re: [PATCH v7] mm: cma: support sysfs Message-ID: References: <20210324205503.2132082-1-minchan@kernel.org> <65840bfd-4471-7c8d-ce71-c4705baf3bfe@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <65840bfd-4471-7c8d-ce71-c4705baf3bfe@gmail.com> X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 7734C139 X-Stat-Signature: gqewpn6rtsrha9n7hf7tg38swptr6o31 Received-SPF: none (gmail.com>: No applicable sender policy available) receiver=imf12; identity=mailfrom; envelope-from=""; helo=mail-pf1-f169.google.com; client-ip=209.85.210.169 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1616622917-708557 Content-Transfer-Encoding: quoted-printable 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: On Thu, Mar 25, 2021 at 12:31:51AM +0300, Dmitry Osipenko wrote: > 24.03.2021 23:55, Minchan Kim =D0=BF=D0=B8=D1=88=D0=B5=D1=82: > > Since CMA is getting used more widely, it's more important to > > keep monitoring CMA statistics for system health since it's > > directly related to user experience. > >=20 > > This patch introduces sysfs statistics for CMA, in order to provide > > some basic monitoring of the CMA allocator. > >=20 > > * the number of CMA page successful allocations > > * the number of CMA page allocation failures > >=20 > > These two values allow the user to calcuate the allocation > > failure rate for each CMA area. > >=20 > > e.g.) > > /sys/kernel/mm/cma/WIFI/alloc_pages_[success|fail] > > /sys/kernel/mm/cma/SENSOR/alloc_pages_[success|fail] > > /sys/kernel/mm/cma/BLUETOOTH/alloc_pages_[success|fail] > >=20 > > The cma_stat was intentionally allocated by dynamic allocation > > to harmonize with kobject lifetime management. > > https://lore.kernel.org/linux-mm/YCOAmXqt6dZkCQYs@kroah.com/ > >=20 > > Reported-by: Dmitry Osipenko > > Tested-by: Dmitry Osipenko > > Suggested-by: Dmitry Osipenko >=20 > The tags are incorrect, I haven't suggested this change. During the development, you have suggested many things to make it clean. That suggested-by couldn't represent all the detail but wanted to give credit for you, too since you spent the time to make it better. Okay, since you didn't like it, I will remove it. >=20 > > Suggested-by: John Hubbard > > Suggested-by: Matthew Wilcox > > Reviewed-by: Greg Kroah-Hartman > > Reviewed-by: John Hubbard >=20 > > Addresses-Coverity: ("Dereference after null check") >=20 > There are no dereferences fixed by this patch. Let me add this: https://lore.kernel.org/linux-mm/20210316100433.17665-1-colin.king@canoni= cal.com/ >=20 > > Signed-off-by: Colin Ian King > > Signed-off-by: Minchan Kim > > --- > ... >=20 > > #include > > +#include > > + > > +struct cma_kobject { > > + struct cma *cma; > > + struct kobject kobj; >=20 > If you'll place the kobj as the first member of the struct, then > container_of will be a no-op. Cool. >=20 > ... > > +#include > > +#include > > +#include > > + > > +#include "cma.h" > > + > > +void cma_sysfs_account_success_pages(struct cma *cma, unsigned long = nr_pages) > > +{ > > + atomic64_add(nr_pages, &cma->nr_pages_succeeded); > > +} > > + > > +void cma_sysfs_account_fail_pages(struct cma *cma, unsigned long nr_= pages) > > +{ > > + atomic64_add(nr_pages, &cma->nr_pages_failed); > > +} > > + > > +#define CMA_ATTR_RO(_name) \ > > + static struct kobj_attribute _name##_attr =3D __ATTR_RO(_name) >=20 > nit: #defines and inlined helpers typically are placed at the top of th= e > code, after includes. No problem since I should resend anyway. >=20 > > +static inline struct cma *cma_from_kobj(struct kobject *kobj) > > +{ > > + struct cma_kobject *cma_kobj =3D container_of(kobj, struct cma_kobj= ect, > > + kobj); > > + struct cma *cma =3D cma_kobj->cma; > > + > > + return cma; >=20 > nit: you can write this as: >=20 > return container_of(kobj, struct cma_kobject, kobj)->cma; Better. >=20 > > +} > > + > > +static ssize_t alloc_pages_success_show(struct kobject *kobj, > > + struct kobj_attribute *attr, char *buf) > > +{ > > + struct cma *cma =3D cma_from_kobj(kobj); > > + > > + return sysfs_emit(buf, "%llu\n", > > + atomic64_read(&cma->nr_pages_succeeded)); >=20 > nit: Algnment isn't right, should be better to write it as single line,= IMO. Let me make it align rather than single line since I know someone who keep asking us to not overflow 80 columns unless it's special. >=20 > ... > > +static int __init cma_sysfs_init(void) > > +{ > > + struct kobject *cma_kobj_root; > > + struct cma_kobject *cma_kobj; > > + struct cma *cma; > > + int i, err; > > + > > + cma_kobj_root =3D kobject_create_and_add("cma", mm_kobj); > > + if (!cma_kobj_root) > > + return -ENOMEM; > > + > > + for (i =3D 0; i < cma_area_count; i++) { > > + cma_kobj =3D kzalloc(sizeof(*cma_kobj), GFP_KERNEL); > > + if (!cma_kobj) { > > + err =3D -ENOMEM; > > + goto out; > > + } > > + > > + cma =3D &cma_areas[i]; > > + cma->cma_kobj =3D cma_kobj; > > + cma_kobj->cma =3D cma; > > + err =3D kobject_init_and_add(&cma_kobj->kobj, &cma_ktype, > > + cma_kobj_root, "%s", cma->name); >=20 > nit: Previousy algnment of the code was better here. Yub. >=20 > Otherwise this is okay to me: >=20 > Reviewed-by: Dmitry Osipenko > Tested-by: Dmitry Osipenko Thanks!