linux-kernel - Re: [PATCH] mm/migrate.c: Rework migration_entry

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: <7899398.tcx7jHFkIm@nvdebian>
Date:   Fri, 5 Nov 2021 18:02:14 +1100
From:   Alistair Popple <apopple@...dia.com>
To:     Matthew Wilcox <willy@...radead.org>
CC:     <akpm@...ux-foundation.org>, <dhowells@...hat.com>,
        <hughd@...gle.com>, <linux-kernel@...r.kernel.org>,
        <linux-mm@...ck.org>, <jglisse@...hat.com>, <jgg@...dia.com>,
        <rcampbell@...dia.com>, <jhubbard@...dia.com>
Subject: Re: [PATCH] mm/migrate.c: Rework migration_entry_wait() to not take a pageref

On Thursday, 4 November 2021 11:21:51 PM AEDT Matthew Wilcox wrote:
> On Thu, Nov 04, 2021 at 09:33:38PM +1100, Alistair Popple wrote:
> > +++ b/mm/filemap.c
> > @@ -1356,6 +1356,88 @@ static inline int wait_on_page_bit_common(wait_queue_head_t *q,
> >  	return wait->flags & WQ_FLAG_WOKEN ? 0 : -EINTR;
> >  }
> >  
> > +/**
> > + * migration_entry_wait_on_locked - Wait for a migration entry to be removed
> > + * @page: page 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().
> > + *
> > + * This follows the same logic as wait_on_page_bit_common() so see the comments
> > + * there.
> > + */
> > +void migration_entry_wait_on_locked(struct page *page, 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 = page_waitqueue(page);
> 
> You're going to need to update this patch to apply to Linus' current
> tree; page_waitqueue() went away in favour of folio_waitqueue().

Argh, thanks I had meant to rebase before sending.

> It seems like it would look simpler if this were a patch which modified
> folio_wait_bit_common() instead of doing a manual inline of it into
> this function.

Yes, happy for some opinions here. I was debating a manual inline vs. modifying
folio_wait_bit_common() but felt an additional two special case arguments would
make things a bit messy and there was no obvious way to refactor or split up
folio_wait_bit_common().

However I just noticed wait and wait_page are related so I might be able to
refactor some of the initialisation to reduce code duplication. Will resend a
rebased version doing that.