lkml.org 
[lkml]   [2020]   [May]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v2 03/10] kexec: separate PageHighMem() and PageHighMemZone() use case
    js1304@gmail.com writes:

    > From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
    >
    > Until now, PageHighMem() is used for two different cases. One is to check
    > if there is a direct mapping for this page or not. The other is to check
    > the zone of this page, that is, weather it is the highmem type zone or not.
    >
    > Now, we have separate functions, PageHighMem() and PageHighMemZone() for
    > each cases. Use appropriate one.
    >
    > Note that there are some rules to determine the proper macro.
    >
    > 1. If PageHighMem() is called for checking if the direct mapping exists
    > or not, use PageHighMem().
    > 2. If PageHighMem() is used to predict the previous gfp_flags for
    > this page, use PageHighMemZone(). The zone of the page is related to
    > the gfp_flags.
    > 3. If purpose of calling PageHighMem() is to count highmem page and
    > to interact with the system by using this count, use PageHighMemZone().
    > This counter is usually used to calculate the available memory for an
    > kernel allocation and pages on the highmem zone cannot be available
    > for an kernel allocation.
    > 4. Otherwise, use PageHighMemZone(). It's safe since it's implementation
    > is just copy of the previous PageHighMem() implementation and won't
    > be changed.
    >
    > I apply the rule #2 for this patch.

    Hmm.

    What happened to the notion of deprecating and reducing the usage of
    highmem? I know that we have some embedded architectures where it is
    still important but this feels like it flies in the face of that.


    This part of kexec would be much more maintainable if it had a proper
    mm layer helper that tested to see if the page matched the passed in
    gfp flags. That way the mm layer could keep changing and doing weird
    gyrations and this code would not care.


    What would be really helpful is if there was a straight forward way to
    allocate memory whose physical address fits in the native word size.


    All I know for certain about this patch is that it takes a piece of code
    that looked like it made sense, and transfroms it into something I can
    not easily verify, and can not maintain.

    As it makes the code unmaintainable.
    Nacked-by: "Eric W. Biederman" <ebiederm@xmission.com>


    Not to say that the code isn't questionable as it is, but this change just
    pushes it over the edge into gobbledy gook.

    Eric


    > Acked-by: Roman Gushchin <guro@fb.com>
    > Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
    > ---
    > kernel/kexec_core.c | 2 +-
    > 1 file changed, 1 insertion(+), 1 deletion(-)
    >
    > diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
    > index ba1d91e..33097b7 100644
    > --- a/kernel/kexec_core.c
    > +++ b/kernel/kexec_core.c
    > @@ -766,7 +766,7 @@ static struct page *kimage_alloc_page(struct kimage *image,
    > * gfp_flags honor the ones passed in.
    > */
    > if (!(gfp_mask & __GFP_HIGHMEM) &&
    > - PageHighMem(old_page)) {
    > + PageHighMemZone(old_page)) {
    > kimage_free_pages(old_page);
    > continue;
    > }

    \
     
     \ /
      Last update: 2020-05-01 16:07    [W:3.814 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site