linux-kernel - Re: [PATCH] device_cgroup: remove branch hint after code refactor

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGudoHESsM03W+Qo3sHP5FEXZOxF_bHBYFErYx81wZwWdq5ANg@mail.gmail.com>
Date: Wed, 7 Jan 2026 15:17:40 +0100
From: Mateusz Guzik <mjguzik@...il.com>
To: Breno Leitao <leitao@...ian.org>
Cc: Christian Brauner <brauner@...nel.org>, linux-kernel@...r.kernel.org, 
	viro@...iv.linux.org.uk, rostedt@...dmis.org, linux-fsdevel@...r.kernel.org, 
	kernel-team@...a.com
Subject: Re: [PATCH] device_cgroup: remove branch hint after code refactor

On Wed, Jan 7, 2026 at 3:06 PM Breno Leitao <leitao@...ian.org> wrote:
>
> commit 4ef4ac360101 ("device_cgroup: avoid access to ->i_rdev in the
> common case in devcgroup_inode_permission()") reordered the checks in
> devcgroup_inode_permission() to check the inode mode before checking
> i_rdev, for better cache behavior.
>
> However, the likely() annotation on the i_rdev check was not updated
> to reflect the new code flow. Originally, when i_rdev was checked
> first, likely(!inode->i_rdev) made sense because most inodes were(?)
> regular files/directories, thus i_rdev == 0.
>
> After the reorder, by the time we reach the i_rdev check, we have
> already confirmed the inode IS a block or character device. Block and
> character special files are precisely defined by having a device number
> (i_rdev), so !inode->i_rdev is now the rare edge case, not the common
> case.
>
> Branch profiling confirmed this is 100% mispredicted:
>
>   correct incorrect  %    Function                      File              Line
>   ------- ---------  -    --------                      ----              ----
>         0   2631904 100   devcgroup_inode_permission    device_cgroup.h   24
>
> Remove likely() to avoid giving the wrong hint to the CPU.
>
> Fixes: 4ef4ac360101 ("device_cgroup: avoid access to ->i_rdev in the common case in devcgroup_inode_permission()")
> Signed-off-by: Breno Leitao <leitao@...ian.org>
> ---
>  include/linux/device_cgroup.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/device_cgroup.h b/include/linux/device_cgroup.h
> index 0864773a57e8..822085bc2d20 100644
> --- a/include/linux/device_cgroup.h
> +++ b/include/linux/device_cgroup.h
> @@ -21,7 +21,7 @@ static inline int devcgroup_inode_permission(struct inode *inode, int mask)
>         if (likely(!S_ISBLK(inode->i_mode) && !S_ISCHR(inode->i_mode)))
>                 return 0;
>
> -       if (likely(!inode->i_rdev))
> +       if (!inode->i_rdev)
>                 return 0;
>

The branch was left there because I could not be bothered to analyze
whether it can be straight up eleminated with the new checks in place.

A quick look at init_special_inode suggests it is an invariant rdev is
there in this case.

So for the time being I would replace likely with WARN_ON_ONCE . Might
be even a good candidate for the pending release.