Page MenuHomeFreeBSD

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

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 - subversion
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

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 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.

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 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[].

@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.

Thanks for the patch, even if FreeBSD did not adopt it.

I still have my environment patched for this (with my ORDERKEY_TID
change included) and I put "pbt" to use most of the times I run top.