LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Song Bao Hua (Barry Song)" <song.bao.hua@hisilicon.com>
To: "andriy.shevchenko@linux.intel.com" 
	<andriy.shevchenko@linux.intel.com>,
	Yury Norov <yury.norov@gmail.com>
Cc: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"dave.hansen@intel.com" <dave.hansen@intel.com>,
	"linux@rasmusvillemoes.dk" <linux@rasmusvillemoes.dk>,
	"rafael@kernel.org" <rafael@kernel.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>,
	Linuxarm <linuxarm@huawei.com>,
	"tiantao (H)" <tiantao6@hisilicon.com>
Subject: RE: [PATCH v7 2/4] topology: use bin_attribute to break the size limitation of cpumap ABI
Date: Mon, 19 Jul 2021 11:10:45 +0000	[thread overview]
Message-ID: <4ab537937f344893bf5bdcb13e46ce04@hisilicon.com> (raw)
In-Reply-To: <YPVAxnR95M0mZWDY@smile.fi.intel.com>



> -----Original Message-----
> From: andriy.shevchenko@linux.intel.com
> [mailto:andriy.shevchenko@linux.intel.com]
> Sent: Monday, July 19, 2021 9:07 PM
> To: Yury Norov <yury.norov@gmail.com>
> Cc: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com>;
> gregkh@linuxfoundation.org; akpm@linux-foundation.org;
> linux-kernel@vger.kernel.org; dave.hansen@intel.com;
> linux@rasmusvillemoes.dk; rafael@kernel.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; Linuxarm <linuxarm@huawei.com>; tiantao (H)
> <tiantao6@hisilicon.com>
> Subject: Re: [PATCH v7 2/4] topology: use bin_attribute to break the size
> limitation of cpumap ABI
> 
> On Fri, Jul 16, 2021 at 06:12:21PM -0700, Yury Norov wrote:
> > On Sat, Jul 17, 2021 at 12:16:48AM +0000, Song Bao Hua (Barry Song) wrote:
> > > > From: Yury Norov [mailto:yury.norov@gmail.com]
> > > > Sent: Saturday, July 17, 2021 8:04 AM
> > > > On Fri, Jul 16, 2021 at 08:49:58AM +0000, Song Bao Hua (Barry Song) wrote:
> 
> ...
> 
> > > Generally good idea. However, for sysfs ABI entries, it might not be
> > > that true.
> > >
> > > A sysfs entry might never be read for its whole life. As I explained
> > > before, a sysfs entry - especially for list,  is randomly "cat" by users.
> > > Many of them won't be read forever. And after they are read once, they
> > > will probably never be read again. The operations to read ABI could be
> > > random and rare. Performance wouldn't be a concern.
> > >
> > > To avoid holding the memory which might never be used, it is better to
> > > allocate and free the memory during runtime. I mean to allocate in show()
> > > and free in show(), aka, to do it on demand.
> > >
> > > For example, for a server with 256CPU and each cpu has dozens of sysfs ABI
> > > entries, only a few of sysfs list entries might be randomly "cat" by users.
> > > Holding 256*entries memory doesn't look good.
> >
> > Ok, makes sense.
> >
> > > > This would require to add bitmap_max_string_size(list, bitmap, nbits),
> > > > but it's O(1), and I think, others will find it helpful.
> > >
> > > What about getting size and memory at the same time?
> >
> > 1. We already have kasprintf()
> > 2. It breaks coding style.
> >
> > Documentation/process/coding-style.rst:
> >         Functions should be short and sweet, and do just one thing.
> >
> > From practical point of view, there should be some balance between
> > granularity and ease-of-use. But in this case, bitmap_list cries for
> > a function that will help to estimate size of output buffer.
> 
> According to the vsnprintf() logic the estimated size is what it returns. If
> user supplies too few bytes available, the comparison with the returned value
> can tell caller that space wasn't big enough.

As far as my understanding, for estimated size in bitmap_max_string_size()
Yury might mean something as below?

* For bitmask:
Each 32bit needs 9 bytes "11111111,", so the max size of mask would be:
9*nr_cpus / 32 ?

* For list:
Maximally  cpu0-9 need 2bytes, like "1,"
Maximally cpu10-99 need 3bytes, like "50,"
Maximally cpu100-999 need 4bytes, like "101,"
Maximally cpu1000-9999 need 5 bytes..
 
So once we know the size of the bitmap, we can figure out the maximum
size of its string for mask and list?

Pls correct me if you don't mean this, yury.


> 
> > And it's
> > easy to imagine a case where the estimated length of bitmap is needed
> > explicitly:
> >
> >         size_t max_size = bitmap_max_string_size(nbits);
> >         char *buf = kmalloc(PAGE_ALIGN(max_size) * nr_cpus);
> >
> > Thought, I don't insist. In your driver you can do:
> >
> >         size_t size = snprintf(NULL, 0, ...);
> >         void *buf = kmalloc(size);
> >
> > It will be fully correct, and you already have everything you need.
> >
> > > ssize_t bitmap_get_print_buf(bool list, char **buf, const unsigned long
> > >  *maskp, int nmaskbits)
> > >
> > > ssize_t cpumap_get_print_buf(bool list, char **buf, const struct cpumask
> *mask);
> > >
> > > This API returns the size of printed buffer, and it also gets the
> > > printed result saved in *buf. Then drivers don't need to do three
> > > steps:
> > >
> > > 1. get cpumap buffer size which is your cpumap_max_string_size()
> > > 2. allocate memory for buffer according to size got in step 1
> > > 3. print bitmap(cpumap) to buffer by "pbl"
> > >
> > > It will only need to call bitmap_get_print_buf() and all three
> > > things are done inside bitmap_get_print_buf().
> > >
> > > How to use the size and memory allocated in cpumap_get_print_buf
> > > will be totally up to users.
> > >
> > > The other benefit for this is that if we get string size during initialization,
> > > and then we print in show() entries, the size got at the beginning might
> be not
> > > enough as system topology might have changed. Sysfs ABI reflects the status
> of
> > > system at this moment.
> 
> --
> With Best Regards,
> Andy Shevchenko
> 

Thanks
Barry


  reply	other threads:[~2021-07-19 11:10 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-15 11:58 [PATCH v7 0/4] " Barry Song
2021-07-15 11:58 ` [PATCH v7 1/4] cpumask: introduce cpumap_print_to_buf to support large bitmask and list Barry Song
2021-07-15 15:28   ` Yury Norov
2021-07-15 21:08     ` Song Bao Hua (Barry Song)
2021-07-16  0:57     ` Song Bao Hua (Barry Song)
2021-07-15 11:58 ` [PATCH v7 2/4] topology: use bin_attribute to break the size limitation of cpumap ABI Barry Song
2021-07-16  8:49   ` Song Bao Hua (Barry Song)
2021-07-16 20:04     ` Yury Norov
2021-07-17  0:16       ` Song Bao Hua (Barry Song)
2021-07-17  1:12         ` Yury Norov
2021-07-19  9:07           ` andriy.shevchenko
2021-07-19 11:10             ` Song Bao Hua (Barry Song) [this message]
2021-07-19 17:10               ` Yury Norov
2021-07-21  9:30                 ` Song Bao Hua (Barry Song)
2021-07-15 11:58 ` [PATCH v7 3/4] drivers/base/node.c: " Barry Song
2021-07-15 11:58 ` [PATCH v7 4/4] lib: test_bitmap: add bitmap_print_to_buf test cases Barry Song
2021-07-15 12:09   ` Andy Shevchenko
2021-07-15 20:47     ` Yury Norov
2021-07-15 21:32       ` Andy Shevchenko
2021-07-15 23:23         ` Yury Norov
2021-07-16  0:41           ` Song Bao Hua (Barry Song)
2021-07-21 11:40           ` Greg Kroah-Hartman
2021-07-22 17:09             ` Yury Norov
2021-07-22 17:47               ` Greg Kroah-Hartman
2021-07-22 18:27                 ` Yury Norov
2021-07-22 18:36                   ` Andy Shevchenko
2021-07-22 18:45                   ` Greg Kroah-Hartman
2021-07-28  9:08                     ` Song Bao Hua (Barry Song)
2021-07-28  9:24                       ` Andy Shevchenko
2021-07-15 15:36   ` Yury Norov
2021-07-28 13:41 ` [PATCH v7 0/4] use bin_attribute to break the size limitation of cpumap ABI Greg KH
2021-07-28 14:53   ` Yury Norov
2021-07-28 15:25     ` Greg KH
2021-07-28 18:58 ` [PATCH] bitmap: extend comment to bitmap_print_to_buf Yury Norov
2021-07-29  6:04   ` Song Bao Hua (Barry Song)

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=4ab537937f344893bf5bdcb13e46ce04@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=dave.hansen@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=guodong.xu@linaro.org \
    --cc=jianpeng.ma@intel.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 v7 2/4] topology: use bin_attribute to break the size limitation of cpumap ABI' \
    /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).