RFC: poll change

RFC: poll change

[Tim Hockin]

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
-- 
Tim Hockin
Systems Software Engineer
Sun Microsystems, Cobalt Server Appliances
thockin@sun.com

Re: RFC: poll change

[David S. Miller]

   From: Tim Hockin <thockin@sun.com>
   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
davem@redhat.com

Re: RFC: poll change

[Herbert Xu]

David S. Miller <davem@redhat.com> wrote:
>   From: Tim Hockin <thockin@sun.com>
>   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.
-- 
Debian GNU/Linux 2.2 is out! ( http://www.debian.org/ )
Email:  Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: RFC: poll change

[David S. Miller]

   From: Herbert Xu <herbert@gondor.apana.org.au>
   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
davem@redhat.com

Re: RFC: poll change

[Andrea Arcangeli]

On Tue, Aug 14, 2001 at 03:42:33PM -0700, David S. Miller wrote:
>    From: Herbert Xu <herbert@gondor.apana.org.au>
>    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!

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

Re: RFC: poll change

[David S. Miller]

   From: Andrea Arcangeli <andrea@suse.de>
   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
davem@redhat.com

Re: RFC: poll change

[Andrea Arcangeli]

On Tue, Aug 14, 2001 at 05:06:20PM -0700, David S. Miller wrote:
>    From: Andrea Arcangeli <andrea@suse.de>
>    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.

> 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

Re: RFC: poll change

[David S. Miller]

   From: Andrea Arcangeli <andrea@suse.de>
   Date: Wed, 15 Aug 2001 02:16:48 +0200
   
   > 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
davem@redhat.com

RE: RFC: poll change

[David Schwartz]

>    From: Herbert Xu <herbert@gondor.apana.org.au>
>    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!

	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

Re: RFC: poll change - nevermind

[Tim Hockin]

Tim Hockin wrote:

> 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.


-- 
Tim Hockin
Systems Software Engineer
Sun Microsystems, Cobalt Server Appliances
thockin@sun.com

Re: RFC: poll change - nevermind

[Andrea Arcangeli]

On Tue, Aug 14, 2001 at 03:03:07PM -0700, Tim Hockin wrote:
> Tim Hockin wrote:
> 
> > 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