Skip to content

Commit 0558f33

Browse files
JasonYanHwmartinkpetersen
authored andcommitted
scsi: libsas: direct call probe and destruct
In commit 87c8331 ("[SCSI] libsas: prevent domain rediscovery competing with ata error handling") introduced disco mutex to prevent rediscovery competing with ata error handling and put the whole revalidation in the mutex. But the rphy add/remove needs to wait for the error handling which also grabs the disco mutex. This may leads to dead lock.So the probe and destruct event were introduce to do the rphy add/remove asynchronously and out of the lock. The asynchronously processed workers makes the whole discovery process not atomic, the other events may interrupt the process. For example, if a loss of signal event inserted before the probe event, the sas_deform_port() is called and the port will be deleted. And sas_port_delete() may run before the destruct event, but the port-x:x is the top parent of end device or expander. This leads to a kernel WARNING such as: [ 82.042979] sysfs group 'power' not found for kobject 'phy-1:0:22' [ 82.042983] ------------[ cut here ]------------ [ 82.042986] WARNING: CPU: 54 PID: 1714 at fs/sysfs/group.c:237 sysfs_remove_group+0x94/0xa0 [ 82.043059] Call trace: [ 82.043082] [<ffff0000082e7624>] sysfs_remove_group+0x94/0xa0 [ 82.043085] [<ffff00000864e320>] dpm_sysfs_remove+0x60/0x70 [ 82.043086] [<ffff00000863ee10>] device_del+0x138/0x308 [ 82.043089] [<ffff00000869a2d0>] sas_phy_delete+0x38/0x60 [ 82.043091] [<ffff00000869a86c>] do_sas_phy_delete+0x6c/0x80 [ 82.043093] [<ffff00000863dc20>] device_for_each_child+0x58/0xa0 [ 82.043095] [<ffff000008696f80>] sas_remove_children+0x40/0x50 [ 82.043100] [<ffff00000869d1bc>] sas_destruct_devices+0x64/0xa0 [ 82.043102] [<ffff0000080e93bc>] process_one_work+0x1fc/0x4b0 [ 82.043104] [<ffff0000080e96c0>] worker_thread+0x50/0x490 [ 82.043105] [<ffff0000080f0364>] kthread+0xfc/0x128 [ 82.043107] [<ffff0000080836c0>] ret_from_fork+0x10/0x50 Make probe and destruct a direct call in the disco and revalidate function, but put them outside the lock. The whole discovery or revalidate won't be interrupted by other events. And the DISCE_PROBE and DISCE_DESTRUCT event are deleted as a result of the direct call. Introduce a new list to destruct the sas_port and put the port delete after the destruct. This makes sure the right order of destroying the sysfs kobject and fix the warning above. In sas_ex_revalidate_domain() have a loop to find all broadcasted device, and sometimes we have a chance to find the same expander twice. Because the sas_port will be deleted at the end of the whole revalidate process, sas_port with the same name cannot be added before this. Otherwise the sysfs will complain of creating duplicate filename. Since the LLDD will send broadcast for every device change, we can only process one expander's revalidation. [mkp: kbuild test robot warning] Signed-off-by: Jason Yan <yanaijie@huawei.com> CC: John Garry <john.garry@huawei.com> CC: Johannes Thumshirn <jthumshirn@suse.de> CC: Ewan Milne <emilne@redhat.com> CC: Christoph Hellwig <hch@lst.de> CC: Tomas Henzl <thenzl@redhat.com> CC: Dan Williams <dan.j.williams@intel.com> Reviewed-by: Hannes Reinecke <hare@suse.com> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
1 parent 517e515 commit 0558f33

File tree

7 files changed

+27
-22
lines changed

7 files changed

+27
-22
lines changed

Diff for: drivers/scsi/libsas/sas_ata.c

-1
Original file line numberDiff line numberDiff line change
@@ -730,7 +730,6 @@ int sas_discover_sata(struct domain_device *dev)
730730
if (res)
731731
return res;
732732

733-
sas_discover_event(dev->port, DISCE_PROBE);
734733
return 0;
735734
}
736735

Diff for: drivers/scsi/libsas/sas_discover.c

+18-14
Original file line numberDiff line numberDiff line change
@@ -212,13 +212,9 @@ void sas_notify_lldd_dev_gone(struct domain_device *dev)
212212
}
213213
}
214214

