Linux RAID subsystem development
 help / color / mirror / Atom feed
* Re: 4.11.2: reshape raid5 -> raid6 atop bcache deadlocks at start on md_attr_store / raid5_make_request
From: NeilBrown @ 2017-05-25  1:30 UTC (permalink / raw)
  To: Shaohua Li; +Cc: Nix, linux-raid
In-Reply-To: <20170524225735.555dpfe24mi6yxrb@kernel.org>

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

On Wed, May 24 2017, Shaohua Li wrote:

> On Wed, May 24, 2017 at 11:24:21AM +1000, Neil Brown wrote:
>> 
>> 
>> 
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 10367ffe92e3..a7b9c0576479 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -324,8 +324,12 @@ static blk_qc_t md_make_request(struct request_queue *q, struct bio *bio)
>>  void mddev_suspend(struct mddev *mddev)
>>  {
>>  	WARN_ON_ONCE(mddev->thread && current == mddev->thread->tsk);
>> +
>>  	if (mddev->suspended++)
>>  		return;
>> +#ifdef CONFIG_LOCKDEP
>> +	WARN_ON_ONCE(debug_locks && lockdep_is_held(&mddev->reconfig_mutex));
>> +#endif
>>  	synchronize_rcu();
>>  	wait_event(mddev->sb_wait, atomic_read(&mddev->active_io) == 0);
>>  	mddev->pers->quiesce(mddev, 1);
>> @@ -3594,9 +3598,12 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
>>  	if (slen == 0 || slen >= sizeof(clevel))
>>  		return -EINVAL;
>>  
>> +	mddev_suspend(mddev);
>>  	rv = mddev_lock(mddev);
>> -	if (rv)
>> +	if (rv) {
>> +		mddev_resume(mddev);
>>  		return rv;
>> +	}
>>  
>>  	if (mddev->pers == NULL) {
>>  		strncpy(mddev->clevel, buf, slen);
>> @@ -3687,7 +3694,6 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
>>  	}
>>  
>>  	/* Looks like we have a winner */
>> -	mddev_suspend(mddev);
>>  	mddev_detach(mddev);
>>  
>>  	spin_lock(&mddev->lock);
>> @@ -3771,13 +3777,13 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
>>  	blk_set_stacking_limits(&mddev->queue->limits);
>>  	pers->run(mddev);
>>  	set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
>> -	mddev_resume(mddev);
>>  	if (!mddev->thread)
>>  		md_update_sb(mddev, 1);
>>  	sysfs_notify(&mddev->kobj, NULL, "level");
>>  	md_new_event(mddev);
>>  	rv = len;
>>  out_unlock:
>> +	mddev_resume(mddev);
>>  	mddev_unlock(mddev);
>>  	return rv;
>>  }
>> @@ -4490,6 +4496,7 @@ action_store(struct mddev *mddev, const char *page, size_t len)
>>  		int err;
>>  		if (mddev->pers->start_reshape == NULL)
>>  			return -EINVAL;
>> +		mddev_suspend(mddev);
>>  		err = mddev_lock(mddev);
>>  		if (!err) {
>>  			if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery))
>> @@ -4500,6 +4507,7 @@ action_store(struct mddev *mddev, const char *page, size_t len)
>>  			}
>>  			mddev_unlock(mddev);
>>  		}
>> +		mddev_resume(mddev);
>>  		if (err)
>>  			return err;
>>  		sysfs_notify(&mddev->kobj, NULL, "degraded");
>
> The analysis makes a lot of sense, thanks! The patch looks not solving the
> problem though, because check_recovery will not write super if suspended isn't
> 0.

Why does that matter?  In what case do you need the superblock to be
written, but it doesn't happen.

check_recovery won't write the superblock while the mddev is locked
either, and it is locked for most of level_store().
When level_store() finished, it unlocks the device and that will trigger
md_check_recovery() to be run, and the metadata will be written then.
I don't think there is a particular need to update it before then.

Thanks,
NeilBrown


>
> I'm thinking of alternative wayt to fix this issue. A request which stalls in
> md_write_start reallys isn't an active IO. We could add another counter in
> md_write_start to record stalled request there. Then in mddev_suspend, wait
> event will wait for atomic_read(&mddev->active_io) == the_new_counter.
>
> Thanks,
> Shaohua

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

^ permalink raw reply

* Re: 4.11.2: reshape raid5 -> raid6 atop bcache deadlocks at start on md_attr_store / raid5_make_request
From: NeilBrown @ 2017-05-25  1:31 UTC (permalink / raw)
  To: Nix; +Cc: linux-raid
In-Reply-To: <87bmqio03i.fsf@esperi.org.uk>

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

On Wed, May 24 2017, Nix wrote:
>
> (I'm not sure what to do if it *works* -- how do I test any later
> changes? I might reshape back to RAID-5 + spare again just so I can test
> later stuff, but that would take ages: the array is over 14TiB...)
>

If it works (which it did), then we'll have enough understanding that it
should be easy to simulate the problem accurately in a VM.
I don't think there is any need for you to experiment further on your
data.

Thanks,
NeilBrown

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

^ permalink raw reply

* Re: 4.11.2: reshape raid5 -> raid6 atop bcache deadlocks at start on md_attr_store / raid5_make_request
From: Shaohua Li @ 2017-05-25  1:46 UTC (permalink / raw)
  To: NeilBrown; +Cc: Nix, linux-raid
In-Reply-To: <877f15iuzf.fsf@notabene.neil.brown.name>

On Thu, May 25, 2017 at 11:30:12AM +1000, Neil Brown wrote:
> On Wed, May 24 2017, Shaohua Li wrote:
> 
> > On Wed, May 24, 2017 at 11:24:21AM +1000, Neil Brown wrote:
> >> 
> >> 
> >> 
> >> diff --git a/drivers/md/md.c b/drivers/md/md.c
> >> index 10367ffe92e3..a7b9c0576479 100644
> >> --- a/drivers/md/md.c
> >> +++ b/drivers/md/md.c
> >> @@ -324,8 +324,12 @@ static blk_qc_t md_make_request(struct request_queue *q, struct bio *bio)
> >>  void mddev_suspend(struct mddev *mddev)
> >>  {
> >>  	WARN_ON_ONCE(mddev->thread && current == mddev->thread->tsk);
> >> +
> >>  	if (mddev->suspended++)
> >>  		return;
> >> +#ifdef CONFIG_LOCKDEP
> >> +	WARN_ON_ONCE(debug_locks && lockdep_is_held(&mddev->reconfig_mutex));
> >> +#endif
> >>  	synchronize_rcu();
> >>  	wait_event(mddev->sb_wait, atomic_read(&mddev->active_io) == 0);
> >>  	mddev->pers->quiesce(mddev, 1);
> >> @@ -3594,9 +3598,12 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
> >>  	if (slen == 0 || slen >= sizeof(clevel))
> >>  		return -EINVAL;
> >>  
> >> +	mddev_suspend(mddev);
> >>  	rv = mddev_lock(mddev);
> >> -	if (rv)
> >> +	if (rv) {
> >> +		mddev_resume(mddev);
> >>  		return rv;
> >> +	}
> >>  
> >>  	if (mddev->pers == NULL) {
> >>  		strncpy(mddev->clevel, buf, slen);
> >> @@ -3687,7 +3694,6 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
> >>  	}
> >>  
> >>  	/* Looks like we have a winner */
> >> -	mddev_suspend(mddev);
> >>  	mddev_detach(mddev);
> >>  
> >>  	spin_lock(&mddev->lock);
> >> @@ -3771,13 +3777,13 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
> >>  	blk_set_stacking_limits(&mddev->queue->limits);
> >>  	pers->run(mddev);
> >>  	set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
> >> -	mddev_resume(mddev);
> >>  	if (!mddev->thread)
> >>  		md_update_sb(mddev, 1);
> >>  	sysfs_notify(&mddev->kobj, NULL, "level");
> >>  	md_new_event(mddev);
> >>  	rv = len;
> >>  out_unlock:
> >> +	mddev_resume(mddev);
> >>  	mddev_unlock(mddev);
> >>  	return rv;
> >>  }
> >> @@ -4490,6 +4496,7 @@ action_store(struct mddev *mddev, const char *page, size_t len)
> >>  		int err;
> >>  		if (mddev->pers->start_reshape == NULL)
> >>  			return -EINVAL;
> >> +		mddev_suspend(mddev);
> >>  		err = mddev_lock(mddev);
> >>  		if (!err) {
> >>  			if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery))
> >> @@ -4500,6 +4507,7 @@ action_store(struct mddev *mddev, const char *page, size_t len)
> >>  			}
> >>  			mddev_unlock(mddev);
> >>  		}
> >> +		mddev_resume(mddev);
> >>  		if (err)
> >>  			return err;
> >>  		sysfs_notify(&mddev->kobj, NULL, "degraded");
> >
> > The analysis makes a lot of sense, thanks! The patch looks not solving the
> > problem though, because check_recovery will not write super if suspended isn't
> > 0.
> 
> Why does that matter?  In what case do you need the superblock to be
> written, but it doesn't happen.
> 
> check_recovery won't write the superblock while the mddev is locked
> either, and it is locked for most of level_store().
> When level_store() finished, it unlocks the device and that will trigger
> md_check_recovery() to be run, and the metadata will be written then.
> I don't think there is a particular need to update it before then.

I get confused. md_write_start is waiting for MD_SB_CHANGE_PENDING cleared,
which is done in check_recovery. Your previous email describes this too.

Thanks,
Shaohua

^ permalink raw reply

* Re: Reducing the number of devices in a degraded RAID-5
From: Andreas Klauer @ 2017-05-25  7:24 UTC (permalink / raw)
  To: NeilBrown; +Cc: linux-raid
In-Reply-To: <87h90bhujz.fsf@notabene.neil.brown.name>

On Wed, May 24, 2017 at 12:12:32PM +1000, NeilBrown wrote:
> What does work is:
>  # start with a degraded array, device 0 missing
>  mdadm --grow /dev/md42 --array-size=.....
>  echo frozen > /sys/block/md42/md/sync_action
>  mdadm /dev/md42 --add /dev/loop0
>  echo 0 > /sys/block/md42/md/dev-loop0/slot
>  mdadm --grow /dev/md42 --backup-file=... --raid-devices=4

Wow. Thanks.

This should merge Step 1+2, but not Step 3, right?

I really need to take a closer look at the things in /sys/.../md/...

Seems like you can do great things with it... terrible, yes, but great.

(
  Step 3 would be turning slot 4 to-be-spare into slot 0, 
  without --add ing another device at all.

  That's what would happen if /dev/loop0 was actually backed 
  by and thus identical with the slot4 device.

  But that's playing dirty.
)

Regards
Andreas Klauer

^ permalink raw reply

* Re: [PATCH RESEND] md: Make flush bios explicitely sync
From: Jan Kara @ 2017-05-25  8:11 UTC (permalink / raw)
  To: Shaohua Li; +Cc: Jan Kara, linux-raid, Mike Snitzer, dm-devel
In-Reply-To: <20170524232236.yrmslb4upgoa7kxb@kernel.org>

On Wed 24-05-17 16:22:36, Shaohua Li wrote:
> On Wed, May 24, 2017 at 01:40:13PM +0200, Jan Kara wrote:
> > Commit b685d3d65ac7 "block: treat REQ_FUA and REQ_PREFLUSH as
> > synchronous" removed REQ_SYNC flag from WRITE_{FUA|PREFLUSH|...}
> > definitions.  generic_make_request_checks() however strips REQ_FUA and
> > REQ_PREFLUSH flags from a bio when the storage doesn't report volatile
> > write cache and thus write effectively becomes asynchronous which can
> > lead to performance regressions
> > 
> > Fix the problem by making sure all bios which are synchronous are
> > properly marked with REQ_SYNC.
> 
> DM and MD are different trees, so probably you should separate them to 2
> patches.

