Messages in this thread | | | Date | Fri, 30 May 2014 10:38:30 +0200 | From | Heiko Carstens <> | Subject | Re: [PATCH 2/2] fs: proc/stat: use usual seq_file ops rather than single_open |
| |
On Wed, May 28, 2014 at 03:37:04PM -0700, Andrew Morton wrote: > On Wed, 28 May 2014 11:01:53 +0200 Heiko Carstens <heiko.carstens@de.ibm.com> wrote: > > With this patch it should not happen anymore that reading /proc/stat > > fails because of a failing high order memory allocation. > > So this deletes the problematic allocation which [1/2] kind-of fixed, > yes?
Yes.
> I agree with Ian - there's a hotplugging race. And [1/2] doesn't do > anything to address the worst-case allocation size. So I think we may > as well do this all in a single patch.
Fine with me. However the hotplugging race in 1/2 doesn't matter: if the result doesn't fit into the preallocated buffer the seq_file infrastructure would simply allocate a buffer twice as large as before and retry.
The point of patch 1/2 was to have a patch that probably solves the problem almost always ;) , without having the problems you describe below.
> Without having looked closely at the code I worry a bit about the > effects. /proc/pid/stat is a complex thing and its contents will vary
It's /proc/stat not /proc/pid/stat.
> So.. can we take this up for 3.16-rc1? See if we can get some careful > review done then and test it for a couple of months?
Sure.
> Meanwhile, the changelog looks a bit hastily thrown together - some > smoothing would be nice, and perhaps some work spent identifying > possible behavioural changes. Timing changes, locking canges, effects > of concurrent fork/exit activity etc?
Well... I'll try to come up with something better. Even though I only forward ported an existing patch to address a memory allocation failure. Oh oh...
| |