Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1751570AbdFZT2O (ORCPT <rfc822;w@1wt.eu>);
        Mon, 26 Jun 2017 15:28:14 -0400
Received: from mail.kernel.org ([198.145.29.99]:48832 "EHLO mail.kernel.org"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1751085AbdFZT2K (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Mon, 26 Jun 2017 15:28:10 -0400
DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 24870217C3
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: Mon, 26 Jun 2017 15:28:07 -0400
From: Steven Rostedt <rostedt@goodmis.org>
To: Joel Fernandes <joelaf@google.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
        Michael Sartain <mikesart@gmail.com>, kernel-team@android.com,
        Ingo Molnar <mingo@redhat.com>
Subject: Re: [PATCH v4 1/3] tracing: Add support for recording tgid of tasks
Message-ID: <20170626152807.7d40edb7@gandalf.local.home>
In-Reply-To: <CAJWu+oqbEuT3cKGVJ_bQAxR5ra_2_CWtSNzb8XpRTPfFQG3K-Q@mail.gmail.com>
References: <20170626053844.5746-1-joelaf@google.com>
        <20170626053844.5746-2-joelaf@google.com>
        <20170626125226.05c9a80e@gandalf.local.home>
        <CAJWu+ooqOTem6yKFOYkC7xn3WZiFMHA1hkvxtV6=R3bN-fjF6g@mail.gmail.com>
        <CAJWu+oqbEuT3cKGVJ_bQAxR5ra_2_CWtSNzb8XpRTPfFQG3K-Q@mail.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: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1142
Lines: 33

On Mon, 26 Jun 2017 12:10:04 -0700
Joel Fernandes <joelaf@google.com> wrote:


> > That seems Ok with me to do, I think a valid point.
> >
> > I think that I should do it in the second call to
> > tracing_start_tgid_record too then (__ftrace_event_enable_disable) to
> > error out if the allocation fails.
> >
> > While going this code I again, I noticed another potential issue in
> > __ftrace_event_enable_disable  
> 
> Sorry I hit send too soon!
> 
> I was saying, While going through this code again I noticed another
> potential issue in __ftrace_event_enable_disable in existing code:
> 
> Here we do:
> ret = call->class->reg(call, TRACE_REG_REGISTER, file);
> 
> But in the error handling path we only do a
> tracing_stop_cmdline_record() and not clear the
> EVENT_FILE_FL_RECORDED_CMD_BIT flag. Is that not an issue? I am
> guessing because enabling of the event itself will fail, that's Ok.
> But just to keep it consistent, I am thinking if we should just clear
> the bit here.

Yeah, we should probably add some better error handling for failed
setting of events. But the tgid-map is low hanging fruit to get
right ;-)

-- Steve