linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Song Bao Hua (Barry Song)" <song.bao.hua@hisilicon.com>
To: Yury Norov <yury.norov@gmail.com>
Cc: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"andriy.shevchenko@linux.intel.com" 
	<andriy.shevchenko@linux.intel.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux@rasmusvillemoes.dk" <linux@rasmusvillemoes.dk>,
	"rafael@kernel.org" <rafael@kernel.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"rdunlap@infradead.org" <rdunlap@infradead.org>,
	"agordeev@linux.ibm.com" <agordeev@linux.ibm.com>,
	"sbrivio@redhat.com" <sbrivio@redhat.com>,
	"jianpeng.ma@intel.com" <jianpeng.ma@intel.com>,
	"valentin.schneider@arm.com" <valentin.schneider@arm.com>,
	"peterz@infradead.org" <peterz@infradead.org>,
	"bristot@redhat.com" <bristot@redhat.com>,
	"guodong.xu@linaro.org" <guodong.xu@linaro.org>,
	tangchengchang <tangchengchang@huawei.com>,
	"Zengtao (B)" <prime.zeng@hisilicon.com>,
	yangyicong <yangyicong@huawei.com>,
	"tim.c.chen@linux.intel.com" <tim.c.chen@linux.intel.com>,
	"tiantao (H)" <tiantao6@hisilicon.com>,
	Jonathan Cameron <jonathan.cameron@huawei.com>,
	Linuxarm <linuxarm@huawei.com>
Subject: RE: [PATCH v4 0/4] use bin_attribute to avoid cpumap buff overflow
Date: Thu, 17 Jun 2021 23:48:53 +0000	[thread overview]
Message-ID: <4ab928f1fb3e4420974dfafe4b32f5b7@hisilicon.com> (raw)
In-Reply-To: <YMu790ZvgzYWrJXV@yury-ThinkPad>



> -----Original Message-----
> From: Yury Norov [mailto:yury.norov@gmail.com]
> Sent: Friday, June 18, 2021 9:18 AM
> To: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com>
> Cc: gregkh@linuxfoundation.org; andriy.shevchenko@linux.intel.com;
> linux-kernel@vger.kernel.org; linux@rasmusvillemoes.dk; rafael@kernel.org;
> akpm@linux-foundation.org; rdunlap@infradead.org; agordeev@linux.ibm.com;
> sbrivio@redhat.com; jianpeng.ma@intel.com; valentin.schneider@arm.com;
> peterz@infradead.org; bristot@redhat.com; guodong.xu@linaro.org;
> tangchengchang <tangchengchang@huawei.com>; Zengtao (B)
> <prime.zeng@hisilicon.com>; yangyicong <yangyicong@huawei.com>;
> tim.c.chen@linux.intel.com; tiantao (H) <tiantao6@hisilicon.com>; Jonathan
> Cameron <jonathan.cameron@huawei.com>; Linuxarm <linuxarm@huawei.com>
> Subject: Re: [PATCH v4 0/4] use bin_attribute to avoid cpumap buff overflow
> 
> On Thu, Jun 17, 2021 at 10:19:06PM +1200, Barry Song wrote:
> > patch #1 adds a new function cpumap_print_to_buf and patch #2 uses
> > this function in drivers/base/topology.c, and patch #3 uses this new
> > function in drivers/base/node.c.
> > patch #4 adds test cases for the new API.
> 
> So, the root problem here is that some machines have so many CPUs that
> their cpumask text representation may not fit into the full page in the
> worst case. Is my understanding correct? Can you share an example of
> such configuration?

in my understanding, I have not found any machine which has really
caused the problem till now. But the whole story began from this
thread when Jonatah and me tried to add a new topology level-cluster
which exists on kunpeng920 and X86 Jacobsville:
https://lore.kernel.org/lkml/YFRGIedW1fUlnmi+@kroah.com/
https://lore.kernel.org/lkml/YFR2kwakbcGiI37w@kroah.com/

in the discussion, Greg had some concern about the potential
overflow of sysfs ABI for topology. Greg's comment is reasonable
and I think we should address the problem.

For this moment, both numa node and cpu use cpu bitmap like 3,ffffffff to
expose hardware topology. When cpu number is large, the page buffer of
sysfs will overflow. This doesn't really happen nowadays as the maximum
NR_CPUS is 8196 for X86_64 and 4096 for ARM64 since 8196 * 9 / 32 = 2305
is still smaller than 4KB page size.

So the existing BUILD_BUG_ON() in drivers/base/node.c is pretty much
preventing future problems similar with Y2K when hardware gets more
and more CPUs:
static ssize_t node_read_cpumap(struct device *dev, bool list, char *buf)
{
 	cpumask_var_t mask;
 	struct node *node_dev = to_node(dev);
 
	/* 2008/04/07: buf currently PAGE_SIZE, need 9 chars per 32 bits. */
	BUILD_BUG_ON((NR_CPUS/32 * 9) > (PAGE_SIZE-1));
}

