linux-kernel - Re: [PATCH v5] fs: Improve eventpoll logging to stop indicting timerfd

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: <CAGudoHFH+bCK6+tjgFRSfUvKhPwPTY6AGcD_S1pWy5ESYKUw2g@mail.gmail.com>
Date: Thu, 4 Jul 2024 14:44:12 +0200
From: Mateusz Guzik <mjguzik@...il.com>
To: Isaac Manjarres <isaacmanjarres@...gle.com>
Cc: tglx@...utronix.de, "Rafael J. Wysocki" <rafael@...nel.org>, Pavel Machek <pavel@....cz>, 
	Len Brown <len.brown@...el.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	Alexander Viro <viro@...iv.linux.org.uk>, Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, 
	saravanak@...gle.com, Manish Varma <varmam@...gle.com>, 
	Kelly Rossmoyer <krossmo@...gle.com>, kernel-team@...roid.com, linux-pm@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v5] fs: Improve eventpoll logging to stop indicting timerfd

On Wed, Jul 3, 2024 at 11:37 PM Isaac Manjarres
<isaacmanjarres@...gle.com> wrote:
>
> On Tue, Jun 25, 2024 at 07:58:43PM +0200, Mateusz Guzik wrote:
> > On Thu, Jun 06, 2024 at 10:28:11AM -0700, Isaac J. Manjarres wrote:
> > > +static atomic_t wakesource_create_id  = ATOMIC_INIT(0);
> > >  static const struct file_operations eventpoll_fops;
> > >
> > >  static inline int is_file_epoll(struct file *f)
> > > @@ -1545,15 +1546,21 @@ static int ep_create_wakeup_source(struct epitem *epi)
> > >  {
> > >     struct name_snapshot n;
> > >     struct wakeup_source *ws;
> > > +   pid_t task_pid;
> > > +   int id;
> > > +
> > > +   task_pid = task_pid_nr(current);
> > >
> > >     if (!epi->ep->ws) {
> > > -           epi->ep->ws = wakeup_source_register(NULL, "eventpoll");
> > > +           id = atomic_inc_return(&wakesource_create_id);
> > > +           epi->ep->ws = wakeup_source_register(NULL, "epoll:%d:%d", id, task_pid);
> >
> > How often does this execute? Is it at most once per task lifespan?
> Thank you for your feedback! This can execute multiple times throughout
> a task's lifespan. However, I haven't seen it execute that often.
>
> > The var probably wants to be annotated with ____cacheline_aligned_in_smp
> > so that it does not accidentally mess with other stuff.
> >
> > I am assuming there is no constant traffic on it.
> Right, I don't see much traffic on it. Can you please elaborate a bit
> more on what interaction you're concerned with here? If it's a
> concern about false sharing, I'm worried that we may be prematurely
> optimizing this.
>

I am concerned with false sharing indeed, specifically with this
landing with something unrelated to epoll.

Preferably the linker would not merge cachelines across different .o
files and that would make the problem mostly sorted out.

In the meantime I would argue basic multicore hygiene dictates vars
like this one get moved out of the way if only to not accidentally
mess with other stuff.

But I am not going to pester you about it, It's not my call for this
code either.
-- 
Mateusz Guzik <mjguzik gmail.com>