linux-kernel - Re: [PATCH v3] mm/khugepaged: add tracepoint to collapse

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20221114103350.4b8a2786@gandalf.local.home>
Date:   Mon, 14 Nov 2022 10:33:50 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Gautam Menghani <gautammenghani201@...il.com>
Cc:     akpm@...ux-foundation.org, mhiramat@...nel.org,
        shy828301@...il.com, zokeefe@...gle.com, david@...hat.com,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] mm/khugepaged: add tracepoint to collapse_file()

On Wed, 26 Oct 2022 10:52:18 +0530
Gautam Menghani <gautammenghani201@...il.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