[PATCH] [CFI] Prevent erase command invocation on suspended chip

[PATCH] [CFI] Prevent erase command invocation on suspended chip

[Alexander Belyakov]

Hello,

while running stress tests we have met cfi_cmdset_0001.c driver issue.
Working on multipartitional devices with erase suspend on write
feature enabled it is possible to get erase operation invoked on chip
with suspended erase. get_chip() looses information about earlier
suspended erase and new erase operation gets issued. New erase
operations report successful completion, but blocks remain dirty
causing, for example, JFFS2 error messages like:

...
Newly-erased block contained word 0x20031985 at offset 0x00200000
Newly-erased block contained word 0x20031985 at offset 0x00280000
Newly-erased block contained word 0x20031985 at offset 0x00240000
...


The patch below fixes that issue.

Signed-off-by: Alexander Belyakov <alexander.belyakov@intel.com>

diff -uNr a/drivers/mtd/chips/cfi_cmdset_0001.c
b/drivers/mtd/chips/cfi_cmdset_0001.c
--- a/drivers/mtd/chips/cfi_cmdset_0001.c	2007-11-06 11:40:24.000000000 +0300
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c	2007-11-06 20:37:03.000000000 +0300
@@ -795,6 +795,7 @@
 static int get_chip(struct map_info *map, struct flchip *chip,
unsigned long adr, int mode)
 {
 	int ret;
+	DECLARE_WAITQUEUE(wait, current);

  retry:
 	if (chip->priv && (mode == FL_WRITING || mode == FL_ERASING
@@ -851,6 +852,20 @@
 			spin_unlock(contender->mutex);
 		}

+		/* Check if we already have suspended erase
+		 * on this chip. Sleep. */
+		if (mode == FL_ERASING && shared->erasing
+		    && shared->erasing->oldstate == FL_ERASING) {
+			spin_unlock(&shared->lock);
+			set_current_state(TASK_UNINTERRUPTIBLE);
+			add_wait_queue(&chip->wq, &wait);
+			spin_unlock(chip->mutex);
+			schedule();
+			remove_wait_queue(&chip->wq, &wait);
+			spin_lock(chip->mutex);
+			goto retry;
+		}
+
 		/* We now own it */
 		shared->writing = chip;
 		if (mode == FL_ERASING)

Re: [PATCH] [CFI] Prevent erase command invocation on suspended chip

[Nicolas Pitre]

On Wed, 7 Nov 2007, Alexander Belyakov wrote:

> Hello,
> 
> while running stress tests we have met cfi_cmdset_0001.c driver issue.
> Working on multipartitional devices with erase suspend on write
> feature enabled it is possible to get erase operation invoked on chip
> with suspended erase. get_chip() looses information about earlier
> suspended erase and new erase operation gets issued. New erase
> operations report successful completion, but blocks remain dirty
> causing, for example, JFFS2 error messages like:
> 
> ...
> Newly-erased block contained word 0x20031985 at offset 0x00200000
> Newly-erased block contained word 0x20031985 at offset 0x00280000
> Newly-erased block contained word 0x20031985 at offset 0x00240000
> ...
> 
> 
> The patch below fixes that issue.
> 
> Signed-off-by: Alexander Belyakov <alexander.belyakov@intel.com>

Acked-by: Nicolas Pitre <nico@cam.org>


> 
> diff -uNr a/drivers/mtd/chips/cfi_cmdset_0001.c
> b/drivers/mtd/chips/cfi_cmdset_0001.c
> --- a/drivers/mtd/chips/cfi_cmdset_0001.c	2007-11-06 11:40:24.000000000 +0300
> +++ b/drivers/mtd/chips/cfi_cmdset_0001.c	2007-11-06 20:37:03.000000000 +0300
> @@ -795,6 +795,7 @@
>  static int get_chip(struct map_info *map, struct flchip *chip,
> unsigned long adr, int mode)
>  {
>  	int ret;
> +	DECLARE_WAITQUEUE(wait, current);
> 
>   retry:
>  	if (chip->priv && (mode == FL_WRITING || mode == FL_ERASING
> @@ -851,6 +852,20 @@
>  			spin_unlock(contender->mutex);
>  		}
> 
> +		/* Check if we already have suspended erase
> +		 * on this chip. Sleep. */
> +		if (mode == FL_ERASING && shared->erasing
> +		    && shared->erasing->oldstate == FL_ERASING) {
> +			spin_unlock(&shared->lock);
> +			set_current_state(TASK_UNINTERRUPTIBLE);
> +			add_wait_queue(&chip->wq, &wait);
> +			spin_unlock(chip->mutex);
> +			schedule();
> +			remove_wait_queue(&chip->wq, &wait);
> +			spin_lock(chip->mutex);
> +			goto retry;
> +		}
> +
>  		/* We now own it */
>  		shared->writing = chip;
>  		if (mode == FL_ERASING)
> 


Nicolas

Re: [PATCH] [CFI] Prevent erase command invocation on suspended chip

[Alexander Belyakov]

On Nov 7, 2007 4:44 PM, Nicolas Pitre <nico@cam.org> wrote:
> On Wed, 7 Nov 2007, Alexander Belyakov wrote:
> >
> > The patch below fixes that issue.
> >
> > Signed-off-by: Alexander Belyakov <alexander.belyakov@intel.com>
>
> Acked-by: Nicolas Pitre <nico@cam.org>
>

Hello David,

Please apply this patch.

Thanks,
Alexander

Re: [PATCH] [CFI] Prevent erase command invocation on suspended chip

[Alexander Belyakov]

On Nov 12, 2007 11:11 AM, Alexander Belyakov <abelyako@googlemail.com> wrote:
> On Nov 7, 2007 4:44 PM, Nicolas Pitre <nico@cam.org> wrote:
> > On Wed, 7 Nov 2007, Alexander Belyakov wrote:
> > >
> > > The patch below fixes that issue.
> > >
> > > Signed-off-by: Alexander Belyakov <alexander.belyakov@intel.com>
> >
> > Acked-by: Nicolas Pitre <nico@cam.org>
> >
>
> Hello David,
>
> Please apply this patch.
>

Just a remainder about patch pending. Please apply.

-a

Re: [PATCH] [CFI] Prevent erase command invocation on suspended chip

[David Woodhouse]

On Mon, 2007-11-26 at 18:48 +0300, Alexander Belyakov wrote:
> Just a remainder about patch pending. Please apply.

Try applying it yourself. Make sure you use the copy you received in
your email.

-- 
dwmw2

Re: [PATCH] [CFI] Prevent erase command invocation on suspended chip

[Alexander Belyakov]

On Nov 26, 2007 6:54 PM, David Woodhouse <dwmw2@infradead.org> wrote:
>
> On Mon, 2007-11-26 at 18:48 +0300, Alexander Belyakov wrote:
> > Just a remainder about patch pending. Please apply.
>
> Try applying it yourself. Make sure you use the copy you received in
> your email.
>

Please excuse me for gmail-corrupted patch and thanks for committing
it. I'll try to avoid such a corruption next time.

-a