Page MenuHomeFreeBSD

Allocate wired pages in linux_alloc_pages().
ClosedPublic

Authored by markj on Jun 3 2019, 3:32 PM.

Details

Summary

This is more consistent with Linux, and in the !PMAP_HAS_DMAP case we
are returning wired pages as well.

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 24668
Build 23440: arc lint + arc unit

Event Timeline

markj created this revision.Jun 3 2019, 3:32 PM
kib accepted this revision.Jun 3 2019, 4:27 PM
This revision is now accepted and ready to land.Jun 3 2019, 4:27 PM

There is some code in the TTM that I wish to remove following this commit. What's the right way to conditionalize that code? __FreeBSD_version is too heavyweight IMO.

hselasky accepted this revision.Jun 3 2019, 5:08 PM

Looks good.

I'm guessing we would also have to revert this recent fix https://github.com/FreeBSDDesktop/kms-drm/commit/4d4b2baf9cbc22588fd379e158c717030bce0d29 ?
We could use LINUXKPI_VERSION if you want to do something like this:

// gfp.h
static inline struct page *
alloc_page(gfp_t flags)
{

#if defined(LINUXKPI_VERSION) && LINUXKPI_VERSION >= 50001
return (linux_alloc_pages(flags & WIRE, 0));
#else
return (linux_alloc_pages(flags, 0));
#endif
}
This way nothing breaks until drivers are updated ( for drm 13-CURRENT supports two branches, 4.16 and 5.0, we need to make sure neither breaks).
LINUXKPI_VERSION macro was added to allow updates to linuxkpi without breaking existing clients. Drivers that use linuxkpi define the version they need. Once all clients are up to the latest linuxkpi version we can remove the condition.

markj added a comment.Jun 3 2019, 6:37 PM

I'm guessing we would also have to revert this recent fix https://github.com/FreeBSDDesktop/kms-drm/commit/4d4b2baf9cbc22588fd379e158c717030bce0d29 ?

Yes. There is similar code in the TTM that I was going to remove as well.

We could use LINUXKPI_VERSION if you want to do something like this:
// gfp.h
static inline struct page *
alloc_page(gfp_t flags)
{
#if defined(LINUXKPI_VERSION) && LINUXKPI_VERSION >= 50001
return (linux_alloc_pages(flags & WIRE, 0));
#else
return (linux_alloc_pages(flags, 0));
#endif
}
This way nothing breaks until drivers are updated ( for drm 13-CURRENT supports two branches, 4.16 and 5.0, we need to make sure neither breaks).
LINUXKPI_VERSION macro was added to allow updates to linuxkpi without breaking existing clients. Drivers that use linuxkpi define the version they need. Once all clients are up to the latest linuxkpi version we can remove the condition.

I want the same behaviour for both 4.16 and 5.0, so this doesn't quite work: as far as I can see, 4.16 doesn't define LINUXKPI_VERSION.

johalun added a comment.EditedJun 3 2019, 6:39 PM

I want the same behaviour for both 4.16 and 5.0, so this doesn't quite work: as far as I can see, 4.16 doesn't define LINUXKPI_VERSION.

It doesn't define it because 4.16 is the fallback/default. LINUXKPI_VERSION was introduced with 5.0.

markj updated this revision to Diff 58201.Jun 3 2019, 7:43 PM

Predicate the change on LINUXKPI_VERSION.

Use the new behaviour for DRM 4.16 since we do not have code
relying on the old behaviour.

This revision now requires review to proceed.Jun 3 2019, 7:43 PM
johalun added inline comments.Jun 3 2019, 7:49 PM
sys/compat/linuxkpi/common/src/linux_page.c
96

Any LINUXKPI_VERSION checks would have to go in the header files since drm drivers, who declare it, are built from ports... That's why my example was for change in gfp.h.

markj updated this revision to Diff 58244.Jun 4 2019, 7:53 PM

Move conditional logic to gfp.h.

markj updated this revision to Diff 58246.Jun 4 2019, 7:57 PM

More typos.

markj added inline comments.Jun 4 2019, 7:58 PM
sys/compat/linuxkpi/common/src/linux_page.c
96

I see now, sorry. I implemented a NOTWIRED flag, so that when 50000 support is dropped we can just delete all of the added code.

johalun accepted this revision.Jun 4 2019, 8:07 PM

Elegant solution! I will ponder a bit over where's the best place to declare LINUXKPI_VERSION in the DRM repo. The best of course would be to have it in one single place, like the root makefile since both drm drivers and linuxkpi_gplv2 depends on it and there's no guarantee that a certain linux header is always included.

This revision is now accepted and ready to land.Jun 4 2019, 8:07 PM
This revision was automatically updated to reflect the committed changes.

Hi

Can you MFC to stable/12 as well?

markj added a comment.Jun 6 2019, 4:58 PM

Hi
Can you MFC to stable/12 as well?

Yes, I put an MFC note in the commit.