Re: Problem with syscall_disestablish() - PR kern/50430

[Masao Uebayashi <uebayasi%gmail.com@localhost>]

On Thu, Nov 19, 2015 at 1:43 PM, Paul Goyette <paul%vps1.whooppee.com@localhost> wrote:
> On Thu, 19 Nov 2015, Masao Uebayashi wrote:
>
>> On Wed, Nov 18, 2015 at 11:07 AM, Paul Goyette <paul%vps1.whooppee.com@localhost>
>> wrote:
>>>
>>> Based on earlier comments, I've come up with a much-less-intrusive
>>> set of changes.  This time around, there are no bit masks and no new
>>> members in any system structures.  (I'm pretty sure we won't even
>>> need a kernel version bump for this.)
>>>
>>> Instead, I've modified sy_call() to check the current value of
>>> l_sysent before updating with the new syscall pointer.  If the
>>> current value is non-NULL then set a new flag bit LP_LOST_SYSENT in
>>> the lwp's l_pflag member.  This check is the only per-syscall cost:
>>>
>>>         if (l->l_sysent != NULL)
>>>                 l->l_pflag |= LP_LOST_SYSENT;
>>
>>
>> My understanding is that l_sysent is *always* overriden by syscalls
>> from within signal handlers, right?
>
>
> It's not a question of "being overwritten".  The real situation is
> that the _initial_ syscall returns first (with ERESTART), and l_sysent
> is reset to NULL.  Then when the signal handler calls some other
> syscall, there is nothing to lose.

I think I understand these now ... I misunderstood that signal is
delivered (signal handler is called) first then kernel stack from
tsleep() is returned back to syscall() ... I am so stupid. :(

I also realize that grasping the big picture of signal delivery is not
difficult once understood ... but still confusing as I wrote in the
other mail.  (I wish I could have understand these 10 years ago.)

You can close the PR.

>
> Then, when the handler returns, the _original_ syscall is automatically
> restarted.
>
> As I said earlier, I have not identified the exact code where all of
> this happens.  But empirical evidence (from printf()s inserted in
> sy_call() both before and after the dispatch) shows that it works as
> I've described.  There was never any non-NULL value in l_sysent at
> the point where sy_call() stores a new value.  And the printf() I
> had inserted after the syscall dispatch clearly showed a return value
> of -3 == ERESTART.
>
>
>> I don't fully understand these things yet, but ... my gut feeling is
>> that l_sysent should be saved on stack in mi_switch(), like oldspl,
>> before cpu_switchto(), and restored after that.  In resuming code
>> path, check if syscall exists, in case syscall was forcibly
>> disestablished.  If it doesn't exist, forcibly kill self lwp ... maybe
>> using sigexit()?
>
>
> I'm expecting that the old l_sysent is being saved on the stack by
> the signal code;  I just don't know where (since there doesn't seem
> to be any such reference to l_sysent!).
>
> When the syscall gets restarted, it will get auto-loaded again if it
> has been disestablished, so nothing is lost.
>
>> (I believe someone from CS department understand such things 100% and
>> correct me if wrong. ;)
>
>
>
> +------------------+--------------------------+-------------------------+
> | Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:       |
> | (Retired)        | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com    |
> | Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org  |
> +------------------+--------------------------+-------------------------+