Commit c8183f54 authored by Julian Wiedmann's avatar Julian Wiedmann Committed by David S. Miller

s390/qeth: fix potential deadlock on workqueue flush

The L2 bridgeport code uses the coarse 'conf_mutex' for guarding access
to its configuration state.
This can result in a deadlock when qeth_l2_stop_card() - called under the
conf_mutex - blocks on flush_workqueue() to wait for the completion of
pending bridgeport workers. Such workers would also need to aquire
the conf_mutex, stalling indefinitely.

Introduce a lock that specifically guards the bridgeport configuration,
so that the workers no longer need the conf_mutex.
Wrapping qeth_l2_promisc_to_bridge() in this fine-grained lock then also
fixes a theoretical race against a concurrent qeth_bridge_port_role_store()
operation.

Fixes: c0a2e4d1 ("s390/qeth: conclude all event processing before offlining a card")
Signed-off-by: default avatarJulian Wiedmann <jwi@linux.ibm.com>
Reviewed-by: default avatarAlexandra Winter <wintera@linux.ibm.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 004b3942
...@@ -839,6 +839,7 @@ struct qeth_card { ...@@ -839,6 +839,7 @@ struct qeth_card {
struct service_level qeth_service_level; struct service_level qeth_service_level;
struct qdio_ssqd_desc ssqd; struct qdio_ssqd_desc ssqd;
debug_info_t *debug; debug_info_t *debug;
struct mutex sbp_lock;
struct mutex conf_mutex; struct mutex conf_mutex;
struct mutex discipline_mutex; struct mutex discipline_mutex;
struct napi_struct napi; struct napi_struct napi;
......
...@@ -467,10 +467,14 @@ static void qeth_l2_set_promisc_mode(struct qeth_card *card) ...@@ -467,10 +467,14 @@ static void qeth_l2_set_promisc_mode(struct qeth_card *card)
if (card->info.promisc_mode == enable) if (card->info.promisc_mode == enable)
return; return;
if (qeth_adp_supported(card, IPA_SETADP_SET_PROMISC_MODE)) if (qeth_adp_supported(card, IPA_SETADP_SET_PROMISC_MODE)) {
qeth_setadp_promisc_mode(card, enable); qeth_setadp_promisc_mode(card, enable);
else if (card->options.sbp.reflect_promisc) } else {
mutex_lock(&card->sbp_lock);
if (card->options.sbp.reflect_promisc)
qeth_l2_promisc_to_bridge(card, enable); qeth_l2_promisc_to_bridge(card, enable);
mutex_unlock(&card->sbp_lock);
}
} }
/* New MAC address is added to the hash table and marked to be written on card /* New MAC address is added to the hash table and marked to be written on card
...@@ -631,6 +635,7 @@ static int qeth_l2_probe_device(struct ccwgroup_device *gdev) ...@@ -631,6 +635,7 @@ static int qeth_l2_probe_device(struct ccwgroup_device *gdev)
int rc; int rc;
qeth_l2_vnicc_set_defaults(card); qeth_l2_vnicc_set_defaults(card);
mutex_init(&card->sbp_lock);
if (gdev->dev.type == &qeth_generic_devtype) { if (gdev->dev.type == &qeth_generic_devtype) {
rc = qeth_l2_create_device_attributes(&gdev->dev); rc = qeth_l2_create_device_attributes(&gdev->dev);
...@@ -804,10 +809,12 @@ static int qeth_l2_set_online(struct ccwgroup_device *gdev) ...@@ -804,10 +809,12 @@ static int qeth_l2_set_online(struct ccwgroup_device *gdev)
} else } else
card->info.hwtrap = 0; card->info.hwtrap = 0;
mutex_lock(&card->sbp_lock);
qeth_bridgeport_query_support(card); qeth_bridgeport_query_support(card);
if (card->options.sbp.supported_funcs) if (card->options.sbp.supported_funcs)
dev_info(&card->gdev->dev, dev_info(&card->gdev->dev,
"The device represents a Bridge Capable Port\n"); "The device represents a Bridge Capable Port\n");
mutex_unlock(&card->sbp_lock);
qeth_l2_register_dev_addr(card); qeth_l2_register_dev_addr(card);
...@@ -1162,9 +1169,9 @@ static void qeth_bridge_state_change_worker(struct work_struct *work) ...@@ -1162,9 +1169,9 @@ static void qeth_bridge_state_change_worker(struct work_struct *work)
/* Role should not change by itself, but if it did, */ /* Role should not change by itself, but if it did, */
/* information from the hardware is authoritative. */ /* information from the hardware is authoritative. */
mutex_lock(&data->card->conf_mutex); mutex_lock(&data->card->sbp_lock);
data->card->options.sbp.role = entry->role; data->card->options.sbp.role = entry->role;
mutex_unlock(&data->card->conf_mutex); mutex_unlock(&data->card->sbp_lock);
snprintf(env_locrem, sizeof(env_locrem), "BRIDGEPORT=statechange"); snprintf(env_locrem, sizeof(env_locrem), "BRIDGEPORT=statechange");
snprintf(env_role, sizeof(env_role), "ROLE=%s", snprintf(env_role, sizeof(env_role), "ROLE=%s",
...@@ -1230,9 +1237,9 @@ static void qeth_bridge_host_event_worker(struct work_struct *work) ...@@ -1230,9 +1237,9 @@ static void qeth_bridge_host_event_worker(struct work_struct *work)
: (data->hostevs.lost_event_mask == 0x02) : (data->hostevs.lost_event_mask == 0x02)
? "Bridge port state change" ? "Bridge port state change"
: "Unknown reason"); : "Unknown reason");
mutex_lock(&data->card->conf_mutex); mutex_lock(&data->card->sbp_lock);
data->card->options.sbp.hostnotification = 0; data->card->options.sbp.hostnotification = 0;
mutex_unlock(&data->card->conf_mutex); mutex_unlock(&data->card->sbp_lock);
qeth_bridge_emit_host_event(data->card, anev_abort, qeth_bridge_emit_host_event(data->card, anev_abort,
0, NULL, NULL); 0, NULL, NULL);
} else } else
......
...@@ -24,6 +24,7 @@ static ssize_t qeth_bridge_port_role_state_show(struct device *dev, ...@@ -24,6 +24,7 @@ static ssize_t qeth_bridge_port_role_state_show(struct device *dev,
if (qeth_l2_vnicc_is_in_use(card)) if (qeth_l2_vnicc_is_in_use(card))
return sprintf(buf, "n/a (VNIC characteristics)\n"); return sprintf(buf, "n/a (VNIC characteristics)\n");
mutex_lock(&card->sbp_lock);
if (qeth_card_hw_is_reachable(card) && if (qeth_card_hw_is_reachable(card) &&
card->options.sbp.supported_funcs) card->options.sbp.supported_funcs)
rc = qeth_bridgeport_query_ports(card, rc = qeth_bridgeport_query_ports(card,
...@@ -57,6 +58,7 @@ static ssize_t qeth_bridge_port_role_state_show(struct device *dev, ...@@ -57,6 +58,7 @@ static ssize_t qeth_bridge_port_role_state_show(struct device *dev,
else else
rc = sprintf(buf, "%s\n", word); rc = sprintf(buf, "%s\n", word);
} }
mutex_unlock(&card->sbp_lock);
return rc; return rc;
} }
...@@ -91,6 +93,7 @@ static ssize_t qeth_bridge_port_role_store(struct device *dev, ...@@ -91,6 +93,7 @@ static ssize_t qeth_bridge_port_role_store(struct device *dev,
return -EINVAL; return -EINVAL;
mutex_lock(&card->conf_mutex); mutex_lock(&card->conf_mutex);
mutex_lock(&card->sbp_lock);
if (qeth_l2_vnicc_is_in_use(card)) if (qeth_l2_vnicc_is_in_use(card))
rc = -EBUSY; rc = -EBUSY;
...@@ -104,6 +107,7 @@ static ssize_t qeth_bridge_port_role_store(struct device *dev, ...@@ -104,6 +107,7 @@ static ssize_t qeth_bridge_port_role_store(struct device *dev,
} else } else
card->options.sbp.role = role; card->options.sbp.role = role;
mutex_unlock(&card->sbp_lock);
mutex_unlock(&card->conf_mutex); mutex_unlock(&card->conf_mutex);
return rc ? rc : count; return rc ? rc : count;
...@@ -158,6 +162,7 @@ static ssize_t qeth_bridgeport_hostnotification_store(struct device *dev, ...@@ -158,6 +162,7 @@ static ssize_t qeth_bridgeport_hostnotification_store(struct device *dev,
return rc; return rc;
mutex_lock(&card->conf_mutex); mutex_lock(&card->conf_mutex);
mutex_lock(&card->sbp_lock);
if (qeth_l2_vnicc_is_in_use(card)) if (qeth_l2_vnicc_is_in_use(card))
rc = -EBUSY; rc = -EBUSY;
...@@ -168,6 +173,7 @@ static ssize_t qeth_bridgeport_hostnotification_store(struct device *dev, ...@@ -168,6 +173,7 @@ static ssize_t qeth_bridgeport_hostnotification_store(struct device *dev,
} else } else
card->options.sbp.hostnotification = enable; card->options.sbp.hostnotification = enable;
mutex_unlock(&card->sbp_lock);
mutex_unlock(&card->conf_mutex); mutex_unlock(&card->conf_mutex);
return rc ? rc : count; return rc ? rc : count;
...@@ -223,6 +229,7 @@ static ssize_t qeth_bridgeport_reflect_store(struct device *dev, ...@@ -223,6 +229,7 @@ static ssize_t qeth_bridgeport_reflect_store(struct device *dev,
return -EINVAL; return -EINVAL;
mutex_lock(&card->conf_mutex); mutex_lock(&card->conf_mutex);
mutex_lock(&card->sbp_lock);
if (qeth_l2_vnicc_is_in_use(card)) if (qeth_l2_vnicc_is_in_use(card))
rc = -EBUSY; rc = -EBUSY;
...@@ -234,6 +241,7 @@ static ssize_t qeth_bridgeport_reflect_store(struct device *dev, ...@@ -234,6 +241,7 @@ static ssize_t qeth_bridgeport_reflect_store(struct device *dev,
rc = 0; rc = 0;
} }
mutex_unlock(&card->sbp_lock);
mutex_unlock(&card->conf_mutex); mutex_unlock(&card->conf_mutex);
return rc ? rc : count; return rc ? rc : count;
...@@ -269,6 +277,8 @@ void qeth_l2_setup_bridgeport_attrs(struct qeth_card *card) ...@@ -269,6 +277,8 @@ void qeth_l2_setup_bridgeport_attrs(struct qeth_card *card)
return; return;
if (!card->options.sbp.supported_funcs) if (!card->options.sbp.supported_funcs)
return; return;
mutex_lock(&card->sbp_lock);
if (card->options.sbp.role != QETH_SBP_ROLE_NONE) { if (card->options.sbp.role != QETH_SBP_ROLE_NONE) {
/* Conditional to avoid spurious error messages */ /* Conditional to avoid spurious error messages */
qeth_bridgeport_setrole(card, card->options.sbp.role); qeth_bridgeport_setrole(card, card->options.sbp.role);
...@@ -280,8 +290,10 @@ void qeth_l2_setup_bridgeport_attrs(struct qeth_card *card) ...@@ -280,8 +290,10 @@ void qeth_l2_setup_bridgeport_attrs(struct qeth_card *card)
rc = qeth_bridgeport_an_set(card, 1); rc = qeth_bridgeport_an_set(card, 1);
if (rc) if (rc)
card->options.sbp.hostnotification = 0; card->options.sbp.hostnotification = 0;
} else } else {
qeth_bridgeport_an_set(card, 0); qeth_bridgeport_an_set(card, 0);
}
mutex_unlock(&card->sbp_lock);
} }
/* VNIC CHARS support */ /* VNIC CHARS support */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment