* [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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox