Linux RAID subsystem development
 help / color / mirror / Atom feed
* Re: [PATCH v2 15/21] xen-blkfront: Make use of the new sg_map helper function
From: Logan Gunthorpe @ 2017-04-27 20:19 UTC (permalink / raw)
  To: Roger Pau Monné
  Cc: Boris Ostrovsky, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
	target-devel-u79uwXL29TY76Z2rM5mHXA, Christoph Hellwig,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b, James E.J. Bottomley,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, Sumit Semwal,
	open-iscsi-/JYPxA39Uh5TLH3MbocFFw,
	linux-media-u79uwXL29TY76Z2rM5mHXA, Juergen Gross, Julien Grall,
	intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	sparmaintainer-GLv8BlqOqDDQT0dZR+AlfA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	megaraidlinux.pdl-dY08KVG/lbpWk0Htik3J/w, Jens Axboe,
	Martin K. Petersen, netdev-u79uwXL29TY76Z2rM5mHXA, Matthew Wilcox,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman
In-Reply-To: <20170426073720.okv33ly2ldepilti-aUbyMND+kyB2Oba8jWPag5QscXo+jHNAQQ4Iyu8u01E@public.gmane.org>



On 26/04/17 01:37 AM, Roger Pau Monné wrote:
> On Tue, Apr 25, 2017 at 12:21:02PM -0600, Logan Gunthorpe wrote:
>> Straightforward conversion to the new helper, except due to the lack
>> of error path, we have to use SG_MAP_MUST_NOT_FAIL which may BUG_ON in
>> certain cases in the future.
>>
>> Signed-off-by: Logan Gunthorpe <logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org>
>> Cc: Boris Ostrovsky <boris.ostrovsky-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
>> Cc: Juergen Gross <jgross-IBi9RG/b67k@public.gmane.org>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
>> Cc: "Roger Pau Monné" <roger.pau-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org>
>> ---
>>  drivers/block/xen-blkfront.c | 20 +++++++++++---------
>>  1 file changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
>> index 3945963..ed62175 100644
>> --- a/drivers/block/xen-blkfront.c
>> +++ b/drivers/block/xen-blkfront.c
>> @@ -816,8 +816,9 @@ static int blkif_queue_rw_req(struct request *req, struct blkfront_ring_info *ri
>>  		BUG_ON(sg->offset + sg->length > PAGE_SIZE);
>>  
>>  		if (setup.need_copy) {
>> -			setup.bvec_off = sg->offset;
>> -			setup.bvec_data = kmap_atomic(sg_page(sg));
>> +			setup.bvec_off = 0;
>> +			setup.bvec_data = sg_map(sg, 0, SG_KMAP_ATOMIC |
>> +						 SG_MAP_MUST_NOT_FAIL);
> 
> I assume that sg_map already adds sg->offset to the address?

Correct.

> Also wondering whether we can get rid of bvec_off and just increment bvec_data,
> adding Julien who IIRC added this code.

bvec_off is used to keep track of the offset within the current mapping
so it's not a great idea given that you'd want to kunmap_atomic the
original address and not something with an offset. It would be nice if
this could be converted to use the sg_miter interface but that's a much
more invasive change that would require someone who knows this code and
can properly test it. I'd be very grateful if someone actually took that on.

Logan

^ permalink raw reply

* Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions
From: Logan Gunthorpe @ 2017-04-27 20:13 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
	target-devel-u79uwXL29TY76Z2rM5mHXA, Sumit Semwal,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b, James E.J. Bottomley,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	open-iscsi-/JYPxA39Uh5TLH3MbocFFw,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	sparmaintainer-GLv8BlqOqDDQT0dZR+AlfA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	megaraidlinux.pdl-dY08KVG/lbpWk0Htik3J/w, Jens Axboe,
	Martin K. Petersen, netdev-u79uwXL29TY76Z2rM5mHXA, Matthew Wilcox,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman
In-Reply-To: <20170426074416.GA7936-jcswGhMUV9g@public.gmane.org>


On 26/04/17 01:44 AM, Christoph Hellwig wrote:
> I think we'll at least need a draft of those to make sense of these
> patches.  Otherwise they just look very clumsy.

Ok, what follows is a draft patch attempting to show where I'm thinking
of going with this. Obviously it will not compile because it assumes
the users throughout the kernel are a bit different than they are today.
Notably, there is no sg_page anymore.

There's also likely a ton of issues and arguments to have over a bunch
of the specifics below and I'd expect the concept to evolve more
as cleanup occurs. This itself is an evolution of the draft I posted
replying to you in my last RFC thread.

Also, before any of this is truly useful to us, pfn_t would have to
infect a few other places in the kernel.

Thanks,

Logan


diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index fad170b..85ef928 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -6,13 +6,14 @@
 #include <linux/bug.h>
 #include <linux/mm.h>
 #include <linux/highmem.h>
+#include <linux/pfn_t.h>
 #include <asm/io.h>

 struct scatterlist {
 #ifdef CONFIG_DEBUG_SG
 	unsigned long	sg_magic;
 #endif
-	unsigned long	page_link;
+	pfn_t  		pfn;
 	unsigned int	offset;
 	unsigned int	length;
 	dma_addr_t	dma_address;
@@ -60,15 +61,68 @@ struct sg_table {

 #define SG_MAGIC	0x87654321

-/*
- * We overload the LSB of the page pointer to indicate whether it's
- * a valid sg entry, or whether it points to the start of a new
scatterlist.
- * Those low bits are there for everyone! (thanks mason :-)
- */
-#define sg_is_chain(sg)		((sg)->page_link & 0x01)
-#define sg_is_last(sg)		((sg)->page_link & 0x02)
-#define sg_chain_ptr(sg)	\
-	((struct scatterlist *) ((sg)->page_link & ~0x03))
+static inline bool sg_is_chain(struct scatterlist *sg)
+{
+	return sg->pfn.val & PFN_SG_CHAIN;
+}
+
+static inline bool sg_is_last(struct scatterlist *sg)
+{
+	return sg->pfn.val & PFN_SG_LAST;
+}
+
+static inline struct scatterlist *sg_chain_ptr(struct scatterlist *sg)
+{
+	unsigned long sgl = pfn_t_to_pfn(sg->pfn);
+	return (struct scatterlist *)(sgl << PAGE_SHIFT);
+}
+
+static inline bool sg_is_iomem(struct scatterlist *sg)
+{
+	return pfn_t_is_iomem(sg->pfn);
+}
+
+/**
+ * sg_assign_pfn - Assign a given pfn_t to an SG entry
+ * @sg:		    SG entry
+ * @pfn:	    The pfn
+ *
+ * Description:
+ *   Assign a pfn to sg entry. Also see sg_set_pfn(), the most commonly
used
+ *   variant.w
+ *
+ **/
+static inline void sg_assign_pfn(struct scatterlist *sg, pfn_t pfn)
+{
+#ifdef CONFIG_DEBUG_SG
+	BUG_ON(sg->sg_magic != SG_MAGIC);
+	BUG_ON(sg_is_chain(sg));
+	BUG_ON(pfn.val & (PFN_SG_CHAIN | PFN_SG_LAST));
+#endif
+
+	sg->pfn = pfn;
+}
+
+/**
+ * sg_set_pfn - Set sg entry to point at given pfn
+ * @sg:		 SG entry
+ * @pfn:	 The page
+ * @len:	 Length of data
+ * @offset:	 Offset into page
+ *
+ * Description:
+ *   Use this function to set an sg entry pointing at a pfn, never assign
+ *   the page directly. We encode sg table information in the lower bits
+ *   of the page pointer. See sg_pfn_t for looking up the pfn_t belonging
+ *   to an sg entry.
+ **/
+static inline void sg_set_pfn(struct scatterlist *sg, pfn_t pfn,
+			      unsigned int len, unsigned int offset)
+{
+	sg_assign_pfn(sg, pfn);
+	sg->offset = offset;
+	sg->length = len;
+}

 /**
  * sg_assign_page - Assign a given page to an SG entry
@@ -82,18 +136,13 @@ struct sg_table {
  **/
 static inline void sg_assign_page(struct scatterlist *sg, struct page
*page)
 {
-	unsigned long page_link = sg->page_link & 0x3;
+	if (!page) {
+		pfn_t null_pfn = {0};
+		sg_assign_pfn(sg, null_pfn);
+		return;
+	}

-	/*
-	 * In order for the low bit stealing approach to work, pages
-	 * must be aligned at a 32-bit boundary as a minimum.
-	 */
-	BUG_ON((unsigned long) page & 0x03);
-#ifdef CONFIG_DEBUG_SG
-	BUG_ON(sg->sg_magic != SG_MAGIC);
-	BUG_ON(sg_is_chain(sg));
-#endif
-	sg->page_link = page_link | (unsigned long) page;
+	sg_assign_pfn(sg, page_to_pfn_t(page));
 }

 /**
@@ -106,8 +155,7 @@ static inline void sg_assign_page(struct scatterlist
*sg, struct page *page)
  * Description:
  *   Use this function to set an sg entry pointing at a page, never assign
  *   the page directly. We encode sg table information in the lower bits
- *   of the page pointer. See sg_page() for looking up the page belonging
- *   to an sg entry.
+ *   of the page pointer.
  *
  **/
 static inline void sg_set_page(struct scatterlist *sg, struct page *page,
@@ -118,13 +166,53 @@ static inline void sg_set_page(struct scatterlist
*sg, struct page *page,
 	sg->length = len;
 }

-static inline struct page *sg_page(struct scatterlist *sg)
+/**
+ * sg_pfn_t - Return the pfn_t for the sg
+ * @sg:		 SG entry
+ *
+ **/
+static inline pfn_t sg_pfn_t(struct scatterlist *sg)
 {
 #ifdef CONFIG_DEBUG_SG
 	BUG_ON(sg->sg_magic != SG_MAGIC);
 	BUG_ON(sg_is_chain(sg));
 #endif
-	return (struct page *)((sg)->page_link & ~0x3);
+
+	return sg->pfn;
+}
+
+/**
+ * sg_to_mappable_page - Try to return a struct page safe for general
+ *	use in the kernel
+ * @sg:		 SG entry
+ * @page:	 A pointer to the returned page
+ *
+ * Description:
+ *   If possible, return a mappable page that's safe for use around the
+ *   kernel. Should only be used in legacy situations. sg_pfn_t() is a
+ *   better choice for new code. This is deliberately more awkward than
+ *   the old sg_page to enforce the __must_check rule and discourage future
+ *   use.
+ *
+ *   An example where this is required is in nvme-fabrics: a page from an
+ *   sgl is placed into a bio. This function would be required until we can
+ *   convert bios to use pfn_t as well. Similar issues with skbs, etc.
+ **/
+static inline __must_check int sg_to_mappable_page(struct scatterlist *sg,
+						   struct page **ret)
+{
+	struct page *pg;
+
+	if (unlikely(sg_is_iomem(sg)))
+		return -EFAULT;
+
+	pg = pfn_t_to_page(sg->pfn);
+	if (unlikely(!pg))
+		return -EFAULT;
+
+	*ret = pg;
+
+	return 0;
 }

 #define SG_KMAP		     (1 << 0)	/* create a mapping with kmap */
@@ -167,8 +255,19 @@ static inline void *sg_map(struct scatterlist *sg,
size_t offset, int flags)
 	unsigned int pg_off;
 	void *ret;

+	if (unlikely(sg_is_iomem(sg))) {
+		ret = ERR_PTR(-EFAULT);
+		goto out;
+	}
+
+	pg = pfn_t_to_page(sg->pfn);
+	if (unlikely(!pg)) {
+		ret = ERR_PTR(-EFAULT);
+		goto out;
+	}
+
 	offset += sg->offset;
-	pg = nth_page(sg_page(sg), offset >> PAGE_SHIFT);
+	pg = nth_page(pg, offset >> PAGE_SHIFT);
 	pg_off = offset_in_page(offset);

 	if (flags & SG_KMAP_ATOMIC)
@@ -178,12 +277,7 @@ static inline void *sg_map(struct scatterlist *sg,
size_t offset, int flags)
 	else
 		ret = ERR_PTR(-EINVAL);

-	/*
-	 * In theory, this can't happen yet. Once we start adding
-	 * unmapable memory, it also shouldn't happen unless developers
-	 * start putting unmappable struct pages in sgls and passing
-	 * it to code that doesn't support it.
-	 */
+out:
 	BUG_ON(flags & SG_MAP_MUST_NOT_FAIL && IS_ERR(ret));

 	return ret;
@@ -202,9 +296,15 @@ static inline void *sg_map(struct scatterlist *sg,
size_t offset, int flags)
 static inline void sg_unmap(struct scatterlist *sg, void *addr,
 			    size_t offset, int flags)
 {
-	struct page *pg = nth_page(sg_page(sg), offset >> PAGE_SHIFT);
+	struct page *pg;
 	unsigned int pg_off = offset_in_page(offset);

+	pg = pfn_t_to_page(sg->pfn);
+	if (unlikely(!pg))
+		return;
+
+	pg = nth_page(pg, offset >> PAGE_SHIFT);
+
 	if (flags & SG_KMAP_ATOMIC)
 		kunmap_atomic(addr - sg->offset - pg_off);
 	else if (flags & SG_KMAP)
@@ -246,17 +346,18 @@ static inline void sg_set_buf(struct scatterlist
*sg, const void *buf,
 static inline void sg_chain(struct scatterlist *prv, unsigned int
prv_nents,
 			    struct scatterlist *sgl)
 {
+	pfn_t pfn;
+	unsigned long _sgl = (unsigned long) sgl;
+
 	/*
 	 * offset and length are unused for chain entry.  Clear them.
 	 */
 	prv[prv_nents - 1].offset = 0;
 	prv[prv_nents - 1].length = 0;

-	/*
-	 * Set lowest bit to indicate a link pointer, and make sure to clear
-	 * the termination bit if it happens to be set.
-	 */
-	prv[prv_nents - 1].page_link = ((unsigned long) sgl | 0x01) & ~0x02;
+	BUG_ON(_sgl & PAGE_MASK);
+	pfn = __pfn_to_pfn_t(_sgl >> PAGE_SHIFT, PFN_SG_CHAIN);
+	prv[prv_nents - 1].pfn = pfn;
 }

 /**
@@ -276,8 +377,8 @@ static inline void sg_mark_end(struct scatterlist *sg)
 	/*
 	 * Set termination bit, clear potential chain bit
 	 */
-	sg->page_link |= 0x02;
-	sg->page_link &= ~0x01;
+	sg->pfn.val |= PFN_SG_LAST;
+	sg->pfn.val &= ~PFN_SG_CHAIN;
 }

 /**
@@ -293,7 +394,7 @@ static inline void sg_unmark_end(struct scatterlist *sg)
 #ifdef CONFIG_DEBUG_SG
 	BUG_ON(sg->sg_magic != SG_MAGIC);
 #endif
-	sg->page_link &= ~0x02;
+	sg->pfn.val &= ~PFN_SG_LAST;
 }

 /**
@@ -301,14 +402,13 @@ static inline void sg_unmark_end(struct
scatterlist *sg)
  * @sg:	     SG entry
  *
  * Description:
- *   This calls page_to_phys() on the page in this sg entry, and adds the
- *   sg offset. The caller must know that it is legal to call
page_to_phys()
- *   on the sg page.
+ *   This calls pfn_t_to_phys() on the pfn in this sg entry, and adds the
+ *   sg offset.
  *
  **/
 static inline dma_addr_t sg_phys(struct scatterlist *sg)
 {
-	return page_to_phys(sg_page(sg)) + sg->offset;
+	return pfn_t_to_phys(sg->pfn) + sg->offset;
 }

 /**
@@ -323,7 +423,12 @@ static inline dma_addr_t sg_phys(struct scatterlist
*sg)
  **/
 static inline void *sg_virt(struct scatterlist *sg)
 {
-	return page_address(sg_page(sg)) + sg->offset;
+	struct page *pg = pfn_t_to_page(sg->pfn);
+
+	BUG_ON(sg_is_iomem(sg));
+	BUG_ON(!pg);
+
+	return page_address(pg) + sg->offset;
 }

 int sg_nents(struct scatterlist *sg);
@@ -422,10 +527,18 @@ void __sg_page_iter_start(struct sg_page_iter *piter,
 /**
  * sg_page_iter_page - get the current page held by the page iterator
  * @piter:	page iterator holding the page
+ *
+ * This function will require some cleanup. Some users simply mark
+ * attributes of the pages which are fine, others actually map it and
+ * will require some saftey there.
  */
 static inline struct page *sg_page_iter_page(struct sg_page_iter *piter)
 {
-	return nth_page(sg_page(piter->sg), piter->sg_pgoffset);
+	struct page *pg = pfn_t_to_page(piter->sg->pfn);
+	if (!pg)
+		return NULL;
+
+	return nth_page(pg, piter->sg_pgoffset);
 }

 /**
@@ -468,11 +581,13 @@ static inline dma_addr_t
sg_page_iter_dma_address(struct sg_page_iter *piter)
 #define SG_MITER_ATOMIC		(1 << 0)	 /* use kmap_atomic */
 #define SG_MITER_TO_SG		(1 << 1)	/* flush back to phys on unmap */
 #define SG_MITER_FROM_SG	(1 << 2)	/* nop */
+#define SG_MITER_SUPPORTS_IOMEM (1 << 3)        /* iteratee supports
iomem */

 struct sg_mapping_iter {
 	/* the following three fields can be accessed directly */
 	struct page		*page;		/* currently mapped page */
 	void			*addr;		/* pointer to the mapped area */
+	void __iomem            *ioaddr;        /* pointer to iomem */
 	size_t			length;		/* length of the mapped area */
 	size_t			consumed;	/* number of consumed bytes */
 	struct sg_page_iter	piter;		/* page iterator */
diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index c6cf822..2d1c58c 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -571,6 +571,8 @@ EXPORT_SYMBOL(sg_miter_skip);
  */
 bool sg_miter_next(struct sg_mapping_iter *miter)
 {
+	void *addr;
+
 	sg_miter_stop(miter);

 	/*
@@ -580,13 +582,25 @@ bool sg_miter_next(struct sg_mapping_iter *miter)
 	if (!sg_miter_get_next_page(miter))
 		return false;

+	if (sg_is_iomem(miter->piter.sg) &&
+	    !(miter->__flags & SG_MITER_SUPPORTS_IOMEM))
+		return false;
+
 	miter->page = sg_page_iter_page(&miter->piter);
 	miter->consumed = miter->length = miter->__remaining;

 	if (miter->__flags & SG_MITER_ATOMIC)
-		miter->addr = kmap_atomic(miter->page) + miter->__offset;
+		addr = kmap_atomic(miter->page) + miter->__offset;
 	else
-		miter->addr = kmap(miter->page) + miter->__offset;
+		addr = kmap(miter->page) + miter->__offset;
+
+	if (sg_is_iomem(miter->piter.sg)) {
+		miter->addr = NULL;
+		miter->ioaddr = (void * __iomem) addr;
+	} else {
+		miter->addr = addr;
+		miter->ioaddr = NULL;
+	}

 	return true;
 }
@@ -651,7 +665,7 @@ size_t sg_copy_buffer(struct scatterlist *sgl,
unsigned int nents, void *buf,
 {
 	unsigned int offset = 0;
 	struct sg_mapping_iter miter;
-	unsigned int sg_flags = SG_MITER_ATOMIC;
+	unsigned int sg_flags = SG_MITER_ATOMIC | SG_MITER_SUPPORTS_IOMEM;

 	if (to_buffer)
 		sg_flags |= SG_MITER_FROM_SG;
@@ -668,10 +682,17 @@ size_t sg_copy_buffer(struct scatterlist *sgl,
unsigned int nents, void *buf,

 		len = min(miter.length, buflen - offset);

-		if (to_buffer)
-			memcpy(buf + offset, miter.addr, len);
-		else
-			memcpy(miter.addr, buf + offset, len);
+		if (miter.addr) {
+			if (to_buffer)
+				memcpy(buf + offset, miter.addr, len);
+			else
+				memcpy(miter.addr, buf + offset, len);
+		} else if (miter.ioaddr) {
+			if (to_buffer)
+				memcpy_fromio(buf + offset, miter.addr, len);
+			else
+				memcpy_toio(miter.addr, buf + offset, len);
+		}

 		offset += len;
 	}

^ permalink raw reply related

* Re: Recover array after I panicked
From: Patrik Dahlström @ 2017-04-27 19:57 UTC (permalink / raw)
  To: Andreas Klauer; +Cc: Brad Campbell, linux-raid
In-Reply-To: <20170425110807.GA5159@metamorpher.de>


On 04/25/2017 01:08 PM, Andreas Klauer wrote:
> On Tue, Apr 25, 2017 at 12:40:37PM +0200, Patrik Dahlström wrote:
>> You mentioned something about linear device mapping before. What is
>> that? Is it something I could experiment with? How do I do that?
> 
> https://www.kernel.org/doc/Documentation/device-mapper/linear.txt
> 
> Once you have found where data overlaps on both raids, you create 
> a linear mapping of start..X of the 6disk raid, followed by X..end 
> of the 5disk RAID.
> 
> That way you get a device that holds your data intact as a whole, 
> whereas the raid sets would give you the first half of data on 
> the 6disk raid set (what was already reshaped) and the other half 
> on the 5disk raid set (what had yet to be reshaped).

Success! Using a 126M as data offset gave me valid data for the 5 disk
raid and using linear device mapping I'm able to access my data again.
Some is probably corrupted from my previous destructive recovery
attempts, but it seems like most of the data is still there and
accessible. I will start backup up the most essential data now.

> This is only a way to get read access at the data, making the raid 
> work as a standalone again (resume reshape with lost raid metadata) 
> is another problem, to be tackled after you backed up your data ;)

Now, how do we do this?

// Patrik

^ permalink raw reply

* Re: Recovery on new 2TB disk: finish=7248.4min (raid1)
From: Reindl Harald @ 2017-04-27 19:42 UTC (permalink / raw)
  To: John Stoffel, Mateusz Korniak; +Cc: Ron Leach, linux-raid
In-Reply-To: <22786.16524.435313.304834@quad.stoffel.home>



Am 27.04.2017 um 21:03 schrieb John Stoffel:
> Mateusz> They will be synced one by one, just like one big md device.
> 
> No, big MD devices are sync'd in parallel assuming MD thinks they're
> on seperate devices

if they are on sepearte drives it's no problem and no "seek storm"

> Now in this case I admit I might have jumped the
> gun, but I'm mostly commenting on the use of multiple MD RAID setups
> on a single pair of disks.
> 
> It's inefficient.  It's a pain to manage.  You lose flexibility to
> resize.

which don't matter if you have LVM on top

> Just create a single MD device across the entire disk (or possibly two
> if you want to boot off one mirrored pair) and then use LVM on top to
> carve out storage.  More flexible

but you can't boot from a RAID5/RAID6/RAID10 so you have a single point 
of failure of a single boot disk or need at least two additional disks 
for a redundant boot device

frankly on a proper designed storage machine you have no need for 
flexibility and resize because for it's entire lifetime you have enough 
storage at all and in case of LVM it don't matter how many md-devices 
are underlying the LVM

^ permalink raw reply

* Re: Recovery on new 2TB disk: finish=7248.4min (raid1)
From: John Stoffel @ 2017-04-27 19:03 UTC (permalink / raw)
  To: Mateusz Korniak; +Cc: John Stoffel, Ron Leach, linux-raid
In-Reply-To: <5492628.c8B43Z4h0G@matkor-lenovo>

>>>>> "Mateusz" == Mateusz Korniak <mateusz-lists@ant.gliwice.pl> writes:

Mateusz> On Thursday 27 of April 2017 10:25:35 John Stoffel wrote:
Ron> issued these commands one after the other:
>> 
Ron> # mdadm --manage -a /dev/mdo /dev/sdb1
Ron> # mdadm --manage -a /dev/md1 /dev/sdb2
Ron> # mdadm --manage -a /dev/md2 /dev/sdb3
Ron> # mdadm --manage -a /dev/md3 /dev/sdb5
Ron> # mdadm --manage -a /dev/md4 /dev/sdb6
Ron> # mdadm --manage -a /dev/md5 /dev/sdb7
Ron> # mdadm --manage -a /dev/md6 /dev/sdb8
Ron> # mdadm --manage -a /dev/md7 /dev/sdb9
>> 
>> Ugh!  You're setting yourself up for a true seek storm here, and way
>> too much pain down the road, IMHO.  Just mirror the entire disk and
>> put LVM volumes on top.  

Mateusz> Why having several md devices leads to "seek storm"?

It might if MD isn't being smart enough, which might happen.  

Mateusz> They will be synced one by one, just like one big md device.

No, big MD devices are sync'd in parallel assuming MD thinks they're
on seperate devices.  Now in this case I admit I might have jumped the
gun, but I'm mostly commenting on the use of multiple MD RAID setups
on a single pair of disks.

It's inefficient.  It's a pain to manage.  You lose flexibility to
resize.

Just create a single MD device across the entire disk (or possibly two
if you want to boot off one mirrored pair) and then use LVM on top to
carve out storage.  More flexible.

John

^ permalink raw reply

* Re: Recovery on new 2TB disk: finish=7248.4min (raid1)
From: Ron Leach @ 2017-04-27 19:01 UTC (permalink / raw)
  To: linux-raid
In-Reply-To: <5595439.7OflYnrvFF@matkor-lenovo>

On 27/04/2017 15:58, Mateusz Korniak wrote:
>
> What is output of
>   iostat -x 30 2 -m
> during recovery ?
>

iostat command is not recognised - it perhaps is not installed; man 
iostat was not recognised either.

What would the command have done?  I may be able to post equivalent 
information - Smartctl is installed and enabled on sdb, the new disk.

Recovery is still going on - I have not wanted to disrupt it in case I 
mess it up.  (Though all the volatile user data is multiply backed up, 
none of the OS or configuration data is so I don't want to risk 
compromising the array contents.)

regards, Ron

^ permalink raw reply

* Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions
From: Logan Gunthorpe @ 2017-04-27 15:57 UTC (permalink / raw)
  To: Jason Gunthorpe, Christoph Hellwig
  Cc: linux-nvdimm-y27Ovi1pjclAfugRpC6u6w,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
	target-devel-u79uwXL29TY76Z2rM5mHXA, Sumit Semwal,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b, James E.J. Bottomley,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA, Matthew Wilcox,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	open-iscsi-/JYPxA39Uh5TLH3MbocFFw,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	sparmaintainer-GLv8BlqOqDDQT0dZR+AlfA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	megaraidlinux.pdl-dY08KVG/lbpWk0Htik3J/w, Jens Axboe,
	Martin K. Petersen, Greg Kroah-Hartman,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170427152720.GA7662-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>



On 27/04/17 09:27 AM, Jason Gunthorpe wrote:
> On Thu, Apr 27, 2017 at 08:53:38AM +0200, Christoph Hellwig wrote:
> How about first switching as many call sites as possible to use
> sg_copy_X_buffer instead of kmap?

Yeah, I could look at doing that first.

One problem is we might get more Naks of the form of Herbert Xu's who
might be concerned with the performance implications.

These are definitely a bit more invasive changes than thin wrappers
around kmap calls.

> A random audit of Logan's series suggests this is actually a fairly
> common thing.

It's not _that_ common but there are a significant fraction. One of my
patches actually did this to two places that seemed to be reimplementing
the sg_copy_X_buffer logic.

Thanks,

Logan

^ permalink raw reply

* Re: [PATCH v2 07/21] crypto: shash, caam: Make use of the new sg_map helper function
From: Logan Gunthorpe @ 2017-04-27 15:45 UTC (permalink / raw)
  To: Herbert Xu
  Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
	target-devel-u79uwXL29TY76Z2rM5mHXA, Christoph Hellwig,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b, James E.J. Bottomley,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, Sumit Semwal,
	open-iscsi-/JYPxA39Uh5TLH3MbocFFw,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	sparmaintainer-GLv8BlqOqDDQT0dZR+AlfA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	megaraidlinux.pdl-dY08KVG/lbpWk0Htik3J/w, Jens Axboe,
	Martin K. Petersen, netdev-u79uwXL29TY76Z2rM5mHXA, Matthew Wilcox,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman,
	David S. Miller
In-Reply-To: <20170427035603.GA32212-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>



On 26/04/17 09:56 PM, Herbert Xu wrote:
> On Tue, Apr 25, 2017 at 12:20:54PM -0600, Logan Gunthorpe wrote:
>> Very straightforward conversion to the new function in the caam driver
>> and shash library.
>>
>> Signed-off-by: Logan Gunthorpe <logang-OTvnGxWRz7hWk0Htik3J/w@public.gmane.org>
>> Cc: Herbert Xu <herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>
>> Cc: "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
>> ---
>>  crypto/shash.c                | 9 ++++++---
>>  drivers/crypto/caam/caamalg.c | 8 +++-----
>>  2 files changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/crypto/shash.c b/crypto/shash.c
>> index 5e31c8d..5914881 100644
>> --- a/crypto/shash.c
>> +++ b/crypto/shash.c
>> @@ -283,10 +283,13 @@ int shash_ahash_digest(struct ahash_request *req, struct shash_desc *desc)
>>  	if (nbytes < min(sg->length, ((unsigned int)(PAGE_SIZE)) - offset)) {
>>  		void *data;
>>  
>> -		data = kmap_atomic(sg_page(sg));
>> -		err = crypto_shash_digest(desc, data + offset, nbytes,
>> +		data = sg_map(sg, 0, SG_KMAP_ATOMIC);
>> +		if (IS_ERR(data))
>> +			return PTR_ERR(data);
>> +
>> +		err = crypto_shash_digest(desc, data, nbytes,
>>  					  req->result);
>> -		kunmap_atomic(data);
>> +		sg_unmap(sg, data, 0, SG_KMAP_ATOMIC);
>>  		crypto_yield(desc->flags);
>>  	} else
>>  		err = crypto_shash_init(desc) ?:
> 
> Nack.  This is an optimisation for the special case of a single
> SG list entry.  In fact in the common case the kmap_atomic should
> disappear altogether in the no-highmem case.  So replacing it
> with sg_map is not acceptable.

What you seem to have missed is that sg_map is just a thin wrapper
around kmap_atomic. Perhaps with a future check for a mappable page.
This change should have zero impact on performance.

Logan

^ permalink raw reply

* Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions
From: Logan Gunthorpe @ 2017-04-27 15:44 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: dri-devel, Stephen Bates, dm-devel, target-devel, Sumit Semwal,
	devel, James E.J. Bottomley, linux-scsi, linux-nvdimm, linux-rdma,
	Ross Zwisler, open-iscsi, linux-media, intel-gfx, sparmaintainer,
	linux-raid, Dan Williams, megaraidlinux.pdl, Jens Axboe,
	Martin K. Petersen, netdev, Matthew Wilcox, linux-mmc,
	linux-kernel, linux-crypto, Greg Kroah-Hartman
In-Reply-To: <20170427065338.GA20677@lst.de>



On 27/04/17 12:53 AM, Christoph Hellwig wrote:
> I think you'll need to follow the existing kmap semantics and never
> fail the iomem version either.  Otherwise you'll have a special case
> that's almost never used that has a different error path.
>
> Again, wrong way.  Suddenly making things fail for your special case
> that normally don't fail is a receipe for bugs.

I don't disagree but these restrictions make the problem impossible to
solve? If there is iomem behind a page in an SGL and someone tries to
map it, we either have to fail or we break iomem safety which was your
original concern.

Logan

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply

* Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions
From: Jason Gunthorpe @ 2017-04-27 15:27 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-nvdimm-y27Ovi1pjclAfugRpC6u6w,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
	target-devel-u79uwXL29TY76Z2rM5mHXA, Sumit Semwal,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b, James E.J. Bottomley,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA, Matthew Wilcox,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	open-iscsi-/JYPxA39Uh5TLH3MbocFFw,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	sparmaintainer-GLv8BlqOqDDQT0dZR+AlfA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	megaraidlinux.pdl-dY08KVG/lbpWk0Htik3J/w, Jens Axboe,
	Martin K. Petersen, Greg Kroah-Hartman,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170427065338.GA20677-jcswGhMUV9g@public.gmane.org>

On Thu, Apr 27, 2017 at 08:53:38AM +0200, Christoph Hellwig wrote:

> > The main difficulty we
> > have now is that neither of those functions are expected to fail and we
> > need them to be able to in cases where the page doesn't map to system
> > RAM. This patch series is trying to address it for users of scatterlist.
> > I'm certainly open to other suggestions.
> 
> I think you'll need to follow the existing kmap semantics and never
> fail the iomem version either.  Otherwise you'll have a special case
> that's almost never used that has a different error path.

How about first switching as many call sites as possible to use
sg_copy_X_buffer instead of kmap?

A random audit of Logan's series suggests this is actually a fairly
common thing.

eg drivers/mmc/host/sdhci.c is only doing this:

                                        buffer = sdhci_kmap_atomic(sg, &flags);
                                        memcpy(buffer, align, size);
                                        sdhci_kunmap_atomic(buffer, &flags);

drivers/scsi/mvsas/mv_sas.c is this:

+			to = sg_map(sg_resp, 0, SG_KMAP_ATOMIC);
+			memcpy(to,
+			       slot->response + sizeof(struct mvs_err_info),
+			       sg_dma_len(sg_resp));
+			sg_unmap(sg_resp, to, 0, SG_KMAP_ATOMIC);

etc.

Lots of other places seem similar, if not sometimes a little bit more
convoluted..

Switching all the trivial cases to use copy might bring more clarity
as to what is actually required for the remaining few users? If there
are only a few then it may no longer matter if the API is not idyllic.

Jason

^ permalink raw reply

* Re: Recovery on new 2TB disk: finish=7248.4min (raid1)
From: Mateusz Korniak @ 2017-04-27 14:58 UTC (permalink / raw)
  To: Ron Leach; +Cc: linux-raid
In-Reply-To: <590117CD.1000009@tesco.net>

On Wednesday 26 of April 2017 22:57:33 Ron Leach wrote:
> is resulting in a very-unusually slow recovery.

What is output of 
 iostat -x 30 2 -m 
during recovery ?

Regards,

-- 
Mateusz Korniak
"(...) mam brata - poważny, domator, liczykrupa, hipokryta, pobożniś,
 	krótko mówiąc - podpora społeczeństwa."
				Nikos Kazantzakis - "Grek Zorba"


^ permalink raw reply

* Re: Recovery on new 2TB disk: finish=7248.4min (raid1)
From: Mateusz Korniak @ 2017-04-27 14:54 UTC (permalink / raw)
  To: John Stoffel; +Cc: Ron Leach, linux-raid
In-Reply-To: <22785.65375.947842.648174@quad.stoffel.home>

On Thursday 27 of April 2017 10:25:35 John Stoffel wrote:
> Ron> issued these commands one after the other:
> 
> Ron> # mdadm --manage -a /dev/mdo /dev/sdb1
> Ron> # mdadm --manage -a /dev/md1 /dev/sdb2
> Ron> # mdadm --manage -a /dev/md2 /dev/sdb3
> Ron> # mdadm --manage -a /dev/md3 /dev/sdb5
> Ron> # mdadm --manage -a /dev/md4 /dev/sdb6
> Ron> # mdadm --manage -a /dev/md5 /dev/sdb7
> Ron> # mdadm --manage -a /dev/md6 /dev/sdb8
> Ron> # mdadm --manage -a /dev/md7 /dev/sdb9
> 
> Ugh!  You're setting yourself up for a true seek storm here, and way
> too much pain down the road, IMHO.  Just mirror the entire disk and
> put LVM volumes on top.  

Why having several md devices leads to "seek storm"?

They will be synced one by one, just like one big md device.

Regards,
-- 
Mateusz Korniak
"(...) mam brata - poważny, domator, liczykrupa, hipokryta, pobożniś,
 	krótko mówiąc - podpora społeczeństwa."
				Nikos Kazantzakis - "Grek Zorba"


^ permalink raw reply

* Re: Recovery on new 2TB disk: finish=7248.4min (raid1)
From: Reindl Harald @ 2017-04-27 14:43 UTC (permalink / raw)
  To: John Stoffel, Ron Leach; +Cc: linux-raid
In-Reply-To: <22785.65375.947842.648174@quad.stoffel.home>



Am 27.04.2017 um 16:25 schrieb John Stoffel:
> 
> Ron> We run a 2TB fileserver in a raid1 configuration.  Today one of
> Ron> the 2 disks (/dev/sdb) failed and we've just replaced it and set
> Ron> up exactly the same partitions as the working, but degraded, raid
> Ron> has on /dev/sda.
> 
> First off, why are you bothering to do this?  You should just mirror
> the entire disk with MD

because he has partitions in use and more than on mdraid as you can see 
in the /proc/mdstat output? because he just want to replace a disk? 
becaus ehe likjely has also the operating system on one of that many 
RAIDs sharing the same disks?

> then build LVM volumes on top of that 

he is replacing a disk in a already existing RAID and has more than on 
RAID volume

> you can then allocate as you see fit, moving your data around,
> growing, shrinking volumes as you need

he is replacing a disk in a already existing RAID and has more than on 
RAID volume

[root@rh:~]$ df
Filesystem     Type  Size  Used Avail Use% Mounted on
/dev/md1       ext4   29G  6.8G   22G  24% /
/dev/md0       ext4  485M   34M  448M   7% /boot
/dev/md2       ext4  3.6T  678G  2.9T  19% /mnt/data

[root@rh:~]$ cat /proc/mdstat
Personalities : [raid10] [raid1]
md0 : active raid1 sda1[0] sdc1[1] sdb1[3] sdd1[2]
       511988 blocks super 1.0 [4/4] [UUUU]

md1 : active raid10 sda2[0] sdc2[1] sdd2[2] sdb2[3]
       30716928 blocks super 1.1 512K chunks 2 near-copies [4/4] [UUUU]

md2 : active raid10 sda3[0] sdc3[1] sdd3[2] sdb3[3]
       3875222528 blocks super 1.1 512K chunks 2 near-copies [4/4] [UUUU]
       [========>............]  check = 44.4% (1721204032/3875222528) 
finish=470.9min speed=76232K/sec




^ permalink raw reply

* Re: Recovery on new 2TB disk: finish=7248.4min (raid1)
From: John Stoffel @ 2017-04-27 14:25 UTC (permalink / raw)
  To: Ron Leach; +Cc: linux-raid
In-Reply-To: <590117CD.1000009@tesco.net>


Ron> We run a 2TB fileserver in a raid1 configuration.  Today one of
Ron> the 2 disks (/dev/sdb) failed and we've just replaced it and set
Ron> up exactly the same partitions as the working, but degraded, raid
Ron> has on /dev/sda.

First off, why are you bothering to do this?  You should just mirror
the entire disk with MD, then build LVM volumes on top of that which
you can then allocate as you see fit, moving your data around,
growing, shrinking volumes as you need.

Ron> Using the commands

Ron> # mdadm --manage -a /dev/mdo /dev/sdb1
Ron> (and so on for md 1->7)

Ron> is resulting in a very-unusually slow recovery.  And mdadm is now
Ron> recovering the largest partition, 1.8TB, but expects to spend 5
Ron> days over it.  I think I must have done something wrong.  May I
Ron> ask a couple of questions?

Did you check that values in
/sys/devices/virtual/block/md0/md/sync_speed* settings?  I suspect you
want to up the sync_speed_max to a higher number on your system.

Ron> 1 Is there a safe command to stop the recovery/add process that
Ron> is ongoing?  I reread man mdadm but did not see a command I could
Ron> use for this.

Why would you want to do this?  

Ron> 2  After the failure of /dev/sdb, mdstat listed sdb x in each md 
Ron> device with an '(F)'.  We then also 'FAIL'ed each sdb partition in 
Ron> each md device, and then powered down the machine to replace sdb. 
Ron> After powering up and booting back into Debian, we created the 
Ron> partitions on (the new) sdb to mirror those on /dev/sda.  We then 
Ron> issued these commands one after the other:

Ron> # mdadm --manage -a /dev/mdo /dev/sdb1
Ron> # mdadm --manage -a /dev/md1 /dev/sdb2
Ron> # mdadm --manage -a /dev/md2 /dev/sdb3
Ron> # mdadm --manage -a /dev/md3 /dev/sdb5
Ron> # mdadm --manage -a /dev/md4 /dev/sdb6
Ron> # mdadm --manage -a /dev/md5 /dev/sdb7
Ron> # mdadm --manage -a /dev/md6 /dev/sdb8
Ron> # mdadm --manage -a /dev/md7 /dev/sdb9

Ugh!  You're setting yourself up for a true seek storm here, and way
too much pain down the road, IMHO.  Just mirror the entire disk and
put LVM volumes on top.  

Ron> Have I missed some vital step, and so causing the recover process to 
Ron> take a very long time?

Ron> mdstat and lsdrv outputs here (UUIDs abbreviated):

Ron> # cat /proc/mdstat
Ron> Personalities : [raid1]
Ron> md7 : active raid1 sdb9[3] sda9[2]
Ron>        1894416248 blocks super 1.2 [2/1] [U_]
Ron>        [>....................]  recovery =  0.0% (1493504/1894416248) 
Ron> finish=7248.4min speed=4352K/sec

Ron> md6 : active raid1 sdb8[3] sda8[2]
Ron>        39060408 blocks super 1.2 [2/1] [U_]
Ron>          resync=DELAYED

Ron> md5 : active raid1 sdb7[3] sda7[2]
Ron>        975860 blocks super 1.2 [2/1] [U_]
Ron>          resync=DELAYED

Ron> md4 : active raid1 sdb6[3] sda6[2]
Ron>        975860 blocks super 1.2 [2/1] [U_]
Ron>          resync=DELAYED

Ron> md3 : active raid1 sdb5[3] sda5[2]
Ron>        4880372 blocks super 1.2 [2/1] [U_]
Ron>          resync=DELAYED

Ron> md2 : active raid1 sdb3[3] sda3[2]
Ron>        9764792 blocks super 1.2 [2/1] [U_]
Ron>          resync=DELAYED

Ron> md1 : active raid1 sdb2[3] sda2[2]
Ron>        2928628 blocks super 1.2 [2/2] [UU]

Ron> md0 : active raid1 sdb1[3] sda1[2]
Ron>        498676 blocks super 1.2 [2/2] [UU]

Ron> unused devices: <none>

Ron> I meant to also ask - why are the /dev/sdb partitions shown with a 
Ron> '(3)'?  Previously I think they had a '(1)'.

Ron> # ./lsdrv
Ron> **Warning** The following utility(ies) failed to execute:
Ron>    sginfo
Ron>    pvs
Ron>    lvs
Ron> Some information may be missing.

Ron> Controller platform [None]
Ron> └platform floppy.0
Ron>   └fd0 4.00k [2:0] Empty/Unknown
Ron> PCI [sata_nv] 00:08.0 IDE interface: nVidia Corporation MCP61 SATA 
Ron> Controller (rev a2)
Ron> ├scsi 0:0:0:0 ATA      WDC WD20EZRX-00D {WD-WC....R1}
Ron> │└sda 1.82t [8:0] Partitioned (dos)
Ron> │ ├sda1 487.00m [8:1] MD raid1 (0/2) (w/ sdb1) in_sync 'Server6:0' 
Ron> {b307....e950}
Ron> │ │└md0 486.99m [9:0] MD v1.2 raid1 (2) clean {b307....e950}
Ron> │ │ │                 ext2 {4ed1....e8b1}
Ron> │ │ └Mounted as /dev/md0 @ /boot
Ron> │ ├sda2 2.79g [8:2] MD raid1 (0/2) (w/ sdb2) in_sync 'Server6:1' 
Ron> {77b1....50f2}
Ron> │ │└md1 2.79g [9:1] MD v1.2 raid1 (2) clean {77b1....50f2}
Ron> │ │ │               jfs {7d08....bae5}
Ron> │ │ └Mounted as /dev/disk/by-uuid/7d08....bae5 @ /
Ron> │ ├sda3 9.31g [8:3] MD raid1 (0/2) (w/ sdb3) in_sync 'Server6:2' 
Ron> {afd6....b694}
Ron> │ │└md2 9.31g [9:2] MD v1.2 raid1 (2) clean DEGRADED, recover 
Ron> (0.00k/18.62g) 0.00k/sec {afd6....b694}
Ron> │ │ │               jfs {81bb....92f8}
Ron> │ │ └Mounted as /dev/md2 @ /usr
Ron> │ ├sda4 1.00k [8:4] Partitioned (dos)
Ron> │ ├sda5 4.66g [8:5] MD raid1 (0/2) (w/ sdb5) in_sync 'Server6:3' 
Ron> {d00a....4e99}
Ron> │ │└md3 4.65g [9:3] MD v1.2 raid1 (2) active DEGRADED, recover 
Ron> (0.00k/9.31g) 0.00k/sec {d00a....4e99}
Ron> │ │ │               jfs {375b....4fd5}
Ron> │ │ └Mounted as /dev/md3 @ /var
Ron> │ ├sda6 953.00m [8:6] MD raid1 (0/2) (w/ sdb6) in_sync 'Server6:4' 
Ron> {25af....d910}
Ron> │ │└md4 952.99m [9:4] MD v1.2 raid1 (2) clean DEGRADED, recover 
Ron> (0.00k/1.86g) 0.00k/sec {25af....d910}
Ron> │ │                   swap {d92f....2ad7}
Ron> │ ├sda7 953.00m [8:7] MD raid1 (0/2) (w/ sdb7) in_sync 'Server6:5' 
Ron> {0034....971a}
Ron> │ │└md5 952.99m [9:5] MD v1.2 raid1 (2) active DEGRADED, recover 
Ron> (0.00k/1.86g) 0.00k/sec {0034....971a}
Ron> │ │ │                 jfs {4bf7....0fff}
Ron> │ │ └Mounted as /dev/md5 @ /tmp
Ron> │ ├sda8 37.25g [8:8] MD raid1 (0/2) (w/ sdb8) in_sync 'Server6:6' 
Ron> {a5d9....568d}
Ron> │ │└md6 37.25g [9:6] MD v1.2 raid1 (2) clean DEGRADED, recover 
Ron> (0.00k/74.50g) 0.00k/sec {a5d9....568d}
Ron> │ │ │                jfs {fdf0....6478}
Ron> │ │ └Mounted as /dev/md6 @ /home
Ron> │ └sda9 1.76t [8:9] MD raid1 (0/2) (w/ sdb9) in_sync 'Server6:7' 
Ron> {9bb1....bbb4}
Ron> │  └md7 1.76t [9:7] MD v1.2 raid1 (2) clean DEGRADED, recover 
Ron> (0.00k/3.53t) 3.01m/sec {9bb1....bbb4}
Ron> │   │               jfs {60bc....33fc}
Ron> │   └Mounted as /dev/md7 @ /srv
Ron> └scsi 1:0:0:0 ATA      ST2000DL003-9VT1 {5Y....HT}
Ron>   └sdb 1.82t [8:16] Partitioned (dos)
Ron>    ├sdb1 487.00m [8:17] MD raid1 (1/2) (w/ sda1) in_sync 'Server6:0' 
Ron> {b307....e950}
Ron>    │└md0 486.99m [9:0] MD v1.2 raid1 (2) clean {b307....e950}
Ron>    │                   ext2 {4ed1....e8b1}
Ron>    ├sdb2 2.79g [8:18] MD raid1 (1/2) (w/ sda2) in_sync 'Server6:1' 
Ron> {77b1....50f2}
Ron>    │└md1 2.79g [9:1] MD v1.2 raid1 (2) clean {77b1....50f2}
Ron>    │                 jfs {7d08....bae5}
Ron>    ├sdb3 9.31g [8:19] MD raid1 (1/2) (w/ sda3) spare 'Server6:2' 
Ron> {afd6....b694}
Ron>    │└md2 9.31g [9:2] MD v1.2 raid1 (2) clean DEGRADED, recover 
Ron> (0.00k/18.62g) 0.00k/sec {afd6....b694}
Ron>    │                 jfs {81bb....92f8}
Ron>    ├sdb4 1.00k [8:20] Partitioned (dos)
Ron>    ├sdb5 4.66g [8:21] MD raid1 (1/2) (w/ sda5) spare 'Server6:3' 
Ron> {d00a....4e99}
Ron>    │└md3 4.65g [9:3] MD v1.2 raid1 (2) active DEGRADED, recover 
Ron> (0.00k/9.31g) 0.00k/sec {d00a....4e99}
Ron>    │                 jfs {375b....4fd5}
Ron>    ├sdb6 953.00m [8:22] MD raid1 (1/2) (w/ sda6) spare 'Server6:4' 
Ron> {25af....d910}
Ron>    │└md4 952.99m [9:4] MD v1.2 raid1 (2) clean DEGRADED, recover 
Ron> (0.00k/1.86g) 0.00k/sec {25af....d910}
Ron>    │                   swap {d92f....2ad7}
Ron>    ├sdb7 953.00m [8:23] MD raid1 (1/2) (w/ sda7) spare 'Server6:5' 
Ron> {0034....971a}
Ron>    │└md5 952.99m [9:5] MD v1.2 raid1 (2) active DEGRADED, recover 
Ron> (0.00k/1.86g) 0.00k/sec {0034....971a}
Ron>    │                   jfs {4bf7....0fff}
Ron>    ├sdb8 37.25g [8:24] MD raid1 (1/2) (w/ sda8) spare 'Server6:6' 
Ron> {a5d9....568d}
Ron>    │└md6 37.25g [9:6] MD v1.2 raid1 (2) clean DEGRADED, recover 
Ron> (0.00k/74.50g) 0.00k/sec {a5d9....568d}
Ron>    │                  jfs {fdf0....6478}
Ron>    ├sdb9 1.76t [8:25] MD raid1 (1/2) (w/ sda9) spare 'Server6:7' 
Ron> {9bb1....bbb4}
Ron>    │└md7 1.76t [9:7] MD v1.2 raid1 (2) clean DEGRADED, recover 
Ron> (0.00k/3.53t) 3.01m/sec {9bb1....bbb4}
Ron>    │                 jfs {60bc....33fc}
Ron>    └sdb10 1.00m [8:26] Empty/Unknown
Ron> PCI [pata_amd] 00:06.0 IDE interface: nVidia Corporation MCP61 IDE 
Ron> (rev a2)
Ron> ├scsi 2:0:0:0 AOPEN    CD-RW CRW5224 
Ron> {AOPEN_CD-RW_CRW5224_1.07_20020606_}
Ron> │└sr0 1.00g [11:0] Empty/Unknown
Ron> └scsi 3:x:x:x [Empty]
Ron> Other Block Devices
Ron> ├loop0 0.00k [7:0] Empty/Unknown
Ron> ├loop1 0.00k [7:1] Empty/Unknown
Ron> ├loop2 0.00k [7:2] Empty/Unknown
Ron> ├loop3 0.00k [7:3] Empty/Unknown
Ron> ├loop4 0.00k [7:4] Empty/Unknown
Ron> ├loop5 0.00k [7:5] Empty/Unknown
Ron> ├loop6 0.00k [7:6] Empty/Unknown
Ron> └loop7 0.00k [7:7] Empty/Unknown

Ron> OS is still as originally installed some years ago - Debian 6/Squeeze. 
Ron>   The OS has been pretty solid, though we've had to renew disks 
Ron> previously but without this very slow recovery.

Ron> I'd be very grateful for any thoughts.

Ron> regards, Ron
Ron> --
Ron> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
Ron> the body of a message to majordomo@vger.kernel.org
Ron> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v1] dm-crypt: replace custom implementation of hex2bin()
From: Andy Shevchenko @ 2017-04-27 13:52 UTC (permalink / raw)
  To: Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li, linux-raid
  Cc: Andy Shevchenko

From: Andy Shevchenko <andy.shevchenko@gmail.com>

There is no need to have a duplication of the generic library, i.e. hex2bin().
Replace the open coded variant.

Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/md/dm-crypt.c | 27 ++-------------------------
 1 file changed, 2 insertions(+), 25 deletions(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 70f765f50c59..ebf9e72d479b 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -1814,30 +1814,6 @@ static void kcryptd_queue_crypt(struct dm_crypt_io *io)
 	queue_work(cc->crypt_queue, &io->work);
 }
 
-/*
- * Decode key from its hex representation
- */
-static int crypt_decode_key(u8 *key, char *hex, unsigned int size)
-{
-	char buffer[3];
-	unsigned int i;
-
-	buffer[2] = '\0';
-
-	for (i = 0; i < size; i++) {
-		buffer[0] = *hex++;
-		buffer[1] = *hex++;
-
-		if (kstrtou8(buffer, 16, &key[i]))
-			return -EINVAL;
-	}
-
-	if (*hex != '\0')
-		return -EINVAL;
-
-	return 0;
-}
-
 static void crypt_free_tfms_aead(struct crypt_config *cc)
 {
 	if (!cc->cipher_tfm.tfms_aead)
@@ -2136,7 +2112,8 @@ static int crypt_set_key(struct crypt_config *cc, char *key)
 	kzfree(cc->key_string);
 	cc->key_string = NULL;
 
-	if (cc->key_size && crypt_decode_key(cc->key, key, cc->key_size) < 0)
+	/* Decode key from its hex representation. */
+	if (cc->key_size && hex2bin(cc->key, key, cc->key_size) < 0)
 		goto out;
 
 	r = crypt_setkey(cc);
-- 
2.11.0


^ permalink raw reply related

* Re: [PATCH 3.18-stable] dm bufio: hide bogus warning
From: Greg KH @ 2017-04-27  9:57 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: stable, Alasdair Kergon, Mike Snitzer, dm-devel, linux-raid,
	linux-kernel
In-Reply-To: <20170421134212.2727744-1-arnd@arndb.de>

On Fri, Apr 21, 2017 at 03:41:10PM +0200, Arnd Bergmann wrote:
> mips-gcc-5.3 warns about correct code on linux-3.18 and earlier:
> 
> In file included from ../include/linux/blkdev.h:4:0,
>                  from ../drivers/md/dm-bufio.h:12,
>                  from ../drivers/md/dm-bufio.c:9:
> ../drivers/md/dm-bufio.c: In function 'alloc_buffer':
> ../include/linux/sched.h:1975:56: warning: 'noio_flag' may be used uninitialized in this function [-Wmaybe-uninitialized]
>   current->flags = (current->flags & ~PF_MEMALLOC_NOIO) | flags;
>                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
> ../drivers/md/dm-bufio.c:325:11: note: 'noio_flag' was declared here
> 
> The warning disappeared on later kernels with this commit: be0c37c985ed
> ("MIPS: Rearrange PTE bits into fixed positions.")  I assume this only
> happened because it changed some inlining decisions.
> 
> On 3.18.y, we can shut up the warning by adding an extra initialization.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Now applied, thanks.

greg k-h

^ permalink raw reply

* Re: [MD PATCH v2 1/1] Use a new variable to count flighting sync requests
From: Coly Li @ 2017-04-27  8:36 UTC (permalink / raw)
  To: Xiao Ni, linux-raid; +Cc: shli, ncroxon
In-Reply-To: <1493281729-5366-1-git-send-email-xni@redhat.com>

On 2017/4/27 下午4:28, Xiao Ni wrote:
> In new barrier codes, raise_barrier waits if conf->nr_pending[idx] is not zero.
> After all the conditions are true, the resync request can go on be handled. But
> it adds conf->nr_pending[idx] again. The next resync request hit the same bucket
> idx need to wait the resync request which is submitted before. The performance
> of resync/recovery is degraded.
> So we should use a new variable to count sync requests which are in flight.
> 
> I did a simple test:
> 1. Without the patch, create a raid1 with two disks. The resync speed:
> Device:         rrqm/s   wrqm/s     r/s     w/s    rMB/s    wMB/s avgrq-sz avgqu-sz   await r_await w_await  svctm  %util
> sdb               0.00     0.00  166.00    0.00    10.38     0.00   128.00     0.03    0.20    0.20    0.00   0.19   3.20
> sdc               0.00     0.00    0.00  166.00     0.00    10.38   128.00     0.96    5.77    0.00    5.77   5.75  95.50
> 2. With the patch, the result is:
> sdb            2214.00     0.00  766.00    0.00   185.69     0.00   496.46     2.80    3.66    3.66    0.00   1.03  79.10
> sdc               0.00  2205.00    0.00  769.00     0.00   186.44   496.52     5.25    6.84    0.00    6.84   1.30 100.10
> 
> Suggested-by: Shaohua Li <shli@kernel.org>
> Signed-off-by: Xiao Ni <xni@redhat.com>

Acked-by: Coly Li <colyli@suse.de>

Thanks for the fix!

Coly


> ---
>  drivers/md/raid1.c | 5 +++--
>  drivers/md/raid1.h | 1 +
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index a34f587..ff5ee53 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -869,7 +869,7 @@ static void raise_barrier(struct r1conf *conf, sector_t sector_nr)
>  			     atomic_read(&conf->barrier[idx]) < RESYNC_DEPTH,
>  			    conf->resync_lock);
>  
> -	atomic_inc(&conf->nr_pending[idx]);
> +	atomic_inc(&conf->nr_sync_pending);
>  	spin_unlock_irq(&conf->resync_lock);
>  }
>  
> @@ -880,7 +880,7 @@ static void lower_barrier(struct r1conf *conf, sector_t sector_nr)
>  	BUG_ON(atomic_read(&conf->barrier[idx]) <= 0);
>  
>  	atomic_dec(&conf->barrier[idx]);
> -	atomic_dec(&conf->nr_pending[idx]);
> +	atomic_dec(&conf->nr_sync_pending);
>  	wake_up(&conf->wait_barrier);
>  }
>  
> @@ -1017,6 +1017,7 @@ static int get_unqueued_pending(struct r1conf *conf)
>  {
>  	int idx, ret;
>  
> +	ret = atomic_read(&conf->nr_sync_pending);
>  	for (ret = 0, idx = 0; idx < BARRIER_BUCKETS_NR; idx++)
>  		ret += atomic_read(&conf->nr_pending[idx]) -
>  			atomic_read(&conf->nr_queued[idx]);
> diff --git a/drivers/md/raid1.h b/drivers/md/raid1.h
> index dd22a37..1668f22 100644
> --- a/drivers/md/raid1.h
> +++ b/drivers/md/raid1.h
> @@ -84,6 +84,7 @@ struct r1conf {
>  	 */
>  	wait_queue_head_t	wait_barrier;
>  	spinlock_t		resync_lock;
> +	atomic_t		nr_sync_pending;
>  	atomic_t		*nr_pending;
>  	atomic_t		*nr_waiting;
>  	atomic_t		*nr_queued;
> 


^ permalink raw reply

* [MD PATCH v2 1/1] Use a new variable to count flighting sync requests
From: Xiao Ni @ 2017-04-27  8:28 UTC (permalink / raw)
  To: linux-raid; +Cc: shli, colyli, ncroxon

In new barrier codes, raise_barrier waits if conf->nr_pending[idx] is not zero.
After all the conditions are true, the resync request can go on be handled. But
it adds conf->nr_pending[idx] again. The next resync request hit the same bucket
idx need to wait the resync request which is submitted before. The performance
of resync/recovery is degraded.
So we should use a new variable to count sync requests which are in flight.

I did a simple test:
1. Without the patch, create a raid1 with two disks. The resync speed:
Device:         rrqm/s   wrqm/s     r/s     w/s    rMB/s    wMB/s avgrq-sz avgqu-sz   await r_await w_await  svctm  %util
sdb               0.00     0.00  166.00    0.00    10.38     0.00   128.00     0.03    0.20    0.20    0.00   0.19   3.20
sdc               0.00     0.00    0.00  166.00     0.00    10.38   128.00     0.96    5.77    0.00    5.77   5.75  95.50
2. With the patch, the result is:
sdb            2214.00     0.00  766.00    0.00   185.69     0.00   496.46     2.80    3.66    3.66    0.00   1.03  79.10
sdc               0.00  2205.00    0.00  769.00     0.00   186.44   496.52     5.25    6.84    0.00    6.84   1.30 100.10

Suggested-by: Shaohua Li <shli@kernel.org>
Signed-off-by: Xiao Ni <xni@redhat.com>
---
 drivers/md/raid1.c | 5 +++--
 drivers/md/raid1.h | 1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index a34f587..ff5ee53 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -869,7 +869,7 @@ static void raise_barrier(struct r1conf *conf, sector_t sector_nr)
 			     atomic_read(&conf->barrier[idx]) < RESYNC_DEPTH,
 			    conf->resync_lock);
 
-	atomic_inc(&conf->nr_pending[idx]);
+	atomic_inc(&conf->nr_sync_pending);
 	spin_unlock_irq(&conf->resync_lock);
 }
 
@@ -880,7 +880,7 @@ static void lower_barrier(struct r1conf *conf, sector_t sector_nr)
 	BUG_ON(atomic_read(&conf->barrier[idx]) <= 0);
 
 	atomic_dec(&conf->barrier[idx]);
-	atomic_dec(&conf->nr_pending[idx]);
+	atomic_dec(&conf->nr_sync_pending);
 	wake_up(&conf->wait_barrier);
 }
 
@@ -1017,6 +1017,7 @@ static int get_unqueued_pending(struct r1conf *conf)
 {
 	int idx, ret;
 
+	ret = atomic_read(&conf->nr_sync_pending);
 	for (ret = 0, idx = 0; idx < BARRIER_BUCKETS_NR; idx++)
 		ret += atomic_read(&conf->nr_pending[idx]) -
 			atomic_read(&conf->nr_queued[idx]);
diff --git a/drivers/md/raid1.h b/drivers/md/raid1.h
index dd22a37..1668f22 100644
--- a/drivers/md/raid1.h
+++ b/drivers/md/raid1.h
@@ -84,6 +84,7 @@ struct r1conf {
 	 */
 	wait_queue_head_t	wait_barrier;
 	spinlock_t		resync_lock;
+	atomic_t		nr_sync_pending;
 	atomic_t		*nr_pending;
 	atomic_t		*nr_waiting;
 	atomic_t		*nr_queued;
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions
From: Christoph Hellwig @ 2017-04-27  6:53 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: Christoph Hellwig, linux-kernel, linux-crypto, linux-media,
	dri-devel, intel-gfx, linux-raid, linux-mmc, linux-nvdimm,
	linux-scsi, open-iscsi, megaraidlinux.pdl, sparmaintainer, devel,
	target-devel, netdev, linux-rdma, dm-devel, Martin K. Petersen,
	James E.J. Bottomley, Jens Axboe, Greg Kroah-Hartman,
	Dan Williams, Ross Zwisler
In-Reply-To: <4736d44e-bbcf-5d59-a1a9-317d0f4da847@deltatee.com>

On Wed, Apr 26, 2017 at 12:11:33PM -0600, Logan Gunthorpe wrote:
> Ok, well for starters I think you are mistaken about kmap being able to
> fail. I'm having a hard time finding many users of that function that
> bother to check for an error when calling it.

A quick audit of the arch code shows you're right - kmap can't fail
anywhere anymore.

> The main difficulty we
> have now is that neither of those functions are expected to fail and we
> need them to be able to in cases where the page doesn't map to system
> RAM. This patch series is trying to address it for users of scatterlist.
> I'm certainly open to other suggestions.

I think you'll need to follow the existing kmap semantics and never
fail the iomem version either.  Otherwise you'll have a special case
that's almost never used that has a different error path.

> There are a fair number of cases in the kernel that do something like:
> 
> if (something)
>     x = kmap(page);
> else
>     x = kmap_atomic(page);
> ...
> if (something)
>     kunmap(page)
> else
>     kunmap_atomic(x)
> 
> Which just seems cumbersome to me.

Passing a different flag based on something isn't really much better.

> In any case, if you can accept an sg_kmap and sg_kmap_atomic api just
> say so and I'll make the change. But I'll still need a flags variable
> for SG_MAP_MUST_NOT_FAIL to support legacy cases that have no fail path
> and both of those functions will need to be pretty nearly replicas of
> each other.

Again, wrong way.  Suddenly making things fail for your special case
that normally don't fail is a receipe for bugs.

^ permalink raw reply

* [PATCH v3] md/r5cache: gracefully handle journal device errors for writeback mode
From: Song Liu @ 2017-04-27  5:43 UTC (permalink / raw)
  To: linux-raid
  Cc: shli, neilb, kernel-team, dan.j.williams, hch, jes.sorensen,
	Song Liu

For the raid456 with writeback cache, when journal device failed during
normal operation, it is still possible to persist all data, as all
pending data is still in stripe cache. However, it is necessary to handle
journal failure gracefully.

During journal failures, this patch makes the follow changes to land data
in cache to raid disks gracefully:

1. In raid5_remove_disk(), flush all cached stripes;
2. In handle_stripe(), allow stripes with data in journal (s.injournal > 0)
   to make progress;
3. In delay_towrite(), only process data in the cache (skip dev->towrite);
4. In __get_priority_stripe(), set try_loprio to true, so no stripe stuck
   in loprio_list
5. In r5l_do_submit_io(), submit io->split_bio first (see inline comments
   for details).

Signed-off-by: Song Liu <songliubraving@fb.com>
---
 drivers/md/raid5-cache.c | 27 ++++++++++++++++++---------
 drivers/md/raid5.c       | 37 +++++++++++++++++++++++++++++++------
 2 files changed, 49 insertions(+), 15 deletions(-)

diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index b6194e0..6b922d3 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -632,20 +632,29 @@ static void r5l_do_submit_io(struct r5l_log *log, struct r5l_io_unit *io)
 	__r5l_set_io_unit_state(io, IO_UNIT_IO_START);
 	spin_unlock_irqrestore(&log->io_list_lock, flags);
 
+	/*
+	 * In case of journal device failures, submit_bio will get error
+	 * and calls endio, then active stripes will continue write
+	 * process. Therefore, it is not necessary to check Faulty bit
+	 * of journal device here.
+	 *
+	 * However, calling r5l_log_endio for current_bio may free the
+	 * io_unit. Therefore, it is necessary to check io->split_bio
+	 * before submitting io->current_bio.
+	 */
+	if (io->split_bio) {
+		if (io->has_flush)
+			io->split_bio->bi_opf |= REQ_PREFLUSH;
+		if (io->has_fua)
+			io->split_bio->bi_opf |= REQ_FUA;
+		submit_bio(io->split_bio);
+	}
+
 	if (io->has_flush)
 		io->current_bio->bi_opf |= REQ_PREFLUSH;
 	if (io->has_fua)
 		io->current_bio->bi_opf |= REQ_FUA;
 	submit_bio(io->current_bio);
-
-	if (!io->split_bio)
-		return;
-
-	if (io->has_flush)
-		io->split_bio->bi_opf |= REQ_PREFLUSH;
-	if (io->has_fua)
-		io->split_bio->bi_opf |= REQ_FUA;
-	submit_bio(io->split_bio);
 }
 
 /* deferred io_unit will be dispatched here */
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 356cd9c..2fb67c2 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -234,10 +234,12 @@ static void do_release_stripe(struct r5conf *conf, struct stripe_head *sh,
 			if (test_bit(R5_InJournal, &sh->dev[i].flags))
 				injournal++;
 	/*
-	 * When quiesce in r5c write back, set STRIPE_HANDLE for stripes with
-	 * data in journal, so they are not released to cached lists
+	 * With writeback journal, stripes with data in the journal are
+	 * released to cached lists. However, when journal device is
+	 * failed or in quiesce, these stripes need to be handled now.
 	 */
-	if (conf->quiesce && r5c_is_writeback(conf->log) &&
+	if ((conf->quiesce || r5l_log_disk_error(conf)) &&
+	    r5c_is_writeback(conf->log) &&
 	    !test_bit(STRIPE_HANDLE, &sh->state) && injournal != 0) {
 		if (test_bit(STRIPE_R5C_CACHING, &sh->state))
 			r5c_make_stripe_write_out(sh);
@@ -2694,6 +2696,15 @@ static void raid5_error(struct mddev *mddev, struct md_rdev *rdev)
 		bdevname(rdev->bdev, b),
 		mdname(mddev),
 		conf->raid_disks - mddev->degraded);
+
+	if (test_bit(Journal, &rdev->flags) && r5c_is_writeback(conf->log)) {
+		pr_warn("md/raid:%s: Journal device failed. Flushing data in the writeback cache.",
+			mdname(mddev));
+		spin_lock_irqsave(&conf->device_lock, flags);
+		r5c_flush_cache(conf, INT_MAX);
+		spin_unlock_irqrestore(&conf->device_lock, flags);
+	}
+
 	r5c_update_on_rdev_error(mddev);
 }
 
@@ -3055,6 +3066,11 @@ sector_t raid5_compute_blocknr(struct stripe_head *sh, int i, int previous)
  *      When LOG_CRITICAL, stripes with injournal == 0 will be sent to
  *      no_space_stripes list.
  *
+ *   3. during journal failure
+ *      In journal failure, we try to flush all cached data to raid disks
+ *      based on data in stripe cache. The array is read-only to upper
+ *      layers, so we would skip all pending writes.
+ *
  */
 static inline bool delay_towrite(struct r5conf *conf,
 				 struct r5dev *dev,
@@ -3068,6 +3084,9 @@ static inline bool delay_towrite(struct r5conf *conf,
 	if (test_bit(R5C_LOG_CRITICAL, &conf->cache_state) &&
 	    s->injournal > 0)
 		return true;
+	/* case 3 above */
+	if (s->log_failed && s->injournal)
+		return true;
 	return false;
 }
 
@@ -4701,10 +4720,15 @@ static void handle_stripe(struct stripe_head *sh)
 	       " to_write=%d failed=%d failed_num=%d,%d\n",
 	       s.locked, s.uptodate, s.to_read, s.to_write, s.failed,
 	       s.failed_num[0], s.failed_num[1]);
-	/* check if the array has lost more than max_degraded devices and,
+	/*
+	 * check if the array has lost more than max_degraded devices and,
 	 * if so, some requests might need to be failed.
+	 *
+	 * When journal device failed (log_failed), we will only process
+	 * the stripe if there is data need write to raid disks
 	 */
-	if (s.failed > conf->max_degraded || s.log_failed) {
+	if (s.failed > conf->max_degraded ||
+	    (s.log_failed && s.injournal == 0)) {
 		sh->check_state = 0;
 		sh->reconstruct_state = 0;
 		break_stripe_batch_list(sh, 0);
@@ -5280,7 +5304,8 @@ static struct stripe_head *__get_priority_stripe(struct r5conf *conf, int group)
 	struct list_head *handle_list = NULL;
 	struct r5worker_group *wg;
 	bool second_try = !r5c_is_writeback(conf->log);
-	bool try_loprio = test_bit(R5C_LOG_TIGHT, &conf->cache_state);
+	bool try_loprio = test_bit(R5C_LOG_TIGHT, &conf->cache_state) ||
+		r5l_log_disk_error(conf);
 
 again:
 	wg = NULL;
-- 
2.9.3


^ permalink raw reply related

* Re: [PATCH v2 07/21] crypto: shash, caam: Make use of the new sg_map helper function
From: Herbert Xu @ 2017-04-27  3:56 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: linux-kernel, linux-crypto, linux-media, dri-devel, intel-gfx,
	linux-raid, linux-mmc, linux-nvdimm, linux-scsi, open-iscsi,
	megaraidlinux.pdl, sparmaintainer, devel, target-devel, netdev,
	linux-rdma, dm-devel, Christoph Hellwig, Martin K. Petersen,
	James E.J. Bottomley, Jens Axboe, Greg Kroah-Hartman,
	Dan Williams, Ross Zwisler
In-Reply-To: <1493144468-22493-8-git-send-email-logang@deltatee.com>

On Tue, Apr 25, 2017 at 12:20:54PM -0600, Logan Gunthorpe wrote:
> Very straightforward conversion to the new function in the caam driver
> and shash library.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> ---
>  crypto/shash.c                | 9 ++++++---
>  drivers/crypto/caam/caamalg.c | 8 +++-----
>  2 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/crypto/shash.c b/crypto/shash.c
> index 5e31c8d..5914881 100644
> --- a/crypto/shash.c
> +++ b/crypto/shash.c
> @@ -283,10 +283,13 @@ int shash_ahash_digest(struct ahash_request *req, struct shash_desc *desc)
>  	if (nbytes < min(sg->length, ((unsigned int)(PAGE_SIZE)) - offset)) {
>  		void *data;
>  
> -		data = kmap_atomic(sg_page(sg));
> -		err = crypto_shash_digest(desc, data + offset, nbytes,
> +		data = sg_map(sg, 0, SG_KMAP_ATOMIC);
> +		if (IS_ERR(data))
> +			return PTR_ERR(data);
> +
> +		err = crypto_shash_digest(desc, data, nbytes,
>  					  req->result);
> -		kunmap_atomic(data);
> +		sg_unmap(sg, data, 0, SG_KMAP_ATOMIC);
>  		crypto_yield(desc->flags);
>  	} else
>  		err = crypto_shash_init(desc) ?:

Nack.  This is an optimisation for the special case of a single
SG list entry.  In fact in the common case the kmap_atomic should
disappear altogether in the no-highmem case.  So replacing it
with sg_map is not acceptable.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [bug report] md-cluster: Fix adding of new disk with new reload code
From: Guoqing Jiang @ 2017-04-27  2:52 UTC (permalink / raw)
  To: Dan Carpenter, rgoldwyn; +Cc: linux-raid
In-Reply-To: <20170426195940.l64pymopsbv36o7e@mwanda>

HI,

Thanks for check.

On 04/27/2017 03:59 AM, Dan Carpenter wrote:
> Hello Goldwyn Rodrigues,
>
> The patch dbb64f8635f5: "md-cluster: Fix adding of new disk with new
> reload code" from Oct 1, 2015, leads to the following static checker
> warning:
>
> 	drivers/md/md-cluster.c:1341 add_new_disk()
> 	warn: inconsistent returns 'cinfo->recv_mutex'.
> 	  Locked on  : 1315,1341
> 	  Unlocked on: 1341
>
> drivers/md/md-cluster.c
>    1300  static int add_new_disk(struct mddev *mddev, struct md_rdev *rdev)
>    1301  {
>    1302          struct md_cluster_info *cinfo = mddev->cluster_info;
>    1303          struct cluster_msg cmsg;
>    1304          int ret = 0;
>    1305          struct mdp_superblock_1 *sb = page_address(rdev->sb_page);
>    1306          char *uuid = sb->device_uuid;
>    1307
>    1308          memset(&cmsg, 0, sizeof(cmsg));
>    1309          cmsg.type = cpu_to_le32(NEWDISK);
>    1310          memcpy(cmsg.uuid, uuid, 16);
>    1311          cmsg.raid_slot = cpu_to_le32(rdev->desc_nr);
>    1312          lock_comm(cinfo, 1);
>                  ^^^^^^^^^^^^^^^^^^^
> We take the lock here.
>
>    1313          ret = __sendmsg(cinfo, &cmsg);
>    1314          if (ret)
>    1315                  return ret;
>
> Should we unlock on failure here?

I agree we may need the unlock here, since both add_new_disk_cancel and
unlock_comm are not called for the failure case, it is not right.

But I think Goldwyn knows better, let's wait for his comment.


>    1316          cinfo->no_new_dev_lockres->flags |= DLM_LKF_NOQUEUE;
>    1317          ret = dlm_lock_sync(cinfo->no_new_dev_lockres, DLM_LOCK_EX);
>    1318          cinfo->no_new_dev_lockres->flags &= ~DLM_LKF_NOQUEUE;
>    1319          /* Some node does not "see" the device */
>    1320          if (ret == -EAGAIN)
>    1321                  ret = -ENOENT;
>    1322          if (ret)
>    1323                  unlock_comm(cinfo);
>
> Because we do here.  I think we're only supposed to hold the lock on
> success but how this all works with cancel etc looked slightly
> complicated so I decided to ask instead of sending a patch.

Please take a look with comments from L1326 to L1337, unlock will be 
called eventually by
metadata_update_finish (for successful case) or 
metadata_update_cancel/add_new_disk_cancel
(for failure case).

>
>    1324          else {
>    1325                  dlm_lock_sync(cinfo->no_new_dev_lockres, DLM_LOCK_CR);
>    1326                  /* Since MD_CHANGE_DEVS will be set in add_bound_rdev which
>    1327                   * will run soon after add_new_disk, the below path will be
>    1328                   * invoked:
>    1329                   *   md_wakeup_thread(mddev->thread)
>    1330                   *      -> conf->thread (raid1d)
>    1331                   *      -> md_check_recovery -> md_update_sb
>    1332                   *      -> metadata_update_start/finish
>    1333                   * MD_CLUSTER_SEND_LOCKED_ALREADY will be cleared eventually.
>    1334                   *
>    1335                   * For other failure cases, metadata_update_cancel and
>    1336                   * add_new_disk_cancel also clear below bit as well.
>    1337                   * */
>    1338                  set_bit(MD_CLUSTER_SEND_LOCKED_ALREADY, &cinfo->state);
>    1339                  wake_up(&cinfo->wait);
>    1340          }
>    1341          return ret;
>    1342  }
>

Regards,
Guoqing

^ permalink raw reply

* Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions
From: Logan Gunthorpe @ 2017-04-26 23:30 UTC (permalink / raw)
  To: Christian König, linux-kernel, linux-crypto, linux-media,
	dri-devel, intel-gfx, linux-raid, linux-mmc, linux-nvdimm,
	linux-scsi, open-iscsi, megaraidlinux.pdl, sparmaintainer, devel,
	target-devel, netdev, linux-rdma, dm-devel
  Cc: Jens Axboe, James E.J. Bottomley, Martin K. Petersen,
	Matthew Wilcox, Greg Kroah-Hartman, Stephen Bates, Ross Zwisler,
	Christoph Hellwig, Dan Williams
In-Reply-To: <5dfc5bf5-482c-b5bb-029c-7cee80925f37@amd.com>



On 26/04/17 02:59 AM,   wrote:
> Good to know that somebody is working on this. Those problems troubled
> us as well.

Thanks Christian. It's a daunting problem and a there's a lot of work to
do before we will ever be where we need to be so any help, even an ack,
is greatly appreciated.

Logan

^ permalink raw reply

* Recovery on new 2TB disk: finish=7248.4min (raid1)
From: Ron Leach @ 2017-04-26 21:57 UTC (permalink / raw)
  To: linux-raid

List, good evening,

We run a 2TB fileserver in a raid1 configuration.  Today one of the 2 
disks (/dev/sdb) failed and we've just replaced it and set up exactly 
the same partitions as the working, but degraded, raid has on /dev/sda.

Using the commands

# mdadm --manage -a /dev/mdo /dev/sdb1
(and so on for md 1->7)

is resulting in a very-unusually slow recovery.  And mdadm is now 
recovering the largest partition, 1.8TB, but expects to spend 5 days 
over it.  I think I must have done something wrong.  May I ask a 
couple of questions?

1  Is there a safe command to stop the recovery/add process that is 
ongoing?  I reread man mdadm but did not see a command I could use for 
this.

2  After the failure of /dev/sdb, mdstat listed sdb x in each md 
device with an '(F)'.  We then also 'FAIL'ed each sdb partition in 
each md device, and then powered down the machine to replace sdb. 
After powering up and booting back into Debian, we created the 
partitions on (the new) sdb to mirror those on /dev/sda.  We then 
issued these commands one after the other:

# mdadm --manage -a /dev/mdo /dev/sdb1
# mdadm --manage -a /dev/md1 /dev/sdb2
# mdadm --manage -a /dev/md2 /dev/sdb3
# mdadm --manage -a /dev/md3 /dev/sdb5
# mdadm --manage -a /dev/md4 /dev/sdb6
# mdadm --manage -a /dev/md5 /dev/sdb7
# mdadm --manage -a /dev/md6 /dev/sdb8
# mdadm --manage -a /dev/md7 /dev/sdb9

Have I missed some vital step, and so causing the recover process to 
take a very long time?

mdstat and lsdrv outputs here (UUIDs abbreviated):

# cat /proc/mdstat
Personalities : [raid1]
md7 : active raid1 sdb9[3] sda9[2]
       1894416248 blocks super 1.2 [2/1] [U_]
       [>....................]  recovery =  0.0% (1493504/1894416248) 
finish=7248.4min speed=4352K/sec

md6 : active raid1 sdb8[3] sda8[2]
       39060408 blocks super 1.2 [2/1] [U_]
         resync=DELAYED

md5 : active raid1 sdb7[3] sda7[2]
       975860 blocks super 1.2 [2/1] [U_]
         resync=DELAYED

md4 : active raid1 sdb6[3] sda6[2]
       975860 blocks super 1.2 [2/1] [U_]
         resync=DELAYED

md3 : active raid1 sdb5[3] sda5[2]
       4880372 blocks super 1.2 [2/1] [U_]
         resync=DELAYED

md2 : active raid1 sdb3[3] sda3[2]
       9764792 blocks super 1.2 [2/1] [U_]
         resync=DELAYED

md1 : active raid1 sdb2[3] sda2[2]
       2928628 blocks super 1.2 [2/2] [UU]

md0 : active raid1 sdb1[3] sda1[2]
       498676 blocks super 1.2 [2/2] [UU]

unused devices: <none>

I meant to also ask - why are the /dev/sdb partitions shown with a 
'(3)'?  Previously I think they had a '(1)'.

# ./lsdrv
**Warning** The following utility(ies) failed to execute:
   sginfo
   pvs
   lvs
Some information may be missing.

Controller platform [None]
└platform floppy.0
  └fd0 4.00k [2:0] Empty/Unknown
PCI [sata_nv] 00:08.0 IDE interface: nVidia Corporation MCP61 SATA 
Controller (rev a2)
├scsi 0:0:0:0 ATA      WDC WD20EZRX-00D {WD-WC....R1}
│└sda 1.82t [8:0] Partitioned (dos)
│ ├sda1 487.00m [8:1] MD raid1 (0/2) (w/ sdb1) in_sync 'Server6:0' 
{b307....e950}
│ │└md0 486.99m [9:0] MD v1.2 raid1 (2) clean {b307....e950}
│ │ │                 ext2 {4ed1....e8b1}
│ │ └Mounted as /dev/md0 @ /boot
│ ├sda2 2.79g [8:2] MD raid1 (0/2) (w/ sdb2) in_sync 'Server6:1' 
{77b1....50f2}
│ │└md1 2.79g [9:1] MD v1.2 raid1 (2) clean {77b1....50f2}
│ │ │               jfs {7d08....bae5}
│ │ └Mounted as /dev/disk/by-uuid/7d08....bae5 @ /
│ ├sda3 9.31g [8:3] MD raid1 (0/2) (w/ sdb3) in_sync 'Server6:2' 
{afd6....b694}
│ │└md2 9.31g [9:2] MD v1.2 raid1 (2) clean DEGRADED, recover 
(0.00k/18.62g) 0.00k/sec {afd6....b694}
│ │ │               jfs {81bb....92f8}
│ │ └Mounted as /dev/md2 @ /usr
│ ├sda4 1.00k [8:4] Partitioned (dos)
│ ├sda5 4.66g [8:5] MD raid1 (0/2) (w/ sdb5) in_sync 'Server6:3' 
{d00a....4e99}
│ │└md3 4.65g [9:3] MD v1.2 raid1 (2) active DEGRADED, recover 
(0.00k/9.31g) 0.00k/sec {d00a....4e99}
│ │ │               jfs {375b....4fd5}
│ │ └Mounted as /dev/md3 @ /var
│ ├sda6 953.00m [8:6] MD raid1 (0/2) (w/ sdb6) in_sync 'Server6:4' 
{25af....d910}
│ │└md4 952.99m [9:4] MD v1.2 raid1 (2) clean DEGRADED, recover 
(0.00k/1.86g) 0.00k/sec {25af....d910}
│ │                   swap {d92f....2ad7}
│ ├sda7 953.00m [8:7] MD raid1 (0/2) (w/ sdb7) in_sync 'Server6:5' 
{0034....971a}
│ │└md5 952.99m [9:5] MD v1.2 raid1 (2) active DEGRADED, recover 
(0.00k/1.86g) 0.00k/sec {0034....971a}
│ │ │                 jfs {4bf7....0fff}
│ │ └Mounted as /dev/md5 @ /tmp
│ ├sda8 37.25g [8:8] MD raid1 (0/2) (w/ sdb8) in_sync 'Server6:6' 
{a5d9....568d}
│ │└md6 37.25g [9:6] MD v1.2 raid1 (2) clean DEGRADED, recover 
(0.00k/74.50g) 0.00k/sec {a5d9....568d}
│ │ │                jfs {fdf0....6478}
│ │ └Mounted as /dev/md6 @ /home
│ └sda9 1.76t [8:9] MD raid1 (0/2) (w/ sdb9) in_sync 'Server6:7' 
{9bb1....bbb4}
│  └md7 1.76t [9:7] MD v1.2 raid1 (2) clean DEGRADED, recover 
(0.00k/3.53t) 3.01m/sec {9bb1....bbb4}
│   │               jfs {60bc....33fc}
│   └Mounted as /dev/md7 @ /srv
└scsi 1:0:0:0 ATA      ST2000DL003-9VT1 {5Y....HT}
  └sdb 1.82t [8:16] Partitioned (dos)
   ├sdb1 487.00m [8:17] MD raid1 (1/2) (w/ sda1) in_sync 'Server6:0' 
{b307....e950}
   │└md0 486.99m [9:0] MD v1.2 raid1 (2) clean {b307....e950}
   │                   ext2 {4ed1....e8b1}
   ├sdb2 2.79g [8:18] MD raid1 (1/2) (w/ sda2) in_sync 'Server6:1' 
{77b1....50f2}
   │└md1 2.79g [9:1] MD v1.2 raid1 (2) clean {77b1....50f2}
   │                 jfs {7d08....bae5}
   ├sdb3 9.31g [8:19] MD raid1 (1/2) (w/ sda3) spare 'Server6:2' 
{afd6....b694}
   │└md2 9.31g [9:2] MD v1.2 raid1 (2) clean DEGRADED, recover 
(0.00k/18.62g) 0.00k/sec {afd6....b694}
   │                 jfs {81bb....92f8}
   ├sdb4 1.00k [8:20] Partitioned (dos)
   ├sdb5 4.66g [8:21] MD raid1 (1/2) (w/ sda5) spare 'Server6:3' 
{d00a....4e99}
   │└md3 4.65g [9:3] MD v1.2 raid1 (2) active DEGRADED, recover 
(0.00k/9.31g) 0.00k/sec {d00a....4e99}
   │                 jfs {375b....4fd5}
   ├sdb6 953.00m [8:22] MD raid1 (1/2) (w/ sda6) spare 'Server6:4' 
{25af....d910}
   │└md4 952.99m [9:4] MD v1.2 raid1 (2) clean DEGRADED, recover 
(0.00k/1.86g) 0.00k/sec {25af....d910}
   │                   swap {d92f....2ad7}
   ├sdb7 953.00m [8:23] MD raid1 (1/2) (w/ sda7) spare 'Server6:5' 
{0034....971a}
   │└md5 952.99m [9:5] MD v1.2 raid1 (2) active DEGRADED, recover 
(0.00k/1.86g) 0.00k/sec {0034....971a}
   │                   jfs {4bf7....0fff}
   ├sdb8 37.25g [8:24] MD raid1 (1/2) (w/ sda8) spare 'Server6:6' 
{a5d9....568d}
   │└md6 37.25g [9:6] MD v1.2 raid1 (2) clean DEGRADED, recover 
(0.00k/74.50g) 0.00k/sec {a5d9....568d}
   │                  jfs {fdf0....6478}
   ├sdb9 1.76t [8:25] MD raid1 (1/2) (w/ sda9) spare 'Server6:7' 
{9bb1....bbb4}
   │└md7 1.76t [9:7] MD v1.2 raid1 (2) clean DEGRADED, recover 
(0.00k/3.53t) 3.01m/sec {9bb1....bbb4}
   │                 jfs {60bc....33fc}
   └sdb10 1.00m [8:26] Empty/Unknown
PCI [pata_amd] 00:06.0 IDE interface: nVidia Corporation MCP61 IDE 
(rev a2)
├scsi 2:0:0:0 AOPEN    CD-RW CRW5224 
{AOPEN_CD-RW_CRW5224_1.07_20020606_}
│└sr0 1.00g [11:0] Empty/Unknown
└scsi 3:x:x:x [Empty]
Other Block Devices
├loop0 0.00k [7:0] Empty/Unknown
├loop1 0.00k [7:1] Empty/Unknown
├loop2 0.00k [7:2] Empty/Unknown
├loop3 0.00k [7:3] Empty/Unknown
├loop4 0.00k [7:4] Empty/Unknown
├loop5 0.00k [7:5] Empty/Unknown
├loop6 0.00k [7:6] Empty/Unknown
└loop7 0.00k [7:7] Empty/Unknown

OS is still as originally installed some years ago - Debian 6/Squeeze. 
  The OS has been pretty solid, though we've had to renew disks 
previously but without this very slow recovery.

I'd be very grateful for any thoughts.

regards, Ron

^ permalink raw reply

* Loan Offers
From: Financial Services @ 2017-04-26 21:07 UTC (permalink / raw)
  To: Recipients

Do you need a loan to pay up bill or to start a business? Email Us

^ permalink raw reply


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