closefrom performance improvement by using dirfd #1757

Closed
gmachin opened this Issue Sep 25, 2016 · 4 comments

Projects

None yet

2 participants

@gmachin
gmachin commented Sep 25, 2016

Issue type Feature request.

Redhat 6 does not have the closefrom system call and using the closefrom in lib/misc.c
The maximum fd for many systems is > 8000 fd. Only a small percentage are open fd and need to closed. The closing of the fd causes significant performance loss if peap/mschapv2 and ntlm_auth is used will cause threads to hang and force children to be killed. Analysis indicated the system was spending to much time in the closefrom function. The fix was to utilize dirfd to only close open fd. Below is the modified closefrom code:

ifndef HAVE_CLOSEFROM

int closefrom(int lowfd)
{
int i;
int maxfd = 256;

ifdef HAVE_DIRFD

    struct dirent *dent;
    DIR *dirp;
    char *endp;
    long fd;

endif

ifdef _SC_OPEN_MAX

    maxfd = sysconf(_SC_OPEN_MAX);
    if (maxfd < 0) {
      maxfd = 256;
    }

endif

ifdef HAVE_DIRFD

    /* Use /proc/self/fd directory if it exists. */
    if ((dirp = opendir("/proc/self/fd")) != NULL) {
        while ((dent = readdir(dirp)) != NULL) {
            fd = strtol(dent->d_name, &endp, 10);
            if (dent->d_name != endp && *endp == '\0' &&
                fd >= 0 && fd < maxfd && fd >= lowfd && fd != dirfd(dirp))
                (void) close((int) fd);
        }
        (void) closedir(dirp);
        return(fd);
    }

endif /* HAVE_DIRFD */

    if (lowfd > maxfd) return 0;

    /*
     *      FIXME: return EINTR?
     *
     *      Use F_CLOSEM?
     */
    for (i = lowfd; i < maxfd; i++) {
      close(i);
    }
    return i;

}

endif

@alandekok
Member
alandekok commented Sep 26, 2016 edited

Hmm.. that's depressing. closefrom() has been around for a LONG time.

I've pushed changes which might help. Does RH6 have F_MAXFD or F_CLOSEM?

i.e. do man fcntl, and look for those names.

@gmachin
gmachin commented Sep 26, 2016

No I don’t see F_MAXFD or F_CLOSEM in the fcntl on RH6 or RH7.

I am running an older version of freeradius 2.2.5, but saw 2.2.9 used the same function, and wanted for you to know what I found.

Even changing maxfd to 1024 had significant impact, however using dirfd and closing only open fds > 3 had the best results.

I first tried closing all fd > 3 until I got a -1 and EBADF errno. That worked as well but that assumed all open fds were sequential and there were no gaps.

I saw other versions of closefrom using dirfd and that appeared to be a better solution.

Glenn

From: Alan DeKok notifications@github.com
Reply-To: FreeRADIUS/freeradius-server reply@reply.github.com
Date: Monday, September 26, 2016 at 6:13 AM
To: FreeRADIUS/freeradius-server freeradius-server@noreply.github.com
Cc: "gmachin@sandia.gov" GMachin@sandia.gov, Author author@noreply.github.com
Subject: [EXTERNAL] Re: [FreeRADIUS/freeradius-server] closefrom performance improvement by using dirfd (#1757)

Hmm.. that's depressing. closefrom() has bene around for a LONG time.

I've pushed changes which might help. Does RH6 have F_MAXFD or F_CLOSEM?

i.e. do man fcntl, and look for those names.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHubhttps://github.com/FreeRADIUS/freeradius-server/issues/1757#issuecomment-249553238, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ANkLxwrkHogWZQzDFRYv7497ToQdSk1Rks5qt7cFgaJpZM4KGAUG.

@alandekok alandekok added a commit that referenced this issue Sep 26, 2016
@alandekok alandekok Use opendir(/proc/self/fd) when we don't have closefrom(). Fixes #1757 9f02ac8
@alandekok
Member

I've pushed a fix. Please verify, as we're trying to release 3.0.12 today or tomorrow.

@alandekok alandekok added a commit that closed this issue Sep 26, 2016
@alandekok alandekok Use opendir(/proc/self/fd) when we don't have closefrom(). Fixes #1757 75a48b7
@alandekok alandekok closed this in 75a48b7 Sep 26, 2016
@alandekok
Member

I've pushed fixes after some checks. They also improve closefrom() performance on OSX.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment