Re: [PATCH] removes filp_count_lock and changes nr_files type to atomic_t

[Christoph Hellwig <hch@infradead.org>]

On Thu, Aug 25, 2005 at 11:49:35PM +0530, Dipankar Sarma wrote:
> On Thu, Aug 25, 2005 at 05:13:05PM +0200, Eric Dumazet wrote:
> > Nick Piggin a ?crit :
> > 
> > >OK, well I would prefer you do the proper atomic operations throughout
> > >where it "really matters" in file_table.c, and do your lazy synchronize
> > >with just the sysctl exported value.
> > >
> > 
> > But... I got complains about atomic_read(&counter) being 'an atomic op' 
> > (untrue), so my second patch just doesnt touch the path where nr_files was 
> > read.
> > 
> 
> Here is a patch that I had done some time ago that uses atomic_t,
> yet retains the sysctl handler. Eric, you earlier patch is incorrect
> exactly for that reason.
> 
> One other thing - the claim that it removes filp_count_lock
> from fast path is bogus. The slab constructor/destructors are
> called only when we return the free file structs to the page
> allocator. That we don't do very often and therefore we
> don't acquire the lock - atleast not for every filp open
> and close.
> 
> This is not to say we don't want a better reference counter like
> a per-cpu counter, but there is some difficult stuff there and
> the returns need to justify that. I would appreciate if you
> or anyone can demonstrate this to be a problem.
> 
> The patch below was meant for debugging some suspected problems
> with -mm.

As mentioned when we last discussed it the nr_files usage in XFS
is boguss and should go away first so we don't need the accessor
and export.  Hopefully we can get rid of the max_files usage
in af_unix aswell, can you ping davem on it?

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/