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: <6d2811f5-a5ee-a49d-012d-b519b2c6ee26@oracle.com>
Date:   Fri, 21 Jul 2023 11:32:14 -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>
In-Reply-To: <31c1393d-4285-0032-7675-737737d21f71@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?aHBjTFR0T05XK1d3cG4vT0RwQUtkYlFKQWlRQnZUK1lSaE9ROWhtSk5SYVFn?=
 =?utf-8?B?RjIxamRPSjQ5ajNJaEtXdlNrd2REdENJS21WUWFMaTVQb1hnaTQvVytvOXVW?=
 =?utf-8?B?ZmN1SUZuZWZnUitsNHJCZ044RStkUjVmdy80OEVXMjNkV3A3aHZhNWcwSnU4?=
 =?utf-8?B?N1hITHkxM3JCOVA4ei9sSXN4bzdTWlhWclRmME5sbXZ2OGRNNlpNWU0zWFpD?=
 =?utf-8?B?KytqMXhvNEE0Um5heVBqSG8zZGtoVlloSlordmJ0WlNtaXR6NnBXQ0x4N3BB?=
 =?utf-8?B?ZTN2dlpObnpJT1FxN1N1OUpDa0lBSkViTXdpVFc1YjF6MjFreEFsYU5ZaE44?=
 =?utf-8?B?TmtuMytOLzZiUmZ0aVJpTFBlclBobDVDejJQdUJmU1U0Z0RpM3N1Vkp3Slp6?=
 =?utf-8?B?aGNSZEdwWUx3QWJCQjhDM01pN3FqMmxZRTdCTUFXakVUbHpYNlJyWkt1MC9K?=
 =?utf-8?B?YU96L1ZHcnVFZ3QyTmc3K3d0akVuci9BWmdKN2I4UEUxTnNGdnlZL3ZXbTE1?=
 =?utf-8?B?T3I3T0h3UllTNWQwVTNYU2tQaEpLeVprdlhIa3ZsZlhYV0lueU5RLzVoWTMx?=
 =?utf-8?B?ZHk3ZXdwL0RSaHluNUw4cy9mRE5CbXhxTHhENExpRHBOdHZQYlBFVFBjSm5m?=
 =?utf-8?B?SkZFZDJkUXRLR0JkM0NZbVB2Vit5K0lxUktSNkhETVg3MFVTaElpR3NZNEt6?=
 =?utf-8?B?VnB5bm9aTjBEL2dSdVRzSG10YTRaKzR1bzQwUm1xQkhVVFQvMzIvd0xYZ3U4?=
 =?utf-8?B?L2pwbjZDYVhVenU3L1BtUUk2ai94ZzNzM3BoYjBHVGl1ZGNtbkwrZ2xlazAy?=
 =?utf-8?B?TklWS216STI1MWFuUndOSjVrRFl6RFNVa2FmSXlYdVhQaWVBSTdwa2h1YjhO?=
 =?utf-8?B?NjlhUyt4NE9xeDBlZWtGckE4UTU0eVc0WDdVcEVhcTRDczRMbkM5d0RvdWJy?=
 =?utf-8?B?b3luU05WLzlKM21ldEpYOWlHUERhR0lTMXRpRFh0enl4L0UxOS9yVTdqdW9m?=
 =?utf-8?B?T2krWmNTaE15RGtkT3VQeW9EUEU5TzB2cU1YRzVDNlVGNEV3cWRzTXh0SDQ2?=
 =?utf-8?B?OVJtVWMxWVZnY2xONUtZU0RZTVBmWUdic1FndU54Z05oSHZVZmdidkd1SjNK?=
 =?utf-8?B?ZFRnVEFERnRHUmtQUWZqNDNqdXc5dzFHMVViY0xMNXpnVU5wNk5qMGF3Lytt?=
 =?utf-8?B?NEc1VFFxK0MyUWJWTFNGOUlLeHFRQmlwOTNFaEd6dkhGc2l3MXVGcGc0blhD?=
 =?utf-8?B?ZWIrR3NSM0M0b29nU3MyaVZHc2xtYWxuVkY1algwL2ovM2dLN1FCRTRHanVX?=
 =?utf-8?B?NXdjTS9jRmU2ekxGNExBRHZRTDc2Nmh4elFsT05IeW5nTHRQcVUxdlhjck9Z?=
 =?utf-8?B?QXFJODdFT25ubFhhazFkSkRGT2FkV3BSOENRc2pzM1Zyd0NENExtV1YwYWRr?=
 =?utf-8?B?UmZSV2hPK042ZkRleU1sWnk0T1NmcS9GSCtuZTRIeVFxcFZjTkF2WEhVMEpj?=
 =?utf-8?B?SVhIcmhvb0VCY01tOGVQakV4MlZrVU9pN0pmak8rTjd6cFZkaTAwR3JTMlcz?=
 =?utf-8?B?anRpMENrWTNBL2t0SEZ0SVBUZTJrV1dWQVlCUHl2dHVlbTEyNFdiY1VvOGUz?=
 =?utf-8?B?RTYrTnZSRVZpZWdiWkxBVjlyYWI2eTdDOVVTb0xIY0kwQU1LVkNYZEMrSzhK?=
 =?utf-8?B?MHJrU0F3QVpieGFBNE0rYXdVNS9jZlZtYVVrSVVTWUFsWGVYaG56WnpiZ2No?=
 =?utf-8?B?dnozM0h3YkpBVzRhaUlZdWZ6UG96R1JIWTQxd3VXWk1INGVzaEsxcy9WeGNr?=
 =?utf-8?B?YnVhcldCKzN0UzVUMzFucGZoT29NVFE2b2dja3lFUUc1aGVoRm5RSWx0ZjAv?=
 =?utf-8?B?UG9QRHVCOEpDL3A3dUg2OHQyeUk4Y1c3MGsrQjB1d0wrTEdvWHRTa3BtWVhr?=
 =?utf-8?B?TUoyc0hpbWtWZ0hhMmk2VXAvWTczYWJPK3o3dGwraXpGdHVEalVzYXVwYW9L?=
 =?utf-8?B?VFI1Y1VUQkJJc2lrTFJKcVZVd1N6cVRIT3piajlqdHZpUUxDVDVMK0I0OVQr?=
 =?utf-8?B?R1Q4aVdGcVlkY1FMeVJzV1lOSk1MQ1BkdXpYM2NNR0dmYUlkTVhOdTV6dm9L?=
 =?utf-8?Q?1/9K2ZqaQ+JbImu/oLP3Yj6mL?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?utf-8?B?ZHNEZGxBeEdidm1qVTMzYUpJaWJBdXpXd0tua1JIUXRPLzZNelB1ZFhteFpJ?=
 =?utf-8?B?ZnZMaXE2RVBnWnRJdDlMMWlBV1k2YjFvdmZjRHNITkxvQWxxTUhjeFBQamlE?=
 =?utf-8?B?bERVWXRHbWpWNmROaEhSc0ZoQVpLQjZoR0hreHA4RnZPVWRLNkR6L0tMSGxs?=
 =?utf-8?B?blBQRHNSbFZnZlFCMnF2NWxxb1BGZjIvUmpQcnozR3JKMkg3Ly9DYW9zcVNm?=
 =?utf-8?B?MXR2dmVOMXl3bTdqV3UySG0zUUc0VGtHM3E5TkJsOXl5eGM1VTd0SWNMeDA4?=
 =?utf-8?B?c2o5UnAxeENmYk9wRHBveVpyRDRVVC95M1dDRU1xU0kva2dMN05LVzl1VGRF?=
 =?utf-8?B?SUQ5bXFpYlJEdktlNWt5WVJDNkQ0Q3JwaWZteWhrb2k5NGZaRnJrc3JWeFB0?=
 =?utf-8?B?QW9YOFNKd2dCSnFwVm03VlVtU0tjVjBJWlRTWjdaVWlqNFkzb0tqa2ZyRDV6?=
 =?utf-8?B?cW9YbW14Q1hPVVFmTXFxU0E2cGZOZVd0b0gyd29tNlpKcWk3ZUxRRTBPR2JU?=
 =?utf-8?B?dmZDK2ZYWGYrM1RIMkdBaUloMGljMHNrMktyTGhpcFN4KzVmRTRyaFBZSlFj?=
 =?utf-8?B?SVFzQkdDenFYbzRjSWF6d1BEOHppUEZySWVkbTlyZ2ZlUitQN1lONjl1dERU?=
 =?utf-8?B?dDlIR1NoUFhnTTltODhDLytGcXNZemRpdEhSamZDcnNmQmE0ZEsyZytGSHZ2?=
 =?utf-8?B?T2t5TjFHQnJYbWdWakd0MUdxRTUrRll5Q016NTR4UGNUNkF2NWtiWUFsdW5v?=
 =?utf-8?B?SnVkOHdMYkpQc3gwRTVVbWFibDR2M2w4WWZrWEpsRktmWWVnUXpEKzZOVGw1?=
 =?utf-8?B?bm9MeVRSb1pPeTJlUW9BeWJOaWMrQ0RRVHUvRk1Jenhxb0VQTGVsT0kzTnN3?=
 =?utf-8?B?QzhBTzFEbFNsUUJYdXV4UXFYL05nYjZmN3RLUVMydmZiaHRDTGhQaU5FTFRO?=
 =?utf-8?B?YWdKYjMzN2ZKdmtNK0NTRXNaTnh6a293Ym5JRkNHU25pREUyWjBWWEpXcFV3?=
 =?utf-8?B?Q2dPNUxpK0IxOUVIM0VybzVwNkN6RVU1N29GT3Bkd1lma2J0UWVab2l3aTRO?=
 =?utf-8?B?UlJPUGJac01xMHdpd2JGS0d0QTNHV1VEY0Y3ZGFKUGM5MC82TjM0ZURYVHMz?=
 =?utf-8?B?NDV2VEpaNVhBZ1BLSUJ4QTArMEkxdkJTSGdjbHVXWGozUTJGUlFvY05QR1Yw?=
 =?utf-8?B?VFpVSEJTUG9UTnA4MjFiRXFSQ25mM1Rhb0ZXZk9uNGVialR2dk4vNXJPS2R5?=
 =?utf-8?B?WkV5eUlkWkVZUGI5VGtJNDBFbllnVjBncjFiK3ZQRHRPcTVqQitIVmxVYW51?=
 =?utf-8?B?ZU1kSlRjL3Q1QTM5dFR3RkRyWFpqcVRlbFU1MGQyL1RLRm5QWURHdXNUYXBy?=
 =?utf-8?B?aWhuVDRBQU1IZnNFc1VqWDJYUkQwMkhhNVpzTXdmc0JOL1pPcWo5dlhrczYz?=
 =?utf-8?B?S3JSL3p6ajJhTlZoMW84VWNlNlJiNDJGUmNkR0JWRHJJQ2lMUmtxT3B2b09E?=
 =?utf-8?B?cnQ5SW5oS3c4ejQyUU9nQVNMSUNvREE5YU5GUVVDeTVvd2ZJR2h5YmZMS1Bw?=
 =?utf-8?B?eUowVmp0TnI4WTc2bkFoSzF4RUFqeEI5SFR4VTNzU3dTVGZKZ3luR3V6eVZ4?=
 =?utf-8?B?ZlFOdXVsK2doZ0QyelhmNFpmZTUxSGlVTG1YK1VRcWRMbzBQZE9TcnZjZnpV?=
 =?utf-8?B?cmhSYS8rdDNxT3hrTlFjWGlGU0xpKzVreTFrQ2UxcjRHL0h3OXprYVFtV1Fr?=
 =?utf-8?B?VG9KQnZLbm9uc3BhdDM4bCtIc3FrN3p5UktsVW5ISEF5WlJBbUFaRUFqaWZN?=
 =?utf-8?B?VFgxci9XV0trV0Z5cW5ud3BQT1lFM3IwWFJLUXN2c2FIZ054TEwvQm1nakdk?=
 =?utf-8?B?UCsyckJmbkgrN1AzR1Axb2dvakJFbUpNQ1RLYlQ2b0dtYUdnbmhsRHFoMFFN?=
 =?utf-8?Q?s9hIgSyooKY=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c4d612bd-de1b-4301-0c4f-08db8a080db2
X-MS-Exchange-CrossTenant-AuthSource: CO1PR10MB4531.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Jul 2023 16:32:19.9204
 (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: Vrkqtzd8gw/+TZwoecQdOLAzxpVMT6DAdxS0xYtZH3s0fWEyU/lhlwQBbhLhKESSOx1ODAD6hhZI4W7C3XO3Pt9LyssjjODQHRC4qV7gsD8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR10MB5372
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-07-21_10,2023-07-20_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 malwarescore=0
 mlxscore=0 adultscore=0 spamscore=0 phishscore=0 bulkscore=0
 mlxlogscore=999 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2306200000 definitions=main-2307210144
X-Proofpoint-ORIG-GUID: _navfhUWneorfAxf2PDAnqus4TK6YeeJ
X-Proofpoint-GUID: _navfhUWneorfAxf2PDAnqus4TK6YeeJ
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/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