Skip to content

Commit 3e16e83

Browse files
Don Bracemartinkpetersen
authored andcommitted
scsi: hpsa: correct race condition in offload enabled
Correct race condition where ioaccel is re-enabled before the raid_map is updated. For RAID_1, RAID_1ADM, and RAID 5/6 there is a BUG_ON called which is bad. - Change event thread to disable ioaccel only. Send all requests down the RAID path instead. - Have rescan thread handle offload_enable. - Since there is only one rescan allowed at a time, turning offload_enabled on/off should not be racy. Each handler queues up a rescan if one is already in progress. - For timing diagram, offload_enabled is initially off due to a change (transformation: splitmirror/remirror), ... otbe = offload_to_be_enabled oe = offload_enabled Time Event Rescan Completion Request Worker Worker Thread Thread ---- ------ ------ ---------- ------- T0 | | + UA | T1 | + rescan started | 0x3f | T2 + Event | | 0x0e | T3 + Ack msg | | | T4 | + if (!dev[i]->oe && | | T5 | | dev[i]->otbe) | | T6 | | get_raid_map | | T7 + otbe = 1 | | | T8 | | | | T9 | + oe = otbe | | T10 | | | + ioaccel request T11 * BUG_ON T0 - I/O completion with UA 0x3f 0x0e sets rescan flag. T1 - rescan worker thread starts a rescan. T2 - event comes in T3 - event thread starts and issues "Acknowledge" message ... T6 - rescan thread has bypassed code to reload new raid map. ... T7 - event thread runs and sets offload_to_be_enabled ... T9 - rescan thread turns on offload_enabled. T10- request comes in and goes down ioaccel path. T11- BUG_ON. - After the patch is applied, ioaccel_enabled can only be re-enabled in the re-scan thread. Link: https://lore.kernel.org/r/158472877894.14200.7077843399036368335.stgit@brunhilda Reviewed-by: Scott Teel <[email protected]> Reviewed-by: Matt Perricone <[email protected]> Reviewed-by: Scott Benesh <[email protected]> Signed-off-by: Don Brace <[email protected]> Signed-off-by: Martin K. Petersen <[email protected]>
1 parent fd6282a commit 3e16e83

File tree

1 file changed

+57
-23
lines changed

1 file changed

+57
-23
lines changed

drivers/scsi/hpsa.c

Lines changed: 57 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,12 @@ static ssize_t host_store_rescan(struct device *dev,
504504
return count;
505505
}
506506

507+
static void hpsa_turn_off_ioaccel_for_device(struct hpsa_scsi_dev_t *device)
508+
{
509+
device->offload_enabled = 0;
510+
device->offload_to_be_enabled = 0;
511+
}
512+
507513
static ssize_t host_show_firmware_revision(struct device *dev,
508514
struct device_attribute *attr, char *buf)
509515
{
@@ -1738,8 +1744,7 @@ static void hpsa_figure_phys_disk_ptrs(struct ctlr_info *h,
17381744
__func__,
17391745
h->scsi_host->host_no, logical_drive->bus,
17401746
logical_drive->target, logical_drive->lun);
1741-
logical_drive->offload_enabled = 0;
1742-
logical_drive->offload_to_be_enabled = 0;
1747+
hpsa_turn_off_ioaccel_for_device(logical_drive);
17431748
logical_drive->queue_depth = 8;
17441749
}
17451750
}
@@ -2499,8 +2504,7 @@ static void process_ioaccel2_completion(struct ctlr_info *h,
24992504
IOACCEL2_SERV_RESPONSE_FAILURE) {
25002505
if (c2->error_data.status ==
25012506
IOACCEL2_STATUS_SR_IOACCEL_DISABLED) {
2502-
dev->offload_enabled = 0;
2503-
dev->offload_to_be_enabled = 0;
2507+
hpsa_turn_off_ioaccel_for_device(dev);
25042508
}
25052509

25062510
if (dev->in_reset) {
@@ -3670,10 +3674,17 @@ static void hpsa_get_ioaccel_status(struct ctlr_info *h,
36703674
this_device->offload_config =
36713675
!!(ioaccel_status & OFFLOAD_CONFIGURED_BIT);
36723676
if (this_device->offload_config) {
3673-
this_device->offload_to_be_enabled =
3677+
bool offload_enabled =
36743678
!!(ioaccel_status & OFFLOAD_ENABLED_BIT);
3675-
if (hpsa_get_raid_map(h, scsi3addr, this_device))
3676-
this_device->offload_to_be_enabled = 0;
3679+
/*
3680+
* Check to see if offload can be enabled.
3681+
*/
3682+
if (offload_enabled) {
3683+
rc = hpsa_get_raid_map(h, scsi3addr, this_device);
3684+
if (rc) /* could not load raid_map */
3685+
goto out;
3686+
this_device->offload_to_be_enabled = 1;
3687+
}
36773688
}
36783689

36793690
out:
@@ -3996,8 +4007,7 @@ static int hpsa_update_device_info(struct ctlr_info *h,
39964007
} else {
39974008
this_device->raid_level = RAID_UNKNOWN;
39984009
this_device->offload_config = 0;
3999-
this_device->offload_enabled = 0;
4000-
this_device->offload_to_be_enabled = 0;
4010+
hpsa_turn_off_ioaccel_for_device(this_device);
40014011
this_device->hba_ioaccel_enabled = 0;
40024012
this_device->volume_offline = 0;
40034013
this_device->queue_depth = h->nr_cmds;
@@ -5230,17 +5240,25 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h,
52305240
/* Handles load balance across RAID 1 members.
52315241
* (2-drive R1 and R10 with even # of drives.)
52325242
* Appropriate for SSDs, not optimal for HDDs
5243+
* Ensure we have the correct raid_map.
52335244
*/
5234-
BUG_ON(le16_to_cpu(map->layout_map_count) != 2);
5245+
if (le16_to_cpu(map->layout_map_count) != 2) {
5246+
hpsa_turn_off_ioaccel_for_device(dev);
5247+
return IO_ACCEL_INELIGIBLE;
5248+
}
52355249
if (dev->offload_to_mirror)
52365250
map_index += le16_to_cpu(map->data_disks_per_row);
52375251
dev->offload_to_mirror = !dev->offload_to_mirror;
52385252
break;
52395253
case HPSA_RAID_ADM:
52405254
/* Handles N-way mirrors (R1-ADM)
52415255
* and R10 with # of drives divisible by 3.)
5256+
* Ensure we have the correct raid_map.
52425257
*/
5243-
BUG_ON(le16_to_cpu(map->layout_map_count) != 3);
5258+
if (le16_to_cpu(map->layout_map_count) != 3) {
5259+
hpsa_turn_off_ioaccel_for_device(dev);
5260+
return IO_ACCEL_INELIGIBLE;
5261+
}
52445262

52455263
offload_to_mirror = dev->offload_to_mirror;
52465264
raid_map_helper(map, offload_to_mirror,
@@ -5265,7 +5283,10 @@ static int hpsa_scsi_ioaccel_raid_map(struct ctlr_info *h,
52655283
r5or6_blocks_per_row =
52665284
le16_to_cpu(map->strip_size) *
52675285
le16_to_cpu(map->data_disks_per_row);
5268-
BUG_ON(r5or6_blocks_per_row == 0);
5286+
if (r5or6_blocks_per_row == 0) {
5287+
hpsa_turn_off_ioaccel_for_device(dev);
5288+
return IO_ACCEL_INELIGIBLE;
5289+
}
52695290
stripesize = r5or6_blocks_per_row *
52705291
le16_to_cpu(map->layout_map_count);
52715292
#if BITS_PER_LONG == 32
@@ -8285,7 +8306,7 @@ static int detect_controller_lockup(struct ctlr_info *h)
82858306
*
82868307
* Called from monitor controller worker (hpsa_event_monitor_worker)
82878308
*
8288-
* A Volume (or Volumes that comprise an Array set may be undergoing a
8309+
* A Volume (or Volumes that comprise an Array set) may be undergoing a
82898310
* transformation, so we will be turning off ioaccel for all volumes that
82908311
* make up the Array.
82918312
*/
@@ -8308,6 +8329,9 @@ static void hpsa_set_ioaccel_status(struct ctlr_info *h)
83088329
* Run through current device list used during I/O requests.
83098330
*/
83108331
for (i = 0; i < h->ndevices; i++) {
8332+
int offload_to_be_enabled = 0;
8333+
int offload_config = 0;
8334+
83118335
device = h->dev[i];
83128336

83138337
if (!device)
@@ -8325,25 +8349,35 @@ static void hpsa_set_ioaccel_status(struct ctlr_info *h)
83258349
continue;
83268350

83278351
ioaccel_status = buf[IOACCEL_STATUS_BYTE];
8328-
device->offload_config =
8352+
8353+
/*
8354+
* Check if offload is still configured on
8355+
*/
8356+
offload_config =
83298357
!!(ioaccel_status & OFFLOAD_CONFIGURED_BIT);
8330-
if (device->offload_config)
8331-
device->offload_to_be_enabled =
8358+
/*
8359+
* If offload is configured on, check to see if ioaccel
8360+
* needs to be enabled.
8361+
*/
8362+
if (offload_config)
8363+
offload_to_be_enabled =
83328364
!!(ioaccel_status & OFFLOAD_ENABLED_BIT);
83338365

8366+
/*
8367+
* If ioaccel is to be re-enabled, re-enable later during the
8368+
* scan operation so the driver can get a fresh raidmap
8369+
* before turning ioaccel back on.
8370+
*/
8371+
if (offload_to_be_enabled)
8372+
continue;
8373+
83348374
/*
83358375
* Immediately turn off ioaccel for any volume the
83368376
* controller tells us to. Some of the reasons could be:
83378377
* transformation - change to the LVs of an Array.
83388378
* degraded volume - component failure
8339-
*
8340-
* If ioaccel is to be re-enabled, re-enable later during the
8341-
* scan operation so the driver can get a fresh raidmap
8342-
* before turning ioaccel back on.
8343-
*
83448379
*/
8345-
if (!device->offload_to_be_enabled)
8346-
device->offload_enabled = 0;
8380+
hpsa_turn_off_ioaccel_for_device(device);
83478381
}
83488382

83498383
kfree(buf);

0 commit comments

Comments
 (0)