Need a second set of eyeballs for a possible startup race condition in vc04_services/vchiq.

I'm looking at linux-next:
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c

First it looks this is some kind of startup notification mechanism and
it is used by the custom RPI kernel on www.github.com. Client drivers call vchiq_add_connected_callback to register for a notification that the driver is good to go.

But I see a few interesting issues:

1. vchiq_add_connected_callback can be called by multiple clients at the same time.  If this happens they will all find that g_once_init isn't set, so all the clients will call mutex_init at the same time. That's sounds like a possible corruption issue.

2. On a multiprocessor machine, I didn't think it was OK to simply set
g_once_init without any kind of protection either since it may not propagate to the other cores.  You know some kind of barrier instruction.

3. A change was made in checkin
826d73b3024485677163253b59ef9bd187ff765.

This changed the function mutex_lock_interruptable which was at that time a macro to a custom function called mutex_lock_interruptable_killable to simply mutex_lock_killable. The old function does a dance with setting the process signal masks, but mutex_lock_killable doesn't.

Like I said, I'm new to trying to contribute to the linux kernel. But I'm not sure the two are completely a drop in replacement. I was wondering if the change is doing the correct thing?

It looks a fix would be easy to implement just by adding an atomic exchange instead of a simple test and set.

Thoughts?

drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c

static void connected_init(void)
{
        if (!g_once_init) {
                mutex_init(&g_connected_mutex);
                g_once_init = 1;
        }
}

/******************************************************************** **
******
*
* This function is used to defer initialization until the vchiq stack is
* initialized. If the stack is already initialized, then the callback will
* be made immediately, otherwise it will be deferred until
* vchiq_call_connected_callbacks is called.
*
*********************************************************************
**
****/

void vchiq_add_connected_callback(VCHIQ_CONNECTED_CALLBACK_T callback)
{
        connected_init();

        if (mutex_lock_killable(&g_connected_mutex) != 0) return;

        if (g_connected)
                /* We're already connected. Call the callback immediately. */

                callback();
        else {
                if (g_num_deferred_callbacks >= MAX_CALLBACKS) vchiq_log_error(vchiq_core_log_level, "There already %d callback registered -
"
                                "please increase MAX_CALLBACKS", g_num_deferred_callbacks); else { g_deferred_callback[g_num_deferred_callbacks] =
                                callback; g_num_deferred_callbacks++;
                }
        }
        mutex_unlock(&g_connected_mutex);
}

/******************************************************************** **
******
*
* This function is called by the vchiq stack once it has been connected
to
* the videocore and clients can start to use the stack.
*
*********************************************************************
**
****/

void vchiq_call_connected_callbacks(void)
{
        int i;

        connected_init();

        if (mutex_lock_killable(&g_connected_mutex) != 0) return;

        for (i = 0; i <  g_num_deferred_callbacks; i++) g_deferred_callback[i]();

        g_num_deferred_callbacks = 0;
        g_connected = 1;
        mutex_unlock(&g_connected_mutex);
}
EXPORT_SYMBOL(vchiq_add_connected_callback);