All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: "Kyungmin Park" <kmpark@infradead.org>
Cc: linux-kernel@vger.kernel.org,
	David Brownell <david-b@pacbell.net>,
	linux-mtd@lists.infradead.org,
	Michael Trimarchi <trimarchimichael@yahoo.it>,
	spi-devel-general@lists.sourceforge.net,
	Josh Boyer <jwboyer@gmail.com>,
	dwmw2@infradead.org, linux-arm-kernel@lists.arm.linux.org.uk
Subject: Re: [PATCH] jffs2 summary allocation
Date: Fri, 4 Apr 2008 18:46:15 -0700	[thread overview]
Message-ID: <20080404184615.deaf3122.akpm@linux-foundation.org> (raw)
In-Reply-To: <9c9fda240804041829r5a768b39n340926485aa12687@mail.gmail.com>

On Sat, 5 Apr 2008 10:29:25 +0900 "Kyungmin Park" <kmpark@infradead.org> wrote:

> On Sat, Apr 5, 2008 at 10:11 AM, Josh Boyer <jwboyer@gmail.com> wrote:
> > On Fri, 2008-04-04 at 16:58 -0700, David Brownell wrote:
> >  > On Friday 04 April 2008, Josh Boyer wrote:
> >  > >
> >  > > >   ... This means specifically that you may _not_ use the
> >  > > >   memory/addresses returned from vmalloc() for DMA.  ...
> >  > > >
> >  > > > So I'm rather surprised to see *ANY* kernel code trying to do
> >  > > > that.  That rule has been in effect for many, many years now.
> >  > >
> >  > > I don't think it was intentional.  You're going through several layers
> >  > > here:
> >  > >
> >  > > JFFS2 -> mtd parts -> mtd dataflash -> atmel_spi.
> >  > >
> >  > > Typically MTD drivers aren't doing DMAs to flash and JFFS2 has no idea
> >  > > which particular chip driver is being used because it's abstracted by
> >  > > MTD.
> >  >
> >  > That's true ... although I can imagine using DMA to
> >  > avoid dcache trashing if its setup cost is low enough,
> >  > with either NAND or NOR chips.
> >  >
> >  > Still:  in this context vmalloc() is wrong.
> >
> >  Agreed.  One issue is that the summary code allocates a buffer that
> >  equals the eraseblock size of the underlying MTD device.  For larger
> >  NAND chips, that may be up to 256KiB.  I believe this is within the
> >  allowable kmalloc size for most architectures these days, but the
> >  summary code is 3 years old and was likely expecting a smaller limit.
> >  And there is always the question on whether finding that much contiguous
> >  memory will be an issue.

Yes.  This is why I'm reluctant to whizz this patch into 2.6.25.  It'll
break more than it fixes.

> In MLC chips it goes up to 512KiB. It means it can't allocate the
> eraseblock size memory with kmalloc().
> In ARM environment I can't see the 256KiB or more memory allocation
> with kmalloc().
> So I now changed the kmalloc eraseblock to vmalloc at both jffs2 and mtd-utils.

Does this eraseblock really really really need to be a single
virtually-contiguous hunk of kernel memory?  Or was that just easy to do at
the time?



This problem comes up pretty often.  Rather than open-coding it yet again
it'd be nice to have a little bit of library code which manages an array of
pages and which has accessors for common operations like
read/write-u8/u16/u32/u64, memset, memcpy, etc.

Then again, given that this memory is often fed into IO subsystems, perhaps
we should do this by adding more accessors and helpers to
scatterlists/sg_table.  Unfortunately they're not presently well set up for
random access.

WARNING: multiple messages have this Message-ID (diff)
From: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
To: "Kyungmin Park" <kmpark-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Michael Trimarchi <trimarchimichael-whZMOeQn8C0@public.gmane.org>,
	spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org,
	Josh Boyer <jwboyer-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org,
	linux-arm-kernel-xIg/pKzrS19vn6HldHNs0ANdhmdF6hFW@public.gmane.org
Subject: Re: [PATCH] jffs2 summary allocation
Date: Fri, 4 Apr 2008 18:46:15 -0700	[thread overview]
Message-ID: <20080404184615.deaf3122.akpm@linux-foundation.org> (raw)
In-Reply-To: <9c9fda240804041829r5a768b39n340926485aa12687-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Sat, 5 Apr 2008 10:29:25 +0900 "Kyungmin Park" <kmpark-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:

