On Wednesday April 30, dan.j.williams@xxxxxxxxx wrote:
> On Wed, Apr 30, 2008 at 5:36 PM, Neil Brown <neilb@xxxxxxx> wrote:
> > On Tuesday April 29, dan.j.williams@xxxxxxxxx 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
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
