linux-kernel - Re: [PATCH v5 07/14] mm, compaction: khugepaged should not give up due to need

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:	Tue, 29 Jul 2014 15:57:41 -0700 (PDT)
From:	David Rientjes <rientjes@...gle.com>
To:	Vlastimil Babka <vbabka@...e.cz>
cc:	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 07/14] mm, compaction: khugepaged should not give up
 due to need_resched()

On Tue, 29 Jul 2014, Vlastimil Babka wrote:

> > > diff --git a/include/linux/compaction.h b/include/linux/compaction.h
> > > index b2e4c92..60bdf8d 100644
> > > --- a/include/linux/compaction.h
> > > +++ b/include/linux/compaction.h
> > > @@ -13,6 +13,14 @@
> > >   /* The full zone was compacted */
> > >   #define COMPACT_COMPLETE	4
> > > 
> > > +/* Used to signal whether compaction detected need_sched() or lock
> > > contention */
> > > +/* No contention detected */
> > > +#define COMPACT_CONTENDED_NONE	0
> > > +/* Either need_sched() was true or fatal signal pending */
> > > +#define COMPACT_CONTENDED_SCHED	1
> > > +/* Zone lock or lru_lock was contended in async compaction */
> > > +#define COMPACT_CONTENDED_LOCK	2
> > > +
> > 
> > Make this an enum?
> 
> I tried originally, but then I would have to define it elsewhere
> (mm/internal.h I think) together with compact_control. I didn't think it was
> worth the extra pollution of shared header, when the return codes are also
> #define and we might still get rid of need_resched() one day.
> 

Ok.

[...]

> > > @@ -2660,15 +2660,36 @@ rebalance:
> > >   	if (!(gfp_mask & __GFP_NO_KSWAPD) || (current->flags & PF_KTHREAD))
> > >   		migration_mode = MIGRATE_SYNC_LIGHT;
> > > 
> > > -	/*
> > > -	 * If compaction is deferred for high-order allocations, it is because
> > > -	 * sync compaction recently failed. In this is the case and the caller
> > > -	 * requested a movable allocation that does not heavily disrupt the
> > > -	 * system then fail the allocation instead of entering direct reclaim.
> > > -	 */
> > > -	if ((deferred_compaction || contended_compaction) &&
> > > -						(gfp_mask & __GFP_NO_KSWAPD))
> > > -		goto nopage;
> > 
> > Hmm, this check will have unfortunately changed in the latest mmotm due to
> > mm-thp-restructure-thp-avoidance-of-light-synchronous-migration.patch.
> 
> I think you were changing (and moving around) a different check so there would
> be merge conflicts but no semantic problem.
> 

The idea is the same, though, I think the check should not rely on 
__GFP_NO_KSWAPD and rather rely on 
(gfp_mask & GFP_TRANSHUGE) == GFP_TRANSHUGE.  In other words, all the 
possibilities under your new test for gfp_mask & __GFP_NO_KSWAPD are thp 
specific and not for the other allocators who pass __GFP_NO_KSWAPD.  This 
patch would be a significant change in logic for those users that doesn't 
seem helpful.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/