All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alistair Popple <apopple@nvidia.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: John Hubbard <jhubbard@nvidia.com>,
	linux-mm@kvack.org, Ralph Campbell <rcampbell@nvidia.com>,
	nouveau@lists.freedesktop.org, stable@vger.kernel.org
Subject: Re: [PATCH] mm: Take a page reference when removing device exclusive entries
Date: Wed, 29 Mar 2023 12:45:44 +1100	[thread overview]
Message-ID: <87lejgtjbh.fsf@nvidia.com> (raw)
In-Reply-To: <20230328125558.90cf40060e238b24add51d23@linux-foundation.org>


John Hubbard <jhubbard@nvidia.com> writes:

>> warnings such as PAGE_FLAGS_CHECK_AT_FREE due to the page being locked
>> when the refcount drops to zero. Note that during removal of the
>> device exclusive entry the PTE is currently re-checked under the PTL
>> so no futher bad page accesses occur once it is locked.
>
> Maybe change that last sentence to something like this:
>
> "Fix this by taking a page reference before starting to remove a device
> exclusive pte. This is done safely in a lock-free way by first getting a
> reference via get_page_unless_zero(), and then re-checking after
> acquiring the PTL, that the page is the correct one."
>
> ?
>
> ...well, maybe that's not all that much help. But it does at least
> provide the traditional description of what the patch *does*, at
> the end of the commit description. But please treat this as just
> an optional suggestion.

My wording was probably a little awkward. The intent was to point out
the existing code subsequent to taking the page lock was already
correct/safe. I figured the patch itself does a pretty good of
describing the actual fix so am inclined to leave it.

Andrew Morton <akpm@linux-foundation.org> writes:

> On Mon, 27 Mar 2023 23:25:49 -0700 John Hubbard <jhubbard@nvidia.com> wrote:
>
>> On the patch process, I see that this applies to linux-stable's 6.1.y
>> branch. I'd suggest two things:
>> 
>> 1) Normally, what I've seen done is to post against either the current
>> top of tree linux.git, or else against one of the mm-stable branches.
>> And then after it's accepted, create a version for -stable. 
>
> Yup.  I had to jiggle the patch a bit because
> mmu_notifier_range_init_owner()'s arguments have changed.  Once this
> hits mainline, the -stable maintainers will probably ask for a version
> which suits the relevant kernel version(s).

Thanks Andrew. That's my bad, I was developing on top of v6.1 and
neglected to rebase. Happy to provide versions for -stable as required.

WARNING: multiple messages have this Message-ID (diff)
From: Alistair Popple <apopple@nvidia.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: nouveau@lists.freedesktop.org,
	Ralph Campbell <rcampbell@nvidia.com>,
	stable@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [Nouveau] [PATCH] mm: Take a page reference when removing device exclusive entries
Date: Wed, 29 Mar 2023 12:45:44 +1100	[thread overview]
Message-ID: <87lejgtjbh.fsf@nvidia.com> (raw)
In-Reply-To: <20230328125558.90cf40060e238b24add51d23@linux-foundation.org>


John Hubbard <jhubbard@nvidia.com> writes:

>> warnings such as PAGE_FLAGS_CHECK_AT_FREE due to the page being locked
>> when the refcount drops to zero. Note that during removal of the
>> device exclusive entry the PTE is currently re-checked under the PTL
>> so no futher bad page accesses occur once it is locked.
>
> Maybe change that last sentence to something like this:
>
> "Fix this by taking a page reference before starting to remove a device
> exclusive pte. This is done safely in a lock-free way by first getting a
> reference via get_page_unless_zero(), and then re-checking after
> acquiring the PTL, that the page is the correct one."
>
> ?
>
> ...well, maybe that's not all that much help. But it does at least
> provide the traditional description of what the patch *does*, at
> the end of the commit description. But please treat this as just
> an optional suggestion.

My wording was probably a little awkward. The intent was to point out
the existing code subsequent to taking the page lock was already
correct/safe. I figured the patch itself does a pretty good of
describing the actual fix so am inclined to leave it.

Andrew Morton <akpm@linux-foundation.org> writes:

> On Mon, 27 Mar 2023 23:25:49 -0700 John Hubbard <jhubbard@nvidia.com> wrote:
>
>> On the patch process, I see that this applies to linux-stable's 6.1.y
>> branch. I'd suggest two things:
>> 
>> 1) Normally, what I've seen done is to post against either the current
>> top of tree linux.git, or else against one of the mm-stable branches.
>> And then after it's accepted, create a version for -stable. 
>
> Yup.  I had to jiggle the patch a bit because
> mmu_notifier_range_init_owner()'s arguments have changed.  Once this
> hits mainline, the -stable maintainers will probably ask for a version
> which suits the relevant kernel version(s).

Thanks Andrew. That's my bad, I was developing on top of v6.1 and
neglected to rebase. Happy to provide versions for -stable as required.

  reply	other threads:[~2023-03-29  1:53 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-28  2:14 [PATCH] mm: Take a page reference when removing device exclusive entries Alistair Popple
2023-03-28  2:14 ` [Nouveau] " Alistair Popple
2023-03-28  6:25 ` John Hubbard
2023-03-28  6:25   ` [Nouveau] " John Hubbard
2023-03-28 19:55   ` Andrew Morton
2023-03-28 19:55     ` [Nouveau] " Andrew Morton
2023-03-29  1:45     ` Alistair Popple [this message]
2023-03-29  1:45       ` Alistair Popple
2023-03-28 19:59 ` Ralph Campbell
2023-03-28 19:59   ` Ralph Campbell
2023-03-29  3:16 ` Matthew Wilcox
2023-03-29  3:16   ` [Nouveau] " Matthew Wilcox
2023-03-29  3:45   ` John Hubbard
2023-03-29  3:45     ` [Nouveau] " John Hubbard
2023-03-30  0:26     ` Alistair Popple
2023-03-30  0:26       ` [Nouveau] " Alistair Popple
2023-03-30  0:55       ` Alistair Popple
2023-03-30  0:55         ` [Nouveau] " Alistair Popple

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=87lejgtjbh.fsf@nvidia.com \
    --to=apopple@nvidia.com \
    --cc=akpm@linux-foundation.org \
    --cc=jhubbard@nvidia.com \
    --cc=linux-mm@kvack.org \
    --cc=nouveau@lists.freedesktop.org \
    --cc=rcampbell@nvidia.com \
    --cc=stable@vger.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.