> On Sat, Apr 5, 2008 at 10:11 AM, Josh Boyer <jwboyer-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Fri, 2008-04-04 at 16:58 -0700, David Brownell wrote:
> >  > On Friday 04 April 2008, Josh Boyer wrote:
> >  > >
> >  > > >   ... This means specifically that you may _not_ use the
> >  > > >   memory/addresses returned from vmalloc() for DMA.  ...
> >  > > >
> >  > > > So I'm rather surprised to see *ANY* kernel code trying to do
> >  > > > that.  That rule has been in effect for many, many years now.
> >  > >
> >  > > I don't think it was intentional.  You're going through several layers
> >  > > here:
> >  > >
> >  > > JFFS2 -> mtd parts -> mtd dataflash -> atmel_spi.
> >  > >
> >  > > Typically MTD drivers aren't doing DMAs to flash and JFFS2 has no idea
> >  > > which particular chip driver is being used because it's abstracted by
> >  > > MTD.
> >  >
> >  > That's true ... although I can imagine using DMA to
> >  > avoid dcache trashing if its setup cost is low enough,
> >  > with either NAND or NOR chips.
> >  >
> >  > Still:  in this context vmalloc() is wrong.
> >
> >  Agreed.  One issue is that the summary code allocates a buffer that
> >  equals the eraseblock size of the underlying MTD device.  For larger
> >  NAND chips, that may be up to 256KiB.  I believe this is within the
> >  allowable kmalloc size for most architectures these days, but the
> >  summary code is 3 years old and was likely expecting a smaller limit.
> >  And there is always the question on whether finding that much contiguous
> >  memory will be an issue.

Yes.  This is why I'm reluctant to whizz this patch into 2.6.25.  It'll
break more than it fixes.

> In MLC chips it goes up to 512KiB. It means it can't allocate the
> eraseblock size memory with kmalloc().
> In ARM environment I can't see the 256KiB or more memory allocation
> with kmalloc().
> So I now changed the kmalloc eraseblock to vmalloc at both jffs2 and mtd-utils.

Does this eraseblock really really really need to be a single
virtually-contiguous hunk of kernel memory?  Or was that just easy to do at
the time?



This problem comes up pretty often.  Rather than open-coding it yet again
it'd be nice to have a little bit of library code which manages an array of
pages and which has accessors for common operations like
read/write-u8/u16/u32/u64, memset, memcpy, etc.

Then again, given that this memory is often fed into IO subsystems, perhaps
we should do this by adding more accessors and helpers to
scatterlists/sg_table.  Unfortunately they're not presently well set up for
random access.

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

WARNING: multiple messages have this Message-ID (diff)
From: Andrew Morton <akpm@linux-foundation.org>
To: "Kyungmin Park" <kmpark@infradead.org>
Cc: "Josh Boyer" <jwboyer@gmail.com>,
	"David Brownell" <david-b@pacbell.net>,
	linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org,
	"Michael Trimarchi" <trimarchimichael@yahoo.it>,
	spi-devel-general@lists.sourceforge.net, dwmw2@infradead.org,
	linux-arm-kernel@lists.arm.linux.org.uk
Subject: Re: [PATCH] jffs2 summary allocation
Date: Fri, 4 Apr 2008 18:46:15 -0700	[thread overview]
Message-ID: <20080404184615.deaf3122.akpm@linux-foundation.org> (raw)
In-Reply-To: <9c9fda240804041829r5a768b39n340926485aa12687@mail.gmail.com>

On Sat, 5 Apr 2008 10:29:25 +0900 "Kyungmin Park" <kmpark@infradead.org> wrote:

