lkml.org 
[lkml]   [2019]   [Nov]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [patch V2 11/16] x86/ioperm: Share I/O bitmap if identical
On Mon, Nov 11, 2019 at 2:35 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> The I/O bitmap is duplicated on fork. That's wasting memory and slows down
> fork. There is no point to do so. As long as the bitmap is not modified it
> can be shared between threads and processes.
>
> Add a refcount and just share it on fork. If a task modifies the bitmap
> then it has to do the duplication if and only if it is shared.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> V2: New patch
> ---
> arch/x86/include/asm/iobitmap.h | 5 +++++
> arch/x86/kernel/ioport.c | 38 ++++++++++++++++++++++++++++++++------
> arch/x86/kernel/process.c | 39 ++++++---------------------------------
> 3 files changed, 43 insertions(+), 39 deletions(-)
>
> --- a/arch/x86/include/asm/iobitmap.h
> +++ b/arch/x86/include/asm/iobitmap.h
> @@ -2,10 +2,12 @@
> #ifndef _ASM_X86_IOBITMAP_H
> #define _ASM_X86_IOBITMAP_H
>
> +#include <linux/refcount.h>
> #include <asm/processor.h>
>
> struct io_bitmap {
> u64 sequence;
> + refcount_t refcnt;
> unsigned int io_bitmap_max;
> union {
> unsigned long bits[IO_BITMAP_LONGS];
> @@ -13,6 +15,9 @@ struct io_bitmap {
> };
> };
>
> +struct task_struct;
> +
> +void io_bitmap_share(struct task_struct *tsk);
> void io_bitmap_exit(void);
>
> void tss_update_io_bitmap(void);
> --- a/arch/x86/kernel/ioport.c
> +++ b/arch/x86/kernel/ioport.c
> @@ -16,6 +16,17 @@
>
> static atomic64_t io_bitmap_sequence;
>
> +void io_bitmap_share(struct task_struct *tsk)
> + {
> + /*
> + * Take a refcount on current's bitmap. It can be used by
> + * both tasks as long as none of them changes the bitmap.
> + */
> + refcount_inc(&current->thread.io_bitmap->refcnt);
> + tsk->thread.io_bitmap = current->thread.io_bitmap;
> + set_tsk_thread_flag(tsk, TIF_IO_BITMAP);
> +}
> +
> void io_bitmap_exit(void)
> {
> struct io_bitmap *iobm = current->thread.io_bitmap;
> @@ -25,7 +36,8 @@ void io_bitmap_exit(void)
> clear_thread_flag(TIF_IO_BITMAP);
> tss_update_io_bitmap();
> preempt_enable();
> - kfree(iobm);
> + if (iobm && refcount_dec_and_test(&iobm->refcnt))
> + kfree(iobm);
> }
>
> /*
> @@ -59,8 +71,26 @@ long ksys_ioperm(unsigned long from, uns
> return -ENOMEM;
>
> memset(iobm->bits, 0xff, sizeof(iobm->bits));
> + refcount_set(&iobm->refcnt, 1);
> + }
> +
> + /*
> + * If the bitmap is not shared, then nothing can take a refcount as
> + * current can obviously not fork at the same time. If it's shared
> + * duplicate it and drop the refcount on the original one.
> + */
> + if (refcount_read(&iobm->refcnt) > 1) {
> + iobm = kmemdup(iobm, sizeof(*iobm), GFP_KERNEL);
> + if (!iobm)
> + return -ENOMEM;
> + io_bitmap_exit();

And change the refcount to 1?

> }
>

Otherwise:

Acked-by: Andy Lutomirski <luto@kernel.org>

(I'm about to send, and I see PeterZ beat me to the punch. You can
still have the ack, though.)

\
 
 \ /
  Last update: 2019-11-12 19:12    [W:0.225 / U:0.420 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site