All of lore.kernel.org
 help / color / mirror / Atom feed
From: Frederic Weisbecker <frederic@kernel.org>
To: "Joel Fernandes (Google)" <joel@joelfernandes.org>
Cc: linux-kernel@vger.kernel.org,
	Josh Triplett <josh@joshtriplett.org>,
	Lai Jiangshan <jiangshanlai@gmail.com>,
	Marco Elver <elver@google.com>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	rcu@vger.kernel.org, Steven Rostedt <rostedt@goodmis.org>,
	"Uladzislau Rezki (Sony)" <urezki@gmail.com>,
	fweisbec@gmail.com, neeraj.iitr10@gmail.com
Subject: Re: [PATCH v8 2/6] rcu/segcblist: Add counters to segcblist datastructure
Date: Mon, 26 Oct 2020 01:32:12 +0100	[thread overview]
Message-ID: <20201026003212.GA104441@lothringen> (raw)
In-Reply-To: <20201021190813.3005054-3-joel@joelfernandes.org>

On Wed, Oct 21, 2020 at 03:08:09PM -0400, Joel Fernandes (Google) wrote:
> @@ -307,6 +317,7 @@ void rcu_segcblist_extract_done_cbs(struct rcu_segcblist *rsclp,
>  
>  	if (!rcu_segcblist_ready_cbs(rsclp))
>  		return; /* Nothing to do. */
> +	rclp->len = rcu_segcblist_get_seglen(rsclp, RCU_DONE_TAIL);

I realize, doesn't it break the unsegmented count in srcu_invoke_callbacks() ?

It still does rcu_segcblist_insert_count(), so it adds zero to rsclp->len
which thus doesn't get cleared and probably keeps growing.

At least srcu_barrier() relies on it. And module exit should warn.

>  	*rclp->tail = rsclp->head;
>  	WRITE_ONCE(rsclp->head, *rsclp->tails[RCU_DONE_TAIL]);
>  	WRITE_ONCE(*rsclp->tails[RCU_DONE_TAIL], NULL);
> @@ -314,6 +325,7 @@ void rcu_segcblist_extract_done_cbs(struct rcu_segcblist *rsclp,
>  	for (i = RCU_CBLIST_NSEGS - 1; i >= RCU_DONE_TAIL; i--)
>  		if (rsclp->tails[i] == rsclp->tails[RCU_DONE_TAIL])
>  			WRITE_ONCE(rsclp->tails[i], &rsclp->head);
> +	rcu_segcblist_set_seglen(rsclp, RCU_DONE_TAIL, 0);
>  }
>  
>  /*
> @@ -330,11 +342,16 @@ void rcu_segcblist_extract_pend_cbs(struct rcu_segcblist *rsclp,
>  
>  	if (!rcu_segcblist_pend_cbs(rsclp))
>  		return; /* Nothing to do. */
> +	rclp->len = rcu_segcblist_get_seglen(rsclp, RCU_WAIT_TAIL) +
> +		    rcu_segcblist_get_seglen(rsclp, RCU_NEXT_READY_TAIL) +
> +		    rcu_segcblist_get_seglen(rsclp, RCU_NEXT_TAIL);
>  	*rclp->tail = *rsclp->tails[RCU_DONE_TAIL];
>  	rclp->tail = rsclp->tails[RCU_NEXT_TAIL];
>  	WRITE_ONCE(*rsclp->tails[RCU_DONE_TAIL], NULL);
> -	for (i = RCU_DONE_TAIL + 1; i < RCU_CBLIST_NSEGS; i++)
> +	for (i = RCU_DONE_TAIL + 1; i < RCU_CBLIST_NSEGS; i++) {
>  		WRITE_ONCE(rsclp->tails[i], rsclp->tails[RCU_DONE_TAIL]);
> +		rcu_segcblist_set_seglen(rsclp, i, 0);

You seem to have forgotten the suggestion?

    rclp->len += rcu_segcblist_get_seglen(rsclp, i)


Thanks.

  reply	other threads:[~2020-10-26  0:32 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-21 19:08 [PATCH v8 0/6] Add support for length of each segment in the segcblist Joel Fernandes (Google)
2020-10-21 19:08 ` [PATCH v8 1/6] rcu/tree: Make rcu_do_batch count how many callbacks were executed Joel Fernandes (Google)
2020-10-21 19:08 ` [PATCH v8 2/6] rcu/segcblist: Add counters to segcblist datastructure Joel Fernandes (Google)
2020-10-26  0:32   ` Frederic Weisbecker [this message]
2020-10-26  5:40     ` Joel Fernandes
2020-10-26 11:24       ` Frederic Weisbecker
2020-10-27 17:30         ` Joel Fernandes
2020-10-26  0:50   ` Frederic Weisbecker
2020-10-26  5:45     ` Joel Fernandes
2020-10-26 11:28       ` Frederic Weisbecker
2020-10-21 19:08 ` [PATCH v8 3/6] rcu/trace: Add tracing for how segcb list changes Joel Fernandes (Google)
2020-10-26 11:59   ` Frederic Weisbecker
2020-11-03 13:47     ` Joel Fernandes
2020-11-05  3:37       ` Paul E. McKenney
2020-10-21 19:08 ` [PATCH v8 4/6] rcu/segcblist: Remove useless rcupdate.h include Joel Fernandes (Google)
2020-10-21 19:08 ` [PATCH v8 5/6] rcu/tree: segcblist: Remove redundant smp_mb()s Joel Fernandes (Google)
2020-10-26 12:01   ` Frederic Weisbecker
2020-10-21 19:08 ` [PATCH v8 6/6] rcu/segcblist: Add additional comments to explain smp_mb() Joel Fernandes (Google)

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=20201026003212.GA104441@lothringen \
    --to=frederic@kernel.org \
    --cc=elver@google.com \
    --cc=fweisbec@gmail.com \
    --cc=jiangshanlai@gmail.com \
    --cc=joel@joelfernandes.org \
    --cc=josh@joshtriplett.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=neeraj.iitr10@gmail.com \
    --cc=paulmck@kernel.org \
    --cc=rcu@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=urezki@gmail.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.