linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Chen Gang <gang.chen@asianux.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 01:31:12 -0700	[thread overview]
Message-ID: <20131015083112.GH5790@linux.vnet.ibm.com> (raw)
In-Reply-To: <525C9D18.6040909@asianux.com>

On Tue, Oct 15, 2013 at 09:40:40AM +0800, Chen Gang wrote:
> 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.

Yes, the patch is about "use truncation instead of memory overflow",
but the truncation would also be a problem on large systems.  Is it
possible to prevent memory from overflow in the first place?

> 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.

True.  But you can make a change that is just as simple that allows you
to test what would happen on a 1024-CPU system even though your own
system has only 2 CPUs.  Can you see what that change is?

							Thanx, Paul

> 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  8:31 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
2013-10-15  8:31           ` Paul E. McKenney [this message]
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=20131015083112.GH5790@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=gang.chen@asianux.com \
    --cc=josh@freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    /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 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).