From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756129AbbDJMS5 (ORCPT ); Fri, 10 Apr 2015 08:18:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34418 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755987AbbDJMSx (ORCPT ); Fri, 10 Apr 2015 08:18:53 -0400 Date: Fri, 10 Apr 2015 08:18:38 -0400 From: Jarod Wilson To: Andrew Morton Cc: linux-kernel@vger.kernel.org, Alexey Dobriyan , "Eric W. Biederman" , Al Viro , Miklos Szeredi , Zefan Li , Oleg Nesterov Subject: Re: [PATCH] fs/proc: allow larger /proc//cmdline output Message-ID: <20150410121838.GA13391@redhat.com> References: <1428638342-6782-1-git-send-email-jarod@redhat.com> <20150409211200.78b8631e.akpm@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150409211200.78b8631e.akpm@linux-foundation.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Apr 09, 2015 at 09:12:00PM -0700, Andrew Morton wrote: > On Thu, 9 Apr 2015 23:59:02 -0400 Jarod Wilson wrote: > > > There are people who run java. Sometimes, when it misbehaves, they try to > > figure out what's going on by dumping /proc//cmdline, but the length > > of that output is currently capped by PAGE_SIZE (so x86_64's 4k, in most > > cases), and sometimes, java command lines are longer than 4k characters. > > > > This change allows the user to request a larger max length, up to 4x > > PAGE_SIZE, but the default out-of-the-box setting should keep things the > > same as they ever were. The 4x maximum is somewhat arbitrary, but seemed > > like it should be more than enough, because really, if you have more than > > 16k characters on your command line, you're probably doing it wrong... > > > > I've tested this lightly with non-java shell commands with really long > > parameters, and things are perfectly stable after several hundred > > iterations of exercising things on a system booted with both > > proc_pid_maxlen=8192 and 16384. I wouldn't call my testing exhaustive, > > and I may not have considered something that will blow up horribly here, > > so comments and clues welcomed. > > > > Using single_open_size() looked less messy than giving proc_pid_cmdline() > > its own .start op that would allow multiple buffers. > > > > Note: I've only added this extended sizing for /proc//cmdline output, > > rather than for all /proc//foo elements, thinking that nothing else > > should ever really be that long, but anything that is can simply switch > > from using the ONE() macro to the ONE_SIZE() macro. > > Why have an upper limit at all? Just trying to be conservative and keep people from doing anything too insane, but I didn't really have any particularly good reason beyond that for capping it. I'll remove the upper bound next iteration. > > --- a/fs/proc/base.c > > +++ b/fs/proc/base.c > > @@ -134,6 +134,30 @@ struct pid_entry { > > NOD(NAME, (S_IFREG|(MODE)), \ > > NULL, &proc_single_file_operations, \ > > { .proc_show = show } ) > > +#define ONE_SIZE(NAME, MODE, show) \ > > + NOD(NAME, (S_IFREG|(MODE)), \ > > + NULL, &proc_single_file_size_operations, \ > > + { .proc_show = show } ) > > + > > +/* > > + * Its hideous, but some java gunk winds up with a cmdline that is longer > > + * than PAGE_SIZE, and some people want to be able to see all of it for > > + * debugging purposes. Allocate at least PAGE_SIZE, and allow the user to > > + * ask for up to PAGE_SIZE << 2 (4x) to help with that situation. > > + */ > > +static unsigned long proc_pid_maxlen = PAGE_SIZE; > > +static int set_proc_pid_maxlen(char *str) > > +{ > > + if (!str) > > + return 0; > > + > > + proc_pid_maxlen = simple_strtoul(str, &str, 0); > > + proc_pid_maxlen = max(PAGE_SIZE, proc_pid_maxlen); > > + proc_pid_maxlen = min(PAGE_SIZE << 2, proc_pid_maxlen); > > + > > + return 1; > > +} > > +__setup("proc_pid_maxlen=", set_proc_pid_maxlen); > > This permits 4k-16k on x86 and 64k-256k on powerpc. This makes the > kernel interface inconsistent across architectures, which is not good - > some applications will work OK on one arch but will fail when moved to > a different arch. > > s/PAGE_SIZE/4096/g would fix that. I was going for minimal disturbance to the status quo, which is that everything is structured around PAGE_SIZE. proc_pid_cmdline() has a BUG_ON(m->size < PAGE_SIZE) in it and the default initial allocation for every seq_file using single_open() is PAGE_SIZE, making it a bit more invasive to change it. Perhaps add a default size #define to seq_file.h and use that for seq_file everywhere instead of PAGE_SIZE to get consistency across all arches? -- Jarod Wilson jarod@redhat.com