From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jens Axboe Subject: Re: [PATCH, RFC] blk-mq: use a delayed work item for timeouts Date: Mon, 12 Oct 2015 14:08:04 -0600 Message-ID: <561C1324.3030800@fb.com> References: <1444678154-24766-1-git-send-email-hch@lst.de> <561C0B3C.70004@fb.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from mx0b-00082601.pphosted.com ([67.231.153.30]:32714 "EHLO mx0b-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750867AbbJLUIf (ORCPT ); Mon, 12 Oct 2015 16:08:35 -0400 In-Reply-To: <561C0B3C.70004@fb.com> Sender: linux-scsi-owner@vger.kernel.org List-Id: linux-scsi@vger.kernel.org To: Christoph Hellwig Cc: hare@suse.de, linux-scsi@vger.kernel.org, linux-nvme@lists.infradead.org On 10/12/2015 01:34 PM, Jens Axboe wrote: > On 10/12/2015 01:29 PM, Christoph Hellwig wrote: >> For some pending NVMe work I'd really love to be able to get my timeouts >> from process context. So far it seems only SCSI and NVMe use the blk-mq >> timeout handler, and both don't seem to be particularly excited about >> being called from time context. Does anyone have an objection against >> the patch below that switches it to use a delayed work item? I could >> make use of this quickly for NVMe, but for SCSI we still have to deal >> with the old request code which can't be switched to a delayed work >> as easily. > > No that's definitely fine with me, imho most error handling callbacks > should be in process context for ease of use in the driver. Took a closer look. The patch looks incomplete. The hot path for blk-mq is blk_add_timer(), looks like you left that one alone in the conversion? Might be easier to just leave the timer alone, and if it actually fires _and_ we have to do something, punt to a workqueue instead of invoking the timeout handler directly. -- Jens Axboe From mboxrd@z Thu Jan 1 00:00:00 1970 From: axboe@fb.com (Jens Axboe) Date: Mon, 12 Oct 2015 14:08:04 -0600 Subject: [PATCH, RFC] blk-mq: use a delayed work item for timeouts In-Reply-To: <561C0B3C.70004@fb.com> References: <1444678154-24766-1-git-send-email-hch@lst.de> <561C0B3C.70004@fb.com> Message-ID: <561C1324.3030800@fb.com> On 10/12/2015 01:34 PM, Jens Axboe wrote: > On 10/12/2015 01:29 PM, Christoph Hellwig wrote: >> For some pending NVMe work I'd really love to be able to get my timeouts >> from process context. So far it seems only SCSI and NVMe use the blk-mq >> timeout handler, and both don't seem to be particularly excited about >> being called from time context. Does anyone have an objection against >> the patch below that switches it to use a delayed work item? I could >> make use of this quickly for NVMe, but for SCSI we still have to deal >> with the old request code which can't be switched to a delayed work >> as easily. > > No that's definitely fine with me, imho most error handling callbacks > should be in process context for ease of use in the driver. Took a closer look. The patch looks incomplete. The hot path for blk-mq is blk_add_timer(), looks like you left that one alone in the conversion? Might be easier to just leave the timer alone, and if it actually fires _and_ we have to do something, punt to a workqueue instead of invoking the timeout handler directly. -- Jens Axboe