BUG in scsi_lib.c due to a bad commit

PATCH Barto 5 participants 15 messages Nov 11, 2014 – Nov 19, 2014 SCSI

Share & tools

Barto

author

2014-11-11 23:33 UTC #0 lore raw

Hello everyone,

I notice a bug since kernel 3.17 ( and also with 3.18 branch ), a random hang at boot on some PC configurations, I did a "git bisect" and I found that the culprit is :

[045065d8a300a37218c548e9aa7becd581c6a0e8] [SCSI] fix qemu boot hang problem

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=045065d8a300a37218c548e9aa7becd581c6a0e8

the author of this commit has choosen to inverse the logic of the if statement in the file drivers/scsi/scsi_lib.c in order to solve an issue with qemu :

drivers/scsi/scsi_lib.c +0 -0

@@ -1774,7 +1774,7 @@ static void scsi_request_fn(struct request_queue *q)

blk_requeue_request(q, req); atomic_dec(&sdev->device_busy);
out_delay:
- if (atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev))
+ if (!atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev)) blk_delay_queue(q, SCSI_QUEUE_DELAY);
}

this change triggers a bug on my PC ( I don't have SCSI devices, but only 3 SATA harddisks and 2 IDE harddisks, SATA disks are on an ICH7 sata controler on the motherboard gigabyte GA-P31-DS3L, and IDE disk on a JMicron JMB363/368 Sata/IDE PCIe card ), every 5~10 boots the boot stops suddenly because of this commit,

If I revert this commit then the bug is gone, more details can be found here, where I created a patch who reverts commit 045065d8 :

https://bugzilla.kernel.org/show_bug.cgi?id=87581

my question: why Guenter Roeck ( the author of the bad commit ) has choosen to inverse the logic in the if statement ? before his commit the if statement was like this :

if (atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev)) blk_delay_queue(q, SCSI_QUEUE_DELAY);

if his decision to inverse the logic of "atomic_read(&sdev->device_busy)" is acceptable then the real bug is probably located elsewhere in the scsi source code, and we must solve this mistery because there is obviously a bug regression in SCSI code because with older kernels ( 3.16.7 and lower ) I don't have the random hang boot bug with my configuration,

another user in archlinux forums has also this bug and he has a more modern PC ( intel i7 core cpu, SSD device ), my fear is when linux distros will move to kernel 3.17 then more users will have this weird random bug who can occur only on boot and only with a specific PC configuration, if the boot step is passed despite the random bug then the bug will not occur, it occurs only during the boot process, which probably means that the faulty source code is only called during the boot process,

thanks for anyone who wants to dig this problem with me

Bjorn Helgaas

2014-11-12 00:17 UTC #1 lore raw

[+cc Guenter, linux-scsi]

Quoted 65 lines · Barto

Hello everyone,
I notice a bug since kernel 3.17 ( and also with 3.18 branch ), a random hang at boot on some PC configurations, I did a "git bisect" and I found that the culprit is :
[045065d8a300a37218c548e9aa7becd581c6a0e8] [SCSI] fix qemu boot hang problem
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=045065d8a300a37218c548e9aa7becd581c6a0e8
the author of this commit has choosen to inverse the logic of the if statement in the file drivers/scsi/scsi_lib.c in order to solve an issue with qemu :
drivers/scsi/scsi_lib.c +0 -0
@@ -1774,7 +1774,7 @@ static void scsi_request_fn(struct request_queue *q)
 blk_requeue_request(q, req);
 atomic_dec(&sdev->device_busy);
