On Fri, Jan 29, 2016 at 1:23 PM, Tejun Heo <tj@xxxxxxxxxx> wrote:
> Hello, Dmitry.
>
> On Fri, Jan 29, 2016 at 12:59:49PM +0100, Dmitry Vyukov wrote:
>> > Hmmm... the port interrupt handler checks for IDLE before calling into
>> > hsm_move, so the only explanation would be that something is resetting
>> > it to IDLE inbetween. ce7514526742 ("libata: prevent HSM state change
>> > race between ISR and PIO") describes and fixes the same problem. The
>> > fix seems correct and I can't find anywhere else where this can
>> > happen. :(
>> >
>> > Can you please post the kernel log leading to the BUG? Also, I don't
>> > think that condition needs to be BUG. I'll change it to WARN.
>>
>> Here are two logs, in both cases no kernel messages prior to the bug:
>> https://gist.githubusercontent.com/dvyukov/5087d633e3620280b6c7/raw/31c9ab1ced92ac5f85cfb15eaf48ec5793c2c3a1/gistfile1.txt
>> https://gist.githubusercontent.com/dvyukov/825b2e3d5fb80ae08a9a/raw/03c5a4f4c4bd9d0a304a71cda2da4c92f4b7f1ba/gistfile1.txt
>
> lol, this is kinda embarrassing. It looks like the poll path wasn't
> doing any locking. Can you please verify the following patch at least
> doesn't crash the machine immediately and if so keep it applied to the
> test kernel so that we can verify that the problem actually goes away?
Great that you managed to debug it without a repro!
I've applied this patch to my tree and will rerun fuzzer. I will
notify you if I see this warning again.
Thanks
> diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
> index 608677d..6991efc 100644
> --- a/drivers/ata/libata-sff.c
> +++ b/drivers/ata/libata-sff.c
> @@ -1362,12 +1362,14 @@ static void ata_sff_pio_task(struct work_struct *work)
> u8 status;
> int poll_next;
>
> + spin_lock_irq(ap->lock);
> +
> BUG_ON(ap->sff_pio_task_link == NULL);
> /* qc can be NULL if timeout occurred */
> qc = ata_qc_from_tag(ap, link->active_tag);
> if (!qc) {
> ap->sff_pio_task_link = NULL;
> - return;
> + goto out_unlock;
> }
>
> fsm_start:
> @@ -1382,11 +1384,14 @@ static void ata_sff_pio_task(struct work_struct *work)
> */
> status = ata_sff_busy_wait(ap, ATA_BUSY, 5);
> if (status & ATA_BUSY) {
> + spin_unlock_irq(ap->lock);
> ata_msleep(ap, 2);
> + spin_lock_irq(ap->lock);
> +
> status = ata_sff_busy_wait(ap, ATA_BUSY, 10);
> if (status & ATA_BUSY) {
> ata_sff_queue_pio_task(link, ATA_SHORT_PAUSE);
> - return;
> + goto out_unlock;
> }
> }
>
> @@ -1403,6 +1408,8 @@ static void ata_sff_pio_task(struct work_struct *work)
> */
> if (poll_next)
> goto fsm_start;
> +out_unlock:
> + spin_unlock_irq(ap->lock);
> }
>
> /**
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller+unsubscribe@xxxxxxxxxxxxxxxx.
> For more options, visit https://groups.google.com/d/optout.
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
