public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Greg KH <gregkh@suse.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Jan Kara <jack@suse.cz>, LKML <linux-kernel@vger.kernel.org>
Subject: Re: Allow setting of number of raw devices as a module parameter
Date: Mon, 2 May 2011 21:22:56 +0200	[thread overview]
Message-ID: <20110502192256.GL4556@quack.suse.cz> (raw)
In-Reply-To: <20110430164141.55d0ef0a@lxorguk.ukuu.org.uk>

On Sat 30-04-11 16:41:41, Alan Cox wrote:
> > > A large vmalloc array is very antisocial on a 32bit x86 box. It looks
> > > like almost all of it would become sane if there was an array of pointers
> > > to raw devices and the devices were initially allocated on need (even if
> > > for now only recovered on rmmod)
> > 
> > In practice, we've never seen a problem with this[1].  Machines that
> > want thousands of raw devices have plenty of memory to handle this
> > situation.
> > 
> > I doubt adding the complexity of dynamically allocating the devices
> > as-needed is worth it for the very few systems that ever use this
> > driver, compounded with the fact that we keep saying that this code
> > isn't to be used by "normal" people anyway.
> 
> That's no excuse for sloppy code. Making it an array populated on demand
> is an improvement for everyone, making it vmalloc hurts every access (in
> TLB misses for one as well as vmalloc overhead).
> 
> I note two of us immediately made the same observation. Doing it
> basically right (array of pointers) is easy. Doing the full works with a
> hash for the lookups is a bit harder and that I would agree is overkill.
  Alan, I might be missing something but is there a significant advantage
in creating the array of pointers? Note that struct raw_device_data which
is what the array is from is:
struct raw_device_data {
        struct block_device *binding;
        int inuse;
};
So 16 bytes on 64-bit arch. You can make it 8 bytes by the
array-of-pointers idea but people requesting this feature want to run with
8192 or even 16384 raw devices in the system (yes, they might be insane).
That gives you 16 to 32 pages for array of pointers which seems too much
to me for kmalloc() anyway.

So if people don't like vmalloc() (which I can understand), then I see no
other option than dynamically allocating struct raw_device_data.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

  parent reply	other threads:[~2011-05-02 19:23 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-28 22:24 Allow setting of number of raw devices as a module parameter Jan Kara
2011-04-29 23:19 ` Greg KH
2011-04-29 23:28 ` Andrew Morton
2011-04-30  0:07   ` Greg KH
2011-04-30  5:09     ` Dave Jones
2011-04-30  5:42       ` Greg KH
2011-04-30 10:29   ` Alan Cox
2011-04-30 15:34     ` Greg KH
2011-04-30 15:41       ` Alan Cox
2011-04-30 15:47         ` Greg KH
2011-05-02 19:22         ` Jan Kara [this message]
2011-05-03  9:42           ` Alan Cox
2011-04-30 12:15 ` Arnd Bergmann
2011-05-02 19:39   ` Jan Kara
2011-05-02 19:44     ` Arnd Bergmann
2011-05-02 21:11       ` Jan Kara
2011-05-03 10:55         ` Arnd Bergmann
2011-05-03 16:26           ` Jan Kara
2011-05-03 17:30             ` Arnd Bergmann

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=20110502192256.GL4556@quack.suse.cz \
    --to=jack@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=gregkh@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox