Linux block layer
 help / color / mirror / Atom feed
* Re: [PATCH v2 5/5] md: fast clone bio in bio_clone_mddev()
From: Shaohua Li @ 2017-02-15 23:19 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Ming Lei, Jens Axboe, linux-kernel, linux-raid, linux-block,
	NeilBrown
In-Reply-To: <20170215192025.fo7jsew7pvagp5hv@kernel.org>

On Wed, Feb 15, 2017 at 11:20:25AM -0800, Shaohua Li wrote:
> On Tue, Feb 14, 2017 at 08:01:09AM -0800, Christoph Hellwig wrote:
> > On Tue, Feb 14, 2017 at 11:29:03PM +0800, Ming Lei wrote:
> > > Firstly bio_clone_mddev() is used in raid normal I/O and isn't
> > > in resync I/O path.
> > > 
> > > Secondly all the direct access to bvec table in raid happens on
> > > resync I/O except for write behind of raid1, in which we still
> > > use bio_clone() for allocating new bvec table.
> > > 
> > > So this patch replaces bio_clone() with bio_clone_fast()
> > > in bio_clone_mddev().
> > > 
> > > Also kill bio_clone_mddev() and call bio_clone_fast() directly, as
> > > suggested by Christoph Hellwig.
> > > 
> > > Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> > 
> > Looks fine,
> > 
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > 
> > Btw, can you also tack on another patch to kill bio_alloc_mddev
> > to be consistent?
> 
> I'll take care of this

Hmm, bio_alloc_mddev is a little different, it could be called without the
bio_set. We can only guarantee the bio_set is valid for bio for md personality
not for metadata. Fully fixing the bio_set issue will need rework mddev struct
allocation, don't think it's worthy doing. So below is one removing the export
of the wrap.

>From eaf70c32e5cca68110a0cf7a0311ef0ac97f4d42 Mon Sep 17 00:00:00 2001
Message-Id: <eaf70c32e5cca68110a0cf7a0311ef0ac97f4d42.1487200103.git.shli@fb.com>
From: Shaohua Li <shli@fb.com>
Date: Wed, 15 Feb 2017 11:41:21 -0800
Subject: [PATCH] md: don't export bio_alloc_mddev

When mddev is running, it has a valid bio_set, so the bio_alloc_mddev
wrap is unncessary. The problem is we run some IO (eg, load superblock)
before mddev is fully initialized. At that time bio_set isn't
initialized. Moving bio_set creation to md_alloc doesn't help either,
because dm-raid doesn't use it.

Signed-off-by: Shaohua Li <shli@fb.com>
---
 drivers/md/md.c     | 12 ++++++------
 drivers/md/md.h     |  2 --
 drivers/md/raid1.c  |  2 +-
 drivers/md/raid10.c |  2 +-
 4 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 55e7e7a..bef1863 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -171,11 +171,12 @@ static const struct block_device_operations md_fops;
 
 static int start_readonly;
 
-/* bio_clone_mddev
- * like bio_clone, but with a local bio set
+/*
+ * This is only required for meta data related bio, where bio_set might not be
+ * initialized yet. dm-raid doesn't use md_alloc, so we can't alloc bio_set
+ * there.
  */
