On Tue, May 09, 2017 at 11:49:16AM -0400, Jeff Layton wrote:
> +++ b/lib/errseq.c
> @@ -0,0 +1,199 @@
> +#include <linux/err.h>
> +#include <linux/bug.h>
> +#include <linux/atomic.h>
> +#include <linux/errseq.h>
> +
> +/*
> + * An errseq_t is a way of recording errors in one place, and allowing any
> + * number of "subscribers" to tell whether it has changed since an arbitrary
> + * time of their choosing.
You use the word "time" in several places in the documentation, but I think
it's clearer to say "sampling point" or "sample", since you're not using jiffies
or nanoseconds. For example, I'd phrase this paragraph this way:
* An errseq_t is a way of recording errors in one place, and allowing any
* number of "subscribers" to tell whether it has changed since they last
* sampled it.
> + * The general idea is for consumers to sample an errseq_t value at a
> + * particular point in time. Later, that value can be used to tell whether any
> + * new errors have occurred since that time.
* The general idea is for consumers to sample an errseq_t value. That
* value can be used to tell whether any new errors have occurred since
* the last time it was sampled.
> +/* The "ones" bit for the counter */
Maybe "The lowest bit of the counter"?
> +/**
> + * errseq_check - has an error occurred since a particular point in time?
"has an error occurred since the last time it was sampled"
> +/**
> + * errseq_check_and_advance - check an errseq_t and advance it to the current value
> + * @eseq: pointer to value being checked reported
"value being checked reported"?
> +int errseq_check_and_advance(errseq_t *eseq, errseq_t *since)
> +{
> + int err = 0;
> + errseq_t old, new;
> +
> + /*
> + * Most callers will want to use the inline wrapper to check this,
> + * so that the common case of no error is handled without needing
> + * to lock.
> + */
> + old = READ_ONCE(*eseq);
> + if (old != *since) {
> + /*
> + * Set the flag and try to swap it into place if it has
> + * changed.
> + *
> + * We don't care about the outcome of the swap here. If the
> + * swap doesn't occur, then it has either been updated by a
> + * writer who is bumping the seq count anyway, or another
> + * reader who is just setting the "seen" flag. Either outcome
> + * is OK, and we can advance "since" and return an error based
> + * on what we have.
> + */
> + new = old | ERRSEQ_SEEN;
> + if (new != old)
> + cmpxchg(eseq, old, new);
> + *since = new;
> + err = -(new & MAX_ERRNO);
> + }
I probably need to read through the patchset some more to understand this.
Naively, surely "since" should be updated to the current value of 'eseq'
if we failed the cmpxchg()?
