All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Rientjes <rientjes@google.com>
To: Ian Kent <raven@themaw.net>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Christoph Hellwig <hch@infradead.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	Andrea Righi <andrea@betterlinux.com>,
	Eric Dumazet <eric.dumazet@gmail.com>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Hendrik Brueckner <brueckner@linux.vnet.ibm.com>,
	Thorsten Diehl <thorsten.diehl@de.ibm.com>,
	"Elliott, Robert (Server Storage)" <Elliott@hp.com>,
	Al Viro <viro@zeniv.linux.org.uk>
Subject: Re: [PATCH 2/2] fs: proc/stat: use usual seq_file ops rather than single_open
Date: Wed, 11 Jun 2014 23:52:31 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.2.02.1406112350010.23724@chino.kir.corp.google.com> (raw)
In-Reply-To: <1402554262.2642.16.camel@perseus.fritz.box>

On Thu, 12 Jun 2014, Ian Kent wrote:

> > > diff --git a/fs/seq_file.c b/fs/seq_file.c
> > > index 1d641bb108d2..fca78a04c0d1 100644
> > > --- a/fs/seq_file.c
> > > +++ b/fs/seq_file.c
> > > @@ -8,8 +8,10 @@
> > >  #include <linux/fs.h>
> > >  #include <linux/export.h>
> > >  #include <linux/seq_file.h>
> > > +#include <linux/vmalloc.h>
> > >  #include <linux/slab.h>
> > >  #include <linux/cred.h>
> > > +#include <linux/mm.h>
> > >  
> > >  #include <asm/uaccess.h>
> > >  #include <asm/page.h>
> > > @@ -82,6 +84,31 @@ int seq_open(struct file *file, const struct seq_operations *op)
> > >  }
> > >  EXPORT_SYMBOL(seq_open);
> > >  
> > > +static void seq_alloc(struct seq_file *m)
> > > +{
> > > +	m->size = PAGE_SIZE;
> > > +	m->buf = kmalloc(m->size, GFP_KERNEL | __GFP_NOWARN);
> > > +	if (!m->buf)
> > > +		m->buf = vmalloc(m->size);
> > > +}
> > > +
> > 
> > If m->size is unconditionally PAGE_SIZE, then how is vmalloc() going to 
> > allocate this if kmalloc() fails?
> 
> This is just the initial allocation.
> If it runs out of room the allocation size doubles.
> 
> I think 2*PAGE_SIZE is probably better here since that's closer to what
> the original heuristic allocation requested and is likely to avoid
> reallocations in most cases.
> 
> The issue of kmalloc() failing for larger allocations on low speced
> hardware with fragmented memory might succeed when vmalloc() is used
> since it doesn't require contiguous memory chunks. But I guess the added
> pressure on the page table might still be a problem, nevertheless it's
> probably worth trying before bailing out. 
> 

I'm not quarreling about using vmalloc() for allocations that are 
high-order, I'm referring to the rather obvious fact that m->size is set 
to PAGE_SIZE unconditionally above and thus vmalloc() isn't going to help 
in the slightest.

  reply	other threads:[~2014-06-12  6:52 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-21 12:25 /proc/stat vs. failed order-4 allocation Heiko Carstens
2014-05-21 14:32 ` Christoph Hellwig
2014-05-22  3:05   ` Elliott, Robert (Server Storage)
2014-05-28  8:58   ` Heiko Carstens
2014-05-28  8:59     ` [PATCH 1/2] fs: proc/stat: use num_online_cpus() for buffer size Heiko Carstens
2014-05-28 11:06       ` Ian Kent
2014-05-28 11:14         ` Ian Kent
2014-05-28  9:01     ` [PATCH 2/2] fs: proc/stat: use usual seq_file ops rather than single_open Heiko Carstens
2014-05-28 22:37       ` Andrew Morton
2014-05-30  8:38         ` Heiko Carstens
2014-05-30 11:36           ` [PATCH] fs: proc/stat: use seq_file iterator interface Heiko Carstens
2014-06-09  8:11         ` [PATCH 2/2] fs: proc/stat: use usual seq_file ops rather than single_open Ian Kent
2014-06-11 12:43           ` Heiko Carstens
2014-06-11 22:29             ` David Rientjes
2014-06-12  6:24               ` Ian Kent
2014-06-12  6:52                 ` David Rientjes [this message]
2014-06-12  7:27                   ` Heiko Carstens
2014-06-12  8:18                     ` Heiko Carstens
2014-06-12 20:59                     ` David Rientjes
2014-06-12 11:09                   ` Ian Kent
2014-05-22 11:29 ` /proc/stat vs. failed order-4 allocation Ian Kent

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=alpine.DEB.2.02.1406112350010.23724@chino.kir.corp.google.com \
    --to=rientjes@google.com \
    --cc=Elliott@hp.com \
    --cc=akpm@linux-foundation.org \
    --cc=andrea@betterlinux.com \
    --cc=brueckner@linux.vnet.ibm.com \
    --cc=eric.dumazet@gmail.com \
    --cc=hch@infradead.org \
    --cc=heiko.carstens@de.ibm.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=raven@themaw.net \
    --cc=thorsten.diehl@de.ibm.com \
    --cc=viro@zeniv.linux.org.uk \
    /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.