> On Sat, Apr 5, 2008 at 10:11 AM, Josh Boyer <jwboyer@gmail.com> wrote:
> > On Fri, 2008-04-04 at 16:58 -0700, David Brownell wrote:
> >  > On Friday 04 April 2008, Josh Boyer wrote:
> >  > >
> >  > > >   ... This means specifically that you may _not_ use the
> >  > > >   memory/addresses returned from vmalloc() for DMA.  ...
> >  > > >
> >  > > > So I'm rather surprised to see *ANY* kernel code trying to do
> >  > > > that.  That rule has been in effect for many, many years now.
> >  > >
> >  > > I don't think it was intentional.  You're going through several layers
> >  > > here:
> >  > >
> >  > > JFFS2 -> mtd parts -> mtd dataflash -> atmel_spi.
> >  > >
> >  > > Typically MTD drivers aren't doing DMAs to flash and JFFS2 has no idea
> >  > > which particular chip driver is being used because it's abstracted by
> >  > > MTD.
> >  >
> >  > That's true ... although I can imagine using DMA to
> >  > avoid dcache trashing if its setup cost is low enough,
> >  > with either NAND or NOR chips.
> >  >
> >  > Still:  in this context vmalloc() is wrong.
> >
> >  Agreed.  One issue is that the summary code allocates a buffer that
> >  equals the eraseblock size of the underlying MTD device.  For larger
> >  NAND chips, that may be up to 256KiB.  I believe this is within the
> >  allowable kmalloc size for most architectures these days, but the
> >  summary code is 3 years old and was likely expecting a smaller limit.
> >  And there is always the question on whether finding that much contiguous
> >  memory will be an issue.

Yes.  This is why I'm reluctant to whizz this patch into 2.6.25.  It'll
break more than it fixes.

> In MLC chips it goes up to 512KiB. It means it can't allocate the
> eraseblock size memory with kmalloc().
> In ARM environment I can't see the 256KiB or more memory allocation
> with kmalloc().
> So I now changed the kmalloc eraseblock to vmalloc at both jffs2 and mtd-utils.

Does this eraseblock really really really need to be a single
virtually-contiguous hunk of kernel memory?  Or was that just easy to do at
the time?



This problem comes up pretty often.  Rather than open-coding it yet again
it'd be nice to have a little bit of library code which manages an array of
pages and which has accessors for common operations like
read/write-u8/u16/u32/u64, memset, memcpy, etc.

Then again, given that this memory is often fed into IO subsystems, perhaps
we should do this by adding more accessors and helpers to
scatterlists/sg_table.  Unfortunately they're not presently well set up for
random access.

  reply	other threads:[~2008-04-05  1:47 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-04 10:23 [PATCH] jffs2 summary allocation Michael Trimarchi
2008-04-04 10:23 ` Michael Trimarchi
2008-04-04 19:48 ` Andrew Morton
2008-04-04 19:48   ` Andrew Morton
2008-04-04 19:48   ` Andrew Morton
2008-04-04 20:09   ` Russell King - ARM Linux
2008-04-04 20:09     ` Russell King - ARM Linux
2008-04-04 20:09     ` Russell King - ARM Linux
2008-04-04 23:09   ` David Brownell
2008-04-04 23:09     ` David Brownell
2008-04-04 23:09     ` David Brownell
2008-04-04 23:21     ` Josh Boyer
2008-04-04 23:21       ` Josh Boyer
2008-04-04 23:21       ` Josh Boyer
2008-04-04 23:58       ` David Brownell
2008-04-04 23:58         ` David Brownell
2008-04-04 23:58         ` David Brownell
2008-04-05  1:11         ` Josh Boyer
2008-04-05  1:11           ` Josh Boyer
2008-04-05  1:11           ` Josh Boyer
2008-04-05  1:29           ` Kyungmin Park
2008-04-05  1:29             ` Kyungmin Park
2008-04-05  1:29             ` Kyungmin Park
2008-04-05  1:46             ` Andrew Morton [this message]
2008-04-05  1:46               ` Andrew Morton
2008-04-05  1:46               ` Andrew Morton
2008-04-05  2:41               ` David Brownell
2008-04-05  2:41                 ` David Brownell
2008-04-05  2:41                 ` David Brownell
2008-04-05  3:27                 ` Andrew Morton
2008-04-05  3:27                   ` Andrew Morton
2008-04-05  3:27                   ` Andrew Morton
2008-04-05  2:17             ` David Brownell
2008-04-05  2:17               ` David Brownell
2008-04-05  2:17               ` David Brownell
  -- strict thread matches above, loose matches on Subject: below --
2008-04-05 14:05 matthieu castet

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=20080404184615.deaf3122.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=david-b@pacbell.net \
    --cc=dwmw2@infradead.org \
    --cc=jwboyer@gmail.com \
    --cc=kmpark@infradead.org \
    --cc=linux-arm-kernel@lists.arm.linux.org.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=spi-devel-general@lists.sourceforge.net \
    --cc=trimarchimichael@yahoo.it \
    /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.