Re: [PATCH 0/5] arm64: vdso: cleanups

Will Deacon
2020-04-14T15:22:12Z

On Tue, Apr 14, 2020 at 11:42:47AM +0100, Mark Rutland wrote:
> While attempting to review an arm64 vdso patch, I noticed some of the existing
> code was somewhat baroque, making it harder than necessary to understand and
> extend. These patches attempt to improve this by making the code more
> consistent and avoiding unnecessary duplication.
>
> The first patch in the series fixes a bug in a boot time error path. This bug
> was made obvious during the refactoring but I've moved it to the start so that
> it can be backported more easily.
>
> The series is based on v5.7-rc1 and can be found in my arm64/vdso-cleanup
> branch [1].

Cheers, this looks really good to me. The only thing I'm slightly confused
by is that we still have something like this in __vdso_init():

/* Allocate the vDSO pagelist, plus a page for the data. */
vdso_pagelist = kcalloc(vdso_lookup[arch_index].vdso_pages + 1,
sizeof(struct page *),
GFP_KERNEL);

But I don't see why this needs to be dynamic, and don't we leak the
allocation on failure? Not a big deal, but seems silly if we could just
have a couple of static page * arrays.

Will