On Thu, Jun 23, 2016 at 08:01:30PM +0300, Sagi Grimberg wrote:
> In case we got an error completion the rdma queue pair
> is in error state, teardown the entire controller. Note
> that in recv or read error completion we might not have
> a controller yet, so check for the controller exsistence.
>
> Signed-off-by: Sagi Grimberg <sagi@xxxxxxxxxxx>
This looks fine minus a few minor codingstyle nitpicks that I'd be
happy to fix up:
> +static void nvmet_rdma_error_comp(struct nvmet_rdma_queue *queue)
> +{
> + if (queue->nvme_sq.ctrl)
> + nvmet_ctrl_fatal_error(queue->nvme_sq.ctrl);
> + else
> + /*
> + * we didn't setup the controller yet in case
> + * of admin connect error, just disconnect and
> + * cleanup the queue
> + */
> + nvmet_rdma_queue_disconnect(queue);
> +}
With such a long comment I'd prefer to have curly braces just to make
the else visually more obvious
> +
> + if (unlikely(wc->status != IB_WC_SUCCESS &&
> + wc->status != IB_WC_WR_FLUSH_ERR)) {
Indenting the second line of a condition by a single tab is always wrong,
either indent it with two tabs, or so that it aligns with first line.
The second is probably nicer here:
if (unlikely(wc->status != IB_WC_SUCCESS &&
wc->status != IB_WC_WR_FLUSH_ERR)) {
Given how many !success not !flush_err conditionals we have in various
drivers I wonder if we should have a helper in the RDMA core, though.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