OK, I can do that.

> For the md part (md.c, raid5-cache.c), some placed which use REQ_FUA
> are missed, like raid5.c and raid5-ppl.c

So ops_run_io() in raid5.c only copy REQ_FUA from some internal raid5
flags. My thinking was that we want to just propagate whatever we were
instructed to do here.

The case in ppl_write_empty_header() is clearly missed, I'll fix that.
Thanks. I'm not quite sure about ppl_submit_iounit() - I don't see a place
where we are waiting for those bios to complete. If it is likely to happen
soon after bio submission, we should add REQ_SYNC there.

> Can't remember if others asked the question in your first post, sorry,
> but why we don't add REQ_SYNC in generic_make_request_checks() if we are
> going to stripe REQ_FUA, REQ_PREFLUSH. That will be less error prone.

Well, strictly speaking users of REQ_FUA do not necessarily have to use
REQ_SYNC. These are two different orthogonal things - one is a request for
bypassing disk cache, the other is a hint to the IO scheduler that there is
someone waiting for the IO to complete. Most of the time you wait for
REQ_FUA request immediately but I can see some uses in filesystems
where we might want to submit REQ_FUA request in the background (like when
doing background cleaning of the journal).

								Honza

> > CC: linux-raid@vger.kernel.org
> > CC: Shaohua Li <shli@kernel.org>
> > CC: Mike Snitzer <snitzer@redhat.com>
> > CC: dm-devel@redhat.com
> > Fixes: b685d3d65ac791406e0dfd8779cc9b3707fea5a3
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> >  drivers/md/dm-snap-persistent.c | 3 ++-
> >  drivers/md/md.c                 | 2 +-
> >  drivers/md/raid5-cache.c        | 4 ++--
> >  3 files changed, 5 insertions(+), 4 deletions(-)
> > 
> > Guys, I don't know enough about DM/MD to judge whether I've identified all the
> > places that want REQ_SYNC right. Can you please have a look?
> > 
> > diff --git a/drivers/md/dm-snap-persistent.c b/drivers/md/dm-snap-persistent.c
> > index b93476c3ba3f..b92ab4cb0710 100644
> > --- a/drivers/md/dm-snap-persistent.c
> > +++ b/drivers/md/dm-snap-persistent.c
> > @@ -741,7 +741,8 @@ static void persistent_commit_exception(struct dm_exception_store *store,
> >  	/*
> >  	 * Commit exceptions to disk.
> >  	 */
> > -	if (ps->valid && area_io(ps, REQ_OP_WRITE, REQ_PREFLUSH | REQ_FUA))
> > +	if (ps->valid && area_io(ps, REQ_OP_WRITE,
> > +				 REQ_SYNC | REQ_PREFLUSH | REQ_FUA))
> >  		ps->valid = 0;
> >  
> >  	/*
> > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > index 10367ffe92e3..212a6777ff31 100644
> > --- a/drivers/md/md.c
> > +++ b/drivers/md/md.c
> > @@ -765,7 +765,7 @@ void md_super_write(struct mddev *mddev, struct md_rdev *rdev,
> >  	    test_bit(FailFast, &rdev->flags) &&
> >  	    !test_bit(LastDev, &rdev->flags))
> >  		ff = MD_FAILFAST;
> > -	bio->bi_opf = REQ_OP_WRITE | REQ_PREFLUSH | REQ_FUA | ff;
> > +	bio->bi_opf = REQ_OP_WRITE | REQ_SYNC | REQ_PREFLUSH | REQ_FUA | ff;
> >  
> >  	atomic_inc(&mddev->pending_writes);
> >  	submit_bio(bio);
> > diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
> > index 4c00bc248287..0a7af8b0a80a 100644
> > --- a/drivers/md/raid5-cache.c
> > +++ b/drivers/md/raid5-cache.c
> > @@ -1782,7 +1782,7 @@ static int r5l_log_write_empty_meta_block(struct r5l_log *log, sector_t pos,
> >  	mb->checksum = cpu_to_le32(crc32c_le(log->uuid_checksum,
> >  					     mb, PAGE_SIZE));
> >  	if (!sync_page_io(log->rdev, pos, PAGE_SIZE, page, REQ_OP_WRITE,
> > -			  REQ_FUA, false)) {
> > +			  REQ_SYNC | REQ_FUA, false)) {
> >  		__free_page(page);
> >  		return -EIO;
> >  	}
> > @@ -2388,7 +2388,7 @@ r5c_recovery_rewrite_data_only_stripes(struct r5l_log *log,
> >  		mb->checksum = cpu_to_le32(crc32c_le(log->uuid_checksum,
> >  						     mb, PAGE_SIZE));
> >  		sync_page_io(log->rdev, ctx->pos, PAGE_SIZE, page,
> > -			     REQ_OP_WRITE, REQ_FUA, false);
> > +			     REQ_OP_WRITE, REQ_SYNC | REQ_FUA, false);
> >  		sh->log_start = ctx->pos;
> >  		list_add_tail(&sh->r5c, &log->stripe_in_journal_list);
> >  		atomic_inc(&log->stripe_in_journal_count);
> > -- 
> > 2.12.0
> > 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply

* [Patch v5] super1: fix sb->max_dev when adding a new disk in linear array
From: Lidong Zhong @ 2017-05-25  9:28 UTC (permalink / raw)
  To: linux-raid, neilb; +Cc: colyli, Jes.Sorensen

The value of sb->max_dev will always be increased by 1 when adding
a new disk in linear array. It causes an inconsistence between each
disk in the array and the "Array State" value of "mdadm --examine DISK"
is wrong. For example, when adding the first new disk into linear array
it will be:

Array State : RAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
('A' == active, '.' == missing, 'R' == replacing)

Adding the second disk into linear array it will be

Array State : .AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
('A' == active, '.' == missing, 'R' == replacing)

Signed-off-by: Lidong Zhong <lzhong@suse.com>
---
 super1.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/super1.c b/super1.c
index 2fcb814..86ec850 100644
--- a/super1.c
+++ b/super1.c
@@ -1267,8 +1267,9 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
 				break;
 		sb->dev_number = __cpu_to_le32(i);
 		info->disk.number = i;
-		if (max >= __le32_to_cpu(sb->max_dev))
+		if (i >= max) {
 			sb->max_dev = __cpu_to_le32(max+1);
+		}
 
 		random_uuid(sb->device_uuid);
 
@@ -1293,7 +1294,11 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
 			}
 		}
 	} else if (strcmp(update, "linear-grow-update") == 0) {
+		int max = __le32_to_cpu(sb->max_dev);
 		sb->raid_disks = __cpu_to_le32(info->array.raid_disks);
+		if (info->array.raid_disks > max) {
+			sb->max_dev = __cpu_to_le32(max+1);
+		}
 		sb->dev_roles[info->disk.number] =
 			__cpu_to_le16(info->disk.raid_disk);
 	} else if (strcmp(update, "resync") == 0) {
-- 
2.12.0


^ permalink raw reply related

* Re: 4.11.2: reshape raid5 -> raid6 atop bcache deadlocks at start on md_attr_store / raid5_make_request
From: Nix @ 2017-05-25 12:14 UTC (permalink / raw)
  To: NeilBrown; +Cc: linux-raid
In-Reply-To: <874lw9iuwn.fsf@notabene.neil.brown.name>

On 25 May 2017, NeilBrown spake thusly:

> On Wed, May 24 2017, Nix wrote:
>>
>> (I'm not sure what to do if it *works* -- how do I test any later
>> changes? I might reshape back to RAID-5 + spare again just so I can test
>> later stuff, but that would take ages: the array is over 14TiB...)
>
> If it works (which it did), then we'll have enough understanding that it
> should be easy to simulate the problem accurately in a VM.
> I don't think there is any need for you to experiment further on your
> data.

If you need me to, I can -- this machine isn't commissioned yet (though
the plan *is* to do so next weekend, I can put it off: I've put it off
before...)

Plus I have plenty of backups. :)

-- 
NULL && (void)

^ permalink raw reply

* Re: [PATCH 10/23] afs: switch to use uuid_t and uuid_gen
From: Christoph Hellwig @ 2017-05-25 13:00 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Mimi Zohar, Amir Goldstein,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Steven Whitehouse,
	David Howells, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, Shaohua Li,
	Christoph Hellwig
In-Reply-To: <1495545099.6967.81.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

On Tue, May 23, 2017 at 04:11:39PM +0300, Andy Shevchenko wrote:
> Since we introduced a union it's possible that we might access the
> member which wasn't last modified one. So, my comment is to give an
> attention on such possibility and avoid if there is an aliasing
> happened.

We do for AFS (and XFS for fs fsid).  My preference would be to
not have the v1 struct defintion but instead provide a few
helpers in uuid.h that use get_unaligned_be* if needed:

	uuid_v1_time_low()
	uuid_v1_time_mid()
	uuid_v1_time_time_hi_and_version()..

From his previously reply it seems like Dave doesn't like that idea
too much, in which case I suspect moving struct uuid_v1 back into
afs and living with cast in it is the way to go.

^ permalink raw reply

* Re: [PATCH 10/23] afs: switch to use uuid_t and uuid_gen
From: Andy Shevchenko @ 2017-05-25 13:29 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Mimi Zohar, Amir Goldstein,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, Shaohua Li,
	Steven Whitehouse
In-Reply-To: <20170525130027.GA30194-jcswGhMUV9g@public.gmane.org>

On Thu, 2017-05-25 at 15:00 +0200, Christoph Hellwig wrote:
> On Tue, May 23, 2017 at 04:11:39PM +0300, Andy Shevchenko wrote:
> > Since we introduced a union it's possible that we might access the
> > member which wasn't last modified one. So, my comment is to give an
> > attention on such possibility and avoid if there is an aliasing
> > happened.
> 
> We do for AFS (and XFS for fs fsid).  My preference would be to
> not have the v1 struct defintion but instead provide a few
> helpers in uuid.h that use get_unaligned_be* if needed:
> 
> 	uuid_v1_time_low()
> 	uuid_v1_time_mid()
> 	uuid_v1_time_time_hi_and_version()..
> 
> From his previously reply it seems like Dave doesn't like that idea
> too much, in which case I suspect moving struct uuid_v1 back into
> afs and living with cast in it is the way to go.

Personally I don't like that union stuff, so, definitely my vote to get
rid of AFS stuff in generic helpers.

OTOH if there will be more users of such API then something like you
proposed would be sufficient without introducing a union.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

^ permalink raw reply

* Re: [Patch v5] super1: fix sb->max_dev when adding a new disk in linear array
From: NeilBrown @ 2017-05-25 21:51 UTC (permalink / raw)
  To: Lidong Zhong, linux-raid; +Cc: colyli, Jes.Sorensen
In-Reply-To: <20170525092811.3868-1-lzhong@suse.com>

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

On Thu, May 25 2017, Lidong Zhong wrote:

> The value of sb->max_dev will always be increased by 1 when adding
> a new disk in linear array. It causes an inconsistence between each
> disk in the array and the "Array State" value of "mdadm --examine DISK"
> is wrong. For example, when adding the first new disk into linear array
> it will be:
>
> Array State : RAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
> ('A' == active, '.' == missing, 'R' == replacing)
>
> Adding the second disk into linear array it will be
>
> Array State : .AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
> ('A' == active, '.' == missing, 'R' == replacing)
>
> Signed-off-by: Lidong Zhong <lzhong@suse.com>

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

Thanks,
NeilBrown

> ---
>  super1.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/super1.c b/super1.c
> index 2fcb814..86ec850 100644
> --- a/super1.c
> +++ b/super1.c
> @@ -1267,8 +1267,9 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
>  				break;
>  		sb->dev_number = __cpu_to_le32(i);
>  		info->disk.number = i;
> -		if (max >= __le32_to_cpu(sb->max_dev))
> +		if (i >= max) {
>  			sb->max_dev = __cpu_to_le32(max+1);
> +		}
>  
>  		random_uuid(sb->device_uuid);
>  
> @@ -1293,7 +1294,11 @@ static int update_super1(struct supertype *st, struct mdinfo *info,
>  			}
>  		}
>  	} else if (strcmp(update, "linear-grow-update") == 0) {
> +		int max = __le32_to_cpu(sb->max_dev);
>  		sb->raid_disks = __cpu_to_le32(info->array.raid_disks);
> +		if (info->array.raid_disks > max) {
> +			sb->max_dev = __cpu_to_le32(max+1);
> +		}
>  		sb->dev_roles[info->disk.number] =
>  			__cpu_to_le16(info->disk.raid_disk);
>  	} else if (strcmp(update, "resync") == 0) {
> -- 
> 2.12.0

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

^ permalink raw reply

* [Patch v1] Detail: fix a bug when trying to show the detail of a raid device
From: Lidong Zhong @ 2017-05-26  2:42 UTC (permalink / raw)
  To: linux-raid; +Cc: colyli, Jes.Sorensen, neilb

It's introduced from commit a4dcdb23ea63.

Signed-off-by: Lidong Zhong <lzhong@suse.com>
---
 Detail.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Detail.c b/Detail.c
index bf881ff..c89b927 100644
--- a/Detail.c
+++ b/Detail.c
@@ -98,8 +98,8 @@ int Detail(char *dev, struct context *c)
 		    sra->array.minor_version == -2);
 	inactive = (sra->array_state == ARRAY_ACTIVE ||
 		    sra->array_state == ARRAY_CLEAR);
-	st = super_by_fd(fd, &subarray);
-	if (md_get_array_info(fd, &array) && errno == ENODEV) {
+	rv = md_get_array_info(fd, &array);
+	if (rv && errno == ENODEV) {
 		if (sra->array.major_version == -1 &&
 		    sra->array.minor_version == -1 &&
 		    sra->devs == NULL) {
@@ -109,7 +109,7 @@ int Detail(char *dev, struct context *c)
 			return rv;
 		}
 		array = sra->array;
-	} else {
+	} else if (rv) {
 		pr_err("cannot get array detail for %s: %s\n",
 		       dev, strerror(errno));
 		close(fd);
@@ -118,8 +118,8 @@ int Detail(char *dev, struct context *c)
 
 	if (fstat(fd, &stb) != 0 && !S_ISBLK(stb.st_mode))
 		stb.st_rdev = 0;
-	rv = 0;
 
+	st = super_by_fd(fd, &subarray);
 	if (st)
 		max_disks = st->max_devs;
 
-- 
2.12.0


^ permalink raw reply related

* Re: 4.11.2: reshape raid5 -> raid6 atop bcache deadlocks at start on md_attr_store / raid5_make_request
From: NeilBrown @ 2017-05-26  3:23 UTC (permalink / raw)
  To: Shaohua Li; +Cc: Nix, linux-raid
In-Reply-To: <20170525014647.vwauunwyizjpdl5i@kernel.org>

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

On Wed, May 24 2017, Shaohua Li wrote:

> On Thu, May 25, 2017 at 11:30:12AM +1000, Neil Brown wrote:
>> On Wed, May 24 2017, Shaohua Li wrote:
>> 
>> > On Wed, May 24, 2017 at 11:24:21AM +1000, Neil Brown wrote:
>> >> 
>> >> 
>> >> 
>> >> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> >> index 10367ffe92e3..a7b9c0576479 100644
>> >> --- a/drivers/md/md.c
>> >> +++ b/drivers/md/md.c
>> >> @@ -324,8 +324,12 @@ static blk_qc_t md_make_request(struct request_queue *q, struct bio *bio)
>> >>  void mddev_suspend(struct mddev *mddev)
>> >>  {
>> >>  	WARN_ON_ONCE(mddev->thread && current == mddev->thread->tsk);
>> >> +
>> >>  	if (mddev->suspended++)
>> >>  		return;
>> >> +#ifdef CONFIG_LOCKDEP
>> >> +	WARN_ON_ONCE(debug_locks && lockdep_is_held(&mddev->reconfig_mutex));
>> >> +#endif
>> >>  	synchronize_rcu();
>> >>  	wait_event(mddev->sb_wait, atomic_read(&mddev->active_io) == 0);
>> >>  	mddev->pers->quiesce(mddev, 1);
>> >> @@ -3594,9 +3598,12 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
>> >>  	if (slen == 0 || slen >= sizeof(clevel))
>> >>  		return -EINVAL;
>> >>  
>> >> +	mddev_suspend(mddev);
>> >>  	rv = mddev_lock(mddev);
>> >> -	if (rv)
>> >> +	if (rv) {
>> >> +		mddev_resume(mddev);
>> >>  		return rv;
>> >> +	}
>> >>  
>> >>  	if (mddev->pers == NULL) {
>> >>  		strncpy(mddev->clevel, buf, slen);
>> >> @@ -3687,7 +3694,6 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
>> >>  	}
>> >>  
>> >>  	/* Looks like we have a winner */
>> >> -	mddev_suspend(mddev);
>> >>  	mddev_detach(mddev);
>> >>  
>> >>  	spin_lock(&mddev->lock);
>> >> @@ -3771,13 +3777,13 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
>> >>  	blk_set_stacking_limits(&mddev->queue->limits);
>> >>  	pers->run(mddev);
>> >>  	set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
>> >> -	mddev_resume(mddev);
>> >>  	if (!mddev->thread)
>> >>  		md_update_sb(mddev, 1);
>> >>  	sysfs_notify(&mddev->kobj, NULL, "level");
>> >>  	md_new_event(mddev);
>> >>  	rv = len;
>> >>  out_unlock:
>> >> +	mddev_resume(mddev);
>> >>  	mddev_unlock(mddev);
>> >>  	return rv;
>> >>  }
>> >> @@ -4490,6 +4496,7 @@ action_store(struct mddev *mddev, const char *page, size_t len)
>> >>  		int err;
>> >>  		if (mddev->pers->start_reshape == NULL)
>> >>  			return -EINVAL;
>> >> +		mddev_suspend(mddev);
>> >>  		err = mddev_lock(mddev);
>> >>  		if (!err) {
>> >>  			if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery))
>> >> @@ -4500,6 +4507,7 @@ action_store(struct mddev *mddev, const char *page, size_t len)
>> >>  			}
>> >>  			mddev_unlock(mddev);
>> >>  		}
>> >> +		mddev_resume(mddev);
>> >>  		if (err)
>> >>  			return err;
>> >>  		sysfs_notify(&mddev->kobj, NULL, "degraded");
>> >
>> > The analysis makes a lot of sense, thanks! The patch looks not solving the
>> > problem though, because check_recovery will not write super if suspended isn't
>> > 0.
>> 
>> Why does that matter?  In what case do you need the superblock to be
>> written, but it doesn't happen.
>> 
>> check_recovery won't write the superblock while the mddev is locked
>> either, and it is locked for most of level_store().
>> When level_store() finished, it unlocks the device and that will trigger
>> md_check_recovery() to be run, and the metadata will be written then.
>> I don't think there is a particular need to update it before then.
>
> I get confused. md_write_start is waiting for MD_SB_CHANGE_PENDING cleared,
> which is done in check_recovery. Your previous email describes this too.

