| Message ID | 20200415004353.130248-9-walken@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add a new mmap locking API wrapping mmap_sem calls | expand |
On Tue, Apr 14, 2020 at 05:43:51PM -0700, Michel Lespinasse wrote: > Define a new initializer for the mmap locking api. > Initially this just evaluates to __RWSEM_INITIALIZER as the API > is defined as wrappers around rwsem. Shouldn't this take mm as the argument like the other stuff in mmap_lock.h?
On Tue, Apr 14, 2020 at 05:43:51PM -0700, Michel Lespinasse wrote: > @@ -90,7 +90,7 @@ static struct mm_struct tboot_mm = { ^^^^^^^^ > .pgd = swapper_pg_dir, > .mm_users = ATOMIC_INIT(2), > .mm_count = ATOMIC_INIT(1), > - .mmap_sem = __RWSEM_INITIALIZER(init_mm.mmap_sem), > + .mmap_sem = MMAP_LOCK_INITIALIZER(init_mm.mmap_sem), ^^^^^^^ Shome mishtake, shirley? I don't see that this particular patch buys us much. The name 'mmap_sem' is still used, and I appreciate we abstract away the type of the lock, but wouldn't this be better? - .mmap_sem = __RWSEM_INITIALIZER(init_mm.mmap_sem), + MMAP_LOCK_INITIALIZER(tboot_mm),
On Mon, Apr 20, 2020 at 12:28 PM Matthew Wilcox <willy@infradead.org> wrote: > > On Tue, Apr 14, 2020 at 05:43:51PM -0700, Michel Lespinasse wrote: > > @@ -90,7 +90,7 @@ static struct mm_struct tboot_mm = { > ^^^^^^^^ > > .pgd = swapper_pg_dir, > > .mm_users = ATOMIC_INIT(2), > > .mm_count = ATOMIC_INIT(1), > > - .mmap_sem = __RWSEM_INITIALIZER(init_mm.mmap_sem), > > + .mmap_sem = MMAP_LOCK_INITIALIZER(init_mm.mmap_sem), > ^^^^^^^ > > Shome mishtake, shirley? > > I don't see that this particular patch buys us much. The name 'mmap_sem' > is still used, and I appreciate we abstract away the type of the lock, > but wouldn't this be better? > > - .mmap_sem = __RWSEM_INITIALIZER(init_mm.mmap_sem), > + MMAP_LOCK_INITIALIZER(tboot_mm), Hmmm, that's significantly different from other initializers we have, which may be a downside ? But other than that, it does seem completely workable to me. Do you have a strong preference ?
diff --git a/arch/x86/kernel/tboot.c b/arch/x86/kernel/tboot.c index b89f6ac6a0c0..4b79335624b1 100644 --- a/arch/x86/kernel/tboot.c +++ b/arch/x86/kernel/tboot.c @@ -90,7 +90,7 @@ static struct mm_struct tboot_mm = { .pgd = swapper_pg_dir, .mm_users = ATOMIC_INIT(2), .mm_count = ATOMIC_INIT(1), - .mmap_sem = __RWSEM_INITIALIZER(init_mm.mmap_sem), + .mmap_sem = MMAP_LOCK_INITIALIZER(init_mm.mmap_sem), .page_table_lock = __SPIN_LOCK_UNLOCKED(init_mm.page_table_lock), .mmlist = LIST_HEAD_INIT(init_mm.mmlist), }; diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c index 911a2bd0f6b7..4bac9d7a48a6 100644 --- a/drivers/firmware/efi/efi.c +++ b/drivers/firmware/efi/efi.c @@ -54,7 +54,7 @@ struct mm_struct efi_mm = { .mm_rb = RB_ROOT, .mm_users = ATOMIC_INIT(2), .mm_count = ATOMIC_INIT(1), - .mmap_sem = __RWSEM_INITIALIZER(efi_mm.mmap_sem), + .mmap_sem = MMAP_LOCK_INITIALIZER(efi_mm.mmap_sem), .page_table_lock = __SPIN_LOCK_UNLOCKED(efi_mm.page_table_lock), .mmlist = LIST_HEAD_INIT(efi_mm.mmlist), .cpu_bitmap = { [BITS_TO_LONGS(NR_CPUS)] = 0}, diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h index 1050257361aa..07efa379664a 100644 --- a/include/linux/mmap_lock.h +++ b/include/linux/mmap_lock.h @@ -1,6 +1,8 @@ #ifndef _LINUX_MMAP_LOCK_H #define _LINUX_MMAP_LOCK_H +#define MMAP_LOCK_INITIALIZER(name) __RWSEM_INITIALIZER(name) + static inline void mmap_init_lock(struct mm_struct *mm) { init_rwsem(&mm->mmap_sem); diff --git a/mm/init-mm.c b/mm/init-mm.c index 19603302a77f..3c128bd6a30c 100644 --- a/mm/init-mm.c +++ b/mm/init-mm.c @@ -31,7 +31,7 @@ struct mm_struct init_mm = { .pgd = swapper_pg_dir, .mm_users = ATOMIC_INIT(2), .mm_count = ATOMIC_INIT(1), - .mmap_sem = __RWSEM_INITIALIZER(init_mm.mmap_sem), + .mmap_sem = MMAP_LOCK_INITIALIZER(init_mm.mmap_sem), .page_table_lock = __SPIN_LOCK_UNLOCKED(init_mm.page_table_lock), .arg_lock = __SPIN_LOCK_UNLOCKED(init_mm.arg_lock), .mmlist = LIST_HEAD_INIT(init_mm.mmlist),
Define a new initializer for the mmap locking api. Initially this just evaluates to __RWSEM_INITIALIZER as the API is defined as wrappers around rwsem. Signed-off-by: Michel Lespinasse <walken@google.com> --- arch/x86/kernel/tboot.c | 2 +- drivers/firmware/efi/efi.c | 2 +- include/linux/mmap_lock.h | 2 ++ mm/init-mm.c | 2 +- 4 files changed, 5 insertions(+), 3 deletions(-)