Improve 'inconsistent memattr' logging.

[kib]

Summary

Avoid duplicated calls to pmap_page_get_memattr().

Avoid logging inconsistency for the /dev/mem device at all. The driver conscientiously ignores memattr, and the corrective action in the device pager handles it correctly.

In the message, name the driver we blame, and show memory attributes values.

[alc]

Can you elaborate on what you mean by "driver conscientiously ignores memattr"? At least, in cases where we read or write through the direct map, the direct map should already have the correct memory attribute set.

sys/vm/device_pager.c

331 ↗

(On Diff #15731)

Could you add a comment to explain why /dev/mem is a special case?

[kib]

In D6149#130933, @alc wrote:

Can you elaborate on what you mean by "driver conscientiously ignores memattr"? At least, in cases where we read or write through the direct map, the direct map should already have the correct memory attribute set.

I mean, the mem driver keeps the value of the memattr passed to the driver d_mmap method, intact. The driver does not query the pmap for the current mapping mode of the page, to return in the *memattr. This is what causes the warnings printed.

OTOH, doing it in the mem driver would just duplicate the code which already exists in the device_pager.c, so the driver (correctly) does nothing in this regard, which I meant by '... conscientiously ignores ...'.

I agree that DMAP already has the right mapping mode, just that the warning is useless.

[pho]

Tested on i386 and amd64 for a day. The WARNING is gone and I have not observed any issues with this patch.

[kib]

Add comment about D_MEM exclusion.

[alc]

There are at least two other drivers that set D_MEM: dev/firewire and i386/bios/smapi.c. However, dev/firewire doesn't support mmap. Are you sure that testing D_MEM is always going to be correct? Should this code really be checking the d_name field for "mem"?

sys/vm/device_pager.c

332 ↗

(On Diff #15760)

"For the /dev/mem ...

333 ↗

(On Diff #15760)

"memattr, pmap_page_get_memattr() ...

[kib]

In D6149#131214, @alc wrote:

There are at least two other drivers that set D_MEM: dev/firewire and i386/bios/smapi.c. However, dev/firewire doesn't support mmap. Are you sure that testing D_MEM is always going to be correct? Should this code really be checking the d_name field for "mem"?

I thought that D_MEM specifically denoted /dev/mem (and /dev/kmem). The fact is that D_MEM is not used for anything right now (my added test is the first real use of it), and my impulse is to remove D_MEM from other drivers.

Alternatively, I can add another flag, but I do not see a reason.

[alc]

Remove it from the other drivers. Can you find a way to assert that no other driver uses it? Otherwise, sooner or later, someone will cut-and-paste it from /dev/mem again.

[kib]

Remove D_MEM from firewire and smapi. Improve comments.

[kib]

In D6149#131229, @alc wrote:

Can you find a way to assert that no other driver uses it? Otherwise, sooner or later, someone will cut-and-paste it from /dev/mem again.

I do not see how to assert this without uglyness. Also, I think that other drivers might want to request the behaviour of /dev/mem there.

Instead, I put a comment near the D_MEM definition, hopefully this would make the driver author to pause for a second.