All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: Chris Metcalf <chris.d.metcalf@gmail.com>,
	Rusty Russell <rusty@rustcorp.com.au>,
	linux-mm@kvack.org
Subject: Re: [PATCH] mm/swap.c: workaround for_each_cpu() bug on UP kernel.
Date: Thu, 7 Feb 2019 06:07:29 -0800	[thread overview]
Message-ID: <344b9779-2866-5c0c-6155-f03fff38f8c9@roeck-us.net> (raw)
In-Reply-To: <1549533189-9177-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp>

On 2/7/19 1:53 AM, Tetsuo Handa wrote:
> Since for_each_cpu(cpu, mask) added by commit 2d3854a37e8b767a ("cpumask:
> introduce new API, without changing anything") did not evaluate the mask
> argument if NR_CPUS == 1 due to CONFIG_SMP=n, lru_add_drain_all() is
> hitting WARN_ON() at __flush_work() added by commit 4d43d395fed12463
> ("workqueue: Try to catch flush_work() without INIT_WORK().")
> by unconditionally calling flush_work() [1].
> 
> We should fix for_each_cpu() etc. but we need enough grace period for
> allowing people to test and fix unexpected behaviors including build
> failures. Therefore, this patch temporarily duplicates flush_work() for
> NR_CPUS == 1 case. This patch will be reverted after for_each_cpu() etc.
> are fixed.
> 
> [1] https://lkml.kernel.org/r/18a30387-6aa5-6123-e67c-57579ecc3f38@roeck-us.net
> 
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

I agree with the fix/workaround. I tried a complete build with fixed macros,
but that doesn't work because (at least) x86 assumes that the "mask" parameter
is _not_ evaluated for non-SMP builds - arch/x86/kernel/cpu/cacheinfo.c
passes cpu_llc_shared_mask(cpu) as parameter, and that is only defined
for SMP builds.

On the plus side, I did not find any other issues, but that doesn't mean
much since various build and boot tests in -next fail for other reasons.

Acked-by: Guenter Roeck <linux@roeck-us.net>

Guenter

> ---
>   mm/swap.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/mm/swap.c b/mm/swap.c
> index 4929bc1..e5e8e15 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -694,11 +694,16 @@ void lru_add_drain_all(void)
>   			INIT_WORK(work, lru_add_drain_per_cpu);
>   			queue_work_on(cpu, mm_percpu_wq, work);
>   			cpumask_set_cpu(cpu, &has_work);
> +#if NR_CPUS == 1
> +			flush_work(work);
> +#endif
>   		}
>   	}
>   
> +#if NR_CPUS != 1
>   	for_each_cpu(cpu, &has_work)
>   		flush_work(&per_cpu(lru_add_drain_work, cpu));
> +#endif
>   
>   	mutex_unlock(&lock);
>   }
> 


  reply	other threads:[~2019-02-07 14:07 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-07  9:53 Tetsuo Handa
2019-02-07 14:07 ` Guenter Roeck [this message]
2019-02-07 14:18 ` William Kucharski
2019-02-12 10:11 ` Michal Hocko
2019-02-12 10:25   ` Tetsuo Handa
2019-02-12 11:21     ` Michal Hocko
2019-02-12 11:29       ` Michal Hocko
2019-02-12 11:37         ` Tetsuo Handa
2019-02-12 21:06         ` Andrew Morton
2019-02-13 12:43           ` Michal Hocko
2019-02-13 21:37             ` Andrew Morton
2019-02-12 23:19         ` [PATCH] mm: handle lru_add_drain_all for UP properly kbuild test robot
2019-02-12 23:48         ` kbuild test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=344b9779-2866-5c0c-6155-f03fff38f8c9@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=akpm@linux-foundation.org \
    --cc=chris.d.metcalf@gmail.com \
    --cc=linux-mm@kvack.org \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=rusty@rustcorp.com.au \
    --subject='Re: [PATCH] mm/swap.c: workaround for_each_cpu() bug on UP kernel.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.