Uhmm.... yes.  I see your point now.  Maybe we need might first patch as
well, so md_check_recovery() can still update the superblock after
->suspended is set.

However, I starting looking at making sure mddev_suspend() was never
called with mddev_lock() held, and that isn't straight forward.
Most callers of mddev_suspend() do hold the lock.
The only exceptions I found were:
  dm-raid.c in raid_postsuspend()
  raid5-cache.c in r5c_disable_writeback_async() and
  r5c_journal_mode_set().

It might be easiest to change all of those to lock the mddev, then
do the md_update_sb in mddev_suspend, when needed.

i.e. something like the following.  Thoughts?

Thanks,
NeilBrown


diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index 7d893228c40f..db79bd22418b 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -3607,8 +3607,11 @@ static void raid_postsuspend(struct dm_target *ti)
 {
 	struct raid_set *rs = ti->private;
 
-	if (!rs->md.suspended)
+	if (!rs->md.suspended) {
+		mddev_lock_nointr(&rs->md);
 		mddev_suspend(&rs->md);
+		mddev_unlock(&rs->md);
+	}
 
 	rs->md.ro = 1;
 }
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 10367ffe92e3..6cbb37a7d445 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -320,14 +320,22 @@ static blk_qc_t md_make_request(struct request_queue *q, struct bio *bio)
  * are completely handled.
  * Once mddev_detach() is called and completes, the module will be
  * completely unused.
+ * The caller must hold the mddev_lock.
+ * mddev_suspend() will update the superblock if it
+ * turns out that something is waiting in md_write_start().
  */
 void mddev_suspend(struct mddev *mddev)
 {
 	WARN_ON_ONCE(mddev->thread && current == mddev->thread->tsk);
 	if (mddev->suspended++)
 		return;
+	lockdep_assert_held(&mddev->reconfig_mutex);
+
 	synchronize_rcu();
-	wait_event(mddev->sb_wait, atomic_read(&mddev->active_io) == 0);
+	wait_event_cmd(mddev->sb_wait, atomic_read(&mddev->active_io) == 0,
+		       if (test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags))
+			       md_update_sb(mddev, 0),
+		);
 	mddev->pers->quiesce(mddev, 1);
 
 	del_timer_sync(&mddev->safemode_timer);
@@ -7971,6 +7979,7 @@ void md_write_start(struct mddev *mddev, struct bio *bi)
 			set_bit(MD_SB_CHANGE_CLEAN, &mddev->sb_flags);
 			set_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags);
 			md_wakeup_thread(mddev->thread);
+			wake_up(&mddev->sb_wait);
 			did_change = 1;
 		}
 		spin_unlock(&mddev->lock);
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 4c00bc248287..c231c4a29903 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -682,13 +682,11 @@ static void r5c_disable_writeback_async(struct work_struct *work)
 	pr_info("md/raid:%s: Disabling writeback cache for degraded array.\n",
 		mdname(mddev));
 
-	/* wait superblock change before suspend */
-	wait_event(mddev->sb_wait,
-		   !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags));
-
+	mddev_lock_nointr(mddev);
 	mddev_suspend(mddev);
 	log->r5c_journal_mode = R5C_JOURNAL_MODE_WRITE_THROUGH;
 	mddev_resume(mddev);
+	mddev_unlock(mddev);
 }
 
 static void r5l_submit_current_io(struct r5l_log *log)
