linux-kernel - Re: [patch 1/8] fdmap v2

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.64.0706070004470.4875@alien.or.mcafeemobile.com>
Date:	Thu, 7 Jun 2007 00:10:58 -0700 (PDT)
From:	Davide Libenzi <davidel@...ilserver.org>
To:	Eric Dumazet <dada1@...mosbay.com>
cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Ulrich Drepper <drepper@...hat.com>,
	Ingo Molnar <mingo@...e.hu>
Subject: Re: [patch 1/8] fdmap v2 - fdmap core

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