Re: [RFC PATCH v4 0/4] vfio-ccw: Fix interrupt handling for HALT/CLEAR

Eric Farman
2021-04-23T19:07:23Z

On Fri, 2021-04-23 at 19:08 +0200, Halil Pasic wrote:
> On Fri, 23 Apr 2021 11:53:06 -0400
> Eric Farman <farman@linux.ibm.com> wrote:
>
> > > Is in your opinion the vfio-ccw kernel module data race free with
> > > this
> > > series applied?
> >
> > I have no further concerns.
>
> I take this for a "yes, in my opinion it is data race free".

You asked about once this series is applied, which it is not.

>
> Please explain me, how do we synchronize access to
> a) private->state
> b) cp->initialized?
>
> Asuming we both use the definition from the C standard, the multiple
> threads potentially concurrently accessing private->state or
> cp->initialized are not hard to find.
>

Correct. The examples you provide below are the subject of patch 2,
which has already been identified as having issues and will be
reworked.

Thanks,
Eric

> For the former you can take vfio_ccw_sch_io_todo() (has both a read
> and a write, and runs from the workqueue) and fsm_io_request() (has
> both read and write). I guess the accesses from fsm_io_request() are
> all with the io_mutex held, but the accesses form
> vfio_ccw_sch_io_todo()
> are not with the io_mutex held AFAICT.
>
> And the same is true for the latter with the difference that
> vfio_ccw_sch_io_todo() manipulates cp->initialized via
> cp_update_scsw()
> and cp_free().
>
> These are either data races, or I'm missing something. Let me also
> note that C programs data races are bad news, because of undefined
> behavior.
>
> Regards,
> Halil
>
>