out_delay:
- if (atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev))
+ if (!atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev)) blk_delay_queue(q, SCSI_QUEUE_DELAY);
}
this change triggers a bug on my PC ( I don't have SCSI devices, but only 3 SATA harddisks and 2 IDE harddisks, SATA disks are on an ICH7 sata controler on the motherboard gigabyte GA-P31-DS3L, and IDE disk on a JMicron JMB363/368 Sata/IDE PCIe card ), every 5~10 boots the boot stops suddenly because of this commit,
If I revert this commit then the bug is gone, more details can be found here, where I created a patch who reverts commit 045065d8 :
https://bugzilla.kernel.org/show_bug.cgi?id=87581
my question: why Guenter Roeck ( the author of the bad commit ) has choosen to inverse the logic in the if statement ? before his commit the if statement was like this :
if (atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev)) blk_delay_queue(q, SCSI_QUEUE_DELAY);
if his decision to inverse the logic of "atomic_read(&sdev->device_busy)" is acceptable then the real bug is probably located elsewhere in the scsi source code, and we must solve this mistery because there is obviously a bug regression in SCSI code because with older kernels ( 3.16.7 and lower ) I don't have the random hang boot bug with my configuration,
another user in archlinux forums has also this bug and he has a more modern PC ( intel i7 core cpu, SSD device ), my fear is when linux distros will move to kernel 3.17 then more users will have this weird random bug who can occur only on boot and only with a specific PC configuration, if the boot step is passed despite the random bug then the bug will not occur, it occurs only during the boot process, which probably means that the faulty source code is only called during the boot process,
thanks for anyone who wants to dig this problem with me
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Guenter Roeck

2014-11-12 02:53 UTC #2 lore raw

Quoted 17 lines · Bjorn Helgaas

[+cc Guenter, linux-scsi]
Quoted 14 lines · Barto
Hello everyone,
I notice a bug since kernel 3.17 ( and also with 3.18 branch ), a random hang at boot on some PC configurations, I did a "git bisect" and I found that the culprit is :
[045065d8a300a37218c548e9aa7becd581c6a0e8] [SCSI] fix qemu boot hang problem
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=045065d8a300a37218c548e9aa7becd581c6a0e8
the author of this commit has choosen to inverse the logic of the if statement in the file drivers/scsi/scsi_lib.c in order to solve an issue with qemu :

Quoted 11 lines

Quoted 11 lines
drivers/scsi/scsi_lib.c +0 -0
@@ -1774,7 +1774,7 @@ static void scsi_request_fn(struct request_queue *q)
 blk_requeue_request(q, req);
 atomic_dec(&sdev->device_busy);
out_delay:
- if (atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev))
+ if (!atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev)) blk_delay_queue(q, SCSI_QUEUE_DELAY);
}

The above commit was a follow-up to commit 71e75c97f97a, which did the following:

- if (sdev->device_busy == 0 && !scsi_device_blocked(sdev))
+ if (atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev))

meaning it reversed the polarity of the check in the original code. Since that commit created problems as observed in qemu, I thought it would be obvious that restoring the original polarity would make sense. This is explained in my patch, so I am somewhat at loss why ...

Quoted 6 lines

Quoted 6 lines
this change triggers a bug on my PC ( I don't have SCSI devices, but only 3 SATA harddisks and 2 IDE harddisks, SATA disks are on an ICH7 sata controler on the motherboard gigabyte GA-P31-DS3L, and IDE disk on a JMicron JMB363/368 Sata/IDE PCIe card ), every 5~10 boots the boot stops suddenly because of this commit,

Quoted 7 lines

Quoted 7 lines
If I revert this commit then the bug is gone, more details can be found here, where I created a patch who reverts commit 045065d8 :
https://bugzilla.kernel.org/show_bug.cgi?id=87581
my question: why Guenter Roeck ( the author of the bad commit ) has choosen to inverse the logic in the if statement ?

this question is asked.

Having said that, I don't really care one way or another. I am fine with reverting my commit; if that is done, I'll simply remove the related scsi tests from my qemu test runs.

Guenter

Quoted 28 lines

Quoted 27 lines
before his commit the if statement was like this :
if (atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev)) blk_delay_queue(q, SCSI_QUEUE_DELAY);
if his decision to inverse the logic of "atomic_read(&sdev->device_busy)" is acceptable then the real bug is probably located elsewhere in the scsi source code, and we must solve this mistery because there is obviously a bug regression in SCSI code because with older kernels ( 3.16.7 and lower ) I don't have the random hang boot bug with my configuration,
another user in archlinux forums has also this bug and he has a more modern PC ( intel i7 core cpu, SSD device ), my fear is when linux distros will move to kernel 3.17 then more users will have this weird random bug who can occur only on boot and only with a specific PC configuration, if the boot step is passed despite the random bug then the bug will not occur, it occurs only during the boot process, which probably means that the faulty source code is only called during the boot process,
thanks for anyone who wants to dig this problem with me
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Barto

