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
