From patchwork Wed Jun 21 21:33:04 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Christie X-Patchwork-Id: 9802995 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 4B540600C5 for ; Wed, 21 Jun 2017 21:33:22 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 34688268AE for ; Wed, 21 Jun 2017 21:33:20 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 294AB28111; Wed, 21 Jun 2017 21:33:20 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 664E0268AE for ; Wed, 21 Jun 2017 21:33:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751130AbdFUVdT (ORCPT ); Wed, 21 Jun 2017 17:33:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50248 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751093AbdFUVdS (ORCPT ); Wed, 21 Jun 2017 17:33:18 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1AD827F404; Wed, 21 Jun 2017 21:33:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 1AD827F404 Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=mchristi@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 1AD827F404 Received: from rh2.redhat.com (ovpn-123-144.rdu2.redhat.com [10.10.123.144]) by smtp.corp.redhat.com (Postfix) with ESMTP id 49F415D961; Wed, 21 Jun 2017 21:33:17 +0000 (UTC) From: Mike Christie To: bryantly@linux.vnet.ibm.com, pkalever@redhat.com, target-devel@vger.kernel.org, nab@linux-iscsi.org Cc: Mike Christie Subject: [PATCH 05/12] tcmu: perfom device add, del and reconfig synchronously Date: Wed, 21 Jun 2017 16:33:04 -0500 Message-Id: <1498080791-13565-6-git-send-email-mchristi@redhat.com> In-Reply-To: <1498080791-13565-1-git-send-email-mchristi@redhat.com> References: <1498080791-13565-1-git-send-email-mchristi@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Wed, 21 Jun 2017 21:33:18 +0000 (UTC) Sender: target-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: target-devel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP This makes the device add, del reconfig operations sync. It fixes the issue where for add and reconfig, we do not know if userspace successfully completely the operation, so we leave invalid kernel structs or report incorrect status for the config/reconfig operations. Signed-off-by: Mike Christie --- Changes; - Add support for reconfig drivers/target/target_core_user.c | 213 ++++++++++++++++++++++++++++++---- include/uapi/linux/target_core_user.h | 7 ++ 2 files changed, 200 insertions(+), 20 deletions(-) diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c index be119c1..345476c 100644 --- a/drivers/target/target_core_user.c +++ b/drivers/target/target_core_user.c @@ -87,6 +87,8 @@ /* Default maximum of the global data blocks(512K * PAGE_SIZE) */ #define TCMU_GLOBAL_MAX_BLOCKS (512 * 1024) +static u8 tcmu_kern_cmd_reply_supported; + static struct device *tcmu_root_device; struct tcmu_hba { @@ -95,6 +97,13 @@ struct tcmu_hba { #define TCMU_CONFIG_LEN 256 +struct tcmu_nl_cmd { + /* wake up thread waiting for reply */ + struct completion complete; + int cmd; + int status; +}; + struct tcmu_dev { struct list_head node; struct kref kref; @@ -135,6 +144,11 @@ struct tcmu_dev { struct timer_list timeout; unsigned int cmd_time_out; + spinlock_t nl_cmd_lock; + struct tcmu_nl_cmd curr_nl_cmd; + /* wake up threads waiting on curr_nl_cmd */ + wait_queue_head_t nl_cmd_wq; + char dev_config[TCMU_CONFIG_LEN]; }; @@ -178,16 +192,128 @@ enum tcmu_multicast_groups { [TCMU_MCGRP_CONFIG] = { .name = "config", }, }; +static struct nla_policy tcmu_attr_policy[TCMU_ATTR_MAX+1] = { + [TCMU_ATTR_DEVICE] = { .type = NLA_STRING }, + [TCMU_ATTR_MINOR] = { .type = NLA_U32 }, + [TCMU_ATTR_CMD_STATUS] = { .type = NLA_S32 }, + [TCMU_ATTR_DEVICE_ID] = { .type = NLA_U32 }, + [TCMU_ATTR_SUPP_KERN_CMD_REPLY] = { .type = NLA_U8 }, +}; + +static int tcmu_genl_cmd_done(struct genl_info *info, int completed_cmd) +{ + struct se_device *dev; + struct tcmu_dev *udev; + struct tcmu_nl_cmd *nl_cmd; + int dev_id, rc, ret = 0; + bool is_removed = (completed_cmd == TCMU_CMD_REMOVED_DEVICE); + + if (!info->attrs[TCMU_ATTR_CMD_STATUS] || + !info->attrs[TCMU_ATTR_DEVICE_ID]) { + printk(KERN_ERR "TCMU_ATTR_CMD_STATUS or TCMU_ATTR_DEVICE_ID not set, doing nothing\n"); + return -EINVAL; + } + + dev_id = nla_get_u32(info->attrs[TCMU_ATTR_DEVICE_ID]); + rc = nla_get_s32(info->attrs[TCMU_ATTR_CMD_STATUS]); + + dev = target_find_device(dev_id, !is_removed); + if (!dev) { + printk(KERN_ERR "tcmu nl cmd %u/%u completion could not find device with dev id %u.\n", + completed_cmd, rc, dev_id); + return -ENODEV; + } + udev = TCMU_DEV(dev); + + spin_lock(&udev->nl_cmd_lock); + nl_cmd = &udev->curr_nl_cmd; + + pr_debug("genl cmd done got id %d curr %d done %d rc %d\n", dev_id, + nl_cmd->cmd, completed_cmd, rc); + + if (nl_cmd->cmd != completed_cmd) { + printk(KERN_ERR "Mismatched commands (Expecting reply for %d. Current %d).\n", + completed_cmd, nl_cmd->cmd); + ret = -EINVAL; + } else { + nl_cmd->status = rc; + } + + spin_unlock(&udev->nl_cmd_lock); + if (!is_removed) + target_undepend_item(&dev->dev_group.cg_item); + if (!ret) + complete(&nl_cmd->complete); + return ret; +} + +static int tcmu_genl_rm_dev_done(struct sk_buff *skb, struct genl_info *info) +{ + return tcmu_genl_cmd_done(info, TCMU_CMD_REMOVED_DEVICE); +} + +static int tcmu_genl_add_dev_done(struct sk_buff *skb, struct genl_info *info) +{ + return tcmu_genl_cmd_done(info, TCMU_CMD_ADDED_DEVICE); +} + +static int tcmu_genl_reconfig_dev_done(struct sk_buff *skb, + struct genl_info *info) +{ + return tcmu_genl_cmd_done(info, TCMU_CMD_RECONFIG_DEVICE); +} + +static int tcmu_genl_set_features(struct sk_buff *skb, struct genl_info *info) +{ + if (info->attrs[TCMU_ATTR_SUPP_KERN_CMD_REPLY]) { + tcmu_kern_cmd_reply_supported = + nla_get_u8(info->attrs[TCMU_ATTR_SUPP_KERN_CMD_REPLY]); + printk(KERN_INFO "tcmu daemon: command reply support %u.\n", + tcmu_kern_cmd_reply_supported); + } + + return 0; +} + +static const struct genl_ops tcmu_genl_ops[] = { + { + .cmd = TCMU_CMD_SET_FEATURES, + .flags = GENL_ADMIN_PERM, + .policy = tcmu_attr_policy, + .doit = tcmu_genl_set_features, + }, + { + .cmd = TCMU_CMD_ADDED_DEVICE_DONE, + .flags = GENL_ADMIN_PERM, + .policy = tcmu_attr_policy, + .doit = tcmu_genl_add_dev_done, + }, + { + .cmd = TCMU_CMD_REMOVED_DEVICE_DONE, + .flags = GENL_ADMIN_PERM, + .policy = tcmu_attr_policy, + .doit = tcmu_genl_rm_dev_done, + }, + { + .cmd = TCMU_CMD_RECONFIG_DEVICE_DONE, + .flags = GENL_ADMIN_PERM, + .policy = tcmu_attr_policy, + .doit = tcmu_genl_reconfig_dev_done, + }, +}; + /* Our generic netlink family */ static struct genl_family tcmu_genl_family __ro_after_init = { .module = THIS_MODULE, .hdrsize = 0, .name = "TCM-USER", - .version = 1, + .version = 2, .maxattr = TCMU_ATTR_MAX, .mcgrps = tcmu_mcgrps, .n_mcgrps = ARRAY_SIZE(tcmu_mcgrps), .netnsok = true, + .ops = tcmu_genl_ops, + .n_ops = ARRAY_SIZE(tcmu_genl_ops), }; #define tcmu_cmd_set_dbi_cur(cmd, index) ((cmd)->dbi_cur = (index)) @@ -989,6 +1115,9 @@ static struct se_device *tcmu_alloc_device(struct se_hba *hba, const char *name) setup_timer(&udev->timeout, tcmu_device_timedout, (unsigned long)udev); + init_waitqueue_head(&udev->nl_cmd_wq); + spin_lock_init(&udev->nl_cmd_lock); + return &udev->se_dev; } @@ -1176,9 +1305,54 @@ static int tcmu_release(struct uio_info *info, struct inode *inode) return 0; } -static int tcmu_netlink_event(enum tcmu_genl_cmd cmd, const char *name, - int minor, int reconfig_attr, - const void *reconfig_data) +static void tcmu_init_genl_cmd_reply(struct tcmu_dev *udev, int cmd) +{ + struct tcmu_nl_cmd *nl_cmd = &udev->curr_nl_cmd; + + if (!tcmu_kern_cmd_reply_supported) + return; +relock: + spin_lock(&udev->nl_cmd_lock); + + if (nl_cmd->cmd != TCMU_CMD_UNSPEC) { + spin_unlock(&udev->nl_cmd_lock); + pr_debug("sleeping for open nl cmd\n"); + wait_event(udev->nl_cmd_wq, (nl_cmd->cmd == TCMU_CMD_UNSPEC)); + goto relock; + } + + memset(nl_cmd, 0, sizeof(*nl_cmd)); + nl_cmd->cmd = cmd; + init_completion(&nl_cmd->complete); + + spin_unlock(&udev->nl_cmd_lock); +} + +static int tcmu_wait_genl_cmd_reply(struct tcmu_dev *udev) +{ + struct tcmu_nl_cmd *nl_cmd = &udev->curr_nl_cmd; + int ret; + DEFINE_WAIT(__wait); + + if (!tcmu_kern_cmd_reply_supported) + return 0; + + pr_debug("sleeping for nl reply\n"); + wait_for_completion(&nl_cmd->complete); + + spin_lock(&udev->nl_cmd_lock); + nl_cmd->cmd = TCMU_CMD_UNSPEC; + ret = nl_cmd->status; + nl_cmd->status = 0; + spin_unlock(&udev->nl_cmd_lock); + + wake_up_all(&udev->nl_cmd_wq); + + return ret;; +} + +static int tcmu_netlink_event(struct tcmu_dev *udev, enum tcmu_genl_cmd cmd, + int reconfig_attr, const void *reconfig_data) { struct sk_buff *skb; void *msg_header; @@ -1192,11 +1366,15 @@ static int tcmu_netlink_event(enum tcmu_genl_cmd cmd, const char *name, if (!msg_header) goto free_skb; - ret = nla_put_string(skb, TCMU_ATTR_DEVICE, name); + ret = nla_put_string(skb, TCMU_ATTR_DEVICE, udev->uio_info.name); if (ret < 0) goto free_skb; - ret = nla_put_u32(skb, TCMU_ATTR_MINOR, minor); + ret = nla_put_u32(skb, TCMU_ATTR_MINOR, udev->uio_info.uio_dev->minor); + if (ret < 0) + goto free_skb; + + ret = nla_put_u32(skb, TCMU_ATTR_DEVICE_ID, udev->se_dev.dev_index); if (ret < 0) goto free_skb; @@ -1224,12 +1402,15 @@ static int tcmu_netlink_event(enum tcmu_genl_cmd cmd, const char *name, genlmsg_end(skb, msg_header); + tcmu_init_genl_cmd_reply(udev, cmd); + ret = genlmsg_multicast_allns(&tcmu_genl_family, skb, 0, TCMU_MCGRP_CONFIG, GFP_KERNEL); - /* We don't care if no one is listening */ if (ret == -ESRCH) ret = 0; + if (!ret) + ret = tcmu_wait_genl_cmd_reply(udev); return ret; free_skb: @@ -1324,8 +1505,7 @@ static int tcmu_configure_device(struct se_device *dev) */ kref_get(&udev->kref); - ret = tcmu_netlink_event(TCMU_CMD_ADDED_DEVICE, udev->uio_info.name, - udev->uio_info.uio_dev->minor, 0, NULL); + ret = tcmu_netlink_event(udev, TCMU_CMD_ADDED_DEVICE, 0, NULL); if (ret) goto err_netlink; @@ -1414,8 +1594,7 @@ static void tcmu_destroy_device(struct se_device *dev) tcmu_blocks_release(udev); if (tcmu_dev_configured(udev)) { - tcmu_netlink_event(TCMU_CMD_REMOVED_DEVICE, udev->uio_info.name, - udev->uio_info.uio_dev->minor, 0, NULL); + tcmu_netlink_event(udev, TCMU_CMD_REMOVED_DEVICE, 0, NULL); uio_unregister_device(&udev->uio_info); } @@ -1600,9 +1779,7 @@ static ssize_t tcmu_dev_config_store(struct config_item *item, const char *page, /* Check if device has been configured before */ if (tcmu_dev_configured(udev)) { - ret = tcmu_netlink_event(TCMU_CMD_RECONFIG_DEVICE, - udev->uio_info.name, - udev->uio_info.uio_dev->minor, + ret = tcmu_netlink_event(udev, TCMU_CMD_RECONFIG_DEVICE, TCMU_ATTR_DEV_CFG, page); if (ret) { pr_err("Unable to reconfigure device\n"); @@ -1639,9 +1816,7 @@ static ssize_t tcmu_dev_size_store(struct config_item *item, const char *page, /* Check if device has been configured before */ if (tcmu_dev_configured(udev)) { - ret = tcmu_netlink_event(TCMU_CMD_RECONFIG_DEVICE, - udev->uio_info.name, - udev->uio_info.uio_dev->minor, + ret = tcmu_netlink_event(udev, TCMU_CMD_RECONFIG_DEVICE, TCMU_ATTR_DEV_SIZE, &val); if (ret) { pr_err("Unable to reconfigure device\n"); @@ -1677,9 +1852,7 @@ static ssize_t tcmu_emulate_write_cache_store(struct config_item *item, /* Check if device has been configured before */ if (tcmu_dev_configured(udev)) { - ret = tcmu_netlink_event(TCMU_CMD_RECONFIG_DEVICE, - udev->uio_info.name, - udev->uio_info.uio_dev->minor, + ret = tcmu_netlink_event(udev, TCMU_CMD_RECONFIG_DEVICE, TCMU_ATTR_WRITECACHE, &val); if (ret) { pr_err("Unable to reconfigure device\n"); diff --git a/include/uapi/linux/target_core_user.h b/include/uapi/linux/target_core_user.h index 4bfc9a1..24a1c4e 100644 --- a/include/uapi/linux/target_core_user.h +++ b/include/uapi/linux/target_core_user.h @@ -131,6 +131,10 @@ enum tcmu_genl_cmd { TCMU_CMD_ADDED_DEVICE, TCMU_CMD_REMOVED_DEVICE, TCMU_CMD_RECONFIG_DEVICE, + TCMU_CMD_ADDED_DEVICE_DONE, + TCMU_CMD_REMOVED_DEVICE_DONE, + TCMU_CMD_RECONFIG_DEVICE_DONE, + TCMU_CMD_SET_FEATURES, __TCMU_CMD_MAX, }; #define TCMU_CMD_MAX (__TCMU_CMD_MAX - 1) @@ -143,6 +147,9 @@ enum tcmu_genl_attr { TCMU_ATTR_DEV_CFG, TCMU_ATTR_DEV_SIZE, TCMU_ATTR_WRITECACHE, + TCMU_ATTR_CMD_STATUS, + TCMU_ATTR_DEVICE_ID, + TCMU_ATTR_SUPP_KERN_CMD_REPLY, __TCMU_ATTR_MAX, }; #define TCMU_ATTR_MAX (__TCMU_ATTR_MAX - 1)