Remove entropy harvesting from tmpfs atime

[mjg]

Summary

The current harvesting code is extremely smp-unfriendly and I don't see any value of having this added here. I could not be bothered to benchmark this particular change in isolation - there are several unrelated global problems which also need cleaning up. I don't feel strongly about it, but without a good reason to keep it I would strongly prefer to see it go.

[emaste]

This looks reasonable to me.
@jmg, your thoughts?

[jmg]

Are you sure this is even a problem? You have to enable FS_ATIME harvest flag before it'll even be a problem, from sys/dev/random/random_harvestq.c https://svnweb.freebsd.org/base/head/sys/dev/random/random_harvestq.c?annotate=324394#l456 :

if (!(harvest_context.hc_source_mask & (1 << origin)))
        return;

Which is before the lock. IMO, FS_ATIME should just go entirely, not just from tmp fs, but I don't see how this code could be a problem.

[mjg]

on kernels i tested harvest_context happens to be aligned at cache size. then hc_source_mask occupies the same line as beginning of the hc_entropy_ring:

struct entropy_ring {
        struct harvest_event ring[RANDOM_RING_MAX];
        volatile u_int in;
        volatile u_int out;
} hc_entropy_ring;

i.e. this induces avoidable fetches

the mask can and should be moved out and annotated with __read_mostly/frequently, but the harvesting in here should be whacked regardless

[mmacy]

Even at barely 3Mpps ether_input spends 30% of its time in random_harvest:

https://people.freebsd.org/~mmacy/2018.05.11/udprx1.svg

[mjg]

Can someone from secteam take a stance on this?

The scalability problem mentioned previously was taken care of, but the doubts about usefulness of this code remain.

Note the that:

1. source is disabled by default
2. there are calls only in tmpfs and ufs, where the former has a copy pasted call and the comment from ufs
3. given the above there is no equivalent call in zfs

If this feature is of any use (which I doubt) it should probably get placed in ag eneral way and removed from fs-specific code (alternatively zfs can be updated to also have it). The current state does not make sense.

Also note just removing the option is a little problematic in that the mask is configured precisely as a bitmask as opposed to a keyword list or anything else extendable.

[cem]

Is this still a material perf problem with extracted hc_source_mask (__read_mostly and checked inline in the header), and the bit disabled by default? Or is the goal at this point more aesthetic?

[mjg]

I noted there is no perf issue anymore. Now the code simply does not make sense to be only found in tmpfs + ufs and not in (now) flagship fs (zfs). It should be in all, in general code, or preferably removed.

[mjg]

I'm going to whack this altogether from tmpfs and ufs if there is no argument for keeping it within a week. You have been warned :)

[emaste]

Fine with me