Re: updating COMPAT_LINUX for linux 2.6.x support (take 2)

[Chuck Silvers <chuq%chuq.com@localhost>]

On Thu, Jun 17, 2010 at 10:25:59AM +0000, Andrew Doran wrote:
> > > - Re: the Linux +ucas_int() hack, [...]
> > 
> > I think the hacky version is actually safe.  the copyin() will have
> > fetched the page into memory and created a pmap mapping for it.
> > in a UP kernel, nothing can happen between the copyin() and the copyout()
> > that could change the page's contents or invalidate the mapping,
> > so the copyout() can't fault and the value that copyin() read can't
> > have changed before the copyout().
> 
> Ok, maybe a brief comment above would do.

as you pointed out later, it's not safe if the page is COW at that point.
I'll change these to use the RAS stuff.


> > as for just disabling COMPAT_LINUX on these platforms,
> > I really don't like disabling other people's features
> > just because it would be convenient for me.
> 
> Perhaps my choice of words was bad.  What we've found is some ports 
> lagging behind in the feature stakes, and what works is to say "please
> implement this because some point down the road X will break without it".
> The presumption here is that it's unreasonable to ask one person to go
> implement feature Z on N different ports.  Given the short timeframe for
> this change I don't object to the CAS hack.. It is horrible though!

when I've tried to get people to do that kind of thing in the past,
I haven't had a lot of success, so these days I just assume I have to
do it all myself.  it does take more of my time, but the work gets done
a lot sooner and it's less stressful for me.


> > > - The dup code for fork1() code makes me uncomfortable.  Maybe it's
> > >   worthwhile changing our native code so that LIDs are always allocated
> > >   from the PID table or something along those lines?  Tend to think these
> > >   should be globally unique with the system and not just within a process.
> > >   Could also be of potential help with things like inter-process pthread
> > >   objects in shared memory.
> > 
> > could you be more specific about the duplicated code that concerns you?
> > do you mean the "Set the new LWP running" chunk at the end of
> > linux_clone_nptl()?  that's copied from sys__lwp_create() rather than
> > fork1(), BTW.
> 
> That type of code, right.
>  
> > there are 7 callers (8 with my new one) of lwp_create(),
> > and all of them have different code for making the new LWP runnable
> > (or not) afterward.  do you suppose it would be worthwhile to
> > collect all of that together into an lwp_launch() or somesuch,
> > with a bunch of flags to select the behaviour?
> 
> They all have different requirements.. I would prefer us to limit the
> proliferation of low-level stuff outside of kern/ but, I don't have a
> stong opinion, it's fine as it was.

ok, I'll leave it as-is for now and look at merging all that stuff later.


> > I'd rather not get into changing LID allocation behaviour of native
> > processes as part of this linux work.
> 
> Sure, it's something we can revisit.
> 
> > > - When resetting l_lid, for safety we should hold p_lock (allthough 
> > >   during early fork we'd probably get away with it due to the process
> > >   being SIDL).
> > >   If we take an approach like the above then we wouldn't need to reset
> > >   l_lid at all.
> > 
> > I added a flag to lwp_create() to have it allocate a new PID to use for
> > the LID of the new thread.  now the only places that the LID of an existing
> > LWP is changed are in execve1() (which already did that) and in the
> > linux fork and exec callbacks (at which point the process has only 1 LWP).
> > do we need to take p_lock in these places?
> 
> In fork is the process still SIDL?  In that case no.. Exec yes I think
> we'd need p_lock held there since the process is still "live" as it were.
> p_reflock may be held to lock out the debugger (can't remember) but
> otherwise the proc is visible.  Re: the existing LID reset in execve1()
> I'll have a look and possibly open a PR.  The only reason to reset the
> LID there is so it looks pretty for ps/top, there should be no assumptions
> about LID 1.

ok.  p_reflock is already held across the relevant portion of execve1(),
so that's not a problem.   I'll add the p_lock usage where needed.


-Chuck