author

2014-11-13 03:28 UTC #3 lore raw

reverting your commit 045065d8a300a37218c is a solution, but it's just a temporary solution,

it's better to search why your commit can create a random hang on boot on some PC configurations,

drivers/scsi/scsi_lib.c +0 -0

@@ -1774,7 +1774,7 @@ static void scsi_request_fn(struct request_queue *q)

perhaps the atomic_read() function doesn't make the expected job on some rare circonstances, I have the same doubts about the blk_delay_queue() function

Quoted 6 lines · Bjorn Helgaas

Quoted 6 lines
[+cc Guenter, linux-scsi]
Quoted 2 lines · Barto
Hello everyone,

Quoted 8 lines

Quoted 8 lines
Quoted 8 lines
I notice a bug since kernel 3.17 ( and also with 3.18 branch ), a random hang at boot on some PC configurations, I did a "git bisect" and I found that the culprit is :
[045065d8a300a37218c548e9aa7becd581c6a0e8] [SCSI] fix qemu boot hang problem

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=045065d8a300a37218c548e9aa7becd581c6a0e8

Quoted 83 lines

Quoted 6 lines
Quoted 6 lines
the author of this commit has choosen to inverse the logic of the if statement in the file drivers/scsi/scsi_lib.c in order to solve an issue with qemu :
Quoted 12 lines
Quoted 6 lines
drivers/scsi/scsi_lib.c +0 -0
@@ -1774,7 +1774,7 @@ static void scsi_request_fn(struct
 request_queue *q)
 blk_requeue_request(q, req);
 atomic_dec(&sdev->device_busy);
Quoted 6 lines
out_delay:
- if (atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev))
+ if (!atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev)) blk_delay_queue(q, SCSI_QUEUE_DELAY);
}
The above commit was a follow-up to commit 71e75c97f97a, which did the following:
(quoted diff) +1 -1
-       if (sdev->device_busy == 0 && !scsi_device_blocked(sdev))
+       if (atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev))
meaning it reversed the polarity of the check in the original code. Since that commit created problems as observed in qemu, I thought it would be obvious that restoring the original polarity would make sense. This is explained in my patch, so I am somewhat at loss why ...
Quoted 6 lines
Quoted 6 lines
this change triggers a bug on my PC ( I don't have SCSI devices, but only 3 SATA harddisks and 2 IDE harddisks, SATA disks are on an ICH7 sata controler on the motherboard gigabyte GA-P31-DS3L, and IDE disk on a JMicron JMB363/368 Sata/IDE PCIe card ), every 5~10 boots the boot stops suddenly because of this commit,
Quoted 7 lines
Quoted 7 lines
If I revert this commit then the bug is gone, more details can be found here, where I created a patch who reverts commit 045065d8 :
https://bugzilla.kernel.org/show_bug.cgi?id=87581
my question: why Guenter Roeck ( the author of the bad commit ) has choosen to inverse the logic in the if statement ?
this question is asked.
Having said that, I don't really care one way or another. I am fine with reverting my commit; if that is done, I'll simply remove the related scsi tests from my qemu test runs.
Guenter
Quoted 29 lines
Quoted 28 lines
before his commit the if statement was like this :
if (atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev)) blk_delay_queue(q, SCSI_QUEUE_DELAY);
if his decision to inverse the logic of "atomic_read(&sdev->device_busy)" is acceptable then the real bug is probably located elsewhere in the scsi source code, and we must solve this mistery because there is obviously a bug regression in SCSI code because with older kernels ( 3.16.7 and lower ) I don't have the random hang boot bug with my configuration,
another user in archlinux forums has also this bug and he has a more modern PC ( intel i7 core cpu, SSD device ), my fear is when linux distros will move to kernel 3.17 then more users will have this weird random bug who can occur only on boot and only with a specific PC configuration, if the boot step is passed despite the random bug then the bug will not occur, it occurs only during the boot process, which probably means that the faulty source code is only called during the boot process,
thanks for anyone who wants to dig this problem with me --
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Elliott, Robert (Server Storage)

2014-11-13 05:35 UTC #4 lore raw

Quoted 29 lines

-----Original Message-----
From: [email protected] [mailto:linux-kernel- [email protected]] On Behalf Of Barto
Sent: Wednesday, November 12, 2014 9:28 PM
To: Guenter Roeck; Bjorn Helgaas
Cc: [email protected]; [email protected]; Joe Perches
Subject: Re: BUG in scsi_lib.c due to a bad commit
reverting your commit 045065d8a300a37218c is a solution, but it's just a temporary solution,
it's better to search why your commit can create a random hang on boot on some PC configurations,
drivers/scsi/scsi_lib.c +0 -0
@@ -1774,7 +1774,7 @@ static void scsi_request_fn(struct request_queue
 *q)
 blk_requeue_request(q, req);
 atomic_dec(&sdev->device_busy);
out_delay:
- if (atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev))
+ if (!atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev)) blk_delay_queue(q, SCSI_QUEUE_DELAY);
}

Quoted 3 lines

perhaps the atomic_read() function doesn't make the expected job on some rare circonstances, I have the same doubts about the blk_delay_queue() function

Were you running with scsi_mod.use_blk_mq=Y or =N?

device_busy is the active queue depth for the device (e.g. 5 means there are 5 commands submitted but not yet completed).

The function reaches this code if it has run out of tags, the host has reached its limit of outstanding commands, or the target has reached its limit. It requeus the request:
* with delay if device_busy is zero
* without delay if device_busy is non_zero

I think this is the reasoning:
If device_busy is zero, trying to process the request again will probably run into the same problem; a delay gives time for the situation to change. If device_busy is non-zero, then the requeued command goes behind others and might get a different result.

With the polarity backwards, the lack of delay hung PA-RISC and SPARC64 systems), not just QEMU. So, I don't think reverting the fix is good.

Changing it to an unconditional delay might be safe - delay regardless of device_busy (until the root cause is understood).

Also, SCSI_QUEUE_DELAY seems like an arbitrary magic number; maybe that value isn't working correctly anymore?

Barto

author

2014-11-13 09:38 UTC #5 lore raw

Hello,

Quoted 1 line

Were you running with scsi_mod.use_blk_mq=Y or =N?

I don't find this value in my .config file related to kernel modules options,

perhaps you talk about a kernel boot option for grub ?
I don't use this kernel option boot in grub ( scsi_mod.use_blk_mq ),

I use archlinux and his default kernel config file for 64 bits CPU, you can find this file here :

https://projects.archlinux.org/svntogit/packages.git/tree/trunk?h=packages/linux

I don't have SCSI devices, I have only 3 SATA harddisks, and 2 IDE harddisks,

before kernel 3.17 I don't have this random hang bug on boot, so we need to know what's going on, it's probably a change made last months ago in SCSI source code ( or something else ) who has introduced a bug who prevents a correct behaviour on some parts in scsi_lib.c ( for example the if statement related to atomic_read(&sdev->device_busy) and blk_delay_queue(q, SCSI_QUEUE_DELAY) )

Guenter Roeck was the first to have been hit by this bug ( his qemu test files hang on boot ), he thought the solution was to revert the polarity of the if statement, it solves his problem but unfortunately a new bug is now triggered on some PC configurations,

it would be interesting to find a definitive solution who can solve both the "qemu bug" and the "random bug on boot" on some PC configurations,

I'm not an expert about the scsi code, perhaps with some unit tests you can spot the defect element in the scsi source code, check if some parts in the source code acts really like it should be ?

Quoted 62 lines · Robert (Server Storage)

