Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751946AbdFVBO5 (ORCPT ); Wed, 21 Jun 2017 21:14:57 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:42041 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751113AbdFVBOz (ORCPT ); Wed, 21 Jun 2017 21:14:55 -0400 References: <1497996510-4032-1-git-send-email-bauerman@linux.vnet.ibm.com> <8737attzw5.fsf@concordia.ellerman.id.au> From: Thiago Jung Bauermann To: Michael Ellerman Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, John Allen , Michael Bringmann , Nathan Fontenot , Thomas Gleixner , Sebastian Andrzej Siewior Subject: Re: [PATCH] powerpc: Only obtain cpu_hotplug_lock if called by rtasd In-reply-to: <8737attzw5.fsf@concordia.ellerman.id.au> Date: Wed, 21 Jun 2017 22:14:41 -0300 MIME-Version: 1.0 Content-Type: text/plain X-TM-AS-MML: disable x-cbid: 17062201-1523-0000-0000-000002AD70EE X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17062201-1524-0000-0000-00002A459698 Message-Id: <87h8z8g6wu.fsf@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-06-21_05:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=1 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1703280000 definitions=main-1706220020 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1848 Lines: 50 Michael Ellerman writes: > Thiago Jung Bauermann writes: > >> Calling arch_update_cpu_topology from a CPU hotplug state machine callback >> hits a deadlock because the function tries to get a read lock on >> cpu_hotplug_lock while the state machine still holds a write lock on it. >> >> Since all callers of arch_update_cpu_topology except rtasd already hold >> cpu_hotplug_lock, this patch changes the function to use >> stop_machine_cpuslocked and creates a separate function for rtasd which >> still tries to obtain the lock. >> >> Michael Bringmann investigated the bug and provided a detailed analysis >> of the deadlock on this previous RFC for an alternate solution: >> >> https://patchwork.ozlabs.org/patch/771293/ > > Do we know when this broke? Or has it never worked? It's been broken since at least v4.4, I think. I don't know about earlier versions. > Should it go to stable? (can't in its current form AFAICS) It's not hard to backport both this patch and commit fe5595c07400 ("stop_machine: Provide stop_machine_cpuslocked()") from branch smp/hotplug in tip.git for stable. Since rtasd only started calling arch_update_cpu_topology since v4.11, for earlier versions this patch can be simplified to making that function call stop_machine_cpuslocked unconditionally instead of defining a separate function. >> Signed-off-by: Thiago Jung Bauermann >> --- >> >> Notes: >> This patch applies on tip/smp/hotplug, it should probably be carried there. > > stop_machine_cpuslocked() doesn't exist in mainline so I think it has to > be carried there right? Yes. I said "probably" because I don't know if you want to wait until that branch is merged so that you can carry this patch in your tree. -- Thiago Jung Bauermann IBM Linux Technology Center