On Thu 14-12-17 13:30:56, David Rientjes wrote:
> Commit 4d4bbd8526a8 ("mm, oom_reaper: skip mm structs with mmu notifiers")
> prevented the oom reaper from unmapping private anonymous memory with the
> oom reaper when the oom victim mm had mmu notifiers registered.
>
> The rationale is that doing mmu_notifier_invalidate_range_{start,end}()
> around the unmap_page_range(), which is needed, can block and the oom
> killer will stall forever waiting for the victim to exit, which may not
> be possible without reaping.
>
> That concern is real, but only true for mmu notifiers that have blockable
> invalidate_range_{start,end}() callbacks. This patch adds a "flags" field
> to mmu notifier ops that can set a bit to indicate that these callbacks do
> not block.
>
> The implementation is steered toward an expensive slowpath, such as after
> the oom reaper has grabbed mm->mmap_sem of a still alive oom victim.
>
> Signed-off-by: David Rientjes <rientjes@xxxxxxxxxx>
Yes, this make sense. I haven't checked all the existing mmu notifiers
but those that you have marked seem to be OK.
I just think that the semantic of the flag should be describe more. See
below
Acked-by: Michal Hocko <mhocko@xxxxxxxx>
> ---
> v2:
> - specifically exclude mmu_notifiers without invalidate callbacks
> - move flags to mmu_notifier_ops per Paolo
> - reverse flag from blockable -> not blockable per Christian
>
> drivers/infiniband/hw/hfi1/mmu_rb.c | 1 +
> drivers/iommu/amd_iommu_v2.c | 1 +
> drivers/iommu/intel-svm.c | 1 +
> drivers/misc/sgi-gru/grutlbpurge.c | 1 +
> include/linux/mmu_notifier.h | 21 +++++++++++++++++++++
> mm/mmu_notifier.c | 31 +++++++++++++++++++++++++++++++
> virt/kvm/kvm_main.c | 1 +
> 7 files changed, 57 insertions(+)
>
[...]
> diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
> --- a/include/linux/mmu_notifier.h
> +++ b/include/linux/mmu_notifier.h
> @@ -10,6 +10,9 @@
> struct mmu_notifier;
> struct mmu_notifier_ops;
>
> +/* mmu_notifier_ops flags */
> +#define MMU_INVALIDATE_DOES_NOT_BLOCK (0x01)
> +
> #ifdef CONFIG_MMU_NOTIFIER
>
> /*
> @@ -26,6 +29,15 @@ struct mmu_notifier_mm {
> };
>
> struct mmu_notifier_ops {
> + /*
> + * Flags to specify behavior of callbacks for this MMU notifier.
> + * Used to determine which context an operation may be called.
> + *
> + * MMU_INVALIDATE_DOES_NOT_BLOCK: invalidate_{start,end} does not
> + * block
> + */
> + int flags;
This should be more specific IMHO. What do you think about the following
wording?
invalidate_{start,end,range} doesn't block on any locks which depend
directly or indirectly (via lock chain or resources e.g. worker context)
on a memory allocation.
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -476,6 +476,7 @@ static void kvm_mmu_notifier_release(struct mmu_notifier *mn,
> }
>
> static const struct mmu_notifier_ops kvm_mmu_notifier_ops = {
> + .flags = MMU_INVALIDATE_DOES_NOT_BLOCK,
> .invalidate_range_start = kvm_mmu_notifier_invalidate_range_start,
> .invalidate_range_end = kvm_mmu_notifier_invalidate_range_end,
> .clear_flush_young = kvm_mmu_notifier_clear_flush_young,
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
