Re: [md PATCH 01/18] md: disconnect device from personality before trying to remove it.

NeilBrown
2016-06-10T06:40:44Z

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

On Sat, Jun 04 2016, Shaohua Li wrote:

> On Thu, Jun 02, 2016 at 04:19:52PM +1000, Neil Brown wrote:
>> When the HOT_REMOVE_DISK ioctl is used to remove a device, we
>> call remove_and_add_spares() which will remove it from the personality
>> if possible. This improves the chances that the removal will succeed.
>>
>> When writing "remove" to dev-XX/state, we don't. So that can fail more easily.
>>
>> So add the remove_and_add_spares() into "remove" handling.
>>
>> Signed-off-by: NeilBrown <neilb@suse.com>
>> ---
>> drivers/md/md.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index 866825f10b4c..2d26099e1160 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -2596,6 +2596,8 @@ state_store(struct md_rdev *rdev, const char *buf, size_t len)
>> else
>> err = -EBUSY;
>> } else if (cmd_match(buf, "remove")) {
>> + clear_bit(Blocked, &rdev->flags);
>> + remove_and_add_spares(rdev->mddev, rdev);
>> if (rdev->raid_disk >= 0)
>> err = -EBUSY;
>> else {
>
> Do we need wakeup rdev->blocked_wait here? I noticed some times we do the
> wakeup but sometimes we not. This makes me worry about
> md_wait_for_blocked_rdev. Will timeout cause anything bad?

(sorry for delay)

Yes, we probably should but it isn't very important. If any code is
waiting then the 'remove' will fail and whatever would normally unblock
the device will still unblock and wake up as it would have done.
It is import to wake blocked_rdev when we write out the bad block list
or other metadata which makes it safe to use the device. If we clear
the flag at other time it doesn't really matter.

Adding a wakeup here might make it more likely for the "remove" to work
in some rare cases, but I think that is the most significant effect.

NeilBrown

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