Re: [PATCH 3/4] mm/hmm: HMM should have a callback before MM is destroyed

From: Andrew Morton
Date: Thu Mar 15 2018 - 18:48:37 EST


On Thu, 15 Mar 2018 14:36:59 -0400 jglisse@xxxxxxxxxx wrote:

> From: Ralph Campbell <rcampbell@xxxxxxxxxx>
>
> The hmm_mirror_register() function registers a callback for when
> the CPU pagetable is modified. Normally, the device driver will
> call hmm_mirror_unregister() when the process using the device is
> finished. However, if the process exits uncleanly, the struct_mm
> can be destroyed with no warning to the device driver.

The changelog doesn't tell us what the runtime effects of the bug are.
This makes it hard for me to answer the "did Jerome consider doing
cc:stable" question.

> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -160,6 +160,23 @@ static void hmm_invalidate_range(struct hmm *hmm,
> up_read(&hmm->mirrors_sem);
> }
>
> +static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
> +{
> + struct hmm *hmm = mm->hmm;
> + struct hmm_mirror *mirror;
> + struct hmm_mirror *mirror_next;
> +
> + VM_BUG_ON(!hmm);

This doesn't add much value. We'll reliably oops on the next statement
anyway, which will provide the same info. And Linus gets all upset at
new BUG_ON() instances.

> + down_write(&hmm->mirrors_sem);
> + list_for_each_entry_safe(mirror, mirror_next, &hmm->mirrors, list) {
> + list_del_init(&mirror->list);
> + if (mirror->ops->release)
> + mirror->ops->release(mirror);
> + }
> + up_write(&hmm->mirrors_sem);
> +}
> +