@@ -2542,6 +2540,7 @@ int r5c_journal_mode_set(struct mddev *mddev, int mode)
 {
 	struct r5conf *conf = mddev->private;
 	struct r5l_log *log = conf->log;
+	int ret = 0;
 
 	if (!log)
 		return -ENODEV;
@@ -2550,17 +2549,20 @@ int r5c_journal_mode_set(struct mddev *mddev, int mode)
 	    mode > R5C_JOURNAL_MODE_WRITE_BACK)
 		return -EINVAL;
 
+	mddev_lock_nointr(mddev);
 	if (raid5_calc_degraded(conf) > 0 &&
 	    mode == R5C_JOURNAL_MODE_WRITE_BACK)
-		return -EINVAL;
-
-	mddev_suspend(mddev);
-	conf->log->r5c_journal_mode = mode;
-	mddev_resume(mddev);
+		ret = -EINVAL;
+	else {
+		mddev_suspend(mddev);
+		conf->log->r5c_journal_mode = mode;
+		mddev_resume(mddev);
 
-	pr_debug("md/raid:%s: setting r5c cache mode to %d: %s\n",
-		 mdname(mddev), mode, r5c_journal_mode_str[mode]);
-	return 0;
+		pr_debug("md/raid:%s: setting r5c cache mode to %d: %s\n",
+			 mdname(mddev), mode, r5c_journal_mode_str[mode]);
+	}
+	mddev_unlock(mddev);
+	return ret;
 }
 EXPORT_SYMBOL(r5c_journal_mode_set);
 

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

^ permalink raw reply related

* Re: Reducing the number of devices in a degraded RAID-5
From: NeilBrown @ 2017-05-26  5:13 UTC (permalink / raw)
  To: Andreas Klauer; +Cc: linux-raid
In-Reply-To: <20170525072436.GA24881@metamorpher.de>

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

On Thu, May 25 2017, Andreas Klauer wrote:

> On Wed, May 24, 2017 at 12:12:32PM +1000, NeilBrown wrote:
>> What does work is:
>>  # start with a degraded array, device 0 missing
>>  mdadm --grow /dev/md42 --array-size=.....
>>  echo frozen > /sys/block/md42/md/sync_action
>>  mdadm /dev/md42 --add /dev/loop0
>>  echo 0 > /sys/block/md42/md/dev-loop0/slot
>>  mdadm --grow /dev/md42 --backup-file=... --raid-devices=4
>
> Wow. Thanks.
>
> This should merge Step 1+2, but not Step 3, right?

Right.  Doing step 3 at the same time is not possible.

NeilBrown


>
> I really need to take a closer look at the things in /sys/.../md/...
>
> Seems like you can do great things with it... terrible, yes, but great.
>
> (
>   Step 3 would be turning slot 4 to-be-spare into slot 0, 
>   without --add ing another device at all.
>
>   That's what would happen if /dev/loop0 was actually backed 
>   by and thus identical with the slot4 device.
>
>   But that's playing dirty.
> )
>
> Regards
> Andreas Klauer

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

^ permalink raw reply

* Re: [Patch v1] Detail: fix a bug when trying to show the detail of a raid device
From: Lidong Zhong @ 2017-05-26  6:33 UTC (permalink / raw)
  To: linux-raid; +Cc: colyli, Jes.Sorensen, neilb
In-Reply-To: <20170526024216.10248-1-lzhong@suse.com>

Hi,
Please ignore this patch. It has been fixed by Tomasz two days ago.
Sorry for the noise.

Thanks,
Lidong

On 05/26/2017 10:42 AM, Lidong Zhong wrote:
> It's introduced from commit a4dcdb23ea63.
>
> Signed-off-by: Lidong Zhong <lzhong@suse.com>
> ---
>  Detail.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/Detail.c b/Detail.c
> index bf881ff..c89b927 100644
> --- a/Detail.c
> +++ b/Detail.c
> @@ -98,8 +98,8 @@ int Detail(char *dev, struct context *c)
>  		    sra->array.minor_version == -2);
>  	inactive = (sra->array_state == ARRAY_ACTIVE ||
>  		    sra->array_state == ARRAY_CLEAR);
> -	st = super_by_fd(fd, &subarray);
> -	if (md_get_array_info(fd, &array) && errno == ENODEV) {
> +	rv = md_get_array_info(fd, &array);
> +	if (rv && errno == ENODEV) {
>  		if (sra->array.major_version == -1 &&
>  		    sra->array.minor_version == -1 &&
>  		    sra->devs == NULL) {
> @@ -109,7 +109,7 @@ int Detail(char *dev, struct context *c)
>  			return rv;
>  		}
>  		array = sra->array;
> -	} else {
> +	} else if (rv) {
>  		pr_err("cannot get array detail for %s: %s\n",
>  		       dev, strerror(errno));
>  		close(fd);
> @@ -118,8 +118,8 @@ int Detail(char *dev, struct context *c)
>
>  	if (fstat(fd, &stb) != 0 && !S_ISBLK(stb.st_mode))
>  		stb.st_rdev = 0;
> -	rv = 0;
>
> +	st = super_by_fd(fd, &subarray);
>  	if (st)
>  		max_disks = st->max_devs;
>
>

^ permalink raw reply

* Re: 4.11.2: reshape raid5 -> raid6 atop bcache deadlocks at start on md_attr_store / raid5_make_request
From: Shaohua Li @ 2017-05-26 16:40 UTC (permalink / raw)
  To: NeilBrown; +Cc: Nix, linux-raid
In-Reply-To: <87inkogv39.fsf@notabene.neil.brown.name>

On Fri, May 26, 2017 at 01:23:06PM +1000, Neil Brown wrote:
> On Wed, May 24 2017, Shaohua Li wrote:
> 
> > On Thu, May 25, 2017 at 11:30:12AM +1000, Neil Brown wrote:
> >> On Wed, May 24 2017, Shaohua Li wrote:
> >> 
> >> > On Wed, May 24, 2017 at 11:24:21AM +1000, Neil Brown wrote:
> >> >> 
> >> >> 
> >> >> 
> >> >> diff --git a/drivers/md/md.c b/drivers/md/md.c
> >> >> index 10367ffe92e3..a7b9c0576479 100644
> >> >> --- a/drivers/md/md.c
> >> >> +++ b/drivers/md/md.c
> >> >> @@ -324,8 +324,12 @@ static blk_qc_t md_make_request(struct request_queue *q, struct bio *bio)
> >> >>  void mddev_suspend(struct mddev *mddev)
> >> >>  {
> >> >>  	WARN_ON_ONCE(mddev->thread && current == mddev->thread->tsk);
> >> >> +
> >> >>  	if (mddev->suspended++)
> >> >>  		return;
> >> >> +#ifdef CONFIG_LOCKDEP
> >> >> +	WARN_ON_ONCE(debug_locks && lockdep_is_held(&mddev->reconfig_mutex));
> >> >> +#endif
> >> >>  	synchronize_rcu();
> >> >>  	wait_event(mddev->sb_wait, atomic_read(&mddev->active_io) == 0);
> >> >>  	mddev->pers->quiesce(mddev, 1);
> >> >> @@ -3594,9 +3598,12 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
> >> >>  	if (slen == 0 || slen >= sizeof(clevel))
> >> >>  		return -EINVAL;
> >> >>  
> >> >> +	mddev_suspend(mddev);
> >> >>  	rv = mddev_lock(mddev);
> >> >> -	if (rv)
> >> >> +	if (rv) {
> >> >> +		mddev_resume(mddev);
> >> >>  		return rv;
> >> >> +	}
> >> >>  
> >> >>  	if (mddev->pers == NULL) {
> >> >>  		strncpy(mddev->clevel, buf, slen);
> >> >> @@ -3687,7 +3694,6 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
> >> >>  	}
> >> >>  
> >> >>  	/* Looks like we have a winner */
> >> >> -	mddev_suspend(mddev);
> >> >>  	mddev_detach(mddev);
> >> >>  
> >> >>  	spin_lock(&mddev->lock);
> >> >> @@ -3771,13 +3777,13 @@ level_store(struct mddev *mddev, const char *buf, size_t len)
> >> >>  	blk_set_stacking_limits(&mddev->queue->limits);
> >> >>  	pers->run(mddev);
> >> >>  	set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags);
> >> >> -	mddev_resume(mddev);
> >> >>  	if (!mddev->thread)
> >> >>  		md_update_sb(mddev, 1);
> >> >>  	sysfs_notify(&mddev->kobj, NULL, "level");
> >> >>  	md_new_event(mddev);
> >> >>  	rv = len;
> >> >>  out_unlock:
> >> >> +	mddev_resume(mddev);
> >> >>  	mddev_unlock(mddev);
> >> >>  	return rv;
> >> >>  }
> >> >> @@ -4490,6 +4496,7 @@ action_store(struct mddev *mddev, const char *page, size_t len)
> >> >>  		int err;
> >> >>  		if (mddev->pers->start_reshape == NULL)
> >> >>  			return -EINVAL;
> >> >> +		mddev_suspend(mddev);
> >> >>  		err = mddev_lock(mddev);
> >> >>  		if (!err) {
> >> >>  			if (test_bit(MD_RECOVERY_RUNNING, &mddev->recovery))
> >> >> @@ -4500,6 +4507,7 @@ action_store(struct mddev *mddev, const char *page, size_t len)
> >> >>  			}
> >> >>  			mddev_unlock(mddev);
> >> >>  		}
> >> >> +		mddev_resume(mddev);
> >> >>  		if (err)
> >> >>  			return err;
> >> >>  		sysfs_notify(&mddev->kobj, NULL, "degraded");
> >> >
> >> > The analysis makes a lot of sense, thanks! The patch looks not solving the
> >> > problem though, because check_recovery will not write super if suspended isn't
> >> > 0.
> >> 
> >> Why does that matter?  In what case do you need the superblock to be
> >> written, but it doesn't happen.
> >> 
> >> check_recovery won't write the superblock while the mddev is locked
> >> either, and it is locked for most of level_store().
> >> When level_store() finished, it unlocks the device and that will trigger
> >> md_check_recovery() to be run, and the metadata will be written then.
> >> I don't think there is a particular need to update it before then.
> >
> > I get confused. md_write_start is waiting for MD_SB_CHANGE_PENDING cleared,
> > which is done in check_recovery. Your previous email describes this too.
> 
> Uhmm.... yes.  I see your point now.  Maybe we need might first patch as
> well, so md_check_recovery() can still update the superblock after
> ->suspended is set.
> 
> However, I starting looking at making sure mddev_suspend() was never
> called with mddev_lock() held, and that isn't straight forward.
> Most callers of mddev_suspend() do hold the lock.
> The only exceptions I found were:
>   dm-raid.c in raid_postsuspend()
>   raid5-cache.c in r5c_disable_writeback_async() and
>   r5c_journal_mode_set().
> 
> It might be easiest to change all of those to lock the mddev, then
> do the md_update_sb in mddev_suspend, when needed.
> 
> i.e. something like the following.  Thoughts?

Looks reasonable. why not hold the lock for mddev_resume too for dm-raid.c, at
least it protects ->suspended. The dm-raid.c is a little unformatable though,
other places always do lock, suspend, resume, unlock. The dm-raid is an
exception. Not quite sure if this is a problem though.

While this fix should work well, did you consider my previous proposal, which
should work I think and looks simplier.

Thanks,
Shaohua

> diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
> index 7d893228c40f..db79bd22418b 100644
> --- a/drivers/md/dm-raid.c
> +++ b/drivers/md/dm-raid.c
> @@ -3607,8 +3607,11 @@ static void raid_postsuspend(struct dm_target *ti)
>  {
>  	struct raid_set *rs = ti->private;
>  
> -	if (!rs->md.suspended)
> +	if (!rs->md.suspended) {
> +		mddev_lock_nointr(&rs->md);
>  		mddev_suspend(&rs->md);
> +		mddev_unlock(&rs->md);
> +	}
>  
>  	rs->md.ro = 1;
>  }
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 10367ffe92e3..6cbb37a7d445 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -320,14 +320,22 @@ static blk_qc_t md_make_request(struct request_queue *q, struct bio *bio)
>   * are completely handled.
>   * Once mddev_detach() is called and completes, the module will be
>   * completely unused.
> + * The caller must hold the mddev_lock.
> + * mddev_suspend() will update the superblock if it
> + * turns out that something is waiting in md_write_start().
>   */
>  void mddev_suspend(struct mddev *mddev)
>  {
>  	WARN_ON_ONCE(mddev->thread && current == mddev->thread->tsk);
>  	if (mddev->suspended++)
>  		return;
> +	lockdep_assert_held(&mddev->reconfig_mutex);
> +
>  	synchronize_rcu();
> -	wait_event(mddev->sb_wait, atomic_read(&mddev->active_io) == 0);
> +	wait_event_cmd(mddev->sb_wait, atomic_read(&mddev->active_io) == 0,
> +		       if (test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags))
> +			       md_update_sb(mddev, 0),
> +		);
>  	mddev->pers->quiesce(mddev, 1);
>  
>  	del_timer_sync(&mddev->safemode_timer);
> @@ -7971,6 +7979,7 @@ void md_write_start(struct mddev *mddev, struct bio *bi)
>  			set_bit(MD_SB_CHANGE_CLEAN, &mddev->sb_flags);
>  			set_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags);
>  			md_wakeup_thread(mddev->thread);
> +			wake_up(&mddev->sb_wait);
>  			did_change = 1;
>  		}
>  		spin_unlock(&mddev->lock);
> diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
> index 4c00bc248287..c231c4a29903 100644
> --- a/drivers/md/raid5-cache.c
> +++ b/drivers/md/raid5-cache.c
> @@ -682,13 +682,11 @@ static void r5c_disable_writeback_async(struct work_struct *work)
>  	pr_info("md/raid:%s: Disabling writeback cache for degraded array.\n",
>  		mdname(mddev));
>  
> -	/* wait superblock change before suspend */
> -	wait_event(mddev->sb_wait,
> -		   !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags));
> -
> +	mddev_lock_nointr(mddev);
>  	mddev_suspend(mddev);
>  	log->r5c_journal_mode = R5C_JOURNAL_MODE_WRITE_THROUGH;
>  	mddev_resume(mddev);
> +	mddev_unlock(mddev);
>  }
>  
>  static void r5l_submit_current_io(struct r5l_log *log)
> @@ -2542,6 +2540,7 @@ int r5c_journal_mode_set(struct mddev *mddev, int mode)
>  {
>  	struct r5conf *conf = mddev->private;
>  	struct r5l_log *log = conf->log;
> +	int ret = 0;
>  
>  	if (!log)
>  		return -ENODEV;
> @@ -2550,17 +2549,20 @@ int r5c_journal_mode_set(struct mddev *mddev, int mode)
>  	    mode > R5C_JOURNAL_MODE_WRITE_BACK)
>  		return -EINVAL;
>  
> +	mddev_lock_nointr(mddev);
>  	if (raid5_calc_degraded(conf) > 0 &&
>  	    mode == R5C_JOURNAL_MODE_WRITE_BACK)
> -		return -EINVAL;
> -
> -	mddev_suspend(mddev);
> -	conf->log->r5c_journal_mode = mode;
> -	mddev_resume(mddev);
> +		ret = -EINVAL;
> +	else {
> +		mddev_suspend(mddev);
> +		conf->log->r5c_journal_mode = mode;
> +		mddev_resume(mddev);
>  
> -	pr_debug("md/raid:%s: setting r5c cache mode to %d: %s\n",
> -		 mdname(mddev), mode, r5c_journal_mode_str[mode]);
> -	return 0;
> +		pr_debug("md/raid:%s: setting r5c cache mode to %d: %s\n",
> +			 mdname(mddev), mode, r5c_journal_mode_str[mode]);
> +	}
> +	mddev_unlock(mddev);
> +	return ret;
>  }
>  EXPORT_SYMBOL(r5c_journal_mode_set);
>  



^ permalink raw reply

* Re: [Patch v5] super1: fix sb->max_dev when adding a new disk in linear array
From: Jes Sorensen @ 2017-05-26 18:35 UTC (permalink / raw)
  To: Lidong Zhong, linux-raid, neilb; +Cc: colyli
In-Reply-To: <20170525092811.3868-1-lzhong@suse.com>

On 05/25/2017 05:28 AM, Lidong Zhong wrote:
> The value of sb->max_dev will always be increased by 1 when adding
> a new disk in linear array. It causes an inconsistence between each
> disk in the array and the "Array State" value of "mdadm --examine DISK"
> is wrong. For example, when adding the first new disk into linear array
> it will be:
> 
> Array State : RAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
> ('A' == active, '.' == missing, 'R' == replacing)
> 
> Adding the second disk into linear array it will be
> 
> Array State : .AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
> ('A' == active, '.' == missing, 'R' == replacing)
> 
> Signed-off-by: Lidong Zhong <lzhong@suse.com>
> ---
>   super1.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)

Applied!

Thanks,
Jes


^ permalink raw reply

* Re: raid5 to raid6 reshape never appeared to start, how to cancel/revert
From: Roger Heflin @ 2017-05-26 19:27 UTC (permalink / raw)
  To: Andreas Klauer; +Cc: Linux RAID
In-Reply-To: <CAAMCDedqUJXZFMwtqPVDjwoau42goEMcOLTbLX1fGdT7teQQQQ@mail.gmail.com>

On Mon, May 22, 2017 at 3:04 PM, Roger Heflin <rogerheflin@gmail.com> wrote:
> On Mon, May 22, 2017 at 2:33 PM, Andreas Klauer
> <Andreas.Klauer@metamorpher.de> wrote:
>> On Mon, May 22, 2017 at 01:57:44PM -0500, Roger Heflin wrote:
>>> I had a 3 disk raid5 with a hot spare.  I ran this:
>>> mdadm --grow /dev/md126 --level=6 --backup-file /root/r6rebuild
>>>
>>> I suspect I should have changed the number of devices in the above command to 4.
>>
>> It doesn't hurt to specify, but that much is implied.
>> Growing 3 device raid5 + spare to raid6 results in 4 device raid6.
>>
>
> Yes.
>
>>> The backup-file was created on a separate ssd.
>>
>> Is there anything meaningful in this file?
>>
>
> 16MB in size, but od -x indicates all zeros, so no, there is nothing
> meaningful in the file.
>
>>> trying assemble now gets this:
>>>  mdadm --assemble /dev/md126 /dev/sd[abe]1 /dev/sdd
>>> --backup-file=/root/r6rebuild
>>> mdadm: Failed to restore critical section for reshape, sorry.
>>>
>>> examine shows this (sdd was the spare when the --grow was issues)
>>>  mdadm --examine /dev/sdd
>>> /dev/sdd1:
>>
>> You wrote /dev/sdd above, is it sdd1 now?
>>
>>>         Version : 0.91.00
>>
>> Ancient metadata. You could probably update it to 1.0...
>>
>
> I know.
>
>>>   Reshape pos'n : 0
>>
>> So maybe nothing at all changed on disk?
>>
>> You could try your luck with overlay
>>
>> https://raid.wiki.kernel.org/index.php/Recovering_a_failed_software_RAID#Making_the_harddisks_read-only_using_an_overlay_file
>>
>> mdadm --create /dev/md42 --metadata=0.90 --level=5 --chunk=64 \
>>       --raid-devices=3 /dev/overlay/{a,b,c}
>>
>>> It does appear that I added sdd rather than sdd1 but I don't believe
>>> that is anything critical to the issue as it should still work fine
>>> with the entire disk.
>>
>> It is critical because if you use the wrong one the data will be shifted.
>>
>> If the partition goes to the very end of the drive, I think the 0.90
>> metadata could be interpreted both ways (as metadata for partition
>> as well as whole drive).
>>
>> If possible you should find some way to migrate to 1.2 metadata.
>> But worry about it once you have access to your data.
>>
>
> I deal with others messing up partition/no partition recoveries often
> enough to not be worried about how to debug and/or fix that mistake.
>
> I found a patch from Neil from 2016 that may be solution to this
> issue, I am not clear if it is an exact match to my issue, it looks
> pretty close.
>
> http://comments.gmane.org/gmane.linux.raid/51095
>
>> Regards
>> Andreas Klauer

Thanks for the ideas.   The patch I mentioned was already in the mdadm
that I had so that was no help.

I got it back by doing an -assume-clean and initially I could see the
pv but not the vg, I checked the device and it did look like a few kb
was missing between the pv label and the first vgdata I saw on the
disk.

I tried a vgcfgrestore and that failed with some weird errors I have
never seen before about failure to write and checksum failures (and I
have used vgcfgrestore a number of times successfully before).  I
finally saved out the first 1M for data to another disk and then
zeroed where the header should have been and did a pvrestore --uuid
and then a vgcfgrestore again and a vgchange -ay and it found the lv
and the filesystem appears to be fully intact.  I am guessing that
something did write to a few k to the disk during the attempt to raid6
it.  I am verifying and/or saving anything that I want (there may be
nothing important on it) and then will rebuild it as a new raid6 with
new metadata.

^ permalink raw reply

* cleanup UUID types V5
From: Christoph Hellwig @ 2017-05-28 10:19 UTC (permalink / raw)
  To: Andy Shevchenko, Amir Goldstein,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Shaohua Li,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells,
	linux-block-u79uwXL29TY76Z2rM5mHXA, Mimi Zohar, Steven Whitehouse

Hi all,

this series, which is a combined effort from Amir, Andy and me introduces
new uuid_t and guid_t type names that are less confusing than the existing
types, adds new helpers for them and starts switching the fs code over to
it.  Andy has additional patches on top to convert many of the users
that use char arrays for UUIDs and GUIDs to these (or rather a predecessor
for now until updated).

Changes since V4:
 - removed the patch to remove uuid_be for now
 - move the md patch to the front of the queue
 - remove the union for V1 uuids and provide accessors instead
 - new patch to set s_uuid for tmpfs
 - revert the patch that moved struct uuid_v1 from afs to the core
 - implement the fsid generation differently in XFS due to the
   change above
 - add a MAINTAINERS entry

Changes since V3:
 - stop exposing uuid_be/uuid_t to userspace
 - remove uuid_be entirely

Changes since V2:
 - various cleanups

^ permalink raw reply

* [PATCH 01/22] Revert "afs: Move UUID struct to linux/uuid.h"
From: Christoph Hellwig @ 2017-05-28 10:19 UTC (permalink / raw)
  To: Andy Shevchenko, Amir Goldstein,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Shaohua Li,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells,
	linux-block-u79uwXL29TY76Z2rM5mHXA, Mimi Zohar, Steven Whitehouse
In-Reply-To: <20170528102008.30276-1-hch-jcswGhMUV9g@public.gmane.org>

This reverts commit ff548773106ec7f8031bc6172e0234bd2a02c19c.

The V1 uuid intrepreatation in struct form isn't really useful to the
rest of the kernel, and not really compatible to it either, so move it
back to AFS instead of polluting the global uuid.h.

Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
---
 fs/afs/cmservice.c   | 28 ++++++++++++++--------------
 fs/afs/internal.h    | 26 +++++++++++++++++++++++++-
 fs/afs/main.c        |  2 +-
 include/linux/uuid.h | 24 ------------------------
 4 files changed, 40 insertions(+), 40 deletions(-)

