Re: [PATCH v2] fs/resctrl: Fix deadlock for errors during mount

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Tony,

On 5/7/26 10:47 AM, Luck, Tony wrote:
> Reinette,
> 
> This looks promising.
> 
> I'll split out the missing call to mon_put_kn_priv(); into its own
> patch before the deadlock fix.
> 
> Going to re-run the tests I did before forcing kernfs_get_tree()
> to fail early without setting new_sb_created, and also late.

Thank you very much.

> 
>> +/*
>> + * Temporary forward declaration for testing only. Move functions instead.
>> + */
>> +static void resctrl_unmount(void);
>> +static void mon_put_kn_priv(void);
> 
> Question: How much are forward declarations hated? And how to handle
> this?

I am not aware of forward declarations being hated and I am not aware of
documented tip rules about this. Even so, I do find it cleaner if they
can be avoided. To handle this a prep patch that just moves the code
without any functional change should work?

> Moving the functions around in the same patch really obscures the
> actual change. Is it OK to have a patch to make the functional
> change including the forward declarations. Then a separate commit
> that does the re-order (where it is obvious that functions are
> being picked up and moved without any code changes)?

My preference would be a prep patch that does the move with new
capability built on top. It seems unnecessary to me to add a forward
declaration in one patch just to remove it in a following patch.
I agree that moving code as part of functional change should be avoided.

Reinette