Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 14 Nov 2022 10:33:50 -0500
From:   Steven Rostedt <rostedt@goodmis.org>
To:     Gautam Menghani <gautammenghani201@gmail.com>
Cc:     akpm@linux-foundation.org, mhiramat@kernel.org,
        shy828301@gmail.com, zokeefe@google.com, david@redhat.com,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] mm/khugepaged: add tracepoint to collapse_file()
Message-ID: <20221114103350.4b8a2786@gandalf.local.home>
In-Reply-To: <20221026052218.148234-1-gautammenghani201@gmail.com>
References: <20221026052218.148234-1-gautammenghani201@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Wed, 26 Oct 2022 10:52:18 +0530
Gautam Menghani <gautammenghani201@gmail.com> wrote:

Ideally we want the trace event structure to be as packed as possible to
not waste space on the ring buffer.

> +	TP_ARGS(mm, hpage, index, is_shmem, addr, file, nr, result),
> +	TP_STRUCT__entry(
> +		__field(struct mm_struct *, mm)  4 / 8 bytes (depending on 32 bit or 64 bit arch)
> +		__field(unsigned long, hpfn)     4 / 8 bytes

The two above is fine.

> +		__field(pgoff_t, index)         4 / 8 bytes
> +		__field(bool, is_shmem)         4 bytes (or less)

> +		__field(unsigned long, addr)    4 / 8 bytes

> +		__string(filename, file->f_path.dentry->d_iname)  4 bytes
> +		__field(int, nr)		4 bytes
> +		__field(int, result)		4 bytes

For best packing, it's best to keep long / pointers together, and ints and
bools together (or paired).

On 64 bit archs, there is likely to be a 4 byte hole between is_shmem and
addr.

> +	),
> +

Better to have it be:


	TP_STRUCT__entry(
		__field(struct mm_struct *, mm)
		__field(unsigned long, hpfn)
		__field(pgoff_t, index)
		__field(unsigned long, addr)
		__field(bool, is_shmem)
		__string(filename, file->f_path.dentry->d_iname)
		__field(int, nr)
		__field(int, result)
	),

Where I swapped is_shmem and addr.

-- Steve