Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

[SCSI] libsas: introduce sas_work to fix sas_drain_work vs sas_queue_work

When requeuing work to a draining workqueue the last work instance may
not be idle, so sas_queue_work() must not touch work->entry. Introduce
sas_work with a drain_node list_head to have a private list for
collecting work deferred due to drain collision.

Fixes reports like:
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [<ffffffff810410d4>] process_one_work+0x2e/0x338

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: James Bottomley <JBottomley@Parallels.com>

authored by

Dan Williams and committed by
James Bottomley
22b9153f f8fc75dc

+83 -62
+14 -14
drivers/scsi/libsas/sas_discover.c
··· 205 205 static void sas_probe_devices(struct work_struct *work) 206 206 { 207 207 struct domain_device *dev, *n; 208 - struct sas_discovery_event *ev = 209 - container_of(work, struct sas_discovery_event, work); 208 + struct sas_discovery_event *ev = to_sas_discovery_event(work); 210 209 struct asd_sas_port *port = ev->port; 211 210 212 211 clear_bit(DISCE_PROBE, &port->disc.pending); ··· 290 291 static void sas_destruct_devices(struct work_struct *work) 291 292 { 292 293 struct domain_device *dev, *n; 293 - struct sas_discovery_event *ev = 294 - container_of(work, struct sas_discovery_event, work); 294 + struct sas_discovery_event *ev = to_sas_discovery_event(work); 295 295 struct asd_sas_port *port = ev->port; 296 296 297 297 clear_bit(DISCE_DESTRUCT, &port->disc.pending); ··· 375 377 { 376 378 struct domain_device *dev; 377 379 int error = 0; 378 - struct sas_discovery_event *ev = 379 - container_of(work, struct sas_discovery_event, work); 380 + struct sas_discovery_event *ev = to_sas_discovery_event(work); 380 381 struct asd_sas_port *port = ev->port; 381 382 382 383 clear_bit(DISCE_DISCOVER_DOMAIN, &port->disc.pending); ··· 434 437 static void sas_revalidate_domain(struct work_struct *work) 435 438 { 436 439 int res = 0; 437 - struct sas_discovery_event *ev = 438 - container_of(work, struct sas_discovery_event, work); 440 + struct sas_discovery_event *ev = to_sas_discovery_event(work); 439 441 struct asd_sas_port *port = ev->port; 440 442 struct sas_ha_struct *ha = port->ha; 441 443 ··· 462 466 463 467 /* ---------- Events ---------- */ 464 468 465 - static void sas_chain_work(struct sas_ha_struct *ha, struct work_struct *work) 469 + static void sas_chain_work(struct sas_ha_struct *ha, struct sas_work *sw) 466 470 { 467 - /* chained work is not subject to SA_HA_DRAINING or SAS_HA_REGISTERED */ 468 - scsi_queue_work(ha->core.shost, work); 471 + /* chained work is not subject to SA_HA_DRAINING or 472 + * SAS_HA_REGISTERED, because it is either submitted in the 473 + * workqueue, or known to be submitted from a context that is 474 + * not racing against draining 475 + */ 476 + scsi_queue_work(ha->core.shost, &sw->work); 469 477 } 470 478 471 479 static void sas_chain_event(int event, unsigned long *pending, 472 - struct work_struct *work, 480 + struct sas_work *sw, 473 481 struct sas_ha_struct *ha) 474 482 { 475 483 if (!test_and_set_bit(event, pending)) { 476 484 unsigned long flags; 477 485 478 486 spin_lock_irqsave(&ha->state_lock, flags); 479 - sas_chain_work(ha, work); 487 + sas_chain_work(ha, sw); 480 488 spin_unlock_irqrestore(&ha->state_lock, flags); 481 489 } 482 490 } ··· 519 519 520 520 disc->pending = 0; 521 521 for (i = 0; i < DISC_NUM_EVENTS; i++) { 522 - INIT_WORK(&disc->disc_work[i].work, sas_event_fns[i]); 522 + INIT_SAS_WORK(&disc->disc_work[i].work, sas_event_fns[i]); 523 523 disc->disc_work[i].port = port; 524 524 } 525 525 }
+13 -11
drivers/scsi/libsas/sas_event.c
··· 27 27 #include "sas_internal.h" 28 28 #include "sas_dump.h" 29 29 30 - void sas_queue_work(struct sas_ha_struct *ha, struct work_struct *work) 30 + void sas_queue_work(struct sas_ha_struct *ha, struct sas_work *sw) 31 31 { 32 32 if (!test_bit(SAS_HA_REGISTERED, &ha->state)) 33 33 return; 34 34 35 - if (test_bit(SAS_HA_DRAINING, &ha->state)) 36 - list_add(&work->entry, &ha->defer_q); 37 - else 38 - scsi_queue_work(ha->core.shost, work); 35 + if (test_bit(SAS_HA_DRAINING, &ha->state)) { 36 + /* add it to the defer list, if not already pending */ 37 + if (list_empty(&sw->drain_node)) 38 + list_add(&sw->drain_node, &ha->defer_q); 39 + } else 40 + scsi_queue_work(ha->core.shost, &sw->work); 39 41 } 40 42 41 43 static void sas_queue_event(int event, unsigned long *pending, 42 - struct work_struct *work, 44 + struct sas_work *work, 43 45 struct sas_ha_struct *ha) 44 46 { 45 47 if (!test_and_set_bit(event, pending)) { ··· 57 55 void __sas_drain_work(struct sas_ha_struct *ha) 58 56 { 59 57 struct workqueue_struct *wq = ha->core.shost->work_q; 60 - struct work_struct *w, *_w; 58 + struct sas_work *sw, *_sw; 61 59 62 60 set_bit(SAS_HA_DRAINING, &ha->state); 63 61 /* flush submitters */ ··· 68 66 69 67 spin_lock_irq(&ha->state_lock); 70 68 clear_bit(SAS_HA_DRAINING, &ha->state); 71 - list_for_each_entry_safe(w, _w, &ha->defer_q, entry) { 72 - list_del_init(&w->entry); 73 - sas_queue_work(ha, w); 69 + list_for_each_entry_safe(sw, _sw, &ha->defer_q, drain_node) { 70 + list_del_init(&sw->drain_node); 71 + sas_queue_work(ha, sw); 74 72 } 75 73 spin_unlock_irq(&ha->state_lock); 76 74 } ··· 153 151 int i; 154 152 155 153 for (i = 0; i < HA_NUM_EVENTS; i++) { 156 - INIT_WORK(&sas_ha->ha_events[i].work, sas_ha_event_fns[i]); 154 + INIT_SAS_WORK(&sas_ha->ha_events[i].work, sas_ha_event_fns[i]); 157 155 sas_ha->ha_events[i].ha = sas_ha; 158 156 } 159 157
+5 -6
drivers/scsi/libsas/sas_init.c
··· 94 94 95 95 void sas_hae_reset(struct work_struct *work) 96 96 { 97 - struct sas_ha_event *ev = 98 - container_of(work, struct sas_ha_event, work); 97 + struct sas_ha_event *ev = to_sas_ha_event(work); 99 98 struct sas_ha_struct *ha = ev->ha; 100 99 101 100 clear_bit(HAE_RESET, &ha->pending); ··· 368 369 369 370 static void phy_reset_work(struct work_struct *work) 370 371 { 371 - struct sas_phy_data *d = container_of(work, typeof(*d), reset_work); 372 + struct sas_phy_data *d = container_of(work, typeof(*d), reset_work.work); 372 373 373 374 d->reset_result = transport_sas_phy_reset(d->phy, d->hard_reset); 374 375 } 375 376 376 377 static void phy_enable_work(struct work_struct *work) 377 378 { 378 - struct sas_phy_data *d = container_of(work, typeof(*d), enable_work); 379 + struct sas_phy_data *d = container_of(work, typeof(*d), enable_work.work); 379 380 380 381 d->enable_result = sas_phy_enable(d->phy, d->enable); 381 382 } ··· 388 389 return -ENOMEM; 389 390 390 391 mutex_init(&d->event_lock); 391 - INIT_WORK(&d->reset_work, phy_reset_work); 392 - INIT_WORK(&d->enable_work, phy_enable_work); 392 + INIT_SAS_WORK(&d->reset_work, phy_reset_work); 393 + INIT_SAS_WORK(&d->enable_work, phy_enable_work); 393 394 d->phy = phy; 394 395 phy->hostdata = d; 395 396
+3 -3
drivers/scsi/libsas/sas_internal.h
··· 45 45 struct mutex event_lock; 46 46 int hard_reset; 47 47 int reset_result; 48 - struct work_struct reset_work; 48 + struct sas_work reset_work; 49 49 int enable; 50 50 int enable_result; 51 - struct work_struct enable_work; 51 + struct sas_work enable_work; 52 52 }; 53 53 54 54 void sas_scsi_recover_host(struct Scsi_Host *shost); ··· 80 80 void sas_porte_link_reset_err(struct work_struct *work); 81 81 void sas_porte_timer_event(struct work_struct *work); 82 82 void sas_porte_hard_reset(struct work_struct *work); 83 - void sas_queue_work(struct sas_ha_struct *ha, struct work_struct *work); 83 + void sas_queue_work(struct sas_ha_struct *ha, struct sas_work *sw); 84 84 85 85 int sas_notify_lldd_dev_found(struct domain_device *); 86 86 void sas_notify_lldd_dev_gone(struct domain_device *);
+7 -14
drivers/scsi/libsas/sas_phy.c
··· 32 32 33 33 static void sas_phye_loss_of_signal(struct work_struct *work) 34 34 { 35 - struct asd_sas_event *ev = 36 - container_of(work, struct asd_sas_event, work); 35 + struct asd_sas_event *ev = to_asd_sas_event(work); 37 36 struct asd_sas_phy *phy = ev->phy; 38 37 39 38 clear_bit(PHYE_LOSS_OF_SIGNAL, &phy->phy_events_pending); ··· 42 43 43 44 static void sas_phye_oob_done(struct work_struct *work) 44 45 { 45 - struct asd_sas_event *ev = 46 - container_of(work, struct asd_sas_event, work); 46 + struct asd_sas_event *ev = to_asd_sas_event(work); 47 47 struct asd_sas_phy *phy = ev->phy; 48 48 49 49 clear_bit(PHYE_OOB_DONE, &phy->phy_events_pending); ··· 51 53 52 54 static void sas_phye_oob_error(struct work_struct *work) 53 55 { 54 - struct asd_sas_event *ev = 55 - container_of(work, struct asd_sas_event, work); 56 + struct asd_sas_event *ev = to_asd_sas_event(work); 56 57 struct asd_sas_phy *phy = ev->phy; 57 58 struct sas_ha_struct *sas_ha = phy->ha; 58 59 struct asd_sas_port *port = phy->port; ··· 82 85 83 86 static void sas_phye_spinup_hold(struct work_struct *work) 84 87 { 85 - struct asd_sas_event *ev = 86 - container_of(work, struct asd_sas_event, work); 88 + struct asd_sas_event *ev = to_asd_sas_event(work); 87 89 struct asd_sas_phy *phy = ev->phy; 88 90 struct sas_ha_struct *sas_ha = phy->ha; 89 91 struct sas_internal *i = ··· 123 127 phy->error = 0; 124 128 INIT_LIST_HEAD(&phy->port_phy_el); 125 129 for (k = 0; k < PORT_NUM_EVENTS; k++) { 126 - INIT_WORK(&phy->port_events[k].work, 127 - sas_port_event_fns[k]); 130 + INIT_SAS_WORK(&phy->port_events[k].work, sas_port_event_fns[k]); 128 131 phy->port_events[k].phy = phy; 129 132 } 130 133 131 134 for (k = 0; k < PHY_NUM_EVENTS; k++) { 132 - INIT_WORK(&phy->phy_events[k].work, 133 - sas_phy_event_fns[k]); 135 + INIT_SAS_WORK(&phy->phy_events[k].work, sas_phy_event_fns[k]); 134 136 phy->phy_events[k].phy = phy; 135 137 } 136 138 ··· 138 144 spin_lock_init(&phy->sas_prim_lock); 139 145 phy->frame_rcvd_size = 0; 140 146 141 - phy->phy = sas_phy_alloc(&sas_ha->core.shost->shost_gendev, 142 - i); 147 + phy->phy = sas_phy_alloc(&sas_ha->core.shost->shost_gendev, i); 143 148 if (!phy->phy) 144 149 return -ENOMEM; 145 150
+5 -10
drivers/scsi/libsas/sas_port.c
··· 208 208 209 209 void sas_porte_bytes_dmaed(struct work_struct *work) 210 210 { 211 - struct asd_sas_event *ev = 212 - container_of(work, struct asd_sas_event, work); 211 + struct asd_sas_event *ev = to_asd_sas_event(work); 213 212 struct asd_sas_phy *phy = ev->phy; 214 213 215 214 clear_bit(PORTE_BYTES_DMAED, &phy->port_events_pending); ··· 218 219 219 220 void sas_porte_broadcast_rcvd(struct work_struct *work) 220 221 { 221 - struct asd_sas_event *ev = 222 - container_of(work, struct asd_sas_event, work); 222 + struct asd_sas_event *ev = to_asd_sas_event(work); 223 223 struct asd_sas_phy *phy = ev->phy; 224 224 unsigned long flags; 225 225 u32 prim; ··· 235 237 236 238 void sas_porte_link_reset_err(struct work_struct *work) 237 239 { 238 - struct asd_sas_event *ev = 239 - container_of(work, struct asd_sas_event, work); 240 + struct asd_sas_event *ev = to_asd_sas_event(work); 240 241 struct asd_sas_phy *phy = ev->phy; 241 242 242 243 clear_bit(PORTE_LINK_RESET_ERR, &phy->port_events_pending); ··· 245 248 246 249 void sas_porte_timer_event(struct work_struct *work) 247 250 { 248 - struct asd_sas_event *ev = 249 - container_of(work, struct asd_sas_event, work); 251 + struct asd_sas_event *ev = to_asd_sas_event(work); 250 252 struct asd_sas_phy *phy = ev->phy; 251 253 252 254 clear_bit(PORTE_TIMER_EVENT, &phy->port_events_pending); ··· 255 259 256 260 void sas_porte_hard_reset(struct work_struct *work) 257 261 { 258 - struct asd_sas_event *ev = 259 - container_of(work, struct asd_sas_event, work); 262 + struct asd_sas_event *ev = to_asd_sas_event(work); 260 263 struct asd_sas_phy *phy = ev->phy; 261 264 262 265 clear_bit(PORTE_HARD_RESET, &phy->port_events_pending);
+36 -4
include/scsi/libsas.h
··· 217 217 struct kref kref; 218 218 }; 219 219 220 - struct sas_discovery_event { 220 + struct sas_work { 221 + struct list_head drain_node; 221 222 struct work_struct work; 223 + }; 224 + 225 + static inline void INIT_SAS_WORK(struct sas_work *sw, void (*fn)(struct work_struct *)) 226 + { 227 + INIT_WORK(&sw->work, fn); 228 + INIT_LIST_HEAD(&sw->drain_node); 229 + } 230 + 231 + struct sas_discovery_event { 232 + struct sas_work work; 222 233 struct asd_sas_port *port; 223 234 }; 235 + 236 + static inline struct sas_discovery_event *to_sas_discovery_event(struct work_struct *work) 237 + { 238 + struct sas_discovery_event *ev = container_of(work, typeof(*ev), work.work); 239 + 240 + return ev; 241 + } 224 242 225 243 struct sas_discovery { 226 244 struct sas_discovery_event disc_work[DISC_NUM_EVENTS]; ··· 262 244 struct list_head destroy_list; 263 245 enum sas_linkrate linkrate; 264 246 265 - struct work_struct work; 247 + struct sas_work work; 266 248 267 249 /* public: */ 268 250 int id; ··· 288 270 }; 289 271 290 272 struct asd_sas_event { 291 - struct work_struct work; 273 + struct sas_work work; 292 274 struct asd_sas_phy *phy; 293 275 }; 276 + 277 + static inline struct asd_sas_event *to_asd_sas_event(struct work_struct *work) 278 + { 279 + struct asd_sas_event *ev = container_of(work, typeof(*ev), work.work); 280 + 281 + return ev; 282 + } 294 283 295 284 /* The phy pretty much is controlled by the LLDD. 296 285 * The class only reads those fields. ··· 358 333 }; 359 334 360 335 struct sas_ha_event { 361 - struct work_struct work; 336 + struct sas_work work; 362 337 struct sas_ha_struct *ha; 363 338 }; 339 + 340 + static inline struct sas_ha_event *to_sas_ha_event(struct work_struct *work) 341 + { 342 + struct sas_ha_event *ev = container_of(work, typeof(*ev), work.work); 343 + 344 + return ev; 345 + } 364 346 365 347 enum sas_ha_state { 366 348 SAS_HA_REGISTERED,