All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chen Gang <gang.chen@asianux.com>
To: paulmck@linux.vnet.ibm.com
Cc: josh@freedesktop.org,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [Suggestion] kernel/rcutorture.c: about using scnprintf() instead of sprintf().
Date: Tue, 15 Oct 2013 09:40:40 +0800	[thread overview]
Message-ID: <525C9D18.6040909@asianux.com> (raw)
In-Reply-To: <20131014112457.GN5790@linux.vnet.ibm.com>

On 10/14/2013 07:24 PM, Paul E. McKenney wrote:
> On Mon, Oct 14, 2013 at 10:22:20AM +0800, Chen Gang wrote:
>>>  - intend to shrink maximized buffer (PAGE_SIZE -> 64, 256 ..) for test.
> 
> This is a good start!  However, you should also test the original kernel
> to be sure that it really fails.  You should of course start by asking
> yourself how you would expect it to fail.
> 

When shrink size, for scnprintf() we already got truncation, that means
if we still use sprintf(), it will cause memory overflow.

For memory overflow, it does not means OS will stop quickly, one of the
worst condition is "it may still seems going 'well' but sometimes
may/will show some amazing things which is not easy to find root cause".

So for this kind of memory overflow, "shrinking size and letting
scnprintf() instead of sprintf() to see whether truncation" is well enough.


> What other change should you make to test this in order to be sure that
> it will really work when someone tries it on 1024 CPUs?  (I am asking
> rather than telling because you really need to have this stuff worked
> out on your initial submission.)
> 

Hmm... Can it really work on 1024 CPUs? sorry I don't know. But in fact,
that is not about this patch (it is just one of case which may cause
issues).

This patch is only about "use truncation instead of memory overflow, and
make sure the modification without negative effect". So in my opinion,
current test case is enough for this requirement.

Maybe you want to let this module get additional test to find additional
issues? (If so, I will try when my time resource is possible).


> Another way of thinking about this is to ask yourself the question "If
> someone ran rcutorture with torture_type=srcu on a system with 1024 CPUs
> (to say nothing of 4096 CPUs), what would they want to happen?"  Then:
> "How would I test for that on a smaller system?"
> 

We have to face the efficiency: it is only a test module, if the tester
(assume he/she is a programmer, too) notices about the truncation, they
can simply extend the maximize length to avoid truncation.

At least for me, "rcutorture.c" is really easy enough for a programmer
to test rcu (and also, it is really a useful tool for test rcu).

And for "1024 CPUs",  I think one of efficient way is: "when some
members use it under 1024 CPUs, if they find something can be
improvement, they can report/send related patch".


Hmm... maybe the comment "it is ... additional test or consideration"
need be removed: 'additional' and 'consideration' are not suitable words
to be appeared in comments (they are not precise word).


Thanks.
-- 
Chen Gang

  reply	other threads:[~2013-10-15  1:41 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-08  8:32 [Suggestion] kernel/rcutorture.c: about using scnprintf() instead of sprintf() Chen Gang
2013-10-13 11:05 ` Paul E. McKenney
2013-10-14  1:41   ` Chen Gang
2013-10-14  2:22     ` Chen Gang
2013-10-14 11:24       ` Paul E. McKenney
2013-10-15  1:40         ` Chen Gang [this message]
2013-10-15  8:31           ` Paul E. McKenney
2013-10-15  9:03             ` Chen Gang
2013-10-14  8:38   ` [PATCH] kernel/rcutorture.c: use " Chen Gang
2013-10-14 11:28     ` Paul E. McKenney
2013-10-15  0:54       ` Chen Gang
2013-10-15  1:51         ` Chen Gang
2013-10-15  8:26           ` Paul E. McKenney
2013-10-15 12:32             ` Chen Gang
2013-10-15 14:47               ` Paul E. McKenney
2013-10-16  2:07                 ` Chen Gang
2013-10-17  1:06                   ` Chen Gang
2013-10-21  5:51                     ` [PATCH] kernel/rcutorture.c: be sure of enough memory for result printing Chen Gang
2013-10-21  6:18                       ` Chen Gang
2013-10-21  9:35                         ` Chen Gang
2013-10-27 14:43                           ` Chen Gang
2013-11-04  9:42                             ` Chen Gang
2013-11-06 20:38                       ` Paul E. McKenney
2013-11-07  2:30                         ` [PATCH v2] " Chen Gang
2013-11-07 20:59                           ` Paul E. McKenney
2013-11-08  0:58                             ` Chen Gang

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=525C9D18.6040909@asianux.com \
    --to=gang.chen@asianux.com \
    --cc=josh@freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.