RCU Archive on lore.kernel.org
 help / color / Atom feed
From: "Paul E. McKenney" <paulmck@linux.ibm.com>
To: Joel Fernandes <joel@joelfernandes.org>
Cc: linux-kernel@vger.kernel.org, byungchul.park@lge.com,
	Davidlohr Bueso <dave@stgolabs.net>,
	Josh Triplett <josh@joshtriplett.org>,
	kernel-team@android.com, kernel-team@lge.com,
	Lai Jiangshan <jiangshanlai@gmail.com>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	max.byungchul.park@gmail.com, Rao Shoaib <rao.shoaib@oracle.com>,
	rcu@vger.kernel.org, Steven Rostedt <rostedt@goodmis.org>
Subject: Re: [PATCH v4 2/2] rcuperf: Add kfree_rcu() performance Tests
Date: Tue, 20 Aug 2019 17:44:36 -0700
Message-ID: <20190821004436.GB28441@linux.ibm.com> (raw)
In-Reply-To: <20190821003132.GA25611@google.com>

On Tue, Aug 20, 2019 at 08:31:32PM -0400, Joel Fernandes wrote:
> On Tue, Aug 20, 2019 at 08:27:05PM -0400, Joel Fernandes wrote:
> [snip]
> > > > > Or is the idea to time the kfree_rcu() loop separately?  (I don't see
> > > > > any such separate timing, though.)
> > > > 
> > > > The kmalloc() times are included within the kfree loop. The timing of
> > > > kfree_rcu() is not separate in my patch.
> > > 
> > > You lost me on this one.  What happens when you just interleave the
> > > kmalloc() and kfree_rcu(), without looping, compared to the looping
> > > above?  Does this get more expensive?  Cheaper?  More vulnerable to OOM?
> > > Something else?
> > 
> > You mean pairing a single kmalloc() with a single kfree_rcu() and doing this
> > several times? The results are very similar to doing kfree_alloc_num
> > kmalloc()s, then do kfree_alloc_num kfree_rcu()s; and repeat the whole thing
> > kfree_loops times (as done by this rcuperf patch we are reviewing).
> > 
> > Following are some numbers. One change is the case where we are not at all
> > batching does seem to complete even faster when we fully interleave kmalloc()
> > with kfree() while the case of batching in the same scenario completes at the
> > same time as did the "not fully interleaved" scenario. However, the grace
> > period reduction improvements and the chances of OOM'ing are pretty much the
> > same in either case.
> [snip]
> > Not fully interleaved: do kfree_alloc_num kmallocs, then do kfree_alloc_num kfree_rcu()s. And repeat this kfree_loops times.
> > =======================
> > (1) Batching
> > rcuperf.kfree_loops=20000 rcuperf.kfree_alloc_num=8000 rcuperf.kfree_no_batch=0 rcuperf.kfree_rcu_test=1
> > 
> > root@(none):/# free -m
> >               total        used        free      shared  buff/cache   available
> > Mem:            977         251         686           0          39         684
> > Swap:             0           0           0
> > 
> > [   15.574402] Total time taken by all kfree'ers: 14185970787 ns, loops: 20000, batches: 1548
> > 
> > (2) No Batching
> > rcuperf.kfree_loops=20000 rcuperf.kfree_alloc_num=8000 rcuperf.kfree_no_batch=1 rcuperf.kfree_rcu_test=1
> > 
> > root@(none):/# free -m
> >               total        used        free      shared  buff/cache   available
> > Mem:            977          82         855           0          39         853
> > Swap:             0           0           0
> > 
> > [   13.724554] Total time taken by all kfree'ers: 12246217291 ns, loops: 20000, batches: 7262
> 
> And the diff for changing the test to do this case is as follows (I don't
> plan to fold this diff in, since I feel the existing test suffices and
> results are similar):

But why not?  It does look to be a nice simplification, after all.

							Thanx, Paul

> diff --git a/kernel/rcu/rcuperf.c b/kernel/rcu/rcuperf.c
> index 46f9c4449348..e4e4be4aaf51 100644
> --- a/kernel/rcu/rcuperf.c
> +++ b/kernel/rcu/rcuperf.c
> @@ -618,18 +618,13 @@ kfree_perf_thread(void *arg)
>  {
>  	int i, loop = 0;
>  	long me = (long)arg;
> -	struct kfree_obj **alloc_ptrs;
> +	struct kfree_obj *alloc_ptr;
>  	u64 start_time, end_time;
>  
>  	VERBOSE_PERFOUT_STRING("kfree_perf_thread task started");
>  	set_cpus_allowed_ptr(current, cpumask_of(me % nr_cpu_ids));
>  	set_user_nice(current, MAX_NICE);
>  
> -	alloc_ptrs = (struct kfree_obj **)kmalloc(sizeof(struct kfree_obj *) * kfree_alloc_num,
> -						  GFP_KERNEL);
> -	if (!alloc_ptrs)
> -		return -ENOMEM;
> -
>  	start_time = ktime_get_mono_fast_ns();
>  
>  	if (atomic_inc_return(&n_kfree_perf_thread_started) >= kfree_nrealthreads) {
> @@ -646,19 +641,17 @@ kfree_perf_thread(void *arg)
>  	 */
>  	do {
>  		for (i = 0; i < kfree_alloc_num; i++) {
> -			alloc_ptrs[i] = kmalloc(sizeof(struct kfree_obj), GFP_KERNEL);
> -			if (!alloc_ptrs[i])
> +			alloc_ptr = kmalloc(sizeof(struct kfree_obj), GFP_KERNEL);
> +			if (!alloc_ptr)
>  				return -ENOMEM;
> -		}
>  
> -		for (i = 0; i < kfree_alloc_num; i++) {
>  			if (!kfree_no_batch) {
> -				kfree_rcu(alloc_ptrs[i], rh);
> +				kfree_rcu(alloc_ptr, rh);
>  			} else {
>  				rcu_callback_t cb;
>  
>  				cb = (rcu_callback_t)(unsigned long)offsetof(struct kfree_obj, rh);
> -				kfree_call_rcu_nobatch(&(alloc_ptrs[i]->rh), cb);
> +				kfree_call_rcu_nobatch(&(alloc_ptr->rh), cb);
>  			}
>  		}
>  
> @@ -682,7 +675,6 @@ kfree_perf_thread(void *arg)
>  		}
>  	}
>  
> -	kfree(alloc_ptrs);
>  	torture_kthread_stopping("kfree_perf_thread");
>  	return 0;
>  }

  reply index

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-14 16:04 [PATCH v4 1/2] rcu/tree: Add basic support for kfree_rcu() batching Joel Fernandes (Google)
2019-08-14 16:04 ` [PATCH v4 2/2] rcuperf: Add kfree_rcu() performance Tests Joel Fernandes (Google)
2019-08-14 22:58   ` Paul E. McKenney
2019-08-19 19:33     ` Joel Fernandes
2019-08-19 22:23       ` Paul E. McKenney
2019-08-19 23:51         ` Joel Fernandes
2019-08-20  2:50           ` Paul E. McKenney
2019-08-21  0:27             ` Joel Fernandes
2019-08-21  0:31               ` Joel Fernandes
2019-08-21  0:44                 ` Paul E. McKenney [this message]
2019-08-21  0:51                   ` Joel Fernandes
2019-08-16 16:43 ` [PATCH v4 1/2] rcu/tree: Add basic support for kfree_rcu() batching Paul E. McKenney
2019-08-16 17:44   ` Joel Fernandes
2019-08-16 19:16     ` Paul E. McKenney
2019-08-17  1:32       ` Joel Fernandes
2019-08-17  3:56         ` Paul E. McKenney
2019-08-17  4:30           ` Joel Fernandes
2019-08-17  5:20             ` Paul E. McKenney
2019-08-17  5:53               ` Joel Fernandes
2019-08-17 21:45                 ` Paul E. McKenney
2019-09-18  9:58 ` Uladzislau Rezki
2019-09-30 20:16   ` Joel Fernandes
2019-10-01 11:27     ` Uladzislau Rezki
2019-10-04 17:20       ` Joel Fernandes
2019-10-08 16:23         ` Uladzislau Rezki

Reply instructions:

You may reply publically 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=20190821004436.GB28441@linux.ibm.com \
    --to=paulmck@linux.ibm.com \
    --cc=byungchul.park@lge.com \
    --cc=dave@stgolabs.net \
    --cc=jiangshanlai@gmail.com \
    --cc=joel@joelfernandes.org \
    --cc=josh@joshtriplett.org \
    --cc=kernel-team@android.com \
    --cc=kernel-team@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=max.byungchul.park@gmail.com \
    --cc=rao.shoaib@oracle.com \
    --cc=rcu@vger.kernel.org \
    --cc=rostedt@goodmis.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

RCU Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/rcu/0 rcu/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 rcu rcu/ https://lore.kernel.org/rcu \
		rcu@vger.kernel.org
	public-inbox-index rcu

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.rcu


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git