| Message ID | 20220629175255.1377052-1-ammar.faizi@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | aarch64 support | expand |
On 6/30/22 12:58 AM, Ammar Faizi wrote: > From: Ammar Faizi <ammarfaizi2@gnuweeb.org> > > Hi Jens, > > This is v2 revision of aarch64 support. > > This series contains nolibc support for aarch64 and one extra irrelevant > cleanup (patch #1). The missing bit from aarch64 is get_page_size() > which is a bit complicated to implement without libc. > > aarch64 supports three values of page size: 4K, 16K, and 64K which are > selected at kernel compilation time. Therefore, we can't hard code the > page size for this arch. In this series we utilize open(), read() and > close() syscall to find the page size from /proc/self/auxv. > > The auxiliary vector contains information about the page size, it is > located at `AT_PAGESZ` keyval pair. This no longer applies, I will send v3 revision soon. If you have some comments, let me know so I can address it together with the rebase.
On 7/4/22 6:52 AM, Ammar Faizi wrote: > On 6/30/22 12:58 AM, Ammar Faizi wrote: >> From: Ammar Faizi <ammarfaizi2@gnuweeb.org> >> >> Hi Jens, >> >> This is v2 revision of aarch64 support. >> >> This series contains nolibc support for aarch64 and one extra irrelevant >> cleanup (patch #1). The missing bit from aarch64 is get_page_size() >> which is a bit complicated to implement without libc. >> >> aarch64 supports three values of page size: 4K, 16K, and 64K which are >> selected at kernel compilation time. Therefore, we can't hard code the >> page size for this arch. In this series we utilize open(), read() and >> close() syscall to find the page size from /proc/self/auxv. >> >> The auxiliary vector contains information about the page size, it is >> located at `AT_PAGESZ` keyval pair. > > This no longer applies, I will send v3 revision soon. If you have some > comments, let me know so I can address it together with the rebase. Just send a v3, didn't have time to go fully over it yet. One note, though - for patch 5, I'd split get_page_size() into two pieces so you just do: static inline long get_page_size(void) { static long cache_val; if (cache_val) return cache_val; return __get_page_size(); } With that, we can have __get_page_size() just do that one thing, open the file and read the value. No need to init static variables to 0. And finally, if the read/open/whatever fails in __get_page_size(), assign cache_val to the fallback value as well. I don't see a point in retrying the same operation later and expect a different result.
On 7/4/22 8:05 PM, Jens Axboe wrote: > On 7/4/22 6:52 AM, Ammar Faizi wrote: >> This no longer applies, I will send v3 revision soon. If you have some >> comments, let me know so I can address it together with the rebase. > > Just send a v3, didn't have time to go fully over it yet. One note, > though - for patch 5, I'd split get_page_size() into two pieces so you > just do: > > static inline long get_page_size(void) > { > static long cache_val; > > if (cache_val) > return cache_val; > > return __get_page_size(); > } > > With that, we can have __get_page_size() just do that one thing, open > the file and read the value. > > No need to init static variables to 0. > > And finally, if the read/open/whatever fails in __get_page_size(), > assign cache_val to the fallback value as well. I don't see a point in > retrying the same operation later and expect a different result. OK, I got the idea, folded that in. Also, it seems we don't have any test that hits that get_page_size() path. Do we? I am going to create a new test: test/nolibc.c That file will test the nolibc functionality, let's do get_page_size() for starting. We can compare the result with a libc function like: long a = sysconf(_SC_PAGESIZE); long b = get_page_size(); if (a != b) { fprintf(stderr, "get_page_size() fails, %ld != %ld", a, b); return T_EXIT_FAIL; }