From mboxrd@z Thu Jan 1 00:00:00 1970 From: Shaohua Li Subject: Re: [md PATCH 09/18] md/raid10: stop print_conf from being too verbose. Date: Fri, 3 Jun 2016 15:39:09 -0700 Message-ID: <20160603223909.GD1898@kernel.org> References: <20160602061319.2939.72280.stgit@noble> <20160602061952.2939.83586.stgit@noble> <22352.32563.81868.525891@quad.stoffel.home> <87d1nzp5q6.fsf@notabene.neil.brown.name> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <87d1nzp5q6.fsf@notabene.neil.brown.name> Sender: linux-raid-owner@vger.kernel.org To: NeilBrown Cc: John Stoffel , linux-raid@vger.kernel.org List-Id: linux-raid.ids 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