All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Kravetz <mike.kravetz@oracle.com>
To: riel@surriel.com
Cc: linux-kernel@vger.kernel.org, kernel-team@meta.com,
	linux-mm@kvack.org, akpm@linux-foundation.org,
	muchun.song@linux.dev, leit@meta.com, willy@infradead.org
Subject: Re: [PATCH 1/3] hugetlbfs: extend hugetlb_vma_lock to private VMAs
Date: Fri, 29 Sep 2023 19:28:42 -0700	[thread overview]
Message-ID: <20230930022842.GA82828@monkey> (raw)
In-Reply-To: <20230926031245.795759-2-riel@surriel.com>

On 09/25/23 23:10, riel@surriel.com wrote:
> From: Rik van Riel <riel@surriel.com>
> 
> Extend the locking scheme used to protect shared hugetlb mappings
> from truncate vs page fault races, in order to protect private
> hugetlb mappings (with resv_map) against MADV_DONTNEED.
> 
> Add a read-write semaphore to the resv_map data structure, and
> use that from the hugetlb_vma_(un)lock_* functions, in preparation
> for closing the race between MADV_DONTNEED and page faults.
> 
> Signed-off-by: Rik van Riel <riel@surriel.com>
> Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
> ---
>  include/linux/hugetlb.h |  6 ++++++
>  mm/hugetlb.c            | 41 +++++++++++++++++++++++++++++++++++++----
>  2 files changed, 43 insertions(+), 4 deletions(-)

My bad during the review of patch 2!

In reply to patch 1, I suggested the changes:

> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index f906c5fa4d09..8f3d5895fffc 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -372,6 +372,11 @@ static void __hugetlb_vma_unlock_write_free(struct vm_area_struct *vma)
>  		struct hugetlb_vma_lock *vma_lock = vma->vm_private_data;
>  
>  		__hugetlb_vma_unlock_write_put(vma_lock);
> +	} else if (__vma_private_lock(vma)) {
> +		struct resv_map *resv_map = vma_resv_map(vma);
> +
> +		/* no free for anon vmas, but still need to unlock */
> +		up_write(&resv_map->rw_sema);
>  	}
>  }

However, the check for 'if (__vma_private_lock(vma))' was dropped.

> @@ -345,6 +372,11 @@ static void __hugetlb_vma_unlock_write_free(struct vm_area_struct *vma)
>  		struct hugetlb_vma_lock *vma_lock = vma->vm_private_data;
>  
>  		__hugetlb_vma_unlock_write_put(vma_lock);
> +	} else {
> +		struct resv_map *resv_map = vma_resv_map(vma);
> +
> +		/* no free for anon vmas, but still need to unlock */
> +		up_write(&resv_map->rw_sema);
>  	}
>  }

So, the map_high_truncate_2 (2M: 32) libhugetlbfs test still BUGs with:

BUG: kernel NULL pointer dereference
-- 
Mike Kravetz


  reply	other threads:[~2023-09-30  2:29 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-26  3:10 [PATCH v4 0/3] hugetlbfs: close race between MADV_DONTNEED and page fault riel
2023-09-26  3:10 ` [PATCH 1/3] hugetlbfs: extend hugetlb_vma_lock to private VMAs riel
2023-09-30  2:28   ` Mike Kravetz [this message]
2023-09-30 19:48     ` Rik van Riel
2023-09-26  3:10 ` [PATCH 2/3] hugetlbfs: close race between MADV_DONTNEED and page fault riel
2023-09-26 21:15   ` Andrew Morton
2023-09-26 23:57     ` Rik van Riel
2023-09-26  3:10 ` [PATCH 3/3] hugetlbfs: replace hugetlb_vma_lock with invalidate_lock riel
  -- strict thread matches above, loose matches on Subject: below --
2023-10-04  3:25 [PATCH v6 0/3] hugetlbfs: close race between MADV_DONTNEED and page fault riel
2023-10-04  3:25 ` [PATCH 1/3] hugetlbfs: extend hugetlb_vma_lock to private VMAs riel
2023-10-01  0:55 [PATCH v5 0/3] hugetlbfs: close race between MADV_DONTNEED and page fault riel
2023-10-01  0:55 ` [PATCH 1/3] hugetlbfs: extend hugetlb_vma_lock to private VMAs riel
2023-09-25 20:28 [PATCH v3 0/3] hugetlbfs: close race between MADV_DONTNEED and page fault riel
2023-09-25 20:28 ` [PATCH 1/3] hugetlbfs: extend hugetlb_vma_lock to private VMAs riel
2023-09-25 22:25   ` Mike Kravetz
2023-09-22 19:02 [PATCH v2 0/3] hugetlbfs: close race between MADV_DONTNEED and page fault riel
2023-09-22 19:02 ` [PATCH 1/3] hugetlbfs: extend hugetlb_vma_lock to private VMAs riel

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=20230930022842.GA82828@monkey \
    --to=mike.kravetz@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=kernel-team@meta.com \
    --cc=leit@meta.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=muchun.song@linux.dev \
    --cc=riel@surriel.com \
    --cc=willy@infradead.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.