On Tuesday, 23 November 2021 5:15:27 AM AEDT David Hildenbrand wrote:
[...]
> > +#ifdef CONFIG_MIGRATION
> > +/**
> > + * migration_entry_wait_on_locked - Wait for a migration entry to be removed
> > + * @folio: folio referenced by the migration entry.
> > + * @ptep: mapped pte pointer. This function will return with the ptep unmapped.
> > + * @ptl: already locked ptl. This function will drop the lock.
> > + *
> > + * Wait for a migration entry referencing the given page to be removed. This is
> > + * equivalent to put_and_wait_on_page_locked(page, TASK_UNINTERRUPTIBLE) except
> > + * this can be called without taking a reference on the page. Instead this
> > + * should be called while holding the ptl for the migration entry referencing
> > + * the page.
> > + *
> > + * Returns after unmapping and unlocking the pte/ptl with pte_unmap_unlock().
>
> You could maybe make it clear that callers have to pass the ptep only
> for PTE migration entries. For a PMD migration entry, pass NULL.
Will do.
> > + *
> > + * This follows the same logic as wait_on_page_bit_common() so see the comments
>
> s/wait_on_page_bit_common/folio_wait_bit_common/ ?
Evidently this escaped my s// when rebasing on top of folio's. Will fix.
> > + * there.
> > + */
> > +void migration_entry_wait_on_locked(struct folio *folio, pte_t *ptep,
> > + spinlock_t *ptl)
> > +{
> > + struct wait_page_queue wait_page;
> > + wait_queue_entry_t *wait = &wait_page.wait;
> > + bool thrashing = false;
> > + bool delayacct = false;
> > + unsigned long pflags;
> > + wait_queue_head_t *q;
> > +
> > + q = folio_waitqueue(folio);
> > + if (!folio_test_uptodate(folio) && folio_test_workingset(folio)) {
> > + if (!folio_test_swapbacked(folio)) {
> > + delayacct_thrashing_start();
> > + delayacct = true;
> > + }
> > + psi_memstall_enter(&pflags);
> > + thrashing = true;
> > + }
> > +
> > + init_wait(wait);
> > + wait->func = wake_page_function;
> > + wait_page.folio = folio;
> > + wait_page.bit_nr = PG_locked;
> > + wait->flags = 0;
> > +
> > + spin_lock_irq(&q->lock);
> > + folio_set_waiters(folio);
> > + if (!folio_trylock_flag(folio, PG_locked, wait))
> > + __add_wait_queue_entry_tail(q, wait);
> > + spin_unlock_irq(&q->lock);
> > +
> > + /*
> > + * If a migration entry exists for the page the migration path must hold
> > + * a valid reference to the page, and it must take the ptl to remove the
> > + * migration entry. So the page is valid until the ptl is dropped.
> > + */
> > + if (ptep)
> > + pte_unmap_unlock(ptep, ptl);
> > + else
> > + spin_unlock(ptl);
> > +
> > + for (;;) {
> > + unsigned int flags;
> > +
> > + set_current_state(TASK_UNINTERRUPTIBLE);
> > +
> > + /* Loop until we've been woken or interrupted */
> > + flags = smp_load_acquire(&wait->flags);
> > + if (!(flags & WQ_FLAG_WOKEN)) {
> > + if (signal_pending_state(TASK_UNINTERRUPTIBLE, current))
> > + break;
> > +
> > + io_schedule();
> > + continue;
> > + }
> > + break;
> > + }
> > +
> > + finish_wait(q, wait);
> > +
> > + if (thrashing) {
> > + if (delayacct)
> > + delayacct_thrashing_end();
> > + psi_memstall_leave(&pflags);
> > + }
> > +}
> > +#endif
> > +
>
> I'm fairly new to the glory details of core migration entry and page bit
> waiting code, but it makes sense to me and removing the temporary extra
> references is very nice! Feel free to add my
>
> Acked-by: David Hildenbrand <david@xxxxxxxxxx>
Thanks for taking a look, really appreciate it!
- Alistair
