Re: [PATCH v2 06/10] vfio: ccw: Make FSM functions atomic

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05/06/2018 15:35, Cornelia Huck wrote:
On Tue, 5 Jun 2018 15:10:11 +0200
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:

On 05/06/2018 13:38, Cornelia Huck wrote:
On Fri, 25 May 2018 12:21:14 +0200
Pierre Morel <pmorel@xxxxxxxxxxxxxxxxxx> wrote:
We use mutex around the FSM function call to make the FSM
event handling and state change atomic.
I'm still not really clear as to what this mutex is supposed to
serialize:

- Modification of the state?
- Any calls in the state machine?
- A combination? (That would imply that we only deal with the state in
    the state machine.)
yes to all
But wouldn't that imply that you need to either take the mutex if you
do something dependent on the state, or fire an event in that case?

Yes, if it is not I forgot something important (like I did in patch 10)
vfio_ccw_fsm_event(private, event) takes the mutex on firering an event.

I have some cases where I do not respect this.
This is false and I must handle this with a new private variable,
this is where I test the state after having fired an event.
I will need to change this, in quiesce, reset, probe and open (others?).



Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxxxxxxx>
---
   drivers/s390/cio/vfio_ccw_drv.c     | 3 +--
   drivers/s390/cio/vfio_ccw_private.h | 3 +++
   2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/s390/cio/vfio_ccw_drv.c b/drivers/s390/cio/vfio_ccw_drv.c
index 6b7112e..98951d5 100644
--- a/drivers/s390/cio/vfio_ccw_drv.c
+++ b/drivers/s390/cio/vfio_ccw_drv.c
@@ -73,8 +73,6 @@ static void vfio_ccw_sch_io_todo(struct work_struct *work)
private = container_of(work, struct vfio_ccw_private, io_work);
   	vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_INTERRUPT);
-	if (private->mdev)
-		private->state = VFIO_CCW_STATE_IDLE;
Looks like an unrelated change? If you want to do all state changes
under the mutex, that should rather be moved than deleted, shouldn't it?
It is moved to fsm_irq() which is called under mutex.
fsm_irq() returns VFIO_CCW_STATE_IDLE.
So, should that go into another patch?

I will see if I can put it inside the patch 01/10 moving state change out of IRQ context.


--
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany

--
To unsubscribe from this list: send the line "unsubscribe linux-s390" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux