On Tue, Jan 07, 2025 at 04:02:12PM +0200, Leon Romanovsky wrote:
> +static void handle_send_state(struct ib_mad_send_wr_private *mad_send_wr,
> + struct ib_mad_agent_private *mad_agent_priv)
> +{
> + if (!mad_send_wr->state) {
What is this doing? state is an enum, what is !state supposed to be? 0
is not a valid value in the enum.
> @@ -1118,15 +1209,12 @@ int ib_post_send_mad(struct ib_mad_send_buf *send_buf,
> mad_send_wr->max_retries = send_buf->retries;
> mad_send_wr->retries_left = send_buf->retries;
> send_buf->retries = 0;
> - /* Reference for work request to QP + response */
> - mad_send_wr->refcount = 1 + (mad_send_wr->timeout > 0);
> - mad_send_wr->status = IB_WC_SUCCESS;
> + mad_send_wr->state = 0;
Same, enums should not be assigned to constants. If you want another
state you need another IB_MAD_STATE value and use it here and above.
> +#define EXPECT_MAD_STATE(mad_send_wr, expected_state) \
> + { \
> + if (IS_ENABLED(CONFIG_LOCKDEP)) \
> + WARN_ON(mad_send_wr->state != expected_state); \
> + }
> +#define EXPECT_MAD_STATE3(mad_send_wr, expected_state1, expected_state2, \
> + expected_state3) \
> + { \
> + if (IS_ENABLED(CONFIG_LOCKDEP)) \
> + WARN_ON(mad_send_wr->state != expected_state1 && \
> + mad_send_wr->state != expected_state2 && \
> + mad_send_wr->state != expected_state3); \
> + }
> +#define NOT_EXPECT_MAD_STATE(mad_send_wr, wrong_state) \
> + { \
> + if (IS_ENABLED(CONFIG_LOCKDEP)) \
> + WARN_ON(mad_send_wr->state == wrong_state); \
> + }
These could all be static inlines, otherwise at least
mad_send_wr->state needs brackets (mad_send_wr)->state
Jason
