cvs commit: src/sys/compat/svr4 svr4_stream.c src/sys/dev/streams streams.c src/sys/fs/devfs devfs_vnops.c src/sys/fs/fifofs fifo_vnops.c src/sys/kern kern_descrip.c kern_event.c sys_generic.c sys_pipe.c uipc_mqueue.c uipc_syscalls.c ...

[Jeff Roberson]

On Sun, 30 Dec 2007, Robert Watson wrote:

>
> On Sun, 30 Dec 2007, Jeff Roberson wrote:
>
>>  Remove explicit locking of struct file.
>>   - Introduce a finit() which is used to initailize the fields of struct 
>> file
>>     in such a way that the ops vector is only valid after the data, type,
>>     and flags are valid.
>>   - Protect f_flag and f_count with atomic operations.
>>   - Remove the global list of all files and associated accounting.
>>   - Rewrite the unp garbage collection such that it no longer requires
>>     the global list of all files and instead uses a list of all unp 
>> sockets.
>>   - Mark sockets in the accept queue so we don't incorrectly gc them.
>
> This sounds like great work!  I do have a few questions though:

Sorry I missed this email earlier.

>
> (1) Could we teach "show files" to call out to a function in UNIX domain
>     sockets to print in-flight file descriptors (possibly under a separate
>     heading)?  When "show files" showed all files, whether attached to
>     processes or not, it was quite useful for debugging problems with the
>    UNIX domain socket GC code.  See point (2).

Yes, that would definitely be possible.  This is the one detracting point 
to the process based view of descriptors.

>
> (2) The approach you've found for GCing without a global file list is quite
>    elegant.  However, I think the definition of unreachable isn't quite
>    right yet.  I'll send you some regression tests via private e-mail that
>    now fail, leading to leaked file descriptors with the new code.
>    Unfortunately, I've not had a chance to try to track them down.

Using your test program I was able to reproduce this.  I didn't discover 
it in my own testing since it wasn't a cyclic reference issue.  The 
problem occurs when you close a descriptor while it has rights in an mbuf 
on the receive queue.  The other descriptor does not have any rights.  I'm 
not sure why the receive queue isn't being properly flushed on close.

>
> (3) The old GC algorithm was not aware of UNIX domain sockets without file
>    descriptors, since it did it based on the global file list.  The new code
>    is aware of these other sockets, such as the UNIX domain sockets in the
>    fifofs code.  I don't see any specific problems in this area, but we
>    should be aware of it and make sure those cases are handled properly.

The old code evaluated all descriptors.  The new code evaluates a smaller 
subset.  I don't think this will cause problems although I did find a case 
where I checked the wrong thing for NULL which could happen when examining 
a non unp socket.  I'll check that in soon.

I appreciate the feedback and excellent test program.

Thanks,
Jeff

>
> Robert N M Watson
> Computer Laboratory
> University of Cambridge
>