LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Yury Norov <yury.norov@gmail.com>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Barry Song <song.bao.hua@hisilicon.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Dave Hansen <dave.hansen@intel.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Randy Dunlap <rdunlap@infradead.org>,
	Alexander Gordeev <agordeev@linux.ibm.com>,
	Stefano Brivio <sbrivio@redhat.com>,
	"Ma, Jianpeng" <jianpeng.ma@intel.com>,
	Valentin Schneider <valentin.schneider@arm.com>,
	"Peter Zijlstra (Intel)" <peterz@infradead.org>,
	Daniel Bristot de Oliveira <bristot@redhat.com>,
	Guodong Xu <guodong.xu@linaro.org>,
	tangchengchang@huawei.com,
	"Zengtao (B)" <prime.zeng@hisilicon.com>,
	yangyicong <yangyicong@huawei.com>,
	tim.c.chen@linux.intel.com, Linuxarm <linuxarm@huawei.com>
Subject: Re: [PATCH v7 4/4] lib: test_bitmap: add bitmap_print_to_buf test cases
Date: Wed, 21 Jul 2021 13:40:36 +0200	[thread overview]
Message-ID: <YPgHtJqXi8cKad1Q@kroah.com> (raw)
In-Reply-To: <YPDDdAwgYEptz2Uq@yury-ThinkPad>

On Thu, Jul 15, 2021 at 04:23:32PM -0700, Yury Norov wrote:
> On Fri, Jul 16, 2021 at 12:32:45AM +0300, Andy Shevchenko wrote:
> > On Thu, Jul 15, 2021 at 11:48 PM Yury Norov <yury.norov@gmail.com> wrote:
> > > On Thu, Jul 15, 2021 at 03:09:39PM +0300, Andy Shevchenko wrote:
> > > > On Thu, Jul 15, 2021 at 11:58:56PM +1200, Barry Song wrote:
> > > > > The added test items cover both cases where bitmap buf of the printed
> > > > > result is greater than and less than 4KB.
> > > > > And it also covers the case where offset for bitmap_print_to_buf is
> > > > > non-zero which will happen when printed buf is larger than one page
> > > > > in sysfs bin_attribute.
> > > >
> > > > More test cases is always a good thing, thanks!
> > >
> > > Generally yes. But in this case... I believe, Barry didn't write that
> > > huge line below by himself. Most probably he copy-pasted the output of
> > > his bitmap_print_buf() into the test. If so, this code tests nothing,
> > > and just enforces current behavior of snprintf.
> > 
> > I'm not sure I got what you are telling me. The big line is to test
> > strings that are bigger than 4k.
> 
> I'm trying to say that human are not able to verify correctness of
> this line. The test is supposed to check bitmap_print_to_buf(), but
> reference output itself is generated by bitmap_print_to_buf(). This
> test will always pass by design, even if there's an error somewhere
> in the middle, isn't it?

Then please manually check it to verify it is correct or not.  Once we
have it verified, that's fine, it will remain static in this test for
always going forward.

That's what "oracles" are for, there is nothing wrong with this test
case or "proof" that I can see.

> > 
> > ...
> > 
> > > > > +static const char large_list[] __initconst = /* more than 4KB */
> > > > > +   "0,4,8,12,16,20,24,28,32-33,36-37,40-41,44-45,48-49,52-53,56-57,60-61,64,68,72,76,80,84,88,92,96-97,100-101,104-1"
> > > > > +   "05,108-109,112-113,116-117,120-121,124-125,128,132,136,140,144,148,152,156,160-161,164-165,168-169,172-173,176-1"
> > > > > +   "77,180-181,184-185,188-189,192,196,200,204,208,212,216,220,224-225,228-229,232-233,236-237,240-241,244-245,248-2"
> > >
> > > I don't like this behavior of the code: each individual line is not a
> > > valid bitmap_list. I would prefer to split original bitmap and print
> > > list representation of parts in a compatible format; considering a
> > > receiving part of this splitting machinery.
> > 
> > I agree that split is not the best here, but after all it's only 1
> > line and this is on purpose.
> 
> What I see is that bitmap_print_to_buf() is called many times,

That is not what the above list shows at all, it's one long string all
together, only split up to make it easier for us to work with.

> and
> each time it returns something that is not a valid bitmap list string.
> If the caller was be able to concatenate all the lines returned by
> bitmap_print_to_buf(), he'd probably get correct result. But in such
> case, why don't he use scnprintf("pbl") directly?

I do not understand the objection here at all.  This series is fixing a
real problem that people are having and your complaining about test
strings is _VERY_ odd.

If you have an alternate solution, please propose it, otherwise I will
be taking this series in the next few days.

thanks,

greg k-h

  parent reply	other threads:[~2021-07-21 11:55 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-15 11:58 [PATCH v7 0/4] use bin_attribute to break the size limitation of cpumap ABI 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)
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 [this message]
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=YPgHtJqXi8cKad1Q@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=agordeev@linux.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=andy.shevchenko@gmail.com \
    --cc=bristot@redhat.com \
    --cc=dave.hansen@intel.com \
    --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=song.bao.hua@hisilicon.com \
    --cc=tangchengchang@huawei.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 4/4] lib: test_bitmap: add bitmap_print_to_buf test cases' \
    /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).