Messages in this thread | | | From | <> | Subject | Re: [PATCH v2 1/5] PCI/switchtec: Error out MRPC execution when MMIO reads fail | Date | Fri, 15 Oct 2021 22:13:36 +0000 |
| |
On Fri, 2021-10-15 at 03:21 +0200, Krzysztof Wilczyński wrote: > Hi Kelvin, > > Thank you for sending the series over! > > I am terribly sorry for a very late comment, especially since Bjorn > already > accepted this series to be included, but allow me for a small > question > below. > > [...] > > @@ -113,6 +127,7 @@ static void stuser_set_state(struct > > switchtec_user *stuser, > > [MRPC_QUEUED] = "QUEUED", > > [MRPC_RUNNING] = "RUNNING", > > [MRPC_DONE] = "DONE", > > + [MRPC_IO_ERROR] = "IO_ERROR", > > Looking at the above, and then looking at stuser_set_state(), which > contains the following local array definition: > > const char * const state_names[] = { > [MRPC_IDLE] = "IDLE", > [MRPC_QUEUED] = "QUEUED", > [MRPC_RUNNING] = "RUNNING", > [MRPC_DONE] = "DONE", > }; > > I was wondering if there might be a small benefit of declaring this > array > state_names[], or list of states if you wish, as static so that we > avoid > having to allocate space and fill it in with values every time this > functions runs? > > The function itself if referenced in few places as per: > > Index File Line Content > 1 drivers/pci/switch/switchtec.c 159 stuser_set_state(stuser, > MRPC_RUNNING); > 2 drivers/pci/switch/switchtec.c 178 stuser_set_state(stuser, > MRPC_QUEUED); > 3 drivers/pci/switch/switchtec.c 206 stuser_set_state(stuser, > MRPC_DONE); > 4 drivers/pci/switch/switchtec.c 567 stuser_set_state(stuser, > MRPC_IDLE); > > Even though the string representation of the state is ever only > printed if > a debug logging is requested, we would allocate and popular this > array > every time anyway, regardless of whether we print any debug > information or > not. > > What do you think?
Thank you Krzysztof. That will be an improvement. I can probably tweak it in the next patchset (coming soon).
Kelvin > > Krzysztof
| |