From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jens Axboe Subject: Re: [PATCH 7/7] [BLOCK] Allow elevators to sort/merge discard requests Date: Tue, 7 Oct 2008 14:07:30 +0200 Message-ID: <20081007120730.GV19428@kernel.dk> References: <1218299181.26926.88.camel@pmac.infradead.org> <1218299626.26926.112.camel@pmac.infradead.org> <20081003132909.1eddde1c.akpm@linux-foundation.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: David Woodhouse , ricwheeler@gmail.com, linux-fsdevel@vger.kernel.org, gilad@codefidence.com, matthew@wil.cx To: Andrew Morton Return-path: Received: from pasmtpb.tele.dk ([80.160.77.98]:53605 "EHLO pasmtpB.tele.dk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754990AbYJGMIA (ORCPT ); Tue, 7 Oct 2008 08:08:00 -0400 Content-Disposition: inline In-Reply-To: <20081003132909.1eddde1c.akpm@linux-foundation.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri, Oct 03 2008, Andrew Morton wrote: > 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? I fixed that up in a later patch. > > -#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. It's not real pretty, but I'd be hard pressed to think of a code example that has ever passed 'rq' as an expression with side effects... -- Jens Axboe