215-
static void sas_probe_devices(struct work_struct *work)
215+
static void sas_probe_devices(struct asd_sas_port *port)
216216
{
217217
struct domain_device *dev, *n;
218-
struct sas_discovery_event *ev = to_sas_discovery_event(work);
219-
struct asd_sas_port *port = ev->port;
220-
221-
clear_bit(DISCE_PROBE, &port->disc.pending);
222218

223219
/* devices must be domain members before link recovery and probe */
224220
list_for_each_entry(dev, &port->disco_list, disco_list_node) {
@@ -294,7 +290,6 @@ int sas_discover_end_dev(struct domain_device *dev)
294290
res = sas_notify_lldd_dev_found(dev);
295291
if (res)
296292
return res;
297-
sas_discover_event(dev->port, DISCE_PROBE);
298293

299294
return 0;
300295
}
@@ -353,13 +348,9 @@ static void sas_unregister_common_dev(struct asd_sas_port *port, struct domain_d
353348
sas_put_device(dev);
354349
}
355350

356-
static void sas_destruct_devices(struct work_struct *work)
351+
void sas_destruct_devices(struct asd_sas_port *port)
357352
{
358353
struct domain_device *dev, *n;
359-
struct sas_discovery_event *ev = to_sas_discovery_event(work);
360-
struct asd_sas_port *port = ev->port;
361-
362-
clear_bit(DISCE_DESTRUCT, &port->disc.pending);
363354

364355
list_for_each_entry_safe(dev, n, &port->destroy_list, disco_list_node) {
365356
list_del_init(&dev->disco_list_node);
@@ -370,6 +361,16 @@ static void sas_destruct_devices(struct work_struct *work)
370361
}
371362
}
372363

364+
static void sas_destruct_ports(struct asd_sas_port *port)
365+
{
366+
struct sas_port *sas_port, *p;
367+
368+
list_for_each_entry_safe(sas_port, p, &port->sas_port_del_list, del_list) {
369+
list_del_init(&sas_port->del_list);
370+
sas_port_delete(sas_port);
371+
}
372+
}
373+
373374
void sas_unregister_dev(struct asd_sas_port *port, struct domain_device *dev)
374375
{
375376
if (!test_bit(SAS_DEV_DESTROY, &dev->state) &&
@@ -384,7 +385,6 @@ void sas_unregister_dev(struct asd_sas_port *port, struct domain_device *dev)
384385
if (!test_and_set_bit(SAS_DEV_DESTROY, &dev->state)) {
385386
sas_rphy_unlink(dev->rphy);
386387
list_move_tail(&dev->disco_list_node, &port->destroy_list);
387-
sas_discover_event(dev->port, DISCE_DESTRUCT);
388388
}
389389
}
390390

@@ -490,6 +490,8 @@ static void sas_discover_domain(struct work_struct *work)
490490
port->port_dev = NULL;
491491
}
492492

493+
sas_probe_devices(port);
494+
493495
SAS_DPRINTK("DONE DISCOVERY on port %d, pid:%d, result:%d\n", port->id,
494496
task_pid_nr(current), error);
495497
}
@@ -523,6 +525,10 @@ static void sas_revalidate_domain(struct work_struct *work)
523525
port->id, task_pid_nr(current), res);
524526
out:
525527
mutex_unlock(&ha->disco_mutex);
528+
529+
sas_destruct_devices(port);
530+
sas_destruct_ports(port);
531+
sas_probe_devices(port);
526532
}
527533

528534
/* ---------- Events ---------- */
@@ -578,10 +584,8 @@ void sas_init_disc(struct sas_discovery *disc, struct asd_sas_port *port)
578584
static const work_func_t sas_event_fns[DISC_NUM_EVENTS] = {
579585
[DISCE_DISCOVER_DOMAIN] = sas_discover_domain,
580586
[DISCE_REVALIDATE_DOMAIN] = sas_revalidate_domain,
581-
[DISCE_PROBE] = sas_probe_devices,
582587
[DISCE_SUSPEND] = sas_suspend_devices,
583588
[DISCE_RESUME] = sas_resume_devices,
584-
[DISCE_DESTRUCT] = sas_destruct_devices,
585589
};
586590

587591
disc->pending = 0;

