Fix script loading synchronization issue and add LuaUnpack submodule

[VictorNogueiraRio]

No description provided.

[mizvekov]

Leaving xdplua dangling.
Also would be weird being in a state where possibly some CPUs managed to get a lua_State, others did not.
Maybe it would make more sense to BUG_ON here.

[mizvekov]

Is spinlock the right concurrency primitive here?
So this is a lock per lua state, and there is a lua state per CPU, and spinlocks are only appropriate for locking accesses between different CPUs to the same data. And also, even in that context, spinlocks are highly inefficient when they are taken for a long time, such as would be the case for executing a lua script.

So if you ever happened upon a case where the same CPU got this lock multiple times, it would have hung forever, so I suspect that you might not even need a lock at all.

[VictorNogueiraRio]

Unfortunately a lock is indeed needed. If a user attempts to load a Lua script while another script is being executed in the same Lua state, we will have concurrent accesses to the a Lua State, which will cause synchronization issues. If not a spinlock, what would you suggest?

[mizvekov]

Based on your other message, I see now that what you probably need is just local_bh_disable and local_bh_enable when loading script.

[mizvekov]

So this struct xdplua is creating another unnecessary level of indirection.
so you already have this:

static DEFINE_PER_CPU(spinlock_t, lua_state_lock);

And you can easily grab this, as you already do:

xdplua->lock = per_cpu_ptr(&lua_state_lock, i);

This might be moot if you just remove the lock anyway.

Also, should be possible, and it would be much superior, if instead of having a lua_State pointer as the per cpu attribute, you just had the value directly, that would be one less unnecessary level of indirection.

[VictorNogueiraRio]

Yes, maybe it would be possible not to use this struct. I'll try this out.