Re: [PATCH liburing v3 05/10] arch/aarch64: lib: Add `get_page_size()` function

[Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>]

On Tue, Jul 5, 2022 at 12:54 AM Ammar Faizi wrote:
> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
>
> A prep patch to add aarch64 nolibc support.
>
> 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. Utilize open(), read() and close() syscall to
> find the page size from /proc/self/auxv. For more details about the
> auxv data structure, check the link below [1].
>
> v3:
>   - Split open/read/close in get_page_size() into a new function.
>   - Cache the fallback value when we fail on the syscalls.
>   - No need to init the static var to zero.
>
> v2:
>   - Fallback to 4K if the syscall fails.
>   - Cache the page size after read as suggested by Jens.
>
> Link: https://github.com/torvalds/linux/blob/v5.19-rc4/fs/binfmt_elf.c#L260 [1]
> Link: https://lore.kernel.org/io-uring/3895dbe1-8d5f-cf53-e94b-5d1545466de1@kernel.dk
> Link: https://lore.kernel.org/io-uring/8bfba71c-55d7-fb49-6593-4d0f9d9c3611@kernel.dk
> Link: https://lore.kernel.org/io-uring/49ed1c4c-46ca-15c4-f288-f6808401b0ff@kernel.dk
> Suggested-by: Jens Axboe <axboe@kernel.dk>
> Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
[...]
> +static inline long __get_page_size(void)
> +{
> +       static const long fallback_ret = 4096;
> +       Elf64_Off buf[2];
> +       long ret;
> +       int fd;
> +
> +       fd = __sys_open("/proc/self/auxv", O_RDONLY, 0);
> +       if (fd < 0)
> +               return fallback_ret;
> +
> +       while (1) {
> +               ssize_t x;
> +
> +               x = __sys_read(fd, buf, sizeof(buf));
> +               if (x < sizeof(buf)) {
> +                       ret = fallback_ret;
> +                       break;
> +               }
> +
> +               if (buf[0] == AT_PAGESZ) {
> +                       ret = buf[1];
> +                       break;
> +               }
> +       }
> +
> +       __sys_close(fd);
> +       return ret;
> +}

fallback_ret var is not needed, just do this, simpler:

static inline long __get_page_size(void)
{
        Elf64_Off buf[2];
        long ret = 4096;
        int fd;

        fd = __sys_open("/proc/self/auxv", O_RDONLY, 0);
        if (fd < 0)
                return ret;

        while (1) {
                ssize_t x;

                x = __sys_read(fd, buf, sizeof(buf));
                if (x < sizeof(buf))
                        break;

                if (buf[0] == AT_PAGESZ) {
                        ret = buf[1];
                        break;
                }
        }

        __sys_close(fd);
        return ret;
}

with that simplification:

Reviewed-by: Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>

tq

-- Viro