Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1727149 > unrolled thread
| Started by | js1304@gmail.com |
|---|---|
| First post | 2017-09-06 06:40 +0200 |
| Last post | 2017-09-06 19:30 +0200 |
| Articles | 4 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH 1/2] mm/slub: wake up kswapd for initial high order allocation js1304@gmail.com - 2017-09-06 06:40 +0200
Re: [PATCH 1/2] mm/slub: wake up kswapd for initial high order allocation Vlastimil Babka <vbabka@suse.cz> - 2017-09-06 10:10 +0200
Re: [PATCH 1/2] mm/slub: wake up kswapd for initial high order allocation Christopher Lameter <cl@linux.com> - 2017-09-06 18:00 +0200
Re: [PATCH 1/2] mm/slub: wake up kswapd for initial high order allocation Michal Hocko <mhocko@kernel.org> - 2017-09-06 19:30 +0200
| From | js1304@gmail.com |
|---|---|
| Date | 2017-09-06 06:40 +0200 |
| Subject | [PATCH 1/2] mm/slub: wake up kswapd for initial high order allocation |
| Message-ID | <umAi5-4Cl-3@gated-at.bofh.it> |
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
slub uses higher order allocation than it actually needs. In this case,
we don't want to do direct reclaim to make such a high order page since
it causes a big latency to the user. Instead, we would like to fallback
lower order allocation that it actually needs.
However, we also want to get this higher order page in the next time
in order to get the best performance and it would be a role of
the background thread like as kswapd and kcompactd. To wake up them,
we should not clear __GFP_KSWAPD_RECLAIM.
Unlike this intention, current code clears __GFP_KSWAPD_RECLAIM so fix it.
Current unintended code is done by Mel's commit 444eb2a449ef ("mm: thp:
set THP defrag by default to madvise and add a stall-free defrag option")
for slub part. It removes a special case in __alloc_page_slowpath()
where including __GFP_THISNODE and lacking ~__GFP_DIRECT_RECLAIM
effectively means also lacking __GFP_KSWAPD_RECLAIM. However, slub
doesn't use __GFP_THISNODE so it is not the case for this purpose. So,
partially reverting this code in slub doesn't hurt Mel's intention.
Note that this patch does some clean up, too.
__GFP_NOFAIL is cleared twice so remove one.
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
mm/slub.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index 163352c..45f4a4b 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1578,8 +1578,12 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
* so we fall-back to the minimum order allocation.
*/
alloc_gfp = (flags | __GFP_NOWARN | __GFP_NORETRY) & ~__GFP_NOFAIL;
- if ((alloc_gfp & __GFP_DIRECT_RECLAIM) && oo_order(oo) > oo_order(s->min))
- alloc_gfp = (alloc_gfp | __GFP_NOMEMALLOC) & ~(__GFP_RECLAIM|__GFP_NOFAIL);
+ if (oo_order(oo) > oo_order(s->min)) {
+ if (alloc_gfp & __GFP_DIRECT_RECLAIM) {
+ alloc_gfp |= __GFP_NOMEMALLOC;
+ alloc_gfp &= ~__GFP_DIRECT_RECLAIM;
+ }
+ }
page = alloc_slab_page(s, alloc_gfp, node, oo);
if (unlikely(!page)) {
--
2.7.4
[toc] | [next] | [standalone]
| From | Vlastimil Babka <vbabka@suse.cz> |
|---|---|
| Date | 2017-09-06 10:10 +0200 |
| Subject | Re: [PATCH 1/2] mm/slub: wake up kswapd for initial high order allocation |
| Message-ID | <umDzj-7di-9@gated-at.bofh.it> |
| In reply to | #1727149 |
On 09/06/2017 06:37 AM, js1304@gmail.com wrote:
> From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>
> slub uses higher order allocation than it actually needs. In this case,
> we don't want to do direct reclaim to make such a high order page since
> it causes a big latency to the user. Instead, we would like to fallback
> lower order allocation that it actually needs.
>
> However, we also want to get this higher order page in the next time
> in order to get the best performance and it would be a role of
> the background thread like as kswapd and kcompactd. To wake up them,
> we should not clear __GFP_KSWAPD_RECLAIM.
>
> Unlike this intention, current code clears __GFP_KSWAPD_RECLAIM so fix it.
> Current unintended code is done by Mel's commit 444eb2a449ef ("mm: thp:
> set THP defrag by default to madvise and add a stall-free defrag option")
> for slub part. It removes a special case in __alloc_page_slowpath()
> where including __GFP_THISNODE and lacking ~__GFP_DIRECT_RECLAIM
> effectively means also lacking __GFP_KSWAPD_RECLAIM. However, slub
> doesn't use __GFP_THISNODE so it is not the case for this purpose. So,
> partially reverting this code in slub doesn't hurt Mel's intention.
>
> Note that this patch does some clean up, too.
> __GFP_NOFAIL is cleared twice so remove one.
>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
> ---
> mm/slub.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 163352c..45f4a4b 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1578,8 +1578,12 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
> * so we fall-back to the minimum order allocation.
> */
> alloc_gfp = (flags | __GFP_NOWARN | __GFP_NORETRY) & ~__GFP_NOFAIL;
> - if ((alloc_gfp & __GFP_DIRECT_RECLAIM) && oo_order(oo) > oo_order(s->min))
> - alloc_gfp = (alloc_gfp | __GFP_NOMEMALLOC) & ~(__GFP_RECLAIM|__GFP_NOFAIL);
> + if (oo_order(oo) > oo_order(s->min)) {
> + if (alloc_gfp & __GFP_DIRECT_RECLAIM) {
> + alloc_gfp |= __GFP_NOMEMALLOC;
> + alloc_gfp &= ~__GFP_DIRECT_RECLAIM;
> + }
> + }
>
> page = alloc_slab_page(s, alloc_gfp, node, oo);
> if (unlikely(!page)) {
>
[toc] | [prev] | [next] | [standalone]
| From | Christopher Lameter <cl@linux.com> |
|---|---|
| Date | 2017-09-06 18:00 +0200 |
| Subject | Re: [PATCH 1/2] mm/slub: wake up kswapd for initial high order allocation |
| Message-ID | <umKU9-3Qp-5@gated-at.bofh.it> |
| In reply to | #1727149 |
On Wed, 6 Sep 2017, js1304@gmail.com wrote:
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1578,8 +1578,12 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
> * so we fall-back to the minimum order allocation.
> */
> alloc_gfp = (flags | __GFP_NOWARN | __GFP_NORETRY) & ~__GFP_NOFAIL;
> - if ((alloc_gfp & __GFP_DIRECT_RECLAIM) && oo_order(oo) > oo_order(s->min))
> - alloc_gfp = (alloc_gfp | __GFP_NOMEMALLOC) & ~(__GFP_RECLAIM|__GFP_NOFAIL);
> + if (oo_order(oo) > oo_order(s->min)) {
> + if (alloc_gfp & __GFP_DIRECT_RECLAIM) {
> + alloc_gfp |= __GFP_NOMEMALLOC;
> + alloc_gfp &= ~__GFP_DIRECT_RECLAIM;
> + }
> + }
>
Can we come up with another inline function in gfp.h for this as well?
Well and needing these functions to manipulate flags actually indicates
that we may need a cleanup of the GFP flags at some point. There is a buch
of flags that disable things and some that enable things.
[toc] | [prev] | [next] | [standalone]
| From | Michal Hocko <mhocko@kernel.org> |
|---|---|
| Date | 2017-09-06 19:30 +0200 |
| Subject | Re: [PATCH 1/2] mm/slub: wake up kswapd for initial high order allocation |
| Message-ID | <umMjg-4Ud-15@gated-at.bofh.it> |
| In reply to | #1727617 |
On Wed 06-09-17 10:59:09, Cristopher Lameter wrote:
> On Wed, 6 Sep 2017, js1304@gmail.com wrote:
>
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -1578,8 +1578,12 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
> > * so we fall-back to the minimum order allocation.
> > */
> > alloc_gfp = (flags | __GFP_NOWARN | __GFP_NORETRY) & ~__GFP_NOFAIL;
> > - if ((alloc_gfp & __GFP_DIRECT_RECLAIM) && oo_order(oo) > oo_order(s->min))
> > - alloc_gfp = (alloc_gfp | __GFP_NOMEMALLOC) & ~(__GFP_RECLAIM|__GFP_NOFAIL);
> > + if (oo_order(oo) > oo_order(s->min)) {
> > + if (alloc_gfp & __GFP_DIRECT_RECLAIM) {
> > + alloc_gfp |= __GFP_NOMEMALLOC;
> > + alloc_gfp &= ~__GFP_DIRECT_RECLAIM;
> > + }
> > + }
> >
>
> Can we come up with another inline function in gfp.h for this as well?
What do you mean? The oo_order thing?
> Well and needing these functions to manipulate flags actually indicates
> that we may need a cleanup of the GFP flags at some point. There is a buch
> of flags that disable things and some that enable things.
Good luck with that
--
Michal Hocko
SUSE Labs
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web