From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM11-BN8-obe.outbound.protection.outlook.com (mail-bn8nam11on2088.outbound.protection.outlook.com [40.107.236.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6EC3FA59 for ; Mon, 27 Jan 2025 19:02:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=fail smtp.client-ip=40.107.236.88 ARC-Seal:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738004524; cv=fail; b=ZppQutUkauyttPHvx/kr3M5nEBMKKRshI7HwcjR8q7FCqTeW9GlNd4tpLdYfoDfwGejq2TqJVpRL5hNuNJA0N2Vmb70L/Pq9yIj0HT4UjUbD6wdBy2HyX5b3qFqXbl7P/AZWB2zjG90vGncbqjGsxhfkIIKnm/W/ybtVHP8bUa0= ARC-Message-Signature:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738004524; c=relaxed/simple; bh=7tYQjq5FjHTiJbHCpEG2ei2CGeHdrr543+QJIs7Qb+Q=; h=Date:From:To:Cc:Subject:Message-ID:References:Content-Type: Content-Disposition:In-Reply-To:MIME-Version; b=lm95JYX/Z2htR4kXTa1ZoS9PYPJCIW/7f0pgjccZ21eE7ef1z4bK/K6YnYf4L8oJw3vrCHEPvIcbcxQYlWHwBfmlIDewVQsma1Hym8619pDjnHOC7TpH3Kmwsi7ATLffM8uwJYpPpDS+3+nD5RDZDUfio1Qs0YsxMcwEtcjKGRw= ARC-Authentication-Results:i=2; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=nvidia.com; spf=fail smtp.mailfrom=nvidia.com; dkim=pass (2048-bit key) header.d=Nvidia.com header.i=@Nvidia.com header.b=YM4u6ZbP; arc=fail smtp.client-ip=40.107.236.88 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=nvidia.com Authentication-Results: smtp.subspace.kernel.org; spf=fail smtp.mailfrom=nvidia.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=Nvidia.com header.i=@Nvidia.com header.b="YM4u6ZbP" ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=Iq+c0zRMotdhnbPG7iM22vTzJcMEIJh/Ly2Je0oThgKrFUsjVqD+JmLckJfbK/zIPHoBo9/MbkJKBMRPfFl+8ioB29WBarQAvH5x/J+dNwrP+ZzkPZ5hhv3t0OZkfZYg4ZCrVkuIJ+7Deb6hx1/mjG6VVBsj2JKNtLgBz6cHdbfw0IrzhkbkcwGqj6RnIfMDuj/2dHIJjBECoXmuJDwQHByT1I0kP+hbk1RmAwhg1W1+8d73TCktcSvqIZs9WhC0PtXWqEgLoLRF6yYVVtyD21A6jaNroHVOU3IvQJfW1nujfDKojP5GTdyxFTsVE/ccBEzUNTfz+at9fF7nGKb2EA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=tql8hdB7OuhhiSvlQvFBS9bqpcUnDpBr5A9FmU20Rhg=; b=yaX+p41KuZea0g9wlFxPLgp+6qGOjy4l1aPwmvInkyYcTaNin/DtrWlhB7iO3F8RokRoF5ss/Zww0p5st0n/sg8BVlsy7XzhnYbP3oPvwEI5YlOTOBeOKYm42RcwsCI4qhxgV5/FPwvpKIlMCLWlBJbI5r99h0CL9u5jl/HMJ3g5pmSdN+4X099zP/puMiwyqRiOvhhqEHkCzSBYgQg4O+tA1pStL7yt9tXmLEnU6pWkVjthm1ca6JKEz5Y/XJBA/9HFish9kvNpftqvTS7dXmVWUYCJbJjUf05H23fKFFjKKqkStmJzZ+zKfItGs6HahFLWCk6P0LC38TCJw7267A== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=tql8hdB7OuhhiSvlQvFBS9bqpcUnDpBr5A9FmU20Rhg=; b=YM4u6ZbPYmlbb9moDwrQcwdYCcE278Kf9qoE40fY4rh10R7NUGXNItUd4E1oVfeUFLXFY8lb1/hN/YlbcG2WfRyQNdfwmLMHTwkNUSwl7LwlMFFlKmhUtiO3gJaOjvrU3Dl4Nxj8X1uKF6hSmfZyN6rjXhqZXu7/ER5Kz7UuAz5A3YUFrzDFW4g2CqQHCg8cxIStD6gvhHSHQ33ukjE4ZzjY8Zy7N0KPW1e4YZ5R6/31vFjYON9125gntu3w0EaeXRbHTeN+lkVy0ISqcn7Vu9xOnNJIaw/vxKlNIiH9prmXwx6AjHutxZxpipTlXEnapLoXeHfDoKVRcq8e+H+qDw== Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=nvidia.com; Received: from CY5PR12MB6405.namprd12.prod.outlook.com (2603:10b6:930:3e::17) by DM4PR12MB5868.namprd12.prod.outlook.com (2603:10b6:8:67::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8377.22; Mon, 27 Jan 2025 19:01:58 +0000 Received: from CY5PR12MB6405.namprd12.prod.outlook.com ([fe80::2119:c96c:b455:53b5]) by CY5PR12MB6405.namprd12.prod.outlook.com ([fe80::2119:c96c:b455:53b5%4]) with mapi id 15.20.8377.021; Mon, 27 Jan 2025 19:01:58 +0000 Date: Mon, 27 Jan 2025 20:01:55 +0100 From: Andrea Righi To: Tejun Heo Cc: David Vernet , Changwoo Min , linux-kernel@vger.kernel.org Subject: Re: [PATCH v4] sched_ext: Fix lock imbalance in dispatch_to_local_dsq() Message-ID: References: <20250125091657.203445-1-arighi@nvidia.com> Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-ClientProxiedBy: FR0P281CA0091.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:a9::11) To CY5PR12MB6405.namprd12.prod.outlook.com (2603:10b6:930:3e::17) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CY5PR12MB6405:EE_|DM4PR12MB5868:EE_ X-MS-Office365-Filtering-Correlation-Id: 8d162ae9-8576-4304-957c-08dd3f05130b X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0;ARA:13230040|1800799024|376014|366016; X-Microsoft-Antispam-Message-Info: =?us-ascii?Q?lwmAatJiElXGKQh2Itpk7+TbyHD7NtrHR9cZ1H5f1Vj+dWyEc1juJzbYbZJn?= =?us-ascii?Q?P04ETG8s+N2GB073Ub7aSogwPC0uA7AluXVd3D1ZHKY8MUUwCV+a9sEWD27z?= =?us-ascii?Q?XBA99o1HzQRRMzLtPzG1CneGqMSJFQgmSIz+bjW30Lto9WYhKSlkHoJFwenj?= =?us-ascii?Q?vsv9i58KjjTXoxWjLJAgbjT2Aygvp7zOKLxVS+oXwFbfGUMaUG414HyZfcLQ?= =?us-ascii?Q?onNvh9tAv4u/vu0D6h1OyxIZkNaO+8T2vPYG1s0mYuMnLed15ppCk0C+hCYp?= =?us-ascii?Q?UClgY7/cSIsagoOG+KqE5xfjylSpm/LDiodqmNQhcO4Tj54gcV9kAvP193qY?= =?us-ascii?Q?sBfi/ibv6vkxV+d5SvDlSkCurxYuE+mN3Ku910j+z11ymC//9GFVF4q+dc0N?= =?us-ascii?Q?mbdb5Zf282lRn2PkI8HiRYWGc8wHJPFdSkmM1CPhppScu+0O7z69RMWZzM+4?= =?us-ascii?Q?v+GzqH+z2Hz8941/MVJAAPhgMIlKVAeS2IdiqCUeDJofVZtVrsOBEuXi+C2N?= =?us-ascii?Q?LHl8AodZGJixHw8bEMdJfUsnCsvTg92JlM8byhepYEVsHgwlKgpUa0wLJnwN?= =?us-ascii?Q?Ka4/pa00/NMAHgeetlwogz8UpWFI6GBC2J7QqcPAfi3+w99LhSmWbC1jDplN?= =?us-ascii?Q?OcJaruStHsqykwRGoq9YFlQedJgK0goFGMoFJZnUgzpi3nqNVHmb4HKfcmKW?= =?us-ascii?Q?4t9yMTobUxvfKX8v25ewsEFbilaf9SWnkuR0hfCggP854Aw1Sey00+KPq9om?= =?us-ascii?Q?PAFoAbOHpNnDx5cIEXTSMpGrU/3sS2UNVAjUF+tljGw+Uinaj59Gn6m+TnX4?= =?us-ascii?Q?ituZxCOHBlhwfhyD0GcGQdxvPAQYB57xCeenEDPXdGycd7TdqXKREU34Lpu+?= =?us-ascii?Q?sYMHBejmeQKaXmXn2a8Ae0JSiLkOEK/qrvAJ5O1WmGteCgncuaeBDCRJNYVW?= =?us-ascii?Q?Ilc38oQ8bcBZHVS4DSr4kiWEPLQj9OkksQE8/HOkZR8vsNTQ0OVOILLQdZ0r?= =?us-ascii?Q?7pXwP0XBLRlvVyL15q+Q6PmBNxkxrYMD4Vv54MnVWTaCPP21pkAIq48rTgtn?= =?us-ascii?Q?HbKnggWupogrAjws5hMldehnJNv1PJAZ2G8fefUBaFFdYi6y37K2t7b0PPmo?= =?us-ascii?Q?XnKRODn/mMrfXZvBhER8pyxoNDvDyx7E00NENswrjlVJ1xIoZGZd/VxZu9/Z?= =?us-ascii?Q?9D7VOQcW2pO8O59Jr00DDt2sNxu9B4JO2rrnmaLoIdXnYQtCu+93y5M3gvgW?= =?us-ascii?Q?Btfwyein5nGrqRiKV/LiyaMD9VNsBOoAuX0G+oYELjTYuJqYshmixjWipqM0?= =?us-ascii?Q?LUlWtXJW6gEsJDVDbR6N5My5CJUzg6sY1XpEvJ8VzmeHkQV07Pv15L7RIjAZ?= =?us-ascii?Q?omU2C5H3BqcFpV5N5YU/oimlVv3T?= X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:CY5PR12MB6405.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230040)(1800799024)(376014)(366016);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?VUAKYYD1YzOFolkwHXhqjsples2xD42+bBWIKvaCuvIocpDL+1IBAHT/6C3t?= =?us-ascii?Q?MGBHWYtXLOv/KXPFN90JgNvDpf/C0+IDrR4G9JHvMz+/n45w+nGzoGKlzEfK?= =?us-ascii?Q?defWd0ijI1ObOrW7zSY23VuaFNUuFMxeQxPwuTbZyYp334eQTejnG/LsZqhb?= =?us-ascii?Q?BjUQMl/Z1ssHw+ejug/gK3ZaH4jPfVqsymrV88YO538zQ9c/oZXw2s9KpOFR?= =?us-ascii?Q?foKiJ2ANW0+P3rxAp8Fa3nY9p/0tGMolXIcOXjCg478SXxaQjk5H0ozWwkqR?= =?us-ascii?Q?p+INm6fdykfbsuX2UOJC7giy8XHsBh8oYqOL93e93S93wK1Ei4UkPzahFh2o?= =?us-ascii?Q?4PwHXZOpNogHamMjIQIfQODrPGI7BnAkd6DWAKB1IWVCdpMe/eq9qAPYApez?= =?us-ascii?Q?fMg77axE4PLcLDwPMZPSHkj/9lzia6GVe+iTitxspsGQ7DxcsJJyqZrEdB7Y?= =?us-ascii?Q?njqPdCpS5KxKZoOchvSHU6meRIxotAZKorBEs0r7dRbHqKccOhBJ/9/ugRwx?= =?us-ascii?Q?sYJZubs71/SWrz7RS/owHwsRozb4AoiU737PPsVCLcJPyZ4o7nr9TcA54zNQ?= =?us-ascii?Q?lR+BSI9OvAnnQFk0TGGOFeacBiQw4Unms2v5eqRHOlcbCTjnAK7Z/r82RS05?= =?us-ascii?Q?vvkWElNtk5paTeqgQs6n0Oq9raoJeSohKolZQxYqEHKqODKnh9Xe8dGwWY8A?= =?us-ascii?Q?MrumrlKaAfzhwfApTRsLkJYwR8Q1BqNHjJTfpZrCXWMg1xXtaoUCVdnYd6XA?= =?us-ascii?Q?SwzM/y1dh5vwG1Br7hF3JQeBAHUsjKqa2HHN3wTr544k49XFVrfBL6HR7jXu?= =?us-ascii?Q?/OeP2HrDhh7pC9OljLz1ztWoiGREsGBeQwZ2kpuh0l0i7QNnL+3vFqKkypdY?= =?us-ascii?Q?wBfYq39riVqQ7W0UW/dWn7OTTq1ylHMRwt5Xps+DasG/YeAjAx09XxL979Gq?= =?us-ascii?Q?vVK8ix8sRfBH20JVy5ehP/TIDdsnDvggvhbVF4XWPFv+oNBLO0C3uDtZG8J/?= =?us-ascii?Q?zplA1yiL5wBhWPeJcOMOdMg/+Q4ChMij4A1LOJipBwAoUUMbP2aGIHMyT6Wl?= =?us-ascii?Q?v7a3fdabjNcIOCvhCbZUwEey212ACi9W8zyY2I8JCYZRFvKYF+D0/6ZyMlLp?= =?us-ascii?Q?oDFlWLa2k0zvoKMbUCRw1J8m5J10X9RDNWgyBSDmngur2948A6McXbVuNBxK?= =?us-ascii?Q?ACuIkh1vmiQjyQF/KwaGTFu4UaFum3zwxKPEb2mvXvRnlEUSLHRo+CY2R+3g?= =?us-ascii?Q?jyBclTI0jumL/8koOA8CO7GvoSIKWLSDyMv2stLEeF0NpwyAWXvuH8bQZ8al?= =?us-ascii?Q?YnfB2bpxu/1F72epIFxSZwWiO0FcIMOTD23A5HNlwI+0cKy1t+Y/1+fd9DRA?= =?us-ascii?Q?bNFyqJGXxlZtpzTlwp7KKQr/sPRDKzoeLcWQcXRCTHr4W6vVUpwbJ/ZUOrY1?= =?us-ascii?Q?bV6PFMKQ1++Em6GzNot9m+Ofbt1Dl+shDVMFI+w43tSV38zob2b0zmj9rbfT?= =?us-ascii?Q?5jHPuv86RlIqZH0gtVm98CzegDAf9elVGV+ME0iqDz0sErcMZOlL3/qnPSSk?= =?us-ascii?Q?f8HHE2IP4lI0eTTCfvhAgCEelFFGotZHjmSpMN9U?= X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-Network-Message-Id: 8d162ae9-8576-4304-957c-08dd3f05130b X-MS-Exchange-CrossTenant-AuthSource: CY5PR12MB6405.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Jan 2025 19:01:58.4525 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: Upnxq2cc00PDwZIvUyiQUsZ9uJx6vyzel4kE7ciN4cO6tj6glE1sgBh9HUHSkDY0bkLyh6p4Vl3cqwYUt+RDNw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB5868 On Mon, Jan 27, 2025 at 08:59:12AM -1000, Tejun Heo wrote: > Hello, > > On Sat, Jan 25, 2025 at 10:16:57AM +0100, Andrea Righi wrote: > ... > > @@ -2253,9 +2253,11 @@ static void move_local_task_to_local_dsq(struct task_struct *p, u64 enq_flags, > > * @dst_rq: rq to move the task into, locked on return > > * > > * Move @p which is currently on @src_rq to @dst_rq's local DSQ. > > + * > > + * Return the rq where @p has been moved. > > */ > > -static void move_remote_task_to_local_dsq(struct task_struct *p, u64 enq_flags, > > - struct rq *src_rq, struct rq *dst_rq) > > +static struct rq *move_remote_task_to_local_dsq(struct task_struct *p, u64 enq_flags, > > + struct rq *src_rq, struct rq *dst_rq) > > { > > lockdep_assert_rq_held(src_rq); > > > > @@ -2277,6 +2279,8 @@ static void move_remote_task_to_local_dsq(struct task_struct *p, u64 enq_flags, > > dst_rq->scx.extra_enq_flags = enq_flags; > > activate_task(dst_rq, p, 0); > > dst_rq->scx.extra_enq_flags = 0; > > + > > + return dst_rq; > > The returned dst_rq always matches the input param, right? Let's please not > do this. The return value can mislead users to assume that the returned > value may be different from the input. e.g. kobj_get() does similar identity > return for convenience and that led people to assume that kobj_get() does > zero-ref testing before inc'ing which led to a group of bugs. Just follow up > the call statement with an explicit assignment if necessary. Nothing > meaningful is achieved by merging that into the call statement. > > > +/** > > + * dispatch_to_local_dsq - Dispatch a task to a local dsq > > + * @rq: current rq which is locked > > + * @dst_dsq: destination DSQ > > + * @p: task to dispatch > > + * @enq_flags: %SCX_ENQ_* > > + * > > + * We're holding @rq lock and want to dispatch @p to @dst_dsq which is a local > > + * DSQ. This function performs all the synchronization dancing needed because > > + * local DSQs are protected with rq locks. > > + * > > + * The caller must have exclusive ownership of @p (e.g. through > > + * %SCX_OPSS_DISPATCHING). > > + */ > > +static void dispatch_to_local_dsq(struct rq *rq, struct scx_dispatch_q *dst_dsq, > > + struct task_struct *p, u64 enq_flags) > > +{ > > + struct rq *src_rq = task_rq(p); > > + struct rq *dst_rq = container_of(dst_dsq, struct rq, scx.local_dsq); > > + > > + /* > > + * We're synchronized against dequeue through DISPATCHING. As @p can't > > + * be dequeued, its task_rq and cpus_allowed are stable too. > > + * > > + * If dispatching to @rq that @p is already on, no lock dancing needed. > > + */ > > + if (rq == src_rq && rq == dst_rq) { > > + dispatch_enqueue(dst_dsq, p, enq_flags | SCX_ENQ_CLEAR_OPSS); > > + return; > > + } > > + > > + move_to_remote_dsq(rq, src_rq, dst_rq, p, enq_flags); > > I'm not sure the refactoring is adding much, but even if it does: > > - All the changes the patch makes and why should be explained in the > description. > > - The refactoring is obscuring the actual fix. If refactoring is desirable, > please put it in a separate patch. Ok, makes sense, I'll send a new patch without the refactoring. Thanks, -Andrea