From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753180AbbFVW7C (ORCPT ); Mon, 22 Jun 2015 18:59:02 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50822 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751511AbbFVW6y (ORCPT ); Mon, 22 Jun 2015 18:58:54 -0400 Date: Tue, 23 Jun 2015 00:57:39 +0200 From: Oleg Nesterov To: Peter Zijlstra Cc: paulmck@linux.vnet.ibm.com, tj@kernel.org, mingo@redhat.com, linux-kernel@vger.kernel.org, der.herr@hofr.at, dave@stgolabs.net, riel@redhat.com, viro@ZenIV.linux.org.uk, torvalds@linux-foundation.org Subject: Re: [RFC][PATCH 09/13] hotplug: Replace hotplug lock with percpu-rwsem Message-ID: <20150622225739.GA5582@redhat.com> References: <20150622121623.291363374@infradead.org> <20150622122256.480062572@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150622122256.480062572@infradead.org> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/22, Peter Zijlstra wrote: > > The cpu hotplug lock is a rwsem with read-in-write and read-in-read > recursion. Implement it as such. And this patch fixes the problem afaics. Currently cpu_hotplug_begin() can livelock because it doesn't stop the new readers. With this patch this is no longer possible. > -static inline void percpu_down_read(struct percpu_rw_semaphore *sem) > +static inline void _percpu_down_read(struct percpu_rw_semaphore *sem) > { > might_sleep(); > > - rwsem_acquire_read(&sem->rw_sem.dep_map, 0, 0, _RET_IP_); > - > preempt_disable(); > /* > * We are in an RCU-sched read-side critical section, so the writer > @@ -46,6 +44,12 @@ static inline void percpu_down_read(stru > */ > } > > +static inline void percpu_down_read(struct percpu_rw_semaphore *sem) > +{ > + rwsem_acquire_read(&sem->rw_sem.dep_map, 0, 0, _RET_IP_); > + _percpu_down_read(sem); > +} ... > void get_online_cpus(void) > { > might_sleep(); > - if (cpu_hotplug.active_writer == current) > + > + /* read in write recursion */ > + if (cpu_hotplug.writer == current) > + return; > + > + /* read in read recursion */ > + if (current->cpuhp_ref++) > return; > - cpuhp_lock_acquire_read(); > - mutex_lock(&cpu_hotplug.lock); > - atomic_inc(&cpu_hotplug.refcount); > - mutex_unlock(&cpu_hotplug.lock); > + > + lock_map_acquire_read(&cpu_hotplug.rwsem.rw_sem.dep_map); > + _percpu_down_read(&cpu_hotplug.rwsem); > } Confused... Why do we need _percpu_down_read()? Can't get_online_cpus() just use percpu_down_read() ? Yes, percpu_down_read() is not recursive, like the normal down_read(). But this does not matter because we rely on ->cpuhp_ref anyway? > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -1410,6 +1410,8 @@ static struct task_struct *copy_process( > p->sequential_io_avg = 0; > #endif > > + cpu_hotplug_init_task(p); This is probably unnecessary, copy_process() should not be called under get_online_cpus(). Oleg. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in Please read the FAQ at http://www.tux.org/lkml/