Page MenuHomeFreeBSD
Differential D58298

LinuxKPI: page.h: use atop() and ptoa() instead of PAGE_SHIFT
Needs ReviewPublic
Actions

Authored by bz on Fri, Jul 17, 1:10 PM.

Details

Reviewers
markj
kib
Group Reviewers
linuxkpi
Summary

With upcoming changes to 'struct page' this will make the lines
easier to read by using the predefined macros from param.h.

Sponsored by: The FreeBSD Foundation
MFC after: 3 days

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 74887
Build 71770: arc lint + arc unit

Event Timeline

bz created this revision.Fri, Jul 17, 1:10 PM
Herald added subscribers: emaste, imp. · View Herald TranscriptFri, Jul 17, 1:10 PM
bz requested review of this revision.Fri, Jul 17, 1:10 PM
Harbormaster completed remote builds in B74872: Diff 182117.Fri, Jul 17, 1:10 PM
bz added a child revision: D58299: LinuxKPI: page.h: resort lines.Fri, Jul 17, 1:18 PM
bz added a parent revision: D58297: LinuxKPI: prefer struct page [*] over struct vm_page[_t].
markj added a comment.Fri, Jul 17, 1:33 PM

Fine, but this introduces a dependency on sys/param.h wherever these macros are used. Is it ok to include that file into Linux sources in general?

bz added a comment.EditedFri, Jul 17, 1:38 PM
In D58298#1337032, @markj wrote:

Fine, but this introduces a dependency on sys/param.h wherever these macros are used. Is it ok to include that file into Linux sources in general?

% grep -r sys/param.h sys/compat/linuxkpi/common/include/ | wc -l
      41

and that includes gfp.h pci.h types.h and kernel.h

So, yes, I believe so :)

Also #include <sys/param.h> is already on the file so no change of status quo.

bz added inline comments.Fri, Jul 17, 1:39 PM
sys/compat/linuxkpi/common/include/linux/page.h
34

sys/param.h is here already.

kib accepted this revision.Fri, Jul 17, 1:47 PM

For the discussion, I am still somewhat surprised that so much of the FreeBSD symbols are leaked into linuxkpi headers, Most amusing and dangerous is PAGE_MASK (AFAIR).

This revision is now accepted and ready to land.Fri, Jul 17, 1:47 PM
markj accepted this revision.Fri, Jul 17, 1:47 PM
markj added inline comments.
sys/compat/linuxkpi/common/include/linux/page.h
34

Oops, I missed it.

bz added a comment.Fri, Jul 17, 1:55 PM
In D58298#1337045, @kib wrote:

For the discussion, I am still somewhat surprised that so much of the FreeBSD symbols are leaked into linuxkpi headers, Most amusing and dangerous is PAGE_MASK (AFAIR).

.. and a lot of that is "old" stuff .. from the first days (again here 2011).
I cannot remember how many bits of those assumptions and short cuts I was forced to clean up in the last 5 years in order to move forward. We'll continue that journey, I am sure (and all for it).

I was contemplating migrating the above out of the header but given things will hopefully change soon I left that for now.

What's also scary is all the semi-native code (which most perceive as native driver/code) which relies in parts of the old shortcuts in LinuxKPI... *sigh*.

bz marked an inline comment as done.Fri, Jul 17, 1:58 PM
bz added inline comments.
sys/compat/linuxkpi/common/include/linux/page.h
78

And before anyone starts counting I removed the extra ( locally already.

bz planned changes to this revision.Fri, Jul 17, 2:01 PM
bz added inline comments.
sys/compat/linuxkpi/common/include/linux/page.h
78

ARGH. I'll upload a new version of this. The above is wrong!

bz updated this revision to Diff 182140.Fri, Jul 17, 3:48 PM

Properly nest calls.

Harbormaster completed remote builds in B74887: Diff 182140.Fri, Jul 17, 3:48 PM

Revision Contents
Changeset List

PathSize
sys/
compat/
linuxkpi/
common/
include/
linux/
4 lines

Diff 182140

View Options

sys/compat/linuxkpi/common/include/linux/page.h

Loading...