From: Rajesh Borundia <rajesh.borun...@qlogic.com>
Date: Mon, 16 Nov 2015 05:58:47 -0500

> +     } else {
> +             if (atomic_read(&mbx->rsp_status) != rsp_status)
> +                     qlcnic_83xx_notify_mbx_response(mbx);

Here we go with more of some "We don't understand what atomic_t's
actually do"...

atomic_read() does _nothing_

It doesn't enforce ordering of the read of the variable wrt.  other
memory accesses or anything like that.  It doesn't make the read
"atomic".

All it does is convert an "atomic_t" into an "int", and such a
conversion is necessary if the implementation makes use of some of the
atomic_t value for implementing atomicity (such as putting a spinlock
in the upper byte of the value, etc.)

Therefore, if you are using an atomic_t to order stores and reads of
an integer value.  That's not what they do, and your usage is wrong.

If you are assuming that atomic_t's store a full 32-bit signed
integer, all 32-bits of which which are unmolested by the
implementation, then your usage is wrong.  The implementation may use
any number of bits of the atomic_t value for it's own usage.

As a result, these uses in the qlogic driver are completely wrong and
give a false impression as to what the atomic_read() function does and
what atomic_t variables are to be used for.

Please correct this, and then respin your patch on top of that.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to