[PATCH 0/2] userfaultfd: handle few NULL check inline

[PATCH 0/2] userfaultfd: handle few NULL check inline

[Jinjie Ruan]

Handle dup_userfaultfd() and anon_vma_fork() NULL check inline to
save some function call overhead. The Unixbench single core process
create has 1% improve with these patches.

Jinjie Ruan (2):
  userfaultfd: handle dup_userfaultfd() NULL check inline
  mm, rmap: handle anon_vma_fork() NULL check inline

 fs/userfaultfd.c              |  5 +----
 include/linux/rmap.h          | 12 +++++++++++-
 include/linux/userfaultfd_k.h | 11 ++++++++++-
 mm/rmap.c                     |  6 +-----
 4 files changed, 23 insertions(+), 11 deletions(-)

-- 
2.34.1

[PATCH 1/2] userfaultfd: handle dup_userfaultfd() NULL check inline

[Jinjie Ruan]

Make the NULL check for vma's userfaultfd ctx inline, so we can
avoid the function call overhead if the ctx is NULL.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 fs/userfaultfd.c              |  5 +----
 include/linux/userfaultfd_k.h | 11 ++++++++++-
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index 7c0bd0b55f88..7e551c234832 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -615,15 +615,12 @@ static void userfaultfd_event_complete(struct userfaultfd_ctx *ctx,
 	__remove_wait_queue(&ctx->event_wqh, &ewq->wq);
 }
 
