Re: [PATCH v2 1/6] kselftests: vm: Add mremap tests

From: John Hubbard <jhubbard@nvidia.com>
To: Kalesh Singh <kaleshsingh@google.com>
Cc: Suren Baghdasaryan <surenb@google.com>,
	Minchan Kim <minchan@google.com>,
	Joel Fernandes <joelaf@google.com>,
	Lokesh Gidra <lokeshgidra@google.com>,
	"Cc: Android Kernel" <kernel-team@android.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	the arch/x86 maintainers <x86@kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Shuah Khan <shuah@kernel.org>, Kees Cook <keescook@chromium.org>,
	Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>,
	Peter Zijlstra <peterz@infradead.org>,
	"Arnd Bergmann" <arnd@arndb.de>,
	Masahiro Yamada <masahiroy@kernel.org>,
	"Sami Tolvanen" <samitolvanen@google.com>,
	Frederic Weisbecker <frederic@kernel.org>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Hassan Naveed <hnaveed@wavecomp.com>,
	Christian Brauner <christian.brauner@ubuntu.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Mark Brown <broonie@kernel.org>,
	"Mike Rapoport" <rppt@kernel.org>, Gavin Shan <gshan@redhat.com>,
	"Chris von Recklinghausen" <crecklin@redhat.com>,
	Jia He <justin.he@arm.com>, Zhenyu Ye <yezhenyu2@huawei.com>,
	Thiago Jung Bauermann <bauerman@linux.ibm.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Mina Almasry <almasrymina@google.com>,
	Ram Pai <linuxram@us.ibm.com>,
	Ralph Campbell <rcampbell@nvidia.com>,
	Sandipan Das <sandipan@linux.ibm.com>,
	Dave Hansen <dave.hansen@intel.com>,
	Colin Ian King <colin.king@canonical.com>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>,
	"Ira Weiny" <ira.weiny@intel.com>,
	SeongJae Park <sjpark@amazon.de>,
	Brian Geffon <bgeffon@google.com>,
	LKML <linux-kernel@vger.kernel.org>,
	"moderated list:ARM64 PORT (AARCH64 ARCHITECTURE)"
	<linux-arm-kernel@lists.infradead.org>,
	"open list:MEMORY MANAGEMENT" <linux-mm@kvack.org>,
	"open list:KERNEL SELFTEST FRAMEWORK"
	<linux-kselftest@vger.kernel.org>
Subject: Re: [PATCH v2 1/6] kselftests: vm: Add mremap tests
Date: Sat, 3 Oct 2020 15:31:14 -0700	[thread overview]
Message-ID: <b62f5c37-34f8-86e3-4f0c-bb5bf3c1eb08@nvidia.com> (raw)
In-Reply-To: <CAC_TJveSgd-=t49hqR3LN7G8PMPwdgN+qkQct3zS-N6=CW7uug@mail.gmail.com>

On 10/3/20 1:01 PM, Kalesh Singh wrote:
> On Sat, Oct 3, 2020 at 3:13 AM John Hubbard <jhubbard@nvidia.com> wrote:
...
>> A remaining nit: there are lots of > 80 col lines here. Only those lines
>> that contain printf output strings really need to be extra-long. The
>> others can all be easily made to fit.
> Thanks for the review John. I can fix the lines that don't need to be
>> 80 col. FWIW checkpatch now allows 100 col lines:
> https://lore.kernel.org/r/9c360bfa43580ce7726dd3d9d247f1216a690ef0.camel@perches.com

Yep, I watched that go by. The new guidelines match what I recommended here,
though.  It's slightly more nuanced than any sort of hard limit at either
80 or 100 lines, as you know from reading that--I'm just saying that out
loud for the benefit of others who may see this thread.

>> ...
>>> +
>>> +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>>> +#define MIN(X, Y) ((X) < (Y) ? (X) : (Y))
>>
>>
>> Not really a comment on this patch, but: at least two other test suites
>> in selftests/ have had to recreate MIN() or min(), and probably a few
>> other common things as well. It would be nice to gather up some of these
>> common things into one of the shared header files in selftests/.
>>
>> I'm certainly not saying that this patchset has that responsibility,
>> though. Just pointing it out in case someone has a moment to clean up a
>> few here and there.
> Thanks for pointing this out. I'll leave it as a task for a separate
> patch if time permits or no one gets around to it before :-)
>>
Sound great. :)

thanks,
-- 
John Hubbard
NVIDIA