Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <3fdc9c92-4a4f-b716-c8bc-056111feb3e0@oracle.com>
Date:   Mon, 1 May 2023 12:21:17 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.5.0
Subject: Re: [PATCH v4] sched/numa: Fix divide by zero for
 sysctl_numa_balancing_scan_size.
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     linux-kernel@vger.kernel.org, dietmar.eggemann@arm.com,
        vincent.guittot@linaro.org, Chen Yu <yu.c.chen@intel.com>,
        Ingo Molnar <mingo@kernel.org>
References: <20230406152633.3136708-1-chris.hyser@oracle.com>
 <20230429145635.GA1495785@hirez.programming.kicks-ass.net>
Content-Language: en-US
From:   chris hyser <chris.hyser@oracle.com>
In-Reply-To: <20230429145635.GA1495785@hirez.programming.kicks-ass.net>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?S2UxQVdWcjhZR20zQkRiL0pHeUFkVDROTWZtWGpoK2hSSi85QzNDa1ZmM1VK?=
 =?utf-8?B?aEtzUStpSHk4M1BWNjVrT09pRno0cmtDN0ZnT2VoRmxPeWlqa09IYS9zNWp6?=
 =?utf-8?B?NFhwSm5JUmQxeUc0NlltZVFDYTIrak9kYnZKd3R4YXRrWERPaUdVQkJsRW5m?=
 =?utf-8?B?YytOQ1E5RmlLdDZqM2FHTjRobFhzN1dXY3pvRTUrNU9Yd29MaGl0ajFYMEZa?=
 =?utf-8?B?eFl6RC9JNjZDL0V5YlJ3MmRROHNSR2JxYXkwNWFBUmFKb0RBRi8wUzAxcXBL?=
 =?utf-8?B?NzJJeHRlZStacHNZcC9HZENKZm5NdjdUaDAvWnZic3ZvREJkbG9CR1k2R2lY?=
 =?utf-8?B?QTRRdGdwdW1LZU4rOHZtMVJ5c05DZWpzQURuTnJuYWowTlpGRXhZZUM4MVdE?=
 =?utf-8?B?Y0U3U0NKdmVYanA3MTVOVzlvdkF2cnJSTm5hSEJXdXIxUnZZbUQzWEVjVjdo?=
 =?utf-8?B?djdnUnZkdE1kdGxncWxHaTdzSC9CNUZ6d0pxRXpLL0hDT0tZOHhlNk0wRVFv?=
 =?utf-8?B?WjVoSVhGZTFCQ0htalk5UGhUMDN3Nk9CV1dldEdwelorMVNlblFFNlBXL0Fq?=
 =?utf-8?B?RFoxZGx6OWdnTVFqMnZWcEZKUWM3QVM2YTdDcTI1L2lDalU2c0FvcDdRTHRn?=
 =?utf-8?B?alArMUVNbXRKcFh3cDNLRTFzTlBuVGE1RUwvK0hVMERTTzF1M0ZKY3YyU3Aw?=
 =?utf-8?B?ZnBDOHdmOGdld2ZXQ0pTSXJxMWNrbHo4SUdoaTd1S2FWVGR0REtqR2dvK1pI?=
 =?utf-8?B?Mnowd0xtQURRUTNDczFkKyt2VzhIM1lXN2NIZFRDclBEYVFvRkZkeUxTdUNr?=
 =?utf-8?B?akVpN09LRWNob29kdGpsdW9sUmZ6QTJObkk0RlJxb3psbVNwRGpxNzRib1RT?=
 =?utf-8?B?RUFIdHlyRHFiT0lkZG1LM0NPemE1aDl6QmJOdnZqWHRRNm9qWjY0QjFDOEZv?=
 =?utf-8?B?MG5iV1QzakJ3UllUMmxpNWMwSXJqQk5Td0Via2Z3UTBRTERWRGNyTnFjZ0Nm?=
 =?utf-8?B?YVZIaFBwYUdDdzhzeTZ1R3BzSm1xWXRYMmRTQ1Ntc1N4Szk2YWViYUlDc2o4?=
 =?utf-8?B?bS9iRkk3OUt4UFFMallEUllwY0NCb0orRlpxNXBJbXpsOXEya09mN0p6WEVG?=
 =?utf-8?B?Z09CSDVXRWFVZTV6OGc2ZEpFcDgzTTRocGhQMlE3TGVsR0JVbGlvOUw5djJI?=
 =?utf-8?B?alAwdUk2NTBmcFZqTTMvc1UrWEZiV3l2aFcvVXlCZWFKWkY1MndLd3p3REhY?=
 =?utf-8?B?c2didFE4a0w0NG5XdlZWRWdEdmdvY1dyNDlPOFFod3RHaVRPRktBaWxocnd0?=
 =?utf-8?B?aEdLVUFlYmpKZHNPTSt5dDJPejNCeHFVZ1Nib0M3dFUwcWwwRENxYTRZZFZY?=
 =?utf-8?B?L3RKWFh2aEVFd3J1WXpmejFacFNkR0ZuT2h4a1lDRS84WVE2cmFFT3NtdnpF?=
 =?utf-8?B?T2I5V3gxNWh5Uzk3dnNJc2tnVUh3U2gzRDRPdVVFM3ZWOW1abUM5aFVPbDNZ?=
 =?utf-8?B?ZUwrUUd4cVJ5RzUwQzdVbHBvbWovNkxKR2x0c28vNlBOazA3MmxRbTJ1N3ZX?=
 =?utf-8?B?aDQ3b0RkVlRaVEtHekVTNWtWem9TQ09xcmxXM1FqOGdhZ1dFTkVmcFNrd0pv?=
 =?utf-8?B?S1ozSmt5RWM0dkk1dFBaaERsS3FqTmgzREQ0UlE3aEpaM2hVVnVXVVorR0Zz?=
 =?utf-8?B?aG80K3hYOUFEQ3R0eFIydUVkOEk5S0RHSnd5a3p2U3RjbkdYeFFGU3ZpdUF3?=
 =?utf-8?B?MWc1TXNDQjYvejFBMzNyK2NHUHJrbC9qL0hIWHQ4UUs5Yjh1ejZXQjJDMnZP?=
 =?utf-8?B?NUtRbFB5SEo3RDhxUWpwdzcwVTg0bDJKYjByVjdsWnlzVVRBS3FXZHBTdDV2?=
 =?utf-8?B?amZ5UFU3SlZnclZjRkMxaWJraXluUk4veE1LWXN4c0VzTVZ0TDJVaklGek0y?=
 =?utf-8?B?ditsRFB5MlRoZXRGWDRFb1RFY09ZTkl5ZHVDUjRNdjQ0V1VMakEySUxKeGxm?=
 =?utf-8?B?OGpnMHljeTQ0WjRucytlSnNwcTBmMnVoMDdtUWNLbWdEK2RORk1EaHFlUWFW?=
 =?utf-8?B?aUhrQnFnMFVJYUdnUTdVY0Rna2R3RHVQVGdyV0pxUkZJZHg3NHM1VXdMUmcr?=
 =?utf-8?B?QStqY2lKa2o5Q2dBZFFMNjlvdExTTjA4b3JKS2EyUUxSMndZME5sOE9tOTk3?=
 =?utf-8?B?eHc9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: NFJYBKxlveqA+/wwzjjXuaUS7YFRxCc85cgICdIZT2MNUOGIHyGClWJh2bYZsg8ptMrd/Uaj4SBHJ7YTh8DA7mqWf4ZnhR/vKc9uQKpY/5X6Ts8DoeGTBbJoXotLsJq/53woviBakxwykbh4HyeV1saMkt0r8wGAIU8SgwNW8RfmZYtdrE8FLk7mpgAkyqn7d803hMqnzeBny2x0OiW78XPcliM0dRCkg341YT3wgeAngiuVzNAA/WKwT/x2jISBwA/ouJAoseov5ZuFPW3JLCCk/351fDrlbfT8nGXyPpwuLQK8zI3R01tEsJ7BAtQgKtmd2BuxrLjNWqmMMKV5bGk8nTTSjG3s9OJXytNovDukQUOE54bPsL202jWszUpIWZYZmE3t9OsGFBxY1F/d46GDWkHUQz6vh9gunhX8kMLq6+DXPeGskBkPG0GLW9u3S7SIJYFAJzO8cYqCtwkEY1t4WNkTo5IGJw6OELxzp4Nsa2/cRB1V2uOZveA7iV5vp2pn2etM6Ge+OPypJF/03cywdBcvznUN3O1azG2bH/nrDHY3VLRgeyxBmSGX5Thn0D2UkMplISEreZ2W5a11/+YgJzUzIbzo24FgxWV+RozPeq/mB2cmU44p8HHtGUYlxJpyuHuZGFAh4HzJ3SzQoxhBu8X+oqPH8RiEv+oAoSES8E+RVTjephjD9HmIbWKfhcvtmeTTFMQv5s+ueJbonI1QzZoBwXw+FYlbOU5PADqooSDveMGoktFCgSpeS1quG9V11EdpSYcOfMMGJY+TvZVohu/QaClJURjo+OZeHQj/DqwXp9Sm7jcg3ev3RmBKAKR8hJtzXtM6TJg6L5rp4QiX7ig0nmuRJWXLkcqXZ622GagL0qkIFJKr6Od7VOf70sdJ6yiZkyIPcJaSpiLOjw==
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 18139961-04a1-43e9-835f-08db4a601bf3
X-MS-Exchange-CrossTenant-AuthSource: CO1PR10MB4705.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 May 2023 16:21:25.1659
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: NzTUHHVATT0oBz3E5iF7hDl1r//U4mH5UKx4ODgYYsnO00QpOKYXnbi8mtUfuG6sa7XOv82pAc5Oi8fCmSghVA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR10MB6518
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22
 definitions=2023-05-01_09,2023-04-27_01,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 malwarescore=0
 mlxlogscore=854 spamscore=0 phishscore=0 adultscore=0 mlxscore=0
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2303200000 definitions=main-2305010132
X-Proofpoint-GUID: Fln-JxEoBgyXsnqI-KoZCgcU0JLQLUmH
X-Proofpoint-ORIG-GUID: Fln-JxEoBgyXsnqI-KoZCgcU0JLQLUmH
X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 4/29/23 10:56, Peter Zijlstra wrote:
> On Thu, Apr 06, 2023 at 11:26:33AM -0400, chris hyser wrote:
>> Commit 6419265899d9 ("sched/fair: Fix division by zero
>> sysctl_numa_balancing_scan_size") prevented a divide by zero by using
>> sysctl mechanisms to return EINVAL for a sysctl_numa_balancing_scan_size
>> value of zero. When moved from a sysctl to a debugfs file, this checking
>> was lost.
>>
>> This patch puts zero checking back in place.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 8a99b6833c88 ("sched: Move SCHED_DEBUG sysctl to debugfs")
>> Tested-by: Chen Yu <yu.c.chen@intel.com>
>> Signed-off-by: Chris Hyser <chris.hyser@oracle.com>
> 
> I suppose.. but is it really worth the hassle? I mean, this is debug
> stuff, just don't write 0 in then?

My understanding of the history is that this was always debug, someone 
found the divide by zero and a convenient sysctl mechanism was used to 
fix it. It did also cleanup a little compiler weirdness. I did not see 
any justifications in the discussion of the inclusion of that patch 
other than showing the nasty stack trace you get when the machine dies 
after writing a zero. It is a major inconvenience, completely 
preventable and technically a regression, but as you point out the new 
fix is a lot more code.

In terms of actually wanting to fix this, I'm a bit confused. It clearly 
was worth fixing the first time around (it has your sign-off), and the 
only thing that has changed is that that fix no longer works.

> 
> If we do find we want this (why?!) then should we not invest in a better
> debugfs_create_u32_minmax() or something so that we don't get to add 40+
> lines for everthing we want to add limits on?

I will look at a way to greatly simplify the bounds checking here as you 
suggest.


-chrish