Message-Id: <20170609172038.GH3721@linux.vnet.ibm.com>
Date:   Fri, 9 Jun 2017 10:20:38 -0700
From:   "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     linux-kernel@...r.kernel.org, mingo@...nel.org,
        jiangshanlai@...il.com, dipankar@...ibm.com,
        akpm@...ux-foundation.org, mathieu.desnoyers@...icios.com,
        josh@...htriplett.org, tglx@...utronix.de, peterz@...radead.org,
        dhowells@...hat.com, edumazet@...gle.com, fweisbec@...il.com,
        oleg@...hat.com, bobby.prani@...il.com
Subject: Re: [PATCH tip/core/rcu 0/88] Commits for 4.13

On Fri, Jun 09, 2017 at 12:39:40PM -0400, Steven Rostedt wrote:
> On Fri, 9 Jun 2017 09:24:07 -0700
> "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com> wrote:
> 
> > On Fri, Jun 09, 2017 at 09:52:10AM -0400, Steven Rostedt wrote:
> > > On Thu, 25 May 2017 14:59:34 -0700
> > > "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com> wrote:
> > >   
> > > > Hello!
> > > > 
> > > > This rather long series mostly removes unused features, shrinks the
> > > > include/linux/rcupdate.h file's .i intermediate-output size, updates
> > > > rcutorture testing, and supplies miscellaneous fixes.  Branching proved
> > > > impractical due to the large footprint of many of the commits, hence the
> > > > long linear series.  On the other hand, this series's diffstat summary
> > > > line is as follows:
> > > > 
> > > >  87 files changed, 1745 insertions(+), 4389 deletions(-)
> > > >  
> > > 
> > > Hi Paul,
> > > 
> > > 88 patches is quite overwhelming. I would recommend breaking something
> > > like this up into multiple patch series with different topics. One
> > > could be the ones that affect seftests only. Another for srcu, another
> > > for documentation, etc.  
> > 
> > No argument!  I usually do that, and will do that in the future, but
> > these patches have many large overlapping pieces, and thus more than
> > the usual conflicts.  I do apologize, but there was just too much
> > overlap between too many commits to make branches this time.
> 
> Hmm, that's sad, but still. Do the selftests really conflict with the
> other parts of the code? Or could that have come as as separate list?

The problem is that some of the self-tests are intertwined with the
addition of things being tested, so in a number of cases, breaking
them out would break the build.

For another example, you remember that little dyntick-idle patch of mine
that accompanied your patches handling the can't-trace window in RCU's
dyntick-idle processing?  That patch generated conflicts no fewer than
four rather diverse patches in the current series.

There are some things that I could break out, but the branches end up
being not topic branches, but rather branches of random unrelated commits
that by random chance could be broken out.  :-/

> > > When one gets 88 patches and sees that it's a hodge podge of various
> > > parts of RCU, they tend to just ignore the entire series. If you want
> > > reviewers, I strongly recommend breaking it down nicer so that those
> > > that are interested in only parts of RCU will be more likely to review
> > > the patches. Otherwise, people will just say "I don't have time to sort
> > > through all this to find what I'm interested in reviewing", and skip
> > > the entire series.  
> > 
> > Again, the next series will have the usual branches.  In the meantime,
> > how about a topic index to the current series, perhaps as shown below?
> > I have reproduced the 0/88 list of patches below to make it easier
> > to locate patches of interest under a given topic.
> > 
> > Does that help?
> 
> Not really. It brings me back to the 80s when I use to walk into the
> library and search through the index cards to find the topics I had to
> do my report on. Those days are gone. We are now in the age of quick
> results. Hitting the "I feel lucky" or die crowd in the google search
> of immediate satisfaction!

You know, there really should be some software tool, that given topic
markings, could present on organized view of commits.

But enough fantasizing about possible futures.  Any thoughts on what
could be done to help with this situation in the here and now?

							Thanx, Paul

