On Sun, Jun 18, 2023 at 4:20 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>
> On Sun, Jun 18, 2023 at 06:57:44AM +0000, Yosry Ahmed wrote:
> > @@ -337,6 +318,7 @@ struct folio {
> > atomic_t _entire_mapcount;
> > atomic_t _nr_pages_mapped;
> > atomic_t _pincount;
> > + atomic_t _mlock_count;
> > #ifdef CONFIG_64BIT
> > unsigned int _folio_nr_pages;
> > #endif
>
> You can't quite do this. On 32-bt systems (I know, I know ...),
> we have:
>
> offset page 0 page 1
> 0 flags flags
> 4 lru head
> 8 lru dtor+order
> 12 mapping entire_mapcount
> 16 index nr_pages_napped
> 20 private pincount
> 24 mapcount
> 28 refcount
>
> so it actually ends up overlapping page->mapcount on the second page,
> which is of course used for counting the number of PTEs which map
> that specific page.
Ah yeah, of course. In retrospect, it was very silly of me not to
notice given that I was modifying the mapcount handling code.
Thanks for pointing this out.
>
> I don't have a scenario where this would matter, but we are quite
> careful to only allocate order-2+ large folios, so I'd suggest putting
> it in page 2 instead of page 1.
Can you point me to the code that does so?
>
> I should probably add a comment to struct folio warning of this dragon.
> I thought the #ifdef CONFIG_64BIT would be enough to warn unwary
> passers-by of its presence, but a more explicit sign must be in order.
>
I saw the CONFIG_64BIT and spent some time trying to figure out why we
need it. I added the size of all explicit fields on 32-bit and they
were less than 32 bytes, so I was confused. I looked at the commit log
with no luck. I should have realized we should not overlay mapcount or
refcount. The presence of _mapcount_1/_refcount_1 fields could have
helped.
Anyway, we can also put it in page 1 inside #ifdef CONFIG_64BIT. We
will need a few extra #ifdef's in mm/mlock.c, and I think that's
pretty much it. For 32-bit, we can just keep piggybacking on mapcount
for all folios, I doubt a huge number of mappings is a concern for
32-bit anyway. I don't have a strong preference.
