Starting from [1], kernel requires suspend lock on member drive remove
path. It causes deadlock with external management because monitor
thread may be locked on suspend and is unable to switch array to active,
for example if badblock is reported in this time.
It is blocking action now, so it must be delegated to managemon thread
but we must ensure that monitor does metadata update first, just after
detecting faulty.
This patch adds appropriative support. Monitor thread detects "faulty",
and updates the metadata. After that, it is asking manager thread to
remove the device. Manager must be careful because closing descriptors
used by select() may lead to abort with D_FORTIFY_SOURCE=2. First, it
must ensure that device descriptors are not used by monitor.
There is unlimited numer of remove retries and recovery is blocked
until all failed drives are removed. It is safe because "faulty"
device is not longer used by MD.
Issue will be also mitigated by optimalization on badlbock recording path
in kernel. It will check if device is not failed before badblock is
recorded but relying on this is not ideologically correct. Userspace
must keep compatibility with kernel and since it is blocking action,
we must tract is as blocking action.
[1] kernel commit
cfa078c8b80d ("md: use new apis to suspend array
for adding/removing rdev from state_store()")
Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
return 0;
}
+/**
+ * managemon_disk_remove()- remove disk from the MD array.
+ * @disk: device to be removed.
+ * @array_devnm: the name of the array to remove disk from.
+ *
+ * It tries to remove the disk from the MD array and if it is successful then it closes all opened
+ * descriptors. Removing action requires suspend, it might take a while.
+ * Invalidating mdi->state_fd will prevent from using this device further (see duplicate_aa()).
+ *
+ * To avoid deadlock, new file descriptor is opened because monitor may already wait on
+ * mdddev_suspend() in kernel and keep saved descriptor locked.
+ *
+ * Returns MDADM_STATUS_SUCCESS if disk has been removed, MDADM_STATUS_ERROR otherwise.
+ */
+static mdadm_status_t managemon_disk_remove(struct mdinfo *disk, char *array_devnm)
+{
+ int new_state_fd = sysfs_open2(array_devnm, disk->sys_name, "state");
+
+ if (!is_fd_valid(new_state_fd))
+ return MDADM_STATUS_ERROR;
+
+ if (write_attr("remove", new_state_fd) != MDADM_STATUS_SUCCESS)
+ return MDADM_STATUS_ERROR;
+
+ close_fd(&new_state_fd);
+ close_fd(&disk->state_fd);
+ close_fd(&disk->recovery_fd);
+ close_fd(&disk->bb_fd);
+ close_fd(&disk->ubb_fd);
+
+ return MDADM_STATUS_SUCCESS;
+}
+
static void manage_member(struct mdstat_ent *mdstat,
struct active_array *a)
{
if (write_attr("0.001", a->safe_mode_delay_fd) == MDADM_STATUS_SUCCESS)
a->info.safe_mode_delay = 1;
+ if (a->check_member_remove) {
+ bool any_removed = false;
+ bool all_removed = true;
+ struct mdinfo *disk;
+
+ for (disk = a->info.devs; disk; disk = disk->next) {
+ if (disk->man_disk_to_remove == false)
+ continue;
+
+ if (disk->mon_descriptors_not_used == false) {
+ /* To early, repeat later */
+ all_removed = false;
+ continue;
+ }
+
+ if (managemon_disk_remove(disk, a->info.sys_name)) {
+ all_removed = false;
+ continue;
+ }
+
+ any_removed = true;
+ }
+
+ if (any_removed) {
+ struct active_array *newa = duplicate_aa(a);
+
+ if (all_removed)
+ newa->check_member_remove = false;
+
+ replace_array(container, a, newa);
+ a = newa;
+ }
+
+ if (!all_removed)
+ return;
+ }
+
/* We don't check the array while any update is pending, as it
* might container a change (such as a spare assignment) which
* could affect our decisions.
return;
newa = duplicate_aa(a);
- if (!newa)
- goto out;
/* prevent the kernel from activating the disk(s) before we
* finish adding them
*/
"sync_action", "recover") == 0)
newa->prev_action = recover;
dprintf("recovery started on %s\n", a->info.sys_name);
- out:
+
while (newdev) {
d = newdev->next;
free(newdev);
if (d2)
/* already have this one */
continue;
- if (!newa) {
+ if (!newa)
newa = duplicate_aa(a);
- if (!newa)
- break;
- }
+
newd = xmalloc(sizeof(*newd));
disk_init_and_add(newd, d, newa);
}
#define DS_EXTERNAL_BB 4096
int prev_state, curr_state, next_state;
+ /* If set by monitor, managemon needs to remove faulty device */
+ bool man_disk_to_remove : 1;
+
+ /* Managemon cannot close descriptors if monitor is using them for select() */
+ bool mon_descriptors_not_used : 1;
+
/* info read from sysfs */
enum {
ARRAY_CLEAR,
enum array_state prev_state, curr_state, next_state;
enum sync_action prev_action, curr_action, next_action;
- int check_degraded; /* flag set by mon, read by manage */
- int check_reshape; /* flag set by mon, read by manage */
+ bool check_degraded : 1; /* flag set by mon, read by manage */
+ bool check_reshape : 1; /* flag set by mon, read by manage */
+
+ /**
+ * Signalize managemon there is a mdi to be removed.
+ * Monitor must acknowledge faulty state first.
+ */
+ bool check_member_remove : 1;
};
/*
static int read_and_act(struct active_array *a)
{
unsigned long long sync_completed;
- int check_degraded = 0;
- int check_reshape = 0;
+ bool disks_to_remove = false;
+ bool check_degraded = false;
+ bool check_reshape = false;
int deactivate = 0;
struct mdinfo *mdi;
int ret = 0;
mdi->next_state = 0;
mdi->curr_state = 0;
- if (!is_fd_valid(mdi->state_fd))
+ if (mdi->man_disk_to_remove)
/* We are removing this device, skip it then */
continue;
write_attr("-blocked", mdi->state_fd);
}
- if ((mdi->next_state & DS_REMOVE) && mdi->state_fd >= 0) {
- /* The kernel may not be able to immediately remove the
- * disk. In that case we wait a little while and
- * try again.
- */
- if (write_attr("remove", mdi->state_fd) == MDADM_STATUS_SUCCESS) {
- dprintf_cont(" %d:removed", mdi->disk.raid_disk);
- close(mdi->state_fd);
- close(mdi->recovery_fd);
- close(mdi->bb_fd);
- close(mdi->ubb_fd);
- mdi->state_fd = -1;
- } else
- ret |= ARRAY_BUSY;
+ if ((mdi->next_state & DS_REMOVE) && !mdi->man_disk_to_remove) {
+ dprintf_cont(" %d:disk_to_remove", mdi->disk.raid_disk);
+ mdi->man_disk_to_remove = true;
+ disks_to_remove = true;
}
+
if (mdi->next_state & DS_INSYNC) {
write_attr("+in_sync", mdi->state_fd);
dprintf_cont(" %d:+in_sync", mdi->disk.raid_disk);
a->prev_action = a->curr_action;
- for (mdi = a->info.devs; mdi ; mdi = mdi->next) {
+ for (mdi = a->info.devs; mdi ; mdi = mdi->next)
mdi->prev_state = mdi->curr_state;
- mdi->next_state = 0;
- }
- if (check_degraded || check_reshape) {
- /* manager will do the actual check */
- if (check_degraded)
- a->check_degraded = 1;
- if (check_reshape)
- a->check_reshape = 1;
+ if (check_degraded || check_reshape || disks_to_remove) {
+
+ a->check_member_remove |= disks_to_remove;
+ a->check_degraded |= check_degraded;
+ a->check_reshape |= check_reshape;
signal_manager();
}
static int wait_and_act(struct supertype *container, int nowait)
{
- fd_set rfds;
- int maxfd = 0;
- struct active_array **aap = &container->arrays;
- struct active_array *a, **ap;
- int rv;
- struct mdinfo *mdi;
+ struct active_array *a, **ap, **aap = &container->arrays;
static unsigned int dirty_arrays = ~0; /* start at some non-zero value */
+ struct mdinfo *mdi;
+ int rv, maxfd = 0;
+ fd_set rfds;
FD_ZERO(&rfds);
add_fd(&rfds, &maxfd, a->info.state_fd);
add_fd(&rfds, &maxfd, a->action_fd);
add_fd(&rfds, &maxfd, a->sync_completed_fd);
+
for (mdi = a->info.devs ; mdi ; mdi = mdi->next) {
+ if (mdi->man_disk_to_remove) {
+ mdi->mon_descriptors_not_used = true;
+
+ /* Managemon could be blocked on suspend in kernel.
+ * Monitor must respond if any badblock is recorded in this time.
+ */
+ container->retry_soon = 1;
+ continue;
+ }
+
add_fd(&rfds, &maxfd, mdi->state_fd);
add_fd(&rfds, &maxfd, mdi->bb_fd);
add_fd(&rfds, &maxfd, mdi->ubb_fd);