On Wed, Jul 3, 2024 at 11:37 PM Isaac Manjarres
<isaacmanjarres@xxxxxxxxxx> 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>
