linux-kernel - Re: [PATCH] mm: initialize deferred pages with interrupts enabled

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 1 Apr 2020 16:31:29 -0400
From:   Pavel Tatashin <pasha.tatashin@...een.com>
To:     Daniel Jordan <daniel.m.jordan@...cle.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Michal Hocko <mhocko@...e.com>, linux-mm <linux-mm@...ck.org>,
        Dan Williams <dan.j.williams@...el.com>,
        Shile Zhang <shile.zhang@...ux.alibaba.com>,
        Kirill Tkhai <ktkhai@...tuozzo.com>,
        David Hildenbrand <david@...hat.com>,
        James Morris <jmorris@...ei.org>,
        Sasha Levin <sashal@...nel.org>
Subject: Re: [PATCH] mm: initialize deferred pages with interrupts enabled

On Wed, Apr 1, 2020 at 4:10 PM Daniel Jordan <daniel.m.jordan@...cle.com> wrote:
>
> On Wed, Apr 01, 2020 at 04:00:27PM -0400, Daniel Jordan wrote:
> > On Wed, Apr 01, 2020 at 03:32:38PM -0400, Pavel Tatashin wrote:
> > > Initializing struct pages is a long task and keeping interrupts disabled
> > > for the duration of this operation introduces a number of problems.
> > >
> > > 1. jiffies are not updated for long period of time, and thus incorrect time
> > >    is reported. See proposed solution and discussion here:
> > >    lkml/20200311123848.118638-1-shile.zhang@...ux.alibaba.com
> > > 2. It prevents farther improving deferred page initialization by allowing
> >
> >                                                                    not allowing
> > >    inter-node multi-threading.
> >
> >      intra-node
> >
> > ...
> > > After:
> > > [    1.632580] node 0 initialised, 12051227 pages in 436ms
> >
> > Fixes: 3a2d7fa8a3d5 ("mm: disable interrupts while initializing deferred pages")
> > Reported-by: Shile Zhang <shile.zhang@...ux.alibaba.com>
> >
> > > Signed-off-by: Pavel Tatashin <pasha.tatashin@...een.com>
> >
> > Freezing jiffies for a while during boot sounds like stable to me, so
> >
> > Cc: <stable@...r.kernel.org>    [4.17.x+]
> >
> >
> > Can you please add a comment to mmzone.h above node_size_lock, something like
> >
> >          * Must be held any time you expect node_start_pfn,
> >          * node_present_pages, node_spanned_pages or nr_zones to stay constant.
> > +        * Also synchronizes pgdat->first_deferred_pfn during deferred page
> > +        * init.
> >          ...
> >         spinlock_t node_size_lock;
> >
> > > @@ -1854,18 +1859,6 @@ deferred_grow_zone(struct zone *zone, unsigned int order)
> > >             return false;
> > >
> > >     pgdat_resize_lock(pgdat, &flags);
> > > -
> > > -   /*
> > > -    * If deferred pages have been initialized while we were waiting for
> > > -    * the lock, return true, as the zone was grown.  The caller will retry
> > > -    * this zone.  We won't return to this function since the caller also
> > > -    * has this static branch.
> > > -    */
> > > -   if (!static_branch_unlikely(&deferred_pages)) {
> > > -           pgdat_resize_unlock(pgdat, &flags);
> > > -           return true;
> > > -   }
> > > -
> >
> > Huh, looks like this wasn't needed even before this change.
> >
> >
> > The rest looks fine.
> >
> > Reviewed-by: Daniel Jordan <daniel.m.jordan@...cle.com>
>
> ...except for I forgot about the touch_nmi_watchdog() calls.  I think you'd
> need something kind of like this before your patch.

Thank you for review. You are right, I will add your patch, and modify
my to change touch_nmi_watchdog() to cond_resched().

Pasha