All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brendan Jackman <jackmanb@google.com>
To: Dev Jain <dev.jain@arm.com>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Shuah Khan <shuah@kernel.org>,
	linux-mm@kvack.org, linux-kselftest@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 04/10] selftests/mm/uffd: Rename nr_cpus -> nr_threads
Date: Mon, 3 Mar 2025 10:34:52 +0000	[thread overview]
Message-ID: <Z8WFzISSAmtjtu3L@google.com> (raw)
In-Reply-To: <18ea9794-3901-4802-875c-b0327984a9d6@arm.com>

On Mon, Mar 03, 2025 at 03:48:38PM +0530, Dev Jain wrote:
> 
> 
> On 03/03/25 3:17 pm, Brendan Jackman wrote:
> > On Fri, Feb 28, 2025 at 11:06:35PM +0530, Dev Jain wrote:
> > > Taking a cursory look at the test, it creates three threads for each cpu.
> > > The bounding of the variable is fine but that being the reason to rename the
> > > variable is not making sense to me.
> > 
> > Hmm yeah the name needs to be more abstract. Do you think nr_workers
> > would be confusing? Or even just "parallelism" or nr_parallel? Or any
> > other ideas?
> > 
> > FWIW I briefly looked at just cleaning this up to remove the global
> > variable but that's a bigger time investment than I can afford here I
> > think. (The local variable in stress() would still need a better name
> > anyway).
> > 
> > Thanks for the review BTW!
> 
> Your welcome.
> 
> I personally prefer leaving it as is; unless someone comes up and completely
> cleans up the structure, let us save our collective brain cycles for more
> meaningful battles than renaming variables :)

Hmm, I think that's a false economy on brain cycles. A variable called
nr_cpus that isn't a number of CPUs is bound to waste a bunch of
mental energy at some point in the future.

Unless you strongly object I'll go for nr_parallel. It's not a great
name but, well... I think that probably just suggests it's not a great
variable, and I don't have time to fix that.

  reply	other threads:[~2025-03-03 10:34 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-28 16:54 [PATCH v3 00/10] selftests/mm: Some cleanups from trying to run them Brendan Jackman
2025-02-28 16:54 ` [PATCH v3 01/10] selftests/mm: Report errno when things fail in gup_longterm Brendan Jackman
2025-02-28 17:17   ` Dev Jain
2025-03-06  9:34   ` David Hildenbrand
2025-02-28 16:54 ` [PATCH v3 02/10] selftests/mm: Skip uffd-stress if userfaultfd not available Brendan Jackman
2025-02-28 17:20   ` Dev Jain
2025-02-28 16:54 ` [PATCH v3 03/10] selftests/mm: Skip uffd-wp-mremap " Brendan Jackman
2025-02-28 17:25   ` Dev Jain
2025-03-03 10:48     ` Brendan Jackman
2025-03-03 11:00       ` Dev Jain
2025-02-28 16:54 ` [PATCH v3 04/10] selftests/mm/uffd: Rename nr_cpus -> nr_threads Brendan Jackman
2025-02-28 17:36   ` Dev Jain
2025-03-03  9:47     ` Brendan Jackman
2025-03-03 10:18       ` Dev Jain
2025-03-03 10:34         ` Brendan Jackman [this message]
2025-03-03 10:46           ` Dev Jain
2025-02-28 16:54 ` [PATCH v3 05/10] selftests/mm: Print some details when uffd-stress gets bad params Brendan Jackman
2025-02-28 17:26   ` Dev Jain
2025-02-28 16:54 ` [PATCH v3 06/10] selftests/mm: Don't fail uffd-stress if too many CPUs Brendan Jackman
2025-02-28 17:37   ` Dev Jain
2025-03-05 11:07   ` Brendan Jackman
2025-02-28 16:54 ` [PATCH v3 07/10] selftests/mm: Skip map_populate on weird filesystems Brendan Jackman
2025-02-28 16:54 ` [PATCH v3 08/10] selftests/mm: Skip gup_longerm tests " Brendan Jackman
2025-03-06  9:28   ` David Hildenbrand
2025-03-06 12:42     ` Brendan Jackman
2025-03-06 14:40       ` David Hildenbrand
2025-03-11 13:00         ` Brendan Jackman
2025-03-11 19:53           ` David Hildenbrand
2025-03-12  8:34             ` Brendan Jackman
2025-03-14 12:10               ` David Hildenbrand
2025-03-14 12:17                 ` David Hildenbrand
2025-03-14 15:56                 ` Brendan Jackman
2025-03-14 21:19                   ` David Hildenbrand
2025-02-28 16:54 ` [PATCH v3 09/10] selftests/mm: Drop unnecessary sudo usage Brendan Jackman
2025-02-28 17:26   ` Dev Jain
2025-02-28 16:54 ` [PATCH v3 10/10] selftests/mm: Ensure uffd-wp-mremap gets pages of each size Brendan Jackman
2025-03-05  8:25 ` [PATCH v3 00/10] selftests/mm: Some cleanups from trying to run them Muhammad Usama Anjum

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=Z8WFzISSAmtjtu3L@google.com \
    --to=jackmanb@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=dev.jain@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=shuah@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.