diff --git a/fs/afs/cmservice.c b/fs/afs/cmservice.c
index 3062cceb5c2a..734604829a13 100644
--- a/fs/afs/cmservice.c
+++ b/fs/afs/cmservice.c
@@ -350,7 +350,7 @@ static int afs_deliver_cb_init_call_back_state3(struct afs_call *call)
 {
 	struct sockaddr_rxrpc srx;
 	struct afs_server *server;
-	struct uuid_v1 *r;
+	struct afs_uuid *r;
 	unsigned loop;
 	__be32 *b;
 	int ret;
@@ -380,15 +380,15 @@ static int afs_deliver_cb_init_call_back_state3(struct afs_call *call)
 		}
 
 		_debug("unmarshall UUID");
-		call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
+		call->request = kmalloc(sizeof(struct afs_uuid), GFP_KERNEL);
 		if (!call->request)
 			return -ENOMEM;
 
 		b = call->buffer;
 		r = call->request;
-		r->time_low			= b[0];
-		r->time_mid			= htons(ntohl(b[1]));
-		r->time_hi_and_version		= htons(ntohl(b[2]));
+		r->time_low			= ntohl(b[0]);
+		r->time_mid			= ntohl(b[1]);
+		r->time_hi_and_version		= ntohl(b[2]);
 		r->clock_seq_hi_and_reserved 	= ntohl(b[3]);
 		r->clock_seq_low		= ntohl(b[4]);
 
