On 1/28/15 00:11, Michael Tokarev wrote:
25.01.2015 14:03, Chen Gang S wrote:
start/end_exclusive() need be pairs, except the start_exclusive() in
need TO be pairs, or should be pairs or should be called in pairs.
stop_all_tasks() which is only used by force_sig(), which will be abort.
which will abort or which will call abort() or which calls abort().
So at present, start_exclusive() in stop_all_task() need not be paired.
queue_signal() may call force_sig(), or return after kill pid (or queue
signal).
or return after killing pid (or queuing signal).
If could return from queue_signal(), stop_all_task() would not
be called in time,
if queue_signal() returns
the next end_exclusive() would be issue.
would be AN issue.
OK, thanks, I shall notice about them, next time.
But actually we're interested to know answer to a slightly different
question: whenever queue_signal() returns or not (it doesn't return in
force_sig case). So whole this part becomes something like:
queue_signal() may either call force_sig() and die, or return. In
the latter case, stop_all_task() would not be called in time, so
next end_exclusive() will be an issue.
OK, it sounds good to me.
And even more, when you look at this function (arm_kernel_cmpxchg64_helper),
you'll notice it has two calls to end_exclusive() in sigsegv case, without
a call to start_exclusive(). _That_ is, I think, the key point here --
the rest of the information here is not really very relevant, because
the actual problem is this double call to end_exclusive() which should
be removed. It is not really that interesting to know that it's not
_necessary_ to call end_exclusive() in some cases which leads to abort(),
because this is not one of them anyway (since queue_signal() might return
just fine), and because while it is not necessary, it is not an error
either. With all this extra info, thie commit message becomes just too
confusing.
For me, when process paired functions, need consider a little more.
- Are there any recurse code between lock/unlock?
- After lock, do any code call unlock indirectly? Or before unlock(),
do any code call lock() indirectly?
- Between 2 unlocks (or 2 locks), does any code call lock (or unlock)
indirectly?
In our case, queue_signal() may call lock indirectly between 2 unlocks,
So for me, the patch is necessary to mention about queue_signal() in
commit comments.
So in arm_kernel_cmpxchg64_helper() for ARM, need remove end_exclusive()
after queue_signal().
need TO remove, and again the missing subject. We need to remove, or
we should remove, or, yet another variant, extra end_exclusive() call
should be removed.
OK.
The related commit: 97cc756 linux-user: Implement
new ARM 64 bit cmpxchg kernel helper.
So, how about this (the subject is fine):
start/end_exclusive() should be paired to each other. However, in
arm_kernel_cmpxchg64_helper() function, end_exclusive() is called
twice in a row. Remove the second, redundrand, call.
Commit which introduced this problem is97cc756 linux-user: Implement
new ARM 64 bit cmpxchg kernel helper.
?
Did I understand the problem correctly?
For me, I still suggest to give some descriptions for queue_signal().
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed
