Skip to content

Commit a2f4c1a

Browse files
ps-ushankaraxboe
authored andcommitted
selftests: ublk: kublk: improve behavior on init failure
Some failure modes are handled poorly by kublk. For example, if ublk_drv is built as a module but not currently loaded into the kernel, ./kublk add ... just hangs forever. This happens because in this case (and a few others), the worker process does not notify its parent (via a write to the shared eventfd) that it has tried and failed to initialize, so the parent hangs forever. Fix this by ensuring that we always notify the parent process of any initialization failure, and have the parent print a (not very descriptive) log line when this happens. Signed-off-by: Uday Shankar <[email protected]> Reviewed-by: Ming Lei <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jens Axboe <[email protected]>
1 parent 43a67dd commit a2f4c1a

File tree

1 file changed

+23
-11
lines changed
  • tools/testing/selftests/ublk

1 file changed

+23
-11
lines changed

tools/testing/selftests/ublk/kublk.c

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1112,21 +1112,23 @@ static int __cmd_dev_add(const struct dev_ctx *ctx)
11121112
__u64 features;
11131113
const struct ublk_tgt_ops *ops;
11141114
struct ublksrv_ctrl_dev_info *info;
1115-
struct ublk_dev *dev;
1115+
struct ublk_dev *dev = NULL;
11161116
int dev_id = ctx->dev_id;
11171117
int ret, i;
11181118

11191119
ops = ublk_find_tgt(tgt_type);
11201120
if (!ops) {
11211121
ublk_err("%s: no such tgt type, type %s\n",
11221122
__func__, tgt_type);
1123-
return -ENODEV;
1123+
ret = -ENODEV;
1124+
goto fail;
11241125
}
11251126

11261127
if (nr_queues > UBLK_MAX_QUEUES || depth > UBLK_QUEUE_DEPTH) {
11271128
ublk_err("%s: invalid nr_queues or depth queues %u depth %u\n",
11281129
__func__, nr_queues, depth);
1129-
return -EINVAL;
1130+
ret = -EINVAL;
1131+
goto fail;
11301132
}
11311133

11321134
/* default to 1:1 threads:queues if nthreads is unspecified */
@@ -1136,30 +1138,37 @@ static int __cmd_dev_add(const struct dev_ctx *ctx)
11361138
if (nthreads > UBLK_MAX_THREADS) {
11371139
ublk_err("%s: %u is too many threads (max %u)\n",
11381140
__func__, nthreads, UBLK_MAX_THREADS);
1139-
return -EINVAL;
1141+
ret = -EINVAL;
1142+
goto fail;
11401143
}
11411144

11421145
if (nthreads != nr_queues && !ctx->per_io_tasks) {
11431146
ublk_err("%s: threads %u must be same as queues %u if "
11441147
"not using per_io_tasks\n",
11451148
__func__, nthreads, nr_queues);
1146-
return -EINVAL;
1149+
ret = -EINVAL;
1150+
goto fail;
11471151
}
11481152

11491153
dev = ublk_ctrl_init();
11501154
if (!dev) {
11511155
ublk_err("%s: can't alloc dev id %d, type %s\n",
11521156
__func__, dev_id, tgt_type);
1153-
return -ENOMEM;
1157+
ret = -ENOMEM;
1158+
goto fail;
11541159
}
11551160

11561161
/* kernel doesn't support get_features */
11571162
ret = ublk_ctrl_get_features(dev, &features);
1158-
if (ret < 0)
1159-
return -EINVAL;
1163+
if (ret < 0) {
1164+
ret = -EINVAL;
1165+
goto fail;
1166+
}
11601167

1161-
if (!(features & UBLK_F_CMD_IOCTL_ENCODE))
1162-
return -ENOTSUP;
1168+
if (!(features & UBLK_F_CMD_IOCTL_ENCODE)) {
1169+
ret = -ENOTSUP;
1170+
goto fail;
1171+
}
11631172

11641173
info = &dev->dev_info;
11651174
info->dev_id = ctx->dev_id;
@@ -1200,7 +1209,8 @@ static int __cmd_dev_add(const struct dev_ctx *ctx)
12001209
fail:
12011210
if (ret < 0)
12021211
ublk_send_dev_event(ctx, dev, -1);
1203-
ublk_ctrl_deinit(dev);
1212+
if (dev)
1213+
ublk_ctrl_deinit(dev);
12041214
return ret;
12051215
}
12061216

@@ -1262,6 +1272,8 @@ static int cmd_dev_add(struct dev_ctx *ctx)
12621272
shmctl(ctx->_shmid, IPC_RMID, NULL);
12631273
/* wait for child and detach from it */
12641274
wait(NULL);
1275+
if (exit_code == EXIT_FAILURE)
1276+
ublk_err("%s: command failed\n", __func__);
12651277
exit(exit_code);
12661278
} else {
12671279
exit(EXIT_FAILURE);

0 commit comments

Comments
 (0)