From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.linutronix.de (146.0.238.70:993) by crypto-ml.lab.linutronix.de with IMAP4-SSL for ; 20 Jun 2018 23:05:58 -0000 Received: from mail.linuxfoundation.org ([140.211.169.12]) by Galois.linutronix.de with esmtps (TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1fVmAn-0006O9-3F for speck@linutronix.de; Thu, 21 Jun 2018 01:05:57 +0200 Date: Thu, 21 Jun 2018 08:05:24 +0900 From: Greg KH Subject: [MODERATED] Re: [patch V4 05/13] cpu/hotplug: Provide knobs to control SMT Message-ID: <20180620230524.GA31814@kroah.com> References: <20180620201907.304694346@linutronix.de> <20180620201932.889408354@linutronix.de> MIME-Version: 1.0 In-Reply-To: <20180620201932.889408354@linutronix.de> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit To: speck@linutronix.de List-ID: On Wed, Jun 20, 2018 at 10:19:12PM +0200, speck for Thomas Gleixner wrote: > +static int cpuhp_smt_disable(enum cpuhp_smt_control ctrlval) > +{ > + int cpu, ret = 0; > + > + cpu_maps_update_begin(); > + for_each_online_cpu(cpu) { > + struct device *dev; > + > + if (topology_is_primary_thread(cpu)) > + continue; > + ret = cpu_down_maps_locked(cpu, CPUHP_OFFLINE); > + if (ret) > + break; > + /* > + * As this needs to hold the cpu maps lock it's impossible > + * to call device_offline(), so nothing would update > + * device:offline state. That would leave the sysfs entry > + * stale and prevent onlining after smt control has been > + * changed to 'off' again. This is called under the sysfs > + * hotplug lock, so it is properly serialized. > + * > + * Temporary workaround until Greg has a smarter idea to do > + * that. > + */ > + dev = get_cpu_device(cpu); > + dev->offline = true; You are right to call me out here. Ugh. I'm missing where device_offline() wants to call any lock other than the lock on the device you are offlineing. Why can't that be called here? If you do just want to "open code" this, you should also tell userspace what just happened, so should you add this line as well: kobject_uevent(&dev->kobj, KOBJ_OFFLINE); thanks, greg k-h