Re: [patch 1/8] fdmap v2 - fdmap core

[Davide Libenzi <davidel@xmailserver.org>]

On Thu, 7 Jun 2007, Eric Dumazet wrote:

> Davide Libenzi a écrit :
> > Core code for the fdmap implementation. Random allocation, exact allocation,
> > de-allocation and lookup are all O(1) operations. It also support the
> "legacy"
> > sequential (compact) file descriptor allocation, that is O(N) like the old
> > fdtable implementation.
> > Like the old "struct fdtable", fdmap is RCU friendly too.
> >
> 
> Hi Davide
> 
> I just took a 10 minutes look before running away this morning, I'll try to
> test this to get performance numbers in about 12 hours.

Ok, thx!


> > +int fdmap_newfd_seq(struct fd_map *fmap, unsigned int start,
> > +		    unsigned int limit, unsigned long flags)
> > +{
> > +	int fd;
> > +
> > +	if (unlikely(start))
> > +		start = start - fmap->base;
> > +	if (likely(start < fmap->fdnext))
> > +		start = fmap->fdnext;
> > +	fd = find_next_zero_bit(fmap->map, fmap->size, start);
> > +	if (unlikely(fd >= limit))
> > +		return -EMFILE;
> > +	if (unlikely(fd >= fmap->size))
> > +		return -ENOSPC;
> 
> > +	fmap->fdnext = fd + 1;
> 
> Here you broke POSIX I'm afraid.
> 
> You might need some test like
> 
>     if (start <= fmap->fdnext)
>         fmap->fdnext = fd + 1;

Whoops :) It's running everything fine on my machine, so I think not many 
sw uses F_DUPFD ;) Will fix tomorrow.
I also have other changes to do, a couple performance related. I also 
forgot the --diffstat option for quilt refresh, that'd show the diffstat 
inside the patch.



> Also I'm not sure the first unlikely() and likely() are worth it.
> 
> They probably match the user code you wrote yourself :)

95% or more of the code, uses get_unused_fd(), that calls with start == 0.
So the likely/unlikely are appropriate.



- Davide