From patchwork Thu Feb 15 09:01:31 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: NeilBrown X-Patchwork-Id: 10220717 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 7410660467 for ; Thu, 15 Feb 2018 09:03:14 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 66A7A290C6 for ; Thu, 15 Feb 2018 09:03:14 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 5A51D290CB; Thu, 15 Feb 2018 09:03:14 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id C941E290C6 for ; Thu, 15 Feb 2018 09:03:13 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 595866AAF5; Thu, 15 Feb 2018 09:03:12 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id DA8B318ADF; Thu, 15 Feb 2018 09:03:11 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 873364A46C; Thu, 15 Feb 2018 09:03:10 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w1F91qSm018426 for ; Thu, 15 Feb 2018 04:01:52 -0500 Received: by smtp.corp.redhat.com (Postfix) id C080D5C545; Thu, 15 Feb 2018 09:01:52 +0000 (UTC) Delivered-To: dm-devel@redhat.com Received: from mx1.redhat.com (ext-mx08.extmail.prod.ext.phx2.redhat.com [10.5.110.32]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 90C8E5C89C; Thu, 15 Feb 2018 09:01:46 +0000 (UTC) Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) (using TLSv1 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9016DC058ECC; Thu, 15 Feb 2018 09:01:40 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id C31A5AC9C; Thu, 15 Feb 2018 09:01:38 +0000 (UTC) From: NeilBrown To: Milan Broz , Mike Snitzer , device-mapper development Date: Thu, 15 Feb 2018 20:01:31 +1100 In-Reply-To: <70cda2a3-f246-d45b-f600-1f9d15ba22ff@gmail.com> References: <70cda2a3-f246-d45b-f600-1f9d15ba22ff@gmail.com> Message-ID: <87h8qipqxg.fsf@notabene.neil.brown.name> MIME-Version: 1.0 X-Greylist: Sender passed SPF test, Sender IP whitelisted by DNSRBL, ACL 207 matched, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Thu, 15 Feb 2018 09:01:40 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Thu, 15 Feb 2018 09:01:40 +0000 (UTC) for IP:'195.135.220.15' DOMAIN:'mx2.suse.de' HELO:'mx2.suse.de' FROM:'neilb@suse.com' RCPT:'' X-RedHat-Spam-Score: -2.301 (RCVD_IN_DNSWL_MED, SPF_PASS) 195.135.220.15 mx2.suse.de 195.135.220.15 mx2.suse.de X-Scanned-By: MIMEDefang 2.78 on 10.5.110.32 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: dm-devel@redhat.com Cc: Linux Kernel Mailing List Subject: [dm-devel] [RFC PATCH] dm: don't assign zero to ->bi_status of an active bio. X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Thu, 15 Feb 2018 09:03:12 +0000 (UTC) X-Virus-Scanned: ClamAV using ClamSMTP Between the moment when generic_make_request() is first called on a bio, and when bio_endio() finally gets past bio_remaining_done(), a bio might have chained children, and might get ->bi_status set asynchronously. So during this time it is not safe to set it to zero. It *is* safe to set it to any other error status as for most purposes, one error is as good as another. This patch is untested and RFC only. It identifies a number of possible problem areas in dm and often suggests fixes. Please don't apply without careful review. Reviewed-by: Nobody yet. Signed-off-by: NeilBrown --- drivers/md/dm-bio-prison-v1.c | 3 ++- drivers/md/dm-bufio.c | 6 ++++-- drivers/md/dm-crypt.c | 3 ++- drivers/md/dm-mpath.c | 3 +++ drivers/md/dm-thin.c | 3 ++- drivers/md/dm-verity-target.c | 3 ++- drivers/md/dm-zoned-target.c | 3 ++- 7 files changed, 17 insertions(+), 7 deletions(-) diff --git a/drivers/md/dm-bio-prison-v1.c b/drivers/md/dm-bio-prison-v1.c index 874841f0fc83..68cb4fe98e9f 100644 --- a/drivers/md/dm-bio-prison-v1.c +++ b/drivers/md/dm-bio-prison-v1.c @@ -238,7 +238,8 @@ void dm_cell_error(struct dm_bio_prison *prison, dm_cell_release(prison, cell, &bios); while ((bio = bio_list_pop(&bios))) { - bio->bi_status = error; + if (error) + bio->bi_status = error; bio_endio(bio); } } diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c index 414c9af54ded..80131e098650 100644 --- a/drivers/md/dm-bufio.c +++ b/drivers/md/dm-bufio.c @@ -565,7 +565,8 @@ static void dmio_complete(unsigned long error, void *context) { struct dm_buffer *b = context; - b->bio.bi_status = error ? BLK_STS_IOERR : 0; + if (error) + b->bio.bi_status = BLK_STS_IOERR; b->bio.bi_end_io(&b->bio); } @@ -614,7 +615,8 @@ static void inline_endio(struct bio *bio) */ bio_reset(bio); - bio->bi_status = status; + if (status) + bio->bi_status = status; end_fn(bio); } diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c index 8168f737590e..3e7db4a1093e 100644 --- a/drivers/md/dm-crypt.c +++ b/drivers/md/dm-crypt.c @@ -1488,7 +1488,8 @@ static void crypt_dec_pending(struct dm_crypt_io *io) else kfree(io->integrity_metadata); - base_bio->bi_status = error; + if (error) + base_bio->bi_status = error; bio_endio(base_bio); } diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index 7d3e572072f5..6f793d7d29f0 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -651,6 +651,9 @@ static int __multipath_map_bio(struct multipath *m, struct bio *bio, mpio->pgpath = pgpath; + /* FIXME If the bio was chained, this could + * over-write an error... is that possible? + */ bio->bi_status = 0; bio_set_dev(bio, pgpath->path.dev->bdev); bio->bi_opf |= REQ_FAILFAST_TRANSPORT; diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c index 629c555890c1..16d4c386cc56 100644 --- a/drivers/md/dm-thin.c +++ b/drivers/md/dm-thin.c @@ -563,7 +563,8 @@ static void error_bio_list(struct bio_list *bios, blk_status_t error) struct bio *bio; while ((bio = bio_list_pop(bios))) { - bio->bi_status = error; + if (error) + bio->bi_status = error; bio_endio(bio); } } diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c index aedb8222836b..86a1d501d915 100644 --- a/drivers/md/dm-verity-target.c +++ b/drivers/md/dm-verity-target.c @@ -503,7 +503,8 @@ static void verity_finish_io(struct dm_verity_io *io, blk_status_t status) struct bio *bio = dm_bio_from_per_bio_data(io, v->ti->per_io_data_size); bio->bi_end_io = io->orig_bi_end_io; - bio->bi_status = status; + if (status) + bio->bi_status = status; verity_fec_finish_io(io); diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c index caff02caf083..cfdacaf79492 100644 --- a/drivers/md/dm-zoned-target.c +++ b/drivers/md/dm-zoned-target.c @@ -637,7 +637,8 @@ static int dmz_end_io(struct dm_target *ti, struct bio *bio, blk_status_t *error return DM_ENDIO_INCOMPLETE; /* Done */ - bio->bi_status = bioctx->status; + if (bioctx->status) + bio->bi_status = bioctx->status; if (bioctx->zone) { struct dm_zone *zone = bioctx->zone;