From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753478AbdK2OA6 (ORCPT ); Wed, 29 Nov 2017 09:00:58 -0500 Received: from mail-wm0-f67.google.com ([74.125.82.67]:42832 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750708AbdK2OA5 (ORCPT ); Wed, 29 Nov 2017 09:00:57 -0500 X-Google-Smtp-Source: AGs4zMYiKzRwh+U+ka95Sr97nUwrGRqyjnZT/CAV5OgYtLbvOBzShgNsxlECsXtajJpL85sRYwJF0A== Message-ID: <1511964054.4897.98.camel@gmail.com> Subject: Re: [PATCH 3/3] trace-cmd: Making stat to report when the stack tracer is ON From: Vladislav Valtchev To: Steven Rostedt Cc: linux-kernel@vger.kernel.org Date: Wed, 29 Nov 2017 16:00:54 +0200 In-Reply-To: <20171129075738.0ccd7d84@gandalf.local.home> 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> Organization: VMware Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.26.1-1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2017-11-29 at 07:57 -0500, Steven Rostedt wrote: > > Let's think about what the user wants. > > If you do a "trace-cmd stat" what are you looking for? You want to see > what ftrace operations are available. Now let's say we do something > weird, or someone has some weird modified kernel, and the stack tracer > shows something that trace-cmd doesn't expect. With a die, it kills the > tool. > > Would you like it if you ran "trace-cmd stat" and got it crashed with > an error message saying the kernel is doing something it doesn't > understand? To me, I'd be pissed. I would be cursing at trace-cmd > saying "I don't give a frick about that, show me what you do know!" > I'm surprised how different kind of users are we :-) To me as user, in case there is a kernel bug, the best thing I'd expect to see is the tool refusing to work and reporting that it does not really know the state of the tracer because of invalid data in tracefs. In other words, I expect a tool to behave like: "I don't know what is that, so I cannot take any decisions. Here's the detailed problem (err msg, data). Now only a human may help now". The other approach is instead: "I don't know what is that, but I'll guess by best trying to not piss off the user". Both approaches have PROs and CONs. It is evident that, in the first case the tool is pedantic and won't give even a try to do something. In the 2nd case instead, the tool might guess even correctly at first but, by not exposing the underlying issue, it might fail (if there's a mem corruption at kernel-level, likely it will) at any moment in a strange way. In any case, the user will be pissed. Just, in the first case he/she will benefit from an "early-stage" error, that might make the problem easier to find. Also, with the 2nd approach, the user won't figure out immediately that the tool is not guilty, while the kernel is: nobody should blame the poor tool when it had no chance to get its job done in the first place. > Now, do you think having a "die" is good there? I prefer the "fail-early" approach in general. For a tool like trace-cmd, I'd implement a layer validating all the input with an option for controlling validation in hot paths. BUT, since that is not the philosophy of the tool, adding a check like that only there, does not make much sense. It makes sense to take an approach and consistently follow it. So, I'll fix my patch as you suggested. I hope I'm not "pissing off" you with my long comments :-) Vlad