@@ -453,7 +453,7 @@ static int afs_deliver_cb_probe(struct afs_call *call)
 static void SRXAFSCB_ProbeUuid(struct work_struct *work)
 {
 	struct afs_call *call = container_of(work, struct afs_call, work);
-	struct uuid_v1 *r = call->request;
+	struct afs_uuid *r = call->request;
 
 	struct {
 		__be32	match;
@@ -476,7 +476,7 @@ static void SRXAFSCB_ProbeUuid(struct work_struct *work)
  */
 static int afs_deliver_cb_probe_uuid(struct afs_call *call)
 {
-	struct uuid_v1 *r;
+	struct afs_uuid *r;
 	unsigned loop;
 	__be32 *b;
 	int ret;
@@ -502,15 +502,15 @@ static int afs_deliver_cb_probe_uuid(struct afs_call *call)
 		}
 
 		_debug("unmarshall UUID");
-		call->request = kmalloc(sizeof(struct uuid_v1), GFP_KERNEL);
+		call->request = kmalloc(sizeof(struct afs_uuid), GFP_KERNEL);
 		if (!call->request)
 			return -ENOMEM;
 
 		b = call->buffer;
 		r = call->request;
-		r->time_low			= b[0];
-		r->time_mid			= htons(ntohl(b[1]));
-		r->time_hi_and_version		= htons(ntohl(b[2]));
+		r->time_low			= ntohl(b[0]);
+		r->time_mid			= ntohl(b[1]);
+		r->time_hi_and_version		= ntohl(b[2]);
 		r->clock_seq_hi_and_reserved 	= ntohl(b[3]);
 		r->clock_seq_low		= ntohl(b[4]);
 
@@ -568,9 +568,9 @@ static void SRXAFSCB_TellMeAboutYourself(struct work_struct *work)
 	memset(&reply, 0, sizeof(reply));
 	reply.ia.nifs = htonl(nifs);
 
-	reply.ia.uuid[0] = afs_uuid.time_low;
-	reply.ia.uuid[1] = htonl(ntohs(afs_uuid.time_mid));
-	reply.ia.uuid[2] = htonl(ntohs(afs_uuid.time_hi_and_version));
+	reply.ia.uuid[0] = htonl(afs_uuid.time_low);
+	reply.ia.uuid[1] = htonl(afs_uuid.time_mid);
+	reply.ia.uuid[2] = htonl(afs_uuid.time_hi_and_version);
 	reply.ia.uuid[3] = htonl((s8) afs_uuid.clock_seq_hi_and_reserved);
 	reply.ia.uuid[4] = htonl((s8) afs_uuid.clock_seq_low);
 	for (loop = 0; loop < 6; loop++)
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 393672997cc2..fb63b8ee4d40 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -410,6 +410,30 @@ struct afs_interface {
 	unsigned	mtu;		/* MTU of interface */
 };
 
+/*
+ * UUID definition [internet draft]
+ * - the timestamp is a 60-bit value, split 32/16/12, and goes in 100ns
+ *   increments since midnight 15th October 1582
+ *   - add AFS_UUID_TO_UNIX_TIME to convert unix time in 100ns units to UUID
+ *     time
+ * - the clock sequence is a 14-bit counter to avoid duplicate times
+ */
+struct afs_uuid {
+	u32		time_low;			/* low part of timestamp */
+	u16		time_mid;			/* mid part of timestamp */
+	u16		time_hi_and_version;		/* high part of timestamp and version  */
+#define AFS_UUID_TO_UNIX_TIME	0x01b21dd213814000ULL
+#define AFS_UUID_TIMEHI_MASK	0x0fff
+#define AFS_UUID_VERSION_TIME	0x1000	/* time-based UUID */
+#define AFS_UUID_VERSION_NAME	0x3000	/* name-based UUID */
+#define AFS_UUID_VERSION_RANDOM	0x4000	/* (pseudo-)random generated UUID */
+	u8		clock_seq_hi_and_reserved;	/* clock seq hi and variant */
+#define AFS_UUID_CLOCKHI_MASK	0x3f
+#define AFS_UUID_VARIANT_STD	0x80
+	u8		clock_seq_low;			/* clock seq low */
+	u8		node[6];			/* spatially unique node ID (MAC addr) */
+};
+
 /*****************************************************************************/
 /*
  * cache.c
@@ -544,7 +568,7 @@ extern int afs_drop_inode(struct inode *);
  * main.c
  */
 extern struct workqueue_struct *afs_wq;
-extern struct uuid_v1 afs_uuid;
+extern struct afs_uuid afs_uuid;
 
 /*
  * misc.c
diff --git a/fs/afs/main.c b/fs/afs/main.c
index 51d7d17bca57..9944770849da 100644
--- a/fs/afs/main.c
+++ b/fs/afs/main.c
@@ -31,7 +31,7 @@ static char *rootcell;
 module_param(rootcell, charp, 0);
 MODULE_PARM_DESC(rootcell, "root AFS cell name and VL server IP addr list");
 
-struct uuid_v1 afs_uuid;
+struct afs_uuid afs_uuid;
 struct workqueue_struct *afs_wq;
 
 /*
diff --git a/include/linux/uuid.h b/include/linux/uuid.h
index 4dff73a89758..2d095fc60204 100644
--- a/include/linux/uuid.h
+++ b/include/linux/uuid.h
@@ -19,30 +19,6 @@
 #include <uapi/linux/uuid.h>
 
 /*
- * V1 (time-based) UUID definition [RFC 4122].
- * - the timestamp is a 60-bit value, split 32/16/12, and goes in 100ns
- *   increments since midnight 15th October 1582
- *   - add AFS_UUID_TO_UNIX_TIME to convert unix time in 100ns units to UUID
- *     time
- * - the clock sequence is a 14-bit counter to avoid duplicate times
- */
-struct uuid_v1 {
-	__be32		time_low;			/* low part of timestamp */
-	__be16		time_mid;			/* mid part of timestamp */
-	__be16		time_hi_and_version;		/* high part of timestamp and version  */
-#define UUID_TO_UNIX_TIME	0x01b21dd213814000ULL
-#define UUID_TIMEHI_MASK	0x0fff
-#define UUID_VERSION_TIME	0x1000	/* time-based UUID */
-#define UUID_VERSION_NAME	0x3000	/* name-based UUID */
-#define UUID_VERSION_RANDOM	0x4000	/* (pseudo-)random generated UUID */
-	u8		clock_seq_hi_and_reserved;	/* clock seq hi and variant */
-#define UUID_CLOCKHI_MASK	0x3f
-#define UUID_VARIANT_STD	0x80
-	u8		clock_seq_low;			/* clock seq low */
-	u8		node[6];			/* spatially unique node ID (MAC addr) */
-};
-
-/*
  * The length of a UUID string ("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee")
  * not including trailing NUL.
  */
-- 
2.11.0

^ permalink raw reply related

* [PATCH 02/22] xfs: use uuid_copy() helper to abstract uuid_t
From: Christoph Hellwig @ 2017-05-28 10:19 UTC (permalink / raw)
  To: Andy Shevchenko, Amir Goldstein,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Shaohua Li,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells,
	linux-block-u79uwXL29TY76Z2rM5mHXA, Mimi Zohar, Steven Whitehouse
In-Reply-To: <20170528102008.30276-1-hch-jcswGhMUV9g@public.gmane.org>

From: Amir Goldstein <amir73il-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

uuid_t definition is about to change.

Signed-off-by: Amir Goldstein <amir73il-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
Reviewed-by: Brian Foster <bfoster-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Reviewed-by: Darrick J. Wong <darrick.wong-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 fs/xfs/xfs_inode_item.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 08cb7d1a4a3a..013cc78d7daf 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -834,9 +834,7 @@ xfs_inode_item_format_convert(
 		in_f->ilf_dsize = in_f32->ilf_dsize;
 		in_f->ilf_ino = in_f32->ilf_ino;
 		/* copy biggest field of ilf_u */
-		memcpy(in_f->ilf_u.ilfu_uuid.__u_bits,
-		       in_f32->ilf_u.ilfu_uuid.__u_bits,
-		       sizeof(uuid_t));
+		uuid_copy(&in_f->ilf_u.ilfu_uuid, &in_f32->ilf_u.ilfu_uuid);
 		in_f->ilf_blkno = in_f32->ilf_blkno;
 		in_f->ilf_len = in_f32->ilf_len;
 		in_f->ilf_boffset = in_f32->ilf_boffset;
@@ -851,9 +849,7 @@ xfs_inode_item_format_convert(
 		in_f->ilf_dsize = in_f64->ilf_dsize;
 		in_f->ilf_ino = in_f64->ilf_ino;
 		/* copy biggest field of ilf_u */
-		memcpy(in_f->ilf_u.ilfu_uuid.__u_bits,
-		       in_f64->ilf_u.ilfu_uuid.__u_bits,
-		       sizeof(uuid_t));
+		uuid_copy(&in_f->ilf_u.ilfu_uuid, &in_f64->ilf_u.ilfu_uuid);
 		in_f->ilf_blkno = in_f64->ilf_blkno;
 		in_f->ilf_len = in_f64->ilf_len;
 		in_f->ilf_boffset = in_f64->ilf_boffset;
-- 
2.11.0

^ permalink raw reply related

* [PATCH 03/22] xfs: use uuid_be to implement the uuid_t type
From: Christoph Hellwig @ 2017-05-28 10:19 UTC (permalink / raw)
  To: Andy Shevchenko, Amir Goldstein,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Shaohua Li,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells,
	linux-block-u79uwXL29TY76Z2rM5mHXA, Mimi Zohar, Steven Whitehouse
In-Reply-To: <20170528102008.30276-1-hch-jcswGhMUV9g@public.gmane.org>

Use the generic Linux definition to implement our UUID type, this will
allow using more generic infrastructure in the future.

Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
Reviewed-by: Amir Goldstein <amir73il-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: Brian Foster <bfoster-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Reviewed-by: Darrick J. Wong <darrick.wong-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 fs/xfs/uuid.h      | 4 ----
 fs/xfs/xfs_linux.h | 3 +++
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/uuid.h b/fs/xfs/uuid.h
index 104db0f3bed6..4f1441ba4fa5 100644
--- a/fs/xfs/uuid.h
+++ b/fs/xfs/uuid.h
@@ -18,10 +18,6 @@
 #ifndef __XFS_SUPPORT_UUID_H__
 #define __XFS_SUPPORT_UUID_H__
 
-typedef struct {
-	unsigned char	__u_bits[16];
-} uuid_t;
-
 extern int uuid_is_nil(uuid_t *uuid);
 extern int uuid_equal(uuid_t *uuid1, uuid_t *uuid2);
 extern void uuid_getnodeuniq(uuid_t *uuid, int fsid [2]);
diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
index 044fb0e15390..89ee5ec66837 100644
--- a/fs/xfs/xfs_linux.h
+++ b/fs/xfs/xfs_linux.h
@@ -19,6 +19,7 @@
 #define __XFS_LINUX__
 
 #include <linux/types.h>
+#include <linux/uuid.h>
 
 /*
  * Kernel specific type declarations for XFS
@@ -38,6 +39,8 @@ typedef __s64			xfs_daddr_t;	/* <disk address> type */
 typedef __u32			xfs_dev_t;
 typedef __u32			xfs_nlink_t;
 
+typedef uuid_be			uuid_t;
+
 #include "xfs_types.h"
 
 #include "kmem.h"
-- 
2.11.0

^ permalink raw reply related

* [PATCH 04/22] md: namespace private helper names
From: Christoph Hellwig @ 2017-05-28 10:19 UTC (permalink / raw)
  To: Andy Shevchenko, Amir Goldstein,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Shaohua Li,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells,
	linux-block-u79uwXL29TY76Z2rM5mHXA, Mimi Zohar, Steven Whitehouse
In-Reply-To: <20170528102008.30276-1-hch-jcswGhMUV9g@public.gmane.org>

From: Amir Goldstein <amir73il-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

The md private helper uuid_equal() collides with a generic helper
of the same name.

Rename the md private helper to md_uuid_equal() and do the same for
md_sb_equal().

Signed-off-by: Amir Goldstein <amir73il-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
Reviewed-by: Shaohua Li <shli-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 drivers/md/md.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 10367ffe92e3..b9ab268ba7f9 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -825,7 +825,7 @@ static int read_disk_sb(struct md_rdev *rdev, int size)
 	return -EINVAL;
 }
 
-static int uuid_equal(mdp_super_t *sb1, mdp_super_t *sb2)
+static int md_uuid_equal(mdp_super_t *sb1, mdp_super_t *sb2)
 {
 	return	sb1->set_uuid0 == sb2->set_uuid0 &&
 		sb1->set_uuid1 == sb2->set_uuid1 &&
@@ -833,7 +833,7 @@ static int uuid_equal(mdp_super_t *sb1, mdp_super_t *sb2)
 		sb1->set_uuid3 == sb2->set_uuid3;
 }
 
-static int sb_equal(mdp_super_t *sb1, mdp_super_t *sb2)
+static int md_sb_equal(mdp_super_t *sb1, mdp_super_t *sb2)
 {
 	int ret;
 	mdp_super_t *tmp1, *tmp2;
@@ -1025,12 +1025,12 @@ static int super_90_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor
 	} else {
 		__u64 ev1, ev2;
 		mdp_super_t *refsb = page_address(refdev->sb_page);
-		if (!uuid_equal(refsb, sb)) {
+		if (!md_uuid_equal(refsb, sb)) {
 			pr_warn("md: %s has different UUID to %s\n",
 				b, bdevname(refdev->bdev,b2));
 			goto abort;
 		}
-		if (!sb_equal(refsb, sb)) {
+		if (!md_sb_equal(refsb, sb)) {
 			pr_warn("md: %s has same UUID but different superblock to %s\n",
 				b, bdevname(refdev->bdev, b2));
 			goto abort;
-- 
2.11.0

^ permalink raw reply related

* [PATCH 05/22] uuid: remove uuid_be defintions from the uapi header
From: Christoph Hellwig @ 2017-05-28 10:19 UTC (permalink / raw)
  To: Andy Shevchenko, Amir Goldstein,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Shaohua Li,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells,
	linux-block-u79uwXL29TY76Z2rM5mHXA, Mimi Zohar, Steven Whitehouse
In-Reply-To: <20170528102008.30276-1-hch-jcswGhMUV9g@public.gmane.org>

We don't use uuid_be and the UUID_BE constants in any uapi headers, so make
them private to the kernel.

Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
Reviewed-by: Amir Goldstein <amir73il-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 include/linux/uuid.h      | 15 +++++++++++++++
 include/uapi/linux/uuid.h | 16 ----------------
 2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/include/linux/uuid.h b/include/linux/uuid.h
index 2d095fc60204..30fb13018e29 100644
--- a/include/linux/uuid.h
+++ b/include/linux/uuid.h
@@ -18,6 +18,21 @@
 
 #include <uapi/linux/uuid.h>
 
+typedef struct {
+	__u8 b[16];
+} uuid_be;
+
+#define UUID_BE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)		\
+((uuid_be)								\
+{{ ((a) >> 24) & 0xff, ((a) >> 16) & 0xff, ((a) >> 8) & 0xff, (a) & 0xff, \
+   ((b) >> 8) & 0xff, (b) & 0xff,					\
+   ((c) >> 8) & 0xff, (c) & 0xff,					\
+   (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
+
+#define NULL_UUID_BE							\
+	UUID_BE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00,	\
+		0x00, 0x00, 0x00, 0x00)
+
 /*
  * The length of a UUID string ("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee")
  * not including trailing NUL.
diff --git a/include/uapi/linux/uuid.h b/include/uapi/linux/uuid.h
index 3738e5fb6a4d..0099756c4bac 100644
--- a/include/uapi/linux/uuid.h
+++ b/include/uapi/linux/uuid.h
@@ -24,10 +24,6 @@ typedef struct {
 	__u8 b[16];
 } uuid_le;
 
-typedef struct {
-	__u8 b[16];
-} uuid_be;
-
 #define UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)		\
 ((uuid_le)								\
 {{ (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff, \
@@ -35,20 +31,8 @@ typedef struct {
    (c) & 0xff, ((c) >> 8) & 0xff,					\
    (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
 
-#define UUID_BE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)		\
-((uuid_be)								\
-{{ ((a) >> 24) & 0xff, ((a) >> 16) & 0xff, ((a) >> 8) & 0xff, (a) & 0xff, \
-   ((b) >> 8) & 0xff, (b) & 0xff,					\
-   ((c) >> 8) & 0xff, (c) & 0xff,					\
-   (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
-
 #define NULL_UUID_LE							\
 	UUID_LE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00,	\
 		0x00, 0x00, 0x00, 0x00)
 
-#define NULL_UUID_BE							\
-	UUID_BE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00,	\
-		0x00, 0x00, 0x00, 0x00)
-
-
 #endif /* _UAPI_LINUX_UUID_H_ */
-- 
2.11.0

^ permalink raw reply related

* [PATCH 06/22] uuid: rename uuid types
From: Christoph Hellwig @ 2017-05-28 10:19 UTC (permalink / raw)
  To: Andy Shevchenko, Amir Goldstein,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Shaohua Li,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells,
	linux-block-u79uwXL29TY76Z2rM5mHXA, Mimi Zohar, Steven Whitehouse
In-Reply-To: <20170528102008.30276-1-hch-jcswGhMUV9g@public.gmane.org>

Our "little endian" UUID really is a Wintel GUID, so rename it and its
helpers such (guid_t).  The big endian UUID is the only true one, so
give it the name uuid_t.  The uuid_le and uuid_be names are retained for
now, but will hopefully go away soon.  The exception to that are the _cmp
helpers that will be replaced by better primitives ASAP and thus don't
get the new names.

Also remove the existing typedef in XFS that's now been superceeded by
the generic type name.

Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
[andy: also update the UUID_LE/UUID_BE macros including fallout]
Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Reviewed-by: Amir Goldstein <amir73il-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: Darrick J. Wong <darrick.wong-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 fs/xfs/xfs_linux.h        |  2 --
 include/linux/uuid.h      | 55 +++++++++++++++++++++++++++--------------------
 include/uapi/linux/uuid.h | 12 +++++++----
 lib/test_uuid.c           | 32 +++++++++++++--------------
 lib/uuid.c                | 28 ++++++++++++------------
 lib/vsprintf.c            |  4 ++--
 6 files changed, 72 insertions(+), 61 deletions(-)

diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
index 89ee5ec66837..2c33d915e550 100644
--- a/fs/xfs/xfs_linux.h
+++ b/fs/xfs/xfs_linux.h
@@ -39,8 +39,6 @@ typedef __s64			xfs_daddr_t;	/* <disk address> type */
 typedef __u32			xfs_dev_t;
 typedef __u32			xfs_nlink_t;
 
-typedef uuid_be			uuid_t;
-
 #include "xfs_types.h"
 
 #include "kmem.h"
diff --git a/include/linux/uuid.h b/include/linux/uuid.h
index 30fb13018e29..a9d0fdba5404 100644
--- a/include/linux/uuid.h
+++ b/include/linux/uuid.h
@@ -20,46 +20,55 @@
 
 typedef struct {
 	__u8 b[16];
-} uuid_be;
+} uuid_t;
 
-#define UUID_BE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)		\
-((uuid_be)								\
+#define UUID(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)			\
+((uuid_t)								\
 {{ ((a) >> 24) & 0xff, ((a) >> 16) & 0xff, ((a) >> 8) & 0xff, (a) & 0xff, \
    ((b) >> 8) & 0xff, (b) & 0xff,					\
    ((c) >> 8) & 0xff, (c) & 0xff,					\
    (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
 
-#define NULL_UUID_BE							\
-	UUID_BE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00,	\
-		0x00, 0x00, 0x00, 0x00)
-
 /*
  * The length of a UUID string ("aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee")
  * not including trailing NUL.
  */
 #define	UUID_STRING_LEN		36
 
-static inline int uuid_le_cmp(const uuid_le u1, const uuid_le u2)
-{
-	return memcmp(&u1, &u2, sizeof(uuid_le));
-}
-
-static inline int uuid_be_cmp(const uuid_be u1, const uuid_be u2)
-{
-	return memcmp(&u1, &u2, sizeof(uuid_be));
-}
-
 void generate_random_uuid(unsigned char uuid[16]);
 
-extern void uuid_le_gen(uuid_le *u);
-extern void uuid_be_gen(uuid_be *u);
+extern void guid_gen(guid_t *u);
+extern void uuid_gen(uuid_t *u);
 
 bool __must_check uuid_is_valid(const char *uuid);
 
-extern const u8 uuid_le_index[16];
-extern const u8 uuid_be_index[16];
+extern const u8 guid_index[16];
+extern const u8 uuid_index[16];
+
+int guid_to_bin(const char *uuid, guid_t *u);
+int uuid_to_bin(const char *uuid, uuid_t *u);
 
-int uuid_le_to_bin(const char *uuid, uuid_le *u);
-int uuid_be_to_bin(const char *uuid, uuid_be *u);
+/* backwards compatibility, don't use in new code */
+typedef uuid_t uuid_be;
+#define UUID_BE(a, _b, c, d0, d1, d2, d3, d4, d5, d6, d7) \
+	UUID(a, _b, c, d0, d1, d2, d3, d4, d5, d6, d7)
+#define NULL_UUID_BE 							\
+	UUID_BE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00,	\
+	     0x00, 0x00, 0x00, 0x00)
+
+#define uuid_le_gen(u)		guid_gen(u)
+#define uuid_be_gen(u)		uuid_gen(u)
+#define uuid_le_to_bin(guid, u)	guid_to_bin(guid, u)
+#define uuid_be_to_bin(uuid, u)	uuid_to_bin(uuid, u)
+
+static inline int uuid_le_cmp(const guid_t u1, const guid_t u2)
+{
+	return memcmp(&u1, &u2, sizeof(guid_t));
+}
+
+static inline int uuid_be_cmp(const uuid_t u1, const uuid_t u2)
+{
+	return memcmp(&u1, &u2, sizeof(uuid_t));
+}
 
 #endif
diff --git a/include/uapi/linux/uuid.h b/include/uapi/linux/uuid.h
index 0099756c4bac..1eeeca973315 100644
--- a/include/uapi/linux/uuid.h
+++ b/include/uapi/linux/uuid.h
@@ -22,17 +22,21 @@
 
 typedef struct {
 	__u8 b[16];
-} uuid_le;
+} guid_t;
 
-#define UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)		\
-((uuid_le)								\
+#define GUID(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)			\
+((guid_t)								\
 {{ (a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff, \
    (b) & 0xff, ((b) >> 8) & 0xff,					\
    (c) & 0xff, ((c) >> 8) & 0xff,					\
    (d0), (d1), (d2), (d3), (d4), (d5), (d6), (d7) }})
 
+/* backwards compatibility, don't use in new code */
+typedef guid_t uuid_le;
+#define UUID_LE(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)		\
+	GUID(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)
 #define NULL_UUID_LE							\
 	UUID_LE(0x00000000, 0x0000, 0x0000, 0x00, 0x00, 0x00, 0x00,	\
-		0x00, 0x00, 0x00, 0x00)
+	     0x00, 0x00, 0x00, 0x00)
 
 #endif /* _UAPI_LINUX_UUID_H_ */
diff --git a/lib/test_uuid.c b/lib/test_uuid.c
index 547d3127a3cf..9cad846fd805 100644
--- a/lib/test_uuid.c
+++ b/lib/test_uuid.c
@@ -11,25 +11,25 @@
 
 struct test_uuid_data {
 	const char *uuid;
-	uuid_le le;
-	uuid_be be;
+	guid_t le;
+	uuid_t be;
 };
 
 static const struct test_uuid_data test_uuid_test_data[] = {
 	{
 		.uuid = "c33f4995-3701-450e-9fbf-206a2e98e576",
-		.le = UUID_LE(0xc33f4995, 0x3701, 0x450e, 0x9f, 0xbf, 0x20, 0x6a, 0x2e, 0x98, 0xe5, 0x76),
-		.be = UUID_BE(0xc33f4995, 0x3701, 0x450e, 0x9f, 0xbf, 0x20, 0x6a, 0x2e, 0x98, 0xe5, 0x76),
+		.le = GUID(0xc33f4995, 0x3701, 0x450e, 0x9f, 0xbf, 0x20, 0x6a, 0x2e, 0x98, 0xe5, 0x76),
+		.be = UUID(0xc33f4995, 0x3701, 0x450e, 0x9f, 0xbf, 0x20, 0x6a, 0x2e, 0x98, 0xe5, 0x76),
 	},
 	{
 		.uuid = "64b4371c-77c1-48f9-8221-29f054fc023b",
-		.le = UUID_LE(0x64b4371c, 0x77c1, 0x48f9, 0x82, 0x21, 0x29, 0xf0, 0x54, 0xfc, 0x02, 0x3b),
-		.be = UUID_BE(0x64b4371c, 0x77c1, 0x48f9, 0x82, 0x21, 0x29, 0xf0, 0x54, 0xfc, 0x02, 0x3b),
+		.le = GUID(0x64b4371c, 0x77c1, 0x48f9, 0x82, 0x21, 0x29, 0xf0, 0x54, 0xfc, 0x02, 0x3b),
+		.be = UUID(0x64b4371c, 0x77c1, 0x48f9, 0x82, 0x21, 0x29, 0xf0, 0x54, 0xfc, 0x02, 0x3b),
 	},
 	{
 		.uuid = "0cb4ddff-a545-4401-9d06-688af53e7f84",
-		.le = UUID_LE(0x0cb4ddff, 0xa545, 0x4401, 0x9d, 0x06, 0x68, 0x8a, 0xf5, 0x3e, 0x7f, 0x84),
-		.be = UUID_BE(0x0cb4ddff, 0xa545, 0x4401, 0x9d, 0x06, 0x68, 0x8a, 0xf5, 0x3e, 0x7f, 0x84),
+		.le = GUID(0x0cb4ddff, 0xa545, 0x4401, 0x9d, 0x06, 0x68, 0x8a, 0xf5, 0x3e, 0x7f, 0x84),
+		.be = UUID(0x0cb4ddff, 0xa545, 0x4401, 0x9d, 0x06, 0x68, 0x8a, 0xf5, 0x3e, 0x7f, 0x84),
 	},
 };
 
@@ -61,13 +61,13 @@ static void __init test_uuid_failed(const char *prefix, bool wrong, bool be,
 
 static void __init test_uuid_test(const struct test_uuid_data *data)
 {
-	uuid_le le;
-	uuid_be be;
+	guid_t le;
+	uuid_t be;
 	char buf[48];
 
 	/* LE */
 	total_tests++;
-	if (uuid_le_to_bin(data->uuid, &le))
+	if (guid_to_bin(data->uuid, &le))
 		test_uuid_failed("conversion", false, false, data->uuid, NULL);
 
 	total_tests++;
@@ -78,7 +78,7 @@ static void __init test_uuid_test(const struct test_uuid_data *data)
 
 	/* BE */
 	total_tests++;
-	if (uuid_be_to_bin(data->uuid, &be))
+	if (uuid_to_bin(data->uuid, &be))
 		test_uuid_failed("conversion", false, true, data->uuid, NULL);
 
 	total_tests++;
@@ -90,17 +90,17 @@ static void __init test_uuid_test(const struct test_uuid_data *data)
 
 static void __init test_uuid_wrong(const char *data)
 {
-	uuid_le le;
-	uuid_be be;
+	guid_t le;
+	uuid_t be;
 
 	/* LE */
 	total_tests++;
-	if (!uuid_le_to_bin(data, &le))
+	if (!guid_to_bin(data, &le))
 		test_uuid_failed("negative", true, false, data, NULL);
 
 	/* BE */
 	total_tests++;
-	if (!uuid_be_to_bin(data, &be))
+	if (!uuid_to_bin(data, &be))
 		test_uuid_failed("negative", true, true, data, NULL);
 }
 
diff --git a/lib/uuid.c b/lib/uuid.c
index 37687af77ff8..f80dc63f6ca8 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -21,10 +21,10 @@
 #include <linux/uuid.h>
 #include <linux/random.h>
 
-const u8 uuid_le_index[16] = {3,2,1,0,5,4,7,6,8,9,10,11,12,13,14,15};
-EXPORT_SYMBOL(uuid_le_index);
-const u8 uuid_be_index[16] = {0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15};
-EXPORT_SYMBOL(uuid_be_index);
+const u8 guid_index[16] = {3,2,1,0,5,4,7,6,8,9,10,11,12,13,14,15};
+EXPORT_SYMBOL(guid_index);
+const u8 uuid_index[16] = {0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15};
+EXPORT_SYMBOL(uuid_index);
 
 /***************************************************************
  * Random UUID interface
@@ -53,21 +53,21 @@ static void __uuid_gen_common(__u8 b[16])
 	b[8] = (b[8] & 0x3F) | 0x80;
 }
 
-void uuid_le_gen(uuid_le *lu)
+void guid_gen(guid_t *lu)
 {
 	__uuid_gen_common(lu->b);
 	/* version 4 : random generation */
 	lu->b[7] = (lu->b[7] & 0x0F) | 0x40;
 }
-EXPORT_SYMBOL_GPL(uuid_le_gen);
+EXPORT_SYMBOL_GPL(guid_gen);
 
-void uuid_be_gen(uuid_be *bu)
+void uuid_gen(uuid_t *bu)
 {
 	__uuid_gen_common(bu->b);
 	/* version 4 : random generation */
 	bu->b[6] = (bu->b[6] & 0x0F) | 0x40;
 }
-EXPORT_SYMBOL_GPL(uuid_be_gen);
+EXPORT_SYMBOL_GPL(uuid_gen);
 
 /**
   * uuid_is_valid - checks if UUID string valid
@@ -115,14 +115,14 @@ static int __uuid_to_bin(const char *uuid, __u8 b[16], const u8 ei[16])
 	return 0;
 }
 
-int uuid_le_to_bin(const char *uuid, uuid_le *u)
+int guid_to_bin(const char *uuid, guid_t *u)
 {
-	return __uuid_to_bin(uuid, u->b, uuid_le_index);
+	return __uuid_to_bin(uuid, u->b, guid_index);
 }
-EXPORT_SYMBOL(uuid_le_to_bin);
+EXPORT_SYMBOL(guid_to_bin);
 
-int uuid_be_to_bin(const char *uuid, uuid_be *u)
+int uuid_to_bin(const char *uuid, uuid_t *u)
 {
-	return __uuid_to_bin(uuid, u->b, uuid_be_index);
+	return __uuid_to_bin(uuid, u->b, uuid_index);
 }
-EXPORT_SYMBOL(uuid_be_to_bin);
+EXPORT_SYMBOL(uuid_to_bin);
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 2d41de3f98a1..9f37d6208e99 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1308,14 +1308,14 @@ char *uuid_string(char *buf, char *end, const u8 *addr,
 	char uuid[UUID_STRING_LEN + 1];
 	char *p = uuid;
 	int i;
-	const u8 *index = uuid_be_index;
+	const u8 *index = uuid_index;
 	bool uc = false;
 
 	switch (*(++fmt)) {
 	case 'L':
 		uc = true;		/* fall-through */
 	case 'l':
-		index = uuid_le_index;
+		index = guid_index;
 		break;
 	case 'B':
 		uc = true;
-- 
2.11.0

^ permalink raw reply related

* [PATCH 07/22] uuid: don't export guid_index and uuid_index
From: Christoph Hellwig @ 2017-05-28 10:19 UTC (permalink / raw)
  To: Andy Shevchenko, Amir Goldstein,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-xfs-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Shaohua Li,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Howells,
	linux-block-u79uwXL29TY76Z2rM5mHXA, Mimi Zohar, Steven Whitehouse
In-Reply-To: <20170528102008.30276-1-hch-jcswGhMUV9g@public.gmane.org>

These are only used in uuid.c and vsprintf.c and aren't something modules
should use directly.

Signed-off-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
Reviewed-by: Amir Goldstein <amir73il-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
 lib/uuid.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/uuid.c b/lib/uuid.c
index f80dc63f6ca8..003bf6823003 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -22,9 +22,7 @@
 #include <linux/random.h>
 
 const u8 guid_index[16] = {3,2,1,0,5,4,7,6,8,9,10,11,12,13,14,15};
-EXPORT_SYMBOL(guid_index);
 const u8 uuid_index[16] = {0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15};
-EXPORT_SYMBOL(uuid_index);
 
 /***************************************************************
  * Random UUID interface
-- 
2.11.0

^ permalink raw reply related


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