All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brendan Jackman <jackmanb@google.com>
To: Ujwal Kundur <ujwal.kundur@gmail.com>
Cc: <akpm@linux-foundation.org>, <peterx@redhat.com>,
	<shuah@kernel.org>,  <linux-mm@kvack.org>,
	<linux-kselftest@vger.kernel.org>,
	 <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 1/1] selftests/mm/uffd: Refactor non-composite global vars into struct
Date: Mon, 26 May 2025 09:08:46 +0000	[thread overview]
Message-ID: <DA5Z38N5WHO5.2FFOQZYC6WKMI@google.com> (raw)
In-Reply-To: <CALkFLLK19Uqr2veWCn79cbLLgde5f+otf9Qx0xSPGdhdnekGrw@mail.gmail.com>

On Sun May 25, 2025 at 7:19 PM UTC, Ujwal Kundur wrote:
>> I'm afraid I'm too ignorant of this code to be able to suggest something
>> good here. But, can we just remove the comment and plumb the gopts
>> through to uffd_poll_thread()->uffd_handle_page_fault()->__copy_page()?
>>
>> This is not pretty but it lets us remove the global vars which is
>> clearly a step in the right direction.
>
> Perhaps Andrew can weigh in? If I understood this correctly, we're
> trying to assert that retrying a successful UFFDIO_COPY operation
> always results in EEXIST. This is being done in a somewhat racy
> fashion where a flag (test_uffdio_copy_eexist) is set every 10 seconds
> using alarm(2). IMO this is a flaky test, we should either:
> - remove this variable and associated logic entirely (preferred)
> - use a probability function to set this a % of the time instead of
> every 10 seconds
> - use an async library that can replace the implementation without the
> use of global vars

Sorry I don't have an opinion on which of these is the best (I can try
to find some time to form an opionion on this later!), but:

Fixing the flakiness sounds great, but I would suggest decoupling that
from the refactoring. If it's practical, focus on removing the globals
first, while leaving the fundamental logic the same, even if it's bad.
Then as a separate series, fix the logic. 

  reply	other threads:[~2025-05-26  9:08 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-01 16:38 [PATCH 0/4] selftests/mm/uffd: refactor global variables Ujwal Kundur
2025-05-01 16:38 ` [PATCH 1/4] selftests/mm/uffd: Refactor non-composite global vars into struct Ujwal Kundur
2025-05-02 12:16   ` Brendan Jackman
2025-05-02 12:28     ` Brendan Jackman
2025-05-03 18:16       ` Ujwal Kundur
2025-05-04  2:25         ` Andrew Morton
2025-05-01 16:38 ` [PATCH 2/4] selftests/mm/uffd: Swap global vars with global test options Ujwal Kundur
2025-05-01 16:38 ` [PATCH 3/4] selftests/mm/uffd: Swap global variables with global test opts Ujwal Kundur
2025-05-01 16:38 ` [PATCH 4/4] " Ujwal Kundur
2025-05-02 12:18 ` [PATCH 0/4] selftests/mm/uffd: refactor global variables Brendan Jackman
2025-05-04  9:48 ` [PATCH v2 1/1] selftests/mm/uffd: Refactor non-composite global vars into struct Ujwal Kundur
2025-05-06  0:57   ` Andrew Morton
2025-05-10 16:03 ` [PATCH v3 " Ujwal Kundur
2025-05-13 12:12   ` Brendan Jackman
2025-05-19 13:50     ` Ujwal Kundur
2025-05-19 21:40       ` Andrew Morton
2025-05-20  9:16       ` Brendan Jackman
2025-05-25 19:19         ` Ujwal Kundur
2025-05-26  9:08           ` Brendan Jackman [this message]
2025-05-30  7:45             ` Ujwal Kundur
2025-05-31  7:46 ` [PATCH v4 " Ujwal Kundur
2025-06-10  6:57   ` Ujwal Kundur
2025-06-10 11:32   ` Brendan Jackman
2025-06-16  6:38     ` Ujwal Kundur
2025-06-16 10:04 ` [PATCH v5 " Ujwal Kundur
2025-06-17  0:26   ` Andrew Morton
2025-06-17 15:52     ` Peter Xu
2025-06-17 17:22       ` Peter Xu
2025-06-18 10:00         ` Brendan Jackman
2025-06-26  5:22           ` Ujwal Kundur
2025-06-26 14:12             ` Peter Xu
2025-06-30 11:25               ` Ujwal Kundur
2025-07-02 15:20 ` [PATCH v6 " Ujwal Kundur
2025-07-04 16:20   ` Peter Xu
2025-07-10  5:07     ` Ujwal Kundur
2025-08-06 15:03       ` Ujwal Kundur
2025-08-07 16:45         ` Peter Xu
2025-08-13 11:33         ` Brendan Jackman
2025-08-16 14:12           ` Ujwal Kundur

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=DA5Z38N5WHO5.2FFOQZYC6WKMI@google.com \
    --to=jackmanb@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=peterx@redhat.com \
    --cc=shuah@kernel.org \
    --cc=ujwal.kundur@gmail.com \
    /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.