From: Jens Axboe Subject: Re: [PATCH 5/8] nowait aio: return on congested block device Date: Fri, 24 Mar 2017 08:39:42 -0600 Message-ID: <65504798-90c0-6abe-30ea-2faf3824eed8@kernel.dk> References: <20170315215107.5628-1-rgoldwyn@suse.de> <20170315215107.5628-6-rgoldwyn@suse.de> <47d742e9-1343-ef5b-f7d4-29a6e22bef84@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: jack@suse.com, hch@infradead.org, linux-block@vger.kernel.org, linux-btrfs@vger.kernel.org, linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org, sagi@grimberg.me, avi@scylladb.com, linux-api@vger.kernel.org, willy@infradead.org, Goldwyn Rodrigues To: Goldwyn Rodrigues , linux-fsdevel@vger.kernel.org Return-path: In-Reply-To: <47d742e9-1343-ef5b-f7d4-29a6e22bef84@suse.de> Sender: linux-block-owner@vger.kernel.org List-Id: linux-ext4.vger.kernel.org On 03/24/2017 05:32 AM, Goldwyn Rodrigues wrote: > > > On 03/16/2017 09:33 AM, Jens Axboe wrote: >> On 03/15/2017 03:51 PM, Goldwyn Rodrigues wrote: >>> diff --git a/block/blk-core.c b/block/blk-core.c >>> index 0eeb99e..2e5cba2 100644 >>> --- a/block/blk-core.c >>> +++ b/block/blk-core.c >>> @@ -2014,7 +2019,7 @@ blk_qc_t generic_make_request(struct bio *bio) >>> do { >>> struct request_queue *q = bdev_get_queue(bio->bi_bdev); >>> >>> - if (likely(blk_queue_enter(q, false) == 0)) { >>> + if (likely(blk_queue_enter(q, bio_flagged(bio, BIO_NOWAIT)) == 0)) { >>> struct bio_list hold; >>> struct bio_list lower, same; >>> >>> @@ -2040,7 +2045,10 @@ blk_qc_t generic_make_request(struct bio *bio) >>> bio_list_merge(&bio_list_on_stack, &same); >>> bio_list_merge(&bio_list_on_stack, &hold); >>> } else { >>> - bio_io_error(bio); >>> + if (unlikely(bio_flagged(bio, BIO_NOWAIT))) >>> + bio_wouldblock_error(bio); >>> + else >>> + bio_io_error(bio); >> >> This doesn't look right. What if the queue is dying, and BIO_NOWAIT just >> happened to be set? >> > > Yes, I need to add a condition here to check for blk_queue_dying(). Thanks. > >> And you're missing wbt_wait() as well as a blocking point. Ditto in >> blk-mq. > > wbt_wait() does not apply to WRITE_ODIRECT It doesn't _currently_ block for it, but that could change. It should have an explicit check. Additionally, the more weird assumptions you make, the more half assed this will be. The other assumption made here is that only direct IO would set nonblock. We have to assume that any IO can have NONBLOCK set, and not make any assumptions about the caller. -- Jens Axboe