From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Morton Subject: Re: [PATCH 7/7] [BLOCK] Allow elevators to sort/merge discard requests Date: Fri, 3 Oct 2008 13:29:09 -0700 Message-ID: <20081003132909.1eddde1c.akpm@linux-foundation.org> References: <1218299181.26926.88.camel@pmac.infradead.org> <1218299626.26926.112.camel@pmac.infradead.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: jens.axboe@oracle.com, ricwheeler@gmail.com, linux-fsdevel@vger.kernel.org, gilad@codefidence.com, matthew@wil.cx To: David Woodhouse Return-path: Received: from smtp1.linux-foundation.org ([140.211.169.13]:49316 "EHLO smtp1.linux-foundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752466AbYJCUaF (ORCPT ); Fri, 3 Oct 2008 16:30:05 -0400 In-Reply-To: <1218299626.26926.112.camel@pmac.infradead.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Sat, 09 Aug 2008 17:33:46 +0100 David Woodhouse wrote: > Signed-off-by: David Woodhouse > --- > block/blk-core.c | 2 +- > block/blk-merge.c | 12 +++++++----- > block/elevator.c | 4 +++- > include/linux/blkdev.h | 5 +++-- > 4 files changed, 14 insertions(+), 9 deletions(-) I seem to be having a grumpier day. ERROR: trailing whitespace #96: FILE: block/blk-merge.c:320: +^Iif (!bio_has_data(bio) || $ ERROR: trailing whitespace #108: FILE: block/blk-merge.c:360: +^Iif (!bio_has_data(bio) || $ ERROR: trailing whitespace #145: FILE: include/linux/blkdev.h:534: +#define blk_account_rq(rq)^I(blk_rq_started(rq) && (blk_fs_request(rq) || blk_discard_rq(rq))) $ WARNING: line over 80 characters #145: FILE: include/linux/blkdev.h:534: +#define blk_account_rq(rq) (blk_rq_started(rq) && (blk_fs_request(rq) || blk_discard_rq(rq))) total: 3 errors, 1 warnings, 71 lines checked Do we think there were tangible benefits to adding new trailing whitespace? > -#define blk_account_rq(rq) (blk_rq_started(rq) && blk_fs_request(rq)) > +#define blk_account_rq(rq) (blk_rq_started(rq) && (blk_fs_request(rq) || blk_discard_rq(rq))) > > #define blk_pm_suspend_request(rq) ((rq)->cmd_type == REQ_TYPE_PM_SUSPEND) > #define blk_pm_resume_request(rq) ((rq)->cmd_type == REQ_TYPE_PM_RESUME) > @@ -588,7 +588,8 @@ static inline void blk_clear_queue_full(struct request_queue *q, int rw) > #define RQ_NOMERGE_FLAGS \ > (REQ_NOMERGE | REQ_STARTED | REQ_HARDBARRIER | REQ_SOFTBARRIER) > #define rq_mergeable(rq) \ > - (!((rq)->cmd_flags & RQ_NOMERGE_FLAGS) && blk_fs_request((rq))) > + (!((rq)->cmd_flags & RQ_NOMERGE_FLAGS) && \ > + (blk_discard_rq(rq) || blk_fs_request((rq)))) Can we please stop writing the kernel in cpp? Not only are these things completely foul and stupid from a readbility POV, they are flat out buggy if passed expression-with-side-effects and will generate large amounts of additional text if called with _any_ expression other than a local variable or something which by some miracle GCC can 100% CSE.