Re: [PATCH 3/3] gssd: base the size of the fd array on the RLIMIT_NOFILE limit.

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 3375 bytes --]

On Thu, 29 Nov 2012 11:30:51 +1100 NeilBrown <neilb@suse.de> wrote:

> On Wed, 28 Nov 2012 08:10:55 -0500 "J. Bruce Fields" <bfields@fieldses.org>
> wrote:
> 
> > On Wed, Nov 28, 2012 at 12:11:23PM +1100, Neil Brown wrote:
> > > We have previously raised the size of the 'pollarray' once (32 -> 256)
> > > and I have had another request to make it bigger.
> > > Rather than changing the hard-coded value, make it depend on
> > > RLIMIT_NOFILE.  This is an upper limit on the size of the array
> > > that can be passed to poll() anyway.
> > 
> > Sounds like a good idea.
> > 
> > Just out of curiosity: how does it fail?  I guess mounts just start
> > failing at some point--how do people find the workaround?
> 
> Error seems to be
> 
> rpcsec_gss: gss_init_sec_context: (major) Miscellaneous failure - (minor) Cannot contact any KDC for requested realm
> 
> in rpc.gssd logs.
> 
> I guess people could read the source to find the work around .... not ideal
> though.  I guess we should get gssd to generate some more helpful message.
> 
> The seem to be further problems that the customer is experiencing so I might
> wait until they are completely resolved to ensure I have complete
> understanding before I propose a further patch.

The "further problem" was that krb5 libraries use select() in a way that does
not support file descriptors higher than 1024.  This is fixed in the latest
krb5 so that is no longer an issue.

I've been thinking about your question, and about how best to deliver a fix
to customers, and I really think it should all "just work".
i.e. the array that gssd should be sized dynamically and RLIMIT_NOFILE should
be increased as needed.

I haven't tested this, but what do people think?   I don't have a problem
changing the rlim_cur limit like this, but I wonder if it is OK to
dynamically set rlim_max.

Thoughts?

NeilBrown


diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index d01ba2f..3576a6f 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -389,18 +389,36 @@ static int
 get_poll_index(int *ind)
 {
 	unsigned int i;
+	struct pollfd *new_pollarray;
+	struct rlimit rlim;
 
 	*ind = -1;
 	for (i=0; i<pollsize; i++) {
 		if (pollarray[i].events == 0) {
 			*ind = i;
-			break;
+			return 0;
 		}
 	}
-	if (*ind == -1) {
+
+	new_pollarray = realloc(pollarray, pollsize * 2 * sizeof(*pollarray));
+	if (!new_pollarray) {
 		printerr(0, "ERROR: No pollarray slots open\n");
 		return -1;
 	}
+	pollarray = new_pollarray;
+	memset(pollarray + pollsize, 0, sizeof(*pollarray) * pollsize);
+	*ind = pollsize;
+	pollsize *= 2;
+
+	/* We will need lots of file descriptors too */
+	if (getrlimit(RLIMIT_NOFILE, &rlim) == 0) {
+		if (rlim.rlim_cur < pollsize+20) {
+			rlim.rlim_cur = pollsize + 20;
+			if (rlim.rlim_max < rlim.rlim_cur)
+				rlim.rlim_max = rlim.rlim_cur;
+			setrlimit(RLIMIT_NOFILE, &rlim);
+		}
+	}
 	return 0;
 }
 
@@ -473,13 +491,9 @@ fail_keep_client:
 void
 init_client_list(void)
 {
-	struct rlimit rlim;
 	TAILQ_INIT(&clnt_list);
 	/* Eventually plan to grow/shrink poll array: */
 	pollsize = FD_ALLOC_BLOCK;
-	if (getrlimit(RLIMIT_NOFILE, &rlim) < 0 &&
-	    rlim.rlim_cur != RLIM_INFINITY)
-		pollsize = rlim.rlim_cur;
 	pollarray = calloc(pollsize, sizeof(struct pollfd));
 }
 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]