diff mbox series

[09/12] selftests/mm: move psize(), pshift() into vm_utils.c

Message ID 20230602013358.900637-10-jhubbard@nvidia.com
State New
Headers
Series A minor flurry of selftest/mm fixes |

Commit Message

John Hubbard June 2, 2023, 1:33 a.m. UTC 
  This is in preparation for linking test programs with both vm_utils.c
and uffd-common.c. The static inline routines would prevent that, and
there is no particular need for inlining here, so turn these into normal
functions that are more flexible to build and link.

Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 tools/testing/selftests/mm/vm_util.c | 14 ++++++++++++++
 tools/testing/selftests/mm/vm_util.h | 16 ++--------------
 2 files changed, 16 insertions(+), 14 deletions(-)

Comments

David Hildenbrand June 2, 2023, 10:19 a.m. UTC | #1 
On 02.06.23 03:33, John Hubbard wrote:
> This is in preparation for linking test programs with both vm_utils.c
> and uffd-common.c. The static inline routines would prevent that, and
> there is no particular need for inlining here, so turn these into normal
> functions that are more flexible to build and link.

I'm probably missing something important, but isn't it the most common 
thing to use "static inline" across multiple objects that we then link?

Hope you can enlighten me what the real issue here is.
John Hubbard June 2, 2023, 9:58 p.m. UTC | #2 
On 6/2/23 03:19, David Hildenbrand wrote:
> On 02.06.23 03:33, John Hubbard wrote:
>> This is in preparation for linking test programs with both vm_utils.c
>> and uffd-common.c. The static inline routines would prevent that, and
>> there is no particular need for inlining here, so turn these into normal
>> functions that are more flexible to build and link.
> 
> I'm probably missing something important, but isn't it the most common thing to use "static inline" across multiple objects that we then link?

Yes, absolutely. I've just had my confidence in things shaken by some
of the quirks in the selftests framework, but you're right.

> 
> Hope you can enlighten me what the real issue here is.
> 

At this point it looks like a header file inclusion mess. As usual,
including header files from other header files leads to problems,
and in this case I'm now seeing vm_utils.h included from uffd-common.h,
causing multiple definitions of these static inline functions.

If I try to undo that it generates a boatload of new errors, so I'm
thinking to just change the commit message to explain that I'm moving
this to a normal function in order to avoid the above situation.

thanks,
diff mbox series

Patch

diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/selftests/mm/vm_util.c
index 9b06a5034808..01296c17df02 100644
--- a/tools/testing/selftests/mm/vm_util.c
+++ b/tools/testing/selftests/mm/vm_util.c
@@ -301,3 +301,17 @@  int uffd_get_features(uint64_t *features)
 
 	return 0;
 }
+
+unsigned int psize(void)
+{
+	if (!__page_size)
+		__page_size = sysconf(_SC_PAGESIZE);
+	return __page_size;
+}
+
+unsigned int pshift(void)
+{
+	if (!__page_shift)
+		__page_shift = (ffsl(psize()) - 1);
+	return __page_shift;
+}
diff --git a/tools/testing/selftests/mm/vm_util.h b/tools/testing/selftests/mm/vm_util.h
index b950bd16083a..232ffeb5805c 100644
--- a/tools/testing/selftests/mm/vm_util.h
+++ b/tools/testing/selftests/mm/vm_util.h
@@ -3,7 +3,6 @@ 
 #include <stdbool.h>
 #include <sys/mman.h>
 #include <err.h>
-#include <string.h> /* ffsl() */
 #include <unistd.h> /* _SC_PAGESIZE */
 
 #define BIT_ULL(nr)                   (1ULL << (nr))
@@ -17,19 +16,8 @@ 
 extern unsigned int __page_size;
 extern unsigned int __page_shift;
 
-static inline unsigned int psize(void)
-{
-	if (!__page_size)
-		__page_size = sysconf(_SC_PAGESIZE);
-	return __page_size;
-}
-
-static inline unsigned int pshift(void)
-{
-	if (!__page_shift)
-		__page_shift = (ffsl(psize()) - 1);
-	return __page_shift;
-}
+unsigned int psize(void);
+unsigned int pshift(void);
 
 uint64_t pagemap_get_entry(int fd, char *start);
 bool pagemap_is_softdirty(int fd, char *start);