Skip to contentSkip to conversation

RFC: poll change

PATCH In review Andrea Arcangeli 3 participants 5 messages Aug 15, 2001 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

#1 lore raw
Quoted 3 lines · David S. Miller

Yeah, the check can be removed, but anyone who cares about performance won't pass in huge arrays of -1 entries if only the low few pollfd entries are actually useful.

yes, on the same lines I'm not sure who needs the -1 entries in first place since one of the major points of poll is to avoid useless browse of non interesting entries. I'm scared people uses poll just to workaround the 1024 bit limit on the fdset structures in glibc and to generate arrays larger than 1024. I hope it's just a resource management issue, where only a few entries are sometime set to -1 and so they can avoid to duplicate the polltable for very minor difference in their utilization or for slow path cases.

Andrea

#2 lore raw

PATCH 1/1 agreed upon poll change

In reply to a message not in the archive
Quoted 5 lines · David S. Miller

Right, and our equivalent is "NR_OPEN".

Ok, I think I see what you and Tim are trying to say. I'm thinking in a select() minded way which is why I didn't understand :-)

Whew! :)

Quoted 3 lines

Yeah, the check can be removed, but anyone who cares about performance won't pass in huge arrays of -1 entries if only the low few pollfd entries are actually useful.

of course...patch attached, for anyone who cares - do we need to send it to anyone else, or are you going to channel it in, Dave?

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

Index: fs/select.c
===================================================================
RCS file: /home/cvs/linux-2.4/fs/select.c,v
retrieving revision 1.5

select.c +0 -3
diff -u -r1.5 select.c
@@ -419,9 +419,6 @@
 	if (nfds > NR_OPEN)
 		return -EINVAL;
 
-	if (nfds > current->files->max_fds)
-		nfds = current->files->max_fds;
-
 	if (timeout) {
 		/* Careful about overflow in the intermediate values */
 		if ((unsigned long) timeout < MAX_SCHEDULE_TIMEOUT / HZ)
#3 lore raw

Re: [PATCH] agreed upon poll change

From: Tim Hockin <[email protected]>
Date: Tue, 14 Aug 2001 17:34:22 -0700

of course...patch attached, for anyone who cares - do we need to send it to anyone else, or are you going to channel it in, Dave?

I'll pass it on.

Later,
David S. Miller
[email protected]

#4 lore raw
In reply to a message not in the archive
Quoted 4 lines · David S. Miller

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

On Tue, Aug 14, 2001 at 04:53:43PM -0700, Tim Hockin wrote:

Quoted 8 lines

Review

Quoted 2 lines
(quoted diff) +1 -1
-	if (nfds > NR_OPEN)
+	if (nfds > current->rlim[RLIMIT_NOFILE].rlim_cur)

Here SuS speaks about OPEN_MAX, not sure if OPEN_MAX corresponds to the current ulimit or to the absolute maximum (to me it sounds more like our NR_OPEN).

Right, and our equivalent is "NR_OPEN".

I was backporting the new version to 2.2 and I noticed that by using NR_OPEN an luser will actually be able to lock into RAM something of the order of the dozen mbytes per process. So I'm wondering that it would be saner to use the rlimit instead, after all I don't see much of a value to use NR_OPEN instead of the rlimit (even if strictly speaking SuS asks us to use NR_OPEN). Any weird program (if any) that would depend on NR_OPEN instead of the rlimit can be easily fixed with a one liner at most. So I guess I'd be more happy with the rlimit instead of NR_OPEN. Comments?

Andrea

#5 lore raw

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

I was backporting the new version to 2.2 and I noticed that by using NR_OPEN an luser will actually be able to lock into RAM something of the order of the dozen mbytes per process.

He can do this with page tables too :-)

Later,
David S. Miller
[email protected]