All of lore.kernel.org
 help / color / mirror / Atom feed
From: Iwo Mergler <iwo@call-direct.com.au>
To: Anders Larsen <al@alarsen.net>
Cc: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>,
	Ian McDonnell <ian@brightstareng.com>,
	Nicolas Pitre <nico@fluxnic.net>,
	linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org,
	Matthias Kaehlcke <matthias@kaehlcke.net>,
	David Woodhouse <dwmw2@infradead.org>
Subject: Re: [PATCH] Fix Oops with Atmel SPI
Date: Wed, 14 Apr 2010 17:30:41 +1000	[thread overview]
Message-ID: <4BC56F21.6040909@call-direct.com.au> (raw)
In-Reply-To: <1271158315l.25331l.9l@i-dmzi_al.realan.de>

Hi Anders,

I wouldn't recommend that. MTD erase blocks are 64K or more. In a typical
embedded system you will not be able to kmalloc that much memory after
a few day's of operation - the page pool gets fragmented.

A possibly better approach is to arrange for that memory to get allocated
at driver start time.

An even better approach would be to change the algorithm to operate on
a list of smaller allocations, e.g. MTD page size.


Best regards,

Iwo


Anders Larsen wrote:
> Tweak MTD's cache allocation to make it work with the atmel DMA'ed SPI.
> Substitute kmalloc for vmalloc so the cache buffer is mappable as per
> the Atmel SPI driver's requirements, otherwise an Oops would occur.
> 
> The original patch by Ian McDonnell <ian@brightstareng.com> was found here:
> http://lists.infradead.org/pipermail/linux-mtd/2007-December/020184.html
> 
> Signed-off-by: Anders Larsen <al@alarsen.net>
> Cc: Ian McDonnell <ian@brightstareng.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Matthias Kaehlcke <matthias@kaehlcke.net>
> Cc: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> Cc: Nicolas Pitre <nico@fluxnic.net>
> ---
>  drivers/mtd/mtdblock.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> Index: b/drivers/mtd/mtdblock.c
> ===================================================================
> --- a/drivers/mtd/mtdblock.c
> +++ b/drivers/mtd/mtdblock.c
> @@ -253,7 +253,11 @@ static int mtdblock_writesect(struct mtd
>  {
>  	struct mtdblk_dev *mtdblk = mtdblks[dev->devnum];
>  	if (unlikely(!mtdblk->cache_data && mtdblk->cache_size)) {
> +#ifdef CONFIG_SPI_ATMEL
> +		mtdblk->cache_data = kmalloc(mtdblk->mtd->erasesize, GFP_KERNEL);
> +#else
>  		mtdblk->cache_data = vmalloc(mtdblk->mtd->erasesize);
> +#endif
>  		if (!mtdblk->cache_data)
>  			return -EINTR;
>  		/* -EINTR is not really correct, but it is the best match
> @@ -322,7 +326,11 @@ static int mtdblock_release(struct mtd_b
>  		mtdblks[dev] = NULL;
>  		if (mtdblk->mtd->sync)
>  			mtdblk->mtd->sync(mtdblk->mtd);
> +#ifdef CONFIG_SPI_ATMEL
> +		kfree(mtdblk->cache_data);
> +#else
>  		vfree(mtdblk->cache_data);
> +#endif
>  		kfree(mtdblk);
>  	}
>  
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 
> 

WARNING: multiple messages have this Message-ID (diff)
From: Iwo Mergler <iwo@call-direct.com.au>
To: Anders Larsen <al@alarsen.net>
Cc: linux-mtd@lists.infradead.org,
	Artem Bityutskiy <Artem.Bityutskiy@nokia.com>,
	Ian McDonnell <ian@brightstareng.com>,
	Nicolas Pitre <nico@fluxnic.net>,
	linux-kernel@vger.kernel.org,
	Matthias Kaehlcke <matthias@kaehlcke.net>,
	David Woodhouse <dwmw2@infradead.org>
Subject: Re: [PATCH] Fix Oops with Atmel SPI
Date: Wed, 14 Apr 2010 17:30:41 +1000	[thread overview]
Message-ID: <4BC56F21.6040909@call-direct.com.au> (raw)
In-Reply-To: <1271158315l.25331l.9l@i-dmzi_al.realan.de>

Hi Anders,

I wouldn't recommend that. MTD erase blocks are 64K or more. In a typical
embedded system you will not be able to kmalloc that much memory after
a few day's of operation - the page pool gets fragmented.

A possibly better approach is to arrange for that memory to get allocated
at driver start time.

An even better approach would be to change the algorithm to operate on
a list of smaller allocations, e.g. MTD page size.


Best regards,

Iwo


Anders Larsen wrote:
> Tweak MTD's cache allocation to make it work with the atmel DMA'ed SPI.
> Substitute kmalloc for vmalloc so the cache buffer is mappable as per
> the Atmel SPI driver's requirements, otherwise an Oops would occur.
> 
> The original patch by Ian McDonnell <ian@brightstareng.com> was found here:
> http://lists.infradead.org/pipermail/linux-mtd/2007-December/020184.html
> 
> Signed-off-by: Anders Larsen <al@alarsen.net>
> Cc: Ian McDonnell <ian@brightstareng.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Matthias Kaehlcke <matthias@kaehlcke.net>
> Cc: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> Cc: Nicolas Pitre <nico@fluxnic.net>
> ---
>  drivers/mtd/mtdblock.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> Index: b/drivers/mtd/mtdblock.c
> ===================================================================
> --- a/drivers/mtd/mtdblock.c
> +++ b/drivers/mtd/mtdblock.c
> @@ -253,7 +253,11 @@ static int mtdblock_writesect(struct mtd
>  {
>  	struct mtdblk_dev *mtdblk = mtdblks[dev->devnum];
>  	if (unlikely(!mtdblk->cache_data && mtdblk->cache_size)) {
> +#ifdef CONFIG_SPI_ATMEL
> +		mtdblk->cache_data = kmalloc(mtdblk->mtd->erasesize, GFP_KERNEL);
> +#else
>  		mtdblk->cache_data = vmalloc(mtdblk->mtd->erasesize);
> +#endif
>  		if (!mtdblk->cache_data)
>  			return -EINTR;
>  		/* -EINTR is not really correct, but it is the best match
> @@ -322,7 +326,11 @@ static int mtdblock_release(struct mtd_b
>  		mtdblks[dev] = NULL;
>  		if (mtdblk->mtd->sync)
>  			mtdblk->mtd->sync(mtdblk->mtd);
> +#ifdef CONFIG_SPI_ATMEL
> +		kfree(mtdblk->cache_data);
> +#else
>  		vfree(mtdblk->cache_data);
> +#endif
>  		kfree(mtdblk);
>  	}
>  
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 
> 



  reply	other threads:[~2010-04-14  7:30 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-13 11:31 [PATCH] Fix Oops with Atmel SPI Anders Larsen
2010-04-13 11:31 ` Anders Larsen
2010-04-14  7:30 ` Iwo Mergler [this message]
2010-04-14  7:30   ` Iwo Mergler
2010-04-14  7:57   ` Anders Larsen
2010-04-14  7:57     ` Anders Larsen
2010-04-14 18:13     ` Kevin Cernekee
2010-04-14 18:13       ` Kevin Cernekee
2010-04-15  7:32     ` Iwo Mergler
2010-04-21 22:24     ` Andrew Morton
2010-04-21 22:24       ` Andrew Morton
2010-05-19 11:05       ` Anders Larsen
2010-05-19 11:05         ` Anders Larsen
2010-05-21 19:01         ` Andrew Morton
2010-05-21 19:01           ` Andrew Morton
2010-05-24 15:09           ` Ian McDonnell
2010-05-24 15:09             ` Ian McDonnell
2010-05-28  9:27           ` Haavard Skinnemoen
2010-05-28  9:27             ` Haavard Skinnemoen
2010-04-27 12:57 ` Artem Bityutskiy
2010-04-27 12:57   ` Artem Bityutskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4BC56F21.6040909@call-direct.com.au \
    --to=iwo@call-direct.com.au \
    --cc=Artem.Bityutskiy@nokia.com \
    --cc=al@alarsen.net \
    --cc=dwmw2@infradead.org \
    --cc=ian@brightstareng.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=matthias@kaehlcke.net \
    --cc=nico@fluxnic.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.