Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accelerating the search for victims #114

Closed
hakavlad opened this issue Feb 14, 2019 · 2 comments
Closed

Accelerating the search for victims #114

hakavlad opened this issue Feb 14, 2019 · 2 comments

Comments

@hakavlad
Copy link
Contributor

hakavlad commented Feb 14, 2019

I noticed that you use not optimal way to search victim:

        // proc contains lots of directories not related to processes,
        // skip them
        if (!isnumeric(d->d_name))
            continue;

I found best way to find PIDs in proc: just check /proc/*/exe existence. This way lets you to exclude zombies and kthreads. This way works about 30% faster in my experience: it reduces search time from 9 ms to 6 ms in nohang.

def get_pid_list():
    """
    Find pid list expect kthreads and zombies.
    """
    pid_list = []
    for pid in os.listdir('/proc'):
        if os.path.exists('/proc/' + pid + '/exe') is True:
            pid_list.append(pid)
    return pid_list

def get_non_decimal_pids():
    """
    Find non-decimal like 'self' and 'thread-self'.
    """
    non_decimal_list = []
    for pid in pid_list:
        if pid[0].isdecimal() is False:
            non_decimal_list.append(pid)
    return non_decimal_list

non_decimal_list = get_non_decimal_pids() # you should run it at startup

pid_list = get_pid_list()

if '1' in pid_list:
    pid_list.remove('1')

for i in non_decimal_list:
    pid_list.remove(i)

# now pid_list contains only real userspace processes
@hakavlad
Copy link
Contributor Author

hakavlad commented Feb 14, 2019

 // Killing the process may have failed because we are not running as root.

If you use the new method, processes with other uids will be invisible to the daemon. And the daemon will only kill processes with the same UID and will never fail if it is not running as root.

@rfjakob
Copy link
Owner

rfjakob commented Apr 13, 2019

processes with other uids will be invisible to the daemon

Actually, I think this is a bug. If another user uses up the memory, all my processes will be killed!

@rfjakob rfjakob closed this as completed Apr 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants