Messages in this thread | | | Date | Tue, 12 Nov 2019 08:14:06 +0100 | From | Ingo Molnar <> | Subject | Re: [patch V2 11/16] x86/ioperm: Share I/O bitmap if identical |
| |
* 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];
> +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(¤t->thread.io_bitmap->refcnt); > + tsk->thread.io_bitmap = current->thread.io_bitmap; > + set_tsk_thread_flag(tsk, TIF_IO_BITMAP); > +}
Ok, this is really neat. I suspect there might be some pathological cases on ancient NUMA systems with a really high NUMA factor and bad caching where this new sharing might regress performance, but I doubt this matters, as both the hardware and this software functionality is legacy.
> + /* > + * 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(); > } > > + /* Set the tasks io_bitmap pointer (might be the same) */
speling nit:
s/tasks /task's
Thanks,
Ingo
| |