From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Carstens Subject: Re: [PATCH 2/2] fs: proc/stat: use usual seq_file ops rather than single_open Date: Fri, 30 May 2014 10:38:30 +0200 Message-ID: <20140530083830.GA4732@osiris> References: <20140521122521.GB7471@osiris> <20140521143229.GA32011@infradead.org> <20140528085841.GA4219@osiris> <20140528090153.GC4219@osiris> <20140528153704.b2a3f46dc39ebf8f681d528a@linux-foundation.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Christoph Hellwig , KAMEZAWA Hiroyuki , Andrea Righi , Eric Dumazet , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, Hendrik Brueckner , Thorsten Diehl , Ian Kent , "Elliott, Robert (Server Storage)" To: Andrew Morton Return-path: Content-Disposition: inline In-Reply-To: <20140528153704.b2a3f46dc39ebf8f681d528a@linux-foundation.org> Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org On Wed, May 28, 2014 at 03:37:04PM -0700, Andrew Morton wrote: > On Wed, 28 May 2014 11:01:53 +0200 Heiko Carstens 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...