From patchwork Wed Oct 18 08:05:46 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Christie X-Patchwork-Id: 10014017 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 24BD1602C8 for ; Wed, 18 Oct 2017 08:05:53 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 11DDD28A24 for ; Wed, 18 Oct 2017 08:05:53 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0662928AF1; Wed, 18 Oct 2017 08:05:53 +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, T_TVD_MIME_EPI 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 3FCCD28A24 for ; Wed, 18 Oct 2017 08:05:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757284AbdJRIFu (ORCPT ); Wed, 18 Oct 2017 04:05:50 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54326 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757197AbdJRIFr (ORCPT ); Wed, 18 Oct 2017 04:05:47 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B84E3883AD; Wed, 18 Oct 2017 08:05:47 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com B84E3883AD Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=mchristi@redhat.com Received: from [10.10.120.106] (ovpn-120-106.rdu2.redhat.com [10.10.120.106]) by smtp.corp.redhat.com (Postfix) with ESMTP id 309E7177EF; Wed, 18 Oct 2017 08:05:47 +0000 (UTC) Subject: Re: [PATCH 16/17] tcmu: make ring buffer timer configurable To: lixiubo@cmss.chinamobile.com, target-devel@vger.kernel.org, nab@linux-iscsi.org References: <1508310852-15366-1-git-send-email-mchristi@redhat.com> <1508310852-15366-17-git-send-email-mchristi@redhat.com> From: Mike Christie Message-ID: <59E70B5A.30705@redhat.com> Date: Wed, 18 Oct 2017 03:05:46 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <1508310852-15366-17-git-send-email-mchristi@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Wed, 18 Oct 2017 08:05:47 +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 On 10/18/2017 02:14 AM, Mike Christie wrote: > This adds a timer, qfull_time_out, that controls how long a > device will wait for ring buffer space to open before > failing the commands in the queue. It is useful to separate > this timer from the cmd_time_out and default 30 sec one, > because for HA setups cmd_time_out may be disbled and 30 > seconds is too long to wait when some OSs like ESX will > timeout commands after as little as 8 - 15 seconds. > Sent a bad patch. This version of the patch had a bug when cmd_time_out=0 and qfull_time_out > 0. I have attached a version 2 which fixes this. From 8830f5f390181125e1e614f19e0c48524da31bb9 Mon Sep 17 00:00:00 2001 From: Mike Christie Date: Wed, 18 Oct 2017 03:02:22 -0500 Subject: [PATCH] tcmu: make ring buffer timer configurable v2 This adds a timer, qfull_time_out, that controls how long a device will wait for ring buffer space to open before failing the commands in the queue. It is useful to separate this timer from the cmd_time_out and default 30 sec one, because for HA setups cmd_time_out may be disbled and 30 seconds is too long to wait when some OSs like ESX will timeout commands after as little as 8 - 15 seconds. v2: Fix bug where if cmd_time_out=0 and qfull_time_out > 0 then inflight commands might timeout after qfull_time_out secs. --- drivers/target/target_core_user.c | 104 +++++++++++++++++++++++++++++--------- 1 file changed, 81 insertions(+), 23 deletions(-) diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c index 09d0b5b..f9f0677 100644 --- a/drivers/target/target_core_user.c +++ b/drivers/target/target_core_user.c @@ -142,8 +142,12 @@ struct tcmu_dev { struct idr commands; - struct timer_list timeout; + struct timer_list cmd_timer; unsigned int cmd_time_out; + + struct timer_list qfull_timer; + unsigned int qfull_time_out; + struct list_head timedout_entry; spinlock_t nl_cmd_lock; @@ -766,18 +770,14 @@ static inline size_t tcmu_cmd_get_cmd_size(struct tcmu_cmd *tcmu_cmd, return command_size; } -static int tcmu_setup_cmd_timer(struct tcmu_cmd *tcmu_cmd) +static int tcmu_setup_cmd_timer(struct tcmu_cmd *tcmu_cmd, unsigned int tmo, + struct timer_list *timer) { struct tcmu_dev *udev = tcmu_cmd->tcmu_dev; - unsigned long tmo = udev->cmd_time_out; int cmd_id; - /* - * If it was on the cmdr queue waiting we do not reset the timer - * for requeues and when it is finally sent to userspace. - */ if (tcmu_cmd->cmd_id) - return 0; + goto setup_timer; cmd_id = idr_alloc(&udev->commands, tcmu_cmd, 1, USHRT_MAX, GFP_NOWAIT); if (cmd_id < 0) { @@ -786,14 +786,15 @@ static int tcmu_setup_cmd_timer(struct tcmu_cmd *tcmu_cmd) } tcmu_cmd->cmd_id = cmd_id; - if (!tmo) - tmo = TCMU_TIME_OUT; - pr_debug("allocated cmd %u for dev %s tmo %lu\n", tcmu_cmd->cmd_id, udev->name, tmo / MSEC_PER_SEC); +setup_timer: + if (!tmo) + return 0; + tcmu_cmd->deadline = round_jiffies_up(jiffies + msecs_to_jiffies(tmo)); - mod_timer(&udev->timeout, tcmu_cmd->deadline); + mod_timer(timer, tcmu_cmd->deadline); return 0; } @@ -802,7 +803,8 @@ static int add_to_cmdr_queue(struct tcmu_cmd *tcmu_cmd) struct tcmu_dev *udev = tcmu_cmd->tcmu_dev; int ret; - ret = tcmu_setup_cmd_timer(tcmu_cmd); + ret = tcmu_setup_cmd_timer(tcmu_cmd, udev->qfull_time_out, + &udev->qfull_timer); if (ret) return ret; @@ -938,7 +940,8 @@ static sense_reason_t queue_cmd_ring(struct tcmu_cmd *tcmu_cmd, int *scsi_err) } entry->req.iov_bidi_cnt = iov_cnt; - ret = tcmu_setup_cmd_timer(tcmu_cmd); + ret = tcmu_setup_cmd_timer(tcmu_cmd, udev->cmd_time_out, + &udev->cmd_timer); if (ret) { tcmu_cmd_free_data(tcmu_cmd, tcmu_cmd->dbi_cnt); *scsi_err = TCM_OUT_OF_RESOURCES; @@ -971,6 +974,11 @@ static sense_reason_t queue_cmd_ring(struct tcmu_cmd *tcmu_cmd, int *scsi_err) return 0; queue: + if (!udev->qfull_time_out) { + *scsi_err = TCM_OUT_OF_RESOURCES; + return -1; + } + if (add_to_cmdr_queue(tcmu_cmd)) { *scsi_err = TCM_OUT_OF_RESOURCES; return -1; @@ -1085,7 +1093,7 @@ static unsigned int tcmu_handle_completions(struct tcmu_dev *udev) } if (mb->cmd_tail == mb->cmd_head && list_empty(&udev->cmdr_queue)) - del_timer(&udev->timeout); /* no more pending or waiting cmds */ + del_timer(&udev->cmd_timer); /* no more pending or waiting cmds */ return handled; } @@ -1105,13 +1113,15 @@ static int tcmu_check_expired_cmd(int id, void *p, void *data) return 0; is_running = list_empty(&cmd->cmdr_queue_entry); - pr_debug("Timing out cmd %u on dev %s that is %s.\n", - id, udev->name, is_running ? "inflight" : "queued"); - - se_cmd = cmd->se_cmd; - cmd->se_cmd = NULL; if (is_running) { + /* + * If cmd_time_out is disabled but qfull is set deadline + * will only reflect the qfull timeout. Ignore it. + */ + if (!udev->cmd_time_out) + return 0; + set_bit(TCMU_CMD_BIT_EXPIRED, &cmd->flags); /* * target_complete_cmd will translate this to LUN COMM FAILURE @@ -1131,6 +1141,12 @@ static int tcmu_check_expired_cmd(int id, void *p, void *data) tcmu_free_cmd(cmd); scsi_status = SAM_STAT_TASK_SET_FULL; } + + pr_debug("Timing out cmd %u on dev %s that is %s.\n", + id, udev->name, is_running ? "inflight" : "queued"); + + se_cmd = cmd->se_cmd; + cmd->se_cmd = NULL; target_complete_cmd(se_cmd, scsi_status); return 0; } @@ -1139,7 +1155,7 @@ static void tcmu_device_timedout(unsigned long data) { struct tcmu_dev *udev = (struct tcmu_dev *)data; - pr_debug("%s cmd timeout has expired\n", udev->name); + pr_debug("%s cmd/qfull timeout has expired\n", udev->name); spin_lock(&timed_out_udevs_lock); if (list_empty(&udev->timedout_entry)) @@ -1186,6 +1202,8 @@ static struct se_device *tcmu_alloc_device(struct se_hba *hba, const char *name) udev->hba = hba; udev->cmd_time_out = TCMU_TIME_OUT; + /* for backwards compat use the cmd_time_out */ + udev->qfull_time_out = TCMU_TIME_OUT; mutex_init(&udev->cmdr_lock); @@ -1194,7 +1212,10 @@ static struct se_device *tcmu_alloc_device(struct se_hba *hba, const char *name) INIT_LIST_HEAD(&udev->waiter); idr_init(&udev->commands); - setup_timer(&udev->timeout, tcmu_device_timedout, + setup_timer(&udev->qfull_timer, tcmu_device_timedout, + (unsigned long)udev); + + setup_timer(&udev->cmd_timer, tcmu_device_timedout, (unsigned long)udev); init_waitqueue_head(&udev->nl_cmd_wq); @@ -1250,6 +1271,8 @@ static bool run_cmdr_queue(struct tcmu_dev *udev) goto done; } } + if (list_empty(&udev->cmdr_queue)) + del_timer(&udev->qfull_timer); done: return drained; } @@ -1777,7 +1800,8 @@ static void tcmu_destroy_device(struct se_device *dev) { struct tcmu_dev *udev = TCMU_DEV(dev); - del_timer_sync(&udev->timeout); + del_timer_sync(&udev->cmd_timer); + del_timer_sync(&udev->qfull_timer); mutex_lock(&root_udev_mutex); list_del(&udev->node); @@ -1962,6 +1986,39 @@ static ssize_t tcmu_cmd_time_out_store(struct config_item *item, const char *pag } CONFIGFS_ATTR(tcmu_, cmd_time_out); +static ssize_t tcmu_qfull_time_out_show(struct config_item *item, char *page) +{ + struct se_dev_attrib *da = container_of(to_config_group(item), + struct se_dev_attrib, da_group); + struct tcmu_dev *udev = TCMU_DEV(da->da_dev); + + return snprintf(page, PAGE_SIZE, "%lu\n", + udev->qfull_time_out / MSEC_PER_SEC); +} + +static ssize_t tcmu_qfull_time_out_store(struct config_item *item, + const char *page, size_t count) +{ + struct se_dev_attrib *da = container_of(to_config_group(item), + struct se_dev_attrib, da_group); + struct tcmu_dev *udev = TCMU_DEV(da->da_dev); + s32 val; + int ret; + + ret = kstrtos32(page, 0, &val); + if (ret < 0) + return ret; + + if (val >= 0) { + udev->qfull_time_out = val * MSEC_PER_SEC; + } else { + printk(KERN_ERR "Invalid qfull timeout value %d\n", val); + return -EINVAL; + } + return count; +} +CONFIGFS_ATTR(tcmu_, qfull_time_out); + static ssize_t tcmu_dev_config_show(struct config_item *item, char *page) { struct se_dev_attrib *da = container_of(to_config_group(item), @@ -2107,6 +2164,7 @@ static ssize_t tcmu_emulate_write_cache_store(struct config_item *item, static struct configfs_attribute *tcmu_attrib_attrs[] = { &tcmu_attr_cmd_time_out, + &tcmu_attr_qfull_time_out, &tcmu_attr_dev_config, &tcmu_attr_dev_size, &tcmu_attr_emulate_write_cache, -- 1.8.3.1