-int dup_userfaultfd(struct vm_area_struct *vma, struct list_head *fcs)
+int __dup_userfaultfd(struct vm_area_struct *vma, struct list_head *fcs)
 {
 	struct userfaultfd_ctx *ctx = NULL, *octx;
 	struct userfaultfd_fork_ctx *fctx;
 
 	octx = vma->vm_userfaultfd_ctx.ctx;
-	if (!octx)
-		return 0;
-
 	if (!(octx->features & UFFD_FEATURE_EVENT_FORK)) {
 		userfaultfd_reset_ctx(vma);
 		return 0;
diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
index cb40f1a1d081..06b47104aa1a 100644
--- a/include/linux/userfaultfd_k.h
+++ b/include/linux/userfaultfd_k.h
@@ -247,7 +247,16 @@ static inline bool vma_can_userfault(struct vm_area_struct *vma,
 	    vma_is_shmem(vma);
 }
 
-extern int dup_userfaultfd(struct vm_area_struct *, struct list_head *);
+int __dup_userfaultfd(struct vm_area_struct *, struct list_head *);
+static inline int dup_userfaultfd(struct vm_area_struct *vma,
+				  struct list_head *fcs)
+{
+	if (likely(!vma->vm_userfaultfd_ctx.ctx))
+		return 0;
+
+	return __dup_userfaultfd(vma, fcs);
+}
+
 extern void dup_userfaultfd_complete(struct list_head *);
 void dup_userfaultfd_fail(struct list_head *);
 
-- 
2.34.1

[PATCH 2/2] mm, rmap: handle anon_vma_fork() NULL check inline

[Jinjie Ruan]

Check the anon_vma of pvma inline so we can avoid the function call
overhead if the anon_vma is NULL.

Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 include/linux/rmap.h | 12 +++++++++++-
 mm/rmap.c            |  6 +-----
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/include/linux/rmap.h b/include/linux/rmap.h
index 683a04088f3f..9ddba9b23a1c 100644
--- a/include/linux/rmap.h
+++ b/include/linux/rmap.h
@@ -154,7 +154,17 @@ void anon_vma_init(void);	/* create anon_vma_cachep */
 int  __anon_vma_prepare(struct vm_area_struct *);
 void unlink_anon_vmas(struct vm_area_struct *);
 int anon_vma_clone(struct vm_area_struct *, struct vm_area_struct *);
-int anon_vma_fork(struct vm_area_struct *, struct vm_area_struct *);
+
+int __anon_vma_fork(struct vm_area_struct *, struct vm_area_struct *);
+static inline int anon_vma_fork(struct vm_area_struct *vma,
+				struct vm_area_struct *pvma)
+{
+	/* Don't bother if the parent process has no anon_vma here. */
+	if (!pvma->anon_vma)
+		return 0;
+
+	return __anon_vma_fork(vma, pvma);
+}
 
 static inline int anon_vma_prepare(struct vm_area_struct *vma)
 {
diff --git a/mm/rmap.c b/mm/rmap.c
index c6c4d4ea29a7..06e9b68447c2 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -331,16 +331,12 @@ int anon_vma_clone(struct vm_area_struct *dst, struct vm_area_struct *src)
  * the corresponding VMA in the parent process is attached to.
  * Returns 0 on success, non-zero on failure.
  */
-int anon_vma_fork(struct vm_area_struct *vma, struct vm_area_struct *pvma)
+int __anon_vma_fork(struct vm_area_struct *vma, struct vm_area_struct *pvma)
 {
 	struct anon_vma_chain *avc;
 	struct anon_vma *anon_vma;
 	int error;
 
-	/* Don't bother if the parent process has no anon_vma here. */
-	if (!pvma->anon_vma)
-		return 0;
-
 	/* Drop inherited anon_vma, we'll reuse existing or allocate new. */
 	vma->anon_vma = NULL;
 
-- 
2.34.1

Re: [PATCH 2/2] mm, rmap: handle anon_vma_fork() NULL check inline

[Matthew Wilcox]

On Mon, Dec 09, 2024 at 09:25:49PM +0800, Jinjie Ruan wrote:
> Check the anon_vma of pvma inline so we can avoid the function call
> overhead if the anon_vma is NULL.

This really gets you 1% perf improvement?  On what hardware?

Re: [PATCH 0/2] userfaultfd: handle few NULL check inline

[Lorenzo Stoakes]

On Mon, Dec 09, 2024 at 09:25:47PM +0800, Jinjie Ruan wrote:
> Handle dup_userfaultfd() and anon_vma_fork() NULL check inline to
> save some function call overhead. The Unixbench single core process
> create has 1% improve with these patches.
>
> Jinjie Ruan (2):
>   userfaultfd: handle dup_userfaultfd() NULL check inline
>   mm, rmap: handle anon_vma_fork() NULL check inline
>
>  fs/userfaultfd.c              |  5 +----
>  include/linux/rmap.h          | 12 +++++++++++-
>  include/linux/userfaultfd_k.h | 11 ++++++++++-
>  mm/rmap.c                     |  6 +-----
>  4 files changed, 23 insertions(+), 11 deletions(-)
>
> --
> 2.34.1
>

Coincidentally I've just diagosed a rather nasty bug in this code [0], so
could we hold off on this change for just a little bit until we can get a
fix out for this please?

I'd rather not complicate anything until we're sure we won't need to change
this.

Thanks!


[0]:https://lore.kernel.org/linux-mm/aa2c1930-becc-4bc5-adfb-96e88290acc7@lucifer.local/

Re: [PATCH 0/2] userfaultfd: handle few NULL check inline

[Jinjie Ruan]

On 2024/12/9 21:42, Lorenzo Stoakes wrote:
> On Mon, Dec 09, 2024 at 09:25:47PM +0800, Jinjie Ruan wrote:
>> Handle dup_userfaultfd() and anon_vma_fork() NULL check inline to
>> save some function call overhead. The Unixbench single core process
>> create has 1% improve with these patches.
>>
>> Jinjie Ruan (2):
>>   userfaultfd: handle dup_userfaultfd() NULL check inline
>>   mm, rmap: handle anon_vma_fork() NULL check inline
>>
>>  fs/userfaultfd.c              |  5 +----
>>  include/linux/rmap.h          | 12 +++++++++++-
>>  include/linux/userfaultfd_k.h | 11 ++++++++++-
>>  mm/rmap.c                     |  6 +-----
>>  4 files changed, 23 insertions(+), 11 deletions(-)
>>
>> --
>> 2.34.1
>>
> 
> Coincidentally I've just diagosed a rather nasty bug in this code [0], so
> could we hold off on this change for just a little bit until we can get a
> fix out for this please?
> 
> I'd rather not complicate anything until we're sure we won't need to change
> this.

Sure, fix the bugs is a more urgent problem.

Thanks!

> 
> Thanks!
> 
> 
> [0]:https://lore.kernel.org/linux-mm/aa2c1930-becc-4bc5-adfb-96e88290acc7@lucifer.local/
> 

Re: [PATCH 2/2] mm, rmap: handle anon_vma_fork() NULL check inline

[Jinjie Ruan]

On 2024/12/9 21:35, Matthew Wilcox wrote:
> On Mon, Dec 09, 2024 at 09:25:49PM +0800, Jinjie Ruan wrote:
>> Check the anon_vma of pvma inline so we can avoid the function call
>> overhead if the anon_vma is NULL.
> 
> This really gets you 1% perf improvement?  On what hardware?

Yes，the total improvement of this two patches is about 1% on our
last-generation arm64 server platform.

During the test of Unixbench single-core process creation, the trace
result shows that the two functions are frequently invoked, and a large
number of check NULL and returned.

> 
>