On Mon, 2021-09-27 at 09:32 -0300, Jason Gunthorpe wrote: > On Fri, Sep 24, 2021 at 04:45:02PM -0400, Eric Farman wrote: > > On Thu, 2021-09-09 at 16:38 -0300, Jason Gunthorpe wrote: > > > Having a mdev pointer floating about in addition to a struct > > > vfio_device > > > is confusing. It is only used for three things: > > > > > > - Getting the mdev 'struct device *' - this is the same as > > > private->vdev.dev > > > > > > - Printing the uuid of the mdev in logging. The uuid is also the > > > dev_name > > > of the mdev so this is the same string as > > > dev_name(private->vdev.dev) > > > > > > - A weird attempt to fence the vfio_ccw_sch_io_todo() work. This > > > work > > > is > > > only queued during states IDLE/PROCESSING/PENDING and flushed > > > when > > > entering CLOSED. Thus the work already cannot run when the mdev > > > is > > > NULL. > > > Remove it. > > > > > > Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx> > > > drivers/s390/cio/vfio_ccw_drv.c | 6 ++-- > > > drivers/s390/cio/vfio_ccw_fsm.c | 48 +++++++++++++-------- > > > ---- > > > drivers/s390/cio/vfio_ccw_ops.c | 16 ++++------ > > > drivers/s390/cio/vfio_ccw_private.h | 2 -- > > > include/linux/mdev.h | 4 --- > > > 5 files changed, 30 insertions(+), 46 deletions(-) > > > > I like this patch. Unfortunately it depends on the removal of a > > hunk in > > patch 4, which sets the FSM state to different values based on > > whether > > private->mdev is NULL or not, so can't go on its own. Need to spend > > more time thinking about that patch. > > The FSM patch is important, really what is happening is the FSM logic > takes on the roles that was being split all over the place with other > logic, like this mdev stuff. To make that work we need a FSM that > makes sense.. No argument from me about that. My point is that I could consume this patch easier than the FSM patch, and need to get back to that one. Eric > > Jason