Skip to content
  • Emanuele Giuseppe Esposito's avatar
    7e8c182f
    block: use transactions as a replacement of ->{can_}set_aio_context() · 7e8c182f
    Emanuele Giuseppe Esposito authored
    
    
    Simplify the way the aiocontext can be changed in a BDS graph.
    There are currently two problems in bdrv_try_set_aio_context:
    - There is a confusion of AioContext locks taken and released, because
      we assume that old aiocontext is always taken and new one is
      taken inside.
    
    - It doesn't look very safe to call bdrv_drained_begin while some
      nodes have already switched to the new aiocontext and others haven't.
      This could be especially dangerous because bdrv_drained_begin polls, so
      something else could be executed while graph is in an inconsistent
      state.
    
    Additional minor nitpick: can_set and set_ callbacks both traverse the
    graph, both using the ignored list of visited nodes in a different way.
    
    Therefore, get rid of all of this and introduce a new callback,
    change_aio_context, that uses transactions to efficiently, cleanly
    and most importantly safely change the aiocontext of a graph.
    
    This new callback is a "merge" of the two previous ones:
    - Just like can_set_aio_context, recursively traverses the graph.
      Marks all nodes that are visited using a GList, and checks if
      they *could* change the aio_context.
    - For each node that passes the above check, drain it and add a new transaction
      that implements a callback that effectively changes the aiocontext.
    - Once done, the recursive function returns if *all* nodes can change
      the AioContext. If so, commit the above transactions.
      Regardless of the outcome, call transaction.clean() to undo all drains
      done in the recursion.
    - The transaction list is scanned only after all nodes are being drained, so
      we are sure that they all are in the same context, and then
      we switch their AioContext, concluding the drain only after all nodes
      switched to the new AioContext. In this way we make sure that
      bdrv_drained_begin() is always called under the old AioContext, and
      bdrv_drained_end() under the new one.
    - Because of the above, we don't need to release and re-acquire the
      old AioContext every time, as everything is done once (and not
      per-node drain and aiocontext change).
    
    Note that the "change" API is not yet invoked anywhere.
    
    Signed-off-by: default avatarEmanuele Giuseppe Esposito <eesposit@redhat.com>
    Message-Id: <20221025084952.2139888-3-eesposit@redhat.com>
    Reviewed-by: default avatarKevin Wolf <kwolf@redhat.com>
    Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
    7e8c182f
    block: use transactions as a replacement of ->{can_}set_aio_context()
    Emanuele Giuseppe Esposito authored
    
    
    Simplify the way the aiocontext can be changed in a BDS graph.
    There are currently two problems in bdrv_try_set_aio_context:
    - There is a confusion of AioContext locks taken and released, because
      we assume that old aiocontext is always taken and new one is
      taken inside.
    
    - It doesn't look very safe to call bdrv_drained_begin while some
      nodes have already switched to the new aiocontext and others haven't.
      This could be especially dangerous because bdrv_drained_begin polls, so
      something else could be executed while graph is in an inconsistent
      state.
    
    Additional minor nitpick: can_set and set_ callbacks both traverse the
    graph, both using the ignored list of visited nodes in a different way.
    
    Therefore, get rid of all of this and introduce a new callback,
    change_aio_context, that uses transactions to efficiently, cleanly
    and most importantly safely change the aiocontext of a graph.
    
    This new callback is a "merge" of the two previous ones:
    - Just like can_set_aio_context, recursively traverses the graph.
      Marks all nodes that are visited using a GList, and checks if
      they *could* change the aio_context.
    - For each node that passes the above check, drain it and add a new transaction
      that implements a callback that effectively changes the aiocontext.
    - Once done, the recursive function returns if *all* nodes can change
      the AioContext. If so, commit the above transactions.
      Regardless of the outcome, call transaction.clean() to undo all drains
      done in the recursion.
    - The transaction list is scanned only after all nodes are being drained, so
      we are sure that they all are in the same context, and then
      we switch their AioContext, concluding the drain only after all nodes
      switched to the new AioContext. In this way we make sure that
      bdrv_drained_begin() is always called under the old AioContext, and
      bdrv_drained_end() under the new one.
    - Because of the above, we don't need to release and re-acquire the
      old AioContext every time, as everything is done once (and not
      per-node drain and aiocontext change).
    
    Note that the "change" API is not yet invoked anywhere.
    
    Signed-off-by: default avatarEmanuele Giuseppe Esposito <eesposit@redhat.com>
    Message-Id: <20221025084952.2139888-3-eesposit@redhat.com>
    Reviewed-by: default avatarKevin Wolf <kwolf@redhat.com>
    Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
Loading