Linux RAID subsystem development
 help / color / mirror / Atom feed
* Re: [PATCH v2 1/3] badblocks: Add core badblock management code
From: Verma, Vishal L @ 2015-12-08 23:47 UTC (permalink / raw)
  To: Williams, Dan J
  Cc: neilb@suse.com, linux-block@vger.kernel.org, jmoyer@redhat.com,
	linux-raid@vger.kernel.org, linux-scsi@vger.kernel.org,
	axboe@fb.com, linux-nvdimm@lists.01.org,
	James.Bottomley@hansenpartnership.com
In-Reply-To: <CAPcyv4gJrdP23sh2cQU94fpq6+Z9GBjxWf0RpBp_4+kydpQAsA@mail.gmail.com>

On Tue, 2015-12-08 at 13:18 -0800, Dan Williams wrote:
> On Tue, Dec 8, 2015 at 1:08 PM, Verma, Vishal L
> <vishal.l.verma@intel.com> wrote:
> > On Wed, 2015-12-09 at 08:03 +1100, NeilBrown wrote:
> > > On Sat, Dec 05 2015, Verma, Vishal L wrote:
> > > > > 
> > > > > > +int badblocks_clear(struct badblocks *bb, sector_t s, int
> > > > > > sectors)
> > > > > > +{
> > > > > [...]
> > > > > > +#define DO_DEBUG 1
> > > > > 
> > > > > Why have this at all if it's unconditionally defined and
> > > > > always
> > > > > set.
> > > > 
> > > > Neil - any reason or anything you had in mind for this? Or is it
> > > > just an
> > > > artifact and can be removed.
> > > 
> > > Like the comment says:
> > > 
> > >       /* Allow clearing via sysfs *only* for testing/debugging.
> > >        * Normally only a successful write may clear a badblock
> > >        */
> > > 
> > > The DO_DEBUG define and ifdefs are documentation identifying bits
> > > of
> > > code that should be removed when it all seems to be working.
> > > Maybe now is a good time to remove that code.
> > > 
> > Hm, I think it would be nice to continue to have the ability to
> > clear
> > badblocks using sysfs at least for a while more, as we test the
> > various
> > error handling paths for NVDIMMS (Dan, thoughts?).
> > 
> > We could either remove it later or (I'm leaning towards) make it a
> > config option similar to FAIL_MAKE_REQUEST and friends..
> 
> "later" as in before v4.5-rc1?  We can always carry this debug feature
> locally for testing.  We don't want userspace growing ABI attachments
> to this capability now that it's more than just md tooling that will
> see this.


Agreed. The following incremental patch removes sysfs support.
All the latest badblocks patches can also be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/vishal/nvdimm.git gendisk-badblocks


8<-----
From 5f0e7ac31d27a132f314106f1db33af22fde03ed Mon Sep 17 00:00:00 2001
From: Vishal Verma <vishal.l.verma@intel.com>
Date: Tue, 8 Dec 2015 16:28:31 -0700
Subject: [PATCH v4 4/3] badblocks: remove support for clearing via sysfs

sysfs support for clearing badblocks was originally meant for testing
only. With the move to generalize the interface, remove this support so
that userspace doesn't start treating this as an ABI.

Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 block/badblocks.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/block/badblocks.c b/block/badblocks.c
index f0ac279..e5d2a91 100644
--- a/block/badblocks.c
+++ b/block/badblocks.c
@@ -503,16 +503,6 @@ ssize_t badblocks_store(struct badblocks *bb, const
char *page, size_t len,
 	int length;
 	char newline;
 
-	/* Allow clearing via sysfs *only* for testing/debugging.
-	 * Normally only a successful write may clear a badblock
-	 */
-	int clear = 0;
-
-	if (page[0] == '-') {
-		clear = 1;
-		page++;
-	}
-
 	switch (sscanf(page, "%llu %d%c", &sector, &length, &newline))
{
 	case 3:
 		if (newline != '\n')
@@ -525,11 +515,6 @@ ssize_t badblocks_store(struct badblocks *bb, const
char *page, size_t len,
 		return -EINVAL;
 	}
 
-	if (clear) {
-		badblocks_clear(bb, sector, length);
-		return len;
-	}
-
 	if (badblocks_set(bb, sector, length, !unack))
 		return -ENOSPC;
 	else
-- 
2.5.0

^ permalink raw reply related

* Re: [PATCH 2/2] raid5-cache: use a mempool for the metadata block
From: NeilBrown @ 2015-12-08 23:27 UTC (permalink / raw)
  To: Christoph Hellwig, shli; +Cc: linux-raid
In-Reply-To: <1449072638-15409-3-git-send-email-hch@lst.de>

[-- Attachment #1: Type: text/plain, Size: 992 bytes --]

On Thu, Dec 03 2015, Christoph Hellwig wrote:

> +
>  error:
>  	md_unregister_thread(&log->reclaim_thread);
>  reclaim_thread:
> +	mempool_destroy(log->meta_pool);
> +out_mempool:
>  	bioset_free(log->bs);
>  io_bs:

These cascading goto labels always bother me.
As mempool_destroy accepts NULL, we don't need "out_mempool", we can
just "goto reclaim_thread", or even "goto error" as md_unregsiter_thread
copes with NULL too.
If we use:
    if (log->bs)
    	bioset_free(log->bs);

or modified bioset_free, we could just have a single 'error' label...

But that is just cosmetic.  I like the patch once we agree on value for
R5L_POOL_SIZE.

Thanks,
NeilBrown


>  	kmem_cache_destroy(log->io_kc);
> @@ -1192,6 +1201,7 @@ io_kc:
>  void r5l_exit_log(struct r5l_log *log)
>  {
>  	md_unregister_thread(&log->reclaim_thread);
> +	mempool_destroy(log->meta_pool);
>  	bioset_free(log->bs);
>  	kmem_cache_destroy(log->io_kc);
>  	kfree(log);
> -- 
> 1.9.1

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

^ permalink raw reply

* Re: [PATCH 1/2] raid5-cache: use a bio_set
From: NeilBrown @ 2015-12-08 23:22 UTC (permalink / raw)
  To: Christoph Hellwig, shli; +Cc: linux-raid
In-Reply-To: <1449072638-15409-2-git-send-email-hch@lst.de>

[-- Attachment #1: Type: text/plain, Size: 3102 bytes --]

On Thu, Dec 03 2015, Christoph Hellwig wrote:

> This allows us to make guaranteed forward progress.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/md/raid5-cache.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
> index 668e973..ef59564 100644
> --- a/drivers/md/raid5-cache.c
> +++ b/drivers/md/raid5-cache.c
> @@ -34,6 +34,12 @@
>  #define RECLAIM_MAX_FREE_SPACE (10 * 1024 * 1024 * 2) /* sector */
>  #define RECLAIM_MAX_FREE_SPACE_SHIFT (2)
>  
> +/*
> + * We only need 2 bios per I/O unit to make progress, but ensure we
> + * have a few more available to not get too tight.
> + */
> +#define R5L_POOL_SIZE	1024
> +

I'm really suspicious of big pool sizes.
The memory allocated to the pool is almost never used - only where no
other memory is available - so large pools are largely wasted.

As you say, we need 2 bios per unit, and unit submission is serialized
(by ->io_mutex) so '2' really should be enough.  For the very brief
periods when there is no other memory, there will only be one or two
units in flight at once, but as each one gets us closer to freeing real
memory, that shouldn't last long.

I can easily justify '4' as "double buffering" is a well understood
technique, but 1024 just seems like gratuitous waste.

If you have performance numbers that tell me I'm wrong I'll stand
corrected, but without evidence I much prefer a smaller number.

Otherwise I really like the change.

Thanks,
NeilBrown


>  struct r5l_log {
>  	struct md_rdev *rdev;
>  
> @@ -70,6 +76,7 @@ struct r5l_log {
>  	struct bio flush_bio;
>  
>  	struct kmem_cache *io_kc;
> +	struct bio_set *bs;
>  
>  	struct md_thread *reclaim_thread;
>  	unsigned long reclaim_target;	/* number of space that need to be
> @@ -248,7 +255,7 @@ static void r5l_submit_current_io(struct r5l_log *log)
>  
>  static struct bio *r5l_bio_alloc(struct r5l_log *log)
>  {
> -	struct bio *bio = bio_kmalloc(GFP_NOIO | __GFP_NOFAIL, BIO_MAX_PAGES);
> +	struct bio *bio = bio_alloc_bioset(GFP_NOIO, BIO_MAX_PAGES, log->bs);
>  
>  	bio->bi_rw = WRITE;
>  	bio->bi_bdev = log->rdev->bdev;
> @@ -1153,6 +1160,10 @@ int r5l_init_log(struct r5conf *conf, struct md_rdev *rdev)
>  	if (!log->io_kc)
>  		goto io_kc;
>  
> +	log->bs = bioset_create(R5L_POOL_SIZE, 0);
> +	if (!log->bs)
> +		goto io_bs;
> +
>  	log->reclaim_thread = md_register_thread(r5l_reclaim_thread,
>  						 log->rdev->mddev, "reclaim");
>  	if (!log->reclaim_thread)
> @@ -1170,6 +1181,8 @@ int r5l_init_log(struct r5conf *conf, struct md_rdev *rdev)
>  error:
>  	md_unregister_thread(&log->reclaim_thread);
>  reclaim_thread:
> +	bioset_free(log->bs);
> +io_bs:
>  	kmem_cache_destroy(log->io_kc);
>  io_kc:
>  	kfree(log);
> @@ -1179,6 +1192,7 @@ io_kc:
>  void r5l_exit_log(struct r5l_log *log)
>  {
>  	md_unregister_thread(&log->reclaim_thread);
> +	bioset_free(log->bs);
>  	kmem_cache_destroy(log->io_kc);
>  	kfree(log);
>  }
> -- 
> 1.9.1

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

^ permalink raw reply

* Re: two small raid5 cache updates
From: NeilBrown @ 2015-12-08 22:57 UTC (permalink / raw)
  To: Shaohua Li, Christoph Hellwig; +Cc: linux-raid
In-Reply-To: <20151203044750.GA3111265@devbig084.prn1.facebook.com>

[-- Attachment #1: Type: text/plain, Size: 304 bytes --]

On Thu, Dec 03 2015, Shaohua Li wrote:

> On Wed, Dec 02, 2015 at 05:06:37PM +0100, Christoph Hellwig wrote:
>> The first is a trival cleanup, the second a similarly trivial reduction
>> in memory usage.
>
> Looks good, thanks!
>
> Reviewed-by: Shaohua Li <shli@fb.com>

Both applied, thanks.

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

^ permalink raw reply

* Re: [PATCH v4 0/3] Badblock tracking for gendisks
From: Finn Thain @ 2015-12-08 21:52 UTC (permalink / raw)
  To: NeilBrown
  Cc: Vishal Verma, linux-nvdimm, linux-block, linux-raid, linux-scsi,
	Jens Axboe, Jeff Moyer, James Bottomley
In-Reply-To: <87poygsmxd.fsf@notabene.neil.brown.name>


On Wed, 9 Dec 2015, NeilBrown wrote:

> On Wed, Dec 09 2015, Vishal Verma <vishal.l.verma@intel.com> wrote:
> 
> >
> > Patch 3 converts md over to use the new badblocks 'library'. I have 
> > done some pretty simple testing on this - created a raid 1 device, 
> > made sure the sysfs entries show up, and can be used to add and view 
> > badblocks. A closer look by the md folks would be nice here.
> 
> Hi,
>  this all looks reasonably sensible.  I haven't gone over it with a
>  fine toothed comb (I wonder where that analogy comes from....)

Delousing. (Not debugging.)

> but
>  nothing is obviously wrong.
> 
>  Acked-by: NeilBrown <neilb@suse.com>
> 
> Thanks,
> NeilBrown
> 

-- 

^ permalink raw reply

* RE: RAID 5,6 sequential writing seems slower in newer kernels
From: Robert Kierski @ 2015-12-08 21:24 UTC (permalink / raw)
  To: Dallas Clement; +Cc: Phil Turmel, linux-raid@vger.kernel.org
In-Reply-To: <CAE9DZUSPPkbPTkkYJE7iHwK2Y95qTmeW87O5EHKBTR45_tsQrA@mail.gmail.com>

Unfortunately, when things seem too good to be true....

I have to go back to the beginning.  I had switched to a system with HDD's, but hadn't noticed that the system has a cache that was skewing my results.   My test script attempts to turn off the cache, but in this case, it's ignored since the cache is battery backed, and therefore knows that I really want it on.

Also, I thought the drives were 10k RPM.  If that's the case, then 270 MB/s would be a fairly reasonable TP on the outer edge.  However, these are 7.2k RPM drives and aren't able to do anywhere near 270 MB/s.  The cache was skewing that result as well.

So... please ignore any message in which I claim to get 1080 MB/s from a RAID6.


Bob Kierski
Senior Storage Performance Engineer
Cray Inc.
380 Jackson Street
Suite 210
St. Paul, MN 55101
Tele: 651-967-9590
Fax:  651-605-9001
Cell: 651-890-7461


^ permalink raw reply

* Re: [PATCH v2 1/3] badblocks: Add core badblock management code
From: Dan Williams @ 2015-12-08 21:18 UTC (permalink / raw)
  To: Verma, Vishal L
  Cc: James.Bottomley@HansenPartnership.com, neilb@suse.com,
	linux-raid@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-nvdimm@lists.01.org, linux-block@vger.kernel.org,
	jmoyer@redhat.com, axboe@fb.com
In-Reply-To: <1449608892.16905.121.camel@intel.com>

On Tue, Dec 8, 2015 at 1:08 PM, Verma, Vishal L
<vishal.l.verma@intel.com> wrote:
> On Wed, 2015-12-09 at 08:03 +1100, NeilBrown wrote:
>> On Sat, Dec 05 2015, Verma, Vishal L wrote:
>> > >
>> > > > +int badblocks_clear(struct badblocks *bb, sector_t s, int
>> > > > sectors)
>> > > > +{
>> > > [...]
>> > > > +#define DO_DEBUG 1
>> > >
>> > > Why have this at all if it's unconditionally defined and always
>> > > set.
>> >
>> > Neil - any reason or anything you had in mind for this? Or is it
>> > just an
>> > artifact and can be removed.
>>
>> Like the comment says:
>>
>>       /* Allow clearing via sysfs *only* for testing/debugging.
>>        * Normally only a successful write may clear a badblock
>>        */
>>
>> The DO_DEBUG define and ifdefs are documentation identifying bits of
>> code that should be removed when it all seems to be working.
>> Maybe now is a good time to remove that code.
>>
> Hm, I think it would be nice to continue to have the ability to clear
> badblocks using sysfs at least for a while more, as we test the various
> error handling paths for NVDIMMS (Dan, thoughts?).
>
> We could either remove it later or (I'm leaning towards) make it a
> config option similar to FAIL_MAKE_REQUEST and friends..

"later" as in before v4.5-rc1?  We can always carry this debug feature
locally for testing.  We don't want userspace growing ABI attachments
to this capability now that it's more than just md tooling that will
see this.

^ permalink raw reply

* Re: [PATCH 2/2] md: update comment for md_allow_write
From: NeilBrown @ 2015-12-08 21:17 UTC (permalink / raw)
  To: Guoqing Jiang; +Cc: linux-raid, rgoldwyn
In-Reply-To: <1449253972-26997-2-git-send-email-gqjiang@suse.com>

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

On Sat, Dec 05 2015, Guoqing Jiang wrote:

> MD_CHANGE_CLEAN had been replaced with MD_CHANGE_PENDING after
> commit 070dc6 ("md: resolve confusion of MD_CHANGE_CLEAN"),
> so make the change accordingly.
>
> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> ---
>  drivers/md/md.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 5ee18a0..0feff1c 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -7701,7 +7701,7 @@ EXPORT_SYMBOL(md_write_end);
>   * attempting a GFP_KERNEL allocation while holding the mddev lock.
>   * Must be called with mddev_lock held.
>   *
> - * In the ->external case MD_CHANGE_CLEAN can not be cleared until mddev->lock
> + * In the ->external case MD_CHANGE_PENDING can not be cleared until mddev->lock
>   * is dropped, so return -EAGAIN after notifying userspace.
>   */
>  int md_allow_write(struct mddev *mddev)
> -- 
> 2.1.4

Applied, thanks.
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

^ permalink raw reply

* Re: [PATCH 1/2] md-cluster: update comments for MD_CLUSTER_SEND_LOCKED_ALREADY
From: NeilBrown @ 2015-12-08 21:17 UTC (permalink / raw)
  To: Guoqing Jiang; +Cc: linux-raid, rgoldwyn
In-Reply-To: <1449253972-26997-1-git-send-email-gqjiang@suse.com>

[-- Attachment #1: Type: text/plain, Size: 2278 bytes --]

On Sat, Dec 05 2015, Guoqing Jiang wrote:

> 1. fix unbalanced parentheses.
> 2. add more description about that MD_CLUSTER_SEND_LOCKED_ALREADY
>    will be cleared after set it in add_new_disk.
>
> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>

Applied, thanks.

NeilBrown

> ---
>  drivers/md/md-cluster.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/md/md-cluster.c b/drivers/md/md-cluster.c
> index ad3ec7d..0ded8e9 100644
> --- a/drivers/md/md-cluster.c
> +++ b/drivers/md/md-cluster.c
> @@ -53,11 +53,12 @@ struct resync_info {
>   * accomodate lock and hold. See next comment.
>   */
>  #define		MD_CLUSTER_SEND_LOCK			4
> -/* If cluster operations must lock the communication channel,
> - * so as to perform extra operations (and no other operation
> - * is allowed on the MD, such as adding a disk. Token needs
> - * to be locked and held until the operation completes with
> - * a md_update_sb(), which would eventually release the lock.
> +/* If cluster operations (such as adding a disk) must lock the
> + * communication channel, so as to perform extra operations
> + * (update metadata) and no other operation is allowed on the
> + * MD. Token needs to be locked and held until the operation
> + * completes witha md_update_sb(), which would eventually release
> + * the lock.
>   */
>  #define		MD_CLUSTER_SEND_LOCKED_ALREADY		5
>  
> @@ -1021,6 +1022,18 @@ static int add_new_disk(struct mddev *mddev, struct md_rdev *rdev)
>  		unlock_comm(cinfo);
>  	else {
>  		dlm_lock_sync(cinfo->no_new_dev_lockres, DLM_LOCK_CR);
> +		/* Since MD_CHANGE_DEVS will be set in add_bound_rdev which
> +		 * will run soon after add_new_disk, the below path will be
> +		 * invoked:
> +		 *   md_wakeup_thread(mddev->thread)
> +		 *	-> conf->thread (raid1d)
> +		 *	-> md_check_recovery -> md_update_sb
> +		 *	-> metadata_update_start/finish
> +		 * MD_CLUSTER_SEND_LOCKED_ALREADY will be cleared eventually.
> +		 *
> +		 * For other failure cases, metadata_update_cancel and
> +		 * add_new_disk_cancel also clear below bit as well.
> +		 * */
>  		set_bit(MD_CLUSTER_SEND_LOCKED_ALREADY, &cinfo->state);
>  		wake_up(&cinfo->wait);
>  	}
> -- 
> 2.1.4

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

^ permalink raw reply

* Re: [PATCH v2 1/3] badblocks: Add core badblock management code
From: Verma, Vishal L @ 2015-12-08 21:08 UTC (permalink / raw)
  To: Williams, Dan J, James.Bottomley@HansenPartnership.com,
	neilb@suse.com
  Cc: linux-raid@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-nvdimm@lists.01.org, linux-block@vger.kernel.org,
	jmoyer@redhat.com, axboe@fb.com
In-Reply-To: <87egewslfa.fsf@notabene.neil.brown.name>

[-- Attachment #1: Type: text/plain, Size: 1154 bytes --]

On Wed, 2015-12-09 at 08:03 +1100, NeilBrown wrote:
> On Sat, Dec 05 2015, Verma, Vishal L wrote:
> > > 
> > > > +int badblocks_clear(struct badblocks *bb, sector_t s, int
> > > > sectors)
> > > > +{
> > > [...]
> > > > +#define DO_DEBUG 1
> > > 
> > > Why have this at all if it's unconditionally defined and always
> > > set.
> > 
> > Neil - any reason or anything you had in mind for this? Or is it
> > just an
> > artifact and can be removed.
> 
> Like the comment says:
> 
> 	/* Allow clearing via sysfs *only* for testing/debugging.
> 	 * Normally only a successful write may clear a badblock
> 	 */
> 
> The DO_DEBUG define and ifdefs are documentation identifying bits of
> code that should be removed when it all seems to be working.
> Maybe now is a good time to remove that code.
> 
Hm, I think it would be nice to continue to have the ability to clear
badblocks using sysfs at least for a while more, as we test the various
error handling paths for NVDIMMS (Dan, thoughts?).

We could either remove it later or (I'm leaning towards) make it a
config option similar to FAIL_MAKE_REQUEST and friends..

	-Vishal

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCH v2 1/3] badblocks: Add core badblock management code
From: NeilBrown @ 2015-12-08 21:03 UTC (permalink / raw)
  To: Verma, Vishal L, James.Bottomley@HansenPartnership.com
  Cc: linux-raid@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-nvdimm@lists.01.org, linux-block@vger.kernel.org,
	jmoyer@redhat.com, axboe@fb.com
In-Reply-To: <1449273524.16905.103.camel@intel.com>

[-- Attachment #1: Type: text/plain, Size: 705 bytes --]

On Sat, Dec 05 2015, Verma, Vishal L wrote:
>> 
>> > +int badblocks_clear(struct badblocks *bb, sector_t s, int sectors)
>> > +{
>> [...]
>> > +#define DO_DEBUG 1
>> 
>> Why have this at all if it's unconditionally defined and always set.
>
> Neil - any reason or anything you had in mind for this? Or is it just an
> artifact and can be removed.

Like the comment says:

	/* Allow clearing via sysfs *only* for testing/debugging.
	 * Normally only a successful write may clear a badblock
	 */

The DO_DEBUG define and ifdefs are documentation identifying bits of
code that should be removed when it all seems to be working.
Maybe now is a good time to remove that code.

NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

^ permalink raw reply

* Re: [PATCH 1/2] [mdadm] fix bug in assemble
From: NeilBrown @ 2015-12-08 20:38 UTC (permalink / raw)
  To: linux-raid; +Cc: dan.j.williams, shli, hch, kernel-team, Song Liu
In-Reply-To: <1449536919-3773378-1-git-send-email-songliubraving@fb.com>

[-- Attachment #1: Type: text/plain, Size: 1888 bytes --]

On Tue, Dec 08 2015, Song Liu wrote:

> In Assemble, getinfo_super() over writes journal_clean, to same
> correct journal clean, keep it in "journal_clean_tmp" before
> getinfo_super().

Change "same" to "save" (and a couple of other little improvements).
>
> Signed-off-by: Song Liu <songliubraving@fb.com>
> Signed-off-by: Shaohua Li <shli@fb.com>
> ---
>  Assemble.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/Assemble.c b/Assemble.c
> index 6b9a6da..0ab5943 100644
> --- a/Assemble.c
> +++ b/Assemble.c
> @@ -1302,6 +1302,7 @@ int Assemble(struct supertype *st, char *mddev,
>  	int bestcnt = 0;
>  	int devcnt;
>  	unsigned int okcnt, sparecnt, rebuilding_cnt, replcnt, journalcnt;
> +	int journal_clean_tmp = 0;

Removed "_tmp", it is just noise.


>  	int i;
>  	int was_forced = 0;
>  	int most_recent = 0;
> @@ -1592,7 +1593,7 @@ try_again:
>  			) {
>  			devices[j].uptodate = 1;
>  			if (devices[j].i.disk.state & (1<<MD_DISK_JOURNAL))
> -				content->journal_clean = 1;
> +				journal_clean_tmp = 1;  /* store in journal_clean_tmp */

 i++; /* increment i */
!!

I removed the comment.

Applied, thanks,
NeilBrown

>  			if (i < content->array.raid_disks * 2) {
>  				if (devices[j].i.recovery_start == MaxSector ||
>  				    (content->reshape_active &&
> @@ -1664,6 +1665,8 @@ try_again:
>  #ifndef MDASSEMBLE
>  	sysfs_init(content, mdfd, NULL);
>  #endif
> +	/* after reload context, store journal_clean in context */
> +	content->journal_clean = journal_clean_tmp;
>  	for (i=0; i<bestcnt; i++) {
>  		int j = best[i];
>  		unsigned int desired_state;
> -- 
> 2.4.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

^ permalink raw reply

* Re: [PATCH v4 0/3] Badblock tracking for gendisks
From: NeilBrown @ 2015-12-08 20:30 UTC (permalink / raw)
  To: linux-nvdimm
  Cc: Vishal Verma, linux-block, linux-raid, linux-scsi, Jens Axboe,
	Jeff Moyer, James Bottomley
In-Reply-To: <1449602295-28781-1-git-send-email-vishal.l.verma@intel.com>

[-- Attachment #1: Type: text/plain, Size: 572 bytes --]

On Wed, Dec 09 2015, Vishal Verma <vishal.l.verma@intel.com> wrote:

>
> Patch 3 converts md over to use the new badblocks 'library'. I have
> done some pretty simple testing on this - created a raid 1 device,
> made sure the sysfs entries show up, and can be used to add and view
> badblocks. A closer look by the md folks would be nice here.

Hi,
 this all looks reasonably sensible.  I haven't gone over it with a
 fine toothed comb (I wonder where that analogy comes from....) but
 nothing is obviously wrong.

 Acked-by: NeilBrown <neilb@suse.com>

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

^ permalink raw reply

* Re: Help Reassembling a raid5 array
From: Phil Turmel @ 2015-12-08 19:48 UTC (permalink / raw)
  To: Andrew Wilkins; +Cc: Linux-RAID
In-Reply-To: <CAJGWEk-UovYPJcyCOhUNO4XahraB_9E5QriObJe+S6uVT_Rs1g@mail.gmail.com>

Hi Andrew,

{Added the list back.  Convention on kernel.org is reply-to-all, trim
replies, and either interleave your reply or bottom post.  I've left
some context untrimmed for the list to see.}

On 12/08/2015 01:03 PM, Andrew Wilkins wrote:
> Hi Again
> Thanks for your help
> 
> I had tried exactly what you suggested several times and it did not
> work which is why i thought i was out of options. It had been loaded
> had loaded from an ubuntu 15.10 disk for recovery
> A colleague loaded a purpose build rescue disk from here
> http://www.sysresccd.org/SystemRescueCd_Homepage
> and the same thing worked, unfortunately i don't have any more details
> on what version it was, but i assume the latest

Does this mean you did or did not use system rescue CD for this incident?

> the only difference between our commands was i used /dev/md/0 and my
> colleague used /dev/md0
> but the difference i assume is just to do with where the different
> distros mount arrays, the extra / was something ubuntu was able to put
> in all by it's self when running --scan

Correct. The difference between /dev/md0 and /dev/md/0 is not relevant.

> this is what i was trying in ubuntu and the output
> 
> root@ubuntu:/home/ubuntu# mdadm --assemble --force /dev/md/0 /dev/sdb1
> /dev/sdd1 /dev/sde1 /dev/sdf1 -v
> mdadm: looking for devices for /dev/md/0
> mdadm: /dev/sdb1 is identified as a member of /dev/md/0, slot 0.
> mdadm: /dev/sdd1 is identified as a member of /dev/md/0, slot 2.
> mdadm: /dev/sde1 is identified as a member of /dev/md/0, slot 3.
> mdadm: /dev/sdf1 is identified as a member of /dev/md/0, slot 4.
> mdadm: no uptodate device for slot 2 of /dev/md/0
> mdadm: added /dev/sdd1 to /dev/md/0 as 2
> mdadm: added /dev/sde1 to /dev/md/0 as 3
> mdadm: added /dev/sdf1 to /dev/md/0 as 4 (possibly out of date)
> mdadm: added /dev/sdb1 to /dev/md/0 as 0
> mdadm: /dev/md/0 assembled from 3 drives - not enough to start the array.
> 
> 
> It's like --force made no difference, does this seem like a bug for it
> to work on this array on one distro but not the other ? or will it be
> down to differences in versions and/or how they are setup

There are differences.  There have been bugs with --force in the past.
That's why I requested you use a recent liveCD.  New kernel and new
mdadm.  Also, try putting --force twice.

Phil

^ permalink raw reply

* Re: RAID 5,6 sequential writing seems slower in newer kernels
From: Dallas Clement @ 2015-12-08 19:38 UTC (permalink / raw)
  To: Robert Kierski; +Cc: Phil Turmel, linux-raid@vger.kernel.org
In-Reply-To: <F7761B9B1D11B64BBB666019E9378117FE0BC3@CFWEX01.americas.cray.com>

On Mon, Dec 7, 2015 at 8:29 AM, Robert Kierski <rkierski@cray.com> wrote:
> I need some clarification on what data you want collected.... do you want iostat -xm collected before and after?  Or at a periodic interval (1 sec)?
>
> I'm measuring sustained performance.  This means I’m runnig for a period of time -- 1 minute.  If you want iostat collected on an interval, that would probably be more data than one would want posted to this mailing list.
>
> But... yes, this is a RAID6.
>
> I've switched to HDD's as storage as there is concern on the list that the DDR disk devices could be causing unintended behavior in RAID6.  With 6 HDD's, each able to do 270 MB/s on their outer edge, I'm getting RAID6 throughput of about 1080 MB/s when I use a thread count of 4, using FIO.
>
> -----Original Message-----
> From: Dallas Clement [mailto:dallas.a.clement@gmail.com]
> Sent: Friday, December 04, 2015 10:08 AM
> To: Robert Kierski
> Cc: Phil Turmel; linux-raid@vger.kernel.org
> Subject: Re: RAID 5,6 sequential writing seems slower in newer kernels
>
> Robert, is this with a RAID 6 array?  Also, what are the actual throughput numbers you are getting?  Would it be possible for you to capture the iostat -xm output and report the individual disk wMB/s and also the disk utilization for RAID 6?

Hi Robert.  Thanks for posting these results.  Your fio direct I/O
results look pretty darn good.  Were you able to confirm with hdparm
-t that you really can get 270 MB/s on your disks in reality?  Also,
what formula are you using to calculate best and worst case RAID 6
read and write speeds?

> However, when I increase TC to 5, my TP is only 750 MB/s.  When I increase TC to 32, my TP is about 250 MB/s.

Now this is pretty disappointing.  I agree, 12 cores should be able to
easily handle this number of threads.
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v4 3/3] md: convert to use the generic badblocks code
From: Vishal Verma @ 2015-12-08 19:18 UTC (permalink / raw)
  To: linux-nvdimm
  Cc: Vishal Verma, linux-block, linux-raid, linux-scsi, Jens Axboe,
	NeilBrown, Jeff Moyer, James Bottomley
In-Reply-To: <1449602295-28781-1-git-send-email-vishal.l.verma@intel.com>

Retain badblocks as part of rdev, but use the accessor functions from
include/linux/badblocks for all manipulation.

Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 drivers/md/md.c | 516 +++-----------------------------------------------------
 drivers/md/md.h |  40 +----
 2 files changed, 28 insertions(+), 528 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 807095f..1e48aa9 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -34,6 +34,7 @@
 
 #include <linux/kthread.h>
 #include <linux/blkdev.h>
+#include <linux/badblocks.h>
 #include <linux/sysctl.h>
 #include <linux/seq_file.h>
 #include <linux/fs.h>
@@ -709,8 +710,7 @@ void md_rdev_clear(struct md_rdev *rdev)
 		put_page(rdev->bb_page);
 		rdev->bb_page = NULL;
 	}
-	kfree(rdev->badblocks.page);
-	rdev->badblocks.page = NULL;
+	badblocks_free(&rdev->badblocks);
 }
 EXPORT_SYMBOL_GPL(md_rdev_clear);
 
@@ -1360,8 +1360,6 @@ static __le32 calc_sb_1_csum(struct mdp_superblock_1 *sb)
 	return cpu_to_le32(csum);
 }
 
-static int md_set_badblocks(struct badblocks *bb, sector_t s, int sectors,
-			    int acknowledged);
 static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_version)
 {
 	struct mdp_superblock_1 *sb;
@@ -1486,8 +1484,7 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
 			count <<= sb->bblog_shift;
 			if (bb + 1 == 0)
 				break;
-			if (md_set_badblocks(&rdev->badblocks,
-					     sector, count, 1) == 0)
+			if (badblocks_set(&rdev->badblocks, sector, count, 1))
 				return -EINVAL;
 		}
 	} else if (sb->bblog_offset != 0)
@@ -2319,7 +2316,7 @@ repeat:
 			rdev_for_each(rdev, mddev) {
 				if (rdev->badblocks.changed) {
 					rdev->badblocks.changed = 0;
-					md_ack_all_badblocks(&rdev->badblocks);
+					ack_all_badblocks(&rdev->badblocks);
 					md_error(mddev, rdev);
 				}
 				clear_bit(Blocked, &rdev->flags);
@@ -2445,7 +2442,7 @@ repeat:
 			clear_bit(Blocked, &rdev->flags);
 
 		if (any_badblocks_changed)
-			md_ack_all_badblocks(&rdev->badblocks);
+			ack_all_badblocks(&rdev->badblocks);
 		clear_bit(BlockedBadBlocks, &rdev->flags);
 		wake_up(&rdev->blocked_wait);
 	}
@@ -3046,11 +3043,17 @@ static ssize_t recovery_start_store(struct md_rdev *rdev, const char *buf, size_
 static struct rdev_sysfs_entry rdev_recovery_start =
 __ATTR(recovery_start, S_IRUGO|S_IWUSR, recovery_start_show, recovery_start_store);
 
-static ssize_t
-badblocks_show(struct badblocks *bb, char *page, int unack);
-static ssize_t
-badblocks_store(struct badblocks *bb, const char *page, size_t len, int unack);
-
+/* sysfs access to bad-blocks list.
+ * We present two files.
+ * 'bad-blocks' lists sector numbers and lengths of ranges that
+ *    are recorded as bad.  The list is truncated to fit within
+ *    the one-page limit of sysfs.
+ *    Writing "sector length" to this file adds an acknowledged
+ *    bad block list.
+ * 'unacknowledged-bad-blocks' lists bad blocks that have not yet
+ *    been acknowledged.  Writing to this file adds bad blocks
+ *    without acknowledging them.  This is largely for testing.
+ */
 static ssize_t bb_show(struct md_rdev *rdev, char *page)
 {
 	return badblocks_show(&rdev->badblocks, page, 0);
@@ -3165,14 +3168,7 @@ int md_rdev_init(struct md_rdev *rdev)
 	 * This reserves the space even on arrays where it cannot
 	 * be used - I wonder if that matters
 	 */
-	rdev->badblocks.count = 0;
-	rdev->badblocks.shift = -1; /* disabled until explicitly enabled */
-	rdev->badblocks.page = kmalloc(PAGE_SIZE, GFP_KERNEL);
-	seqlock_init(&rdev->badblocks.lock);
-	if (rdev->badblocks.page == NULL)
-		return -ENOMEM;
-
-	return 0;
+	return badblocks_init(&rdev->badblocks, 0);
 }
 EXPORT_SYMBOL_GPL(md_rdev_init);
 /*
@@ -8478,254 +8474,9 @@ void md_finish_reshape(struct mddev *mddev)
 }
 EXPORT_SYMBOL(md_finish_reshape);
 
-/* Bad block management.
- * We can record which blocks on each device are 'bad' and so just
- * fail those blocks, or that stripe, rather than the whole device.
- * Entries in the bad-block table are 64bits wide.  This comprises:
- * Length of bad-range, in sectors: 0-511 for lengths 1-512
- * Start of bad-range, sector offset, 54 bits (allows 8 exbibytes)
- *  A 'shift' can be set so that larger blocks are tracked and
- *  consequently larger devices can be covered.
- * 'Acknowledged' flag - 1 bit. - the most significant bit.
- *
- * Locking of the bad-block table uses a seqlock so md_is_badblock
- * might need to retry if it is very unlucky.
- * We will sometimes want to check for bad blocks in a bi_end_io function,
- * so we use the write_seqlock_irq variant.
- *
- * When looking for a bad block we specify a range and want to
- * know if any block in the range is bad.  So we binary-search
- * to the last range that starts at-or-before the given endpoint,
- * (or "before the sector after the target range")
- * then see if it ends after the given start.
- * We return
- *  0 if there are no known bad blocks in the range
- *  1 if there are known bad block which are all acknowledged
- * -1 if there are bad blocks which have not yet been acknowledged in metadata.
- * plus the start/length of the first bad section we overlap.
- */
-int md_is_badblock(struct badblocks *bb, sector_t s, int sectors,
-		   sector_t *first_bad, int *bad_sectors)
-{
-	int hi;
-	int lo;
-	u64 *p = bb->page;
-	int rv;
-	sector_t target = s + sectors;
-	unsigned seq;
-
-	if (bb->shift > 0) {
-		/* round the start down, and the end up */
-		s >>= bb->shift;
-		target += (1<<bb->shift) - 1;
-		target >>= bb->shift;
-		sectors = target - s;
-	}
-	/* 'target' is now the first block after the bad range */
-
-retry:
-	seq = read_seqbegin(&bb->lock);
-	lo = 0;
-	rv = 0;
-	hi = bb->count;
-
-	/* Binary search between lo and hi for 'target'
-	 * i.e. for the last range that starts before 'target'
-	 */
-	/* INVARIANT: ranges before 'lo' and at-or-after 'hi'
-	 * are known not to be the last range before target.
-	 * VARIANT: hi-lo is the number of possible
-	 * ranges, and decreases until it reaches 1
-	 */
-	while (hi - lo > 1) {
-		int mid = (lo + hi) / 2;
-		sector_t a = BB_OFFSET(p[mid]);
-		if (a < target)
-			/* This could still be the one, earlier ranges
-			 * could not. */
-			lo = mid;
-		else
-			/* This and later ranges are definitely out. */
-			hi = mid;
-	}
-	/* 'lo' might be the last that started before target, but 'hi' isn't */
-	if (hi > lo) {
-		/* need to check all range that end after 's' to see if
-		 * any are unacknowledged.
-		 */
-		while (lo >= 0 &&
-		       BB_OFFSET(p[lo]) + BB_LEN(p[lo]) > s) {
-			if (BB_OFFSET(p[lo]) < target) {
-				/* starts before the end, and finishes after
-				 * the start, so they must overlap
-				 */
-				if (rv != -1 && BB_ACK(p[lo]))
-					rv = 1;
-				else
-					rv = -1;
-				*first_bad = BB_OFFSET(p[lo]);
-				*bad_sectors = BB_LEN(p[lo]);
-			}
-			lo--;
-		}
-	}
-
-	if (read_seqretry(&bb->lock, seq))
-		goto retry;
-
-	return rv;
-}
-EXPORT_SYMBOL_GPL(md_is_badblock);
-
-/*
- * Add a range of bad blocks to the table.
- * This might extend the table, or might contract it
- * if two adjacent ranges can be merged.
- * We binary-search to find the 'insertion' point, then
- * decide how best to handle it.
- */
-static int md_set_badblocks(struct badblocks *bb, sector_t s, int sectors,
-			    int acknowledged)
-{
-	u64 *p;
-	int lo, hi;
-	int rv = 1;
-	unsigned long flags;
-
-	if (bb->shift < 0)
-		/* badblocks are disabled */
-		return 0;
-
-	if (bb->shift) {
-		/* round the start down, and the end up */
-		sector_t next = s + sectors;
-		s >>= bb->shift;
-		next += (1<<bb->shift) - 1;
-		next >>= bb->shift;
-		sectors = next - s;
-	}
-
-	write_seqlock_irqsave(&bb->lock, flags);
-
-	p = bb->page;
-	lo = 0;
-	hi = bb->count;
-	/* Find the last range that starts at-or-before 's' */
-	while (hi - lo > 1) {
-		int mid = (lo + hi) / 2;
-		sector_t a = BB_OFFSET(p[mid]);
-		if (a <= s)
-			lo = mid;
-		else
-			hi = mid;
-	}
-	if (hi > lo && BB_OFFSET(p[lo]) > s)
-		hi = lo;
-
-	if (hi > lo) {
-		/* we found a range that might merge with the start
-		 * of our new range
-		 */
-		sector_t a = BB_OFFSET(p[lo]);
-		sector_t e = a + BB_LEN(p[lo]);
-		int ack = BB_ACK(p[lo]);
-		if (e >= s) {
-			/* Yes, we can merge with a previous range */
-			if (s == a && s + sectors >= e)
-				/* new range covers old */
-				ack = acknowledged;
-			else
-				ack = ack && acknowledged;
-
-			if (e < s + sectors)
-				e = s + sectors;
-			if (e - a <= BB_MAX_LEN) {
-				p[lo] = BB_MAKE(a, e-a, ack);
-				s = e;
-			} else {
-				/* does not all fit in one range,
-				 * make p[lo] maximal
-				 */
-				if (BB_LEN(p[lo]) != BB_MAX_LEN)
-					p[lo] = BB_MAKE(a, BB_MAX_LEN, ack);
-				s = a + BB_MAX_LEN;
-			}
-			sectors = e - s;
-		}
-	}
-	if (sectors && hi < bb->count) {
-		/* 'hi' points to the first range that starts after 's'.
-		 * Maybe we can merge with the start of that range */
-		sector_t a = BB_OFFSET(p[hi]);
-		sector_t e = a + BB_LEN(p[hi]);
-		int ack = BB_ACK(p[hi]);
-		if (a <= s + sectors) {
-			/* merging is possible */
-			if (e <= s + sectors) {
-				/* full overlap */
-				e = s + sectors;
-				ack = acknowledged;
-			} else
-				ack = ack && acknowledged;
-
-			a = s;
-			if (e - a <= BB_MAX_LEN) {
-				p[hi] = BB_MAKE(a, e-a, ack);
-				s = e;
-			} else {
-				p[hi] = BB_MAKE(a, BB_MAX_LEN, ack);
-				s = a + BB_MAX_LEN;
-			}
-			sectors = e - s;
-			lo = hi;
-			hi++;
-		}
-	}
-	if (sectors == 0 && hi < bb->count) {
-		/* we might be able to combine lo and hi */
-		/* Note: 's' is at the end of 'lo' */
-		sector_t a = BB_OFFSET(p[hi]);
-		int lolen = BB_LEN(p[lo]);
-		int hilen = BB_LEN(p[hi]);
-		int newlen = lolen + hilen - (s - a);
-		if (s >= a && newlen < BB_MAX_LEN) {
-			/* yes, we can combine them */
-			int ack = BB_ACK(p[lo]) && BB_ACK(p[hi]);
-			p[lo] = BB_MAKE(BB_OFFSET(p[lo]), newlen, ack);
-			memmove(p + hi, p + hi + 1,
-				(bb->count - hi - 1) * 8);
-			bb->count--;
-		}
-	}
-	while (sectors) {
-		/* didn't merge (it all).
-		 * Need to add a range just before 'hi' */
-		if (bb->count >= MD_MAX_BADBLOCKS) {
-			/* No room for more */
-			rv = 0;
-			break;
-		} else {
-			int this_sectors = sectors;
-			memmove(p + hi + 1, p + hi,
-				(bb->count - hi) * 8);
-			bb->count++;
-
-			if (this_sectors > BB_MAX_LEN)
-				this_sectors = BB_MAX_LEN;
-			p[hi] = BB_MAKE(s, this_sectors, acknowledged);
-			sectors -= this_sectors;
-			s += this_sectors;
-		}
-	}
-
-	bb->changed = 1;
-	if (!acknowledged)
-		bb->unacked_exist = 1;
-	write_sequnlock_irqrestore(&bb->lock, flags);
-
-	return rv;
-}
+/* Bad block management */
 
+/* Returns 1 on success, 0 on failure */
 int rdev_set_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
 		       int is_new)
 {
@@ -8734,114 +8485,19 @@ int rdev_set_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
 		s += rdev->new_data_offset;
 	else
 		s += rdev->data_offset;
-	rv = md_set_badblocks(&rdev->badblocks,
-			      s, sectors, 0);
-	if (rv) {
+	rv = badblocks_set(&rdev->badblocks, s, sectors, 0);
+	if (rv == 0) {
 		/* Make sure they get written out promptly */
 		sysfs_notify_dirent_safe(rdev->sysfs_state);
 		set_bit(MD_CHANGE_CLEAN, &rdev->mddev->flags);
 		set_bit(MD_CHANGE_PENDING, &rdev->mddev->flags);
 		md_wakeup_thread(rdev->mddev->thread);
-	}
-	return rv;
+		return 1;
+	} else
+		return 0;
 }
 EXPORT_SYMBOL_GPL(rdev_set_badblocks);
 
-/*
- * Remove a range of bad blocks from the table.
- * This may involve extending the table if we spilt a region,
- * but it must not fail.  So if the table becomes full, we just
- * drop the remove request.
- */
-static int md_clear_badblocks(struct badblocks *bb, sector_t s, int sectors)
-{
-	u64 *p;
-	int lo, hi;
-	sector_t target = s + sectors;
-	int rv = 0;
-
-	if (bb->shift > 0) {
-		/* When clearing we round the start up and the end down.
-		 * This should not matter as the shift should align with
-		 * the block size and no rounding should ever be needed.
-		 * However it is better the think a block is bad when it
-		 * isn't than to think a block is not bad when it is.
-		 */
-		s += (1<<bb->shift) - 1;
-		s >>= bb->shift;
-		target >>= bb->shift;
-		sectors = target - s;
-	}
-
-	write_seqlock_irq(&bb->lock);
-
-	p = bb->page;
-	lo = 0;
-	hi = bb->count;
-	/* Find the last range that starts before 'target' */
-	while (hi - lo > 1) {
-		int mid = (lo + hi) / 2;
-		sector_t a = BB_OFFSET(p[mid]);
-		if (a < target)
-			lo = mid;
-		else
-			hi = mid;
-	}
-	if (hi > lo) {
-		/* p[lo] is the last range that could overlap the
-		 * current range.  Earlier ranges could also overlap,
-		 * but only this one can overlap the end of the range.
-		 */
-		if (BB_OFFSET(p[lo]) + BB_LEN(p[lo]) > target) {
-			/* Partial overlap, leave the tail of this range */
-			int ack = BB_ACK(p[lo]);
-			sector_t a = BB_OFFSET(p[lo]);
-			sector_t end = a + BB_LEN(p[lo]);
-
-			if (a < s) {
-				/* we need to split this range */
-				if (bb->count >= MD_MAX_BADBLOCKS) {
-					rv = -ENOSPC;
-					goto out;
-				}
-				memmove(p+lo+1, p+lo, (bb->count - lo) * 8);
-				bb->count++;
-				p[lo] = BB_MAKE(a, s-a, ack);
-				lo++;
-			}
-			p[lo] = BB_MAKE(target, end - target, ack);
-			/* there is no longer an overlap */
-			hi = lo;
-			lo--;
-		}
-		while (lo >= 0 &&
-		       BB_OFFSET(p[lo]) + BB_LEN(p[lo]) > s) {
-			/* This range does overlap */
-			if (BB_OFFSET(p[lo]) < s) {
-				/* Keep the early parts of this range. */
-				int ack = BB_ACK(p[lo]);
-				sector_t start = BB_OFFSET(p[lo]);
-				p[lo] = BB_MAKE(start, s - start, ack);
-				/* now low doesn't overlap, so.. */
-				break;
-			}
-			lo--;
-		}
-		/* 'lo' is strictly before, 'hi' is strictly after,
-		 * anything between needs to be discarded
-		 */
-		if (hi - lo > 1) {
-			memmove(p+lo+1, p+hi, (bb->count - hi) * 8);
-			bb->count -= (hi - lo - 1);
-		}
-	}
-
-	bb->changed = 1;
-out:
-	write_sequnlock_irq(&bb->lock);
-	return rv;
-}
-
 int rdev_clear_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
 			 int is_new)
 {
@@ -8849,133 +8505,11 @@ int rdev_clear_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
 		s += rdev->new_data_offset;
 	else
 		s += rdev->data_offset;
-	return md_clear_badblocks(&rdev->badblocks,
+	return badblocks_clear(&rdev->badblocks,
 				  s, sectors);
 }
 EXPORT_SYMBOL_GPL(rdev_clear_badblocks);
 
-/*
- * Acknowledge all bad blocks in a list.
- * This only succeeds if ->changed is clear.  It is used by
- * in-kernel metadata updates
- */
-void md_ack_all_badblocks(struct badblocks *bb)
-{
-	if (bb->page == NULL || bb->changed)
-		/* no point even trying */
-		return;
-	write_seqlock_irq(&bb->lock);
-
-	if (bb->changed == 0 && bb->unacked_exist) {
-		u64 *p = bb->page;
-		int i;
-		for (i = 0; i < bb->count ; i++) {
-			if (!BB_ACK(p[i])) {
-				sector_t start = BB_OFFSET(p[i]);
-				int len = BB_LEN(p[i]);
-				p[i] = BB_MAKE(start, len, 1);
-			}
-		}
-		bb->unacked_exist = 0;
-	}
-	write_sequnlock_irq(&bb->lock);
-}
-EXPORT_SYMBOL_GPL(md_ack_all_badblocks);
-
-/* sysfs access to bad-blocks list.
- * We present two files.
- * 'bad-blocks' lists sector numbers and lengths of ranges that
- *    are recorded as bad.  The list is truncated to fit within
- *    the one-page limit of sysfs.
- *    Writing "sector length" to this file adds an acknowledged
- *    bad block list.
- * 'unacknowledged-bad-blocks' lists bad blocks that have not yet
- *    been acknowledged.  Writing to this file adds bad blocks
- *    without acknowledging them.  This is largely for testing.
- */
-
-static ssize_t
-badblocks_show(struct badblocks *bb, char *page, int unack)
-{
-	size_t len;
-	int i;
-	u64 *p = bb->page;
-	unsigned seq;
-
-	if (bb->shift < 0)
-		return 0;
-
-retry:
-	seq = read_seqbegin(&bb->lock);
-
-	len = 0;
-	i = 0;
-
-	while (len < PAGE_SIZE && i < bb->count) {
-		sector_t s = BB_OFFSET(p[i]);
-		unsigned int length = BB_LEN(p[i]);
-		int ack = BB_ACK(p[i]);
-		i++;
-
-		if (unack && ack)
-			continue;
-
-		len += snprintf(page+len, PAGE_SIZE-len, "%llu %u\n",
-				(unsigned long long)s << bb->shift,
-				length << bb->shift);
-	}
-	if (unack && len == 0)
-		bb->unacked_exist = 0;
-
-	if (read_seqretry(&bb->lock, seq))
-		goto retry;
-
-	return len;
-}
-
-#define DO_DEBUG 1
-
-static ssize_t
-badblocks_store(struct badblocks *bb, const char *page, size_t len, int unack)
-{
-	unsigned long long sector;
-	int length;
-	char newline;
-#ifdef DO_DEBUG
-	/* Allow clearing via sysfs *only* for testing/debugging.
-	 * Normally only a successful write may clear a badblock
-	 */
-	int clear = 0;
-	if (page[0] == '-') {
-		clear = 1;
-		page++;
-	}
-#endif /* DO_DEBUG */
-
-	switch (sscanf(page, "%llu %d%c", &sector, &length, &newline)) {
-	case 3:
-		if (newline != '\n')
-			return -EINVAL;
-	case 2:
-		if (length <= 0)
-			return -EINVAL;
-		break;
-	default:
-		return -EINVAL;
-	}
-
-#ifdef DO_DEBUG
-	if (clear) {
-		md_clear_badblocks(bb, sector, length);
-		return len;
-	}
-#endif /* DO_DEBUG */
-	if (md_set_badblocks(bb, sector, length, !unack))
-		return len;
-	else
-		return -ENOSPC;
-}
-
 static int md_notify_reboot(struct notifier_block *this,
 			    unsigned long code, void *x)
 {
diff --git a/drivers/md/md.h b/drivers/md/md.h
index 2bea51e..389afc4 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -17,6 +17,7 @@
 
 #include <linux/blkdev.h>
 #include <linux/backing-dev.h>
+#include <linux/badblocks.h>
 #include <linux/kobject.h>
 #include <linux/list.h>
 #include <linux/mm.h>
@@ -28,13 +29,6 @@
 
 #define MaxSector (~(sector_t)0)
 
-/* Bad block numbers are stored sorted in a single page.
- * 64bits is used for each block or extent.
- * 54 bits are sector number, 9 bits are extent size,
- * 1 bit is an 'acknowledged' flag.
- */
-#define MD_MAX_BADBLOCKS	(PAGE_SIZE/8)
-
 /*
  * MD's 'extended' device
  */
@@ -117,22 +111,7 @@ struct md_rdev {
 	struct kernfs_node *sysfs_state; /* handle for 'state'
 					   * sysfs entry */
 
-	struct badblocks {
-		int	count;		/* count of bad blocks */
-		int	unacked_exist;	/* there probably are unacknowledged
-					 * bad blocks.  This is only cleared
-					 * when a read discovers none
-					 */
-		int	shift;		/* shift from sectors to block size
-					 * a -ve shift means badblocks are
-					 * disabled.*/
-		u64	*page;		/* badblock list */
-		int	changed;
-		seqlock_t lock;
-
-		sector_t sector;
-		sector_t size;		/* in sectors */
-	} badblocks;
+	struct badblocks badblocks;
 };
 enum flag_bits {
 	Faulty,			/* device is known to have a fault */
@@ -185,22 +164,11 @@ enum flag_bits {
 				 */
 };
 
-#define BB_LEN_MASK	(0x00000000000001FFULL)
-#define BB_OFFSET_MASK	(0x7FFFFFFFFFFFFE00ULL)
-#define BB_ACK_MASK	(0x8000000000000000ULL)
-#define BB_MAX_LEN	512
-#define BB_OFFSET(x)	(((x) & BB_OFFSET_MASK) >> 9)
-#define BB_LEN(x)	(((x) & BB_LEN_MASK) + 1)
-#define BB_ACK(x)	(!!((x) & BB_ACK_MASK))
-#define BB_MAKE(a, l, ack) (((a)<<9) | ((l)-1) | ((u64)(!!(ack)) << 63))
-
-extern int md_is_badblock(struct badblocks *bb, sector_t s, int sectors,
-			  sector_t *first_bad, int *bad_sectors);
 static inline int is_badblock(struct md_rdev *rdev, sector_t s, int sectors,
 			      sector_t *first_bad, int *bad_sectors)
 {
 	if (unlikely(rdev->badblocks.count)) {
-		int rv = md_is_badblock(&rdev->badblocks, rdev->data_offset + s,
+		int rv = badblocks_check(&rdev->badblocks, rdev->data_offset + s,
 					sectors,
 					first_bad, bad_sectors);
 		if (rv)
@@ -213,8 +181,6 @@ extern int rdev_set_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
 			      int is_new);
 extern int rdev_clear_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
 				int is_new);
-extern void md_ack_all_badblocks(struct badblocks *bb);
-
 struct md_cluster_info;
 
 struct mddev {
-- 
2.5.0


^ permalink raw reply related

* [PATCH v4 2/3] block: Add badblock management for gendisks
From: Vishal Verma @ 2015-12-08 19:18 UTC (permalink / raw)
  To: linux-nvdimm
  Cc: Vishal Verma, linux-block, linux-raid, linux-scsi, Jens Axboe,
	NeilBrown, Jeff Moyer, James Bottomley
In-Reply-To: <1449602295-28781-1-git-send-email-vishal.l.verma@intel.com>

NVDIMM devices, which can behave more like DRAM rather than block
devices, may develop bad cache lines, or 'poison'. A block device
exposed by the pmem driver can then consume poison via a read (or
write), and cause a machine check. On platforms without machine
check recovery features, this would mean a crash.

The block device maintaining a runtime list of all known sectors that
have poison can directly avoid this, and also provide a path forward
to enable proper handling/recovery for DAX faults on such a device.

Use the new badblock management interfaces to add a badblocks list to
gendisks.

Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 block/genhd.c         | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/genhd.h |  7 +++++
 2 files changed, 83 insertions(+)

diff --git a/block/genhd.c b/block/genhd.c
index e5cafa5..81dcf32 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -20,6 +20,7 @@
 #include <linux/idr.h>
 #include <linux/log2.h>
 #include <linux/pm_runtime.h>
+#include <linux/badblocks.h>
 
 #include "blk.h"
 
@@ -505,6 +506,16 @@ static int exact_lock(dev_t devt, void *data)
 	return 0;
 }
 
+int disk_alloc_badblocks(struct gendisk *disk)
+{
+	disk->bb = kzalloc(sizeof(*(disk->bb)), GFP_KERNEL);
+	if (!disk->bb)
+		return -ENOMEM;
+
+	return badblocks_init(disk->bb, 1);
+}
+EXPORT_SYMBOL(disk_alloc_badblocks);
+
 static void register_disk(struct gendisk *disk)
 {
 	struct device *ddev = disk_to_dev(disk);
@@ -659,6 +670,11 @@ void del_gendisk(struct gendisk *disk)
 	blk_unregister_queue(disk);
 	blk_unregister_region(disk_devt(disk), disk->minors);
 
+	if (disk->bb) {
+		badblocks_free(disk->bb);
+		kfree(disk->bb);
+	}
+
 	part_stat_set_all(&disk->part0, 0);
 	disk->part0.stamp = 0;
 
@@ -672,6 +688,63 @@ void del_gendisk(struct gendisk *disk)
 }
 EXPORT_SYMBOL(del_gendisk);
 
+/*
+ * The gendisk usage of badblocks does not track acknowledgements for
+ * badblocks. We always assume they are acknowledged.
+ */
+int disk_check_badblocks(struct gendisk *disk, sector_t s, int sectors,
+		   sector_t *first_bad, int *bad_sectors)
+{
+	if (!disk->bb)
+		return 0;
+
+	return badblocks_check(disk->bb, s, sectors, first_bad, bad_sectors);
+}
+EXPORT_SYMBOL(disk_check_badblocks);
+
+int disk_set_badblocks(struct gendisk *disk, sector_t s, int sectors)
+{
+	if (!disk->bb)
+		return 0;
+
+	return badblocks_set(disk->bb, s, sectors, 1);
+}
+EXPORT_SYMBOL(disk_set_badblocks);
+
+int disk_clear_badblocks(struct gendisk *disk, sector_t s, int sectors)
+{
+	if (!disk->bb)
+		return 0;
+
+	return badblocks_clear(disk->bb, s, sectors);
+}
+EXPORT_SYMBOL(disk_clear_badblocks);
+
+/* sysfs access to bad-blocks list. */
+static ssize_t disk_badblocks_show(struct device *dev,
+					struct device_attribute *attr,
+					char *page)
+{
+	struct gendisk *disk = dev_to_disk(dev);
+
+	if (!disk->bb)
+		return 0;
+
+	return badblocks_show(disk->bb, page, 0);
+}
+
+static ssize_t disk_badblocks_store(struct device *dev,
+					struct device_attribute *attr,
+					const char *page, size_t len)
+{
+	struct gendisk *disk = dev_to_disk(dev);
+
+	if (!disk->bb)
+		return 0;
+
+	return badblocks_store(disk->bb, page, len, 0);
+}
+
 /**
  * get_gendisk - get partitioning information for a given device
  * @devt: device to get partitioning information for
@@ -990,6 +1063,8 @@ static DEVICE_ATTR(discard_alignment, S_IRUGO, disk_discard_alignment_show,
 static DEVICE_ATTR(capability, S_IRUGO, disk_capability_show, NULL);
 static DEVICE_ATTR(stat, S_IRUGO, part_stat_show, NULL);
 static DEVICE_ATTR(inflight, S_IRUGO, part_inflight_show, NULL);
+static DEVICE_ATTR(badblocks, S_IRUGO | S_IWUSR, disk_badblocks_show,
+		disk_badblocks_store);
 #ifdef CONFIG_FAIL_MAKE_REQUEST
 static struct device_attribute dev_attr_fail =
 	__ATTR(make-it-fail, S_IRUGO|S_IWUSR, part_fail_show, part_fail_store);
@@ -1011,6 +1086,7 @@ static struct attribute *disk_attrs[] = {
 	&dev_attr_capability.attr,
 	&dev_attr_stat.attr,
 	&dev_attr_inflight.attr,
+	&dev_attr_badblocks.attr,
 #ifdef CONFIG_FAIL_MAKE_REQUEST
 	&dev_attr_fail.attr,
 #endif
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 847cc1d..0bbec68 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -162,6 +162,7 @@ struct disk_part_tbl {
 };
 
 struct disk_events;
+struct badblocks;
 
 #if defined(CONFIG_BLK_DEV_INTEGRITY)
 
@@ -213,6 +214,7 @@ struct gendisk {
 	struct kobject integrity_kobj;
 #endif	/* CONFIG_BLK_DEV_INTEGRITY */
 	int node_id;
+	struct badblocks *bb;
 };
 
 static inline struct gendisk *part_to_disk(struct hd_struct *part)
@@ -433,6 +435,11 @@ extern void add_disk(struct gendisk *disk);
 extern void del_gendisk(struct gendisk *gp);
 extern struct gendisk *get_gendisk(dev_t dev, int *partno);
 extern struct block_device *bdget_disk(struct gendisk *disk, int partno);
+int disk_alloc_badblocks(struct gendisk *disk);
+extern int disk_check_badblocks(struct gendisk *disk, sector_t s, int sectors,
+		   sector_t *first_bad, int *bad_sectors);
+extern int disk_set_badblocks(struct gendisk *disk, sector_t s, int sectors);
+extern int disk_clear_badblocks(struct gendisk *disk, sector_t s, int sectors);
 
 extern void set_device_ro(struct block_device *bdev, int flag);
 extern void set_disk_ro(struct gendisk *disk, int flag);
-- 
2.5.0


^ permalink raw reply related

* [PATCH v4 1/3] badblocks: Add core badblock management code
From: Vishal Verma @ 2015-12-08 19:18 UTC (permalink / raw)
  To: linux-nvdimm
  Cc: Vishal Verma, linux-block, linux-raid, linux-scsi, Jens Axboe,
	NeilBrown, Jeff Moyer, James Bottomley
In-Reply-To: <1449602295-28781-1-git-send-email-vishal.l.verma@intel.com>

Take the core badblocks implementation from md, and make it generally
available. This follows the same style as kernel implementations of
linked lists, rb-trees etc, where you can have a structure that can be
embedded anywhere, and accessor functions to manipulate the data.

The only changes in this copy of the code are ones to generalize
function/variable names from md-specific ones. Also add init and free
functions.

Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 block/Makefile            |   2 +-
 block/badblocks.c         | 576 ++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/badblocks.h |  53 +++++
 3 files changed, 630 insertions(+), 1 deletion(-)
 create mode 100644 block/badblocks.c
 create mode 100644 include/linux/badblocks.h

diff --git a/block/Makefile b/block/Makefile
index 00ecc97..db5f622 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -8,7 +8,7 @@ obj-$(CONFIG_BLOCK) := bio.o elevator.o blk-core.o blk-tag.o blk-sysfs.o \
 			blk-iopoll.o blk-lib.o blk-mq.o blk-mq-tag.o \
 			blk-mq-sysfs.o blk-mq-cpu.o blk-mq-cpumap.o ioctl.o \
 			genhd.o scsi_ioctl.o partition-generic.o ioprio.o \
-			partitions/
+			badblocks.o partitions/
 
 obj-$(CONFIG_BOUNCE)	+= bounce.o
 obj-$(CONFIG_BLK_DEV_BSG)	+= bsg.o
diff --git a/block/badblocks.c b/block/badblocks.c
new file mode 100644
index 0000000..f0ac279
--- /dev/null
+++ b/block/badblocks.c
@@ -0,0 +1,576 @@
+/*
+ * Bad block management
+ *
+ * - Heavily based on MD badblocks code from Neil Brown
+ *
+ * Copyright (c) 2015, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/badblocks.h>
+#include <linux/seqlock.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/stddef.h>
+#include <linux/types.h>
+#include <linux/slab.h>
+
+/**
+ * badblocks_check() - check a given range for bad sectors
+ * @bb:		the badblocks structure that holds all badblock information
+ * @s:		sector (start) at which to check for badblocks
+ * @sectors:	number of sectors to check for badblocks
+ * @first_bad:	pointer to store location of the first badblock
+ * @bad_sectors: pointer to store number of badblocks after @first_bad
+ *
+ * We can record which blocks on each device are 'bad' and so just
+ * fail those blocks, or that stripe, rather than the whole device.
+ * Entries in the bad-block table are 64bits wide.  This comprises:
+ * Length of bad-range, in sectors: 0-511 for lengths 1-512
+ * Start of bad-range, sector offset, 54 bits (allows 8 exbibytes)
+ *  A 'shift' can be set so that larger blocks are tracked and
+ *  consequently larger devices can be covered.
+ * 'Acknowledged' flag - 1 bit. - the most significant bit.
+ *
+ * Locking of the bad-block table uses a seqlock so badblocks_check
+ * might need to retry if it is very unlucky.
+ * We will sometimes want to check for bad blocks in a bi_end_io function,
+ * so we use the write_seqlock_irq variant.
+ *
+ * When looking for a bad block we specify a range and want to
+ * know if any block in the range is bad.  So we binary-search
+ * to the last range that starts at-or-before the given endpoint,
+ * (or "before the sector after the target range")
+ * then see if it ends after the given start.
+ *
+ * Return:
+ *  0: there are no known bad blocks in the range
+ *  1: there are known bad block which are all acknowledged
+ * -1: there are bad blocks which have not yet been acknowledged in metadata.
+ * plus the start/length of the first bad section we overlap.
+ */
+int badblocks_check(struct badblocks *bb, sector_t s, int sectors,
+			sector_t *first_bad, int *bad_sectors)
+{
+	int hi;
+	int lo;
+	u64 *p = bb->page;
+	int rv;
+	sector_t target = s + sectors;
+	unsigned seq;
+
+	if (bb->shift > 0) {
+		/* round the start down, and the end up */
+		s >>= bb->shift;
+		target += (1<<bb->shift) - 1;
+		target >>= bb->shift;
+		sectors = target - s;
+	}
+	/* 'target' is now the first block after the bad range */
+
+retry:
+	seq = read_seqbegin(&bb->lock);
+	lo = 0;
+	rv = 0;
+	hi = bb->count;
+
+	/* Binary search between lo and hi for 'target'
+	 * i.e. for the last range that starts before 'target'
+	 */
+	/* INVARIANT: ranges before 'lo' and at-or-after 'hi'
+	 * are known not to be the last range before target.
+	 * VARIANT: hi-lo is the number of possible
+	 * ranges, and decreases until it reaches 1
+	 */
+	while (hi - lo > 1) {
+		int mid = (lo + hi) / 2;
+		sector_t a = BB_OFFSET(p[mid]);
+
+		if (a < target)
+			/* This could still be the one, earlier ranges
+			 * could not.
+			 */
+			lo = mid;
+		else
+			/* This and later ranges are definitely out. */
+			hi = mid;
+	}
+	/* 'lo' might be the last that started before target, but 'hi' isn't */
+	if (hi > lo) {
+		/* need to check all range that end after 's' to see if
+		 * any are unacknowledged.
+		 */
+		while (lo >= 0 &&
+		       BB_OFFSET(p[lo]) + BB_LEN(p[lo]) > s) {
+			if (BB_OFFSET(p[lo]) < target) {
+				/* starts before the end, and finishes after
+				 * the start, so they must overlap
+				 */
+				if (rv != -1 && BB_ACK(p[lo]))
+					rv = 1;
+				else
+					rv = -1;
+				*first_bad = BB_OFFSET(p[lo]);
+				*bad_sectors = BB_LEN(p[lo]);
+			}
+			lo--;
+		}
+	}
+
+	if (read_seqretry(&bb->lock, seq))
+		goto retry;
+
+	return rv;
+}
+EXPORT_SYMBOL_GPL(badblocks_check);
+
+/**
+ * badblocks_set() - Add a range of bad blocks to the table.
+ * @bb:		the badblocks structure that holds all badblock information
+ * @s:		first sector to mark as bad
+ * @sectors:	number of sectors to mark as bad
+ * @acknowledged: weather to mark the bad sectors as acknowledged
+ *
+ * This might extend the table, or might contract it if two adjacent ranges
+ * can be merged. We binary-search to find the 'insertion' point, then
+ * decide how best to handle it.
+ *
+ * Return:
+ *  0: success
+ *  1: failed to set badblocks (out of space)
+ */
+int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
+			int acknowledged)
+{
+	u64 *p;
+	int lo, hi;
+	int rv = 0;
+	unsigned long flags;
+
+	if (bb->shift < 0)
+		/* badblocks are disabled */
+		return 0;
+
+	if (bb->shift) {
+		/* round the start down, and the end up */
+		sector_t next = s + sectors;
+
+		s >>= bb->shift;
+		next += (1<<bb->shift) - 1;
+		next >>= bb->shift;
+		sectors = next - s;
+	}
+
+	write_seqlock_irqsave(&bb->lock, flags);
+
+	p = bb->page;
+	lo = 0;
+	hi = bb->count;
+	/* Find the last range that starts at-or-before 's' */
+	while (hi - lo > 1) {
+		int mid = (lo + hi) / 2;
+		sector_t a = BB_OFFSET(p[mid]);
+
+		if (a <= s)
+			lo = mid;
+		else
+			hi = mid;
+	}
+	if (hi > lo && BB_OFFSET(p[lo]) > s)
+		hi = lo;
+
+	if (hi > lo) {
+		/* we found a range that might merge with the start
+		 * of our new range
+		 */
+		sector_t a = BB_OFFSET(p[lo]);
+		sector_t e = a + BB_LEN(p[lo]);
+		int ack = BB_ACK(p[lo]);
+
+		if (e >= s) {
+			/* Yes, we can merge with a previous range */
+			if (s == a && s + sectors >= e)
+				/* new range covers old */
+				ack = acknowledged;
+			else
+				ack = ack && acknowledged;
+
+			if (e < s + sectors)
+				e = s + sectors;
+			if (e - a <= BB_MAX_LEN) {
+				p[lo] = BB_MAKE(a, e-a, ack);
+				s = e;
+			} else {
+				/* does not all fit in one range,
+				 * make p[lo] maximal
+				 */
+				if (BB_LEN(p[lo]) != BB_MAX_LEN)
+					p[lo] = BB_MAKE(a, BB_MAX_LEN, ack);
+				s = a + BB_MAX_LEN;
+			}
+			sectors = e - s;
+		}
+	}
+	if (sectors && hi < bb->count) {
+		/* 'hi' points to the first range that starts after 's'.
+		 * Maybe we can merge with the start of that range
+		 */
+		sector_t a = BB_OFFSET(p[hi]);
+		sector_t e = a + BB_LEN(p[hi]);
+		int ack = BB_ACK(p[hi]);
+
+		if (a <= s + sectors) {
+			/* merging is possible */
+			if (e <= s + sectors) {
+				/* full overlap */
+				e = s + sectors;
+				ack = acknowledged;
+			} else
+				ack = ack && acknowledged;
+
+			a = s;
+			if (e - a <= BB_MAX_LEN) {
+				p[hi] = BB_MAKE(a, e-a, ack);
+				s = e;
+			} else {
+				p[hi] = BB_MAKE(a, BB_MAX_LEN, ack);
+				s = a + BB_MAX_LEN;
+			}
+			sectors = e - s;
+			lo = hi;
+			hi++;
+		}
+	}
+	if (sectors == 0 && hi < bb->count) {
+		/* we might be able to combine lo and hi */
+		/* Note: 's' is at the end of 'lo' */
+		sector_t a = BB_OFFSET(p[hi]);
+		int lolen = BB_LEN(p[lo]);
+		int hilen = BB_LEN(p[hi]);
+		int newlen = lolen + hilen - (s - a);
+
+		if (s >= a && newlen < BB_MAX_LEN) {
+			/* yes, we can combine them */
+			int ack = BB_ACK(p[lo]) && BB_ACK(p[hi]);
+
+			p[lo] = BB_MAKE(BB_OFFSET(p[lo]), newlen, ack);
+			memmove(p + hi, p + hi + 1,
+				(bb->count - hi - 1) * 8);
+			bb->count--;
+		}
+	}
+	while (sectors) {
+		/* didn't merge (it all).
+		 * Need to add a range just before 'hi'
+		 */
+		if (bb->count >= MAX_BADBLOCKS) {
+			/* No room for more */
+			rv = 1;
+			break;
+		} else {
+			int this_sectors = sectors;
+
+			memmove(p + hi + 1, p + hi,
+				(bb->count - hi) * 8);
+			bb->count++;
+
+			if (this_sectors > BB_MAX_LEN)
+				this_sectors = BB_MAX_LEN;
+			p[hi] = BB_MAKE(s, this_sectors, acknowledged);
+			sectors -= this_sectors;
+			s += this_sectors;
+		}
+	}
+
+	bb->changed = 1;
+	if (!acknowledged)
+		bb->unacked_exist = 1;
+	write_sequnlock_irqrestore(&bb->lock, flags);
+
+	return rv;
+}
+EXPORT_SYMBOL_GPL(badblocks_set);
+
+/**
+ * badblocks_clear() - Remove a range of bad blocks to the table.
+ * @bb:		the badblocks structure that holds all badblock information
+ * @s:		first sector to mark as bad
+ * @sectors:	number of sectors to mark as bad
+ *
+ * This may involve extending the table if we spilt a region,
+ * but it must not fail.  So if the table becomes full, we just
+ * drop the remove request.
+ *
+ * Return:
+ *  0: success
+ *  1: failed to clear badblocks
+ */
+int badblocks_clear(struct badblocks *bb, sector_t s, int sectors)
+{
+	u64 *p;
+	int lo, hi;
+	sector_t target = s + sectors;
+	int rv = 0;
+
+	if (bb->shift > 0) {
+		/* When clearing we round the start up and the end down.
+		 * This should not matter as the shift should align with
+		 * the block size and no rounding should ever be needed.
+		 * However it is better the think a block is bad when it
+		 * isn't than to think a block is not bad when it is.
+		 */
+		s += (1<<bb->shift) - 1;
+		s >>= bb->shift;
+		target >>= bb->shift;
+		sectors = target - s;
+	}
+
+	write_seqlock_irq(&bb->lock);
+
+	p = bb->page;
+	lo = 0;
+	hi = bb->count;
+	/* Find the last range that starts before 'target' */
+	while (hi - lo > 1) {
+		int mid = (lo + hi) / 2;
+		sector_t a = BB_OFFSET(p[mid]);
+
+		if (a < target)
+			lo = mid;
+		else
+			hi = mid;
+	}
+	if (hi > lo) {
+		/* p[lo] is the last range that could overlap the
+		 * current range.  Earlier ranges could also overlap,
+		 * but only this one can overlap the end of the range.
+		 */
+		if (BB_OFFSET(p[lo]) + BB_LEN(p[lo]) > target) {
+			/* Partial overlap, leave the tail of this range */
+			int ack = BB_ACK(p[lo]);
+			sector_t a = BB_OFFSET(p[lo]);
+			sector_t end = a + BB_LEN(p[lo]);
+
+			if (a < s) {
+				/* we need to split this range */
+				if (bb->count >= MAX_BADBLOCKS) {
+					rv = -ENOSPC;
+					goto out;
+				}
+				memmove(p+lo+1, p+lo, (bb->count - lo) * 8);
+				bb->count++;
+				p[lo] = BB_MAKE(a, s-a, ack);
+				lo++;
+			}
+			p[lo] = BB_MAKE(target, end - target, ack);
+			/* there is no longer an overlap */
+			hi = lo;
+			lo--;
+		}
+		while (lo >= 0 &&
+		       BB_OFFSET(p[lo]) + BB_LEN(p[lo]) > s) {
+			/* This range does overlap */
+			if (BB_OFFSET(p[lo]) < s) {
+				/* Keep the early parts of this range. */
+				int ack = BB_ACK(p[lo]);
+				sector_t start = BB_OFFSET(p[lo]);
+
+				p[lo] = BB_MAKE(start, s - start, ack);
+				/* now low doesn't overlap, so.. */
+				break;
+			}
+			lo--;
+		}
+		/* 'lo' is strictly before, 'hi' is strictly after,
+		 * anything between needs to be discarded
+		 */
+		if (hi - lo > 1) {
+			memmove(p+lo+1, p+hi, (bb->count - hi) * 8);
+			bb->count -= (hi - lo - 1);
+		}
+	}
+
+	bb->changed = 1;
+out:
+	write_sequnlock_irq(&bb->lock);
+	return rv;
+}
+EXPORT_SYMBOL_GPL(badblocks_clear);
+
+/**
+ * ack_all_badblocks() - Acknowledge all bad blocks in a list.
+ * @bb:		the badblocks structure that holds all badblock information
+ *
+ * This only succeeds if ->changed is clear.  It is used by
+ * in-kernel metadata updates
+ */
+void ack_all_badblocks(struct badblocks *bb)
+{
+	if (bb->page == NULL || bb->changed)
+		/* no point even trying */
+		return;
+	write_seqlock_irq(&bb->lock);
+
+	if (bb->changed == 0 && bb->unacked_exist) {
+		u64 *p = bb->page;
+		int i;
+
+		for (i = 0; i < bb->count ; i++) {
+			if (!BB_ACK(p[i])) {
+				sector_t start = BB_OFFSET(p[i]);
+				int len = BB_LEN(p[i]);
+
+				p[i] = BB_MAKE(start, len, 1);
+			}
+		}
+		bb->unacked_exist = 0;
+	}
+	write_sequnlock_irq(&bb->lock);
+}
+EXPORT_SYMBOL_GPL(ack_all_badblocks);
+
+/**
+ * badblocks_show() - sysfs access to bad-blocks list
+ * @bb:		the badblocks structure that holds all badblock information
+ * @page:	buffer received from sysfs
+ * @unack:	weather to show unacknowledged badblocks
+ *
+ * Return:
+ *  Length of returned data
+ */
+ssize_t badblocks_show(struct badblocks *bb, char *page, int unack)
+{
+	size_t len;
+	int i;
+	u64 *p = bb->page;
+	unsigned seq;
+
+	if (bb->shift < 0)
+		return 0;
+
+retry:
+	seq = read_seqbegin(&bb->lock);
+
+	len = 0;
+	i = 0;
+
+	while (len < PAGE_SIZE && i < bb->count) {
+		sector_t s = BB_OFFSET(p[i]);
+		unsigned int length = BB_LEN(p[i]);
+		int ack = BB_ACK(p[i]);
+
+		i++;
+
+		if (unack && ack)
+			continue;
+
+		len += snprintf(page+len, PAGE_SIZE-len, "%llu %u\n",
+				(unsigned long long)s << bb->shift,
+				length << bb->shift);
+	}
+	if (unack && len == 0)
+		bb->unacked_exist = 0;
+
+	if (read_seqretry(&bb->lock, seq))
+		goto retry;
+
+	return len;
+}
+EXPORT_SYMBOL_GPL(badblocks_show);
+
+/**
+ * badblocks_store() - sysfs access to bad-blocks list
+ * @bb:		the badblocks structure that holds all badblock information
+ * @page:	buffer received from sysfs
+ * @len:	length of data received from sysfs
+ * @unack:	weather to show unacknowledged badblocks
+ *
+ * Return:
+ *  Length of the buffer processed or -ve error.
+ */
+ssize_t badblocks_store(struct badblocks *bb, const char *page, size_t len,
+			int unack)
+{
+	unsigned long long sector;
+	int length;
+	char newline;
+
+	/* Allow clearing via sysfs *only* for testing/debugging.
+	 * Normally only a successful write may clear a badblock
+	 */
+	int clear = 0;
+
+	if (page[0] == '-') {
+		clear = 1;
+		page++;
+	}
+
+	switch (sscanf(page, "%llu %d%c", &sector, &length, &newline)) {
+	case 3:
+		if (newline != '\n')
+			return -EINVAL;
+	case 2:
+		if (length <= 0)
+			return -EINVAL;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (clear) {
+		badblocks_clear(bb, sector, length);
+		return len;
+	}
+
+	if (badblocks_set(bb, sector, length, !unack))
+		return -ENOSPC;
+	else
+		return len;
+}
+EXPORT_SYMBOL_GPL(badblocks_store);
+
+/**
+ * badblocks_init() - initialize the badblocks structure
+ * @bb:		the badblocks structure that holds all badblock information
+ * @enable:	weather to enable badblocks accounting
+ *
+ * Return:
+ *  0: success
+ *  -ve errno: on error
+ */
+int badblocks_init(struct badblocks *bb, int enable)
+{
+	bb->count = 0;
+	if (enable)
+		bb->shift = 0;
+	else
+		bb->shift = -1;
+	bb->page = (u64 *)__get_free_page(GFP_KERNEL);
+	if (bb->page == (u64 *)0) {
+		bb->shift = -1;
+		return -ENOMEM;
+	}
+	seqlock_init(&bb->lock);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(badblocks_init);
+
+/**
+ * badblocks_free() - free the badblocks structure
+ * @bb:		the badblocks structure that holds all badblock information
+ */
+void badblocks_free(struct badblocks *bb)
+{
+	free_page((unsigned long)bb->page);
+	bb->page = NULL;
+}
+EXPORT_SYMBOL_GPL(badblocks_free);
diff --git a/include/linux/badblocks.h b/include/linux/badblocks.h
new file mode 100644
index 0000000..9293446
--- /dev/null
+++ b/include/linux/badblocks.h
@@ -0,0 +1,53 @@
+#ifndef _LINUX_BADBLOCKS_H
+#define _LINUX_BADBLOCKS_H
+
+#include <linux/seqlock.h>
+#include <linux/kernel.h>
+#include <linux/stddef.h>
+#include <linux/types.h>
+
+#define BB_LEN_MASK	(0x00000000000001FFULL)
+#define BB_OFFSET_MASK	(0x7FFFFFFFFFFFFE00ULL)
+#define BB_ACK_MASK	(0x8000000000000000ULL)
+#define BB_MAX_LEN	512
+#define BB_OFFSET(x)	(((x) & BB_OFFSET_MASK) >> 9)
+#define BB_LEN(x)	(((x) & BB_LEN_MASK) + 1)
+#define BB_ACK(x)	(!!((x) & BB_ACK_MASK))
+#define BB_MAKE(a, l, ack) (((a)<<9) | ((l)-1) | ((u64)(!!(ack)) << 63))
+
+/* Bad block numbers are stored sorted in a single page.
+ * 64bits is used for each block or extent.
+ * 54 bits are sector number, 9 bits are extent size,
+ * 1 bit is an 'acknowledged' flag.
+ */
+#define MAX_BADBLOCKS	(PAGE_SIZE/8)
+
+struct badblocks {
+	int count;		/* count of bad blocks */
+	int unacked_exist;	/* there probably are unacknowledged
+				 * bad blocks.  This is only cleared
+				 * when a read discovers none
+				 */
+	int shift;		/* shift from sectors to block size
+				 * a -ve shift means badblocks are
+				 * disabled.*/
+	u64 *page;		/* badblock list */
+	int changed;
+	seqlock_t lock;
+	sector_t sector;
+	sector_t size;		/* in sectors */
+};
+
+int badblocks_check(struct badblocks *bb, sector_t s, int sectors,
+		   sector_t *first_bad, int *bad_sectors);
+int badblocks_set(struct badblocks *bb, sector_t s, int sectors,
+			int acknowledged);
+int badblocks_clear(struct badblocks *bb, sector_t s, int sectors);
+void ack_all_badblocks(struct badblocks *bb);
+ssize_t badblocks_show(struct badblocks *bb, char *page, int unack);
+ssize_t badblocks_store(struct badblocks *bb, const char *page, size_t len,
+			int unack);
+int badblocks_init(struct badblocks *bb, int enable);
+void badblocks_free(struct badblocks *bb);
+
+#endif
-- 
2.5.0


^ permalink raw reply related

* [PATCH v4 0/3] Badblock tracking for gendisks
From: Vishal Verma @ 2015-12-08 19:18 UTC (permalink / raw)
  To: linux-nvdimm
  Cc: Vishal Verma, linux-block, linux-raid, linux-scsi, Jens Axboe,
	NeilBrown, Jeff Moyer, James Bottomley

v4:
  - Rebase to v4.4-rc4

v3:
  - Add kernel-doc style comments to all exported functions in badblocks.c (James)
  - Make return values from badblocks functions consistent with themselves
    and the kernel style. Change the polarity of badblocks_set, and update
    all callers accordingly (James)
  - In gendisk, don't unconditionally allocate badblocks, export the initializer.
    This also allows the initializer to be a non-void return type, so that the
    badblocks user can act upon failures better (James)


v2:
  - In badblocks_free, make 'page' NULL (patch 1)
  - Move the core badblocks code to a new .c file (patch 1) (Jens)
  - Fix a sizeof usage in disk_alloc_badblocks (patch 2) (Dan)
  - Since disk_alloc_badblocks can fail, check disk->bb for NULL in the
    genhd wrappers (patch 2) (Jeff)
  - Update the md conversion to also ise the badblocks init and free
    functions (patch 3)
  - Remove the BB_* macros from md.h as they are now in badblocks.h (patch 3)

Patch 1 copies badblock management code into a header of its own,
making it generally available. It follows common libraries of code
such as linked lists, where anyone may embed a core data structure
in another place, and use the provided accessor functions to
manipulate the data.

Patch 2 adds badblock tracking to gendisks (in preparation for use
by NVDIMM devices).

Patch 3 converts md over to use the new badblocks 'library'. I have
done some pretty simple testing on this - created a raid 1 device,
made sure the sysfs entries show up, and can be used to add and view
badblocks. A closer look by the md folks would be nice here.


Vishal Verma (3):
  badblocks: Add core badblock management code
  block: Add badblock management for gendisks
  md: convert to use the generic badblocks code

 block/Makefile            |   2 +-
 block/badblocks.c         | 576 ++++++++++++++++++++++++++++++++++++++++++++++
 block/genhd.c             |  76 ++++++
 drivers/md/md.c           | 516 ++---------------------------------------
 drivers/md/md.h           |  40 +---
 include/linux/badblocks.h |  53 +++++
 include/linux/genhd.h     |   7 +
 7 files changed, 741 insertions(+), 529 deletions(-)
 create mode 100644 block/badblocks.c
 create mode 100644 include/linux/badblocks.h

-- 
2.5.0


^ permalink raw reply

* Re: dm verity: use sector_div for division
From: Mike Snitzer @ 2015-12-08 17:10 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: dm-devel, Neil Brown, linux-kernel, linux-raid, Alasdair Kergon,
	Sami Tolvanen, linux-arm-kernel
In-Reply-To: <2666833.eM84rj4CYY@wuerfel>

On Tue, Dec 08 2015 at 10:42am -0500,
Arnd Bergmann <arnd@arndb.de> wrote:

> The dm verity sec implementation uses do_div for dividing a sector_t,
> which is slower than necessary when sector_t is a 32-bit type, and
> we now get a warning for this case:
> 
> include/asm-generic/div64.h:224:22: warning: passing argument 1 of '__div64_32' from incompatible pointer type [-Wincompatible-pointer-types]
> drivers/md/dm-verity-fec.c:725:6: note: in expansion of macro 'do_div'
>   if (do_div(f->rounds, f->rsn))
> 
> This changes the code to use sector_div instead, which does the
> right thing and avoids the warning.

Thanks, I'll fold this in.

(Btw, I did review this use of do_div(): but only to answer whether there
was potential for a 64bit divide.  I concluded do_div() was fine but didn't
look to see the data type for f->rounds was actually sector_t.)

^ permalink raw reply

* [PATCH] dm verity: use sector_div for division
From: Arnd Bergmann @ 2015-12-08 15:42 UTC (permalink / raw)
  To: Mike Snitzer, dm-devel
  Cc: Alasdair Kergon, linux-raid, Neil Brown, linux-kernel,
	linux-arm-kernel, Sami Tolvanen

The dm verity sec implementation uses do_div for dividing a sector_t,
which is slower than necessary when sector_t is a 32-bit type, and
we now get a warning for this case:

include/asm-generic/div64.h:224:22: warning: passing argument 1 of '__div64_32' from incompatible pointer type [-Wincompatible-pointer-types]
drivers/md/dm-verity-fec.c:725:6: note: in expansion of macro 'do_div'
  if (do_div(f->rounds, f->rsn))

This changes the code to use sector_div instead, which does the
right thing and avoids the warning.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/md/dm-verity-fec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

I believe we had a fix for this in the series that introduced the optimized
do_div, but the file got moved around and the fix got lost.

diff --git a/drivers/md/dm-verity-fec.c b/drivers/md/dm-verity-fec.c
index e722ce57520f..1bddaca37e98 100644
--- a/drivers/md/dm-verity-fec.c
+++ b/drivers/md/dm-verity-fec.c
@@ -722,7 +722,7 @@ int verity_fec_ctr(struct dm_verity *v)
 	}
 
 	f->rounds = f->blocks;
-	if (do_div(f->rounds, f->rsn))
+	if (sector_div(f->rounds, f->rsn))
 		f->rounds++;
 
 	/*
-- 
2.1.0.rc2

^ permalink raw reply related

* Re: I/O errors without erros from underlying device
From: Arkadiusz Miskiewicz @ 2015-12-08 11:05 UTC (permalink / raw)
  To: linux-raid
In-Reply-To: <201512072146.28378.a.miskiewicz@gmail.com>

On Monday 07 of December 2015, Arkadiusz Miskiewicz wrote:

> Anyway I would expect my problem to be related to badblock lists which
> numbers are close to dmesg error message: [  848.988518] Buffer I/O error
> on dev md7, logical block 3907148544, async page read
> 
> > >> http://sprunge.us/XSWI
> 
> But how to repair these if write() also fails and
> http://www.spinics.net/lists/raid/msg49325.html suggests that write should
> "fix" these (by using replacement blocks I guess) ?

Tried to get rid of badblock lists (well, corruption in that area is better 
than no access at all):

mdadm --assemble /dev/md7 --force --update=no-bbl
mdadm: Cannot remove active bbl from /dev/sdae1
mdadm: Cannot remove active bbl from /dev/sdag1
mdadm: Cannot remove active bbl from /dev/sdai1
mdadm: Cannot remove active bbl from /dev/sdn1
mdadm: Cannot remove active bbl from /dev/sdg
mdadm: Cannot remove active bbl from /dev/sdad1
mdadm: /dev/md7 has been started with 10 drives.

Is there a way to archieve that anyway?

-- 
Arkadiusz Miśkiewicz, arekm / ( maven.pl | pld-linux.org )
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: Help Reassembling a raid5 array
From: Andrew Wilkins @ 2015-12-08  5:02 UTC (permalink / raw)
  To: Phil Turmel; +Cc: linux-raid
In-Reply-To: <56665129.6050404@turmel.org>

That was my bad I was gonna blank out all the uuids then figured it
was pointless

On 8 Dec 2015 03:40, "Phil Turmel" <philip@turmel.org> wrote:
>
> Hi Andrew,
>
> On 12/07/2015 08:26 PM, Andrew Wilkins wrote:
> > Hi, please can someone help me with a raid array which as gone wrong
> > On Friday a backup server was rebooted, but failed to boot, possibly
> > with an unrelated issue
> >
> >
> > On Monday someone booted from a usb disk to try and recover it
> > they were originally able to assemble the array but now they are not,
> > 2 out of the 5 drives are showing out of date.
> > Device 1 is showing
> >   Bad Block Log : 512 entries available at offset 72 sectors
> >   Array State : AAAAA
> > and is showing 256 less a events
> > last updated 2 hours before the others
> >
> > Device 4 is showing
> >  Array State : A.AAA
> > and 2 less events
> > last update 1 hour before the others
> >
> > The rest all show
> >  Array State : A.AA.
> >
> > I've tried manually assembling without device 1, but even with --force
> > it refused to assemble, force seems to do nothing at all, i am
> > assuming this maybe because the array state has been updated to say
> > missing for 2 devices.
>
> > The example here
> > https://raid.wiki.kernel.org/index.php/RAID_Recovery
> > seems to suggest that in my situation recreating might be my only option
>
> Possibly. Let's try all other options first.
>
> > Are there any better options before i go ahead and do this?
>
> At least one anomaly below.
>
> > /dev/sdb1:
>
> >      Array UUID : 7847e506:deaf87e5:7c6cd502:abb20cf1
>
> > /dev/sdc1:
>
> >      Array UUID : xxxxxxxx:xxxxxxxx:xxxxxxxx:abb20cf1
>
> How did this happen ? ^^^^^^^^^^^^^^^^^^^^^^
>
> > /dev/sdd1:
>
> >      Array UUID : 7847e506:deaf87e5:7c6cd502:abb20cf1
>
> > /dev/sde1:
>
> >      Array UUID : 7847e506:deaf87e5:7c6cd502:abb20cf1
>
> > /dev/sdf1:
>
> >      Array UUID : 7847e506:deaf87e5:7c6cd502:abb20cf1
>
> Try forcing re-assembly with sdb1, sdd1, sde1, and sdf1.
>
> If that doesn't work with your current environment, try again with new
> kernel and mdadm from booting a rescue environment.  I usually recommend
> the LiveCD from sysrescuecd.org.
>
> If that succeeds (in either case), add sdc1 and let it rebuild before
> doing anything else.
>
> Phil

^ permalink raw reply

* Re: I/O errors without erros from underlying device
From: John Stoffel @ 2015-12-08  4:02 UTC (permalink / raw)
  To: arekm; +Cc: John Stoffel, linux-raid
In-Reply-To: <201512072146.28378.a.miskiewicz@gmail.com>

>>>>> "Arkadiusz" == Arkadiusz Miskiewicz <a.miskiewicz@gmail.com> writes:

Arkadiusz> On Monday 07 of December 2015, John Stoffel wrote:
>> >>>>> "Arkadiusz" == Arkadiusz Miśkiewicz <arekm@maven.pl> writes:
Arkadiusz> On Monday 07 of December 2015, John Stoffel wrote:
>> 
Arkadiusz> 4.3.0 kernel, raid6 array:
>> >> I think there's a bug in the 4.3.x and 4.4-rc3 and lower with block
>> >> merges.  I ran into these over the weekend, where v4.2.6 was stable,
>> >> but anything higher would lock up and crash on me.
>> 
Arkadiusz> Well, no crashes here.
>> 
>> That's good.  It was hard(er) to hit when I wasn't running KVM VMs at
>> the same time on the server, and I was running strictly RAID1 disks,
>> so it's hard to know.
>> 
>> >> So first step would be to make sure you get and test v4.4-rc4.
>> 
Arkadiusz> Do you know which commit there?
>> 
>> Try this, from the master lkml git repository:
>> 
>> 2873d32ff493ecbfb7d2c7f56812ab941dda42f4

Arkadiusz> It's merge commit. Don't see any obvious patch in that merge that would help 
Arkadiusz> my case.

The merge from Jens Axboe talking about blk something or other.  In my
case, it lead to instant lockups.  In your case... hard to know.
Sorry. 

Arkadiusz> Anyway I would expect my problem to be related to badblock
Arkadiusz> lists which numbers are close to dmesg error message: [
Arkadiusz> 848.988518] Buffer I/O error on dev md7, logical block
Arkadiusz> 3907148544, async page read

>> >> http://sprunge.us/XSWI

Arkadiusz> But how to repair these if write() also fails and 
Arkadiusz> http://www.spinics.net/lists/raid/msg49325.html suggests that write should 
Arkadiusz> "fix" these (by using replacement blocks I guess) ?


 
Arkadiusz> md7 : active raid6 sdg[10] sdad1[9] sdac1[8] sdag1[7] sdaf1[6]
>> 
>> >> sdae1[5] sdaj1[4] sdai1[3] sdah1[2] sdn1[1] Arkadiusz>       31255089152
>> >> blocks super 1.2 level 6, 512k chunk, algorithm 2 [10/10] [UUUUUUUUUU]
>> 
Arkadiusz> bitmap: 1/30 pages [4KB], 65536KB chunk
>> 
Arkadiusz> array had weird failure where many disks went into failed state
>> 
>> >> but Arkadiusz> remove && adding these disks "fixed" it (turns out not
>> >> really fixed it).
>> 
Arkadiusz> Unfortunately now some reads fail:
>> 
Arkadiusz> pread(4, 0x1483a00, 4096, 16003680464896) = -1 EIO (Input/output
>> 
>> >> error)
>> 
Arkadiusz> To reproduce used xfs_io
Arkadiusz> xfs_io -d -c "pread 16003680464896 4096" /dev/md7
Arkadiusz> pread64: Input/output error
Arkadiusz> which does pread exactly as shown above.
>> 
Arkadiusz> write also fails for that area:
Arkadiusz> xfs_io -d -c "pwrite 16003680464896 4096" /dev/md7
Arkadiusz> pwrite64: Input/output error
>> 
Arkadiusz> Note that nothing is written in dmesg when that happens.
>> 
Arkadiusz> I've tried various offsets and sizes of pread and at some point
>> 
>> >> that was logged: Arkadiusz> [  848.988518] Buffer I/O error on dev md7,
>> >> logical block 3907148544, async page read
>> 
Arkadiusz> but no error from underlying devices.
>> 
Arkadiusz> List of bad blocks:
Arkadiusz> http://sprunge.us/XSWI
>> 
Arkadiusz> What can I do now?
>> 
Arkadiusz> (loosing data from that few sectors is acceptable if the rest
>> 
>> >> will be readable)
>> 
Arkadiusz> Thanks,
Arkadiusz> --
Arkadiusz> Arkadiusz Miśkiewicz, arekm / ( maven.pl | pld-linux.org )
Arkadiusz> --
Arkadiusz> To unsubscribe from this list: send the line "unsubscribe
>> 
>> >> linux-raid" in Arkadiusz> the body of a message to
>> >> majordomo@vger.kernel.org
>> 
Arkadiusz> More majordomo info at
>> 
>> >> http://vger.kernel.org/majordomo-info.html
>> 
Arkadiusz> --
Arkadiusz> Arkadiusz Miśkiewicz, arekm / ( maven.pl | pld-linux.org )


Arkadiusz> -- 
Arkadiusz> Arkadiusz Miśkiewicz, arekm / ( maven.pl | pld-linux.org )

Arkadiusz> -- 
Arkadiusz> Arkadiusz Miśkiewicz, arekm / ( maven.pl | pld-linux.org )
Arkadiusz> --
Arkadiusz> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
Arkadiusz> the body of a message to majordomo@vger.kernel.org
Arkadiusz> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: Help Reassembling a raid5 array
From: Phil Turmel @ 2015-12-08  3:40 UTC (permalink / raw)
  To: Andrew Wilkins, linux-raid
In-Reply-To: <CAJGWEk93QAmCdik4_rxGPcq7dm=2tXX1U19Zkn5OwcAog0Z-RA@mail.gmail.com>

Hi Andrew,

On 12/07/2015 08:26 PM, Andrew Wilkins wrote:
> Hi, please can someone help me with a raid array which as gone wrong
> On Friday a backup server was rebooted, but failed to boot, possibly
> with an unrelated issue
> 
> 
> On Monday someone booted from a usb disk to try and recover it
> they were originally able to assemble the array but now they are not,
> 2 out of the 5 drives are showing out of date.
> Device 1 is showing
>   Bad Block Log : 512 entries available at offset 72 sectors
>   Array State : AAAAA
> and is showing 256 less a events
> last updated 2 hours before the others
> 
> Device 4 is showing
>  Array State : A.AAA
> and 2 less events
> last update 1 hour before the others
> 
> The rest all show
>  Array State : A.AA.
> 
> I've tried manually assembling without device 1, but even with --force
> it refused to assemble, force seems to do nothing at all, i am
> assuming this maybe because the array state has been updated to say
> missing for 2 devices.

> The example here
> https://raid.wiki.kernel.org/index.php/RAID_Recovery
> seems to suggest that in my situation recreating might be my only option

Possibly. Let's try all other options first.

> Are there any better options before i go ahead and do this?

At least one anomaly below.

> /dev/sdb1:

>      Array UUID : 7847e506:deaf87e5:7c6cd502:abb20cf1

> /dev/sdc1:

>      Array UUID : xxxxxxxx:xxxxxxxx:xxxxxxxx:abb20cf1

How did this happen ? ^^^^^^^^^^^^^^^^^^^^^^

> /dev/sdd1:

>      Array UUID : 7847e506:deaf87e5:7c6cd502:abb20cf1

> /dev/sde1:

>      Array UUID : 7847e506:deaf87e5:7c6cd502:abb20cf1

> /dev/sdf1:

>      Array UUID : 7847e506:deaf87e5:7c6cd502:abb20cf1

Try forcing re-assembly with sdb1, sdd1, sde1, and sdf1.

If that doesn't work with your current environment, try again with new
kernel and mdadm from booting a rescue environment.  I usually recommend
the LiveCD from sysrescuecd.org.

If that succeeds (in either case), add sdc1 and let it rebuild before
doing anything else.

Phil

^ 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