> -- Steve
> 
> > 
> > 							Thanx, Paul
> > 
> > ------------------------------------------------------------------------
> > Topics:
> > ------------------------------------------------------------------------
> > 
> > Documentation: 19, 20, 21, 30, 31.
> > 
> > Miscellaneous fixes: 6, 10, 13, 16, 18, 32, 33-35, 37-38, 44, 45, 58, 69. 
> > 
> > rcuperf (performance test): 11-12, 14, 18, 22, 28.
> > 
> > rcutorture: 7-8, 24, 27, 29, 81.
> > 
> > Simplification/shrinking: 25, 26, 42-43, 49-57, 59-65, 70, 72-80,
> > 	82-84, 85-86, 87-88.
> > 
> > SRCU: 9, 23, 36, 39, 40, 41, 47, 66, 68, 71-73.
> > 
> > SRCU rcutorture: 1-5, 15.
> > 
> > Deferred to the v4.14 merge window: 46, 48.
> > 
> > ------------------------------------------------------------------------
> > List of patches in numerical order:
> > ------------------------------------------------------------------------
> > 
> > 1-5.	Adjust rcutorture testing to better cover SRCU.
> > 
> > 6.	Performance fix that prevents rcu_barrier() from starting
> > 	needless grace periods.
> > 
> > 7-8.	Fix rcutorture bugs that were failing to test certain
> > 	Kconfig options in some rcutorture scenarios.
> > 
> > 9.	Fix a long-standing counter-wrap bug in SRCU.
> > 
> > 10.	Fix a bug where preemptible RCU would fail to complain about
> > 	blocking (as opposed to preemption) within an RCU read-side
> > 	critical section.
> > 
> > 11-12.	Fix argument-checking bug in the rcuperf performance/scalability
> > 	checking module and remove conflicting Kconfig options.
> > 
> > 13.	Remove obsolete references to the long-departed synchronize_kernel()
> > 	RCU API member.
> > 
> > 14.	Upgrade rcuperf so that it can performance-test the asynchronous
> > 	call_rcu() primitives.
> > 
> > 15.	Add a Kconfig-fragment file for Classic SRCU.
> > 
> > 16.	Make sync_rcu_preempt_exp_done() return bool instead of int.
> > 
> > 17.	Now that expedited RCU grace periods do not rely on stop-CPUs
> > 	mechanisms and don't IPI idle/nohz_full CPUs, remove the
> > 	checkpatch.pl warning about them.
> > 
> > 18.	Add an rcuperf test for dynamically initialized srcu_struct
> > 	structures.
> > 
> > 19.	Clarify atomic_ops.rst definition of smp_mb__{before,after}_atomic().
> > 
> > 20.	Add header comment to spin_unlock_wait() defining its semantics.
> > 
> > 21.	Fix typo in memory-barriers.txt, courtesy of Stan Drozd.
> > 
> > 22.	Add the ability to do rcuperf performance tests on tiny RCU flavors.
> > 
> > 23.	Make SRCU flavors announce themselves at boot.
> > 
> > 24.	Reduce the number of CPUs used in Classic SRCU testing.
> > 
> > 25.	Shrink Tiny SRCU a bit more by rearranging and shrinking fields
> > 	in the srcu_struct.
> > 
> > 26.	Set more user-friendly kernel-boot parameter defaults.
> > 
> > 27.	Use /usr/bin/awk instead of /bin/awk, courtesy of Priyalee
> > 	Kushwaha.
> > 
> > 28.	Add writer_holdoff boot parameter to rcuperf to test auto-expediting.
> > 
> > 29.	Add "git diff" output to rcutorture's testid.txt file to
> > 	allow exact after-the-fact reconstruction of exactly what
> > 	source code was tested.
> > 
> > 30.	Document SRCU auto-expediting requirement.
> > 
> > 31.	Add tail-recursion possibility to RCU requirements docuemntation.
> > 
> > 32.	Make CONFIG_PROVE_LOCKING kernels warn about failure to have
> > 	preemption disabled in calls to rcu_sched_qs() and rcu_bh_qs().
> > 
> > 33-34.	Improve dmesg record of non-default Kconfig and boot-parameter
> > 	settings.
> > 
> > 35.	Make the exp_holdoff module parameter be static.
> > 
> > 36.	Add dmesg record of non-default auto-expedite holdoff times.
> > 
> > 37-38.	Add assertions to enforce lock-held and irq-disabled preconditions.
> > 
> > 39.	Make SRCU again be optional.
> > 
> > 40.	Inline __srcu_read_lock() to shrink Tiny SRCU.
> > 
> > 41.	Add DEBUG_OBJECTS_RCU_HEAD checking to SRCU callbacks.
> > 
> > 42-43.	Make synchronize_rcu_mult() check for duplicates, getting rid
> > 	of an ugly #ifdef in sched_cpu_deactivate().
> > 
> > 44.	Rename the nonsensical RCU_NOGP_WAKE flags to RCU_NOCB_WAKE_.
> > 
> > 45.	Add memory barriers for NOCB leader wakeup.
> > 
> > 46.	Add kconfig argument to rcutorture testing to avoid the need
> > 	for lots of special-case Kconfig-fragment files.
> > 
> > 47.	Add comments explaining why rcu_node_tree.h and rcu_segcblist.h
> > 	are visible external to the kernel/rcu directory.
> > 
> > 48.	Fix a bug in rcutorture where it would wait for kernels to
> > 	complete running even though all builds failed for that batch.
> > 
> > 49-57.	Shrink include/linux/rcupdate.h to speed up kernel builds.
> > 
> > 58.	Improve the __call_rcu() debug-objects error message.
> > 
> > 59-65.	More shrinking include/linux/rcupdate.h to speed up kernel builds,
> > 	including shrinking files included by this file.
> > 
> > 66.	Prevent sdp->srcu_gp_seq_needed counter wrap.
> > 
> > 67.	Shrink include/linux/srcu.h (and files it includes) to speed
> > 	up kernel builds.
> > 
> > 68.	Move to trivial callback lists to further shrink Tiny SRCU.
> > 
> > 69.	Use consistent printing primitives within a given function in
> > 	lockdep.c.
> > 
> > 70.	Refactor #includes from include/linux/rcupdate.h to reduce the
> > 	amount of material included, in turn speeding up kernel builds.
> > 
> > 71-73.	Convert rnp->lock wrappers to macros for SRCU use, thus
> > 	consolidating code.
> > 
> > 72-80.	Remove unused code and options.
> > 
> > 81.	Fix typo in code generating rcutorture statistics.
> > 
> > 82-84.	Remove more unused code and options.
> > 
> > 85-86.	Move RCU Kconfig options to kernel/rcu.
> > 
> > 87-88.	Remove yet more unused code and options.
> 

Powered by blists - more mailing lists