From patchwork Wed Jun 10 08:11:38 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoph Hellwig X-Patchwork-Id: 6577551 Return-Path: X-Original-To: patchwork-linux-btrfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 75FBBC0020 for ; Wed, 10 Jun 2015 08:12:09 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 71F72205E4 for ; Wed, 10 Jun 2015 08:12:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 50DFD205E3 for ; Wed, 10 Jun 2015 08:12:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751748AbbFJILx (ORCPT ); Wed, 10 Jun 2015 04:11:53 -0400 Received: from verein.lst.de ([213.95.11.211]:51075 "EHLO newverein.lst.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753819AbbFJILk (ORCPT ); Wed, 10 Jun 2015 04:11:40 -0400 Received: by newverein.lst.de (Postfix, from userid 2407) id 7FC35691EA; Wed, 10 Jun 2015 10:11:38 +0200 (CEST) Date: Wed, 10 Jun 2015 10:11:38 +0200 From: Christoph Hellwig To: Mike Snitzer Cc: Jens Axboe , linux-raid@vger.kernel.org, dm-devel@redhat.com, linux-btrfs@vger.kernel.org Subject: Re: block: add a bi_error field to struct bio Message-ID: <20150610081138.GA3841@lst.de> References: <1433338959-24808-1-git-send-email-hch@lst.de> <1433338959-24808-2-git-send-email-hch@lst.de> <20150604153106.GA31567@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20150604153106.GA31567@redhat.com> User-Agent: Mutt/1.5.17 (2007-11-01) Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Jun 04, 2015 at 11:31:07AM -0400, Mike Snitzer wrote: > This patch _really_ concerns me because just in DM alone I found you > took liberties that you shouldn't have and created a regression. First > issue is a real bug (your proposed dm-io.c:dmio_complete change missed > that dm-io uses error_bits and not traditional error code like expected) Point taken. I already wanted to complain about the mess due to the bio error abuse with it's own values in DM in the first posting, guess I need to add that to the second one. I don't think overloading common interfaces with your private error codes is a good idea, but let's leave that for a separate discussion. > the other issue being you added extra branching that isn't needed and > made review more tedious (dm.c:clone_endio). I think the code is better than what it was before, but it's still a bit of a mess. What do you think of the patch below which I'd like to add before the big bi_error patch as a preparatory one? > For DM, please add Signed-off-by: Mike Snitzer once > you've folded in this patch, thanks! FYI, that wasn't a foldable patch but updated hunks of the old one. Not really a problem, but a little confusing. From f095cbeba5135afa6cf102718319f0d0c1e7b422 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Wed, 10 Jun 2015 10:04:45 +0200 Subject: dm: use a single error code variable in clone_endio MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit clone_endio currently uses two variables for tracking error state, with values getting bounce? forth and back between the two, which makes the code hard to read. Signed-off-by: Christoph Hellwig --- drivers/md/dm.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/drivers/md/dm.c b/drivers/md/dm.c index 2161ed9..8467976 100644 --- a/drivers/md/dm.c +++ b/drivers/md/dm.c @@ -956,7 +956,6 @@ static void disable_write_same(struct mapped_device *md) static void clone_endio(struct bio *bio, int error) { - int r = error; struct dm_target_io *tio = container_of(bio, struct dm_target_io, clone); struct dm_io *io = tio->io; struct mapped_device *md = tio->io->md; @@ -966,23 +965,22 @@ static void clone_endio(struct bio *bio, int error) error = -EIO; if (endio) { - r = endio(tio->ti, bio, error); - if (r < 0 || r == DM_ENDIO_REQUEUE) - /* - * error and requeue request are handled - * in dec_pending(). - */ - error = r; - else if (r == DM_ENDIO_INCOMPLETE) + error = endio(tio->ti, bio, error); + if (error == DM_ENDIO_INCOMPLETE) { /* The target will handle the io */ return; - else if (r) { - DMWARN("unimplemented target endio return value: %d", r); + } + + if (error > 0 && error != DM_ENDIO_REQUEUE) { + DMWARN("unimplemented target endio return value: %d", + error); BUG(); } + + /* Error and requeue request are handled in dec_pending(). */ } - if (unlikely(r == -EREMOTEIO && (bio->bi_rw & REQ_WRITE_SAME) && + if (unlikely(error == -EREMOTEIO && (bio->bi_rw & REQ_WRITE_SAME) && !bdev_get_queue(bio->bi_bdev)->limits.max_write_same_sectors)) disable_write_same(md);