Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <506ca1a6-1122-5755-fc74-60f7c7bfbd0d@acm.org>
Date:   Wed, 20 Jul 2022 09:51:21 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH v2 2/2] scsi: sd: Rework asynchronous resume support
Content-Language: en-US
To:     Geert Uytterhoeven <geert@linux-m68k.org>
Cc:     "Martin K . Petersen" <martin.petersen@oracle.com>,
        Jaegeuk Kim <jaegeuk@kernel.org>,
        scsi <linux-scsi@vger.kernel.org>,
        Ming Lei <ming.lei@redhat.com>, Hannes Reinecke <hare@suse.de>,
        John Garry <john.garry@huawei.com>, ericspero@icloud.com,
        jason600.groome@gmail.com,
        Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
References: <20220630195703.10155-1-bvanassche@acm.org>
 <20220630195703.10155-3-bvanassche@acm.org>
 <alpine.DEB.2.22.394.2207191125130.1006766@ramsan.of.borg>
 <db19ed29-e7f9-e5b0-3a6c-f2812078a07d@acm.org>
 <CAMuHMdVzsgSYtbJQnaigNax_JbxPsQfU+gHcteS-ojWbxUdMfw@mail.gmail.com>
 <CAMuHMdWtxBj8ug7AHTqentF8UD4jpO2sgoWWcQCOvEKLJtdq8A@mail.gmail.com>
From:   Bart Van Assche <bvanassche@acm.org>
In-Reply-To: <CAMuHMdWtxBj8ug7AHTqentF8UD4jpO2sgoWWcQCOvEKLJtdq8A@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 7/20/22 00:47, Geert Uytterhoeven wrote:
> With more debug options enabled, it prints:
> 
> INFO: task kworker/0:7:283 blocked for more than 120 seconds.
>        Not tainted 5.19.0-rc7-salvator-x-00794-g6780eb02b605 #1287
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> task:kworker/0:7     state:D stack:    0 pid:  283 ppid:     2 flags:0x00000008
> Workqueue: events ata_scsi_dev_rescan
> Call trace:
>   __switch_to+0xbc/0x124
>   __schedule+0x540/0x71c
>   schedule+0x58/0xa0
>   io_schedule+0x18/0x34
>   blk_mq_get_tag+0x138/0x244
>   __blk_mq_alloc_requests+0x130/0x2f0
>   blk_mq_alloc_request+0x74/0xa8
>   scsi_alloc_request+0x10/0x30
>   __scsi_execute+0x5c/0x18c
>   scsi_vpd_inquiry+0x7c/0xdc
>   scsi_get_vpd_size+0x34/0xa8
>   scsi_get_vpd_buf+0x28/0xf4
>   scsi_attach_vpd+0x44/0x170
>   scsi_rescan_device+0x30/0x98
>   ata_scsi_dev_rescan+0xc8/0xfc
>   process_one_work+0x2e0/0x474
>   worker_thread+0x1cc/0x270
>   kthread+0xd8/0xe8
>   ret_from_fork+0x10/0x20
> 
> This doesn't look like it's blocked in the R-Car SATA driver, but on
> some I/O scheduling event in the block core?

I'm not familiar with the SATA code but from a quick look it seems like 
the above code is only triggered from inside the ATA error handler 
(ata_do_eh() -> ata_eh_recover() -> ata_eh_revalidate_and_attach() -> 
schedule_work(&(ap->scsi_rescan_task) -> ata_scsi_dev_rescan()). It 
doesn't seem normal to me that the ATA error handler gets invoked during 
a resume. How about testing the following two code changes?
* In sd_start_stop_device(), change "return sd_submit_start(sdkp, cmd, 
sizeof(cmd))" into "sd_submit_start(sdkp, cmd, sizeof(cmd))" and below 
that call add "flush_work(&sdkp->start_done_work)". This makes 
sd_start_stop_device() again synchronous. This will learn us whether the 
behavior change is caused by submitting the START command from another 
context or by not waiting until the START command has finished.
* Back out the above change, change "return sd_submit_start(sdkp, cmd, 
sizeof(cmd))" again into "sd_submit_start(sdkp, cmd, sizeof(cmd))" and 
below that statement add a call to 
scsi_run_queue(sdkp->device->request_queue). If this change helps it 
means that the scsi_run_queue() call is necessary to prevent reordering 
of the START command with other SCSI commands.

Thanks,

Bart.