Skip to contentSkip to conversation

RFC: poll change

DISCUSSION Tim Hockin 5 participants 11 messages Aug 14, 2001 – Aug 15, 2001

lore
Share & tools
lore

Tim Hockin

author
#0 lore raw

hey all, I have a request from someone to investigate this.

poll() currently does not allow you to pass more fd's than you have open. At first this seems reasonable. However, I have seen now at least one app that takes advantage of a behavior found on some poll() implementations (I don't have the standard available..).

On these systems, you may pass in as many fds as you want, and any structure with the fd member set to < 0 is ignored. This allows apps to save on allocating and re-allocating, or sorting the fd arrays.

What would the feeling be if I implemented this for Linux?

Tim

Signature · 4 lines
Tim Hockin
Systems Software Engineer
Sun Microsystems, Cobalt Server Appliances
[email protected]
#1 lore raw

From: Tim Hockin <[email protected]>
Date: Tue, 14 Aug 2001 14:08:40 -0700

poll() currently does not allow you to pass more fd's than you have open.

Tim, please check the latest sources, this has been fixed in 2.4.x for several months.

Later,
David S. Miller
[email protected]

#2 lore raw
Quoted 7 lines · David S. Miller

From: Tim Hockin <[email protected]>
Date: Tue, 14 Aug 2001 14:08:40 -0700

poll() currently does not allow you to pass more fd's than you have open.

Tim, please check the latest sources, this has been fixed in 2.4.x for several months.

Hmm, it still seems to be wrong:

        /* Do a sanity check on nfds ... */
        if (nfds > NR_OPEN)
                return -EINVAL;

        if (nfds > current->files->max_fds)
                nfds = current->files->max_fds;

The second if statement should be removed. And it might be better to use current->rlim[RLIMIT_NOFILE].rlim_cur instead of NR_OPEN.

Signature · 4 lines
Debian GNU/Linux 2.2 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#3 lore raw

From: Herbert Xu <[email protected]>
Date: Wed, 15 Aug 2001 08:38:02 +1000

Hmm, it still seems to be wrong:
...

           if (nfds > current->files->max_fds)
                   nfds = current->files->max_fds;

The second if statement should be removed. And it might be better to use current->rlim[RLIMIT_NOFILE].rlim_cur instead of NR_OPEN.

It has to be limited to "max_fds", that is how many files we may legally address in current->files!

Later,
David S. Miller
[email protected]

#4 lore raw
Quoted 10 lines

From: Herbert Xu <[email protected]>
Date: Wed, 15 Aug 2001 08:38:02 +1000

Hmm, it still seems to be wrong:
...

           if (nfds > current->files->max_fds)
                   nfds = current->files->max_fds;
Quoted 6 lines

The second if statement should be removed. And it might be
better to use
current->rlim[RLIMIT_NOFILE].rlim_cur instead of NR_OPEN.

It has to be limited to "max_fds", that is how many files we may legally address in current->files!

Though stupid, I believe it is perfectly legal to, say, have one socket
open and poll for it with two separate entries, one checking readability and one checking writability.

DS

#5 lore raw
In reply to Herbert Xu
Quoted 8 lines · David S. Miller

From: Andrea Arcangeli <[email protected]>
Date: Wed, 15 Aug 2001 02:03:03 +0200

but OTOH the feedback I had was just happy with the 2.4 fix (so I didn't even checked if the 2.4 fix was fully compliant or not..).

This is why I'm saying there is no practical reason to make this change.

Probably there isn't pratical reason agreed.

Quoted 2 lines

The current code can actually improve performance, avoiding needlessly scanning fd==-1 entries.

you assume the entries above max_fds are set to -1, while strictly speaking they don't need to.

Andrea

#6 lore raw

From: Andrea Arcangeli <[email protected]>
Date: Wed, 15 Aug 2001 02:16:48 +0200

Quoted 2 lines

The current code can actually improve performance, avoiding needlessly scanning fd==-1 entries.

you assume the entries above max_fds are set to -1, while strictly speaking they don't need to.

Right, this was my core misunderstanding, see my most recent mail.

Later,
David S. Miller
[email protected]

#7 lore raw
In reply to David S. Miller
Quoted 6 lines · David S. Miller

From: Herbert Xu <[email protected]>
Date: Wed, 15 Aug 2001 08:38:02 +1000

Hmm, it still seems to be wrong:
...

Quoted 8 lines
           if (nfds > current->files->max_fds)
                   nfds = current->files->max_fds;

The second if statement should be removed. And it might be better to use current->rlim[RLIMIT_NOFILE].rlim_cur instead of NR_OPEN.

It has to be limited to "max_fds", that is how many files we may legally address in current->files!

current->files only matters for the contents of the pollfd struct, not for the number of pollfd structures passed on, removing such two lines shouldn't destabilize anything, I think it's just a matter of API, reading SuS it seems not to imply we have to truncate it to the max_fds but OTOH the feedback I had was just happy with the 2.4 fix (so I didn't even checked if the 2.4 fix was fully compliant or not..).

Andrea

#8 lore raw

From: Andrea Arcangeli <[email protected]>
Date: Wed, 15 Aug 2001 02:03:03 +0200

but OTOH the feedback I had was just happy with the 2.4 fix (so I didn't even checked if the 2.4 fix was fully compliant or not..).

This is why I'm saying there is no practical reason to make this change.

The current code can actually improve performance, avoiding needlessly scanning fd==-1 entries.

Later,
David S. Miller
[email protected]

author
#9 lore raw

Re: RFC: poll change - nevermind

In reply to Tim Hockin
Quoted 1 line · Tim Hockin

poll() currently does not allow you to pass more fd's than you have

I'll shut up - I was looking at 2.2.x source.

Signature · 4 lines
Tim Hockin
Systems Software Engineer
Sun Microsystems, Cobalt Server Appliances
[email protected]
#10 lore raw

Re: RFC: poll change - nevermind

In reply to Tim Hockin
Quoted 5 lines · Tim Hockin
Quoted 1 line · Tim Hockin

poll() currently does not allow you to pass more fd's than you have

I'll shut up - I was looking at 2.2.x source.

btw, it is also fixed in my 2.2 tree since 2.2.20pre8aa1:

	ftp://ftp.kernel.org/pub/linux/kernel/people/andrea/kernels/v2.2/2.2.20pre8aa1/00_poll-max_fds-1

Andrea