Received-SPF: pass (google.com: domain of linux-kernel+bounces-54525-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: Sven Schnelle <svens@linux.ibm.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>,
        Mathieu Desnoyers
 <mathieu.desnoyers@efficios.com>,
        linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
        Mete Durlu <meted@linux.ibm.com>
Subject: Re: [PATCH] tracing: use ring_buffer_record_is_set_on() in
 tracer_tracing_is_on()
In-Reply-To: <yt9d34u63xxz.fsf@linux.ibm.com> (Sven Schnelle's message of
	"Tue, 06 Feb 2024 07:32:08 +0100")
References: <20240205065340.2848065-1-svens@linux.ibm.com>
	<20240205075504.1b55f29c@rorschach.local.home>
	<yt9djznj3vbl.fsf@linux.ibm.com>
	<20240205092353.523cc1ef@rorschach.local.home>
	<yt9d34u63xxz.fsf@linux.ibm.com>
Date: Tue, 06 Feb 2024 09:48:16 +0100
Message-ID: <yt9dsf262d2n.fsf@linux.ibm.com>
User-Agent: Gnus/5.13 (Gnus v5.13)
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain

Sven Schnelle <svens@linux.ibm.com> writes:

> Looking at trace_save_cmdline():
>
> tpid = tsk->pid & (PID_MAX_DEFAULT - 1); where PID_MAX_DEFAULT = 0x8000
>
> so this is basically
>
> tpid = tsk->pid & 0x7fff;
>
> further on:
>
>         // might clash with other pid if (otherpid & 0x7fff) == (tsk->pid & 0x7fff)
> 	idx = savedcmd->map_pid_to_cmdline[tpid];
> 	if (idx == NO_CMDLINE_MAP) {
>                 // This will pick an existing entry if there are
>                 // more than cmdline_num entries present
> 		idx = (savedcmd->cmdline_idx + 1) % savedcmd->cmdline_num;      
> 		savedcmd->map_pid_to_cmdline[tpid] = idx;
> 		savedcmd->cmdline_idx = idx;
> 	}
>
> So i think the problem that sometimes '<...>' instead of the correct
> comm is logged is just expected behaviour given the code above. 

I added some logging, and the test is not triggering this issue. So i
assume the default of 128 cmdline entries is just to small. Sorry for
the noise. Lets see whether we're still triggering some failures with
the other fix applied in CI. If we do, maybe we want to increase the
saved_cmdline_size for the ftrace test suite.