Quoted 29 lines
-----Original Message-----
From: [email protected] [mailto:linux-kernel- [email protected]] On Behalf Of Barto
Sent: Wednesday, November 12, 2014 9:28 PM
To: Guenter Roeck; Bjorn Helgaas
Cc: [email protected]; [email protected]; Joe Perches
Subject: Re: BUG in scsi_lib.c due to a bad commit
reverting your commit 045065d8a300a37218c is a solution, but it's just a temporary solution,
it's better to search why your commit can create a random hang on boot on some PC configurations,
drivers/scsi/scsi_lib.c +0 -0
@@ -1774,7 +1774,7 @@ static void scsi_request_fn(struct request_queue
 *q)
 blk_requeue_request(q, req);
 atomic_dec(&sdev->device_busy);
out_delay:
- if (atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev))
+ if (!atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev)) blk_delay_queue(q, SCSI_QUEUE_DELAY);
}
Quoted 3 lines
perhaps the atomic_read() function doesn't make the expected job on some rare circonstances, I have the same doubts about the blk_delay_queue() function
Were you running with scsi_mod.use_blk_mq=Y or =N?
device_busy is the active queue depth for the device (e.g. 5 means there are 5 commands submitted but not yet completed).
The function reaches this code if it has run out of tags, the host has reached its limit of outstanding commands, or the target has reached its limit. It requeus the request:
* with delay if device_busy is zero
* without delay if device_busy is non_zero
I think this is the reasoning:
If device_busy is zero, trying to process the request again will probably run into the same problem; a delay gives time for the situation to change. If device_busy is non-zero, then the requeued command goes behind others and might get a different result.
With the polarity backwards, the lack of delay hung PA-RISC and SPARC64 systems), not just QEMU. So, I don't think reverting the fix is good.
Changing it to an unconditional delay might be safe - delay regardless of device_busy (until the root cause is understood).
Also, SCSI_QUEUE_DELAY seems like an arbitrary magic number; maybe that value isn't working correctly anymore?

Christoph Hellwig

2014-11-13 14:29 UTC #6 lore raw

Guenters commit just fixes the inversion from commit

71e75c97f97a9645d25fbf3d8e4165a558f18747 scsi: convert device_busy to atomic_t

Can you please test the commit before that:
74665016086615bbaa3fa6f83af410a0a4e029ee scsi: convert host_busy to atomic_t

and check if that works for you or not.

Barto

author

2014-11-13 15:13 UTC #7 lore raw

I am not sure to understand your request, it's quite confusing,

you ask me to revert this commit and see if the bug is gone ? :

74665016086615bbaa3fa6f83af410a0a4e029ee scsi: convert host_busy to atomic_t

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=74665016086615bbaa3fa6f83af410a0a4e029ee

Quoted 9 lines · Christoph Hellwig

Guenters commit just fixes the inversion from commit
71e75c97f97a9645d25fbf3d8e4165a558f18747 scsi: convert device_busy to atomic_t
Can you please test the commit before that:
74665016086615bbaa3fa6f83af410a0a4e029ee scsi: convert host_busy to atomic_t
and check if that works for you or not.

Barto

author

2014-11-13 17:14 UTC #8 lore raw

In reply to Christoph Hellwig

Hello Christoph,

I tested this commit :

7ae65c0f9646c29432b69580b80e08632e6cd813 scsi: convert target_busy to an atomic_t

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=7ae65c0f9646c29432b69580b80e08632e6cd813

there is no bug, boot process is ok whit this commit,

when I did the "git bisect" the first bad commit was Guenters' commit ( 045065d8a300a37218c548e9aa7becd581c6a0e8 ), it seems that all commits before 045065d8a300a37218c548e9aa7becd581c6a0e8 are ok,

you can find the log of the bisect here :

https://bugzilla.kernel.org/attachment.cgi?id=157211

Quoted 9 lines · Christoph Hellwig

Guenters commit just fixes the inversion from commit
71e75c97f97a9645d25fbf3d8e4165a558f18747 scsi: convert device_busy to atomic_t
Can you please test the commit before that:
74665016086615bbaa3fa6f83af410a0a4e029ee scsi: convert host_busy to atomic_t
and check if that works for you or not.

Christoph Hellwig

2014-11-13 17:54 UTC #9 lore raw

Quoted 7 lines · Barto

