All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Atish Patra <atish.patra@wdc.com>
Cc: "maintainer:X86 ARCHITECTURE \(32-BIT AND 64-BIT\)"
	<x86@kernel.org>, Albert Ou <aou@eecs.berkeley.edu>,
	Thomas Gleixner <tglx@linutronix.de>,
	Kees Cook <keescook@chromium.org>,
	linux-mm@kvack.org, Anup Patel <anup@brainfault.org>,
	Palmer Dabbelt <palmer@sifive.com>,
	linux-kernel@vger.kernel.org,
	Christoph Hellwig <hch@infradead.org>,
	Changbin Du <changbin.du@intel.com>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Vlastimil Babka <vbabka@suse.cz>, Gary Guo <gary@garyguo.net>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-riscv@lists.infradead.org,
	Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
Subject: Re: [PATCH v2 1/3] x86: Move DEBUG_TLBFLUSH option.
Date: Mon, 29 Apr 2019 22:05:54 +0200	[thread overview]
Message-ID: <20190429200554.GA102486@gmail.com> (raw)
In-Reply-To: <20190429195759.18330-2-atish.patra@wdc.com>


* Atish Patra <atish.patra@wdc.com> wrote:

> CONFIG_DEBUG_TLBFLUSH was added in 'commit 3df3212f9722 ("x86/tlb: add
> tlb_flushall_shift knob into debugfs")' to support tlb_flushall_shift
> knob. The knob was removed in 'commit e9f4e0a9fe27 ("x86/mm: Rip out
> complicated, out-of-date, buggy TLB flushing")'.  However, the debug
> option was never removed from Kconfig. It was reused in commit
> '9824cf9753ec ("mm: vmstats: tlb flush counters")' but the commit text
> was never updated accordingly.

Please, when you mention several commits, put them into new lines to make 
it readable, i.e.:

  3df3212f9722 ("x86/tlb: add tlb_flushall_shift knob into debugfs")

etc.

> Update the Kconfig option description as per its current usage.
> 
> Take this opprtunity to make this kconfig option a common option as it
> touches the common vmstat code. Introduce another arch specific config
> HAVE_ARCH_DEBUG_TLBFLUSH that can be selected to enable this config.

"opprtunity"?

> +config HAVE_ARCH_DEBUG_TLBFLUSH
> +	bool
> +	depends on DEBUG_KERNEL
> +
> +config DEBUG_TLBFLUSH
> +	bool "Save tlb flush statstics to vmstat"
> +	depends on HAVE_ARCH_DEBUG_TLBFLUSH
> +	help
> +
> +	Add tlbflush statstics to vmstat. It is really helpful understand tlbflush
> +	performance and behavior. It should be enabled only for debugging purpose
> +	by individual architectures explicitly by selecting HAVE_ARCH_DEBUG_TLBFLUSH.

"statstics"??

Please put a spell checker into your workflow or read what you are 
writing ...

Thanks,

	Ingo

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

WARNING: multiple messages have this Message-ID (diff)
From: Ingo Molnar <mingo@kernel.org>
To: Atish Patra <atish.patra@wdc.com>
Cc: linux-kernel@vger.kernel.org, Albert Ou <aou@eecs.berkeley.edu>,
	Andrew Morton <akpm@linux-foundation.org>,
	Anup Patel <anup@brainfault.org>, Borislav Petkov <bp@alien8.de>,
	Changbin Du <changbin.du@intel.com>, Gary Guo <gary@garyguo.net>,
	"H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@redhat.com>,
	Kees Cook <keescook@chromium.org>,
	linux-mm@kvack.org, linux-riscv@lists.infradead.org,
	Luc Van Oostenryck <luc.vanoostenryck@gmail.com>,
	Palmer Dabbelt <palmer@sifive.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Vlastimil Babka <vbabka@suse.cz>,
	"maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
	<x86@kernel.org>, Christoph Hellwig <hch@infradead.org>
Subject: Re: [PATCH v2 1/3] x86: Move DEBUG_TLBFLUSH option.
Date: Mon, 29 Apr 2019 22:05:54 +0200	[thread overview]
Message-ID: <20190429200554.GA102486@gmail.com> (raw)
In-Reply-To: <20190429195759.18330-2-atish.patra@wdc.com>


* Atish Patra <atish.patra@wdc.com> wrote:

> CONFIG_DEBUG_TLBFLUSH was added in 'commit 3df3212f9722 ("x86/tlb: add
> tlb_flushall_shift knob into debugfs")' to support tlb_flushall_shift
> knob. The knob was removed in 'commit e9f4e0a9fe27 ("x86/mm: Rip out
> complicated, out-of-date, buggy TLB flushing")'.  However, the debug
> option was never removed from Kconfig. It was reused in commit
> '9824cf9753ec ("mm: vmstats: tlb flush counters")' but the commit text
> was never updated accordingly.

Please, when you mention several commits, put them into new lines to make 
it readable, i.e.:

  3df3212f9722 ("x86/tlb: add tlb_flushall_shift knob into debugfs")

etc.

> Update the Kconfig option description as per its current usage.
> 
> Take this opprtunity to make this kconfig option a common option as it
> touches the common vmstat code. Introduce another arch specific config
> HAVE_ARCH_DEBUG_TLBFLUSH that can be selected to enable this config.

"opprtunity"?

> +config HAVE_ARCH_DEBUG_TLBFLUSH
> +	bool
> +	depends on DEBUG_KERNEL
> +
> +config DEBUG_TLBFLUSH
> +	bool "Save tlb flush statstics to vmstat"
> +	depends on HAVE_ARCH_DEBUG_TLBFLUSH
> +	help
> +
> +	Add tlbflush statstics to vmstat. It is really helpful understand tlbflush
> +	performance and behavior. It should be enabled only for debugging purpose
> +	by individual architectures explicitly by selecting HAVE_ARCH_DEBUG_TLBFLUSH.

"statstics"??

Please put a spell checker into your workflow or read what you are 
writing ...

Thanks,

	Ingo


  reply	other threads:[~2019-04-29 20:06 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-29 19:57 [PATCH v2 0/3] TLB flush counters Atish Patra
2019-04-29 19:57 ` Atish Patra
2019-04-29 19:57 ` [PATCH v2 1/3] x86: Move DEBUG_TLBFLUSH option Atish Patra
2019-04-29 19:57   ` Atish Patra
2019-04-29 20:05   ` Ingo Molnar [this message]
2019-04-29 20:05     ` Ingo Molnar
2019-04-29 21:29     ` Atish Patra
2019-04-29 21:29       ` Atish Patra
2019-04-29 19:57 ` [PATCH v2 2/3] RISC-V: Enable TLBFLUSH counters for debug kernel Atish Patra
2019-04-29 19:57   ` Atish Patra
2019-04-29 19:57 ` [PATCH v2 3/3] RISC-V: Update tlb flush counters Atish Patra
2019-04-29 19:57   ` Atish Patra

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=20190429200554.GA102486@gmail.com \
    --to=mingo@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=anup@brainfault.org \
    --cc=aou@eecs.berkeley.edu \
    --cc=atish.patra@wdc.com \
    --cc=bp@alien8.de \
    --cc=changbin.du@intel.com \
    --cc=gary@garyguo.net \
    --cc=hch@infradead.org \
    --cc=hpa@zytor.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=luc.vanoostenryck@gmail.com \
    --cc=mingo@redhat.com \
    --cc=palmer@sifive.com \
    --cc=tglx@linutronix.de \
    --cc=vbabka@suse.cz \
    --cc=x86@kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.