| Message ID | 1283357021-16187-1-git-send-email-w.sang@pengutronix.de |
|---|---|
| State | RFC |
| Headers | show |
This 'yield()' was introduced by dwmw2, so he may have strong feelings about it. CCed. On Wed, 2010-09-01 at 18:03 +0200, Wolfram Sang wrote: > yield() has different semantics meanwhile and even causes RT-kernels to > BUG. Replace the only appearance left in jffs2. > > Signed-off-by: Wolfram Sang <w.sang@pengutronix.de> > Cc: Artem Bityutskiy <Artem.Bityutskiy@nokia.com> > Cc: Ingo Molnar <mingo@elte.hu> > --- > > The aforementioned BUG() showed up in one of our customer's RT-projects. While > this could be handled by rearranging his thread-priorities, I wondered if such > a patch would be worthwhile, still. Reading through the material covering > yield() and related replacements, I believe this patch should be appropriate. > Please say if I missed some side-effects. > > fs/jffs2/erase.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c > index abac961..e513f19 100644 > --- a/fs/jffs2/erase.c > +++ b/fs/jffs2/erase.c > @@ -151,7 +151,7 @@ int jffs2_erase_pending_blocks(struct jffs2_sb_info *c, int count) > } > > /* Be nice */ > - yield(); > + cond_resched(); > mutex_lock(&c->erase_free_sem); > spin_lock(&c->erase_completion_lock); > }
> > This 'yield()' was introduced by dwmw2, so he may have strong feelings > about it. CCed. Actually, it was introduced by me with commit fd5324909e410a3202c1b01bd507c2dfba58fca5 : [JFFS2] Fix hanging close for /dev/mtd character device. When pdflush is erasing lots of sectors, drivers calling mtd->sync will hang until all blocks are erased. Be nicer. Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> Signed-off-by: David Woodhouse <dwmw2@infradead.org> @@ -142,7 +142,7 @@ void jffs2_erase_pending_blocks(struct jffs2_sb_info *c, int count) } /* Be nice */ - cond_resched(); + yield(); spin_lock(&c->erase_completion_lock); } At the time cond_resched() was a problem but now I don't know. The erasing has moved to the GC thread and been somewhat reworked. I don't know if it still needs yield(). Perhaps David can tell? Jocke > > On Wed, 2010-09-01 at 18:03 +0200, Wolfram Sang wrote: > > yield() has different semantics meanwhile and even causes RT-kernels to > > BUG. Replace the only appearance left in jffs2. > > > > Signed-off-by: Wolfram Sang <w.sang@pengutronix.de> > > Cc: Artem Bityutskiy <Artem.Bityutskiy@nokia.com> > > Cc: Ingo Molnar <mingo@elte.hu> > > --- > > > > The aforementioned BUG() showed up in one of our customer's RT-projects. While > > this could be handled by rearranging his thread-priorities, I wondered if such > > a patch would be worthwhile, still. Reading through the material covering > > yield() and related replacements, I believe this patch should be appropriate. > > Please say if I missed some side-effects. > > > > fs/jffs2/erase.c | 2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c > > index abac961..e513f19 100644 > > --- a/fs/jffs2/erase.c > > +++ b/fs/jffs2/erase.c > > @@ -151,7 +151,7 @@ int jffs2_erase_pending_blocks(struct jffs2_sb_info *c, int count) > > } > > > > /* Be nice */ > > - yield(); > > + cond_resched(); > > mutex_lock(&c->erase_free_sem); > > spin_lock(&c->erase_completion_lock); > > } > > -- > Best Regards, > Artem Bityutskiy (Битюцкий Артём) > > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/
On Thu, 2010-09-02 at 09:01 +0200, Joakim Tjernlund wrote: > At the time cond_resched() was a problem but now I don't know. The erasing > has moved to the GC thread and been somewhat reworked. I don't know if > it still needs yield(). Perhaps David can tell? I think 'yield()' is rather a hack than a real solution.
Artem Bityutskiy <dedekind1@gmail.com> wrote on 2010/09/02 12:59:09: > > On Thu, 2010-09-02 at 09:01 +0200, Joakim Tjernlund wrote: > > At the time cond_resched() was a problem but now I don't know. The erasing > > has moved to the GC thread and been somewhat reworked. I don't know if > > it still needs yield(). Perhaps David can tell? > > I think 'yield()' is rather a hack than a real solution. Yes, the real fix is to move erasing to GC and that has been done now. There is a good chance yield() can be removed without regression but I am not sure nor do I have the time to test that. Jocke
On Wed, 2010-09-01 at 18:03 +0200, Wolfram Sang wrote: > yield() has different semantics meanwhile and even causes RT-kernels to > BUG. Replace the only appearance left in jffs2. > > Signed-off-by: Wolfram Sang <w.sang@pengutronix.de> > Cc: Artem Bityutskiy <Artem.Bityutskiy@nokia.com> > Cc: Ingo Molnar <mingo@elte.hu> FYI, I've pushed this patch to my l2-mtd-2.6.git tree, thanks.
diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c index abac961..e513f19 100644 --- a/fs/jffs2/erase.c +++ b/fs/jffs2/erase.c @@ -151,7 +151,7 @@ int jffs2_erase_pending_blocks(struct jffs2_sb_info *c, int count) } /* Be nice */ - yield(); + cond_resched(); mutex_lock(&c->erase_free_sem); spin_lock(&c->erase_completion_lock); }
yield() has different semantics meanwhile and even causes RT-kernels to BUG. Replace the only appearance left in jffs2. Signed-off-by: Wolfram Sang <w.sang@pengutronix.de> Cc: Artem Bityutskiy <Artem.Bityutskiy@nokia.com> Cc: Ingo Molnar <mingo@elte.hu> --- The aforementioned BUG() showed up in one of our customer's RT-projects. While this could be handled by rearranging his thread-priorities, I wondered if such a patch would be worthwhile, still. Reading through the material covering yield() and related replacements, I believe this patch should be appropriate. Please say if I missed some side-effects. fs/jffs2/erase.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)