Hello Christoph,
I tested this commit :
7ae65c0f9646c29432b69580b80e08632e6cd813 scsi: convert target_busy to an atomic_t

Quoted 7 lines

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=7ae65c0f9646c29432b69580b80e08632e6cd813
there is no bug, boot process is ok whit this commit,
when I did the "git bisect" the first bad commit was Guenters' commit ( 045065d8a300a37218c548e9aa7becd581c6a0e8 ), it seems that all commits before 045065d8a300a37218c548e9aa7becd581c6a0e8 are ok,

The bisect isn't quite interesting here as we're trying to find the real root ause.

045065d8a300a37218c548e9aa7becd581c6a0e8 reverts the polarity of the check that 71e75c97f97a9645d25fbf3d8e4165a558f18747 incorrectly chaged.

The behavior you see doesn't make a lot of sense from a high level view, so we need to find out what's going on.

When you say all commits before 045065d8a300a37218c548e9aa7becd581c6a0e8 I wonder if the bisect simply skiped to many revisions.

Please explicitly test commits:

71e75c97f97a9645d25fbf3d8e4165a558f18747

and

74665016086615bbaa3fa6f83af410a0a4e029ee

Thanks!

Barto

author

2014-11-13 22:55 UTC #10 lore raw

Hello Christoph,

Quoted 3 lines

Please explicitly test commits:
71e75c97f97a9645d25fbf3d8e4165a558f18747

with commit 71e75c97f97a9645d25fbf3d8e4165a558f18747 there is no problem

Quoted 3 lines

and
74665016086615bbaa3fa6f83af410a0a4e029ee

it's interesting, with this commit 74665016086615bbaa3fa6f83af410a0a4e029ee I have the bug :

scsi: convert host_busy to atomic_t :

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=74665016086615bbaa3fa6f83af410a0a4e029ee

Quoted 40 lines · Christoph Hellwig

Quoted 7 lines · Barto
Hello Christoph,
I tested this commit :
7ae65c0f9646c29432b69580b80e08632e6cd813 scsi: convert target_busy to an atomic_t
Quoted 7 lines
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=7ae65c0f9646c29432b69580b80e08632e6cd813
there is no bug, boot process is ok whit this commit,
when I did the "git bisect" the first bad commit was Guenters' commit ( 045065d8a300a37218c548e9aa7becd581c6a0e8 ), it seems that all commits before 045065d8a300a37218c548e9aa7becd581c6a0e8 are ok,
The bisect isn't quite interesting here as we're trying to find the real root ause.
045065d8a300a37218c548e9aa7becd581c6a0e8 reverts the polarity of the check that 71e75c97f97a9645d25fbf3d8e4165a558f18747 incorrectly chaged.
The behavior you see doesn't make a lot of sense from a high level view, so we need to find out what's going on.
When you say all commits before 045065d8a300a37218c548e9aa7becd581c6a0e8 I wonder if the bisect simply skiped to many revisions.
Please explicitly test commits:
71e75c97f97a9645d25fbf3d8e4165a558f18747
and
74665016086615bbaa3fa6f83af410a0a4e029ee
Thanks!

Christoph Hellwig

2014-11-14 07:32 UTC #11 lore raw

Quoted 4 lines · Barto

it's interesting, with this commit 74665016086615bbaa3fa6f83af410a0a4e029ee I have the bug :
scsi: convert host_busy to atomic_t :

At this point we'll need a bisction between v3.16 as the last good point, and 74665016086615bbaa3fa6f83af410a0a4e029ee as the known bad point.

Barto

author

2014-11-14 16:30 UTC #12 lore raw

Hello Christoph,

I did the new bisect,

the first bad commit is :

74665016086615bbaa3fa6f83af410a0a4e029ee
scsi: convert host_busy to atomic_t

you can find the bisect log here :

https://bugzilla.kernel.org/attachment.cgi?id=157621

Quoted 10 lines · Christoph Hellwig

Quoted 4 lines · Barto
it's interesting, with this commit 74665016086615bbaa3fa6f83af410a0a4e029ee I have the bug :
scsi: convert host_busy to atomic_t :
At this point we'll need a bisction between v3.16 as the last good point, and 74665016086615bbaa3fa6f83af410a0a4e029ee as the known bad point.

