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: <a6dd6e27-e8a7-09ac-f33d-03318dd5695c@oracle.com>
Date:   Thu, 3 Aug 2023 13:20:28 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH v25 01/10] drivers/base: refactor cpu.c to use
 .is_visible()
Content-Language: en-US
From:   Eric DeVolder <eric.devolder@oracle.com>
To:     Greg KH <gregkh@linuxfoundation.org>
Cc:     linux-kernel@vger.kernel.org, david@redhat.com, osalvador@suse.de,
        corbet@lwn.net, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
        dave.hansen@linux.intel.com, x86@kernel.org, bhe@redhat.com,
        ebiederm@xmission.com, kexec@lists.infradead.org, hpa@zytor.com,
        rafael@kernel.org, vgoyal@redhat.com, dyoung@redhat.com,
        lf32.dev@gmail.com, akpm@linux-foundation.org,
        naveen.n.rao@linux.vnet.ibm.com, zohar@linux.ibm.com,
        bhelgaas@google.com, vbabka@suse.cz, tiwai@suse.de,
        seanjc@google.com, linux@weissschuh.net, vschneid@redhat.com,
        linux-mm@kvack.org, linux-doc@vger.kernel.org,
        sourabhjain@linux.ibm.com, konrad.wilk@oracle.com,
        boris.ostrovsky@oracle.com
References: <20230629192119.6613-1-eric.devolder@oracle.com>
 <20230629192119.6613-2-eric.devolder@oracle.com>
 <2023070342-human-spill-a62c@gregkh>
 <31c1393d-4285-0032-7675-737737d21f71@oracle.com>
 <6d2811f5-a5ee-a49d-012d-b519b2c6ee26@oracle.com>
