From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752809AbbGFORs (ORCPT ); Mon, 6 Jul 2015 10:17:48 -0400 Received: from mx0b-0016f401.pphosted.com ([67.231.156.173]:29798 "EHLO mx0b-0016f401.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752014AbbGFORq (ORCPT ); Mon, 6 Jul 2015 10:17:46 -0400 Date: Mon, 6 Jul 2015 22:17:09 +0800 From: Jisheng Zhang To: Thomas Gleixner CC: , , , Subject: Re: [PATCH v3 2/2] irqchip: dw-apb-ictl: add irq_set_affinity support Message-ID: <20150706221709.3c120bb9@xhacker> In-Reply-To: References: <1436156141-3674-1-git-send-email-jszhang@marvell.com> <1436156141-3674-3-git-send-email-jszhang@marvell.com> <20150706211035.6676916f@xhacker> X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.28; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2015-07-06_06:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 kscore.is_bulkscore=0 kscore.compositescore=1 compositescore=0.9 suspectscore=0 malwarescore=0 phishscore=0 bulkscore=0 kscore.is_spamscore=0 rbsscore=0.9 spamscore=0 urlsuspectscore=0.9 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1506180000 definitions=main-1507060224 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Dear Thomas, On Mon, 6 Jul 2015 15:51:28 +0200 Thomas Gleixner wrote: > On Mon, 6 Jul 2015, Jisheng Zhang wrote: > > On Mon, 6 Jul 2015 12:30:01 +0200 > > Thomas Gleixner wrote: > > > > > On Mon, 6 Jul 2015, Jisheng Zhang wrote: > > > > +static int dw_apb_ictl_set_affinity(struct irq_data *d, > > > > + const struct cpumask *mask_val, > > > > + bool force) > > > > +{ > > > > + struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); > > > > + struct dw_apb_ictl_priv *priv = gc->private; > > > > + struct irq_chip *chip = irq_get_chip(priv->parent_irq); > > > > + struct irq_data *data = irq_get_irq_data(priv->parent_irq); > > > > + > > > > + if (chip && chip->irq_set_affinity) > > > > + return chip->irq_set_affinity(data, mask_val, force); > > > > > > This is wrong as it lacks proper locking of the parent irq. That needs > > > to be solved at the core code level in a clean way. > > > > Is it acceptable to call irq_set_affinity() or irq_force_affinity() as the > > following: > > > > if (force) > > return irq_force_affinity(priv->parent_irq, mask_val); > > else > > return irq_set_affinity(priv->parent_irq, mask_val); > > Not from the driver, as you run into lock nesting hell. As I said, > this needs to be solved at the core code level and needs a proper > thought out design. Got it. Thanks for the clarification. > > Just for the record: I'm not too happy about that 'fiddle with the > parent' mechanism because it opens just a large can of worms. I wish > hardware designers would talk to OS people before they implement random > nonsense. > Fully agree with you. I'm requesting our HW people to connect timer to GIC directly in future chips. But in existing chips, it seems we have to wait for core code ready or use this lack of proper locking set_affinity patch ourself. Thanks, Jisheng