All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ralph Campbell <rcampbell@nvidia.com>
To: Alistair Popple <apopple@nvidia.com>,
	linux-mm@kvack.org, nouveau@lists.freedesktop.org,
	bskeggs@redhat.com, akpm@linux-foundation.org
Cc: linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	kvm-ppc@vger.kernel.org, dri-devel@lists.freedesktop.org,
	jhubbard@nvidia.com, jglisse@redhat.com,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH v4 4/8] mm/rmap: Split migration into its own function
Date: Mon, 08 Mar 2021 18:58:12 +0000	[thread overview]
Message-ID: <9fa77684-149a-e6ed-296d-dc852aecea97@nvidia.com> (raw)
In-Reply-To: <20210304061645.29747-5-apopple@nvidia.com>


On 3/3/21 10:16 PM, Alistair Popple wrote:
> Migration is currently implemented as a mode of operation for
> try_to_unmap_one() generally specified by passing the TTU_MIGRATION flag
> or in the case of splitting a huge anonymous page TTU_SPLIT_FREEZE.
> 
> However it does not have much in common with the rest of the unmap
> functionality of try_to_unmap_one() and thus splitting it into a
> separate function reduces the complexity of try_to_unmap_one() making it
> more readable.
> 
> Several simplifications can also be made in try_to_migrate_one() based
> on the following observations:
> 
>   - All users of TTU_MIGRATION also set TTU_IGNORE_MLOCK.
>   - No users of TTU_MIGRATION ever set TTU_IGNORE_HWPOISON.
>   - No users of TTU_MIGRATION ever set TTU_BATCH_FLUSH.
> 
> TTU_SPLIT_FREEZE is a special case of migration used when splitting an
> anonymous page. This is most easily dealt with by calling the correct
> function from unmap_page() in mm/huge_memory.c  - either
> try_to_migrate() for PageAnon or try_to_unmap().
> 
> Signed-off-by: Alistair Popple <apopple@nvidia.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Looks reasonable to me. I do worry a bit about code duplication.
At some point in the future, it might be discovered that other combinations
of TTU_XXX flags are needed in which case a careful check of try_to_migrate()
and try_to_unmap() will be needed.

Reviewed-by: Ralph Campbell <rcampbell@nvidia.com>

WARNING: multiple messages have this Message-ID (diff)
From: Ralph Campbell <rcampbell@nvidia.com>
To: Alistair Popple <apopple@nvidia.com>, <linux-mm@kvack.org>,
	<nouveau@lists.freedesktop.org>, <bskeggs@redhat.com>,
	<akpm@linux-foundation.org>
Cc: <linux-doc@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<kvm-ppc@vger.kernel.org>, <dri-devel@lists.freedesktop.org>,
	<jhubbard@nvidia.com>, <jglisse@redhat.com>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH v4 4/8] mm/rmap: Split migration into its own function
Date: Mon, 8 Mar 2021 10:58:12 -0800	[thread overview]
Message-ID: <9fa77684-149a-e6ed-296d-dc852aecea97@nvidia.com> (raw)
In-Reply-To: <20210304061645.29747-5-apopple@nvidia.com>


On 3/3/21 10:16 PM, Alistair Popple wrote:
> Migration is currently implemented as a mode of operation for
> try_to_unmap_one() generally specified by passing the TTU_MIGRATION flag
> or in the case of splitting a huge anonymous page TTU_SPLIT_FREEZE.
> 
> However it does not have much in common with the rest of the unmap
> functionality of try_to_unmap_one() and thus splitting it into a
> separate function reduces the complexity of try_to_unmap_one() making it
> more readable.
> 
> Several simplifications can also be made in try_to_migrate_one() based
> on the following observations:
> 
>   - All users of TTU_MIGRATION also set TTU_IGNORE_MLOCK.
>   - No users of TTU_MIGRATION ever set TTU_IGNORE_HWPOISON.
>   - No users of TTU_MIGRATION ever set TTU_BATCH_FLUSH.
> 
> TTU_SPLIT_FREEZE is a special case of migration used when splitting an
> anonymous page. This is most easily dealt with by calling the correct
> function from unmap_page() in mm/huge_memory.c  - either
> try_to_migrate() for PageAnon or try_to_unmap().
> 
> Signed-off-by: Alistair Popple <apopple@nvidia.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Looks reasonable to me. I do worry a bit about code duplication.
At some point in the future, it might be discovered that other combinations
of TTU_XXX flags are needed in which case a careful check of try_to_migrate()
and try_to_unmap() will be needed.

Reviewed-by: Ralph Campbell <rcampbell@nvidia.com>

WARNING: multiple messages have this Message-ID (diff)
From: Ralph Campbell <rcampbell@nvidia.com>
To: Alistair Popple <apopple@nvidia.com>, <linux-mm@kvack.org>,
	<nouveau@lists.freedesktop.org>, <bskeggs@redhat.com>,
	<akpm@linux-foundation.org>
Cc: linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	dri-devel@lists.freedesktop.org, kvm-ppc@vger.kernel.org,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [Nouveau] [PATCH v4 4/8] mm/rmap: Split migration into its own function
Date: Mon, 8 Mar 2021 10:58:12 -0800	[thread overview]
Message-ID: <9fa77684-149a-e6ed-296d-dc852aecea97@nvidia.com> (raw)
In-Reply-To: <20210304061645.29747-5-apopple@nvidia.com>


On 3/3/21 10:16 PM, Alistair Popple wrote:
> Migration is currently implemented as a mode of operation for
> try_to_unmap_one() generally specified by passing the TTU_MIGRATION flag
> or in the case of splitting a huge anonymous page TTU_SPLIT_FREEZE.
> 
> However it does not have much in common with the rest of the unmap
> functionality of try_to_unmap_one() and thus splitting it into a
> separate function reduces the complexity of try_to_unmap_one() making it
> more readable.
> 
> Several simplifications can also be made in try_to_migrate_one() based
> on the following observations:
> 
>   - All users of TTU_MIGRATION also set TTU_IGNORE_MLOCK.
>   - No users of TTU_MIGRATION ever set TTU_IGNORE_HWPOISON.
>   - No users of TTU_MIGRATION ever set TTU_BATCH_FLUSH.
> 
> TTU_SPLIT_FREEZE is a special case of migration used when splitting an
> anonymous page. This is most easily dealt with by calling the correct
> function from unmap_page() in mm/huge_memory.c  - either
> try_to_migrate() for PageAnon or try_to_unmap().
> 
> Signed-off-by: Alistair Popple <apopple@nvidia.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Looks reasonable to me. I do worry a bit about code duplication.
At some point in the future, it might be discovered that other combinations
of TTU_XXX flags are needed in which case a careful check of try_to_migrate()
and try_to_unmap() will be needed.

Reviewed-by: Ralph Campbell <rcampbell@nvidia.com>
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

WARNING: multiple messages have this Message-ID (diff)
From: Ralph Campbell <rcampbell@nvidia.com>
To: Alistair Popple <apopple@nvidia.com>, <linux-mm@kvack.org>,
	<nouveau@lists.freedesktop.org>, <bskeggs@redhat.com>,
	<akpm@linux-foundation.org>
Cc: linux-doc@vger.kernel.org, jhubbard@nvidia.com,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	jglisse@redhat.com, kvm-ppc@vger.kernel.org,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH v4 4/8] mm/rmap: Split migration into its own function
Date: Mon, 8 Mar 2021 10:58:12 -0800	[thread overview]
Message-ID: <9fa77684-149a-e6ed-296d-dc852aecea97@nvidia.com> (raw)
In-Reply-To: <20210304061645.29747-5-apopple@nvidia.com>


On 3/3/21 10:16 PM, Alistair Popple wrote:
> Migration is currently implemented as a mode of operation for
> try_to_unmap_one() generally specified by passing the TTU_MIGRATION flag
> or in the case of splitting a huge anonymous page TTU_SPLIT_FREEZE.
> 
> However it does not have much in common with the rest of the unmap
> functionality of try_to_unmap_one() and thus splitting it into a
> separate function reduces the complexity of try_to_unmap_one() making it
> more readable.
> 
> Several simplifications can also be made in try_to_migrate_one() based
> on the following observations:
> 
>   - All users of TTU_MIGRATION also set TTU_IGNORE_MLOCK.
>   - No users of TTU_MIGRATION ever set TTU_IGNORE_HWPOISON.
>   - No users of TTU_MIGRATION ever set TTU_BATCH_FLUSH.
> 
> TTU_SPLIT_FREEZE is a special case of migration used when splitting an
> anonymous page. This is most easily dealt with by calling the correct
> function from unmap_page() in mm/huge_memory.c  - either
> try_to_migrate() for PageAnon or try_to_unmap().
> 
> Signed-off-by: Alistair Popple <apopple@nvidia.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Looks reasonable to me. I do worry a bit about code duplication.
At some point in the future, it might be discovered that other combinations
of TTU_XXX flags are needed in which case a careful check of try_to_migrate()
and try_to_unmap() will be needed.

