Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 3 Mar 1999 12:01:01 -0800
From:      John Plevyak <jplevyak@inktomi.com>
To:        Terry Lambert <tlambert@primenet.com>, John Plevyak <jplevyak@inktomi.com>
Cc:        hackers@FreeBSD.ORG
Subject:   Re: lockf and kernel threads
Message-ID:  <19990303120101.A21432@proxydev.inktomi.com>
In-Reply-To: <199903020312.UAA21878@usr01.primenet.com>; from Terry Lambert on Tue, Mar 02, 1999 at 03:12:35AM %2B0000
References:  <19990301091105.B21935@tsdev.inktomi.com> <199903020312.UAA21878@usr01.primenet.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Mar 02, 1999 at 03:12:35AM +0000, Terry Lambert wrote:
> > Alternatively, if one can ensure that the p_leader does not 
> > die before the peers, then one can preserve the locking code
> > and refraim from changing the proc strucure since one can
> > simply pass p->p_leader down to the locking code.  Since for
> > unthreaded programs p == p->p_leader this yields very simple
> > code for both cases.
> 
> I think you can do this by recursing on the peers in the middle
> of handling the kill before you pass it to the trampoline (then
> it's too late).

Why is it too late after that?  In the patch I did the wait in exit1()
right after the 'kill' of the peers.

        /* are we a task leader? */ 
        if(p == p->p_leader) {
                struct kill_args killArgs;
                killArgs.signum = SIGKILL;
                q = p->p_peers;
                while(q) {
                        killArgs.pid = q->p_pid;
                        /*
                         * The interface for kill is better
                         * than the internal signal
                         */
                        kill(p, &killArgs);
                        nq = q; 
                        q = q->p_peers;
                }
**              while (p->p_peers)
**                  tsleep((caddr_t)p, PWAIT, "exit1", 0);
        }

This seemed to work.

The peers signal when they take themselves off the list
farther down in exit1, after the fdfree() call which 
can result in the close and which would benefit from
having the p_leader still around.

> 
> 
> > Moreover, this change enables other such threading problems to be fixed
> > because logically 'process-type' state can be stored in
> > p->p_leader->foo and logically 'thread-type' state can be stored
> > in p->foo.
> > 
> > (this is the change I chose)
> 
> Really, the thread specific data needs to be seperated from the
> proc struct; that is, the p->p_leader->foo should become meaningless.
> 
> Basically, the litany on this is "everything that can differ between
> threads".

You are very right.  In the short term it might be best to
separate the parts of 'struct proc' which are thread specific
from those which are process specific.  The process specific
parts can then always be accessed via 'p->p_leader' which 
would amount to 't->t_proc' when at some future time these
two structures can be unwoven.

> 
> 
> > > POSIX is very explicit, that the first close on a file within a single
> > > process results in the locks being deasserted.
> > > 
> > > I think that your patches fail to address this issue, since in reality,
> > > I believe that you want to treat each thread as if it were a seperate
> > > process for the purpose of specifying POSIX close semantics.
> > 
> > The patch does in fact handle this case.  The first closing
> > thread with pass p->p_leader into the VOP_ADVLOCK and cause the lock
> > to be released. 
> > 
> > In the current system, only if the original thread which obtained
> > the lock was the first to close it would the lock be released.
> 
> I'm confused about the semantics you desire, I think.  My opinion
> would be that the POSIX close/unlock coupling is highly undesirable
> in general, and that you would want the threads to compete as if
> they were processes for the semantics.
> 
> I think this would depends on the lock scoping?  E.g.:
> 
> 	PTHREAD_PROCESS_PRIVATE
> 	PTHREAD_PROCESS_SHARED
> 
> ?

I would need to read the spec on this.  PTHREAD_PROCESS_SHARED/PRIVATE
I have only seen applied to mutexes.  While I can see the benefit
to having the same semantic options for file locks, I don't
see how to set them.  I can find no pthread_attr_file_lock_setpshared()
call.

> 
> 
> > I understand the problem for NFS locking, but for threaded programs
> > it would seem that lock shadowing would be the desired behavior
> > The program is logically one process, and the lock ranges are
> > shared state, not thread-specific state.  
> 
> I think the POSIX semantics, since they predate threads, can be
> reasonably interpreted either way.
> 
> However, the use that you described seemed to want to scope file
> descriptors to a particular thread, such that a thread exit would
> result in the close.  I don't think this is reasonable (the descriptor
> space is shared between all threads in a process).  The only way
> to reasonably achieve such scoping is pthread_cleanup_push().

Actually, the opposite.  What I would like is for file locks
to have process scope so that (for example) one thread could
open the file, a second could take out the lock, and a third
could close it, and the lock would be released.   Currently
the lock is left hanging past when the process exits.

I understand that generally signalling and threads is a bit messed up,
but I (somewhat urgently) have to solve the smaller problem of file 
locking being broken for programs which use kernel threads.  In any case, 
FreeBSD should not be leaving the lock lying around past when the 
process/thread which originated it died.  That is clearly a bug.

I would like to get some patch into the FreeBSD source since
this is going to be an issue for production sites which would likely
prefer a 'blessed' solution rather than my personal patch.

There are a number of possible solutions including the patch I proposed,
using a seperate id (rpid), adding additional bookkeeping to the
open fds vector etc.

However, I was hoping that by keeping the p_leader around, we could
use it as the 'process' and migrate to a model of seperate threads
and processes.  That is what I am proposing.

What do you think?

john

-- 
John Bradley Plevyak,    PhD,    jplevyak@inktomi.com,     PGP KeyID: 051130BD
Inktomi Corporation,  1900 S. Norfolk Street,  Suite 110,  San Mateo, CA 94403
W:(415)653-2830 F:(415)653-2801 P:(888)491-1332/5103192436.4911332@pagenet.net


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-hackers" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?19990303120101.A21432>