Subject: Re: [PATCH tip 0/3] Improvements of scheduler related Tracepoints
To: Peter Zijlstra <peterz@infradead.org>, Teng Qin <qinteng@fb.com>
References: <20171214202044.1629279-1-qinteng@fb.com>
 <20171214204932.GH3326@worktop>
CC: <mingo@redhat.com>, <bgregg@netflix.com>, <daniel@iogearbox.net>,
        <yhs@fb.com>, <linux-kernel@vger.kernel.org>, <Kernel-team@fb.com>
From: Alexei Starovoitov <ast@fb.com>
Message-ID: <1632e487-ee65-b50d-85e5-82f42c69fea1@fb.com>
Date: Thu, 14 Dec 2017 19:16:00 -0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:45.0)
 Gecko/20100101 Thunderbird/45.8.0
MIME-Version: 1.0
In-Reply-To: <20171214204932.GH3326@worktop>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Dec 2017 03:16:04.1974 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 5ce03644-9714-4741-414d-08d5436a2d9b
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR15MB0966
X-OriginatorOrg: fb.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-12-15_01:,,
 signatures=0
X-Proofpoint-Spam-Reason: safe
X-FB-Internal: Safe
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1659
Lines: 34

On 12/14/17 12:49 PM, Peter Zijlstra wrote:
> On Thu, Dec 14, 2017 at 12:20:41PM -0800, Teng Qin wrote:
>> This set of commits attempts to improve three scheduler related
>> Tracepoints: sched_switch, sched_process_fork, sched_process_exit.
>>
>> Firstly, these commit add additional flag values, namely preempt,
>> clone_flags and group_dead to these Tracepoints, to make information
>> exposed via the Tracepoints more useful and complete.
>>
>> Secondly, these commits exposes task_struct pointers in these
>> Tracepoints. The task_struct pointers are arguments of the Tracepoints
>> and currently only used to compute struct field values. But for BPF
>> programs attached to these Tracepoints, we may want to read additional
>> task information via the task_struct pointers. This is currently either
>> impossible, or we have to make assumption of whether the Tracepoint is
>> running from previous / parent or next / child, and use current pointer
>> instead. Exposing the task_struct pointers explicitly makes such use
>> case easier and more reliable.
>>
>
> NAK

not sure what is the concern here.
Is it first or second part of the above ?
preempt and group_dead are bool and clone_flags has uapi defined
flags, so no kernel internals being exposed.
Two task_struct pointers are unusable outside of bpf progs.
There are plenty of other tracepoints that store pointers to
kernel structs and bpf progs are looking into them.
So nothing new being exposed here as well.

Note that TP_printk() kept unchanged, so typical user tracing
that parses trace_pipe won't see any difference.
Apps that use binary apis are using libs like libtracecmd and also fine.