file locking.

[John Baldwin]

On Thursday 16 August 2007 04:18:51 pm Jeff Roberson wrote:
> On Thu, 16 Aug 2007, John Baldwin wrote:
> 
> > On Thursday 16 August 2007 03:01:18 am Jeff Roberson wrote:
> >> I have been looking at file locking for 8.0 and have come up with a way 
to
> >> completely eliminate the file lock, reduce the size of struct file by
> >> ~4 pointers (22%), remove the global list of files and associated lock,
> >> and restrict the scope of unp_gc while removing several race conditions.
> >
> 
> Thanks for the review.
> 
> > I like finit().  I would maybe change socketpair() to pass so1 and so2 to
> > finit() rather than setting f_data twice.
> 
> I'm not sure what you mean?

In socketpair() the new code does this:

	fp1->f_data = so1;
	...
	fp2->f_data = so2;
	...
	finit(fp1, ..., fp1->f_data, ...);
	finit(fp2, ..., fp2->f_data, ...);

It might be cleaner to do this:

	...
	...
	finit(fp1, ..., so1, ...);
	finit(fp2, ..., so2, ...);

> > Did you consider using refcount_* for f_count rather than using the atomic
> > operations directly?
> 
> Yes, I may change it.  I was investigating some schemes where it may not 
> have been sufficient but I think it will work fine for what I've settled 
> on.
> 
> I also think I'll wrap some atomics for manipulating f_type in macros.

That would be good.

> > It appears you never call unp_discard() and thus 'closef()' on the sockets 
in
> > unp_gc() now.  Perhaps the fdrop()'s in the end of the loop should be
> > unp_discard()'s instead?
> 
> unp_discard() happens as a side-effect of sorflush().

So, in the old code there's a really big comment about how it makes sure to 
only do closef() (via unp_discard()) once but does a sorflush() for each 
f_msgcount.  Was that comment no longer true?

> > Also, it's a bit of a shame to lose 'show files' from ddb.
> 
> Yes, I can re-implement that using the same technique as sysctl kern.file. 
> What's more troubling is the continued erosion of support for libkvm as it 
> uses filelist.  I don't think libkvm is a strong enough case to keep 
> filelist around.  I guess I will have to hack it to work similarly to 
> sysctl as well.
> 
> Do we have an official stance on libkvm?  Now that we have sysctl for 
> run-time it's only useful for crashdump debugging.  Really in most cases 
> it could be replaced with a reasonable set of gcc scripts.

s/gcc/gdb/.  At work we do mostly post-mortem analysis, so having working 
libkvm is still very important for us.  xref the way I just fixed netstat to 
work again on coredumps recently.  Breaking fstat on coredumps would probably 
be very annoying.  libkvm can always use the same algo as the sysctl if 
necessary though.

How much overhead is the filelist if it is only used in file 
creation/destruction?

-- 
John Baldwin