From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753095AbdK3O4k (ORCPT ); Thu, 30 Nov 2017 09:56:40 -0500 Received: from mail.kernel.org ([198.145.29.99]:34390 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751787AbdK3O4j (ORCPT ); Thu, 30 Nov 2017 09:56:39 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2ACE4214E3 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=goodmis.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=rostedt@goodmis.org Date: Thu, 30 Nov 2017 09:56:36 -0500 From: Steven Rostedt To: Vladislav Valtchev Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/3] trace-cmd: Making stat to report when the stack tracer is ON Message-ID: <20171130095636.7c0ae4f9@gandalf.local.home> In-Reply-To: <1512041215.4897.160.camel@gmail.com> References: <20171122180202.9519-1-vladislav.valtchev@gmail.com> <20171122180202.9519-4-vladislav.valtchev@gmail.com> <20171122145002.17c39637@gandalf.local.home> <1511440352.2719.34.camel@gmail.com> <20171129075738.0ccd7d84@gandalf.local.home> <1511964054.4897.98.camel@gmail.com> <20171129111815.1171c7f3@gandalf.local.home> <1512041215.4897.160.camel@gmail.com> X-Mailer: Claws Mail 3.14.0 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 30 Nov 2017 13:26:55 +0200 Vladislav Valtchev wrote: > I proposed die() because, by looking at the original code of read_proc(): > > static char read_proc(void) > { > char buf[1]; > int fd; > int n; > > fd = open(PROC_FILE, O_RDONLY); > if (fd < 0) > die("reading %s", PROC_FILE); > n = read(fd, buf, 1); > close(fd); > if (n != 1) > die("error reading %s", PROC_FILE); > > return buf[0]; > } > > I saw that trace-cmd dies in case: > - the file cannot be opened [e.g. file not found, no permissions etc.] > - the file is empty Right, which perhaps it shouldn't die, but there shouldn't be a case where this happens. As for file not found, it should be checked before calling this function, as you noticed this is done below. > > So I thought that the approach was: > "if the contract is violated, I die" > > > Now, do you want also that the cases when the > PROC_FILE, /proc/sys/kernel/stack_tracer_enabled, cannot be opened > or it is empty should be to gracefully handled showing a warning + unknown > status for the stack tracer? Let's just have the trace-cmd stat say stack tracing is "indeterminate". > > I noticed also this function: > > static void test_available(void) > { > struct stat buf; > int fd; > > fd = stat(PROC_FILE, &buf); > if (fd < 0) > die("stack tracer not configured on running kernel"); > } > > Called by trace_stack(). I start to think: maybe it's fine for 'stat' to > just assume that the stack tracer is not configured on the running kernel > if the file does not exist, but it should show a warning + UNKNOWN status > is the file is empty. Right? Actually, if the file doesn't exist, then it isn't enabled for the kernel. In that case, we do nothing (don't report stack tracing). Stat is about what is enabled and configured into the kernel. Not what isn't configured into the kernel. Having stack tracing not enabled is common in some configurations. I don't want to add noise to the output. Unless we add a "-v" for verbose option. Then perhaps with verbose set, we can say what isn't enabled. Better yet, have: -v - show all that is configured into the kernel but not enabled -vv - show all that trace-cmd knows about but isn't configured into the kernel. -- Steve