* [PATCH] UML: UBD: Fix for processes stuck in D state forever in UserModeLinux
@ 2014-08-23 13:47 Thorsten Knabe
2014-08-23 15:34 ` Richard Weinberger
0 siblings, 1 reply; 9+ messages in thread
From: Thorsten Knabe @ 2014-08-23 13:47 UTC (permalink / raw)
To: Richard Weinberger; +Cc: linux-kernel, Thomas Meyer, Valdis.Kletnieks, linux
From: Thorsten Knabe <linux@thorsten-knabe.de>
UML: UBD: Fix for processes stuck in D state forever in UserModeLinux.
Starting with Linux 3.12 processes get stuck in D state forever in
UserModeLinux under sync heavy workloads. This bug was introduced by
commit 805f11a0d5 (um: ubd: Add REQ_FLUSH suppport).
Fix bug by adding a check if FLUSH request was successfully submitted to
the I/O thread and keeping the FLUSH request on the request queue on
submission failures.
Fixes: 805f11a0d5 (um: ubd: Add REQ_FLUSH suppport)
Signed-off-by: Thorsten Knabe <linux@thorsten-knabe.de>
---
Patch applies to 3.16.1.
diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
index 3716e69..b7d2840 100644
--- a/arch/um/drivers/ubd_kern.c
+++ b/arch/um/drivers/ubd_kern.c
@@ -1277,7 +1277,7 @@ static void do_ubd_request(struct request_queue *q)
while(1){
struct ubd *dev = q->queuedata;
- if(dev->end_sg == 0){
+ if(dev->request == NULL){
struct request *req = blk_fetch_request(q);
if(req == NULL)
return;
@@ -1299,7 +1299,8 @@ static void do_ubd_request(struct request_queue *q)
return;
}
prepare_flush_request(req, io_req);
- submit_request(io_req, dev);
+ if (submit_request(io_req, dev) == false)
+ return;
}
while(dev->start_sg < dev->end_sg){
--
___
| | / E-Mail: linux@thorsten-knabe.de
|horsten |/\nabe WWW: http://linux.thorsten-knabe.de
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] UML: UBD: Fix for processes stuck in D state forever in UserModeLinux
2014-08-23 13:47 [PATCH] UML: UBD: Fix for processes stuck in D state forever in UserModeLinux Thorsten Knabe
@ 2014-08-23 15:34 ` Richard Weinberger
2014-08-23 17:43 ` Thorsten Knabe
0 siblings, 1 reply; 9+ messages in thread
From: Richard Weinberger @ 2014-08-23 15:34 UTC (permalink / raw)
To: Thorsten Knabe; +Cc: linux-kernel, Thomas Meyer, Valdis.Kletnieks
Hi!
Am 23.08.2014 15:47, schrieb Thorsten Knabe:
> From: Thorsten Knabe <linux@thorsten-knabe.de>
>
> UML: UBD: Fix for processes stuck in D state forever in UserModeLinux.
>
> Starting with Linux 3.12 processes get stuck in D state forever in
> UserModeLinux under sync heavy workloads. This bug was introduced by
> commit 805f11a0d5 (um: ubd: Add REQ_FLUSH suppport).
> Fix bug by adding a check if FLUSH request was successfully submitted to
> the I/O thread and keeping the FLUSH request on the request queue on
> submission failures.
>
> Fixes: 805f11a0d5 (um: ubd: Add REQ_FLUSH suppport)
> Signed-off-by: Thorsten Knabe <linux@thorsten-knabe.de>
Thanks a lot for hunting this issue down.
> ---
> Patch applies to 3.16.1.
>
> diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
> index 3716e69..b7d2840 100644
> --- a/arch/um/drivers/ubd_kern.c
> +++ b/arch/um/drivers/ubd_kern.c
> @@ -1277,7 +1277,7 @@ static void do_ubd_request(struct request_queue *q)
>
> while(1){
> struct ubd *dev = q->queuedata;
> - if(dev->end_sg == 0){
> + if(dev->request == NULL){
Why do we need this specific change?
Thanks,
//richard
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] UML: UBD: Fix for processes stuck in D state forever in UserModeLinux
2014-08-23 15:34 ` Richard Weinberger
@ 2014-08-23 17:43 ` Thorsten Knabe
2014-08-24 12:11 ` Richard Weinberger
0 siblings, 1 reply; 9+ messages in thread
From: Thorsten Knabe @ 2014-08-23 17:43 UTC (permalink / raw)
To: Richard Weinberger; +Cc: linux-kernel, Thomas Meyer, Valdis.Kletnieks
Hi Richard.
On 08/23/2014 05:34 PM, Richard Weinberger wrote:
> Hi!
>
> Am 23.08.2014 15:47, schrieb Thorsten Knabe:
>> From: Thorsten Knabe <linux@thorsten-knabe.de>
>>
>> UML: UBD: Fix for processes stuck in D state forever in UserModeLinux.
>>
>> Starting with Linux 3.12 processes get stuck in D state forever in
>> UserModeLinux under sync heavy workloads. This bug was introduced by
>> commit 805f11a0d5 (um: ubd: Add REQ_FLUSH suppport).
>> Fix bug by adding a check if FLUSH request was successfully submitted to
>> the I/O thread and keeping the FLUSH request on the request queue on
>> submission failures.
>>
>> Fixes: 805f11a0d5 (um: ubd: Add REQ_FLUSH suppport)
>> Signed-off-by: Thorsten Knabe <linux@thorsten-knabe.de>
>
> Thanks a lot for hunting this issue down.
>
>> ---
>> Patch applies to 3.16.1.
>>
>> diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
>> index 3716e69..b7d2840 100644
>> --- a/arch/um/drivers/ubd_kern.c
>> +++ b/arch/um/drivers/ubd_kern.c
>> @@ -1277,7 +1277,7 @@ static void do_ubd_request(struct request_queue *q)
>>
>> while(1){
>> struct ubd *dev = q->queuedata;
>> - if(dev->end_sg == 0){
>> + if(dev->request == NULL){
>
> Why do we need this specific change?
This change is required, because for FLUSH requests dev->end_sg is
initialized to 0 by blk_rq_map_sg() a few lines above, as FLUSH requests
have no data blocks attached to themselves.
Checking for dev->end_sg == 0 would then replace a not yet submitted
FLUSH request by the next request on the next iteration of the while
loop, also the FLUSH request was scheduled for resubmission to the I/O
thread.
Kind regards
Thorsten
>
> Thanks,
> //richard
>
--
___
| | / E-Mail: linux@thorsten-knabe.de
|horsten |/\nabe WWW: http://linux.thorsten-knabe.de
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] UML: UBD: Fix for processes stuck in D state forever in UserModeLinux
2014-08-23 17:43 ` Thorsten Knabe
@ 2014-08-24 12:11 ` Richard Weinberger
2014-08-24 23:02 ` Thorsten Knabe
0 siblings, 1 reply; 9+ messages in thread
From: Richard Weinberger @ 2014-08-24 12:11 UTC (permalink / raw)
To: Thorsten Knabe; +Cc: linux-kernel, Thomas Meyer, Valdis.Kletnieks
Am 23.08.2014 19:43, schrieb Thorsten Knabe:
> Hi Richard.
>
> On 08/23/2014 05:34 PM, Richard Weinberger wrote:
>> Hi!
>>
>> Am 23.08.2014 15:47, schrieb Thorsten Knabe:
>>> From: Thorsten Knabe <linux@thorsten-knabe.de>
>>>
>>> UML: UBD: Fix for processes stuck in D state forever in UserModeLinux.
>>>
>>> Starting with Linux 3.12 processes get stuck in D state forever in
>>> UserModeLinux under sync heavy workloads. This bug was introduced by
>>> commit 805f11a0d5 (um: ubd: Add REQ_FLUSH suppport).
>>> Fix bug by adding a check if FLUSH request was successfully submitted to
>>> the I/O thread and keeping the FLUSH request on the request queue on
>>> submission failures.
>>>
>>> Fixes: 805f11a0d5 (um: ubd: Add REQ_FLUSH suppport)
>>> Signed-off-by: Thorsten Knabe <linux@thorsten-knabe.de>
>>
>> Thanks a lot for hunting this issue down.
>>
>>> ---
>>> Patch applies to 3.16.1.
>>>
>>> diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
>>> index 3716e69..b7d2840 100644
>>> --- a/arch/um/drivers/ubd_kern.c
>>> +++ b/arch/um/drivers/ubd_kern.c
>>> @@ -1277,7 +1277,7 @@ static void do_ubd_request(struct request_queue *q)
>>>
>>> while(1){
>>> struct ubd *dev = q->queuedata;
>>> - if(dev->end_sg == 0){
>>> + if(dev->request == NULL){
>>
>> Why do we need this specific change?
>
> This change is required, because for FLUSH requests dev->end_sg is
> initialized to 0 by blk_rq_map_sg() a few lines above, as FLUSH requests
> have no data blocks attached to themselves.
You meant "below"? Looks like I really miss something here.
At the bottom of the while(1) loop we have
dev->end_sg = 0;
dev->request = NULL;
Thanks,
//richard
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] UML: UBD: Fix for processes stuck in D state forever in UserModeLinux
2014-08-24 12:11 ` Richard Weinberger
@ 2014-08-24 23:02 ` Thorsten Knabe
2014-08-25 13:25 ` Richard Weinberger
0 siblings, 1 reply; 9+ messages in thread
From: Thorsten Knabe @ 2014-08-24 23:02 UTC (permalink / raw)
To: Richard Weinberger; +Cc: linux-kernel, Thomas Meyer, Valdis.Kletnieks
On 08/24/2014 02:11 PM, Richard Weinberger wrote:
> Am 23.08.2014 19:43, schrieb Thorsten Knabe:
>> Hi Richard.
>>
>> On 08/23/2014 05:34 PM, Richard Weinberger wrote:
>>> Hi!
>>>
>>> Am 23.08.2014 15:47, schrieb Thorsten Knabe:
>>>> From: Thorsten Knabe <linux@thorsten-knabe.de>
>>>>
>>>> UML: UBD: Fix for processes stuck in D state forever in UserModeLinux.
>>>>
>>>> Starting with Linux 3.12 processes get stuck in D state forever in
>>>> UserModeLinux under sync heavy workloads. This bug was introduced by
>>>> commit 805f11a0d5 (um: ubd: Add REQ_FLUSH suppport).
>>>> Fix bug by adding a check if FLUSH request was successfully submitted to
>>>> the I/O thread and keeping the FLUSH request on the request queue on
>>>> submission failures.
>>>>
>>>> Fixes: 805f11a0d5 (um: ubd: Add REQ_FLUSH suppport)
>>>> Signed-off-by: Thorsten Knabe <linux@thorsten-knabe.de>
>>>
>>> Thanks a lot for hunting this issue down.
>>>
>>>> ---
>>>> Patch applies to 3.16.1.
>>>>
>>>> diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
>>>> index 3716e69..b7d2840 100644
>>>> --- a/arch/um/drivers/ubd_kern.c
>>>> +++ b/arch/um/drivers/ubd_kern.c
>>>> @@ -1277,7 +1277,7 @@ static void do_ubd_request(struct request_queue *q)
>>>>
>>>> while(1){
>>>> struct ubd *dev = q->queuedata;
>>>> - if(dev->end_sg == 0){
>>>> + if(dev->request == NULL){
>>>
>>> Why do we need this specific change?
>>
>> This change is required, because for FLUSH requests dev->end_sg is
>> initialized to 0 by blk_rq_map_sg() a few lines above, as FLUSH requests
>> have no data blocks attached to themselves.
>
> You meant "below"? Looks like I really miss something here.
> At the bottom of the while(1) loop we have
> dev->end_sg = 0;
> dev->request = NULL;
No. The problematic line is:
dev->end_sg = blk_rq_map_sg(q, req, dev->sg);
and blk_rq_map_sg() returning 0 for REQ_FLUSH requests, because they
have no associated data blocks.
Hence on the next iteration of the while(1) loop:
if(dev->end_sg == 0){
will be true, even if the request has not been successfully submitted to
the I/O thread in the previous iteration of the while(1) loop and a new
request will be fetched:
struct request *req = blk_fetch_request(q);
if(req == NULL)
return;
dev->request = req;
dev->rq_pos = blk_rq_pos(req);
dev->start_sg = 0;
dev->end_sg = blk_rq_map_sg(q, req, dev->sg);
}
Thus the REQ_FLUSH request got lost and will never get submitted to the
I/O thread, there will be no matching answer from the I/O thread and the
lost REQ_FLUSH request will never complete...
Regards
Thorsten
>
> Thanks,
> //richard
>
--
___
| | / E-Mail: linux@thorsten-knabe.de
|horsten |/\nabe WWW: http://linux.thorsten-knabe.de
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] UML: UBD: Fix for processes stuck in D state forever in UserModeLinux
2014-08-24 23:02 ` Thorsten Knabe
@ 2014-08-25 13:25 ` Richard Weinberger
2014-08-25 14:00 ` Thorsten Knabe
0 siblings, 1 reply; 9+ messages in thread
From: Richard Weinberger @ 2014-08-25 13:25 UTC (permalink / raw)
To: Thorsten Knabe; +Cc: linux-kernel, Thomas Meyer, Valdis.Kletnieks
Am 25.08.2014 01:02, schrieb Thorsten Knabe:
> On 08/24/2014 02:11 PM, Richard Weinberger wrote:
>> Am 23.08.2014 19:43, schrieb Thorsten Knabe:
>>> Hi Richard.
>>>
>>> On 08/23/2014 05:34 PM, Richard Weinberger wrote:
>>>> Hi!
>>>>
>>>> Am 23.08.2014 15:47, schrieb Thorsten Knabe:
>>>>> From: Thorsten Knabe <linux@thorsten-knabe.de>
>>>>>
>>>>> UML: UBD: Fix for processes stuck in D state forever in UserModeLinux.
>>>>>
>>>>> Starting with Linux 3.12 processes get stuck in D state forever in
>>>>> UserModeLinux under sync heavy workloads. This bug was introduced by
>>>>> commit 805f11a0d5 (um: ubd: Add REQ_FLUSH suppport).
>>>>> Fix bug by adding a check if FLUSH request was successfully submitted to
>>>>> the I/O thread and keeping the FLUSH request on the request queue on
>>>>> submission failures.
>>>>>
>>>>> Fixes: 805f11a0d5 (um: ubd: Add REQ_FLUSH suppport)
>>>>> Signed-off-by: Thorsten Knabe <linux@thorsten-knabe.de>
>>>>
>>>> Thanks a lot for hunting this issue down.
>>>>
>>>>> ---
>>>>> Patch applies to 3.16.1.
>>>>>
>>>>> diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
>>>>> index 3716e69..b7d2840 100644
>>>>> --- a/arch/um/drivers/ubd_kern.c
>>>>> +++ b/arch/um/drivers/ubd_kern.c
>>>>> @@ -1277,7 +1277,7 @@ static void do_ubd_request(struct request_queue *q)
>>>>>
>>>>> while(1){
>>>>> struct ubd *dev = q->queuedata;
>>>>> - if(dev->end_sg == 0){
>>>>> + if(dev->request == NULL){
>>>>
>>>> Why do we need this specific change?
>>>
>>> This change is required, because for FLUSH requests dev->end_sg is
>>> initialized to 0 by blk_rq_map_sg() a few lines above, as FLUSH requests
>>> have no data blocks attached to themselves.
>>
>> You meant "below"? Looks like I really miss something here.
>> At the bottom of the while(1) loop we have
>> dev->end_sg = 0;
>> dev->request = NULL;
>
> No. The problematic line is:
> dev->end_sg = blk_rq_map_sg(q, req, dev->sg);
> and blk_rq_map_sg() returning 0 for REQ_FLUSH requests, because they
> have no associated data blocks.
>
> Hence on the next iteration of the while(1) loop:
> if(dev->end_sg == 0){
At the bottom of the while loop dev->end_sg will be set to 0 anyway, this is what puzzles
me so hard.
(And dev->request too)
Thanks,
//richard
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] UML: UBD: Fix for processes stuck in D state forever in UserModeLinux
2014-08-25 13:25 ` Richard Weinberger
@ 2014-08-25 14:00 ` Thorsten Knabe
2014-08-25 14:04 ` Richard Weinberger
0 siblings, 1 reply; 9+ messages in thread
From: Thorsten Knabe @ 2014-08-25 14:00 UTC (permalink / raw)
To: Richard Weinberger; +Cc: linux-kernel, Thomas Meyer, Valdis.Kletnieks
On 08/25/2014 03:25 PM, Richard Weinberger wrote:
> Am 25.08.2014 01:02, schrieb Thorsten Knabe:
>> On 08/24/2014 02:11 PM, Richard Weinberger wrote:
>>> Am 23.08.2014 19:43, schrieb Thorsten Knabe:
>>>> Hi Richard.
>>>>
>>>> On 08/23/2014 05:34 PM, Richard Weinberger wrote:
>>>>> Hi!
>>>>>
>>>>> Am 23.08.2014 15:47, schrieb Thorsten Knabe:
>>>>>> From: Thorsten Knabe <linux@thorsten-knabe.de>
>>>>>>
>>>>>> UML: UBD: Fix for processes stuck in D state forever in UserModeLinux.
>>>>>>
>>>>>> Starting with Linux 3.12 processes get stuck in D state forever in
>>>>>> UserModeLinux under sync heavy workloads. This bug was introduced by
>>>>>> commit 805f11a0d5 (um: ubd: Add REQ_FLUSH suppport).
>>>>>> Fix bug by adding a check if FLUSH request was successfully submitted to
>>>>>> the I/O thread and keeping the FLUSH request on the request queue on
>>>>>> submission failures.
>>>>>>
>>>>>> Fixes: 805f11a0d5 (um: ubd: Add REQ_FLUSH suppport)
>>>>>> Signed-off-by: Thorsten Knabe <linux@thorsten-knabe.de>
>>>>>
>>>>> Thanks a lot for hunting this issue down.
>>>>>
>>>>>> ---
>>>>>> Patch applies to 3.16.1.
>>>>>>
>>>>>> diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
>>>>>> index 3716e69..b7d2840 100644
>>>>>> --- a/arch/um/drivers/ubd_kern.c
>>>>>> +++ b/arch/um/drivers/ubd_kern.c
>>>>>> @@ -1277,7 +1277,7 @@ static void do_ubd_request(struct request_queue *q)
>>>>>>
>>>>>> while(1){
>>>>>> struct ubd *dev = q->queuedata;
>>>>>> - if(dev->end_sg == 0){
>>>>>> + if(dev->request == NULL){
>>>>>
>>>>> Why do we need this specific change?
>>>>
>>>> This change is required, because for FLUSH requests dev->end_sg is
>>>> initialized to 0 by blk_rq_map_sg() a few lines above, as FLUSH requests
>>>> have no data blocks attached to themselves.
>>>
>>> You meant "below"? Looks like I really miss something here.
>>> At the bottom of the while(1) loop we have
>>> dev->end_sg = 0;
>>> dev->request = NULL;
>>
>> No. The problematic line is:
>> dev->end_sg = blk_rq_map_sg(q, req, dev->sg);
>> and blk_rq_map_sg() returning 0 for REQ_FLUSH requests, because they
>> have no associated data blocks.
>>
>> Hence on the next iteration of the while(1) loop:
>> if(dev->end_sg == 0){
>
> At the bottom of the while loop dev->end_sg will be set to 0 anyway, this is what puzzles
> me so hard.
> (And dev->request too)
Yes, but the statements:
dev->end_sg = 0;
dev->request = NULL;
at the end of the while loop will only be reached (for FLUSH requests
only with my patch applied) when submit_request() succeeds. Otherwise
the function do_ubd_request() returns early leaving dev->end_sg and
dev->request untouched.
>
> Thanks,
> //richard
>
--
___
| | / E-Mail: linux@thorsten-knabe.de
|horsten |/\nabe WWW: http://linux.thorsten-knabe.de
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] UML: UBD: Fix for processes stuck in D state forever in UserModeLinux
2014-08-25 14:00 ` Thorsten Knabe
@ 2014-08-25 14:04 ` Richard Weinberger
2014-09-16 16:50 ` Richard Weinberger
0 siblings, 1 reply; 9+ messages in thread
From: Richard Weinberger @ 2014-08-25 14:04 UTC (permalink / raw)
To: Thorsten Knabe; +Cc: linux-kernel, Thomas Meyer, Valdis.Kletnieks
Am 25.08.2014 16:00, schrieb Thorsten Knabe:
> On 08/25/2014 03:25 PM, Richard Weinberger wrote:
>> Am 25.08.2014 01:02, schrieb Thorsten Knabe:
>>> On 08/24/2014 02:11 PM, Richard Weinberger wrote:
>>>> Am 23.08.2014 19:43, schrieb Thorsten Knabe:
>>>>> Hi Richard.
>>>>>
>>>>> On 08/23/2014 05:34 PM, Richard Weinberger wrote:
>>>>>> Hi!
>>>>>>
>>>>>> Am 23.08.2014 15:47, schrieb Thorsten Knabe:
>>>>>>> From: Thorsten Knabe <linux@thorsten-knabe.de>
>>>>>>>
>>>>>>> UML: UBD: Fix for processes stuck in D state forever in UserModeLinux.
>>>>>>>
>>>>>>> Starting with Linux 3.12 processes get stuck in D state forever in
>>>>>>> UserModeLinux under sync heavy workloads. This bug was introduced by
>>>>>>> commit 805f11a0d5 (um: ubd: Add REQ_FLUSH suppport).
>>>>>>> Fix bug by adding a check if FLUSH request was successfully submitted to
>>>>>>> the I/O thread and keeping the FLUSH request on the request queue on
>>>>>>> submission failures.
>>>>>>>
>>>>>>> Fixes: 805f11a0d5 (um: ubd: Add REQ_FLUSH suppport)
>>>>>>> Signed-off-by: Thorsten Knabe <linux@thorsten-knabe.de>
>>>>>>
>>>>>> Thanks a lot for hunting this issue down.
>>>>>>
>>>>>>> ---
>>>>>>> Patch applies to 3.16.1.
>>>>>>>
>>>>>>> diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
>>>>>>> index 3716e69..b7d2840 100644
>>>>>>> --- a/arch/um/drivers/ubd_kern.c
>>>>>>> +++ b/arch/um/drivers/ubd_kern.c
>>>>>>> @@ -1277,7 +1277,7 @@ static void do_ubd_request(struct request_queue *q)
>>>>>>>
>>>>>>> while(1){
>>>>>>> struct ubd *dev = q->queuedata;
>>>>>>> - if(dev->end_sg == 0){
>>>>>>> + if(dev->request == NULL){
>>>>>>
>>>>>> Why do we need this specific change?
>>>>>
>>>>> This change is required, because for FLUSH requests dev->end_sg is
>>>>> initialized to 0 by blk_rq_map_sg() a few lines above, as FLUSH requests
>>>>> have no data blocks attached to themselves.
>>>>
>>>> You meant "below"? Looks like I really miss something here.
>>>> At the bottom of the while(1) loop we have
>>>> dev->end_sg = 0;
>>>> dev->request = NULL;
>>>
>>> No. The problematic line is:
>>> dev->end_sg = blk_rq_map_sg(q, req, dev->sg);
>>> and blk_rq_map_sg() returning 0 for REQ_FLUSH requests, because they
>>> have no associated data blocks.
>>>
>>> Hence on the next iteration of the while(1) loop:
>>> if(dev->end_sg == 0){
>>
>> At the bottom of the while loop dev->end_sg will be set to 0 anyway, this is what puzzles
>> me so hard.
>> (And dev->request too)
>
> Yes, but the statements:
> dev->end_sg = 0;
> dev->request = NULL;
> at the end of the while loop will only be reached (for FLUSH requests
> only with my patch applied) when submit_request() succeeds. Otherwise
> the function do_ubd_request() returns early leaving dev->end_sg and
> dev->request untouched.
Can you please add this info into the commit message, I'm sure others get also
confused by that.^^
The logic in do_ubd_request() royal mess which needs fixing too. :-\
Thanks,
//richard
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] UML: UBD: Fix for processes stuck in D state forever in UserModeLinux
2014-08-25 14:04 ` Richard Weinberger
@ 2014-09-16 16:50 ` Richard Weinberger
0 siblings, 0 replies; 9+ messages in thread
From: Richard Weinberger @ 2014-09-16 16:50 UTC (permalink / raw)
To: Richard Weinberger; +Cc: Thorsten Knabe, LKML, Thomas Meyer, Valdis Kletnieks
On Mon, Aug 25, 2014 at 4:04 PM, Richard Weinberger <richard@nod.at> wrote:
> Am 25.08.2014 16:00, schrieb Thorsten Knabe:
>> On 08/25/2014 03:25 PM, Richard Weinberger wrote:
>>> Am 25.08.2014 01:02, schrieb Thorsten Knabe:
>>>> On 08/24/2014 02:11 PM, Richard Weinberger wrote:
>>>>> Am 23.08.2014 19:43, schrieb Thorsten Knabe:
>>>>>> Hi Richard.
>>>>>>
>>>>>> On 08/23/2014 05:34 PM, Richard Weinberger wrote:
>>>>>>> Hi!
>>>>>>>
>>>>>>> Am 23.08.2014 15:47, schrieb Thorsten Knabe:
>>>>>>>> From: Thorsten Knabe <linux@thorsten-knabe.de>
>>>>>>>>
>>>>>>>> UML: UBD: Fix for processes stuck in D state forever in UserModeLinux.
>>>>>>>>
>>>>>>>> Starting with Linux 3.12 processes get stuck in D state forever in
>>>>>>>> UserModeLinux under sync heavy workloads. This bug was introduced by
>>>>>>>> commit 805f11a0d5 (um: ubd: Add REQ_FLUSH suppport).
>>>>>>>> Fix bug by adding a check if FLUSH request was successfully submitted to
>>>>>>>> the I/O thread and keeping the FLUSH request on the request queue on
>>>>>>>> submission failures.
>>>>>>>>
>>>>>>>> Fixes: 805f11a0d5 (um: ubd: Add REQ_FLUSH suppport)
>>>>>>>> Signed-off-by: Thorsten Knabe <linux@thorsten-knabe.de>
>>>>>>>
>>>>>>> Thanks a lot for hunting this issue down.
>>>>>>>
>>>>>>>> ---
>>>>>>>> Patch applies to 3.16.1.
>>>>>>>>
>>>>>>>> diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c
>>>>>>>> index 3716e69..b7d2840 100644
>>>>>>>> --- a/arch/um/drivers/ubd_kern.c
>>>>>>>> +++ b/arch/um/drivers/ubd_kern.c
>>>>>>>> @@ -1277,7 +1277,7 @@ static void do_ubd_request(struct request_queue *q)
>>>>>>>>
>>>>>>>> while(1){
>>>>>>>> struct ubd *dev = q->queuedata;
>>>>>>>> - if(dev->end_sg == 0){
>>>>>>>> + if(dev->request == NULL){
>>>>>>>
>>>>>>> Why do we need this specific change?
>>>>>>
>>>>>> This change is required, because for FLUSH requests dev->end_sg is
>>>>>> initialized to 0 by blk_rq_map_sg() a few lines above, as FLUSH requests
>>>>>> have no data blocks attached to themselves.
>>>>>
>>>>> You meant "below"? Looks like I really miss something here.
>>>>> At the bottom of the while(1) loop we have
>>>>> dev->end_sg = 0;
>>>>> dev->request = NULL;
>>>>
>>>> No. The problematic line is:
>>>> dev->end_sg = blk_rq_map_sg(q, req, dev->sg);
>>>> and blk_rq_map_sg() returning 0 for REQ_FLUSH requests, because they
>>>> have no associated data blocks.
>>>>
>>>> Hence on the next iteration of the while(1) loop:
>>>> if(dev->end_sg == 0){
>>>
>>> At the bottom of the while loop dev->end_sg will be set to 0 anyway, this is what puzzles
>>> me so hard.
>>> (And dev->request too)
>>
>> Yes, but the statements:
>> dev->end_sg = 0;
>> dev->request = NULL;
>> at the end of the while loop will only be reached (for FLUSH requests
>> only with my patch applied) when submit_request() succeeds. Otherwise
>> the function do_ubd_request() returns early leaving dev->end_sg and
>> dev->request untouched.
>
> Can you please add this info into the commit message, I'm sure others get also
> confused by that.^^
> The logic in do_ubd_request() royal mess which needs fixing too. :-\
Hmm, you didn't resend the patch.
I'll amend the commit message myself and push it to Linus this week.
--
Thanks,
//richard
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-09-16 16:50 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-23 13:47 [PATCH] UML: UBD: Fix for processes stuck in D state forever in UserModeLinux Thorsten Knabe
2014-08-23 15:34 ` Richard Weinberger
2014-08-23 17:43 ` Thorsten Knabe
2014-08-24 12:11 ` Richard Weinberger
2014-08-24 23:02 ` Thorsten Knabe
2014-08-25 13:25 ` Richard Weinberger
2014-08-25 14:00 ` Thorsten Knabe
2014-08-25 14:04 ` Richard Weinberger
2014-09-16 16:50 ` Richard Weinberger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.