From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.0 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3C23DC07EBF for ; Fri, 18 Jan 2019 19:00:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0C7CC2086D for ; Fri, 18 Jan 2019 19:00:59 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=kernel-dk.20150623.gappssmtp.com header.i=@kernel-dk.20150623.gappssmtp.com header.b="dunjGScR" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729252AbfARTA5 (ORCPT ); Fri, 18 Jan 2019 14:00:57 -0500 Received: from mail-pg1-f195.google.com ([209.85.215.195]:37417 "EHLO mail-pg1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728543AbfARTA5 (ORCPT ); Fri, 18 Jan 2019 14:00:57 -0500 Received: by mail-pg1-f195.google.com with SMTP id c25so6477487pgb.4 for ; Fri, 18 Jan 2019 11:00:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel-dk.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=yfBlyynR755OjSbhKTiQ0PqRJqFi7tSrB6SBleZABgI=; b=dunjGScRSr90OVJidwPBnNo2qD21x4RSKPhNQhk+RE4OsiS2QK6ex0U6qKsCvRPZcP 82Ac3+NDJPtAtcXA+U0XrJZmzszqNbVVYHx4uWnZavjQMerox0G1qUMdvM6ZDbfnSypc 4yDT548zulPnY6tAXD188ZKBBloISdn5SEfk4ltg9pmBFZ7UU3xkw0OS0XAOQV6ZjHjH 4GSMuiQyPwvrGryWbQvXqSNaElNE7+H78VzT7aLz/Lgb+n+MHOxstlMKwFE28ab/aVrU COUuSVmN8Oomm7COoBIdqcg2MdBh0VdvNbppinB2wABXS7vrlAixzwTOJnT2gvRp58M5 Za3w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=yfBlyynR755OjSbhKTiQ0PqRJqFi7tSrB6SBleZABgI=; b=rxjzKzaqzq1VKvPlvqEbk5/4iNT/u38BCaX4QVP6f9HHXrM3KnEkqwgtPsrUkmJYHM PVZfy/gg/V433FjtfOkjkjhpvLsiCfiVrLL4zNsm4K5l8mBaXNA283hWi1plSGe/UW5X +U6YPnO8Oy2Zx6W1ra2F9oO0LSSmX3kloADeM66tQnZXIkwY3wPt7tHAzsk6hZO7VAQN 0qi0xzg2syZcXWbQykAh2R2jUk7/8WzXw4IRvW1oyEc/oOj6mPsH9GT8qY5R240PwtIb m3L7Gdh748FzHqwvlyHFcIhkb6hz/qbQCu9IuamqRWgRRd1r1BOExqLsBvSb1nTaMOSc 0/jw== X-Gm-Message-State: AJcUukd7rcWcvt22Riu8rJJNYtHbcHDeu++lhdcYeNBX0OzALSAXAdx/ cE7yk03zceNCmSCZs1Es8tH8Hg== X-Google-Smtp-Source: ALg8bN40yCEBM2MC5BRuhj3wucG/p7kSRiczSRBbL7BOFTscgPDfNjPyeZed+8ACtazyMHIj7hlRLQ== X-Received: by 2002:a62:1a44:: with SMTP id a65mr20853577pfa.30.1547838056201; Fri, 18 Jan 2019 11:00:56 -0800 (PST) Received: from [192.168.1.121] (66.29.188.166.static.utbb.net. [66.29.188.166]) by smtp.gmail.com with ESMTPSA id x26sm7486592pfn.50.2019.01.18.11.00.53 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 18 Jan 2019 11:00:55 -0800 (PST) Subject: Re: dd hangs when reading large partitions To: Bart Van Assche , "jianchao.wang" , Marc Gonzalez , fsdevel , linux-block Cc: SCSI , Alexander Viro , Jan Kara , Christoph Hellwig , Joao Pinto , Fujita Tomonori , Paolo Valente References: <398a6e83-d482-6e72-5806-6d5bbe8bfdd9@oracle.com> <1547833876.102272.0.camel@acm.org> From: Jens Axboe Message-ID: Date: Fri, 18 Jan 2019 12:00:52 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <1547833876.102272.0.camel@acm.org> Content-Type: text/plain; charset=UTF-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On 1/18/19 10:51 AM, Bart Van Assche wrote: > On Fri, 2019-01-18 at 10:48 -0700, Jens Axboe wrote: >> It's UFS that totally buggy, if you look at its queuecommand, it does: >> >> if (!down_read_trylock(&hba->clk_scaling_lock)) >> return SCSI_MLQUEUE_HOST_BUSY; >> >> UFS either needs to get fixed up, or we'll want a way to do something like >> the below. >> >> Marc, can you test this? >> >> >> diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c >> index eaf329db3973..e28c3420a9d9 100644 >> --- a/drivers/scsi/hosts.c >> +++ b/drivers/scsi/hosts.c >> @@ -412,6 +412,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize) >> shost->hostt = sht; >> shost->this_id = sht->this_id; >> shost->can_queue = sht->can_queue; >> + shost->queue_may_block = sht->queue_may_block; >> shost->sg_tablesize = sht->sg_tablesize; >> shost->sg_prot_tablesize = sht->sg_prot_tablesize; >> shost->cmd_per_lun = sht->cmd_per_lun; >> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c >> index b13cc9288ba0..4e266af2871f 100644 >> --- a/drivers/scsi/scsi_lib.c >> +++ b/drivers/scsi/scsi_lib.c >> @@ -1902,6 +1902,8 @@ int scsi_mq_setup_tags(struct Scsi_Host *shost) >> shost->tag_set.flags = BLK_MQ_F_SHOULD_MERGE | BLK_MQ_F_SG_MERGE; >> shost->tag_set.flags |= >> BLK_ALLOC_POLICY_TO_MQ_FLAG(shost->hostt->tag_alloc_policy); >> + if (shost->queue_may_blocK) >> + shost->tag_set.flags |= BLK_MQ_F_BLOCKING; >> shost->tag_set.driver_data = shost; >> >> return blk_mq_alloc_tag_set(&shost->tag_set); >> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c >> index 9ba7671b84f8..9ab354e43630 100644 >> --- a/drivers/scsi/ufs/ufshcd.c >> +++ b/drivers/scsi/ufs/ufshcd.c >> @@ -6981,6 +6981,7 @@ static struct scsi_host_template ufshcd_driver_template = { >> .sg_tablesize = SG_ALL, >> .cmd_per_lun = UFSHCD_CMD_PER_LUN, >> .can_queue = UFSHCD_CAN_QUEUE, >> + .queue_may_block = 1, >> .max_host_blocked = 1, >> .track_queue_depth = 1, >> .sdev_groups = ufshcd_driver_groups, >> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h >> index 6ca954e9f752..30aa7b6c4342 100644 >> --- a/include/scsi/scsi_host.h >> +++ b/include/scsi/scsi_host.h >> @@ -339,6 +339,11 @@ struct scsi_host_template { >> */ >> int can_queue; >> >> + /* >> + * If the ->queuecommand() ever blocks, this should be set >> + */ >> + int queue_may_block; >> + >> /* >> * In many instances, especially where disconnect / reconnect are >> * supported, our host also has an ID on the SCSI bus. If this is >> @@ -584,6 +589,7 @@ struct Scsi_Host { >> >> int this_id; >> int can_queue; >> + int queue_may_block; >> short cmd_per_lun; >> short unsigned int sg_tablesize; >> short unsigned int sg_prot_tablesize; > > Hi Jens, > > Did you perhaps want to include a change from down_read_trylock() into > down_read() in the UFS queuecommand function in this patch? Yeah, that should be done as well. But honestly, looking at UFS, it's in dire need of love from someone that has some experience in writing a driver. The locking is absolutely miserable. For one, the clk_scaling_lock needs to die. The clock scaling change should coordinate with the requests, not try to lock out request handling. It's totally an upside down approach. -- Jens Axboe