Re: [PATCH v5 1/6] IB/core, cma: Nice log-friendly string helpers

Doug Ledford
2015-05-12T17:21:47Z

[-- Attachment #1: Type: text/plain, Size: 2639 bytes --]

On Tue, 2015-05-12 at 09:56 -0400, Chuck Lever wrote:
> On May 12, 2015, at 8:59 AM, Or Gerlitz <gerlitz.or-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> > On Tue, May 12, 2015 at 3:05 PM, Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> >> Some of us keep revisiting the code to decode enumerations that
> >> appear in out logs. Let's borrow the nice logging helpers that
> >> exists in xprtrdma and rds for CMA events, IB events and WC statuses.
> >
> > [...]
> >
> >> +static const char * const cma_events[] = {
> >> + [RDMA_CM_EVENT_ADDR_RESOLVED] = "ADDR_RESOLVED",
> >> + [RDMA_CM_EVENT_ADDR_ERROR] = "ADDR_ERROR",
> >> + [RDMA_CM_EVENT_ROUTE_RESOLVED] = "ROUTE_RESOLVED",
> >> + [RDMA_CM_EVENT_ROUTE_ERROR] = "ROUTE_ERROR",
> >> + [RDMA_CM_EVENT_CONNECT_REQUEST] = "CONNECT_REQUEST",
> >> + [RDMA_CM_EVENT_CONNECT_RESPONSE]= "CONNECT_RESPONSE",
> >> + [RDMA_CM_EVENT_CONNECT_ERROR] = "CONNECT_ERROR",
> >> + [RDMA_CM_EVENT_UNREACHABLE] = "UNREACHABLE",
> >> + [RDMA_CM_EVENT_REJECTED] = "REJECTED",
> >> + [RDMA_CM_EVENT_ESTABLISHED] = "ESTABLISHED",
> >> + [RDMA_CM_EVENT_DISCONNECTED] = "DISCONNECTED",
> >> + [RDMA_CM_EVENT_DEVICE_REMOVAL] = "DEVICE_REMOVAL",
> >> + [RDMA_CM_EVENT_MULTICAST_JOIN] = "MULTICAST_JOIN",
> >> + [RDMA_CM_EVENT_MULTICAST_ERROR] = "MULTICAST_ERROR",
> >> + [RDMA_CM_EVENT_ADDR_CHANGE] = "ADDR_CHANGE",
> >> + [RDMA_CM_EVENT_TIMEWAIT_EXIT] = "TIMEWAIT_EXIT",
> >> +};
> >
> > Sagi,
> >
> > My vote goes to using easy-to-digest human-readable non-capital
> > letters strings here and
> > elsewhere (IB events, etc), as done today by the NFS code and
> > generally by errno based
> > helpers such as strerror, e.g
> >
> > "address resolved"
> > "address error"
> > "route resolved"
> >
> > Will be happy to hear what others think
>
> I generally prefer spelled-out strings over symbol names.
>
> For the strings in xprtrdma, I looked up the symbols in the IBTA spec,
> and used part of the description of each error. It’s obvious in most
> cases, but there are one or two that are a little odd (eg. MW_BIND_ERR
> is “memory management operation error”).

Likewise, I prefer the more complete description to a simple
regurgitation of C preprocessor symbols. Aside from that, I'm glad to
see this patchset come along because I hate looking up error numbers
too.

--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG KeyID: 0E572FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]