All of lore.kernel.org
 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 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.