Re: [PATCH 02/10] vfio: ccw: Transform FSM functions to return state

Cornelia Huck
2018-04-30T13:58:43Z

On Tue, 24 Apr 2018 10:22:15 +0200
Pierre Morel <pmorel@linux.vnet.ibm.com> wrote:

> On 24/04/2018 09:25, Dong Jia Shi wrote:
> > * Pierre Morel <pmorel@linux.vnet.ibm.com> [2018-04-19 16:48:05 +0200]:
> >
> >> We change the FSM functions to return the next state,
> >> and adapt the fsm_func_t function type.
> > I think I'd need to read the rest patches to understand why we need this
> > one, but no hurt to write some ideas that I noticed at my first glance.
> > See below please.
> >
> >> Signed-off-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
> >> ---
> >> drivers/s390/cio/vfio_ccw_fsm.c | 24 ++++++++++++++++--------
> >> drivers/s390/cio/vfio_ccw_private.h | 5 +++--
> >> 2 files changed, 19 insertions(+), 10 deletions(-)

> >> diff --git a/drivers/s390/cio/vfio_ccw_private.h b/drivers/s390/cio/vfio_ccw_private.h
> >> index 78a66d9..f526b18 100644
> >> --- a/drivers/s390/cio/vfio_ccw_private.h
> >> +++ b/drivers/s390/cio/vfio_ccw_private.h
> >> @@ -83,13 +83,14 @@ enum vfio_ccw_event {
> >> /*
> >> * Action called through jumptable.
> >> */
> >> -typedef void (fsm_func_t)(struct vfio_ccw_private *, enum vfio_ccw_event);
> >> +typedef int (fsm_func_t)(struct vfio_ccw_private *, enum vfio_ccw_event);
> >> extern fsm_func_t *vfio_ccw_jumptable[NR_VFIO_CCW_STATES][NR_VFIO_CCW_EVENTS];
> >>
> >> static inline void vfio_ccw_fsm_event(struct vfio_ccw_private *private,
> >> int event)
> >> {
> >> - vfio_ccw_jumptable[private->state][event](private, event);
> >> + private->state = vfio_ccw_jumptable[private->state][event](private,
> >> + event);
> > Since here it assigns new value to private->state, there is no need to
> > do that inside each fsm_func?
> Absolutely.
> I just kept the previous code, just adding the return private->state in
> the functions
> in this patch.
> merging the state and the return value is done in a later patch.
> If you prefer I can do it in this patch.

I think we should revisit this later. It's hard to judge this patch on
its own.