Diff for: drivers/scsi/libsas/sas_expander.c

+3-5
Original file line numberDiff line numberDiff line change
@@ -1916,7 +1916,8 @@ static void sas_unregister_devs_sas_addr(struct domain_device *parent,
19161916
sas_port_delete_phy(phy->port, phy->phy);
19171917
sas_device_set_phy(found, phy->port);
19181918
if (phy->port->num_phys == 0)
1919-
sas_port_delete(phy->port);
1919+
list_add_tail(&phy->port->del_list,
1920+
&parent->port->sas_port_del_list);
19201921
phy->port = NULL;
19211922
}
19221923
}
@@ -2124,7 +2125,7 @@ int sas_ex_revalidate_domain(struct domain_device *port_dev)
21242125
struct domain_device *dev = NULL;
21252126

21262127
res = sas_find_bcast_dev(port_dev, &dev);
2127-
while (res == 0 && dev) {
2128+
if (res == 0 && dev) {
21282129
struct expander_device *ex = &dev->ex_dev;
21292130
int i = 0, phy_id;
21302131

@@ -2136,9 +2137,6 @@ int sas_ex_revalidate_domain(struct domain_device *port_dev)
21362137
res = sas_rediscover(dev, phy_id);
21372138
i = phy_id + 1;
21382139
} while (i < ex->num_phys);
2139-
2140-
dev = NULL;
2141-
res = sas_find_bcast_dev(port_dev, &dev);
21422140
}
21432141
return res;
21442142
}

Diff for: drivers/scsi/libsas/sas_internal.h

+1
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ int sas_try_ata_reset(struct asd_sas_phy *phy);
101101
void sas_hae_reset(struct work_struct *work);
102102

103103
void sas_free_device(struct kref *kref);
104+
void sas_destruct_devices(struct asd_sas_port *port);
104105

105106
extern const work_func_t sas_phy_event_fns[PHY_NUM_EVENTS];
106107
extern const work_func_t sas_port_event_fns[PORT_NUM_EVENTS];

Diff for: drivers/scsi/libsas/sas_port.c

+3
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ static void sas_resume_port(struct asd_sas_phy *phy)
6666
rc = sas_notify_lldd_dev_found(dev);
6767
if (rc) {
6868
sas_unregister_dev(port, dev);
69+
sas_destruct_devices(port);
6970
continue;
7071
}
7172

@@ -220,6 +221,7 @@ void sas_deform_port(struct asd_sas_phy *phy, int gone)
220221

221222
if (port->num_phys == 1) {
222223
sas_unregister_domain_devices(port, gone);
224+
sas_destruct_devices(port);
223225
sas_port_delete(port->port);
224226
port->port = NULL;
225227
} else {
@@ -317,6 +319,7 @@ static void sas_init_port(struct asd_sas_port *port,
317319
INIT_LIST_HEAD(&port->dev_list);
318320
INIT_LIST_HEAD(&port->disco_list);
319321
INIT_LIST_HEAD(&port->destroy_list);
322+
INIT_LIST_HEAD(&port->sas_port_del_list);
320323
spin_lock_init(&port->phy_list_lock);
321324
INIT_LIST_HEAD(&port->phy_list);
322325
port->ha = sas_ha;

Diff for: include/scsi/libsas.h

+1-2
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,8 @@ enum phy_event {
8282
enum discover_event {
8383
DISCE_DISCOVER_DOMAIN = 0U,
8484
DISCE_REVALIDATE_DOMAIN,
85-
DISCE_PROBE,
8685
DISCE_SUSPEND,
8786
DISCE_RESUME,
88-
DISCE_DESTRUCT,
8987
DISC_NUM_EVENTS,
9088
};
9189

@@ -262,6 +260,7 @@ struct asd_sas_port {
262260
struct list_head dev_list;
263261
struct list_head disco_list;
264262
struct list_head destroy_list;
263+
struct list_head sas_port_del_list;
265264
enum sas_linkrate linkrate;
266265

267266
struct sas_work work;

Diff for: include/scsi/scsi_transport_sas.h

+1
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ struct sas_port {
156156

157157
struct mutex phy_list_mutex;
158158
struct list_head phy_list;
159+
struct list_head del_list; /* libsas only */
159160
};
160161

161162
#define dev_to_sas_port(d) \

0 commit comments

Comments
 (0)