Page MenuHomeFreeBSD

Create a new macro for static DPCPU data
ClosedPublic

Authored by andrew on Jul 5 2018, 1:35 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Jun 3, 2:49 PM
Unknown Object (File)
Fri, May 31, 7:02 PM
Unknown Object (File)
May 3 2024, 12:09 PM
Unknown Object (File)
Apr 25 2024, 7:44 AM
Unknown Object (File)
Apr 24 2024, 6:26 AM
Unknown Object (File)
Apr 23 2024, 11:26 AM
Unknown Object (File)
Apr 19 2024, 6:00 AM
Unknown Object (File)
Mar 30 2024, 9:52 PM

Details

Summary

On arm64 (and possible other architectures) we are unable to use static
DPCPU data in kernel modules. This is because the compiler will generate
PC-relative accesses, however the runtime-linker expects to be able to
relocate these.

In preperation to fix this create two macros depending on if teh data
is global, or static.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This revision is now accepted and ready to land.Jul 5 2018, 1:44 PM
This revision now requires review to proceed.Jul 5 2018, 1:54 PM

Add you two for the DPCPU (and later VNET) static file local load relative issue (1st step) solution.

I'm fine with this as a NFC first step

This revision is now accepted and ready to land.Jul 5 2018, 2:02 PM

Looks like dpcpu(9) needs to be updated.

What about (what do others think):

  • leave the non static and not rename them to *_GLOBAL_*
  • rename the static to STATIC_DPCPU_DEFINE

That way grep()ing for DPCPU_DEFINE will still give us all of them, there's less noise in the change, and only the problematic case is (will be) dealt with.

As @andrew will point out his version will make people less likely put static before a DPCPU_GLOBAL_DEFINE(); not sure if that's worth the trouble.

A similar change for VNET will be about 250 globals (touched or untouched) and 350 statics that will need changing.

I should add that should we find a more elegant solution where we can automatically handle the cases using the compiler/linker bits, we would have to undo less?

Discuss!

In D16140#342262, @bz wrote:

What about (what do others think):

  • leave the non static and not rename them to *_GLOBAL_*
  • rename the static to STATIC_DPCPU_DEFINE

That way grep()ing for DPCPU_DEFINE will still give us all of them, there's less noise in the change, and only the problematic case is (will be) dealt with.

I like this better, but think it should be DPCPU_DEFINE_STATIC instead.

As @andrew will point out his version will make people less likely put static before a DPCPU_GLOBAL_DEFINE(); not sure if that's worth the trouble.

Can we define the macro so that the use of "static" results in a compile error? e.g.,

#define DPCPU_DEFINE(t, n) struct __hack; t DPCPU_NAME(n) ...

A similar change for VNET will be about 250 globals (touched or untouched) and 350 statics that will need changing.

I should add that should we find a more elegant solution where we can automatically handle the cases using the compiler/linker bits, we would have to undo less?

Discuss!

In D16140#342262, @bz wrote:

What about (what do others think):

  • leave the non static and not rename them to *_GLOBAL_*
  • rename the static to STATIC_DPCPU_DEFINE

That way grep()ing for DPCPU_DEFINE will still give us all of them, there's less noise in the change, and only the problematic case is (will be) dealt with.

Hrm, I can see the logic in that approach. What about DPCPU_DEFINE_STATIC or DPCPU_DEFINE_LOCAL instead, for the static-including one?

As @andrew will point out his version will make people less likely put static before a DPCPU_GLOBAL_DEFINE(); not sure if that's worth the trouble.

This seems like something we could include in a set of some sort of local lint rules; anyway I don't see it as a big deal - most likely folks adding new ones will cut and paste an existing example.

Rename the static macro to DPCPU_DEFINE_STATIC
Keep DPCPU_DEFINE, but make it a compile time error to use it with static

Stil todo: Update the man page

This revision now requires review to proceed.Jul 5 2018, 3:32 PM
sys/sys/pcpu.h
84 ↗(On Diff #44912)

maybe a comment to explain the use of struct _hack;

Happy with those bits if you do the man page as you say :)

This revision is now accepted and ready to land.Jul 5 2018, 3:36 PM

Add a comment on why struct _hack
Update the man page

This revision now requires review to proceed.Jul 5 2018, 4:18 PM
This revision is now accepted and ready to land.Jul 5 2018, 4:31 PM

do whatever you want with the comment and commit

This revision was automatically updated to reflect the committed changes.