On 01/15/18 09:06, Konstantin Belousov wrote:
On Mon, Jan 15, 2018 at 07:33:01AM -0800, Nathan Whitehorn wrote:
On 01/15/18 03:18, Konstantin Belousov wrote:
On Sun, Jan 14, 2018 at 03:46:38PM -0800, Nathan Whitehorn wrote:
On 01/14/18 15:42, Nathan Whitehorn wrote:
On 01/14/18 09:57, Nathan Whitehorn wrote:
On 01/14/18 09:52, Konstantin Belousov wrote:
On Sun, Jan 14, 2018 at 09:30:53AM -0800, Nathan Whitehorn wrote:
The immediate consequence of that is that no MI code that knows about
direct maps can possibly take advantage of the direct map on this
platform. Do we really want that to save some conditional logic that
would get optimized out on amd64 and arm64 anyway? I really do not see
the benefit here.
It is not clear what do you mean. Are you saying that there is no
benefit
of providing the conditional logic, or that it is not benefit of
exclusing
powerpc ?
Sorry, that was poorly stated. Let me try again:
If we make a PPC_PHYS_TO_DMAP(), but there is an MI PHYS_TO_DMAP()
API, consumer code in the MI parts of the kernel won't be able to
benefit from the PPC direct map, which seems unfortunate. The cost
from a code perspective of having an if (direct_map_available) seems
low, since on systems where direct_map_available is defined to be 1,
the compiler will optimize it to the same code as if gated by #ifdef.
It might be more cumbersome to write the code, however.
I do not object against adding the conditional, but it should not be
too clumsy to use.
OK. Let me try to draft something in the next couple days and see how
much of a pain it is in practice.
-Nathan
How about the attached? It makes PHYS_TO_DMAP() return 0 if no mapping
exists. This is straightforward, does not introduce extra macros, and
can pretty easily replace SFBUF_OPTIONAL_DIRECT_MAP on the assumption
that PHYS_TO_DMAP() is cheap. I've modified the other MI-ish consumers
in the tree accordingly; compat/linuxkpi/common/src/linux_page.c
already does the right thing and needed no modifications.
-Nathan
I think that this is fine from the PoV of code complexity.
We now require MI (but not amd64 and arm64 MD) code to check for
PHYS_TO_DMAP() return value, which is redundand for a*64. I am not sure
if this is good choice from the PoV of possible microoptimizations.
You promised something which is trivially detectable by compiler as
an excess code.
Fair enough -- the logic was that a lot of code already checks for NULL
pointers (the linux_page.c for instance required no changes to do the
right thing).
Most likely this is an accidental feature of the linux code and not the
specific decision by the freebsd emulation of it.
If we want it to be fully compiler-transparent, we could
also add a flag, but that would add more code complexity. Do you have a
preference? I would be happy to draft that too.
I think I am fine with amd64 doing
#define PMAP_HAS_DMAP 1
in machine/param.h. I do not insist on the name.
Then ppc could define its version as a reference to the variable. I thought
that might be you can create less clumsy model of propagating this to the
MI VM level.
That seems fine to me. I don't think a less-clumsy way that does not
involve extra indirection is possible. The PHYS_TO_DMAP() returning NULL
is about the best thing I can come up with from a clumsiness standpoint
since plenty of code checks for null pointers already, but doesn't
cleanly handle the rarer case where you want to test for the existence
of direct maps in general without testing some potemkin address.
My one reservation about PMAP_HAS_DMAP or the like as a selector is that
it does not encode the full shape of the problem: one could imagine
having a direct map that only covers a limited range of RAM (I am not
sure whether the existence of dmaplimit on amd64 implies this can happen
with non-device memory in real life), for example. These cases are
currently covered by an assert() in PHYS_TO_DMAP(), whereas having
PHYS_TO_DMAP() return NULL allows a more flexible signalling and the
potential for the calling code to do something reasonable to handle the
error. A single global flag can't convey information at this kind of
granularity. Is this a reasonable concern? Or am I overthinking things?
-Nathan
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