-
-struct bio *bio_alloc_mddev(gfp_t gfp_mask, int nr_iovecs,
+static struct bio *bio_alloc_mddev(gfp_t gfp_mask, int nr_iovecs,
 			    struct mddev *mddev)
 {
 	struct bio *b;
@@ -188,7 +189,6 @@ struct bio *bio_alloc_mddev(gfp_t gfp_mask, int nr_iovecs,
 		return NULL;
 	return b;
 }
-EXPORT_SYMBOL_GPL(bio_alloc_mddev);
 
 /*
  * We have a system wide 'event count' that is incremented
@@ -393,7 +393,7 @@ static void submit_flushes(struct work_struct *ws)
 			atomic_inc(&rdev->nr_pending);
 			atomic_inc(&rdev->nr_pending);
 			rcu_read_unlock();
-			bi = bio_alloc_mddev(GFP_NOIO, 0, mddev);
+			bi = bio_alloc_bioset(GFP_NOIO, 0, mddev->bio_set);
 			bi->bi_end_io = md_end_flush;
 			bi->bi_private = rdev;
 			bi->bi_bdev = rdev->bdev;
diff --git a/drivers/md/md.h b/drivers/md/md.h
index b8859cb..44fe227 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -673,8 +673,6 @@ extern void md_rdev_clear(struct md_rdev *rdev);
 
 extern void mddev_suspend(struct mddev *mddev);
 extern void mddev_resume(struct mddev *mddev);
-extern struct bio *bio_alloc_mddev(gfp_t gfp_mask, int nr_iovecs,
-				   struct mddev *mddev);
 
 extern void md_unplug(struct blk_plug_cb *cb, bool from_schedule);
 extern void md_reload_sb(struct mddev *mddev, int raid_disk);
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index ad5c948..2180a9a 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2281,7 +2281,7 @@ static int narrow_write_error(struct r1bio *r1_bio, int i)
 				vcnt--;
 			}
 
-			wbio = bio_alloc_mddev(GFP_NOIO, vcnt, mddev);
+			wbio = bio_alloc_bioset(GFP_NOIO, vcnt, mddev->bio_set);
 			memcpy(wbio->bi_io_vec, vec, vcnt * sizeof(struct bio_vec));
 
 			wbio->bi_vcnt = vcnt;
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index ade7d69..a1f8e98 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -4473,7 +4473,7 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
 		return sectors_done;
 	}
 
-	read_bio = bio_alloc_mddev(GFP_KERNEL, RESYNC_PAGES, mddev);
+	read_bio = bio_alloc_bioset(GFP_KERNEL, RESYNC_PAGES, mddev->bio_set);
 
 	read_bio->bi_bdev = rdev->bdev;
 	read_bio->bi_iter.bi_sector = (r10_bio->devs[r10_bio->read_slot].addr
-- 
2.9.3

^ permalink raw reply related

* Re: [PATCHv4 4/4] MAINTAINERS: Remove powerpc's opal match
From: Andrew Donnellan @ 2017-02-15 23:55 UTC (permalink / raw)
  To: Jon Derrick
  Cc: Jens Axboe, Rafael Antognolli, Greg Kroah-Hartman, linux-kernel,
	linux-block, linuxppc-dev, Christoph Hellwig, Scott Bauer
In-Reply-To: <1487195159-12599-5-git-send-email-jonathan.derrick@intel.com>

On 16/02/17 08:45, Jon Derrick wrote:
> PPC's 'opal' match pattern also matches block/sed-opal.c, where it looks
> like the 'arch/powerpc' file pattern should be enough to match powerpc
> opal code by itself. Remove the opal regex pattern from powerpc.
>
> Signed-off-by: Jon Derrick <jonathan.derrick@intel.com>

It looks like this change will exclude the following PPC OPAL related files:

drivers/tty/hvc/hvc_opal.c - HYPERVISOR VIRTUAL CONSOLE DRIVER (which 
lists linuxppc-dev as the mailing list, doesn't name a maintainer though)

drivers/i2c/busses/i2c-opal.c - I2C SUBSYSTEM

drivers/rtc/rtc-opal.c - REAL TIME CLOCK (RTC) SUBSYSTEM

Documentation/devicetree/bindings/i2c/i2c-opal.txt - I2C SUBSYSTEM, OPEN 
FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Documentation/devicetree/bindings/powerpc/opal - OPEN FIRMWARE AND 
FLATTENED DEVICE TREE BINDINGS

Documentation/devicetree/bindings/powerpc/opal/oppanel-opal.txt - OPEN 
FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Documentation/devicetree/bindings/rtc/rtc-opal.txt - OPEN FIRMWARE AND 
FLATTENED DEVICE TREE BINDINGS, REAL TIME CLOCK (RTC) SUBSYSTEM

Documentation/ABI/stable/sysfs-firmware-opal-elog - no other subsystem

Documentation/ABI/stable/sysfs-firmware-opal-dump - no other subsystem


-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

^ permalink raw reply

* [PATCH] opal: Use empty structure when not defined
From: Keith Busch @ 2017-02-16  0:16 UTC (permalink / raw)
  To: linux-block, Scott Bauer, Jonathan Derrick; +Cc: Jens Axboe, Keith Busch

No need to use space if it can't be used. This reduces the size of struct
nvme_ctrl by a little over 4k when CONFIG_BLK_SED_OPAL is not set.

Signed-off-by: Keith Busch <keith.busch@intel.com>
---
 include/linux/sed-opal.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
index 205d520..ed19902 100644
--- a/include/linux/sed-opal.h
+++ b/include/linux/sed-opal.h
@@ -91,6 +91,7 @@ struct parsed_resp {
 	struct opal_resp_tok toks[MAX_TOKS];
 };
 
+#ifdef CONFIG_BLK_SED_OPAL
 /**
  * struct opal_dev - The structure representing a OPAL enabled SED.
  * @supported: Whether or not OPAL is supported on this controller.
@@ -129,7 +130,6 @@ struct opal_dev {
 	struct list_head unlk_lst;
 };
 
-#ifdef CONFIG_BLK_SED_OPAL
 bool opal_unlock_from_suspend(struct opal_dev *dev);
 void init_opal_dev(struct opal_dev *opal_dev, sec_send_recv *send_recv);
 int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *ioctl_ptr);
@@ -154,6 +154,9 @@ static inline bool is_sed_ioctl(unsigned int cmd)
 	return false;
 }
 #else
+struct opal_dev {
+};
+
 static inline bool is_sed_ioctl(unsigned int cmd)
 {
 	return false;
@@ -171,8 +174,6 @@ static inline bool opal_unlock_from_suspend(struct opal_dev *dev)
 static inline void init_opal_dev(struct opal_dev *opal_dev,
 				 sec_send_recv *send_recv)
 {
-	opal_dev->supported = false;
-	opal_dev->initialized = true;
 }
 #endif /* CONFIG_BLK_SED_OPAL */
 #endif /* LINUX_OPAL_H */
-- 
2.5.5

^ permalink raw reply related

* Re: [PATCHv3 4/4] MAINTAINERS: Remove powerpc's opal match
From: Stewart Smith @ 2017-02-16  0:37 UTC (permalink / raw)
  To: Jon Derrick
  Cc: Jon Derrick, linux-block, linux-kernel, linuxppc-dev, Scott Bauer,
	Rafael Antognolli, Jens Axboe, Christoph Hellwig,
	Greg Kroah-Hartman
In-Reply-To: <1487187731-10293-5-git-send-email-jonathan.derrick@intel.com>

Jon Derrick <jonathan.derrick@intel.com> writes:
> PPC's 'opal' match pattern also matches block/sed-opal.c, where it looks
> like the 'arch/powerpc' file pattern should be enough to match powerpc
> opal code by itself. Remove the opal regex pattern from powerpc.

This patch will end up missing some code, what about this instead:


Remove OPAL regex in powerpc to avoid false match

Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

---
 MAINTAINERS |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3960e7f..25ed25a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7393,18 +7393,24 @@ L:	linuxppc-dev@lists.ozlabs.org
 Q:	http://patchwork.ozlabs.org/project/linuxppc-dev/list/
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git
 S:	Supported
+F:	Documentation/ABI/stable/sysfs-firmware-opal-*
+F:	Documentation/devicetree/bindings/powerpc/opal/
+F:	Documentation/devicetree/bindings/rtc/rtc-opal.txt
+F:	Documentation/devicetree/bindings/i2c/i2c-opal.txt
 F:	Documentation/powerpc/
 F:	arch/powerpc/
 F:	drivers/char/tpm/tpm_ibmvtpm*
 F:	drivers/crypto/nx/
 F:	drivers/crypto/vmx/
+F:	drivers/i2c/busses/ic2-opal.c
 F:	drivers/net/ethernet/ibm/ibmveth.*
 F:	drivers/net/ethernet/ibm/ibmvnic.*
 F:	drivers/pci/hotplug/pnv_php.c
 F:	drivers/pci/hotplug/rpa*
+F:	drivers/rtc/rtc-opal.c
 F:	drivers/scsi/ibmvscsi/
+F:	drivers/tty/hvc/hvc_opal.c
 F:	tools/testing/selftests/powerpc
-N:	opal
 N:	/pmac
 N:	powermac
 N:	powernv


-- 
Stewart Smith
OPAL Architect, IBM.

^ permalink raw reply related

* Re: [PATCHv3 4/4] MAINTAINERS: Remove powerpc's opal match
From: Andrew Donnellan @ 2017-02-16  1:00 UTC (permalink / raw)
  To: Stewart Smith, Jon Derrick
  Cc: Jens Axboe, Rafael Antognolli, Greg Kroah-Hartman, linux-kernel,
	linux-block, linuxppc-dev, Christoph Hellwig, Scott Bauer
In-Reply-To: <87h93v7yys.fsf@linux.vnet.ibm.com>

On 16/02/17 11:37, Stewart Smith wrote:
> Jon Derrick <jonathan.derrick@intel.com> writes:
>> PPC's 'opal' match pattern also matches block/sed-opal.c, where it looks
>> like the 'arch/powerpc' file pattern should be enough to match powerpc
>> opal code by itself. Remove the opal regex pattern from powerpc.
>
> This patch will end up missing some code, what about this instead:
>
>
> Remove OPAL regex in powerpc to avoid false match
>
> Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>

Per my reply to v4 of this series, your list matches mine. With the one 
typo correction below:

Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

>
> ---
>  MAINTAINERS |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3960e7f..25ed25a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7393,18 +7393,24 @@ L:	linuxppc-dev@lists.ozlabs.org
>  Q:	http://patchwork.ozlabs.org/project/linuxppc-dev/list/
>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git
>  S:	Supported
> +F:	Documentation/ABI/stable/sysfs-firmware-opal-*
> +F:	Documentation/devicetree/bindings/powerpc/opal/
> +F:	Documentation/devicetree/bindings/rtc/rtc-opal.txt
> +F:	Documentation/devicetree/bindings/i2c/i2c-opal.txt
>  F:	Documentation/powerpc/
>  F:	arch/powerpc/
>  F:	drivers/char/tpm/tpm_ibmvtpm*
>  F:	drivers/crypto/nx/
>  F:	drivers/crypto/vmx/
> +F:	drivers/i2c/busses/ic2-opal.c

i2c-opal

>  F:	drivers/net/ethernet/ibm/ibmveth.*
>  F:	drivers/net/ethernet/ibm/ibmvnic.*
>  F:	drivers/pci/hotplug/pnv_php.c
>  F:	drivers/pci/hotplug/rpa*
> +F:	drivers/rtc/rtc-opal.c
>  F:	drivers/scsi/ibmvscsi/
> +F:	drivers/tty/hvc/hvc_opal.c
>  F:	tools/testing/selftests/powerpc
> -N:	opal
>  N:	/pmac
>  N:	powermac
>  N:	powernv
>
>

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

^ permalink raw reply

* Re: [PATCH] opal: Use empty structure when not defined
From: Scott Bauer @ 2017-02-16  2:01 UTC (permalink / raw)
  To: Keith Busch; +Cc: linux-block, Jonathan Derrick, Jens Axboe
In-Reply-To: <1487204187-4004-1-git-send-email-keith.busch@intel.com>

On Wed, Feb 15, 2017 at 07:16:27PM -0500, Keith Busch wrote:
> No need to use space if it can't be used. This reduces the size of struct
> nvme_ctrl by a little over 4k when CONFIG_BLK_SED_OPAL is not set.
> 
> Signed-off-by: Keith Busch <keith.busch@intel.com>
> ---
>  include/linux/sed-opal.h | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
> index 205d520..ed19902 100644
> --- a/include/linux/sed-opal.h
> +++ b/include/linux/sed-opal.h
> @@ -91,6 +91,7 @@ struct parsed_resp {
>  	struct opal_resp_tok toks[MAX_TOKS];
>  };
>  
> +#ifdef CONFIG_BLK_SED_OPAL
>  /**
>   * struct opal_dev - The structure representing a OPAL enabled SED.
>   * @supported: Whether or not OPAL is supported on this controller.
> @@ -129,7 +130,6 @@ struct opal_dev {
>  	struct list_head unlk_lst;
>  };
>  
> -#ifdef CONFIG_BLK_SED_OPAL
>  bool opal_unlock_from_suspend(struct opal_dev *dev);
>  void init_opal_dev(struct opal_dev *opal_dev, sec_send_recv *send_recv);
>  int sed_ioctl(struct opal_dev *dev, unsigned int cmd, void __user *ioctl_ptr);
> @@ -154,6 +154,9 @@ static inline bool is_sed_ioctl(unsigned int cmd)
>  	return false;
>  }
>  #else
> +struct opal_dev {
> +};
> +
>  static inline bool is_sed_ioctl(unsigned int cmd)
>  {
>  	return false;
> @@ -171,8 +174,6 @@ static inline bool opal_unlock_from_suspend(struct opal_dev *dev)
>  static inline void init_opal_dev(struct opal_dev *opal_dev,
>  				 sec_send_recv *send_recv)
>  {
> -	opal_dev->supported = false;
> -	opal_dev->initialized = true;
>  }
>  #endif /* CONFIG_BLK_SED_OPAL */
>  #endif /* LINUX_OPAL_H */
> -- 
> 2.5.5
> 

Looks good, thank you.
Reviewed-by: Scott Bauer <scott.bauer@intel.com>

^ permalink raw reply

* Re: sense handling improvements
From: Martin K. Petersen @ 2017-02-16  3:42 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: axboe, linux-block, linux-scsi
In-Reply-To: <20170214191600.17480-1-hch@lst.de>

>>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:

Christoph> this series is on top of the scsi_request changes in Jens'
Christoph> tree and further improves the handling of the sense buffer.

Very nice cleanup!

Christoph> The first patch prevents any possibily of reusing stale sense
Christoph> codes in sense headers, and is a bug fix that we should
Christoph> probably get into the block tree ASAP.

Christoph> The rest cleans up handling of the parsed sense data and
Christoph> could go in either through the block tree, or a SCSI branch
Christoph> on top of the block tree.

I can bring them in after Linus' initial block pull.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply

* Re: [PATCHv3 4/4] MAINTAINERS: Remove powerpc's opal match
From: Michael Ellerman @ 2017-02-16  5:23 UTC (permalink / raw)
  To: Jon Derrick
  Cc: Jens Axboe, Rafael Antognolli, Greg Kroah-Hartman, linux-kernel,
	linux-block, Jon Derrick, linuxppc-dev, Christoph Hellwig,
	Scott Bauer
In-Reply-To: <1487187731-10293-5-git-send-email-jonathan.derrick@intel.com>

Jon Derrick <jonathan.derrick@intel.com> writes:

> PPC's 'opal' match pattern also matches block/sed-opal.c, where it looks
> like the 'arch/powerpc' file pattern should be enough to match powerpc
> opal code by itself. Remove the opal regex pattern from powerpc.

We thought of it first.

Can't you just rename your driver, Opal Storage Specification, so "oss",
that should be pretty unique?

... :)

I don't like this version, but I'll merge the one from Stewart which
drops the pattern and adds the paths for the existing drivers.

cheers

> diff --git a/MAINTAINERS b/MAINTAINERS
> index b983b25..430dd02 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7404,7 +7404,6 @@ F:	drivers/pci/hotplug/pnv_php.c
>  F:	drivers/pci/hotplug/rpa*
>  F:	drivers/scsi/ibmvscsi/
>  F:	tools/testing/selftests/powerpc
> -N:	opal
>  N:	/pmac
>  N:	powermac
>  N:	powernv
> -- 
> 1.8.3.1

^ permalink raw reply

* Re: [PATCH] opal: Use empty structure when not defined
From: Christoph Hellwig @ 2017-02-16  7:58 UTC (permalink / raw)
  To: Keith Busch; +Cc: linux-block, Scott Bauer, Jonathan Derrick, Jens Axboe
In-Reply-To: <1487204187-4004-1-git-send-email-keith.busch@intel.com>

I'd rather prefer to make the structure separately allocated as
discussed before.  Scott, can you test the patch below?  I'm not near
my devices I could test on.

---
>From b2cda0c7ec5c0ec66582655751838f519cfa1706 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@lst.de>
Date: Thu, 16 Feb 2017 08:49:56 +0100
Subject: block/sed-opal: make struct opal_dev private

This moves the definition of struct opal_dev into sed-opal.c.  For this a
new private data field is added to it that is passed to the send/receive
callback.  After that a lot of internals can be made private.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/opal_proto.h       |  23 ++++++++++
 block/sed-opal.c         | 101 +++++++++++++++++++++++++++++++++++++----
 drivers/nvme/host/core.c |   9 ++--
 drivers/nvme/host/nvme.h |  14 ++----
 drivers/nvme/host/pci.c  |   8 +++-
 include/linux/sed-opal.h | 116 ++---------------------------------------------
 6 files changed, 131 insertions(+), 140 deletions(-)

diff --git a/block/opal_proto.h b/block/opal_proto.h
index af9abc56c157..f40c9acf8895 100644
--- a/block/opal_proto.h
+++ b/block/opal_proto.h
@@ -19,6 +19,29 @@
 #ifndef _OPAL_PROTO_H
 #define _OPAL_PROTO_H
 
+/*
+ * These constant values come from:
+ * SPC-4 section
+ * 6.30 SECURITY PROTOCOL IN command / table 265.
+ */
+enum {
+	TCG_SECP_00 = 0,
+	TCG_SECP_01,
+};
+
+/*
+ * Token defs derived from:
+ * TCG_Storage_Architecture_Core_Spec_v2.01_r1.00
+ * 3.2.2 Data Stream Encoding
+ */
+enum opal_response_token {
+	OPAL_DTA_TOKENID_BYTESTRING = 0xe0,
+	OPAL_DTA_TOKENID_SINT = 0xe1,
+	OPAL_DTA_TOKENID_UINT = 0xe2,
+	OPAL_DTA_TOKENID_TOKEN = 0xe3, /* actual token is returned */
+	OPAL_DTA_TOKENID_INVALID = 0X0
+};
+
 #define DTAERROR_NO_METHOD_STATUS 0x89
 #define GENERIC_HOST_SESSION_NUM 0x41
 
diff --git a/block/sed-opal.c b/block/sed-opal.c
index bf1406e5159b..bdab4dfcbafd 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -31,6 +31,77 @@
 
 #include "opal_proto.h"
 
+#define IO_BUFFER_LENGTH 2048
+#define MAX_TOKS 64
+
+typedef int (*opal_step)(struct opal_dev *dev);
+
+enum opal_atom_width {
+	OPAL_WIDTH_TINY,
+	OPAL_WIDTH_SHORT,
+	OPAL_WIDTH_MEDIUM,
+	OPAL_WIDTH_LONG,
+	OPAL_WIDTH_TOKEN
+};
+
+/*
+ * On the parsed response, we don't store again the toks that are already
+ * stored in the response buffer. Instead, for each token, we just store a
+ * pointer to the position in the buffer where the token starts, and the size
+ * of the token in bytes.
+ */
+struct opal_resp_tok {
+	const u8 *pos;
+	size_t len;
+	enum opal_response_token type;
+	enum opal_atom_width width;
+	union {
+		u64 u;
+		s64 s;
+	} stored;
+};
+
+/*
+ * From the response header it's not possible to know how many tokens there are
+ * on the payload. So we hardcode that the maximum will be MAX_TOKS, and later
+ * if we start dealing with messages that have more than that, we can increase
+ * this number. This is done to avoid having to make two passes through the
+ * response, the first one counting how many tokens we have and the second one
+ * actually storing the positions.
+ */
+struct parsed_resp {
+	int num;
+	struct opal_resp_tok toks[MAX_TOKS];
+};
+
+struct opal_dev {
+	bool supported;
+
+	void *data;
+	sec_send_recv *send_recv;
+
+	const opal_step *funcs;
+	void **func_data;
+	int state;
+	struct mutex dev_lock;
+	u16 comid;
+	u32 hsn;
+	u32 tsn;
+	u64 align;
+	u64 lowest_lba;
+
+	size_t pos;
+	u8 cmd[IO_BUFFER_LENGTH];
+	u8 resp[IO_BUFFER_LENGTH];
+
+	struct parsed_resp parsed;
+	size_t prev_d_len;
+	void *prev_data;
+
+	struct list_head unlk_lst;
+};
+
+
 static const u8 opaluid[][OPAL_UID_LENGTH] = {
 	/* users */
 	[OPAL_SMUID_UID] =
@@ -243,14 +314,14 @@ static u16 get_comid_v200(const void *data)
 
 static int opal_send_cmd(struct opal_dev *dev)
 {
-	return dev->send_recv(dev, dev->comid, TCG_SECP_01,
+	return dev->send_recv(dev->data, dev->comid, TCG_SECP_01,
 			      dev->cmd, IO_BUFFER_LENGTH,
 			      true);
 }
 
 static int opal_recv_cmd(struct opal_dev *dev)
 {
-	return dev->send_recv(dev, dev->comid, TCG_SECP_01,
+	return dev->send_recv(dev->data, dev->comid, TCG_SECP_01,
 			      dev->resp, IO_BUFFER_LENGTH,
 			      false);
 }
@@ -1943,16 +2014,24 @@ static int check_opal_support(struct opal_dev *dev)
 	return ret;
 }
 
-void init_opal_dev(struct opal_dev *opal_dev, sec_send_recv *send_recv)
+struct opal_dev *init_opal_dev(void *data, sec_send_recv *send_recv)
 {
-	if (opal_dev->initialized)
-		return;
-	INIT_LIST_HEAD(&opal_dev->unlk_lst);
-	mutex_init(&opal_dev->dev_lock);
-	opal_dev->send_recv = send_recv;
-	if (check_opal_support(opal_dev) < 0)
+	struct opal_dev *dev;
+
+	dev = kmalloc(sizeof(*dev), GFP_KERNEL);
+	if (!dev)
+		return NULL;
+
+	INIT_LIST_HEAD(&dev->unlk_lst);
+	mutex_init(&dev->dev_lock);
+	dev->data = data;
+	dev->send_recv = send_recv;
+	if (check_opal_support(dev) < 0) {
 		pr_warn("Opal is not supported on this device\n");
-	opal_dev->initialized = true;
+		kfree(dev);
+		return NULL;
+	}
+	return dev;
 }
 EXPORT_SYMBOL(init_opal_dev);
 
@@ -2350,6 +2429,8 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd, unsigned long ptr)
 
 	if (!capable(CAP_SYS_ADMIN))
 		return -EACCES;
+	if (!dev)
+		return -ENOTSUPP;
 	if (!dev->supported) {
 		pr_err("Not supported\n");
 		return -ENOTSUPP;
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index d9f49036819c..84eb86cc4a45 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -811,7 +811,7 @@ static int nvme_ioctl(struct block_device *bdev, fmode_t mode,
 			return nvme_nvm_ioctl(ns, cmd, arg);
 #endif
 		if (is_sed_ioctl(cmd))
-			return sed_ioctl(&ns->ctrl->opal_dev, cmd, arg);
+			return sed_ioctl(ns->ctrl->opal_dev, cmd, arg);
 		return -ENOTTY;
 	}
 }
@@ -1084,18 +1084,17 @@ static const struct pr_ops nvme_pr_ops = {
 };
 
 #ifdef CONFIG_BLK_SED_OPAL
-int nvme_sec_submit(struct opal_dev *dev, u16 spsp, u8 secp,
-		    void *buffer, size_t len, bool send)
+int nvme_sec_submit(void *data, u16 spsp, u8 secp, void *buffer, size_t len,
+		bool send)
 {
+	struct nvme_ctrl *ctrl = data;
 	struct nvme_command cmd;
-	struct nvme_ctrl *ctrl = NULL;
 
 	memset(&cmd, 0, sizeof(cmd));
 	if (send)
 		cmd.common.opcode = nvme_admin_security_send;
 	else
 		cmd.common.opcode = nvme_admin_security_recv;
-	ctrl = container_of(dev, struct nvme_ctrl, opal_dev);
 	cmd.common.nsid = 0;
 	cmd.common.cdw10[0] = cpu_to_le32(((u32)secp) << 24 | ((u32)spsp) << 8);
 	cmd.common.cdw10[1] = cpu_to_le32(len);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 569cba14cede..5126c4bbee1a 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -126,7 +126,7 @@ struct nvme_ctrl {
 	struct list_head node;
 	struct ida ns_ida;
 
-	struct opal_dev opal_dev;
+	struct opal_dev *opal_dev;
 
 	char name[12];
 	char serial[20];
@@ -270,16 +270,8 @@ int nvme_init_identify(struct nvme_ctrl *ctrl);
 void nvme_queue_scan(struct nvme_ctrl *ctrl);
 void nvme_remove_namespaces(struct nvme_ctrl *ctrl);
 
-#ifdef CONFIG_BLK_SED_OPAL
-int nvme_sec_submit(struct opal_dev *dev, u16 spsp, u8 secp,
-		    void *buffer, size_t len, bool send);
-#else
-static inline int nvme_sec_submit(struct opal_dev *dev, u16 spsp, u8 secp,
-				  void *buffer, size_t len, bool send)
-{
-	return 0;
-}
-#endif /* CONFIG_BLK_DEV_SED_OPAL */
+int nvme_sec_submit(void *data, u16 spsp, u8 secp, void *buffer, size_t len,
+		bool send);
 
 #define NVME_NR_AERS	1
 void nvme_complete_async_event(struct nvme_ctrl *ctrl, __le16 status,
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index e25d632bd9f2..5db8a38a8b43 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1739,6 +1739,7 @@ static void nvme_pci_free_ctrl(struct nvme_ctrl *ctrl)
 	if (dev->ctrl.admin_q)
 		blk_put_queue(dev->ctrl.admin_q);
 	kfree(dev->queues);
+	kfree(dev->ctrl.opal_dev);
 	kfree(dev);
 }
 
@@ -1788,10 +1789,13 @@ static void nvme_reset_work(struct work_struct *work)
 	if (result)
 		goto out;
 
-	init_opal_dev(&dev->ctrl.opal_dev, &nvme_sec_submit);
+	if (!dev->ctrl.opal_dev) {
+		dev->ctrl.opal_dev =
+			init_opal_dev(&dev->ctrl, &nvme_sec_submit);
+	}
 
 	if (was_suspend)
-		opal_unlock_from_suspend(&dev->ctrl.opal_dev);
+		opal_unlock_from_suspend(dev->ctrl.opal_dev);
 
 	result = nvme_setup_io_queues(dev);
 	if (result)
diff --git a/include/linux/sed-opal.h b/include/linux/sed-opal.h
index af1a85eae193..da39002285d0 100644
--- a/include/linux/sed-opal.h
+++ b/include/linux/sed-opal.h
@@ -21,117 +21,14 @@
 #include <uapi/linux/sed-opal.h>
 #include <linux/kernel.h>
 
-/*
- * These constant values come from:
- * SPC-4 section
- * 6.30 SECURITY PROTOCOL IN command / table 265.
- */
-enum {
-	TCG_SECP_00 = 0,
-	TCG_SECP_01,
-};
 struct opal_dev;
 
-#define IO_BUFFER_LENGTH 2048
-#define MAX_TOKS 64
-
-typedef int (*opal_step)(struct opal_dev *dev);
-typedef int (sec_send_recv)(struct opal_dev *ctx, u16 spsp, u8 secp,
-			    void *buffer, size_t len, bool send);
-
-
-enum opal_atom_width {
-	OPAL_WIDTH_TINY,
-	OPAL_WIDTH_SHORT,
-	OPAL_WIDTH_MEDIUM,
-	OPAL_WIDTH_LONG,
-	OPAL_WIDTH_TOKEN
-};
-
-/*
- * Token defs derived from:
- * TCG_Storage_Architecture_Core_Spec_v2.01_r1.00
- * 3.2.2 Data Stream Encoding
- */
-enum opal_response_token {
-	OPAL_DTA_TOKENID_BYTESTRING = 0xe0,
-	OPAL_DTA_TOKENID_SINT = 0xe1,
-	OPAL_DTA_TOKENID_UINT = 0xe2,
-	OPAL_DTA_TOKENID_TOKEN = 0xe3, /* actual token is returned */
-	OPAL_DTA_TOKENID_INVALID = 0X0
-};
-
-/*
- * On the parsed response, we don't store again the toks that are already
- * stored in the response buffer. Instead, for each token, we just store a
- * pointer to the position in the buffer where the token starts, and the size
- * of the token in bytes.
- */
-struct opal_resp_tok {
-	const u8 *pos;
-	size_t len;
-	enum opal_response_token type;
-	enum opal_atom_width width;
-	union {
-		u64 u;
-		s64 s;
-	} stored;
-};
-
-/*
- * From the response header it's not possible to know how many tokens there are
- * on the payload. So we hardcode that the maximum will be MAX_TOKS, and later
- * if we start dealing with messages that have more than that, we can increase
- * this number. This is done to avoid having to make two passes through the
- * response, the first one counting how many tokens we have and the second one
- * actually storing the positions.
- */
-struct parsed_resp {
-	int num;
-	struct opal_resp_tok toks[MAX_TOKS];
-};
-
-/**
- * struct opal_dev - The structure representing a OPAL enabled SED.
- * @supported: Whether or not OPAL is supported on this controller.
- * @send_recv: The combined sec_send/sec_recv function pointer.
- * @opal_step: A series of opal methods that are necessary to complete a command.
- * @func_data: An array of parameters for the opal methods above.
- * @state: Describes the current opal_step we're working on.
- * @dev_lock: Locks the entire opal_dev structure.
- * @parsed: Parsed response from controller.
- * @prev_data: Data returned from a method to the controller.
- * @unlk_lst: A list of Locking ranges to unlock on this device during a resume.
- */
-struct opal_dev {
-	bool initialized;
-	bool supported;
-	sec_send_recv *send_recv;
-
-	const opal_step *funcs;
-	void **func_data;
-	int state;
-	struct mutex dev_lock;
-	u16 comid;
-	u32 hsn;
-	u32 tsn;
-	u64 align;
-	u64 lowest_lba;
-
-	size_t pos;
-	u8 cmd[IO_BUFFER_LENGTH];
-	u8 resp[IO_BUFFER_LENGTH];
-
-	struct parsed_resp parsed;
-	size_t prev_d_len;
-	void *prev_data;
-
-	struct list_head unlk_lst;
-};
+typedef int (sec_send_recv)(void *data, u16 spsp, u8 secp, void *buffer,
+		size_t len, bool send);
 
 #ifdef CONFIG_BLK_SED_OPAL
 bool opal_unlock_from_suspend(struct opal_dev *dev);
-void init_opal_dev(struct opal_dev *opal_dev, sec_send_recv *send_recv);
+struct opal_dev * init_opal_dev(void *data, sec_send_recv *send_recv);
 int sed_ioctl(struct opal_dev *dev, unsigned int cmd, unsigned long ptr);
 
 static inline bool is_sed_ioctl(unsigned int cmd)
@@ -168,11 +65,6 @@ static inline bool opal_unlock_from_suspend(struct opal_dev *dev)
 {
 	return false;
 }
-static inline void init_opal_dev(struct opal_dev *opal_dev,
-				 sec_send_recv *send_recv)
-{
-	opal_dev->supported = false;
-	opal_dev->initialized = true;
-}
+#define init_opal_dev(data, send_recv)		NULL
 #endif /* CONFIG_BLK_SED_OPAL */
 #endif /* LINUX_OPAL_H */
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH] blk-mq-sched: don't hold queue_lock when calling exit_icq
From: Paolo Valente @ 2017-02-16 10:31 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Omar Sandoval, linux-block, kernel-team
In-Reply-To: <9fe1b3a9-2a80-8891-1044-f83558e28d15@fb.com>


> Il giorno 15 feb 2017, alle ore 19:04, Jens Axboe <axboe@fb.com> ha =
scritto:
>=20
> On 02/15/2017 10:58 AM, Jens Axboe wrote:
>> On 02/15/2017 10:24 AM, Paolo Valente wrote:
>>>=20
>>>> Il giorno 10 feb 2017, alle ore 19:32, Omar Sandoval =
<osandov@osandov.com> ha scritto:
>>>>=20
>>>> From: Omar Sandoval <osandov@fb.com>
>>>>=20
>>>> None of the other blk-mq elevator hooks are called with this lock =
held.
>>>> Additionally, it can lead to circular locking dependencies between
>>>> queue_lock and the private scheduler lock.
>>>>=20
>>>=20
>>> Hi Omar,
>>> I'm sorry but it seems that a new potential deadlock has showed up.
>>> See lockdep splat below.
>>>=20
>>> I've tried to think about different solutions than turning back to
>>> deferring the body of exit_icq, but at no avail.
>>=20
>> Looks like a interaction between bfqd->lock and q->queue_lock. Since =
the
>> core has no notion of you bfqd->lock, the naturally dependency here
>> would be to nest bfqd->lock inside q->queue_lock. Is that possible =
for
>> you?
>>=20
>> Looking at the code a bit, maybe it'd just be simpler to get rid of
>> holding the queue lock for that spot. For the mq scheduler, we really
>> don't want places where we invoke with that held already. Does the =
below
>> work for you?
>=20
> Would need to remove one more lockdep assert. And only test this for
> the mq parts, we'd need to spread a bit of love on the classic
> scheduling icq exit path for this to work on that side.
>=20

Sorry Jens, same splat.  What confuses me is the second column
in the possible scenario:

[  139.368477]        CPU0                    	CPU1
[  139.369129]        ----                    	----
[  139.369774]   lock(&(&ioc->lock)->rlock);
[  139.370339]                                		=
lock(&(&q->__queue_lock)->rlock);
[  139.390579]                                		=
lock(&(&ioc->lock)->rlock);
[  139.391522]   lock(&(&bfqd->lock)->rlock);

I could not find any code path, related to the reported call traces,
and taking first q->queue_lock and then ioc->lock.

Any suggestion on how to go on, and hopefully help with this problem is
welcome.

Thanks,
Paolo

> diff --git a/block/blk-ioc.c b/block/blk-ioc.c
> index b12f9c87b4c3..546ff8f81ede 100644
> --- a/block/blk-ioc.c
> +++ b/block/blk-ioc.c
> @@ -54,7 +54,7 @@ static void ioc_exit_icq(struct io_cq *icq)
> 	icq->flags |=3D ICQ_EXITED;
> }
>=20
> -/* Release an icq.  Called with both ioc and q locked. */
> +/* Release an icq.  Called with ioc locked. */
> static void ioc_destroy_icq(struct io_cq *icq)
> {
> 	struct io_context *ioc =3D icq->ioc;
> @@ -62,7 +62,6 @@ static void ioc_destroy_icq(struct io_cq *icq)
> 	struct elevator_type *et =3D q->elevator->type;
>=20
> 	lockdep_assert_held(&ioc->lock);
> -	lockdep_assert_held(q->queue_lock);
>=20
> 	radix_tree_delete(&ioc->icq_tree, icq->q->id);
> 	hlist_del_init(&icq->ioc_node);
> @@ -222,25 +221,34 @@ void exit_io_context(struct task_struct *task)
> 	put_io_context_active(ioc);
> }
>=20
> +static void __ioc_clear_queue(struct list_head *icq_list)
> +{
> +	while (!list_empty(icq_list)) {
> +		struct io_cq *icq =3D list_entry(icq_list->next,
> +					       struct io_cq, q_node);
> +		struct io_context *ioc =3D icq->ioc;
> +
> +		spin_lock_irq(&ioc->lock);
> +		ioc_destroy_icq(icq);
> +		spin_unlock_irq(&ioc->lock);
> +	}
> +}
> +
> /**
>  * ioc_clear_queue - break any ioc association with the specified =
queue
>  * @q: request_queue being cleared
>  *
> - * Walk @q->icq_list and exit all io_cq's.  Must be called with @q =
locked.
> + * Walk @q->icq_list and exit all io_cq's.
>  */
> void ioc_clear_queue(struct request_queue *q)
> {
> -	lockdep_assert_held(q->queue_lock);
> +	LIST_HEAD(icq_list);
>=20
> -	while (!list_empty(&q->icq_list)) {
> -		struct io_cq *icq =3D list_entry(q->icq_list.next,
> -					       struct io_cq, q_node);
> -		struct io_context *ioc =3D icq->ioc;
> +	spin_lock_irq(q->queue_lock);
> +	list_splice_init(&q->icq_list, &icq_list);
> +	spin_unlock_irq(q->queue_lock);
>=20
> -		spin_lock(&ioc->lock);
> -		ioc_destroy_icq(icq);
> -		spin_unlock(&ioc->lock);
> -	}
> +	__ioc_clear_queue(&icq_list);
> }
>=20
> int create_task_io_context(struct task_struct *task, gfp_t gfp_flags, =
int node)
> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
> index 070d81bae1d5..1944aa1cb899 100644
> --- a/block/blk-sysfs.c
> +++ b/block/blk-sysfs.c
> @@ -815,9 +815,7 @@ static void blk_release_queue(struct kobject =
*kobj)
> 	blkcg_exit_queue(q);
>=20
> 	if (q->elevator) {
> -		spin_lock_irq(q->queue_lock);
> 		ioc_clear_queue(q);
> -		spin_unlock_irq(q->queue_lock);
> 		elevator_exit(q->elevator);
> 	}
>=20
> diff --git a/block/elevator.c b/block/elevator.c
> index a25bdd90b270..aaa1e9836512 100644
> --- a/block/elevator.c
> +++ b/block/elevator.c
> @@ -985,9 +985,7 @@ static int elevator_switch(struct request_queue =
*q, struct elevator_type *new_e)
> 		if (old_registered)
> 			elv_unregister_queue(q);
>=20
> -		spin_lock_irq(q->queue_lock);
> 		ioc_clear_queue(q);
> -		spin_unlock_irq(q->queue_lock);
> 	}
>=20
> 	/* allocate, init and register new elevator */
>=20
> --=20
> Jens Axboe

^ permalink raw reply

* Re: [PATCH 7/8] mq-deadline: add blk-mq adaptation of the deadline IO scheduler
From: Paolo Valente @ 2017-02-16 10:46 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Jens Axboe, linux-block, Linux-Kernal, osandov
In-Reply-To: <1481933536-12844-8-git-send-email-axboe@fb.com>


> Il giorno 17 dic 2016, alle ore 01:12, Jens Axboe <axboe@fb.com> ha =
scritto:
>=20
> This is basically identical to deadline-iosched, except it registers
> as a MQ capable scheduler. This is still a single queue design.
>=20
> Signed-off-by: Jens Axboe <axboe@fb.com>
...
> +
> +static void dd_merged_requests(struct request_queue *q, struct =
request *req,
> +			       struct request *next)
> +{
> +	/*
> +	 * if next expires before rq, assign its expire time to rq
> +	 * and move into next position (next will be deleted) in fifo
> +	 */
> +	if (!list_empty(&req->queuelist) && =
!list_empty(&next->queuelist)) {
> +		if (time_before((unsigned long)next->fifo_time,
> +				(unsigned long)req->fifo_time)) {
> +			list_move(&req->queuelist, &next->queuelist);
> +			req->fifo_time =3D next->fifo_time;
> +		}
> +	}
> +

Jens,
while trying to imagine the possible causes of Bart's hang with
bfq-mq, I've bumped into the following doubt: in the above function
(in my case, in bfq-mq-'s equivalent of the above function), are
we sure that neither req or next could EVER be in dd->dispatch instead
of dd->fifo_list?  I've tried to verify it, but, although I think it has =
never
happened in my tests, I was not able to make sure that no unlucky
combination may ever happen (considering also the use of
blk_rq_is_passthrough too, to decide where to put a new request).

I'm making a blunder, right?

Thanks,
Paolo

^ permalink raw reply

* [PATCH 00/17] md: cleanup on direct access to bvec table
From: Ming Lei @ 2017-02-16 11:45 UTC (permalink / raw)
  To: Shaohua Li, Jens Axboe, linux-kernel, linux-raid, linux-block,
	Christoph Hellwig, NeilBrown
  Cc: Ming Lei

In MD's resync I/O path, there are lots of direct access to bio's
bvec table. This patchset kills most of them, and the conversion
is quite straightforward.

Once direct access to bvec table in MD is cleaned up, we may make
multipage bvec moving on.

Thanks,
Ming

Ming Lei (17):
  block: introduce bio_segments_all()
  block: introduce bio_remove_last_page()
  md: raid1/raid10: use bio_remove_last_page()
  md: introduce helpers for dealing with fetch/store preallocated pages
    in bio
  md: raid1/raid10: use the introduced helpers
  md: raid1/raid10: borrow .bi_error as pre-allocated page index
  md: raid1/raid10: don't use .bi_vcnt to check if all pages are added
  md: raid1: simplify r1buf_pool_free()
  md: raid1/raid10: use bio helper in *_pool_free
  md: raid1: remove direct access to bvec table in fix_sync_read_error
  md: raid1: use bio helper in process_checks()
  md: raid1: avoid direct access to bvec table in process_checks()
  md: raid1: use bio_segments_all()
  md: raid10: avoid direct access to bvec table in sync_request_write()
  md: raid10: avoid direct access to bvec table in
    fix_recovery_read_error
  md: raid10: avoid direct access to bvec table in reshape_request
  md: raid10: avoid direct access to bvec table in
    handle_reshape_read_error

 block/bio.c         | 23 ++++++++++++++
 drivers/md/md.h     | 21 +++++++++++++
 drivers/md/raid1.c  | 87 +++++++++++++++++++++++++++++++++++++----------------
 drivers/md/raid10.c | 76 ++++++++++++++++++++++++++++++++++------------
 include/linux/bio.h |  8 +++++
 5 files changed, 169 insertions(+), 46 deletions(-)

-- 
2.7.4

^ permalink raw reply

* [PATCH 01/17] block: introduce bio_segments_all()
From: Ming Lei @ 2017-02-16 11:45 UTC (permalink / raw)
  To: Shaohua Li, Jens Axboe, linux-kernel, linux-raid, linux-block,
	Christoph Hellwig, NeilBrown
  Cc: Ming Lei
In-Reply-To: <1487245547-24384-1-git-send-email-tom.leiming@gmail.com>

So that we can replace the direct access to .bi_vcnt.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 include/linux/bio.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/linux/bio.h b/include/linux/bio.h
index 8e521194f6fc..3364b3ed90e7 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -293,6 +293,13 @@ static inline void bio_get_last_bvec(struct bio *bio, struct bio_vec *bv)
 		bv->bv_len = iter.bi_bvec_done;
 }
 
+static inline unsigned bio_segments_all(struct bio *bio)
+{
+	WARN_ON_ONCE(bio_flagged(bio, BIO_CLONED));
+
+	return bio->bi_vcnt;
+}
+
 enum bip_flags {
 	BIP_BLOCK_INTEGRITY	= 1 << 0, /* block layer owns integrity data */
 	BIP_MAPPED_INTEGRITY	= 1 << 1, /* ref tag has been remapped */
-- 
2.7.4

^ permalink raw reply related

* [PATCH 02/17] block: introduce bio_remove_last_page()
From: Ming Lei @ 2017-02-16 11:45 UTC (permalink / raw)
  To: Shaohua Li, Jens Axboe, linux-kernel, linux-raid, linux-block,
	Christoph Hellwig, NeilBrown
  Cc: Ming Lei
In-Reply-To: <1487245547-24384-1-git-send-email-tom.leiming@gmail.com>

MD need this helper to remove the last added page, so introduce
it.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 block/bio.c         | 23 +++++++++++++++++++++++
 include/linux/bio.h |  1 +
 2 files changed, 24 insertions(+)

diff --git a/block/bio.c b/block/bio.c
index 5eec5e08417f..0ce7ffcd7939 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -837,6 +837,29 @@ int bio_add_pc_page(struct request_queue *q, struct bio *bio, struct page
 EXPORT_SYMBOL(bio_add_pc_page);
 
 /**
+ *	bio_remove_last_page	-	remove the last added page
+ *	@bio: destination bio
+ *
+ *	Attempt to remove the last added page from the bio_vec maplist.
+ */
+void bio_remove_last_page(struct bio *bio)
+{
+	/*
+	 * cloned bio must not modify vec list
+	 */
+	if (WARN_ON_ONCE(bio_flagged(bio, BIO_CLONED)))
+		return;
+
+	if (bio->bi_vcnt > 0) {
+		struct bio_vec *bv = &bio->bi_io_vec[bio->bi_vcnt - 1];
+
+		bio->bi_iter.bi_size -= bv->bv_len;
+		bio->bi_vcnt--;
+	}
+}
+EXPORT_SYMBOL(bio_remove_last_page);
+
+/**
  *	bio_add_page	-	attempt to add page to bio
  *	@bio: destination bio
  *	@page: page to add
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 3364b3ed90e7..32aeb493d1fe 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -443,6 +443,7 @@ extern void bio_init(struct bio *bio, struct bio_vec *table,
 extern void bio_reset(struct bio *);
 void bio_chain(struct bio *, struct bio *);
 
+extern void bio_remove_last_page(struct bio *bio);
 extern int bio_add_page(struct bio *, struct page *, unsigned int,unsigned int);
 extern int bio_add_pc_page(struct request_queue *, struct bio *, struct page *,
 			   unsigned int, unsigned int);
-- 
2.7.4

^ permalink raw reply related

* [PATCH 03/17] md: raid1/raid10: use bio_remove_last_page()
From: Ming Lei @ 2017-02-16 11:45 UTC (permalink / raw)
  To: Shaohua Li, Jens Axboe, linux-kernel, linux-raid, linux-block,
	Christoph Hellwig, NeilBrown
  Cc: Ming Lei
In-Reply-To: <1487245547-24384-1-git-send-email-tom.leiming@gmail.com>

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/md/raid1.c  | 3 +--
 drivers/md/raid10.c | 6 ++----
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 85f309836fd7..6e4e0b868ff2 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2824,8 +2824,7 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr,
 						if (bio->bi_end_io==NULL)
 							continue;
 						/* remove last page from this bio */
-						bio->bi_vcnt--;
-						bio->bi_iter.bi_size -= len;
+						bio_remove_last_page(bio);
 						bio_clear_flag(bio, BIO_SEG_VALID);
 					}
 					goto bio_full;
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 063c43d83b72..aa37d4c7900a 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3447,8 +3447,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 			     bio2 && bio2 != bio;
 			     bio2 = bio2->bi_next) {
 				/* remove last page from this bio */
-				bio2->bi_vcnt--;
-				bio2->bi_iter.bi_size -= len;
+				bio_remove_last_page(bio2);
 				bio_clear_flag(bio2, BIO_SEG_VALID);
 			}
 			goto bio_full;
@@ -4538,8 +4537,7 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr,
 			     bio2 && bio2 != bio;
 			     bio2 = bio2->bi_next) {
 				/* Remove last page from this bio */
-				bio2->bi_vcnt--;
-				bio2->bi_iter.bi_size -= len;
+				bio_remove_last_page(bio2);
 				bio_clear_flag(bio2, BIO_SEG_VALID);
 			}
 			goto bio_full;
-- 
2.7.4

^ permalink raw reply related

* [PATCH 05/17] md: raid1/raid10: use the introduced helpers
From: Ming Lei @ 2017-02-16 11:45 UTC (permalink / raw)
  To: Shaohua Li, Jens Axboe, linux-kernel, linux-raid, linux-block,
	Christoph Hellwig, NeilBrown
  Cc: Ming Lei
In-Reply-To: <1487245547-24384-1-git-send-email-tom.leiming@gmail.com>

This patch uses the introduced helpers to fetch pre-allocated
page from bio bvec table, and store it back.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/md/raid1.c  | 4 ++--
 drivers/md/raid10.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 6e4e0b868ff2..c4791fbd69ac 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2814,10 +2814,10 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr,
 		for (i = 0 ; i < conf->raid_disks * 2; i++) {
 			bio = r1_bio->bios[i];
 			if (bio->bi_end_io) {
-				page = bio->bi_io_vec[bio->bi_vcnt].bv_page;
+				page = mdev_get_page_from_bio(bio, bio->bi_vcnt);
 				if (bio_add_page(bio, page, len, 0) == 0) {
 					/* stop here */
-					bio->bi_io_vec[bio->bi_vcnt].bv_page = page;
+					mdev_put_page_to_bio(bio, bio->bi_vcnt, page);
 					while (i > 0) {
 						i--;
 						bio = r1_bio->bios[i];
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index aa37d4c7900a..b7dfbca869a3 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3437,12 +3437,12 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 			break;
 		for (bio= biolist ; bio ; bio=bio->bi_next) {
 			struct bio *bio2;
-			page = bio->bi_io_vec[bio->bi_vcnt].bv_page;
+			page = mdev_get_page_from_bio(bio, bio->bi_vcnt);
 			if (bio_add_page(bio, page, len, 0))
 				continue;
 
 			/* stop here */
-			bio->bi_io_vec[bio->bi_vcnt].bv_page = page;
+			mdev_put_page_to_bio(bio, bio->bi_vcnt, page);
 			for (bio2 = biolist;
 			     bio2 && bio2 != bio;
 			     bio2 = bio2->bi_next) {
-- 
2.7.4

^ permalink raw reply related

* [PATCH 04/17] md: introduce helpers for dealing with fetch/store preallocated pages in bio
From: Ming Lei @ 2017-02-16 11:45 UTC (permalink / raw)
  To: Shaohua Li, Jens Axboe, linux-kernel, linux-raid, linux-block,
	Christoph Hellwig, NeilBrown
  Cc: Ming Lei
In-Reply-To: <1487245547-24384-1-git-send-email-tom.leiming@gmail.com>

Both raid1 and raid10 uses bio's bvec table to store pre-allocated
pages, then fetch and add it to bio.

This patch introduces two helpers for dealing with the special case,
like what bio_iov_iter_get_pages() does.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/md/md.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/md/md.h b/drivers/md/md.h
index a86ad62079de..21897cb514af 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -708,4 +708,25 @@ static inline void mddev_clear_unsupported_flags(struct mddev *mddev,
 {
 	mddev->flags &= ~unsupported_flags;
 }
+
+/*
+ * Both raid1 and raid10 use bio's bvec table to store the preallocated
+ * pages, so we introduces the following helpers for this kind of usage.
+ *
+ * Actually the usage is a bit similar with bio_iov_iter_get_pages().
+ *
+ * Please make sure .bi_io_vec[idx] points to one unused vector by the
+ * bio.
+ */
+static inline struct page *mdev_get_page_from_bio(struct bio *bio, unsigned idx)
+{
+	return bio->bi_io_vec[idx].bv_page;
+}
+
+static inline void mdev_put_page_to_bio(struct bio *bio, unsigned idx,
+					struct page *page)
+{
+	bio->bi_io_vec[idx].bv_page = page;
+}
+
 #endif /* _MD_MD_H */
-- 
2.7.4

^ permalink raw reply related

* [PATCH 07/17] md: raid1/raid10: don't use .bi_vcnt to check if all pages are added
From: Ming Lei @ 2017-02-16 11:45 UTC (permalink / raw)
  To: Shaohua Li, Jens Axboe, linux-kernel, linux-raid, linux-block,
	Christoph Hellwig, NeilBrown
  Cc: Ming Lei
In-Reply-To: <1487245547-24384-1-git-send-email-tom.leiming@gmail.com>

Instead we use the index of the pre-allocated pages buffer, it should
be more explicit because the index(.bi_error) just means how many pages
are added to the bio.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/md/raid1.c  | 2 +-
 drivers/md/raid10.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 8904a9149671..23a3a678a9ed 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2835,7 +2835,7 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr,
 		nr_sectors += len>>9;
 		sector_nr += len>>9;
 		sync_blocks -= (len>>9);
-	} while (r1_bio->bios[disk]->bi_vcnt < RESYNC_PAGES);
+	} while (r1_bio->bios[disk]->bi_error < RESYNC_PAGES);
  bio_full:
 	/* return .bi_error back to bio */
 	for (i = 0 ; i < conf->raid_disks * 2; i++) {
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 9cfc22cd1330..8262f3e1dd93 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3453,7 +3453,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 		}
 		nr_sectors += len>>9;
 		sector_nr += len>>9;
-	} while (biolist->bi_vcnt < RESYNC_PAGES);
+	} while (biolist->bi_error < RESYNC_PAGES);
  bio_full:
 	/* return .bi_error back to bio, and set resync's as -EIO */
 	for (bio= biolist ; bio ; bio=bio->bi_next)
-- 
2.7.4

^ permalink raw reply related

* [PATCH 06/17] md: raid1/raid10: borrow .bi_error as pre-allocated page index
From: Ming Lei @ 2017-02-16 11:45 UTC (permalink / raw)
  To: Shaohua Li, Jens Axboe, linux-kernel, linux-raid, linux-block,
	Christoph Hellwig, NeilBrown
  Cc: Ming Lei
In-Reply-To: <1487245547-24384-1-git-send-email-tom.leiming@gmail.com>

Before bio is submitted, it is safe to borrow .bi_error. This
patch uses .bi_error as index of pre-allocated page in bio, so
that we can avoid to mess .bi_vcnt. Especially the old way
will not work any more when multipage bvec is introduced.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/md/raid1.c  | 12 ++++++++++--
 drivers/md/raid10.c | 14 ++++++++++----
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index c4791fbd69ac..8904a9149671 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2811,13 +2811,14 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr,
 				len = sync_blocks<<9;
 		}
 
+		/* borrow .bi_error as pre-allocated page index */
 		for (i = 0 ; i < conf->raid_disks * 2; i++) {
 			bio = r1_bio->bios[i];
 			if (bio->bi_end_io) {
-				page = mdev_get_page_from_bio(bio, bio->bi_vcnt);
+				page = mdev_get_page_from_bio(bio, bio->bi_error++);
 				if (bio_add_page(bio, page, len, 0) == 0) {
 					/* stop here */
-					mdev_put_page_to_bio(bio, bio->bi_vcnt, page);
+					mdev_put_page_to_bio(bio, --bio->bi_error, page);
 					while (i > 0) {
 						i--;
 						bio = r1_bio->bios[i];
@@ -2836,6 +2837,13 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr,
 		sync_blocks -= (len>>9);
 	} while (r1_bio->bios[disk]->bi_vcnt < RESYNC_PAGES);
  bio_full:
+	/* return .bi_error back to bio */
+	for (i = 0 ; i < conf->raid_disks * 2; i++) {
+		bio = r1_bio->bios[i];
+		if (bio->bi_end_io)
+			bio->bi_error = 0;
+	}
+
 	r1_bio->sectors = nr_sectors;
 
 	if (mddev_is_clustered(mddev) &&
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index b7dfbca869a3..9cfc22cd1330 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3348,7 +3348,6 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 
 			bio = r10_bio->devs[i].bio;
 			bio_reset(bio);
-			bio->bi_error = -EIO;
 			rcu_read_lock();
 			rdev = rcu_dereference(conf->mirrors[d].rdev);
 			if (rdev == NULL || test_bit(Faulty, &rdev->flags)) {
@@ -3392,7 +3391,6 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 			/* Need to set up for writing to the replacement */
 			bio = r10_bio->devs[i].repl_bio;
 			bio_reset(bio);
-			bio->bi_error = -EIO;
 
 			sector = r10_bio->devs[i].addr;
 			bio->bi_next = biolist;
@@ -3435,14 +3433,15 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 			len = (max_sector - sector_nr) << 9;
 		if (len == 0)
 			break;
+		/* borrow .bi_error as pre-allocated page index */
 		for (bio= biolist ; bio ; bio=bio->bi_next) {
 			struct bio *bio2;
-			page = mdev_get_page_from_bio(bio, bio->bi_vcnt);
+			page = mdev_get_page_from_bio(bio, bio->bi_error++);
 			if (bio_add_page(bio, page, len, 0))
 				continue;
 
 			/* stop here */
-			mdev_put_page_to_bio(bio, bio->bi_vcnt, page);
+			mdev_put_page_to_bio(bio, --bio->bi_error, page);
 			for (bio2 = biolist;
 			     bio2 && bio2 != bio;
 			     bio2 = bio2->bi_next) {
@@ -3456,6 +3455,13 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 		sector_nr += len>>9;
 	} while (biolist->bi_vcnt < RESYNC_PAGES);
  bio_full:
+	/* return .bi_error back to bio, and set resync's as -EIO */
+	for (bio= biolist ; bio ; bio=bio->bi_next)
+		if (test_bit(MD_RECOVERY_SYNC, &mddev->recovery))
+			bio->bi_error = -EIO;
+		else
+			bio->bi_error = 0;
+
 	r10_bio->sectors = nr_sectors;
 
 	while (biolist) {
-- 
2.7.4

^ permalink raw reply related

* [PATCH 08/17] md: raid1: simplify r1buf_pool_free()
From: Ming Lei @ 2017-02-16 11:45 UTC (permalink / raw)
  To: Shaohua Li, Jens Axboe, linux-kernel, linux-raid, linux-block,
	Christoph Hellwig, NeilBrown
  Cc: Ming Lei
In-Reply-To: <1487245547-24384-1-git-send-email-tom.leiming@gmail.com>

This patch gets each page's reference of each bio for resync,
then r1buf_pool_free() gets simplified.

The same policy has been taken in raid10's buf pool allocation/free
too.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/md/raid1.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 23a3a678a9ed..ebea8615344a 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -143,9 +143,12 @@ static void * r1buf_pool_alloc(gfp_t gfp_flags, void *data)
 	/* If not user-requests, copy the page pointers to all bios */
 	if (!test_bit(MD_RECOVERY_REQUESTED, &pi->mddev->recovery)) {
 		for (i=0; i<RESYNC_PAGES ; i++)
-			for (j=1; j<pi->raid_disks; j++)
-				r1_bio->bios[j]->bi_io_vec[i].bv_page =
+			for (j=1; j<pi->raid_disks; j++) {
+				struct page *page =
 					r1_bio->bios[0]->bi_io_vec[i].bv_page;
+				get_page(page);
+				r1_bio->bios[j]->bi_io_vec[i].bv_page = page;
+			}
 	}
 
 	r1_bio->master_bio = NULL;
@@ -170,12 +173,8 @@ static void r1buf_pool_free(void *__r1_bio, void *data)
 	struct r1bio *r1bio = __r1_bio;
 
 	for (i = 0; i < RESYNC_PAGES; i++)
-		for (j = pi->raid_disks; j-- ;) {
-			if (j == 0 ||
-			    r1bio->bios[j]->bi_io_vec[i].bv_page !=
-			    r1bio->bios[0]->bi_io_vec[i].bv_page)
-				safe_put_page(r1bio->bios[j]->bi_io_vec[i].bv_page);
-		}
+		for (j = pi->raid_disks; j-- ;)
+			safe_put_page(r1bio->bios[j]->bi_io_vec[i].bv_page);
 	for (i=0 ; i < pi->raid_disks; i++)
 		bio_put(r1bio->bios[i]);
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH 09/17] md: raid1/raid10: use bio helper in *_pool_free
From: Ming Lei @ 2017-02-16 11:45 UTC (permalink / raw)
  To: Shaohua Li, Jens Axboe, linux-kernel, linux-raid, linux-block,
	Christoph Hellwig, NeilBrown
  Cc: Ming Lei
In-Reply-To: <1487245547-24384-1-git-send-email-tom.leiming@gmail.com>

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/md/raid1.c  | 13 ++++++++++---
 drivers/md/raid10.c | 11 +++++++----
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index ebea8615344a..1dd6b2760fba 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -172,9 +172,16 @@ static void r1buf_pool_free(void *__r1_bio, void *data)
 	int i,j;
 	struct r1bio *r1bio = __r1_bio;
 
-	for (i = 0; i < RESYNC_PAGES; i++)
-		for (j = pi->raid_disks; j-- ;)
-			safe_put_page(r1bio->bios[j]->bi_io_vec[i].bv_page);
+	for (i = 0; i < pi->raid_disks; i++) {
+		struct bio_vec *bvl;
+		struct bio *bio = r1bio->bios[i];
+
+		/* make sure all pages can be freed */
+		bio->bi_vcnt = RESYNC_PAGES;
+
+		bio_for_each_segment_all(bvl, bio, j)
+			safe_put_page(bvl->bv_page);
+	}
 	for (i=0 ; i < pi->raid_disks; i++)
 		bio_put(r1bio->bios[i]);
 
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 8262f3e1dd93..5c698f3d3083 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -230,10 +230,13 @@ static void r10buf_pool_free(void *__r10_bio, void *data)
 	for (j=0; j < conf->copies; j++) {
 		struct bio *bio = r10bio->devs[j].bio;
 		if (bio) {
-			for (i = 0; i < RESYNC_PAGES; i++) {
-				safe_put_page(bio->bi_io_vec[i].bv_page);
-				bio->bi_io_vec[i].bv_page = NULL;
-			}
+			struct bio_vec *bvl;
+
+			/* make sure all pages can be freed */
+			bio->bi_vcnt = RESYNC_PAGES;
+
+			bio_for_each_segment_all(bvl, bio, i)
+				safe_put_page(bvl->bv_page);
 			bio_put(bio);
 		}
 		bio = r10bio->devs[j].repl_bio;
-- 
2.7.4

^ permalink raw reply related

* [PATCH 13/17] md: raid1: use bio_segments_all()
From: Ming Lei @ 2017-02-16 11:45 UTC (permalink / raw)
  To: Shaohua Li, Jens Axboe, linux-kernel, linux-raid, linux-block,
	Christoph Hellwig, NeilBrown
  Cc: Ming Lei
In-Reply-To: <1487245547-24384-1-git-send-email-tom.leiming@gmail.com>

Use this helper, instead of direct access to .bi_vcnt.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/md/raid1.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 54ec32be3277..02cfece74981 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -998,7 +998,8 @@ static void alloc_behind_pages(struct bio *bio, struct r1bio *r1_bio)
 {
 	int i;
 	struct bio_vec *bvec;
-	struct bio_vec *bvecs = kzalloc(bio->bi_vcnt * sizeof(struct bio_vec),
+	unsigned vcnt = bio_segments_all(bio);
+	struct bio_vec *bvecs = kzalloc(vcnt * sizeof(struct bio_vec),
 					GFP_NOIO);
 	if (unlikely(!bvecs))
 		return;
@@ -1014,12 +1015,12 @@ static void alloc_behind_pages(struct bio *bio, struct r1bio *r1_bio)
 		kunmap(bvec->bv_page);
 	}
 	r1_bio->behind_bvecs = bvecs;
-	r1_bio->behind_page_count = bio->bi_vcnt;
+	r1_bio->behind_page_count = vcnt;
 	set_bit(R1BIO_BehindIO, &r1_bio->state);
 	return;
 
 do_sync_io:
-	for (i = 0; i < bio->bi_vcnt; i++)
+	for (i = 0; i < vcnt; i++)
 		if (bvecs[i].bv_page)
 			put_page(bvecs[i].bv_page);
 	kfree(bvecs);
-- 
2.7.4

^ permalink raw reply related

* [PATCH 12/17] md: raid1: avoid direct access to bvec table in process_checks()
From: Ming Lei @ 2017-02-16 11:45 UTC (permalink / raw)
  To: Shaohua Li, Jens Axboe, linux-kernel, linux-raid, linux-block,
	Christoph Hellwig, NeilBrown
  Cc: Ming Lei
In-Reply-To: <1487245547-24384-1-git-send-email-tom.leiming@gmail.com>

Given process_checks is only called in resync path, it should be
ok to allocate three stack variable(total 320byteds) to store
pages from bios.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/md/raid1.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 4400fbe7ce8c..54ec32be3277 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2010,6 +2010,9 @@ static void process_checks(struct r1bio *r1_bio)
 	int primary;
 	int i;
 	int vcnt;
+	struct bio_vec *bi;
+	int page_len[RESYNC_PAGES];
+	struct page *pbio_pages[RESYNC_PAGES], *sbio_pages[RESYNC_PAGES];
 
 	/* Fix variable parts of all bios */
 	vcnt = (r1_bio->sectors + PAGE_SIZE / 512 - 1) >> (PAGE_SHIFT - 9);
@@ -2017,7 +2020,6 @@ static void process_checks(struct r1bio *r1_bio)
 		int j;
 		int size;
 		int error;
-		struct bio_vec *bi;
 		struct bio *b = r1_bio->bios[i];
 		if (b->bi_end_io != end_sync_read)
 			continue;
@@ -2051,6 +2053,11 @@ static void process_checks(struct r1bio *r1_bio)
 			break;
 		}
 	r1_bio->read_disk = primary;
+
+	/* .bi_vcnt has been set for all read bios */
+	bio_for_each_segment_all(bi, r1_bio->bios[primary], i)
+		pbio_pages[i] = bi->bv_page;
+
 	for (i = 0; i < conf->raid_disks * 2; i++) {
 		int j;
 		struct bio *pbio = r1_bio->bios[primary];
@@ -2062,14 +2069,19 @@ static void process_checks(struct r1bio *r1_bio)
 		/* Now we can 'fixup' the error value */
 		sbio->bi_error = 0;
 
+		bio_for_each_segment_all(bi, sbio, j) {
+			sbio_pages[j] = bi->bv_page;
+			page_len[j] = bi->bv_len;
+		}
+
 		if (!error) {
 			for (j = vcnt; j-- ; ) {
 				struct page *p, *s;
-				p = pbio->bi_io_vec[j].bv_page;
-				s = sbio->bi_io_vec[j].bv_page;
+				p = pbio_pages[j];
+				s = sbio_pages[j];
 				if (memcmp(page_address(p),
 					   page_address(s),
-					   sbio->bi_io_vec[j].bv_len))
+					   page_len[j]))
 					break;
 			}
 		} else
-- 
2.7.4

^ permalink raw reply related

* [PATCH 11/17] md: raid1: use bio helper in process_checks()
From: Ming Lei @ 2017-02-16 11:45 UTC (permalink / raw)
  To: Shaohua Li, Jens Axboe, linux-kernel, linux-raid, linux-block,
	Christoph Hellwig, NeilBrown
  Cc: Ming Lei
In-Reply-To: <1487245547-24384-1-git-send-email-tom.leiming@gmail.com>

Avoid to direct access to bvec table.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/md/raid1.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 02ee8542295d..4400fbe7ce8c 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2017,6 +2017,7 @@ static void process_checks(struct r1bio *r1_bio)
 		int j;
 		int size;
 		int error;
+		struct bio_vec *bi;
 		struct bio *b = r1_bio->bios[i];
 		if (b->bi_end_io != end_sync_read)
 			continue;
@@ -2033,9 +2034,7 @@ static void process_checks(struct r1bio *r1_bio)
 		b->bi_private = r1_bio;
 
 		size = b->bi_iter.bi_size;
-		for (j = 0; j < vcnt ; j++) {
-			struct bio_vec *bi;
-			bi = &b->bi_io_vec[j];
+		bio_for_each_segment_all(bi, b, j) {
 			bi->bv_offset = 0;
 			if (size > PAGE_SIZE)
 				bi->bv_len = PAGE_SIZE;
-- 
2.7.4

^ permalink raw reply related

* [PATCH 14/17] md: raid10: avoid direct access to bvec table in sync_request_write()
From: Ming Lei @ 2017-02-16 11:45 UTC (permalink / raw)
  To: Shaohua Li, Jens Axboe, linux-kernel, linux-raid, linux-block,
	Christoph Hellwig, NeilBrown
  Cc: Ming Lei
In-Reply-To: <1487245547-24384-1-git-send-email-tom.leiming@gmail.com>

The cost is 256bytes(8*16*2) stack space, and just use the bio
helper to retrieve pages from bio.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/md/raid10.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 5c698f3d3083..69fe2a3cef89 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -2036,6 +2036,8 @@ static void sync_request_write(struct mddev *mddev, struct r10bio *r10_bio)
 	int i, first;
 	struct bio *tbio, *fbio;
 	int vcnt;
+	struct bio_vec *bvl;
+	struct page *fbio_pages[RESYNC_PAGES], *tbio_pages[RESYNC_PAGES];
 
 	atomic_set(&r10_bio->remaining, 1);
 
@@ -2052,6 +2054,10 @@ static void sync_request_write(struct mddev *mddev, struct r10bio *r10_bio)
 	fbio->bi_iter.bi_size = r10_bio->sectors << 9;
 	fbio->bi_iter.bi_idx = 0;
 
+	/* the bio has been filled up in raid10_sync_request */
+	bio_for_each_segment_all(bvl, fbio, i)
+		fbio_pages[i] = bvl->bv_page;
+
 	vcnt = (r10_bio->sectors + (PAGE_SIZE >> 9) - 1) >> (PAGE_SHIFT - 9);
 	/* now find blocks with errors */
 	for (i=0 ; i < conf->copies ; i++) {
@@ -2072,12 +2078,17 @@ static void sync_request_write(struct mddev *mddev, struct r10bio *r10_bio)
 			 * All vec entries are PAGE_SIZE;
 			 */
 			int sectors = r10_bio->sectors;
+
+			/* the bio has been filled up in raid10_sync_request */
+			bio_for_each_segment_all(bvl, tbio, j)
+				tbio_pages[j] = bvl->bv_page;
+
 			for (j = 0; j < vcnt; j++) {
 				int len = PAGE_SIZE;
 				if (sectors < (len / 512))
 					len = sectors * 512;
-				if (memcmp(page_address(fbio->bi_io_vec[j].bv_page),
-					   page_address(tbio->bi_io_vec[j].bv_page),
+				if (memcmp(page_address(fbio_pages[j]),
+					   page_address(tbio_pages[j]),
 					   len))
 					break;
 				sectors -= len/512;
-- 
2.7.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox