lkml.org 
[lkml]   [2017]   [Aug]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [BUG][bisected 270065e] linux-next fails to boot on powerpc
From
Date
On 08/18/2017 04:41 PM, Bart Van Assche wrote:
> On Fri, 2017-08-18 at 16:04 -0500, Brian King wrote:
>> I think I have an understanding what is going on and why Bart's patch is causing problems for ipr.
>> I can work around the boot hang in ipr, but ultimately I think we need to figure out a fix
>> in scsi / block. I added some tracing and confirmed its not a matter of commands getting stuck
>> in ipr. The issue is we are retrying failed commands until we finally run out of time. This is
>> what I see:
>>
>> 1. sd_revalidate_disk calls scsi_report_opcode
>> 2. ipr RAID arrays don't support MAINTENANCE_IN / MI_REPORT_SUPPORTED_OPERATION_CODES
>> 3. ipr returns the command with DID_ERROR
>> 4. scsi_decide_disposition goes to maybe_retry, increments scmd->retries, and returns NEEDS_RETRY
>> 5. scsi_softirq_done calls scsi_queue_insert to requeue the command, which calls scsi_mq_requeue_cmd
>> 6. With Bart's change, we then clear RQF_DONTPREP in this path, while prior we did not
>> 7. This results in the command getting scmd->retries zeroed out when it gets re-queued,
>> since we go through prep again and we lose our retry counter, resulting in lots and lots of retries.
>> 8. Since the default command timeout for an ipr RAID array is 120 seconds, these retries go on for
>> quite a long time...
>> 9. Finally, the command has been retried so long we trip over the overall retry timer
>> in scsi_softirq_done and we timeout the command.
>>
>> I'll follow up with a patch to ipr to workaround the hang, but I think we need to somehow preserve
>> the retry counter in the scsi command, as this will likely cause issues with other drivers.
>
> Hello Brian,
>
> Thanks for the detailed analysis. This is very helpful. Have you considered
> to change the ipr driver such that it terminates REPORT SUPPORTED OPERATION
> CODES commands with the appropriate check condition code instead of DID_ERROR?

Yes. That data is actually in the sense buffer, but since I'm also setting DID_ERROR,
scsi_decide_disposition isn't using it. I've got a patch to do just as you suggest,
to stop setting DID_ERROR when there is more detailed error data available,
but it will need some additional testing before I submit, as it will impact much
more than just this case.

To add to my analysis above, #9 should not be there... It looks like
jiffies_at_alloc would also be getting reinitialized in this case, resulting in
a perpetual retry, which is what I was seeing.

Thanks,

Brian

--
Brian King
Power Linux I/O
IBM Linux Technology Center

\
 
 \ /
  Last update: 2017-08-18 23:58    [W:0.089 / U:0.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site