But other ABIs exposing cpu lists are much more tricky, they could be
like: 0, 3, 5, 7, 9, 11... etc. so nobody knows the size till the last
moment.

So in the previous discussion, we agreed to remove this kind of
limitation totally and remove the BUILD_BUG_ON().

> 
> I think that the proper solution would be to create a function like
> void *cpumap_alloc_pagebuf(bool list), so that cpumap_print_to_pagebuf()
> will be always passed with enough portion of memory, and we'll just drop
> the PAGE_SIZE limit in cpumap_print_to_pagebuf().

I am sorry we missed you in the previous discussion:
https://lore.kernel.org/lkml/1619679819-45256-1-git-send-email-tiantao6@hisilicon.com/#t

I think we have figured out bin_attribute is the way to workaround
the limitation of sysfs:

https://lore.kernel.org/lkml/fd78ac30-dd3b-a7d7-eae8-193b09a7d49a@intel.com/
https://lore.kernel.org/lkml/YIueOR4fOYa1dSAb@kroah.com/

Again sorry for you were missed in the previous discussion.

> 
> >
> > v4:
> > add test cases for bitmap_print_to_buf API;
> > add Reviewed-by of Jonathan Cameron for patches 1-3, thanks!
> >
> > v3:
> > fixed the strlen issue and patch #1,#2,#3 minor formatting issues, thanks
> > to Andy Shevchenko and Jonathan Cameron.
> >
> > v2:
> > split the original patch #1 into two patches and use kasprintf() in
> > patch #1 to simplify the code. do some minor formatting adjustments.
> >
> > Barry Song (1):
> >   lib: test_bitmap: add bitmap_print_to_buf test cases
> >
> > Tian Tao (3):
> >   lib: bitmap: introduce bitmap_print_to_buf
> >   topology: use bin_attribute to avoid buff overflow
> >   drivers/base/node.c: use bin_attribute to avoid buff overflow
> >
> >  drivers/base/node.c     |  52 +++++++++-----
> >  drivers/base/topology.c | 115 +++++++++++++++++--------------
> >  include/linux/bitmap.h  |   2 +
> >  include/linux/cpumask.h |  21 ++++++
> >  lib/bitmap.c            |  37 +++++++++-
> >  lib/test_bitmap.c       | 149 ++++++++++++++++++++++++++++++++++++++++
> >  6 files changed, 304 insertions(+), 72 deletions(-)
> >
> > --
> > 2.25.1

Thanks
Barry


      reply	other threads:[~2021-06-17 23:48 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-17 10:19 Barry Song
2021-06-17 10:19 ` [PATCH v4 1/4] lib: bitmap: introduce bitmap_print_to_buf Barry Song
2021-06-17 10:47   ` Greg KH
2021-06-18  2:14     ` Song Bao Hua (Barry Song)
2021-06-17 20:54   ` Yury Norov
2021-07-01 11:59     ` Song Bao Hua (Barry Song)
2021-06-17 10:19 ` [PATCH v4 2/4] topology: use bin_attribute to avoid buff overflow Barry Song
2021-06-17 10:54   ` Greg KH
2021-06-17 11:06     ` Song Bao Hua (Barry Song)
2021-06-17 10:55   ` Greg KH
2021-06-17 11:03     ` Song Bao Hua (Barry Song)
2021-06-17 10:19 ` [PATCH v4 3/4] drivers/base/node.c: " Barry Song
2021-06-17 10:55   ` Greg KH
2021-06-17 10:19 ` [PATCH v4 4/4] lib: test_bitmap: add bitmap_print_to_buf test cases Barry Song
2021-06-17 21:17 ` [PATCH v4 0/4] use bin_attribute to avoid cpumap buff overflow Yury Norov
2021-06-17 23:48   ` Song Bao Hua (Barry Song) [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4ab928f1fb3e4420974dfafe4b32f5b7@hisilicon.com \
    --to=song.bao.hua@hisilicon.com \
    --cc=agordeev@linux.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=bristot@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=guodong.xu@linaro.org \
    --cc=jianpeng.ma@intel.com \
    --cc=jonathan.cameron@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=linuxarm@huawei.com \
    --cc=peterz@infradead.org \
    --cc=prime.zeng@hisilicon.com \
    --cc=rafael@kernel.org \
    --cc=rdunlap@infradead.org \
    --cc=sbrivio@redhat.com \
    --cc=tangchengchang@huawei.com \
    --cc=tiantao6@hisilicon.com \
    --cc=tim.c.chen@linux.intel.com \
    --cc=valentin.schneider@arm.com \
    --cc=yangyicong@huawei.com \
    --cc=yury.norov@gmail.com \
    --subject='RE: [PATCH v4 0/4] use bin_attribute to avoid cpumap buff overflow' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).