From 1840aa0974aa792565175cb7b4ecf1670266760d Mon Sep 17 00:00:00 2001 From: Alasdair Kergon Date: Mon, 13 Jun 2011 03:32:45 +0000 Subject: [PATCH] Maintain a count of the number of suspended devices in libdevmapper and use this for the LVM critical section logic. Also report an error if code tries to load a table while any device is known to be in the suspended state. (If the variety of problems these changes are showing up can't be fixed before the next release, the error messages can be reduced to debug level.) --- WHATS_NEW | 1 + WHATS_NEW_DM | 2 ++ lib/mm/memlock.c | 38 +++++++++++++++++++------------------- libdm/ioctl/libdm-iface.c | 17 +++++++++++++++++ libdm/libdevmapper.h | 5 +++++ libdm/libdm-common.c | 23 +++++++++++++++++++++++ libdm/libdm-common.h | 3 +++ libdm/libdm-deptree.c | 16 +++++++++++----- 8 files changed, 81 insertions(+), 24 deletions(-) diff --git a/WHATS_NEW b/WHATS_NEW index 4f5b4a3a2..c2af237d4 100644 --- a/WHATS_NEW +++ b/WHATS_NEW @@ -1,5 +1,6 @@ Version 2.02.86 - ================================= + Use dm_get_suspended_counter in replacement critical_section logic. Downgrade critical_section errors to debug level until it is moved to libdm. Fix ignored background polling default in vgchange -ay. Fix pvmove activation sequences to avoid trapped I/O with multiple LVs. diff --git a/WHATS_NEW_DM b/WHATS_NEW_DM index 038c18d85..5be3acf80 100644 --- a/WHATS_NEW_DM +++ b/WHATS_NEW_DM @@ -1,5 +1,7 @@ Version 1.02.65 - ================================== + Warn if a table is loaded while a device is known to be in suspended state. + Add dm_get_suspended_counter() for number of devs in suspended state by lib. Fix "all" report field prefix matching to include label fields with pv_all. Delay resuming new preloaded mirror devices with core logs in deptree code. Accept new kernel version 3 uname formats in initialisation. diff --git a/lib/mm/memlock.c b/lib/mm/memlock.c index 201eff2f2..bdda2ca5a 100644 --- a/lib/mm/memlock.c +++ b/lib/mm/memlock.c @@ -75,7 +75,7 @@ static size_t _size_malloc = 2000000; static void *_malloc_mem = NULL; static int _mem_locked = 0; -static int _critical_section_count = 0; +static int _critical_section = 0; static int _memlock_count_daemon = 0; static int _priority; static int _default_priority; @@ -374,10 +374,10 @@ static void _unlock_mem(struct cmd_context *cmd) static void _lock_mem_if_needed(struct cmd_context *cmd) { - log_debug("Lock: Memlock counters: locked:%d critical:%d daemon:%d", - _mem_locked, _critical_section_count, _memlock_count_daemon); + log_debug("Lock: Memlock counters: locked:%d critical:%d daemon:%d suspended:%d", + _mem_locked, _critical_section, _memlock_count_daemon, dm_get_suspended_counter()); if (!_mem_locked && - ((_critical_section_count + _memlock_count_daemon) == 1)) { + ((_critical_section + _memlock_count_daemon) == 1)) { _mem_locked = 1; _lock_mem(cmd); } @@ -385,10 +385,10 @@ static void _lock_mem_if_needed(struct cmd_context *cmd) static void _unlock_mem_if_possible(struct cmd_context *cmd) { - log_debug("Unlock: Memlock counters: locked:%d critical:%d daemon:%d", - _mem_locked, _critical_section_count, _memlock_count_daemon); + log_debug("Unlock: Memlock counters: locked:%d critical:%d daemon:%d suspended:%d", + _mem_locked, _critical_section, _memlock_count_daemon, dm_get_suspended_counter()); if (_mem_locked && - !_critical_section_count && + !_critical_section && !_memlock_count_daemon) { _unlock_mem(cmd); _mem_locked = 0; @@ -397,25 +397,25 @@ static void _unlock_mem_if_possible(struct cmd_context *cmd) void critical_section_inc(struct cmd_context *cmd, const char *reason) { - ++_critical_section_count; - log_debug("critical_section_inc to %d (%s).", _critical_section_count, - reason); + if (!_critical_section) { + _critical_section = 1; + log_debug("Entering critical section (%s).", reason); + } + _lock_mem_if_needed(cmd); } void critical_section_dec(struct cmd_context *cmd, const char *reason) { - /* FIXME Maintain accurate suspended device counter in libdevmapper */ - if (!_critical_section_count) - log_debug("_critical_section has dropped below 0."); - --_critical_section_count; - log_debug("critical_section_dec to %d (%s).", _critical_section_count, - reason); + if (_critical_section && !dm_get_suspended_counter()) { + _critical_section = 0; + log_debug("Leaving critical section (%s).", reason); + } } int critical_section(void) { - return _critical_section_count; + return _critical_section; } /* @@ -428,7 +428,7 @@ int critical_section(void) void memlock_inc_daemon(struct cmd_context *cmd) { ++_memlock_count_daemon; - if (_memlock_count_daemon == 1 && _critical_section_count > 0) + if (_memlock_count_daemon == 1 && _critical_section > 0) log_error(INTERNAL_ERROR "_memlock_inc_daemon used in critical section."); log_debug("memlock_count_daemon inc to %d", _memlock_count_daemon); _lock_mem_if_needed(cmd); @@ -460,7 +460,7 @@ void memlock_reset(void) { log_debug("memlock reset."); _mem_locked = 0; - _critical_section_count = 0; + _critical_section = 0; _memlock_count_daemon = 0; } diff --git a/libdm/ioctl/libdm-iface.c b/libdm/ioctl/libdm-iface.c index ad3434472..fb0d43adb 100644 --- a/libdm/ioctl/libdm-iface.c +++ b/libdm/ioctl/libdm-iface.c @@ -2026,6 +2026,7 @@ int dm_task_run(struct dm_task *dmt) unsigned command; int check_udev; int udev_only; + int suspended_counter; #ifdef DM_COMPAT if (_dm_version == 1) @@ -2057,6 +2058,22 @@ int dm_task_run(struct dm_task *dmt) return 0; } + if ((suspended_counter = dm_get_suspended_counter()) && + dmt->type == DM_DEVICE_RELOAD) + log_error("Performing unsafe table load while %d device(s) " + "are known to be suspended: " + "%s%s %s %s%.0d%s%.0d%s%s", + suspended_counter, + dmt->new_uuid ? "UUID " : "", + dmi->name, + dmi->uuid, + dmt->major > 0 ? "(" : "", + dmt->major > 0 ? dmt->major : 0, + dmt->major > 0 ? ":" : "", + dmt->minor > 0 ? dmt->minor : 0, + dmt->major > 0 && dmt->minor == 0 ? "0" : "", + dmt->major > 0 ? ") " : ""); + /* FIXME Detect and warn if cookie set but should not be. */ repeat_ioctl: if (!(dmi = _do_dm_ioctl(dmt, command, _ioctl_buffer_double_factor))) { diff --git a/libdm/libdevmapper.h b/libdm/libdevmapper.h index 88bddb104..a3ae88070 100644 --- a/libdm/libdevmapper.h +++ b/libdm/libdevmapper.h @@ -74,6 +74,11 @@ void dm_log_init(dm_log_fn fn); */ int dm_log_is_non_default(void); +/* + * Number of devices currently in suspended state (via the library). + */ +int dm_get_suspended_counter(void); + enum { DM_DEVICE_CREATE, DM_DEVICE_RELOAD, diff --git a/libdm/libdm-common.c b/libdm/libdm-common.c index 43d78588b..b2b34b814 100644 --- a/libdm/libdm-common.c +++ b/libdm/libdm-common.c @@ -60,6 +60,7 @@ union semun static char _dm_dir[PATH_MAX] = DEV_DIR DM_DIR; static int _verbose = 0; +static int _suspended_dev_counter = 0; #ifdef HAVE_SELINUX_LABEL_H static struct selabel_handle *_selabel_handle = NULL; @@ -173,6 +174,28 @@ int dm_get_library_version(char *version, size_t size) return 1; } +void inc_suspended(void) +{ + _suspended_dev_counter++; + log_debug("Suspended device counter increased to %d", _suspended_dev_counter); +} + +void dec_suspended(void) +{ + if (!_suspended_dev_counter) { + log_error("Attempted to decrement suspended device counter below zero."); + return; + } + + _suspended_dev_counter--; + log_debug("Suspended device counter reduced to %d", _suspended_dev_counter); +} + +int dm_get_suspended_counter(void) +{ + return _suspended_dev_counter; +} + struct dm_task *dm_task_create(int type) { struct dm_task *dmt = dm_zalloc(sizeof(*dmt)); diff --git a/libdm/libdm-common.h b/libdm/libdm-common.h index 3267cfc49..2435f2747 100644 --- a/libdm/libdm-common.h +++ b/libdm/libdm-common.h @@ -33,4 +33,7 @@ int set_dev_node_read_ahead(const char *dev_name, uint32_t read_ahead, void update_devs(void); void selinux_release(void); +void inc_suspended(void); +void dec_suspended(void); + #endif diff --git a/libdm/libdm-deptree.c b/libdm/libdm-deptree.c index 4860ec278..044b7ffe4 100644 --- a/libdm/libdm-deptree.c +++ b/libdm/libdm-deptree.c @@ -1034,7 +1034,7 @@ out: static int _resume_node(const char *name, uint32_t major, uint32_t minor, uint32_t read_ahead, uint32_t read_ahead_flags, struct dm_info *newinfo, uint32_t *cookie, - uint16_t udev_flags) + uint16_t udev_flags, int already_suspended) { struct dm_task *dmt; int r = 0; @@ -1066,8 +1066,11 @@ static int _resume_node(const char *name, uint32_t major, uint32_t minor, if (!dm_task_set_cookie(dmt, cookie, udev_flags)) goto out; - if ((r = dm_task_run(dmt))) + if ((r = dm_task_run(dmt))) { + if (already_suspended) + dec_suspended(); r = dm_task_get_info(dmt, newinfo); + } out: dm_task_destroy(dmt); @@ -1106,8 +1109,10 @@ static int _suspend_node(const char *name, uint32_t major, uint32_t minor, if (no_flush && !dm_task_no_flush(dmt)) log_error("Failed to set no_flush flag."); - if ((r = dm_task_run(dmt))) + if ((r = dm_task_run(dmt))) { + inc_suspended(); r = dm_task_get_info(dmt, newinfo); + } dm_task_destroy(dmt); @@ -1352,7 +1357,7 @@ int dm_tree_activate_children(struct dm_tree_node *dnode, if (!_resume_node(child->name, child->info.major, child->info.minor, child->props.read_ahead, child->props.read_ahead_flags, - &newinfo, &child->dtree->cookie, child->udev_flags)) { + &newinfo, &child->dtree->cookie, child->udev_flags, child->info.suspended)) { log_error("Unable to resume %s (%" PRIu32 ":%" PRIu32 ")", child->name, child->info.major, child->info.minor); @@ -1919,7 +1924,8 @@ int dm_tree_preload_children(struct dm_tree_node *dnode, if (!_resume_node(child->name, child->info.major, child->info.minor, child->props.read_ahead, child->props.read_ahead_flags, - &newinfo, &child->dtree->cookie, child->udev_flags)) { + &newinfo, &child->dtree->cookie, child->udev_flags, + child->info.suspended)) { log_error("Unable to resume %s (%" PRIu32 ":%" PRIu32 ")", child->name, child->info.major, child->info.minor);