Re: [PATCH 3/3] md: add new / required locking for calls to blk_remove_plug and blk_plug_queue

On Wednesday April 30, dan.j.williams@intel.com wrote:

On Wed, Apr 30, 2008 at 5:36 PM, Neil Brown [off-list ref] wrote:

quoted

On Tuesday April 29, dan.j.williams@intel.com wrote:
 > Now that queue flags are no longer atomic (commit:
 > 75ad23bc0fcb4f992a5d06982bf0857ab1738e9e)  we must protect calls to
 > blk_remove_plug with spin_lock(q->queue_lock).

 Can't we just do

  q->queue_lock = &conf->device_lock

 and appropriate places in the various ->run functions?

 It seems to be that we are doing appropriate locking, we just need to
 convince queue_flag_set / queue_flag_clear that the correct lock is
 locked.
 ??

That's much simpler.  Hmm... as an additional cleanup should
device_lock then move to mddev_t since it really is a mddev-level
lock?  There seem to be more than a few places that cast
mddev->private just to get conf->device_lock.  Then all this lock
initialization is centralized.

Conversely, there seem to be plenty of places where we access
conf->device_lock and don't have an 'mddev' handy.
They could of course become
      conf->mddev->device_lock
but I'm not sure that is an improvement.

I think you would need to actually do the change, then decide if the
benefits outweigh the costs.

It's probably best to find a minimal solution to the current problem
and just add the assignments to q->queue_lock for now.  We can then
explore whether there are cleanup opportunities.

Thanks,
NeilBrown