From patchwork Fri Aug 9 00:09:43 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kent Overstreet X-Patchwork-Id: 2841482 X-Patchwork-Delegate: snitzer@redhat.com Return-Path: X-Original-To: patchwork-dm-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 199A4BF546 for ; Fri, 9 Aug 2013 00:14:18 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id CFB262033F for ; Fri, 9 Aug 2013 00:14:16 +0000 (UTC) Received: from mx4-phx2.redhat.com (mx4-phx2.redhat.com [209.132.183.25]) by mail.kernel.org (Postfix) with ESMTP id 8485A20335 for ; Fri, 9 Aug 2013 00:14:15 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx4-phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r7909nMR011000; Thu, 8 Aug 2013 20:09:51 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r7909lV3031030 for ; Thu, 8 Aug 2013 20:09:47 -0400 Received: from mx1.redhat.com (ext-mx13.extmail.prod.ext.phx2.redhat.com [10.5.110.18]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r7909lwJ004544 for ; Thu, 8 Aug 2013 20:09:47 -0400 Received: from mail-pa0-f46.google.com (mail-pa0-f46.google.com [209.85.220.46]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r7909iUM009110 for ; Thu, 8 Aug 2013 20:09:45 -0400 Received: by mail-pa0-f46.google.com with SMTP id fa1so2354747pad.5 for ; Thu, 08 Aug 2013 17:09:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=JudwYHoO8UtpHE7cRdD+Yi0xw2eCHyRZeoVQK7qdFi0=; b=mw7fwDl/2fb+FxSOybIib40teiQpVcIQanW29lO3RNjp/8rLEd3uOQL9Caj1Agj+Rn zSBcpNZsMW2/j5Jji/nCpJJCU546tSH7RAx/Akm9ChIZ8vHWOK8NskOLrhPDKp4Sdd3s NYpjAqhP3eVHluf6kZbCIYSRJ4VJyZmfZMqC2ruhFVVtfpdpKt+zj65d2nUN2B1bkjCo aIMtVs5e7VpcnMnfs7BLnoNzAAhwx72cQOtm2RAHnXb7IlpegDpkBuaAaPVjssKIFz0W JNk+hpja+voBcaq1upIKbCoYN1sNsPzchsqvlSwd+Br7C1BeLoHLBi8CO9N3mrA7HqZu 4KcA== X-Gm-Message-State: ALoCoQmzqSkZxWryeSVth0Xd4MqXXLqljspN++fvOjdf0jxrscUT0AEX3m1RVDRnyzNPXJy7rvjH X-Received: by 10.68.252.233 with SMTP id zv9mr8609808pbc.69.1376006984501; Thu, 08 Aug 2013 17:09:44 -0700 (PDT) Received: from kmo-pixel (173-13-132-141-sfba.hfc.comcastbusiness.net. [173.13.132.141]) by mx.google.com with ESMTPSA id nm10sm16653817pbc.28.2013.08.08.17.09.42 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Thu, 08 Aug 2013 17:09:43 -0700 (PDT) Date: Thu, 8 Aug 2013 17:09:43 -0700 From: Kent Overstreet To: Ed Cashin Message-ID: <20130809000943.GC15409@kmo-pixel> References: <1375912471-5106-1-git-send-email-kmo@daterainc.com> <1375912471-5106-5-git-send-email-kmo@daterainc.com> <2D8F228B-65AE-4046-A910-6B354AF0058E@coraid.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <2D8F228B-65AE-4046-A910-6B354AF0058E@coraid.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-RedHat-Spam-Score: -2.589 (BAYES_00, RCVD_IN_DNSWL_LOW, T_FRT_FREE, URIBL_BLOCKED) X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Scanned-By: MIMEDefang 2.68 on 10.5.110.18 X-loop: dm-devel@redhat.com Cc: axboe@kernel.dk, linux-kernel@vger.kernel.org, linux-raid@vger.kernel.org, dm-devel@redhat.com, linux-fsdevel@vger.kernel.org Subject: Re: [dm-devel] [PATCH 04/22] block: Abstract out bvec iterator X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk Reply-To: device-mapper development 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-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_FRT_FREE, UNPARSEABLE_RELAY autolearn=unavailable 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 Wed, Aug 07, 2013 at 10:04:36PM -0400, Ed Cashin wrote: > On Aug 7, 2013, at 5:54 PM, Kent Overstreet wrote: > > > Immutable biovecs are going to require an explicit iterator. To > > implement immutable bvecs, a later patch is going to add a bi_bvec_done > > member to this struct; for now, this patch effectively just renames > > things. > > Hi, Kent Overstreet. Thanks for Cc'ing me and for the promising work. > > Were you able to do sanity tests with aoe this time around? Last time, basic I/O was not working with the immutable biovec patches applied. > > Here is my 28 June email about my experiences with git://evilpiepirate.org/~kent/linux-bcache.git at that time. It also includes information about creating an easy software-only aoe test environment. > > http://thread.gmane.org/gmane.linux.kernel/1505222/focus=1517924 Hey, thanks for testing it - sorry, I think I remember seeing that email last time and got sidetracked before I got around to setting up some tests. I think I've got it working now, it's running the same stress tests I use for bcache. Here's a fixed patch, I broke the aoe changes out into their own patch since they were more involved than most of the others: (I am seeing a bug where it's getting stuck after running stress tests for awhile, but I can reproduce that without the aoe changes too...) >From 351ff4cdbbc670321b87ad38b5d5edbbb2dd2cf1 Mon Sep 17 00:00:00 2001 From: Kent Overstreet Date: Thu, 8 Aug 2013 16:48:33 -0700 Subject: [PATCH] aoe: Convert to immutable biovecs Now that we've got a mechanism for immutable biovecs - bi_iter.bi_bvec_done - we need to convert drivers to use primitives that respect it instead of using the bvec array directly. The aoe code no longer has to manually iterate over partial bvecs, so some struct members go away - other struct members are effectively renamed: buf->resid -> buf->iter.bi_size buf->sector -> buf->iter.bi_sector f->bcnt -> f->iter.bi_size f->lba -> f->iter.bi_sector Signed-off-by: Kent Overstreet Cc: Jens Axboe Cc: "Ed L. Cashin" --- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel diff --git a/drivers/block/aoe/aoe.h b/drivers/block/aoe/aoe.h index 025c41d..c5e1e9b 100644 --- a/drivers/block/aoe/aoe.h +++ b/drivers/block/aoe/aoe.h @@ -100,11 +100,8 @@ enum { struct buf { ulong nframesout; - ulong resid; - ulong bv_resid; - sector_t sector; struct bio *bio; - struct bio_vec *bv; + struct bvec_iter iter; struct request *rq; }; @@ -120,13 +117,10 @@ struct frame { ulong waited; ulong waited_total; struct aoetgt *t; /* parent target I belong to */ - sector_t lba; struct sk_buff *skb; /* command skb freed on module exit */ struct sk_buff *r_skb; /* response skb for async processing */ struct buf *buf; - struct bio_vec *bv; - ulong bcnt; - ulong bv_off; + struct bvec_iter iter; char flags; }; diff --git a/drivers/block/aoe/aoecmd.c b/drivers/block/aoe/aoecmd.c index f17260b..cacd48e 100644 --- a/drivers/block/aoe/aoecmd.c +++ b/drivers/block/aoe/aoecmd.c @@ -196,8 +196,7 @@ aoe_freetframe(struct frame *f) t = f->t; f->buf = NULL; - f->lba = 0; - f->bv = NULL; + memset(&f->iter, 0, sizeof(f->iter)); f->r_skb = NULL; f->flags = 0; list_add(&f->head, &t->ffree); @@ -295,21 +294,14 @@ newframe(struct aoedev *d) } static void -skb_fillup(struct sk_buff *skb, struct bio_vec *bv, ulong off, ulong cnt) +skb_fillup(struct sk_buff *skb, struct bio *bio, struct bvec_iter iter) { int frag = 0; - ulong fcnt; -loop: - fcnt = bv->bv_len - (off - bv->bv_offset); - if (fcnt > cnt) - fcnt = cnt; - skb_fill_page_desc(skb, frag++, bv->bv_page, off, fcnt); - cnt -= fcnt; - if (cnt <= 0) - return; - bv++; - off = bv->bv_offset; - goto loop; + struct bio_vec bv; + + __bio_for_each_segment(bv, bio, iter, iter) + skb_fill_page_desc(skb, frag++, bv.bv_page, + bv.bv_offset, bv.bv_len); } static void @@ -346,12 +338,10 @@ ata_rw_frameinit(struct frame *f) t->nout++; f->waited = 0; f->waited_total = 0; - if (f->buf) - f->lba = f->buf->sector; /* set up ata header */ - ah->scnt = f->bcnt >> 9; - put_lba(ah, f->lba); + ah->scnt = f->iter.bi_size >> 9; + put_lba(ah, f->iter.bi_sector); if (t->d->flags & DEVFL_EXT) { ah->aflags |= AOEAFL_EXT; } else { @@ -360,11 +350,11 @@ ata_rw_frameinit(struct frame *f) ah->lba3 |= 0xe0; /* LBA bit + obsolete 0xa0 */ } if (f->buf && bio_data_dir(f->buf->bio) == WRITE) { - skb_fillup(skb, f->bv, f->bv_off, f->bcnt); + skb_fillup(skb, f->buf->bio, f->iter); ah->aflags |= AOEAFL_WRITE; - skb->len += f->bcnt; - skb->data_len = f->bcnt; - skb->truesize += f->bcnt; + skb->len += f->iter.bi_size; + skb->data_len = f->iter.bi_size; + skb->truesize += f->iter.bi_size; t->wpkts++; } else { t->rpkts++; @@ -383,7 +373,6 @@ aoecmd_ata_rw(struct aoedev *d) struct aoetgt *t; struct sk_buff *skb; struct sk_buff_head queue; - ulong bcnt, fbcnt; buf = nextbuf(d); if (buf == NULL) @@ -392,39 +381,22 @@ aoecmd_ata_rw(struct aoedev *d) if (f == NULL) return 0; t = *d->tgt; - bcnt = d->maxbcnt; - if (bcnt == 0) - bcnt = DEFAULTBCNT; - if (bcnt > buf->resid) - bcnt = buf->resid; - fbcnt = bcnt; - f->bv = buf->bv; - f->bv_off = f->bv->bv_offset + (f->bv->bv_len - buf->bv_resid); - do { - if (fbcnt < buf->bv_resid) { - buf->bv_resid -= fbcnt; - buf->resid -= fbcnt; - break; - } - fbcnt -= buf->bv_resid; - buf->resid -= buf->bv_resid; - if (buf->resid == 0) { - d->ip.buf = NULL; - break; - } - buf->bv++; - buf->bv_resid = buf->bv->bv_len; - WARN_ON(buf->bv_resid == 0); - } while (fbcnt); /* initialize the headers & frame */ f->buf = buf; - f->bcnt = bcnt; - ata_rw_frameinit(f); + f->iter = buf->iter; + f->iter.bi_size = min_t(unsigned long, + d->maxbcnt ?: DEFAULTBCNT, + f->iter.bi_size); + bio_advance_iter(buf->bio, &buf->iter, f->iter.bi_size); + + if (!buf->iter.bi_size) + d->ip.buf = NULL; /* mark all tracking fields and load out */ buf->nframesout += 1; - buf->sector += bcnt >> 9; + + ata_rw_frameinit(f); skb = skb_clone(f->skb, GFP_ATOMIC); if (skb) { @@ -617,10 +589,7 @@ reassign_frame(struct frame *f) skb = nf->skb; nf->skb = f->skb; nf->buf = f->buf; - nf->bcnt = f->bcnt; - nf->lba = f->lba; - nf->bv = f->bv; - nf->bv_off = f->bv_off; + nf->iter = f->iter; nf->waited = 0; nf->waited_total = f->waited_total; nf->sent = f->sent; @@ -652,19 +621,19 @@ probe(struct aoetgt *t) } f->flags |= FFL_PROBE; ifrotate(t); - f->bcnt = t->d->maxbcnt ? t->d->maxbcnt : DEFAULTBCNT; + f->iter.bi_size = t->d->maxbcnt ? t->d->maxbcnt : DEFAULTBCNT; ata_rw_frameinit(f); skb = f->skb; - for (frag = 0, n = f->bcnt; n > 0; ++frag, n -= m) { + for (frag = 0, n = f->iter.bi_size; n > 0; ++frag, n -= m) { if (n < PAGE_SIZE) m = n; else m = PAGE_SIZE; skb_fill_page_desc(skb, frag, empty_page, 0, m); } - skb->len += f->bcnt; - skb->data_len = f->bcnt; - skb->truesize += f->bcnt; + skb->len += f->iter.bi_size; + skb->data_len = f->iter.bi_size; + skb->truesize += f->iter.bi_size; skb = skb_clone(f->skb, GFP_ATOMIC); if (skb) { @@ -936,12 +905,8 @@ bufinit(struct buf *buf, struct request *rq, struct bio *bio) memset(buf, 0, sizeof(*buf)); buf->rq = rq; buf->bio = bio; - buf->resid = bio->bi_iter.bi_size; - buf->sector = bio->bi_iter.bi_sector; + buf->iter = bio->bi_iter; bio_pageinc(bio); - buf->bv = __bio_iovec(bio); - buf->bv_resid = buf->bv->bv_len; - WARN_ON(buf->bv_resid == 0); } static struct buf * @@ -1126,24 +1091,19 @@ gettgt(struct aoedev *d, char *addr) } static void -bvcpy(struct bio_vec *bv, ulong off, struct sk_buff *skb, long cnt) +bvcpy(struct sk_buff *skb, struct bio *bio, struct bvec_iter iter, long cnt) { - ulong fcnt; - char *p; int soff = 0; -loop: - fcnt = bv->bv_len - (off - bv->bv_offset); - if (fcnt > cnt) - fcnt = cnt; - p = page_address(bv->bv_page) + off; - skb_copy_bits(skb, soff, p, fcnt); - soff += fcnt; - cnt -= fcnt; - if (cnt <= 0) - return; - bv++; - off = bv->bv_offset; - goto loop; + struct bio_vec bv; + + BUG_ON(cnt > iter.bi_size); + iter.bi_size = cnt; + + __bio_for_each_segment(bv, bio, iter, iter) { + char *p = page_address(bv.bv_page) + bv.bv_offset; + skb_copy_bits(skb, soff, p, bv.bv_len); + soff += bv.bv_len; + } } void @@ -1236,7 +1196,7 @@ noskb: if (buf) clear_bit(BIO_UPTODATE, &buf->bio->bi_flags); break; } - bvcpy(f->bv, f->bv_off, skb, n); + bvcpy(skb, f->buf->bio, f->iter, n); case ATA_CMD_PIO_WRITE: case ATA_CMD_PIO_WRITE_EXT: spin_lock_irq(&d->lock); @@ -1279,7 +1239,7 @@ out: aoe_freetframe(f); - if (buf && --buf->nframesout == 0 && buf->resid == 0) + if (buf && --buf->nframesout == 0 && buf->iter.bi_size == 0) aoe_end_buf(d, buf); spin_unlock_irq(&d->lock); @@ -1734,7 +1694,7 @@ aoe_failbuf(struct aoedev *d, struct buf *buf) { if (buf == NULL) return; - buf->resid = 0; + buf->iter.bi_size = 0; clear_bit(BIO_UPTODATE, &buf->bio->bi_flags); if (buf->nframesout == 0) aoe_end_buf(d, buf);