Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751950AbdF3Xht (ORCPT ); Fri, 30 Jun 2017 19:37:49 -0400 Received: from mail-oi0-f49.google.com ([209.85.218.49]:33021 "EHLO mail-oi0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751560AbdF3Xhs (ORCPT ); Fri, 30 Jun 2017 19:37:48 -0400 MIME-Version: 1.0 In-Reply-To: <20170630175057.71de9d39@gandalf.local.home> References: <20170630171748.sf5rnjb7pyws6ewu@mikesart-cos> <20170630175057.71de9d39@gandalf.local.home> From: Joel Fernandes Date: Fri, 30 Jun 2017 16:37:47 -0700 Message-ID: Subject: Re: [PATCH] tracing: Add saved_tgids file to show cached pid to tgid mappings To: Steven Rostedt Cc: Michael Sartain , LKML , Ingo Molnar Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1239 Lines: 33 On Fri, Jun 30, 2017 at 2:50 PM, Steven Rostedt wrote: > On Fri, 30 Jun 2017 11:17:50 -0600 [..] >> >> Signed-off-by: Michael Sartain >> --- >> kernel/trace/trace.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 59 insertions(+), 1 deletion(-) >> >> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c >> index 68c214b..ca84c97 100644 >> --- a/kernel/trace/trace.c >> +++ b/kernel/trace/trace.c >> @@ -4692,6 +4692,7 @@ static const struct file_operations tracing_readme_fops = { >> static void *saved_cmdlines_next(struct seq_file *m, void *v, loff_t *pos) >> { >> unsigned int *ptr = v; >> + long tgid_check = (long) m->private; > > I really don't like the subtle use of having m->private != NULL mean > this is for tgid listing. > > In fact, I don't see the purpose of reusing the seq code. The cmdlines > and tgid map are quite different. Just create its own functions. I > don't see the benefit of trying to reuse this except for making the > code more complex. I agree with Steven, this patch should be rewritten to use separate functions and not reuse the other ones as its otherwise really confusing and not good design. Thanks, Joel