Return-Path: Date: Tue, 15 Sep 2015 10:13:19 +0100 From: Charles Keepax To: Viresh Kumar CC: , , Rafael Wysocki , , , Mark Brown , Akinobu Mita , Alexander Duyck , "moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..." , Andrew Morton , Andy Lutomirski , Arik Nemtsov , "open list:QUALCOMM ATHEROS ATH10K WIRELESS DRIVER" , "open list:QUALCOMM ATHEROS ATH9K WIRELESS DRIVER" , Avri Altman , "open list:B43 WIRELESS DRIVER" , Borislav Petkov , Brian Silverman , Catalin Marinas , Chaya Rachel Ivgi , Davidlohr Bueso , Dmitry Monakhov , Doug Thompson , Eliad Peller , Emmanuel Grumbach , Florian Fainelli , Gustavo Padovan , Haggai Eran , Hariprasad S , Ingo Molnar , Intel Linux Wireless , "open list:AMD IOMMU (AMD-VI)" , "James E.J. Bottomley" , Jaroslav Kysela , Jiri Slaby , Joe Perches , Joerg Roedel , Johan Hedberg , Johannes Berg , Johannes Weiner , Jonathan Corbet , Joonsoo Kim , Kalle Valo , Larry Finger , Len Brown , Liam Girdwood , "open list:ACPI" , "moderated list:ARM64 PORT (AARCH64 ARCHITECTURE)" , "open list:BLUETOOTH DRIVERS" , "open list:DOCUMENTATION" , "open list:EDAC-CORE" , open list , "open list:MEMORY MANAGEMENT" , "open list:CISCO SCSI HBA DRIVER" , "open list:ULTRA-WIDEBAND (UWB) SUBSYSTEM:" , "open list:NETWORKING DRIVERS (WIRELESS)" , Luciano Coelho , "Luis R. Rodriguez" , Marcel Holtmann , Mauro Carvalho Chehab , Mel Gorman , Michael Kerrisk , Michal Hocko , Narsimhulu Musini , "open list:CXGB4 ETHERNET DRIVER (CXGB4)" , Nick Kossifidis , "open list:WOLFSON MICROELECTRONICS DRIVERS" , Peter Zijlstra , QCA ath9k Development , Richard Fitzgerald , Sasha Levin , Sebastian Andrzej Siewior , Sebastian Ott , Sesidhar Baddela , Stanislaw Gruszka , Steven Rostedt , Takashi Iwai , Tejun Heo , Thomas Gleixner , Tomas Winkler , Vlastimil Babka , Wang Long , Will Deacon Subject: Re: [PATCH V3 2/2] debugfs: don't assume sizeof(bool) to be 4 bytes Message-ID: <20150915091319.GH11200@ck-lbox> References: <9b705747a138c96c26faee5218f7b47403195b28.1442305897.git.viresh.kumar@linaro.org> <27d37898b4be6b9b9f31b90135f8206ca079a868.1442305897.git.viresh.kumar@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" In-Reply-To: <27d37898b4be6b9b9f31b90135f8206ca079a868.1442305897.git.viresh.kumar@linaro.org> List-ID: On Tue, Sep 15, 2015 at 02:04:59PM +0530, Viresh Kumar wrote: > Long back 'bool' type used to be a typecast to 'int', but that changed > in v2.6.19. And that is a typecast to _Bool now, which (mostly) takes > just a byte. Anyway, the bool type is implementation defined, and better > we don't assume its size to be 4 bytes or 1. > > The problem with current code is that it reads/writes 4 bytes for a > boolean, which will read/update 3 excess bytes following the boolean > variable (when sizeof(bool) is 1 byte). And that can lead to hard to fix > bugs. It was a nightmare cracking this one. > > The debugfs code had this bug since the first time it got introduced, > but was never got caught, strange. Maybe the bool variables (monitored > by debugfs) were followed by an 'int' or something bigger and the pad > bytes made sure, we never see this issue. > > But the OPP (Operating performance points) library have three booleans > allocated to contiguous bytes and this bug got hit quite soon (The > debugfs support for OPP is yet to be merged). It showed up as corruption > of the debugfs boolean symbols, where Y were becoming N and vice versa. > > Fix it properly by changing the last argument of debugfs_create_bool(), > to type 'bool *' instead of 'u32 *', so that it doesn't depend on sizeof > bool at all. > > That required updates to all user sites as well in a single commit. > regmap core was also using debugfs_{read|write}_file_bool(), directly > and variable types were updated for that to be bool as well. > > Acked-by: Mark Brown > Signed-off-by: Viresh Kumar > --- For the minor wm_adsp change: Acked-by: Charles Keepax Thanks, Charles