From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtprelay0130.hostedemail.com ([216.40.44.130]:55026 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751379AbdLKXqJ (ORCPT ); Mon, 11 Dec 2017 18:46:09 -0500 Message-ID: <1513035963.3036.17.camel@perches.com> Subject: Re: [PATCH v4 72/73] xfs: Convert mru cache to XArray From: Joe Perches Date: Mon, 11 Dec 2017 15:46:03 -0800 In-Reply-To: <20171211224301.GA3925@bombadil.infradead.org> References: <20171208223654.GP5858@dastard> <1512838818.26342.7.camel@perches.com> <20171211214300.GT5858@dastard> <1513030348.3036.5.camel@perches.com> <20171211224301.GA3925@bombadil.infradead.org> Content-Type: text/plain; charset="ISO-8859-1" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-xfs-owner@vger.kernel.org List-ID: List-Id: xfs To: Matthew Wilcox , Andrew Morton , Andy Whitcroft Cc: Dave Chinner , Alan Stern , Byungchul Park , Theodore Ts'o , Matthew Wilcox , Ross Zwisler , Jens Axboe , Rehas Sachdeva , linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net, linux-nilfs@vger.kernel.org, linux-btrfs@vger.kernel.org, linux-xfs@vger.kernel.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@lge.com On Mon, 2017-12-11 at 14:43 -0800, Matthew Wilcox wrote: > On Mon, Dec 11, 2017 at 02:12:28PM -0800, Joe Perches wrote: > > Completely reasonable. Thanks. > > If we're doing "completely reasonable" complaints, then ... > > - I don't understand why plain 'unsigned' is deemed bad. That was a David Miller preference. > - The rule about all function parameters in prototypes having a name > doesn't make sense. Example: > > int ida_get_new_above(struct ida *ida, int starting_id, int *p_id); Improvements to regex welcomed. > - Forcing a blank line after variable declarations sometimes makes for > some weird-looking code. True. I don't care for this one myself. > Constructively, I think this warning can be suppressed for blocks > that are under, say, 8 lines. Not easy to do as checkpatch works on patches. > 6) Functions > ------------ > > Functions should be short and sweet, and do just one thing. They should > fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24, > as we all know), and do one thing and do that well. > > I'm not expecting you to be able to write a perl script that checks > the first line, but we have way too many 200-plus line functions in > the kernel. I'd like a warning on anything over 200 lines (a factor > of 4 over Linus's stated goal). Maybe reasonable. Some declaration blocks for things like: void foo(void) { static const struct foobar array[] = { { long count of lines... }; [body] } might make that warning unreasonable though. > - I don't understand the error for xa_head here: > > struct xarray { > spinlock_t xa_lock; > gfp_t xa_flags; > void __rcu * xa_head; > }; > > Do people really think that: > > struct xarray { > spinlock_t xa_lock; > gfp_t xa_flags; > void __rcu *xa_head; > }; > > is more aesthetically pleasing? And not just that, but it's an *error* > so the former is *RIGHT* and this is *WRONG*. And not just a matter > of taste? No opinion really. That's from Andy Whitcroft's original implementation.