Re: [PATCH 1/4] module: Add module_subinit{_noexit} and module_subeixt helper macros

Goffredo Baroncelli
2024-07-25T17:16:55Z

On 25/07/2024 17.34, Christoph Hellwig wrote:
> On Thu, Jul 25, 2024 at 05:30:58PM +0200, Arnd Bergmann wrote:
>> Now I think we could just make the module_init() macro
>> do the same thing as a built-in initcall() and put
>> an entry in a special section, to let you have multiple
>> entry points in a loadable module.
>>
>> There are still at least two problems though:
>>
>> - while link order is defined between files in a module,
>> I don't think there is any guarantee for the order between
>> two initcalls of the same level within a single file.
>
> I think the sanest answer is to only allow one per file. If you
> are in the same file anyway calling one function from the other
> is not a big burden. It really is when they are spread over files
> when it is annoying, and the three examples show that pretty
> clearly.
>
>> - For built-in code we don't have to worry about matching
>> the order of the exit calls since they don't exist there.
>> As I understand, the interesting part of this patch
>> series is about making sure the order matches between
>> init and exit, so there still needs to be a way to
>> express a pair of such calls.
>
> That's why you want a single macro to define the init and exit
> callbacks, so that the order can be matched up and so that
> error unwinding can use the relative position easily.
>

Instead of relying to the "expected" order of the compiler/linker,
why doesn't manage the chain explicitly ? Something like:

struct __subexitcall_node {
void (*exitfn)(void);
struct subexitcall_node *next;

static inline void __subexitcall_rollback(struct __subexitcall_node *p)
{
while (p) {
p->exitfn();
p = p->next;
}
}

#define __subinitcall_noexit(initfn, rollback) \
do { \
int _ret; \
_ret = initfn(); \
if (_ret < 0) { \
__subexitcall_rollback(rollback); \
return _ret; \
} \
} while (0)

#define __subinitcall(initfn, exitfn, rollback) \
do { \
static subexitcall_node node = {exitfn, rollback->head}; \
__subinitcall_noexit(initfn, rollback); \
rollback = &node;
} while (0)

#define MODULE_SUBINIT_INIT(rollback) \
struct __subexitcall_node *rollback = NULL

#define MODULE_SUBINIT_CALL(initfn, exitfn, rollback) \
__subinitcall(initfn, exitfn, rollback)

#define MODULE_SUBINIT_CALL_NOEXIT(initfn, rollback) \
__subinitcall_noexit(initfn, rollback)

#define MODULE_SUBEXIT(rollback) \
do { \
__subexitcall_rollback(rollback); \
rollback = NULL; \
} while(0)

usage:

MODULE_SUBINIT_INIT(rollback);

MODULE_SUBINIT_CALL(init_a, exit_a, rollback);
MODULE_SUBINIT_CALL(init_b, exit_b, rollback);
MODULE_SUBINIT_CALL_NOEXIT(init_c, rollback);

MODULE_SUBEXIT(rollback);

this would cost +1 pointer for each function. But this would save from situation like

r = init_a();
if (r)
init_b();
init_c();

--
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D 17B2 0EDA 9B37 8B82 E0B5