Messages in this thread | | | Subject | Re: [PATCH v2 10/10] vfio: ccw: Let user wait when busy on IO | From | Pierre Morel <> | Date | Tue, 5 Jun 2018 15:02:06 +0200 |
| |
On 25/05/2018 16:04, Heiko Carstens wrote: > On Fri, May 25, 2018 at 12:21:18PM +0200, Pierre Morel wrote: >> In the current implementation, we do not want to start a new SSCH >> command before the last one ends. >> >> Currently the user needs to poll on the -EBUSY error to >> wait before sending a new request. >> >> Let's be friendly with global warming and let the user sleep >> until he may send a new request. >> >> Let's make the caller wait until the last SSCH ends. >> >> Signed-off-by: Pierre Morel <pmorel@linux.vnet.ibm.com> >> --- >> drivers/s390/cio/vfio_ccw_fsm.c | 4 ++++ >> drivers/s390/cio/vfio_ccw_ops.c | 6 ++++++ >> drivers/s390/cio/vfio_ccw_private.h | 1 + >> 3 files changed, 11 insertions(+) >> >> diff --git a/drivers/s390/cio/vfio_ccw_fsm.c b/drivers/s390/cio/vfio_ccw_fsm.c >> index c37052d..97b74a1 100644 >> --- a/drivers/s390/cio/vfio_ccw_fsm.c >> +++ b/drivers/s390/cio/vfio_ccw_fsm.c >> @@ -200,6 +200,10 @@ static int fsm_irq(struct vfio_ccw_private *private) >> >> if (private->io_trigger) >> eventfd_signal(private->io_trigger, 1); >> + >> + if (private->io_completion) >> + complete(private->io_completion); >> + >> return VFIO_CCW_STATE_IDLE; >> } >> >> diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c >> index b202e73..39beb6e 100644 >> --- a/drivers/s390/cio/vfio_ccw_ops.c >> +++ b/drivers/s390/cio/vfio_ccw_ops.c >> @@ -183,6 +183,7 @@ static ssize_t vfio_ccw_mdev_write(struct mdev_device *mdev, >> struct vfio_ccw_private *private; >> struct ccw_io_region *region; >> union scsw *scsw; >> + DECLARE_COMPLETION_ONSTACK(completion); >> >> if (*ppos + count > sizeof(*region)) >> return -EINVAL; >> @@ -196,6 +197,11 @@ static ssize_t vfio_ccw_mdev_write(struct mdev_device *mdev, >> scsw = (union scsw *) ®ion->scsw_area; >> switch (scsw->cmd.fctl) { >> case SCSW_FCTL_START_FUNC: >> + if (private->state == VFIO_CCW_STATE_BUSY) { >> + private->io_completion = &completion; >> + if (wait_for_completion_interruptible(&completion)) >> + return -EINTR; >> + } > What prevents a state change between checking the state and before > private->io_completion is set? If that happens you would end with an > endless wait. > > Similarly, you would have memory corruption if the task would be > interrupted and if the function would be left, ending up with a stale > private->io_completion completion pointer. > The complete(private->io_completion) call will then write to a memory > location that might already be reused. > > Just my 0.02 after having a very very short look ;)
Right, completely false, I should pay a little more (at least) attention.
Thanks to have had a very very short (but sharp)look.
Pierre
-- Pierre Morel Linux/KVM/QEMU in Böblingen - Germany
| |