From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933569AbbBDKT3 (ORCPT ); Wed, 4 Feb 2015 05:19:29 -0500 Received: from casper.infradead.org ([85.118.1.10]:41970 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753206AbbBDKTL (ORCPT ); Wed, 4 Feb 2015 05:19:11 -0500 Date: Wed, 4 Feb 2015 11:18:49 +0100 From: Peter Zijlstra To: Jens Axboe Cc: Linus Torvalds , Benjamin LaHaise , linux-aio@kvack.org, Linux Kernel Subject: [PATCH] block: Simplify bsg complete all Message-ID: <20150204101849.GC2896@worktop.programming.kicks-ass.net> References: <20150201144058.GM2974@kvack.org> <20150201221458.GN2974@kvack.org> <20150202001628.GO2974@kvack.org> <20150203112733.GM26304@twins.programming.kicks-ass.net> <20150203113348.GH24151@twins.programming.kicks-ass.net> <20150203115531.GI24151@twins.programming.kicks-ass.net> <54D158C7.6020606@kernel.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <54D158C7.6020606@kernel.dk> User-Agent: Mutt/1.5.22.1 (2013-10-16) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Feb 03, 2015 at 04:24:55PM -0700, Jens Axboe wrote: > > Yes, from a cursory look, that seems fine to me. Though I will hold the fact > that you label my code as 'seriously obtuse' against you. Some day. Hehe, fair enough. I'm sure I've written my fair share of it too, we've all got our 'bad' days, I'll get you a beer in BOS to make up if you like :-) > I can pull this in for testing for 3.20. Mind sending a properly formatted > patch (signed off, commit message, all that stuff)? Sure, here goes. --- Subject: block: Simplify bsg complete all From: Peter Zijlstra Date: Tue, 3 Feb 2015 12:55:31 +0100 It took me a few tries to figure out what this code did; lets rewrite it into a more regular form. The thing that makes this one 'special' is the BSG_F_BLOCK flag, if that is not set we're not supposed/allowed to block and should spin wait for completion. The (new) io_wait_event() will never see a false condition in case of the spinning and we will therefore not block. Cc: Linus Torvalds Cc: Jens Axboe Signed-off-by: Peter Zijlstra (Intel) --- block/bsg.c | 72 +++++++++++++++++---------------------------------- include/linux/wait.h | 15 ++++++++++ 2 files changed, 40 insertions(+), 47 deletions(-) --- a/block/bsg.c +++ b/block/bsg.c @@ -136,42 +136,6 @@ static inline struct hlist_head *bsg_dev return &bsg_device_list[index & (BSG_LIST_ARRAY_SIZE - 1)]; } -static int bsg_io_schedule(struct bsg_device *bd) -{ - DEFINE_WAIT(wait); - int ret = 0; - - spin_lock_irq(&bd->lock); - - BUG_ON(bd->done_cmds > bd->queued_cmds); - - /* - * -ENOSPC or -ENODATA? I'm going for -ENODATA, meaning "I have no - * work to do", even though we return -ENOSPC after this same test - * during bsg_write() -- there, it means our buffer can't have more - * bsg_commands added to it, thus has no space left. - */ - if (bd->done_cmds == bd->queued_cmds) { - ret = -ENODATA; - goto unlock; - } - - if (!test_bit(BSG_F_BLOCK, &bd->flags)) { - ret = -EAGAIN; - goto unlock; - } - - prepare_to_wait(&bd->wq_done, &wait, TASK_UNINTERRUPTIBLE); - spin_unlock_irq(&bd->lock); - io_schedule(); - finish_wait(&bd->wq_done, &wait); - - return ret; -unlock: - spin_unlock_irq(&bd->lock); - return ret; -} - static int blk_fill_sgv4_hdr_rq(struct request_queue *q, struct request *rq, struct sg_io_v4 *hdr, struct bsg_device *bd, fmode_t has_write_perm) @@ -482,6 +446,30 @@ static int blk_complete_sgv4_hdr_rq(stru return ret; } +static bool bsg_complete(struct bsg_device *bd) +{ + bool ret = false; + bool spin; + + do { + spin_lock_irq(&bd->lock); + + BUG_ON(bd->done_cmds > bd->queued_cmds); + + /* + * All commands consumed. + */ + if (bd->done_cmds == bd->queued_cmds) + ret = true; + + spin = !test_bit(BSG_F_BLOCK, &bd->flags); + + spin_unlock_irq(&bd->lock); + } while (!ret && spin); + + return ret; +} + static int bsg_complete_all_commands(struct bsg_device *bd) { struct bsg_command *bc; @@ -492,17 +480,7 @@ static int bsg_complete_all_commands(str /* * wait for all commands to complete */ - ret = 0; - do { - ret = bsg_io_schedule(bd); - /* - * look for -ENODATA specifically -- we'll sometimes get - * -ERESTARTSYS when we've taken a signal, but we can't - * return until we're done freeing the queue, so ignore - * it. The signal will get handled when we're done freeing - * the bsg_device. - */ - } while (ret != -ENODATA); + io_wait_event(bd->wq_done, bsg_complete(bd)); /* * discard done commands --- a/include/linux/wait.h +++ b/include/linux/wait.h @@ -267,6 +267,21 @@ do { \ __wait_event(wq, condition); \ } while (0) +#define __io_wait_event(wq, condition) \ + (void)___wait_event(wq, condition, TASK_UNINTERRUPTIBLE, 0, 0, \ + io_schedule()) + +/* + * io_wait_event() -- like wait_event() but with io_schedule() + */ +#define io_wait_event(wq, condition) \ +do { \ + might_sleep(); \ + if (condition) \ + break; \ + __io_wait_event(wq, condition); \ +} while (0) + #define __wait_event_freezable(wq, condition) \ ___wait_event(wq, condition, TASK_INTERRUPTIBLE, 0, 0, \ schedule(); try_to_freeze())