diff mbox series

[v4,01/10] mmap locking API: initial implementation as rwsem wrappers

Message ID 20200415004353.130248-2-walken@google.com (mailing list archive)
State New, archived
Headers show
Series Add a new mmap locking API wrapping mmap_sem calls | expand

Commit Message

Michel Lespinasse April 15, 2020, 12:43 a.m. UTC
This change wraps the existing mmap_sem related rwsem calls into a new
mmap locking API. There are two justifications for the new API:

- At first, it provides an easy hooking point to instrument mmap_sem
  locking latencies independently of any other rwsems.

- In the future, it may be a starting point for replacing the rwsem
  implementation with a different one, such as range locks.

Signed-off-by: Michel Lespinasse <walken@google.com>
Reviewed-by: Daniel Jordan <daniel.m.jordan@oracle.com>
---
 include/linux/mm.h        |  1 +
 include/linux/mmap_lock.h | 54 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)
 create mode 100644 include/linux/mmap_lock.h

Comments

Davidlohr Bueso April 20, 2020, 7:48 p.m. UTC | #1
On Tue, 14 Apr 2020, Michel Lespinasse wrote:

>This change wraps the existing mmap_sem related rwsem calls into a new
>mmap locking API. There are two justifications for the new API:
>
>- At first, it provides an easy hooking point to instrument mmap_sem
>  locking latencies independently of any other rwsems.
>
>- In the future, it may be a starting point for replacing the rwsem
>  implementation with a different one, such as range locks.
>
>Signed-off-by: Michel Lespinasse <walken@google.com>
>Reviewed-by: Daniel Jordan <daniel.m.jordan@oracle.com>

Reviewed-by: Davidlohr Bueso <dbueso@suse.de>

With one observation below.

>+static inline void mmap_downgrade_write_lock(struct mm_struct *mm)
>+{
>+	downgrade_write(&mm->mmap_sem);
>+}

Shouldn't this really be just mmap_downgrade_write()? In locking
normally don't add the _lock at the end as it implies the operation
of acquiring the lock.

Thanks,
Davidlohr
Michel Lespinasse April 21, 2020, 12:48 a.m. UTC | #2
On Mon, Apr 20, 2020 at 12:51 PM Davidlohr Bueso <dave@stgolabs.net> wrote:
> With one observation below.
>
> >+static inline void mmap_downgrade_write_lock(struct mm_struct *mm)
> >+{
> >+      downgrade_write(&mm->mmap_sem);
> >+}
>
> Shouldn't this really be just mmap_downgrade_write()? In locking
> normally don't add the _lock at the end as it implies the operation
> of acquiring the lock.

Hmmm, I see your point. Actually here the operation is downgrade, so
arguably the name should be mmap_write_downgrade ? I'm not sure I like
the name though, as it does not make it entirely obvious that the
function is a locking primitive.
diff mbox series

Patch

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 5a323422d783..051ec782bdbb 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -15,6 +15,7 @@ 
 #include <linux/atomic.h>
 #include <linux/debug_locks.h>
 #include <linux/mm_types.h>
+#include <linux/mmap_lock.h>
 #include <linux/range.h>
 #include <linux/pfn.h>
 #include <linux/percpu-refcount.h>
diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
new file mode 100644
index 000000000000..8b5a3cd56118
--- /dev/null
+++ b/include/linux/mmap_lock.h
@@ -0,0 +1,54 @@ 
+#ifndef _LINUX_MMAP_LOCK_H
+#define _LINUX_MMAP_LOCK_H
+
+static inline void mmap_init_lock(struct mm_struct *mm)
+{
+	init_rwsem(&mm->mmap_sem);
+}
+
+static inline void mmap_write_lock(struct mm_struct *mm)
+{
+	down_write(&mm->mmap_sem);
+}
+
+static inline int mmap_write_lock_killable(struct mm_struct *mm)
+{
+	return down_write_killable(&mm->mmap_sem);
+}
+
+static inline bool mmap_write_trylock(struct mm_struct *mm)
+{
+	return down_write_trylock(&mm->mmap_sem) != 0;
+}
+
+static inline void mmap_write_unlock(struct mm_struct *mm)
+{
+	up_write(&mm->mmap_sem);
+}
+
+static inline void mmap_downgrade_write_lock(struct mm_struct *mm)
+{
+	downgrade_write(&mm->mmap_sem);
+}
+
+static inline void mmap_read_lock(struct mm_struct *mm)
+{
+	down_read(&mm->mmap_sem);
+}
+
+static inline int mmap_read_lock_killable(struct mm_struct *mm)
+{
+	return down_read_killable(&mm->mmap_sem);
+}
+
+static inline bool mmap_read_trylock(struct mm_struct *mm)
+{
+	return down_read_trylock(&mm->mmap_sem) != 0;
+}
+
+static inline void mmap_read_unlock(struct mm_struct *mm)
+{
+	up_read(&mm->mmap_sem);
+}
+
+#endif /* _LINUX_MMAP_LOCK_H */