[PATCH][JFFS2] Reschedule during scan of partly empty eraseblock

[PATCH][JFFS2] Reschedule during scan of partly empty eraseblock

[Nielsen David Marqvar]

This patch adds rescheduling to the loop that scans an empty area of an
eraseblock.

Rationale:
When scanning the an empty area of an eraseblock no scheduling occurs. But
this scan may take significantly long to complete depending on the size of 
the empty space within the eraseblock and the speed of the flash (on my
system
it could theoretically be up to 100 ms).
The patch will add a call to cond_resched() for each 2 KiB scanned (while the
first check may be done before 2 KiB the code is simple and fast).

The patch has been tested and running on our systems for a month now.


Signed-off-by: David Marqvar Nielsen <david.nielsen@frequentis.com>
---
diff --git a/fs/jffs2/scan.c b/fs/jffs2/scan.c
index 1d437de..044a4a1 100644
--- a/fs/jffs2/scan.c
+++ b/fs/jffs2/scan.c
@@ -654,6 +654,10 @@ scan_more:
                                        goto scan_more;
                                }

+                               /* check for resched - searching an
eraseblock may take several ms */
+                               if ( (ofs & 0x7ff /* for each 2KiB */) == 0)
+                                       cond_resched();
+
                                inbuf_ofs+=4;
                                ofs += 4;
                        }

Re: [PATCH][JFFS2] Reschedule during scan of partly empty eraseblock

[Artem Bityutskiy]

On Thu, 2008-05-29 at 11:29 +0200, Nielsen David Marqvar wrote:
> This patch adds rescheduling to the loop that scans an empty area of an
> eraseblock.
> 
> Rationale:
> When scanning the an empty area of an eraseblock no scheduling occurs. But
> this scan may take significantly long to complete depending on the size of 
> the empty space within the eraseblock and the speed of the flash (on my
> system
> it could theoretically be up to 100 ms).
> The patch will add a call to cond_resched() for each 2 KiB scanned (while the
> first check may be done before 2 KiB the code is simple and fast).
> 
> The patch has been tested and running on our systems for a month now.
...
>                                 }
> 
> +                               /* check for resched - searching an
> eraseblock may take several ms */
> +                               if ( (ofs & 0x7ff /* for each 2KiB */) == 0)
> +                                       cond_resched();
> +
>                                 inbuf_ofs+=4;
>                                 ofs += 4;
>                         }

I would rather add cond_resched() to jffs2_fill_scan_buf()
unconditionally. Conditional cond_resched() really looks bad.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

RE: [PATCH][JFFS2] Reschedule during scan of partly emptyeraseblock

[Nielsen David Marqvar]

On Thu, 2008-05-29 at 11:47 +0200, Artem Bityutskiy wrote:
> > This patch adds rescheduling to the loop that scans an empty area of
> > an eraseblock.
> > + /* check for resched - searching an eraseblock may take several ms */
> > + if ( (ofs & 0x7ff /* for each 2KiB */) == 0)
> > + 	cond_resched();
> > +
> 
> I would rather add cond_resched() to jffs2_fill_scan_buf()
> unconditionally. Conditional cond_resched() really looks bad.

I don't see how adding cond_resched() to jffs2_fill_scan_buf() will
remove the long delay imposed by the while() loop that scans an empty
area of an eraseblock - the loop (placed just after the label
"more_empty:") will scan from ofs until scan_end unless it finds a
non-erased area.
There's an outer while loop around this one and the outer loop 
already contains a cond_resched(), so it's not a new thing that
cond_resched() may be needed during the scan.

If I misunderstood what you wrote, please try again :)


--
Br,
David Marqvar Nielsen

RE: [PATCH][JFFS2] Reschedule during scan of partly emptyeraseblock

[Artem Bityutskiy]

On Thu, 2008-05-29 at 13:40 +0200, Nielsen David Marqvar wrote:
> On Thu, 2008-05-29 at 11:47 +0200, Artem Bityutskiy wrote:
> > > This patch adds rescheduling to the loop that scans an empty area of
> > > an eraseblock.
> > > + /* check for resched - searching an eraseblock may take several ms */
> > > + if ( (ofs & 0x7ff /* for each 2KiB */) == 0)
> > > + 	cond_resched();
> > > +
> > 
> > I would rather add cond_resched() to jffs2_fill_scan_buf()
> > unconditionally. Conditional cond_resched() really looks bad.
> 
> I don't see how adding cond_resched() to jffs2_fill_scan_buf() will
> remove the long delay imposed by the while() loop that scans an empty
> area of an eraseblock - the loop (placed just after the label
> "more_empty:") will scan from ofs until scan_end unless it finds a
> non-erased area.
> There's an outer while loop around this one and the outer loop 
> already contains a cond_resched(), so it's not a new thing that
> cond_resched() may be needed during the scan.

Ok, I see. 

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)