In-Reply-To: <6d2811f5-a5ee-a49d-012d-b519b2c6ee26@oracle.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UkJjRW1PZytvQVNkWFNqRDBGSkY1dmN3eEd4SHl3TENBU1VIWWRmK2laRDl6?=
 =?utf-8?B?NlM5WEdIWFpyZGRUUjZyYng3Z09DY004eDA3Z1Y2Ry9jSnM2WGUweWxvRnRD?=
 =?utf-8?B?RzJCbE5NbHcyVldDODFaaWg2WjVYSzgxZjloNW5OVzRhczZjM3NLZFdpQm56?=
 =?utf-8?B?WnFsTUg3Zlg1QUk2VURwOGJ0bUFBV2R3ZW1aTDBDTVRvekxYaWIzaG5wblkz?=
 =?utf-8?B?V3JVdDFUVDAzQ2cwZjgxNktsUk90eWRVSkRRaVJJdWhOUnFtbjV5ZnpDNG1v?=
 =?utf-8?B?Y3dJOFVxUVB4RWIxUjY4MHRvaTdrQ1BDRUhYSGZHeXZZaUtjSDFRSjR6RFEy?=
 =?utf-8?B?cnB4SWhmWkFqU1krRWRkR3ZDK21vSG9Xb005OElVejQ3OUJsME9jc0JVQ01B?=
 =?utf-8?B?UTFDTmJabVVTdm8xK0VOY2hhby9uMDFLRTZGMUltWmVtM29uNHlBRVdhKy9X?=
 =?utf-8?B?MmZOU242WjdPUFJQN2J6VDZTbFFRV2hiUndybklNcm4xb2pkUHpHSXc0b0U0?=
 =?utf-8?B?Y2NnMDZ1aWdnWjM0bURVb1AyWkFEYlNISFdlNk1hZ2RTNmJLS043NEtJZHEr?=
 =?utf-8?B?aFRYd3FQaTRTWUE1ai9zbDkrNWp6OGQ1eXRIYnhVNXJ2NXVFRUJUTjlEYVpo?=
 =?utf-8?B?WXdRSEZ4YXRMVUI5YUZEaVhhQWZXK1pQWEFnVXQvN21kYWorMk1JYkY4Y3FC?=
 =?utf-8?B?WFRmTE1oRElkWStCWFVXVXFBdklFaGxTMlltYVRpYTlqaE9CWGk3MG51WmNy?=
 =?utf-8?B?STVOa0Y3TjVoaHBzeVQ2dTQ3U2pscGc0Z2tmL25ramEySWVnbTFGUUFKSmI4?=
 =?utf-8?B?RThDV1R0clp5NGM5RUNuUHNGZCtBeVBabGloaWFRY3FiT0FRaXVwQWM4aDRp?=
 =?utf-8?B?RGQ4SU1SRDJETHVUNGRQd2hSUkRHbzVxT1ZMa3pJZ2c3ZjZuYmY4SGxobDR4?=
 =?utf-8?B?U1ZqS09NakFVN09OcFJQTGpncHk4K2ppc2tsUnpFU2k4anRScTdhWHExdmll?=
 =?utf-8?B?YlB2SGhTakpCUUx4SHF6dVIwcVV2d3V1bm5JZkhIc09idWw3SWtzS1gwamxo?=
 =?utf-8?B?THhTcEJlMzhEUHpjVHdoazFWclpLWWFGL3JIVUFWenJxVWh0T1pPeElPZUFj?=
 =?utf-8?B?MXBucnpOZnU3UzRlVStqWnJUOEFRbFJ3ektRYlBDR1YvaGtHb1RHU3hVRmI1?=
 =?utf-8?B?THFWRkYvamNMQTN1ZFNFaHlFZDNlYm9iTm5MZEMyc2hNZXNmcTUvaFI4RmhQ?=
 =?utf-8?B?M2taNDdObzRYTXRTY0VBT0JxWjlZU052L2hmMUU2L0d3RTgrUkRHVGR1T0xI?=
 =?utf-8?B?S2VYUlpzNzQwcFNIRzFkNWZnWkxwK3B2Wm9IclB6TXRtNXF5Nkp1bXBUNkZI?=
 =?utf-8?B?em5uS3NnT2plYVg3TE5GUTJmWDJ4d1owWTQzR1lPRHA2NnJ0dFRrbFZjdFdF?=
 =?utf-8?B?WGlRSGVES0NPbG5PUUpiUHZyb0hvaGxML1NGTllHV0xPNWY1bUtjVGFmTFNp?=
 =?utf-8?B?YnpPbW82QXoxSVVvYXcySHp3VmhTQy95dk4vUDBuYnlPNnFUL1ByY01RV3Fn?=
 =?utf-8?B?bk9mbUdEU2NZYVRROEc4YWFKVHRmakdEenNhVGZha2dtWiszaXZSUUtDZ1M4?=
 =?utf-8?B?eEErZndzZ0FvTmo4VnNTc3B0eVIydWkvMFErQzRXVmFYeXFRTllhZWV2NTVY?=
 =?utf-8?B?b2xUTlIrNXBlY3FXWE1KMzdGQXh6QmV5MEJhNTZUd3puVFpheGlaaUovTXpu?=
 =?utf-8?B?Z09sNk9FcDJQOFY5TUhJUWFiZ0phb2ZnVHE0ZEV6czE4WEpHcENqQlgvbGFz?=
 =?utf-8?B?WXkvdlpYR0pFemZoa2pPRG9udHA5MUFCamN6RkErcVNZL1VpWUZKdUp2SklI?=
 =?utf-8?B?Vm1kTFdGaFlYUXRudGVpZjM2dnR6SHpGNjdQOVVHQU01TFU3Zis5VTR1K3M4?=
 =?utf-8?B?S2czRE01S2o0QlhFMzNJaUcwd1RxWXM4VnVKZGRRSHNlRjJZSGFQR0Y0UGE2?=
 =?utf-8?B?RkhDQm1CZVg2SXZ6VXl3cEZDNGNQemQ2RXRhR291UFdOdCtIVURlMUdPdWp5?=
 =?utf-8?B?cTIzWFNXTlVGSGJJa2VxRXdQNVlBZWUwUUMzUzE1bSttcnhGMHJmRVZ5cW95?=
 =?utf-8?B?VHhzRkFIOVFqRjZWeWFLQWthNi84dHVoNG9tMWk4bUVpeEhCOHJWWUJpUjhF?=
 =?utf-8?B?cnc9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?utf-8?B?d1Z2c2VYSHVvZlFEQVNJQmo4bmkvQVRqMks5ek8rdEtIakQ2c29scGlRNWox?=
 =?utf-8?B?U3dSSzZrZ3FtdUJ6c25VUkwwT29wN0F4TGRabkxMb2owZFJoOFppb25FYzI1?=
 =?utf-8?B?bjJZMXZaSzlwdml4KzlmUmhhVlBKODg5NlA3d0lzUGxBbm1McVRPc2w1VHEv?=
 =?utf-8?B?c0NhZDZXbWNraFpjUmRpNmlpODEyQkpFdUhHRTArSGF1YUN3anVtd0N2dmFG?=
 =?utf-8?B?cFA1NWpwcmtYZUJGcFN2clRNVGlVSkFiZi93aVEwTXBObzBMRjc2ZWlNZWtG?=
 =?utf-8?B?YTNwQjR4dUtpR3RzNDNlbU1aQ3lWV3R1QXJESzg1S1UzKzNnZ1ZGREpmSzMy?=
 =?utf-8?B?dzI4WmIvWHBBTDVMWHoxZjRyUUxSVlVJbERycnZubm51UHU3VldLSXRDZU91?=
 =?utf-8?B?L0o5KzQ5a3BWcmduam1yVG1KbW9NUzA3US9FQ3NQOG0wd1czT2RPdzJsLzY1?=
 =?utf-8?B?R242MEJaZ2dWTnVsaU9obE1KMm14N09vc3JBR1RPOVpYNDcrbjI4bXFhbEtD?=
 =?utf-8?B?WFBmSW00a1ZwRVBHTm5xSXpUekpLRTl4Zm9zUW1OVCs5N09McFdaYUk0TFdj?=
 =?utf-8?B?bVhWV25lemRyN2VKUWd3dFV4QmdaaWdZM2E3UWc3UmdKOVFmRXl5Q1Ywam9S?=
 =?utf-8?B?cHUzcDdPM1ZSV3c0VjlNOEJkOUlnejBDUCtVenBuUkpuRHN2TFozS09iQm5s?=
 =?utf-8?B?SnhQY0NBTXV6a2dvVHVraG83WFFvYnhNRE4zVENqbXJmRkJEanVWcGZqckkw?=
 =?utf-8?B?RmRxcVNjbjRRcFJuRUJMU3BGeGsrRDgyM05wd21TWUVlb2JqYU5kaXEvL2I5?=
 =?utf-8?B?MHpGQkgwdTBubmprR3lYTVBkV0pUKzVwUWp2TGtPQk1BRW4veSt0SlVMYk1Y?=
 =?utf-8?B?WExDZk1Sc1V6SzlCb3pDUG84aFVVSHBoVWUvSFpOYnJXemFDQ1RkS3hZbFlL?=
 =?utf-8?B?WlJ0L1VydkVkZG9RZXJRUzZIUFBZcEdETnNlMzlTOGZKM2FKNzY2Z01WZStr?=
 =?utf-8?B?QTBHaFp3RDN3OTlvVWRwOWRUUE91Y1FVME1UNVR5czY4WWM5TFpnSGIxSEtX?=
 =?utf-8?B?MXFuZFFqL3VIUWZzWlJCbm5VaE42QUcwZi9sYVVCKzhjYStpUXRrdFd3YlU2?=
 =?utf-8?B?dDd2cWtBNnpIYVkzakViSXlRbGFyNG1wY05zaWxpQUUyc05YQ29YSFQzdmJM?=
 =?utf-8?B?Ni9NY2hsQUk0MEw1dGwzSU1jN1FGcm9VSjNCSDlVOWpmTk5QcUpPMGY1cXZL?=
 =?utf-8?B?VXNEQ1lPSDE0N3lpTDZzclYxRlBhMDY2NUhXc3VvRVBoZUhIUG9OaW5LcFJN?=
 =?utf-8?B?NTBFNHJiNVZNanBSWm5HbFJtcUV1L2N5Vk9abWYveEQ0N1hZalR4VU1oVENH?=
 =?utf-8?B?aGxER3hwdWZhUHFwQzhjV0lyam9kL1c1aE1sNHhONEZVZDJ0U2taQ252ZlE4?=
 =?utf-8?B?TXZEYWpWT003cmRQMW1WMjlMNzNpQzlHVU9NRS9vWTFvT2ZmYWZSNW9RQ3Jk?=
 =?utf-8?B?T0pGMW1vS0N0eGNicFd6SURNdmI4SWlqUWpuSHArREdqK3d4SktXanVRTnIv?=
 =?utf-8?B?SFV3T1Exd2VZYkphK0VjVEhLYUxTZHZlc01ybG9GaGJrOWMzQ1BVZzAyaWFl?=
 =?utf-8?B?T1M0bWl1V1FUb3hKbWRYc3NUbUpLaU90RzFUN3R3SUNrK1RKWmRGRFFVSE5k?=
 =?utf-8?B?NUg4THdYVkFyQVFmRXRVamMrYnEvTndJSXgrSlhEVzRiMzRqdzM2ZGducHB2?=
 =?utf-8?B?TnFDQXI3cU02NWVNcXlHNTZpQ3ozS1NzOUZjbDEwNVoyWUNuMmFEVE9relYz?=
 =?utf-8?B?TjNJWFMwVkQvWjFXZkpWOXhtenB5ZXMwSjAwMm1FVTk0WjBhUmtrcE1ZYUxK?=
 =?utf-8?B?bjNrL2VFTkg1ZVN3dTlTaTBMcjdBRERDeHlnZVpzUkpYNDV4djBUcnBJM1JL?=
 =?utf-8?Q?0SF9lgcPUb4=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d5af5a0c-053a-4d05-a88d-08db944e53ff
X-MS-Exchange-CrossTenant-AuthSource: CO1PR10MB4531.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Aug 2023 18:20:34.3917
 (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: i6MWNO6R0OIVfELjsCoIkiqkUc6PiXa3maCK55GFYpfVEEIiNfwwEaWCRqfZj5GPgtbPMwWUKkMwODkmVC5jkX4Qs8eUmE0F3148jTYXIHQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR10MB5706
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26
 definitions=2023-08-03_19,2023-08-03_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 malwarescore=0
 adultscore=0 bulkscore=0 suspectscore=0 spamscore=0 mlxscore=0
 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2306200000 definitions=main-2308030165
X-Proofpoint-ORIG-GUID: NkniB4urIwSKTU92992GLlaklmST8eSy
X-Proofpoint-GUID: NkniB4urIwSKTU92992GLlaklmST8eSy
X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,
        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 7/21/23 11:32, Eric DeVolder wrote:
> 
> 
> On 7/3/23 11:53, Eric DeVolder wrote:
>>
>>
>> On 7/3/23 08:05, Greg KH wrote:
>>> On Thu, Jun 29, 2023 at 03:21:10PM -0400, Eric DeVolder wrote:
>>>>   - the function body of the callback functions are now wrapped with
>>>>     IS_ENABLED(); as the callback function must exist now that the
>>>>     attribute is always compiled-in (though not necessarily visible).
>>>
>>> Why do you need to do this last thing?  Is it a code savings goal?  Or
>>> something else?  The file will not be present in the system if the
>>> option is not enabled, so it should be safe to not do this unless you
>>> feel it's necessary for some reason?
>>
>> To accommodate the request, all DEVICE_ATTR() must be unconditionally present in this file. The 
>> DEVICE_ATTR() requires the .show() callback. As the callback is referenced from a data structure, 
>> the callback has to be present for link. All the callbacks for these attributes are in this file.
>>
>> I have two basic choices for gutting the function body if the config feature is not enabled. I can 
>> either use #ifdef or IS_ENABLED(). Thomas has made it clear I need to use IS_ENABLED(). I can 
>> certainly use #ifdef (which is what I did in v24).
>>
>>>
>>> Not doing this would make the diff easier to read :)
>>
>> I agree this is messy. I'm not really sure what this request/effort achieves as these attributes 
>> are not strongly related (unlike cacheinfo) and the way the file was before results in less code.
>>
>> At any rate, please indicate if you'd rather I use #ifdef.
>> Thanks for your time!
>> eric
>>
>>>
>>> thanks,
>>>
>>> greg k-h
> 
> Hi Greg,
> I was wondering if you might weigh-in so that I can proceed.
> 
> I think there are three options on the table:
> - use #ifdef to comment out these function bodies, which keeps the diff much more readable
> - use IS_ENABLED() as Thomas has requested I do, but makes the diff more difficult to read
> - remove this refactor altogether, perhaps post-poning until after this crash hotplug series merges, 
> as this refactor is largely unrelated to crash hotplug.
> 
> Thank you for your time on this topic!
> eric

Hi Greg,
If you have an opinion on how to proceed, please provide.
Thanks,
eric