[Xen-devel] Blockdriver Concurency

[Horms <horms@xxxxxxxxxxxx>]

Hi,

I've been looking over blkfront.c and I'm quite concerned about the
concurrency issues in there. Overnight I noticed a commit[1] to try and fix
a race, so I thought I'd air my thoughts here. Particularly as I'm
quite unfamiliar with Xen.

[1] 
http://xenbits.xensource.com/xen-unstable.hg?cmd=changeset;node=f6fdb6e0d3c96241354fe87cb264d740dfe4087c

There seem to be 2 classes of functions in blkfront.c, 

Control functions which are registered with xenbus_unregister_driver()
  blkfront_probe
  blkfront_resume
  blkfront_remove
  backend_changed

And call-back type functions
  blkif_restart_queue_callback
  blkif_int
  blkif_restart

My first area of concern is that while nothing else will
ever happen in the driver while blkfront_probe() is being called,
because the driver just doesn't exist at that point, is the same
true of blkfront_resume(), blkfront_remove() and backend_changed()?
If they can be called concurrently there seems to be ample scope
for all sorts of bad things to happen.

So my question is, can these functions be called concurrently?

My second area of concern relates to the callback functions.  They are
pretty good about holding blkif_io_lock when doing anything, so they
won't tread on each other. However the control functions only really
have a lock around access to info->control, and in the case of
backend_changed() it doesn't even take a lock around accessing
info->control, so its possible for a callback to start while one of the
control functions (excluding blkfront_probe()) is running.

Even with the patch above, blkfront_resume() and blkfront_remove() could
run while the worker thread is running blkif_restart. And worse still,
the worker queue could run after blkfront_remove() and thus try and
access a freed info structure.

Lastly, I am not at all clear on what should be done with 
any io queued on info->rq if blkfront_remove() is called.
And I'm not sure why the handling of info->rq isn't done
with a callback registered with blk_queue_make_request().


-- 
Horms

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel