On Tue, Dec 10, 2024 at 12:35 AM Sebastian Andrzej Siewior
<bigeasy@xxxxxxxxxxxxx> wrote:
>
> On 2024-12-09 18:39:32 [-0800], Alexei Starovoitov wrote:
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index d511e68903c6..a969a62ec0c3 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -1251,9 +1254,33 @@ static void free_one_page(struct zone *zone, struct page *page,
> > unsigned long pfn, unsigned int order,
> > fpi_t fpi_flags)
> > {
> > + struct llist_head *llhead;
> > unsigned long flags;
> >
> > - spin_lock_irqsave(&zone->lock, flags);
> > + if (!spin_trylock_irqsave(&zone->lock, flags)) {
> > + if (unlikely(fpi_flags & FPI_TRYLOCK)) {
> > + /* Remember the order */
> > + page->order = order;
> > + /* Add the page to the free list */
> > + llist_add(&page->pcp_llist, &zone->trylock_free_pages);
> > + return;
> > + }
> > + spin_lock_irqsave(&zone->lock, flags);
> > + }
> > +
> > + /* The lock succeeded. Process deferred pages. */
> > + llhead = &zone->trylock_free_pages;
> > + if (unlikely(!llist_empty(llhead))) {
> > + struct llist_node *llnode;
> > + struct page *p, *tmp;
> > +
> > + llnode = llist_del_all(llhead);
>
> Do you really need to turn the list around?
I didn't think LIFO vs FIFO would make a difference.
Why spend time rotating it?
> > + llist_for_each_entry_safe(p, tmp, llnode, pcp_llist) {
> > + unsigned int p_order = p->order;
> > + split_large_buddy(zone, p, page_to_pfn(p), p_order, fpi_flags);
> > + __count_vm_events(PGFREE, 1 << p_order);
> > + }
>
> We had something like that (returning memory in IRQ/ irq-off) in RT tree
> and we got rid of it before posting the needed bits to mm.
>
> If we really intend to do something like this, could we please process
> this list in an explicitly locked section? I mean not in a try-lock
> fashion which might have originated in an IRQ-off region on PREEMPT_RT
> but in an explicit locked section which would remain preemptible. This
> would also avoid the locking problem down the road when
> shuffle_pick_tail() invokes get_random_u64() which in turn acquires a
> spinlock_t.
I see. So the concern is though spin_lock_irqsave(&zone->lock)
is sleepable in RT, bpf prog might have been called in the context
where preemption is disabled and do split_large_buddy() for many
pages might take too much time?
How about kicking irq_work then? The callback is in kthread in RT.
We can irq_work_queue() right after llist_add().
Or we can process only N pages at a time in this loop and
llist_add() leftover back into zone->trylock_free_pages.
