Skip to content

Commit 465b10f

Browse files
julianwiedmanngregkh
authored andcommitted
s390/qeth: fix dangling IO buffers after halt/clear
[ Upstream commit f9e50b0 ] The cio layer's intparm logic does not align itself well with how qeth manages cmd IOs. When an active IO gets terminated via halt/clear, the corresponding IRQ's intparm does not reflect the cmd buffer but rather the intparm that was passed to ccw_device_halt() / ccw_device_clear(). This behaviour was recently clarified in commit b91d9e6 ("s390/cio: fix intparm documentation"). As a result, qeth_irq() currently doesn't cancel a cmd that was terminated via halt/clear. This primarily causes us to leak card->read_cmd after the qeth device is removed, since our IO path still holds a refcount for this cmd. For qeth this means that we need to keep track of which IO is pending on a device ('active_cmd'), and use this as the intparm when calling halt/clear. Otherwise qeth_irq() can't match the subsequent IRQ to its cmd buffer. Since we now keep track of the _expected_ intparm, we can also detect any mismatch; this would constitute a bug somewhere in the lower layers. In this case cancel the active cmd - we effectively "lost" the IRQ and should not expect any further notification for this IO. Fixes: 4055489 ("s390/qeth: add support for dynamically allocated cmds") Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent ccbc5d0 commit 465b10f

5 files changed

Lines changed: 67 additions & 46 deletions

File tree

drivers/s390/net/qeth_core.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -620,6 +620,7 @@ struct qeth_ipato {
620620

621621
struct qeth_channel {
622622
struct ccw_device *ccwdev;
623+
struct qeth_cmd_buffer *active_cmd;
623624
enum qeth_channel_states state;
624625
atomic_t irq_pending;
625626
};
@@ -1024,6 +1025,8 @@ int qeth_do_run_thread(struct qeth_card *, unsigned long);
10241025
void qeth_clear_thread_start_bit(struct qeth_card *, unsigned long);
10251026
void qeth_clear_thread_running_bit(struct qeth_card *, unsigned long);
10261027
int qeth_core_hardsetup_card(struct qeth_card *card, bool *carrier_ok);
1028+
int qeth_stop_channel(struct qeth_channel *channel);
1029+
10271030
void qeth_print_status_message(struct qeth_card *);
10281031
int qeth_init_qdio_queues(struct qeth_card *);
10291032
int qeth_send_ipa_cmd(struct qeth_card *, struct qeth_cmd_buffer *,

drivers/s390/net/qeth_core_main.c

Lines changed: 51 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,9 @@ static int __qeth_issue_next_read(struct qeth_card *card)
515515

516516
QETH_CARD_TEXT(card, 6, "noirqpnd");
517517
rc = ccw_device_start(channel->ccwdev, ccw, (addr_t) iob, 0, 0);
518-
if (rc) {
518+
if (!rc) {
519+
channel->active_cmd = iob;
520+
} else {
519521
QETH_DBF_MESSAGE(2, "error %i on device %x when starting next read ccw!\n",
520522
rc, CARD_DEVID(card));
521523
atomic_set(&channel->irq_pending, 0);
@@ -986,8 +988,21 @@ static void qeth_irq(struct ccw_device *cdev, unsigned long intparm,
986988
QETH_CARD_TEXT(card, 5, "data");
987989
}
988990

989-
if (qeth_intparm_is_iob(intparm))
990-
iob = (struct qeth_cmd_buffer *) __va((addr_t)intparm);
991+
if (intparm == 0) {
992+
QETH_CARD_TEXT(card, 5, "irqunsol");
993+
} else if ((addr_t)intparm != (addr_t)channel->active_cmd) {
994+
QETH_CARD_TEXT(card, 5, "irqunexp");
995+
996+
dev_err(&cdev->dev,
997+
"Received IRQ with intparm %lx, expected %px\n",
998+
intparm, channel->active_cmd);
999+
if (channel->active_cmd)
1000+
qeth_cancel_cmd(channel->active_cmd, -EIO);
1001+
} else {
1002+
iob = (struct qeth_cmd_buffer *) (addr_t)intparm;
1003+
}
1004+
1005+
channel->active_cmd = NULL;
9911006

9921007
rc = qeth_check_irb_error(card, cdev, irb);
9931008
if (rc) {
@@ -1007,15 +1022,10 @@ static void qeth_irq(struct ccw_device *cdev, unsigned long intparm,
10071022
if (irb->scsw.cmd.fctl & (SCSW_FCTL_HALT_FUNC))
10081023
channel->state = CH_STATE_HALTED;
10091024

1010-
if (intparm == QETH_CLEAR_CHANNEL_PARM) {
1011-
QETH_CARD_TEXT(card, 6, "clrchpar");
1012-
/* we don't have to handle this further */
1013-
intparm = 0;
1014-
}
1015-
if (intparm == QETH_HALT_CHANNEL_PARM) {
1016-
QETH_CARD_TEXT(card, 6, "hltchpar");
1017-
/* we don't have to handle this further */
1018-
intparm = 0;
1025+
if (iob && (irb->scsw.cmd.fctl & (SCSW_FCTL_CLEAR_FUNC |
1026+
SCSW_FCTL_HALT_FUNC))) {
1027+
qeth_cancel_cmd(iob, -ECANCELED);
1028+
iob = NULL;
10191029
}
10201030

10211031
cstat = irb->scsw.cmd.cstat;
@@ -1408,7 +1418,7 @@ static int qeth_clear_channel(struct qeth_card *card,
14081418

14091419
QETH_CARD_TEXT(card, 3, "clearch");
14101420
spin_lock_irq(get_ccwdev_lock(channel->ccwdev));
1411-
rc = ccw_device_clear(channel->ccwdev, QETH_CLEAR_CHANNEL_PARM);
1421+
rc = ccw_device_clear(channel->ccwdev, (addr_t)channel->active_cmd);
14121422
spin_unlock_irq(get_ccwdev_lock(channel->ccwdev));
14131423

14141424
if (rc)
@@ -1430,7 +1440,7 @@ static int qeth_halt_channel(struct qeth_card *card,
14301440

14311441
QETH_CARD_TEXT(card, 3, "haltch");
14321442
spin_lock_irq(get_ccwdev_lock(channel->ccwdev));
1433-
rc = ccw_device_halt(channel->ccwdev, QETH_HALT_CHANNEL_PARM);
1443+
rc = ccw_device_halt(channel->ccwdev, (addr_t)channel->active_cmd);
14341444
spin_unlock_irq(get_ccwdev_lock(channel->ccwdev));
14351445

14361446
if (rc)
@@ -1444,6 +1454,25 @@ static int qeth_halt_channel(struct qeth_card *card,
14441454
return 0;
14451455
}
14461456

1457+
int qeth_stop_channel(struct qeth_channel *channel)
1458+
{
1459+
struct ccw_device *cdev = channel->ccwdev;
1460+
int rc;
1461+
1462+
rc = ccw_device_set_offline(cdev);
1463+
1464+
spin_lock_irq(get_ccwdev_lock(cdev));
1465+
if (channel->active_cmd) {
1466+
dev_err(&cdev->dev, "Stopped channel while cmd %px was still active\n",
1467+
channel->active_cmd);
1468+
channel->active_cmd = NULL;
1469+
}
1470+
spin_unlock_irq(get_ccwdev_lock(cdev));
1471+
1472+
return rc;
1473+
}
1474+
EXPORT_SYMBOL_GPL(qeth_stop_channel);
1475+
14471476
static int qeth_halt_channels(struct qeth_card *card)
14481477
{
14491478
int rc1 = 0, rc2 = 0, rc3 = 0;
@@ -1747,6 +1776,8 @@ static int qeth_send_control_data(struct qeth_card *card,
17471776
spin_lock_irq(get_ccwdev_lock(channel->ccwdev));
17481777
rc = ccw_device_start_timeout(channel->ccwdev, __ccw_from_cmd(iob),
17491778
(addr_t) iob, 0, 0, timeout);
1779+
if (!rc)
1780+
channel->active_cmd = iob;
17501781
spin_unlock_irq(get_ccwdev_lock(channel->ccwdev));
17511782
if (rc) {
17521783
QETH_DBF_MESSAGE(2, "qeth_send_control_data on device %x: ccw_device_start rc = %i\n",
@@ -4625,12 +4656,12 @@ EXPORT_SYMBOL_GPL(qeth_vm_request_mac);
46254656

46264657
static void qeth_determine_capabilities(struct qeth_card *card)
46274658
{
4659+
struct qeth_channel *channel = &card->data;
4660+
struct ccw_device *ddev = channel->ccwdev;
46284661
int rc;
4629-
struct ccw_device *ddev;
46304662
int ddev_offline = 0;
46314663

46324664
QETH_CARD_TEXT(card, 2, "detcapab");
4633-
ddev = CARD_DDEV(card);
46344665
if (!ddev->online) {
46354666
ddev_offline = 1;
46364667
rc = ccw_device_set_online(ddev);
@@ -4669,7 +4700,7 @@ static void qeth_determine_capabilities(struct qeth_card *card)
46694700

46704701
out_offline:
46714702
if (ddev_offline == 1)
4672-
ccw_device_set_offline(ddev);
4703+
qeth_stop_channel(channel);
46734704
out:
46744705
return;
46754706
}
@@ -4870,9 +4901,9 @@ int qeth_core_hardsetup_card(struct qeth_card *card, bool *carrier_ok)
48704901
QETH_DBF_MESSAGE(2, "Retrying to do IDX activates on device %x.\n",
48714902
CARD_DEVID(card));
48724903
rc = qeth_qdio_clear_card(card, !IS_IQD(card));
4873-
ccw_device_set_offline(CARD_DDEV(card));
4874-
ccw_device_set_offline(CARD_WDEV(card));
4875-
ccw_device_set_offline(CARD_RDEV(card));
4904+
qeth_stop_channel(&card->data);
4905+
qeth_stop_channel(&card->write);
4906+
qeth_stop_channel(&card->read);
48764907
qdio_free(CARD_DDEV(card));
48774908
rc = ccw_device_set_online(CARD_RDEV(card));
48784909
if (rc)

drivers/s390/net/qeth_core_mpc.h

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,20 +28,6 @@ extern unsigned char IPA_PDU_HEADER[];
2828
#define QETH_TIMEOUT (10 * HZ)
2929
#define QETH_IPA_TIMEOUT (45 * HZ)
3030

31-
#define QETH_CLEAR_CHANNEL_PARM -10
32-
#define QETH_HALT_CHANNEL_PARM -11
33-
34-
static inline bool qeth_intparm_is_iob(unsigned long intparm)
35-
{
36-
switch (intparm) {
37-
case QETH_CLEAR_CHANNEL_PARM:
38-
case QETH_HALT_CHANNEL_PARM:
39-
case 0:
40-
return false;
41-
}
42-
return true;
43-
}
44-
4531
/*****************************************************************************/
4632
/* IP Assist related definitions */
4733
/*****************************************************************************/

drivers/s390/net/qeth_l2_main.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -877,9 +877,9 @@ static int qeth_l2_set_online(struct ccwgroup_device *gdev)
877877

878878
out_remove:
879879
qeth_l2_stop_card(card);
880-
ccw_device_set_offline(CARD_DDEV(card));
881-
ccw_device_set_offline(CARD_WDEV(card));
882-
ccw_device_set_offline(CARD_RDEV(card));
880+
qeth_stop_channel(&card->data);
881+
qeth_stop_channel(&card->write);
882+
qeth_stop_channel(&card->read);
883883
qdio_free(CARD_DDEV(card));
884884

885885
mutex_unlock(&card->conf_mutex);
@@ -910,9 +910,9 @@ static int __qeth_l2_set_offline(struct ccwgroup_device *cgdev,
910910
rtnl_unlock();
911911

912912
qeth_l2_stop_card(card);
913-
rc = ccw_device_set_offline(CARD_DDEV(card));
914-
rc2 = ccw_device_set_offline(CARD_WDEV(card));
915-
rc3 = ccw_device_set_offline(CARD_RDEV(card));
913+
rc = qeth_stop_channel(&card->data);
914+
rc2 = qeth_stop_channel(&card->write);
915+
rc3 = qeth_stop_channel(&card->read);
916916
if (!rc)
917917
rc = (rc2) ? rc2 : rc3;
918918
if (rc)

drivers/s390/net/qeth_l3_main.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2383,9 +2383,9 @@ static int qeth_l3_set_online(struct ccwgroup_device *gdev)
23832383
return 0;
23842384
out_remove:
23852385
qeth_l3_stop_card(card);
2386-
ccw_device_set_offline(CARD_DDEV(card));
2387-
ccw_device_set_offline(CARD_WDEV(card));
2388-
ccw_device_set_offline(CARD_RDEV(card));
2386+
qeth_stop_channel(&card->data);
2387+
qeth_stop_channel(&card->write);
2388+
qeth_stop_channel(&card->read);
23892389
qdio_free(CARD_DDEV(card));
23902390

23912391
mutex_unlock(&card->conf_mutex);
@@ -2421,9 +2421,10 @@ static int __qeth_l3_set_offline(struct ccwgroup_device *cgdev,
24212421
call_netdevice_notifiers(NETDEV_REBOOT, card->dev);
24222422
rtnl_unlock();
24232423
}
2424-
rc = ccw_device_set_offline(CARD_DDEV(card));
2425-
rc2 = ccw_device_set_offline(CARD_WDEV(card));
2426-
rc3 = ccw_device_set_offline(CARD_RDEV(card));
2424+
2425+
rc = qeth_stop_channel(&card->data);
2426+
rc2 = qeth_stop_channel(&card->write);
2427+
rc3 = qeth_stop_channel(&card->read);
24272428
if (!rc)
24282429
rc = (rc2) ? rc2 : rc3;
24292430
if (rc)

0 commit comments

Comments
 (0)