Re: [PATCH] jffs2: Do not assume erase will fail

[David Woodhouse <dwmw2@infradead.org>]

On Thu, 2010-10-07 at 18:29 +0200, Joakim Tjernlund wrote:
> 
> Test if it did and then abort.
> 
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
> ---
>  fs/jffs2/nodemgmt.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/jffs2/nodemgmt.c b/fs/jffs2/nodemgmt.c
> index 694aa5b..49ee5de 100644
> --- a/fs/jffs2/nodemgmt.c
> +++ b/fs/jffs2/nodemgmt.c
> @@ -260,9 +260,9 @@ static int jffs2_find_nextblock(struct jffs2_sb_info *c)
>                 spin_lock(&c->erase_completion_lock);
>  
>                 /* An erase may have failed, decreasing the
> -                  amount of free space available. So we must
> -                  restart from the beginning */
> -               return -EAGAIN;
> +                  amount of free space available. */
> +               if (list_empty(&c->free_list))
> +                       return -EAGAIN; /* restart from the beginning */ 

Hm, but there could have been more than one erase pending (or in
progress). And if one fails and another succeeds then you could have a
non-empty free_list but you could *also* now have run short of
free/freeable space so that a userspace write should now receive
-ENOSPC.

Is this really a performance issue? It should just come straight back if
the conditions are still met, surely?

And if we're hitting this code path that often, we should look at
erasing more aggressively so that we *don't* have to erase stuff on
demand.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation