-
Notifications
You must be signed in to change notification settings - Fork 45
allow Lua to send signals to kernel threads #277
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
The pid needs to be conditional and have to improved
added new functions, (needs few improvements as well) need to add flush_signals functions as well
add block() -- blocks given signal minor updates added enum for signals...
--empty
This comment was marked as resolved.
This comment was marked as resolved.
Added spin locks
Minor changes are needed...
|
|
||
| int signum = luaL_checkinteger(L, 2); | ||
|
|
||
| spin_lock_irq(&task->sighand->siglock); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this lock really necessary? can you point to from where you get this pattern?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This lock prevents race condition between concurrent signal senders (we can say signal masks) {safety :)}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my impression is that it should be done internally by signal API, not by its clients.. can you point to some API client doing this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
flush_signals
The above function is a API client used by us to flush and pending signals and takes lock manually before manipulating them
Sorry for taking too much of time to respond,
I was searching for other examples...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's the internal API implementation.. I meant a client from this API.. I think you are mixing the internal implementation with the API usage..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm.. I see the problem, you want to allow/block a signal on a specific task.. I think we shouldn't go to this path.. the common pattern as I see is to allow/block signal in the current task.. so such functions would probably belong to lualinux functions and not to luathread methods
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that makes sense now.
You are right, I was trying to manipulate signals on a specific task,
ok ill move these into Lualinux . instead and stick to using the standard allow_signal() API for the current task.
Ill make a new updated PR for that, Shall I close this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you still need send() and pending(), right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you're right. I’ll check once send() and pending() in threads to make sure everything is good.
For now, I’ll go ahead
and move allow(), isallowed() and block() to lualinux
I’ll update everything properly and move on from there. Sounds good?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup.. thanks!
| return luaL_error(L, "thread task is NULL"); | ||
|
|
||
| int signum = luaL_checkinteger(L, 2); | ||
| if (sigismember(&task->blocked, signum)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need this? can you provide some documentation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we dont check before sending signal, the blocked signal wont be handled properly, it will be queued. This is what I remember. I will add the source as well (have to look where I read this )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wasn't it expected? why shouldn't it be queued?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my bad, we don't need to check this here. Thanks for pointing this out. I mixed two different concepts here 😅,
I just checked the official docs. This is required in synchronous signals, which we are not doing. I'll update it soon
17ac113 to
925bfd9
Compare
The title is what it was supposed to do.
The pid needs to be conditional and have to improved
the send_sig is better than what i used