Page MenuHomeFreeBSD

top: fix PID sorting after r340742; add process birth time sorting
AcceptedPublic

Authored by yuripv on Dec 25 2018, 3:42 PM.

Details

Reviewers
eadler
bcr
Group Reviewers
manpages
Summary

When -opid is specified, top does NOT do any sorting at the moment relying on the process list to be sorted internally (and it was, by birthtime rather than by PID though).

rS340742 changed that, and now we need to explicitly implement sorting by PID. I have also added the pbt sorting option for those who'd like the previous behavior of sorting by birth time instead.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

yuripv created this revision.Dec 25 2018, 3:42 PM
bcr accepted this revision.Dec 25 2018, 4:51 PM
bcr added a subscriber: bcr.

OK from manpages.

This revision is now accepted and ready to land.Dec 25 2018, 4:51 PM
mjg added a subscriber: mjg.Dec 27 2018, 9:01 PM
mjg added inline comments.
usr.bin/top/machine.c
1567

The array is no longer NULL-terminated. Looks like string_index will trip over this for unknown names.

mjg added inline comments.Dec 27 2018, 9:03 PM
usr.bin/top/machine.c
1567

urm, scratch that. only order-names is passed there. The commit message should explain why dropping the termination is fine.

yuripv marked an inline comment as done.Dec 27 2018, 9:03 PM
yuripv added inline comments.
usr.bin/top/machine.c
1567

It wasn't NULL-terminated previously as well, NULL in there was to say "don't do any sorting for pid". Indexes are above in ordernames[].

yuripv added a comment.Feb 6 2019, 4:02 AM

@eadler any comments on this as you requested pre-commit review for top?

In order to keep the thread order stable for process-birth-time,
I ended up also using:

#define ORDERKEY_TID(a, b) do { \
        int diff = (int)b->ki_tid - (int)a->ki_tid; \
        if (diff != 0) \
                return (diff > 0 ? 1 : -1); \
} while (0)

in compare_processbt:

static int
compare_processbt(const void *arg1, const void *arg2)
{
        const struct kinfo_proc *p1 = *(const struct kinfo_proc * const *)arg1;
        const struct kinfo_proc *p2 = *(const struct kinfo_proc * const *)arg2;
                
        ORDERKEY_PBT(p1, p2);
        if (!ps.thread) ORDERKEY_PID(p1, p2);
        else            ORDERKEY_TID(p1, p2);
        ORDERKEY_PCTCPU(p1, p2);
        ORDERKEY_CPTICKS(p1, p2);
        ORDERKEY_STATE(p1, p2);
        ORDERKEY_PRIO(p1, p2);
        ORDERKEY_RSSIZE(p1, p2);
        ORDERKEY_MEM(p1, p2);
                        
        return (0);
}

It turns out that ORDERKEY_PBT already groups by PID (of
course), so the ORDERKEY_PID case is technically redundant.
As are ORDERKEY_PCTCPU and beyond. (I kept the changes
closer to minimal.)

An interesting point is that the process-birth-time display
order for the very early processes is not in decreasing PID
order.