Re: [md PATCH 09/18] md/raid10: stop print_conf from being too verbose.

[Shaohua Li <shli@kernel.org>]

On Fri, Jun 03, 2016 at 08:48:33AM +1000, Neil Brown wrote:
> On Fri, Jun 03 2016, John Stoffel wrote:
> 
> > NeilBrown> raid10 arrays can usefully be very large.  When they are,
> > NeilBrown> the noise generated by print_conf can over-whelm logs.  So
> > NeilBrown> truncate the listing at 16 devices.
> >
> > Why is this too noisy and how often does this print_conf() happen?
> > And why 16 devices?  I guess I don't like the magic number of 16 here,
> > I'd prefer it to be a define, and possibly even something that can by
> > dynamically changed.
> 
> print_conf happens whenever a device becomes active in the array, or a
> device is removed from the array (usually because it has failed).
> 
> I got 16 by choosing a random number and multiplying by 4 (or maybe by
> raising 2 to that power) :-)
> 
> More seriously, I guessed that most arrays were 16 devices or less, so
> this would not affect most arrays.
> 
> I definitely don't think it needs a define.  I'm very tempted to remove
> print_conf() completely, but it is sometimes useful.  So having it
> present as long as it isn't a burden seems reasonable.
> 
> If configuration was important I would change it to use pr_debug(), but
> then the default would be to not see the messages at all, and they can
> be useful in diagnosing problems reported on mailing lists.
> 
> >
> > But how big a problem is this really?  And what about for big RAID5/6
> > arrays as well?
> 
> When you have 2000 devices in your RAID10 and half of them are removed
> at once, it currently reports on 2,000,000 devices.  With the patch it
> is only 32000.  Still possibly too many.
> 
> If you have 2000 devices in your RAID5/6 and half of them are removed,
> you have other problems.
> 
> >
> > Or would it be also good to condence the output of print_conf()
> > itself?
> 
> Probably a very good idea.  Maybe the default could print a fixed-size
> summary and the rest goes in pr_debug()??
> 
> >
> > Of if it's noise, why not just remove it completely?  Can this
> > information be found in /proc/mdstat instead?
> 
> Its value is historical - trying to understand a past sequence of
> events.  For that, something in the logs beats something in /proc.
> 
> >
> > Sorry I havent' looked in the code deeply, but this just struck me as
> > a change that might not be ideal.
> 
> Fair enough - your comments are very valid.  I'm not really sure what is
> best.  I only know what is worst :-) and want to avoid that.

I would prefer pr_debug. pr_debug can be enabled dynamically. If the info is
helpful for diagnosing, enabling it is simple.

Thanks,
Shaohua