When a structure contains bitfields, these are updated by
a read-modify-write of the whole word.
If two of these updates can race, corruption can occurs - updates can
be lost.
To avoid this, it is best to protect updates with a spinlock.
Many updates to the os_* bit fields are already protected by
->os_lock. This patch addes the lock to the remaining updates.
In many cases, this only requires moving a 'lock' earlier, or an
'unlock' later.
Signed-off-by: Mr NeilBrown <neilb@suse.de>
Change-Id: I2335d08cd53dcda98d8046d730829347456a6c5d
Reviewed-on: https://review.whamcloud.com/38974
Tested-by: jenkins <devops@whamcloud.com>
Reviewed-by: Alex Zhuravlev <bzzz@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: James Simmons <jsimmons@infradead.org>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
__u64 os_new_checked;
__u64 os_pos_current;
__u32 os_start_flags;
__u64 os_new_checked;
__u64 os_pos_current;
__u32 os_start_flags;
+ /* Some of these bits can be set by different threads so
+ * all updates must be protected by ->os_lock to avoid
+ * racing read-modify-write cycles causing corruption.
+ */
unsigned int os_in_prior:1, /* process inconsistent item
* found by RPC prior */
os_waiting:1, /* Waiting for scan window. */
unsigned int os_in_prior:1, /* process inconsistent item
* found by RPC prior */
os_waiting:1, /* Waiting for scan window. */
- if (!scrub->os_partial_scan)
+ if (!scrub->os_partial_scan) {
+ spin_lock(&scrub->os_lock);
scrub->os_full_speed = 1;
scrub->os_full_speed = 1;
+ spin_unlock(&scrub->os_lock);
+ }
switch (val) {
case SCRUB_NEXT_NOLMA:
switch (val) {
case SCRUB_NEXT_NOLMA:
GOTO(out, rc = 0);
} else {
GOTO(out, rc = 0);
} else {
- if (!scrub->os_partial_scan)
+ if (!scrub->os_partial_scan) {
+ spin_lock(&scrub->os_lock);
scrub->os_full_speed = 1;
scrub->os_full_speed = 1;
+ spin_unlock(&scrub->os_lock);
+ }
sf->sf_flags |= SF_INCONSISTENT;
/* XXX: If the device is restored from file-level backup, then
sf->sf_flags |= SF_INCONSISTENT;
/* XXX: If the device is restored from file-level backup, then
if (flags & SS_RESET)
scrub_file_reset(scrub, dev->od_uuid, 0);
if (flags & SS_RESET)
scrub_file_reset(scrub, dev->od_uuid, 0);
+ spin_lock(&scrub->os_lock);
if (flags & SS_AUTO_FULL) {
scrub->os_full_speed = 1;
scrub->os_partial_scan = 0;
if (flags & SS_AUTO_FULL) {
scrub->os_full_speed = 1;
scrub->os_partial_scan = 0;
scrub->os_partial_scan = 0;
}
scrub->os_partial_scan = 0;
}
- spin_lock(&scrub->os_lock);
scrub->os_in_prior = 0;
scrub->os_waiting = 0;
scrub->os_paused = 0;
scrub->os_in_prior = 0;
scrub->os_waiting = 0;
scrub->os_paused = 0;
oii = list_entry(scrub->os_inconsistent_items.next,
struct osd_inconsistent_item, oii_list);
oii = list_entry(scrub->os_inconsistent_items.next,
struct osd_inconsistent_item, oii_list);
- spin_unlock(&scrub->os_lock);
*oic = &oii->oii_cache;
scrub->os_in_prior = 1;
*oic = &oii->oii_cache;
scrub->os_in_prior = 1;
+ spin_unlock(&scrub->os_lock);
rc = osd_scrub_check_update(info, dev, oic, rc);
if (rc != 0) {
rc = osd_scrub_check_update(info, dev, oic, rc);
if (rc != 0) {
+ spin_lock(&scrub->os_lock);
+ spin_unlock(&scrub->os_lock);
}
if (scrub->os_in_prior) {
}
if (scrub->os_in_prior) {
+ spin_lock(&scrub->os_lock);
+ spin_unlock(&scrub->os_lock);
LASSERT(!(flags & SS_AUTO_PARTIAL));
down_write(&scrub->os_rwsem);
LASSERT(!(flags & SS_AUTO_PARTIAL));
down_write(&scrub->os_rwsem);
+ spin_lock(&scrub->os_lock);
scrub->os_in_join = 1;
if (flags & SS_SET_FAILOUT)
sf->sf_param |= SP_FAILOUT;
scrub->os_in_join = 1;
if (flags & SS_SET_FAILOUT)
sf->sf_param |= SP_FAILOUT;
sf->sf_flags |= SF_AUTO;
scrub->os_full_speed = 1;
}
sf->sf_flags |= SF_AUTO;
scrub->os_full_speed = 1;
}
+ spin_unlock(&scrub->os_lock);
scrub->os_new_checked = 0;
if (sf->sf_pos_last_checkpoint != 0)
scrub->os_new_checked = 0;
if (sf->sf_pos_last_checkpoint != 0)
* FID directly, instead, the OI scrub will scan the OI structure
* and try to re-generate the LMA from the OI mapping. But if the
* OI mapping crashed or lost also, then we have to give up under
* FID directly, instead, the OI scrub will scan the OI structure
* and try to re-generate the LMA from the OI mapping. But if the
* OI mapping crashed or lost also, then we have to give up under
- * double failure cases. */
+ * double failure cases.
+ */
+ spin_lock(&scrub->os_lock);
scrub->os_convert_igif = 1;
scrub->os_convert_igif = 1;
+ spin_unlock(&scrub->os_lock);
child = osd_lookup_one_len_unlocked(dev, dot_lustre_name, dentry,
strlen(dot_lustre_name));
if (IS_ERR(child)) {
child = osd_lookup_one_len_unlocked(dev, dot_lustre_name, dentry,
strlen(dot_lustre_name));
if (IS_ERR(child)) {
/* od_otable_mutex: prevent curcurrent start/stop */
mutex_lock(&dev->od_otable_mutex);
/* od_otable_mutex: prevent curcurrent start/stop */
mutex_lock(&dev->od_otable_mutex);
+ spin_lock(&scrub->os_lock);
+ spin_unlock(&scrub->os_lock);
scrub_stop(scrub);
mutex_unlock(&dev->od_otable_mutex);
}
scrub_stop(scrub);
mutex_unlock(&dev->od_otable_mutex);
}
oii->oii_cache = *oic;
oii->oii_insert = insert;
oii->oii_cache = *oic;
oii->oii_insert = insert;
+ spin_lock(&lscrub->os_lock);
if (lscrub->os_partial_scan) {
__u64 now = ktime_get_real_seconds();
if (lscrub->os_partial_scan) {
__u64 now = ktime_get_real_seconds();
lscrub->os_full_scrub = 1;
}
lscrub->os_full_scrub = 1;
}
- spin_lock(&lscrub->os_lock);
if (unlikely(!thread_is_running(thread))) {
spin_unlock(&lscrub->os_lock);
OBD_FREE_PTR(oii);
if (unlikely(!thread_is_running(thread))) {
spin_unlock(&lscrub->os_lock);
OBD_FREE_PTR(oii);
+ spin_lock(&scrub->os_lock);
scrub->os_full_speed = 1;
scrub->os_full_speed = 1;
+ spin_unlock(&scrub->os_lock);
+
sf->sf_flags |= SF_INCONSISTENT;
} else if (oid == oid2) {
GOTO(out, rc = 0);
sf->sf_flags |= SF_INCONSISTENT;
} else if (oid == oid2) {
GOTO(out, rc = 0);
+ spin_lock(&scrub->os_lock);
scrub->os_full_speed = 1;
scrub->os_full_speed = 1;
+ spin_unlock(&scrub->os_lock);
sf->sf_flags |= SF_INCONSISTENT;
}
sf->sf_flags |= SF_INCONSISTENT;
}
if (flags & SS_RESET)
scrub_file_reset(scrub, dev->od_uuid, 0);
if (flags & SS_RESET)
scrub_file_reset(scrub, dev->od_uuid, 0);
+ spin_lock(&scrub->os_lock);
scrub->os_partial_scan = 0;
if (flags & SS_AUTO_FULL) {
scrub->os_full_speed = 1;
scrub->os_partial_scan = 0;
if (flags & SS_AUTO_FULL) {
scrub->os_full_speed = 1;
scrub->os_full_speed = 0;
}
scrub->os_full_speed = 0;
}
- spin_lock(&scrub->os_lock);
scrub->os_in_prior = 0;
scrub->os_waiting = 0;
scrub->os_paused = 0;
scrub->os_in_prior = 0;
scrub->os_waiting = 0;
scrub->os_paused = 0;
spin_unlock(&scrub->os_lock);
}
} else {
spin_unlock(&scrub->os_lock);
}
} else {
+ spin_lock(&scrub->os_lock);
+ spin_unlock(&scrub->os_lock);
/* od_otable_sem: prevent concurrent start/stop */
down(&dev->od_otable_sem);
/* od_otable_sem: prevent concurrent start/stop */
down(&dev->od_otable_sem);
+ spin_lock(&scrub->os_lock);
+ spin_unlock(&scrub->os_lock);
scrub_stop(scrub);
up(&dev->od_otable_sem);
scrub_stop(scrub);
up(&dev->od_otable_sem);