Re: [PATCH] powerpc/powernv/prd: Unregister OPAL_MSG_PRD2 notifier during module unload

Hi Vasant,

Commit 587164cd, introduced new opal message type (OPAL_MSG_PRD2) and added
opal notifier. But I missed to unregister the notifier during module unload
path. This results in below call trace if you try to unload and load
opal_prd module.

Fixes: 587164cd ("powerpc/powernv: Add new opal message type")

In reviewing this patch, I've become a bit worried the underlying patch
has another issue that we should fix.

Consider opal_prd_probe and the opal_prd_event_nb:

static struct notifier_block opal_prd_event_nb = {
	.notifier_call	= opal_prd_msg_notifier,
	.next		= NULL,
	.priority	= 0,
};

static int opal_prd_probe(struct platform_device *pdev)
{
...
	rc = opal_message_notifier_register(OPAL_MSG_PRD, &opal_prd_event_nb);
	if (rc) { ... }

	rc = opal_message_notifier_register(OPAL_MSG_PRD2, &opal_prd_event_nb);
	if (rc) { ... }

I don't think you can reuse a single notifier block for multiple
notifier_register calls. opal_message_notify_register calls
atomic_notifier_chain_register which takes a spinlock and calls
notifer_chain_register which reads:

static int notifier_chain_register(struct notifier_block **nl,
		struct notifier_block *n)
{
	while ((*nl) != NULL) {
        // ... skip some error detection
        // ... find the right spot in the list to add this
		if (n->priority > (*nl)->priority)
			break;
		nl = &((*nl)->next);
	}
	n->next = *nl; // <-- mutate the notifier block!!
	rcu_assign_pointer(*nl, n);
	return 0;
}

So we have the same notifier block getting inserted into two different
linked lists. This is unlikely to break at the moment because nothing
else registers an MSG_PRD/PRD2 notifier, but nonetheless I think you
need to use a different notifer_block struct for each list you insert
your notifier into.

Likewise this could cause other problems during removal, if there were
other entries in either notifier list.

Kind regards,
Daniel