Barto

author

2014-11-16 18:30 UTC #13 lore raw

In reply to Christoph Hellwig

Hello everyone,

Quoted 2 lines

Also, SCSI_QUEUE_DELAY seems like an arbitrary magic number; maybe that value isn't working correctly anymore?

this is an excellent remark from Robert Elliot,

this gives me an idea : try to set manually a value in the if() statement ( line 1779 in file /drivers/scsi/scsi_lib.c )

by default the value of SCSI_QUEUE_DELAY is 3 ms, which might be inapropriate with some slow harddisks and with the changes made by the commit
74665016086615bbaa3fa6f83af410a0a4e029ee ( scsi: convert host_busy to atomic_t ),

after further tests I discover that the value 40 ms solves my problem, the bug is gone with this value,

here is the patch who sets 40 ms in the if() statement :

drivers/scsi/scsi_lib.c +1 -1

@@ -1776,7 +1776,7 @@ static void scsi_request_fn(struct reque
 	atomic_dec(&sdev->device_busy);
 out_delay:
 	if (!atomic_read(&sdev->device_busy) && !scsi_device_blocked(sdev))
-		blk_delay_queue(q, SCSI_QUEUE_DELAY);
+		blk_delay_queue(q, 40);
 }
 
 static inline int prep_to_mq(int ret)

with this patch the value of SCSI_QUEUE_DELAY is still 3 ms, but here we use 40 ms only in a specific part of scsi_lib.c file ( line 1779, it's this part where the bug seems to be triggered, so that's why I set 40 ms here in the blk_delay_queue() function )

after applying this patch I don't see problems related to I/O performance/speed, all is ok,

the question is now : why putting a higher value in line 1779 does solve the bug ?
and why before the commit 74665016086615bbaa3fa6f83af410a0a4e029ee I don't have problems even with a value of 3 ms for SCSI_QUEUE_DELAY ?

Quoted 10 lines · Christoph Hellwig

Quoted 4 lines · Barto
it's interesting, with this commit 74665016086615bbaa3fa6f83af410a0a4e029ee I have the bug :
scsi: convert host_busy to atomic_t :
At this point we'll need a bisction between v3.16 as the last good point, and 74665016086615bbaa3fa6f83af410a0a4e029ee as the known bad point.

Barto

author

2014-11-19 20:21 UTC #14 lore raw

In reply to Christoph Hellwig

Hello,

I solved the mystery,

I found that the element who triggers the bug ( random hang at boot with kernel 3.17 and 3.18 ) is the combination of 3 elements :

- the use of a SATA DVD burner ( Liteon iHAS124 C ) on a ICH7 Sata controler
- the use of a gigabyte motherboard GA-P31-DSL3 ( bios F10A, ICH7 controler, intel P31 chipset )
- commit 74665016086615bbaa3fa6f83af410a0a4e029ee ( scsi: convert host_busy to atomic_t )

If I connect this Sata DVD burner and a sata harddisk by using the SATA ports of the motherboard then the bug will occur ( but the bug will occur only on kernels 3.17 and 3.18, there is no problems with older kernels, and no problems with Windows 7 )

If I disconnect the SATA DVD burner then the bug is gone, no problems with kernels 3.17 and 3.18,

And if I connect the SATA DVD burner on my JMicron SATA/IDE PCIe card then there is no problem, no bugs, this is a perfect workaround for my problem, because I can use kernel 3.17/3.18 without problem with this configuration.

But I don't know which element I should blame, my gigabyte motherboard ? ( faulty bios ? ) The use of "atomic_t" in scsi source code ? ( innapropriate way to handle SATA devices, it breaks compatibility with some PC configurations ? )

Quoted 10 lines · Christoph Hellwig

Quoted 4 lines · Barto
it's interesting, with this commit 74665016086615bbaa3fa6f83af410a0a4e029ee I have the bug :
scsi: convert host_busy to atomic_t :
At this point we'll need a bisction between v3.16 as the last good point, and 74665016086615bbaa3fa6f83af410a0a4e029ee as the known bad point.