Reviewed-by: Ralph Campbell <rcampbell@nvidia.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2021-03-08 18:58 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-04  6:16 [PATCH v4 0/8] Add support for SVM atomics in Nouveau Alistair Popple
2021-03-04  6:16 ` Alistair Popple
2021-03-04  6:16 ` [Nouveau] " Alistair Popple
2021-03-04  6:16 ` Alistair Popple
2021-03-04  6:16 ` [PATCH v4 1/8] mm: Remove special swap entry functions Alistair Popple
2021-03-04  6:16   ` Alistair Popple
2021-03-04  6:16   ` [Nouveau] " Alistair Popple
2021-03-04  6:16   ` Alistair Popple
2021-03-08 18:24   ` Ralph Campbell
2021-03-08 18:24     ` Ralph Campbell
2021-03-08 18:24     ` [Nouveau] " Ralph Campbell
2021-03-08 18:24     ` Ralph Campbell
2021-03-04  6:16 ` [PATCH v4 2/8] mm/swapops: Rework swap entry manipulation code Alistair Popple
2021-03-04  6:16   ` Alistair Popple
2021-03-04  6:16   ` [Nouveau] " Alistair Popple
2021-03-04  6:16   ` Alistair Popple
2021-03-08 18:30   ` Ralph Campbell
2021-03-08 18:30     ` Ralph Campbell
2021-03-08 18:30     ` [Nouveau] " Ralph Campbell
2021-03-08 18:30     ` Ralph Campbell
2021-03-04  6:16 ` [PATCH v4 3/8] mm/rmap: Split try_to_munlock from try_to_unmap Alistair Popple
2021-03-04  6:16   ` Alistair Popple
2021-03-04  6:16   ` [Nouveau] " Alistair Popple
2021-03-04  6:16   ` Alistair Popple
2021-03-08 18:39   ` Ralph Campbell
2021-03-08 18:39     ` Ralph Campbell
2021-03-08 18:39     ` [Nouveau] " Ralph Campbell
2021-03-08 18:39     ` Ralph Campbell
2021-03-04  6:16 ` [PATCH v4 4/8] mm/rmap: Split migration into its own function Alistair Popple
2021-03-04  6:16   ` Alistair Popple
2021-03-04  6:16   ` [Nouveau] " Alistair Popple
2021-03-04  6:16   ` Alistair Popple
2021-03-08 18:58   ` Ralph Campbell [this message]
2021-03-08 18:58     ` Ralph Campbell
2021-03-08 18:58     ` [Nouveau] " Ralph Campbell
2021-03-08 18:58     ` Ralph Campbell
2021-03-09  0:01     ` Alistair Popple
2021-03-09  0:01       ` Alistair Popple
2021-03-09  0:01       ` [Nouveau] " Alistair Popple
2021-03-09  0:01       ` Alistair Popple
2021-03-04  6:16 ` [PATCH v4 5/8] mm: Device exclusive memory access Alistair Popple
2021-03-04  6:16   ` Alistair Popple
2021-03-04  6:16   ` [Nouveau] " Alistair Popple
2021-03-04  6:16   ` Alistair Popple
2021-03-08 19:44   ` Ralph Campbell
2021-03-08 19:44     ` Ralph Campbell
2021-03-08 19:44     ` [Nouveau] " Ralph Campbell
2021-03-08 19:44     ` Ralph Campbell
2021-03-09  1:17     ` Alistair Popple
2021-03-09  1:17       ` Alistair Popple
2021-03-09  1:17       ` [Nouveau] " Alistair Popple
2021-03-09  1:17       ` Alistair Popple
2021-03-04  6:16 ` [PATCH v4 6/8] mm: Selftests for exclusive device memory Alistair Popple
2021-03-04  6:16   ` Alistair Popple
2021-03-04  6:16   ` [Nouveau] " Alistair Popple
2021-03-04  6:16   ` Alistair Popple
2021-03-04  6:16 ` [PATCH v4 7/8] nouveau/svm: Refactor nouveau_range_fault Alistair Popple
2021-03-04  6:16   ` Alistair Popple
2021-03-04  6:16   ` [Nouveau] " Alistair Popple
2021-03-04  6:16   ` Alistair Popple
2021-03-04  6:16 ` [PATCH v4 8/8] nouveau/svm: Implement atomic SVM access Alistair Popple
2021-03-04  6:16   ` Alistair Popple
2021-03-04  6:16   ` [Nouveau] " Alistair Popple
2021-03-04  6:16   ` 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=9fa77684-149a-e6ed-296d-dc852aecea97@nvidia.com \
    --to=rcampbell@nvidia.com \
    --cc=akpm@linux-foundation.org \
    --cc=apopple@nvidia.com \
    --cc=bskeggs@redhat.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hch@lst.de \
    --cc=jglisse@redhat.com \
    --cc=jhubbard@nvidia.com \
    --cc=kvm-ppc@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=nouveau@lists.freedesktop.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.