The patch titled
Subject: mm/hmm: use reference counting for HMM struct
has been removed from the -mm tree. Its filename was
mm-hmm-use-reference-counting-for-hmm-struct.patch
This patch was dropped because an updated version will be merged
------------------------------------------------------
From: Jérôme Glisse <jglisse@xxxxxxxxxx>
Subject: mm/hmm: use reference counting for HMM struct
Patch series "HMM updates for 5.1".
This patchset improves the HMM driver API and add support for hugetlbfs
and DAX mirroring. The improvement motivation was to make the ODP to HMM
conversion easier [1]. Because we have nouveau bits schedule for 5.1 and
to avoid any multi-tree synchronization this patchset adds few lines of
inline function that wrap the existing HMM driver API to the improved API.
The nouveau driver was tested before and after this patchset and it
builds and works on both case so there is no merging issue [2]. The
nouveau bit are queue up for 5.1 so this is why i added those inline.
If this get merge in 5.1 the plans is to merge the HMM to ODP in 5.2 or
5.3 if testing shows any issues (so far no issues has been found with
limited testing but Mellanox will be running heavier testing for longer
time).
This is what I intend to use as a base for AMD and Intel patches (v2 with
more thing of some rfc which were already posted in the past).
[1] https://cgit.freedesktop.org/~glisse/linux/log/?h=odp-hmm
[2] https://cgit.freedesktop.org/~glisse/linux/log/?h=hmm-for-5.1
This patch (of 10):
Every time I read the code to check that the HMM structure does not vanish
before it should thanks to the many lock protecting its removal I get a
headache. Switch to reference counting instead it is much easier to
follow and harder to break. This also remove some code that is no longer
needed with refcounting.
Link: http://lkml.kernel.org/r/20190129165428.3931-2-jglisse@xxxxxxxxxx
Signed-off-by: Jérôme Glisse <jglisse@xxxxxxxxxx>
Reviewed-by: Ralph Campbell <rcampbell@xxxxxxxxxx>
Cc: John Hubbard <jhubbard@xxxxxxxxxx>
Cc: Felix Kuehling <Felix.Kuehling@xxxxxxx>
Cc: Christian König <christian.koenig@xxxxxxx>
Cc: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
Cc: Ben Skeggs <bskeggs@xxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---
--- a/include/linux/hmm.h~mm-hmm-use-reference-counting-for-hmm-struct
+++ a/include/linux/hmm.h
@@ -131,6 +131,7 @@ enum hmm_pfn_value_e {
/*
* struct hmm_range - track invalidation lock on virtual address range
*
+ * @hmm: the core HMM structure this range is active against
* @vma: the vm area struct for the range
* @list: all range lock are on a list
* @start: range virtual start address (inclusive)
@@ -142,6 +143,7 @@ enum hmm_pfn_value_e {
* @valid: pfns array did not change since it has been fill by an HMM function
*/
struct hmm_range {
+ struct hmm *hmm;
struct vm_area_struct *vma;
struct list_head list;
unsigned long start;
--- a/mm/hmm.c~mm-hmm-use-reference-counting-for-hmm-struct
+++ a/mm/hmm.c
@@ -50,6 +50,7 @@ static const struct mmu_notifier_ops hmm
*/
struct hmm {
struct mm_struct *mm;
+ struct kref kref;
spinlock_t lock;
struct list_head ranges;
struct list_head mirrors;
@@ -57,6 +58,16 @@ struct hmm {
struct rw_semaphore mirrors_sem;
};
+static inline struct hmm *hmm_get(struct mm_struct *mm)
+{
+ struct hmm *hmm = READ_ONCE(mm->hmm);
+
+ if (hmm && kref_get_unless_zero(&hmm->kref))
+ return hmm;
+
+ return NULL;
+}
+
/*
* hmm_register - register HMM against an mm (HMM internal)
*
@@ -67,14 +78,9 @@ struct hmm {
*/
static struct hmm *hmm_register(struct mm_struct *mm)
{
- struct hmm *hmm = READ_ONCE(mm->hmm);
+ struct hmm *hmm = hmm_get(mm);
bool cleanup = false;
- /*
- * The hmm struct can only be freed once the mm_struct goes away,
- * hence we should always have pre-allocated an new hmm struct
- * above.
- */
if (hmm)
return hmm;
@@ -86,6 +92,7 @@ static struct hmm *hmm_register(struct m
hmm->mmu_notifier.ops = NULL;
INIT_LIST_HEAD(&hmm->ranges);
spin_lock_init(&hmm->lock);
+ kref_init(&hmm->kref);
hmm->mm = mm;
spin_lock(&mm->page_table_lock);
@@ -106,7 +113,7 @@ static struct hmm *hmm_register(struct m
if (__mmu_notifier_register(&hmm->mmu_notifier, mm))
goto error_mm;
- return mm->hmm;
+ return hmm;
error_mm:
spin_lock(&mm->page_table_lock);
@@ -118,9 +125,41 @@ error:
return NULL;
}
+static void hmm_free(struct kref *kref)
+{
+ struct hmm *hmm = container_of(kref, struct hmm, kref);
+ struct mm_struct *mm = hmm->mm;
+
+ mmu_notifier_unregister_no_release(&hmm->mmu_notifier, mm);
+
+ spin_lock(&mm->page_table_lock);
+ if (mm->hmm == hmm)
+ mm->hmm = NULL;
+ spin_unlock(&mm->page_table_lock);
+
+ kfree(hmm);
+}
+
+static inline void hmm_put(struct hmm *hmm)
+{
+ kref_put(&hmm->kref, hmm_free);
+}
+
void hmm_mm_destroy(struct mm_struct *mm)
{
- kfree(mm->hmm);
+ struct hmm *hmm;
+
+ spin_lock(&mm->page_table_lock);
+ hmm = hmm_get(mm);
+ mm->hmm = NULL;
+ if (hmm) {
+ hmm->mm = NULL;
+ spin_unlock(&mm->page_table_lock);
+ hmm_put(hmm);
+ return;
+ }
+
+ spin_unlock(&mm->page_table_lock);
}
static int hmm_invalidate_range(struct hmm *hmm, bool device,
@@ -165,7 +204,7 @@ static int hmm_invalidate_range(struct h
static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
{
struct hmm_mirror *mirror;
- struct hmm *hmm = mm->hmm;
+ struct hmm *hmm = hmm_get(mm);
down_write(&hmm->mirrors_sem);
mirror = list_first_entry_or_null(&hmm->mirrors, struct hmm_mirror,
@@ -186,36 +225,50 @@ static void hmm_release(struct mmu_notif
struct hmm_mirror, list);
}
up_write(&hmm->mirrors_sem);
+
+ hmm_put(hmm);
}
static int hmm_invalidate_range_start(struct mmu_notifier *mn,
const struct mmu_notifier_range *range)
{
struct hmm_update update;
- struct hmm *hmm = range->mm->hmm;
+ struct hmm *hmm = hmm_get(range->mm);
+ int ret;
VM_BUG_ON(!hmm);
+ /* Check if hmm_mm_destroy() was call. */
+ if (hmm->mm == NULL)
+ return 0;
+
update.start = range->start;
update.end = range->end;
update.event = HMM_UPDATE_INVALIDATE;
update.blockable = range->blockable;
- return hmm_invalidate_range(hmm, true, &update);
+ ret = hmm_invalidate_range(hmm, true, &update);
+ hmm_put(hmm);
+ return ret;
}
static void hmm_invalidate_range_end(struct mmu_notifier *mn,
const struct mmu_notifier_range *range)
{
struct hmm_update update;
- struct hmm *hmm = range->mm->hmm;
+ struct hmm *hmm = hmm_get(range->mm);
VM_BUG_ON(!hmm);
+ /* Check if hmm_mm_destroy() was call. */
+ if (hmm->mm == NULL)
+ return;
+
update.start = range->start;
update.end = range->end;
update.event = HMM_UPDATE_INVALIDATE;
update.blockable = true;
hmm_invalidate_range(hmm, false, &update);
+ hmm_put(hmm);
}
static const struct mmu_notifier_ops hmm_mmu_notifier_ops = {
@@ -241,24 +294,13 @@ int hmm_mirror_register(struct hmm_mirro
if (!mm || !mirror || !mirror->ops)
return -EINVAL;
-again:
mirror->hmm = hmm_register(mm);
if (!mirror->hmm)
return -ENOMEM;
down_write(&mirror->hmm->mirrors_sem);
- if (mirror->hmm->mm == NULL) {
- /*
- * A racing hmm_mirror_unregister() is about to destroy the hmm
- * struct. Try again to allocate a new one.
- */
- up_write(&mirror->hmm->mirrors_sem);
- mirror->hmm = NULL;
- goto again;
- } else {
- list_add(&mirror->list, &mirror->hmm->mirrors);
- up_write(&mirror->hmm->mirrors_sem);
- }
+ list_add(&mirror->list, &mirror->hmm->mirrors);
+ up_write(&mirror->hmm->mirrors_sem);
return 0;
}
@@ -273,33 +315,18 @@ EXPORT_SYMBOL(hmm_mirror_register);
*/
void hmm_mirror_unregister(struct hmm_mirror *mirror)
{
- bool should_unregister = false;
- struct mm_struct *mm;
- struct hmm *hmm;
+ struct hmm *hmm = READ_ONCE(mirror->hmm);
- if (mirror->hmm == NULL)
+ if (hmm == NULL)
return;
- hmm = mirror->hmm;
down_write(&hmm->mirrors_sem);
list_del_init(&mirror->list);
- should_unregister = list_empty(&hmm->mirrors);
+ /* To protect us against double unregister ... */
mirror->hmm = NULL;
- mm = hmm->mm;
- hmm->mm = NULL;
up_write(&hmm->mirrors_sem);
- if (!should_unregister || mm == NULL)
- return;
-
- mmu_notifier_unregister_no_release(&hmm->mmu_notifier, mm);
-
- spin_lock(&mm->page_table_lock);
- if (mm->hmm == hmm)
- mm->hmm = NULL;
- spin_unlock(&mm->page_table_lock);
-
- kfree(hmm);
+ hmm_put(hmm);
}
EXPORT_SYMBOL(hmm_mirror_unregister);
@@ -708,6 +735,8 @@ int hmm_vma_get_pfns(struct hmm_range *r
struct mm_walk mm_walk;
struct hmm *hmm;
+ range->hmm = NULL;
+
/* Sanity check, this really should not happen ! */
if (range->start < vma->vm_start || range->start >= vma->vm_end)
return -EINVAL;
@@ -717,14 +746,18 @@ int hmm_vma_get_pfns(struct hmm_range *r
hmm = hmm_register(vma->vm_mm);
if (!hmm)
return -ENOMEM;
- /* Caller must have registered a mirror, via hmm_mirror_register() ! */
- if (!hmm->mmu_notifier.ops)
+
+ /* Check if hmm_mm_destroy() was call. */
+ if (hmm->mm == NULL) {
+ hmm_put(hmm);
return -EINVAL;
+ }
/* FIXME support hugetlb fs */
if (is_vm_hugetlb_page(vma) || (vma->vm_flags & VM_SPECIAL) ||
vma_is_dax(vma)) {
hmm_pfns_special(range);
+ hmm_put(hmm);
return -EINVAL;
}
@@ -736,6 +769,7 @@ int hmm_vma_get_pfns(struct hmm_range *r
* operations such has atomic access would not work.
*/
hmm_pfns_clear(range, range->pfns, range->start, range->end);
+ hmm_put(hmm);
return -EPERM;
}
@@ -758,6 +792,12 @@ int hmm_vma_get_pfns(struct hmm_range *r
mm_walk.pte_hole = hmm_vma_walk_hole;
walk_page_range(range->start, range->end, &mm_walk);
+ /*
+ * Transfer hmm reference to the range struct it will be drop inside
+ * the hmm_vma_range_done() function (which _must_ be call if this
+ * function return 0).
+ */
+ range->hmm = hmm;
return 0;
}
EXPORT_SYMBOL(hmm_vma_get_pfns);
@@ -802,25 +842,27 @@ EXPORT_SYMBOL(hmm_vma_get_pfns);
*/
bool hmm_vma_range_done(struct hmm_range *range)
{
- unsigned long npages = (range->end - range->start) >> PAGE_SHIFT;
- struct hmm *hmm;
+ bool ret = false;
- if (range->end <= range->start) {
+ /* Sanity check this really should not happen. */
+ if (range->hmm == NULL || range->end <= range->start) {
BUG();
return false;
}
- hmm = hmm_register(range->vma->vm_mm);
- if (!hmm) {
- memset(range->pfns, 0, sizeof(*range->pfns) * npages);
- return false;
- }
-
- spin_lock(&hmm->lock);
+ spin_lock(&range->hmm->lock);
list_del_rcu(&range->list);
- spin_unlock(&hmm->lock);
+ ret = range->valid;
+ spin_unlock(&range->hmm->lock);
- return range->valid;
+ /* Is the mm still alive ? */
+ if (range->hmm->mm == NULL)
+ ret = false;
+
+ /* Drop reference taken by hmm_vma_fault() or hmm_vma_get_pfns() */
+ hmm_put(range->hmm);
+ range->hmm = NULL;
+ return ret;
}
EXPORT_SYMBOL(hmm_vma_range_done);
@@ -880,6 +922,8 @@ int hmm_vma_fault(struct hmm_range *rang
struct hmm *hmm;
int ret;
+ range->hmm = NULL;
+
/* Sanity check, this really should not happen ! */
if (range->start < vma->vm_start || range->start >= vma->vm_end)
return -EINVAL;
@@ -891,14 +935,18 @@ int hmm_vma_fault(struct hmm_range *rang
hmm_pfns_clear(range, range->pfns, range->start, range->end);
return -ENOMEM;
}
- /* Caller must have registered a mirror using hmm_mirror_register() */
- if (!hmm->mmu_notifier.ops)
+
+ /* Check if hmm_mm_destroy() was call. */
+ if (hmm->mm == NULL) {
+ hmm_put(hmm);
return -EINVAL;
+ }
/* FIXME support hugetlb fs */
if (is_vm_hugetlb_page(vma) || (vma->vm_flags & VM_SPECIAL) ||
vma_is_dax(vma)) {
hmm_pfns_special(range);
+ hmm_put(hmm);
return -EINVAL;
}
@@ -910,6 +958,7 @@ int hmm_vma_fault(struct hmm_range *rang
* operations such has atomic access would not work.
*/
hmm_pfns_clear(range, range->pfns, range->start, range->end);
+ hmm_put(hmm);
return -EPERM;
}
@@ -945,7 +994,16 @@ int hmm_vma_fault(struct hmm_range *rang
hmm_pfns_clear(range, &range->pfns[i], hmm_vma_walk.last,
range->end);
hmm_vma_range_done(range);
+ hmm_put(hmm);
+ } else {
+ /*
+ * Transfer hmm reference to the range struct it will be drop
+ * inside the hmm_vma_range_done() function (which _must_ be
+ * call if this function return 0).
+ */
+ range->hmm = hmm;
}
+
return ret;
}
EXPORT_SYMBOL(hmm_vma_fault);
_
Patches currently in -mm which might be from jglisse@xxxxxxxxxx are
