Skip to contentSkip to conversation

RFC: poll change

DISCUSSION Tim Hockin 4 participants 7 messages Aug 14, 2001 – Aug 15, 2001 1 message not in the archive

lore
Share & tools
lore

Tim Hockin

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

The standard also says that any pollfd with (fd < 0) is ignored. Holes are explicitly ALLOWED.

Dude, it ignores negative fds, check fs/select.c:do_pollfd()

Right - we're running in circles.

The standard says negative fd's are ignored. We get that right. What we are left with is an overly paranoid check against max_fds. This check should go away. You should be able to pass in up to your rlimit fds, and let negative ones (holes or tails) be ignored.

I'm attaching a patch :).

Am I still not making the problem clear?

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 +1 -4
diff -u -r1.5 select.c
@@ -416,11 +416,8 @@
 	int nchunks, nleft;
 
 	/* Do a sanity check on nfds ... */
-	if (nfds > NR_OPEN)
+	if (nfds > current->rlim[RLIMIT_NOFILE].rlim_cur)
 		return -EINVAL;
-
-	if (nfds > current->files->max_fds)
-		nfds = current->files->max_fds;
 
 	if (timeout) {
 		/* Careful about overflow in the intermediate values */
#2 lore raw

From: Tim Hockin <[email protected]>
Date: Tue, 14 Aug 2001 16:53:43 -0700

The standard says negative fd's are ignored. We get that right. What we are left with is an overly paranoid check against max_fds. This check should go away. You should be able to pass in up to your rlimit fds, and let negative ones (holes or tails) be ignored.

I am saying there is no problem.

In both cases, for a properly written application we ignore the invalid fds. The behavior is identical both before and after your change, so there is no reason to make it.

Later,
David S. Miller
[email protected]

#3 lore raw
In reply to a message not in the archive

Review

Quoted 2 lines · Tim Hockin
(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).

Andrea

#4 lore raw

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

Review

Quoted 2 lines · Tim Hockin
(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".

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 :-)

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.

Later,
David S. Miller
[email protected]

#5 lore raw
In reply to Andrea Arcangeli
Quoted 8 lines · David S. Miller

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 :-)

Since 2.2 we have the free_pgtables to release the pagetables under unused pgd slots, that was used to work pretty well last time I checked.

Andrea

#6 lore raw
Quoted 3 lines · Andrea Arcangeli

Since 2.2 we have the free_pgtables to release the pagetables under unused pgd slots, that was used to work pretty well last time I checked.

Funny you mention that: I noticed a while back that actually it doesn't work well with i386 PAE - presumably looks for an empty 1GB.

Hugh

#7 lore raw
In reply to Andrea Arcangeli
Quoted 7 lines · Hugh Dickins
Quoted 3 lines · Andrea Arcangeli

Since 2.2 we have the free_pgtables to release the pagetables under unused pgd slots, that was used to work pretty well last time I checked.

Funny you mention that: I noticed a while back that actually it doesn't work well with i386 PAE - presumably looks for an empty 1GB.

correct, we'd need the equivalent for the pmd too, this applies to the other 3-level (or more) pagetables archs too.

Andrea