Re: location of bioq lock

scott, i probably did not provide enough context...

To reassure you, I dont intend to change the status quo - the
queue is owned by the driver, and the locking scheme remains the same.

What we are trying to do is abstract the disk scheduler API so that
if a user, or a subsystem, or a piece of hardware, could benefit
from a different scheduler than the default, it can be easily plugged in.

    In RELENG_4 and RELENG_5 most drivers use bioq_disksort(), which
    optimizes for throughput, but may not be ideal when apps have      
    even soft real-time requirements (e.g. media players), plus
    there are different approaches (e.g. the anticipatory scheduling
    that someone referenced) that could prove more effective. Stuff     
    like RAID drivers might have different request ordering
    requirements. Even the ATA driver in HEAD uses a different 
    scheduler than bioq_disksort, but for lack of a proper API
    that's hardwired in the driver.
    And then as you say, the hardware might have an intelligent 
    controller so it might be worthwhile disabling sorting - but
    then, the same driver e.g.  SCSI or ATA might talk to differently   
    'smart' hardware and so having configurable per-device schedulers  
    might be useful.

Now, if one sees the disk scheduler as a boot-time option, then
the bioq_*() functions are all one needs - the API calls assume
that the subsystem is already locked so nobody except the driver
needs to know about the lock.

However, if one wants to be able to switch schedulers at runtime
(e.g. through a sysctl), then at the time of a switch you may need to
move requests from the old scheduler to the new one, and in the 
process you have to lock each queue before playing with it.

So the issue is _not_ changing the locking scheme, but just making
the sysctl handler aware of the address (and maybe type) of the lock.

Here are the two ways that I suggested - 1) put the lock in the queue
so its address is implicitly known, or 2) pass it as an additional
argument to bioq_init(). Either way, when the sysctl handler needs
to play with the bioq outside the normal requests issued by
the driver, it knows which lock to grab.  

I am totally with you when say that a single lock covering not
just the bioq is more efficient - this seems to push towards
method #2, which overall is more flexible.

	cheers
	luigi

On Fri, Jul 08, 2005 at 08:41:22PM +0100, Robert Watson wrote:
> 
> On Fri, 8 Jul 2005, Luigi Rizzo wrote:
> 
> > 1) put the lock in the struct bio_queue_head.
> >   This is the same thing done in struct g_bioq defined in
> >   sys/geom/geom.h . Quite clean, except that perhaps some
> >   users of bio_queue_head still run under Giant (e.g. cam/scsi ?)
> >   and so it is not possible to 'bring in' the lock.
> >
> > 2) change bioq_init() so that it takes also a pointer to the mtx
> >   that protects the queue.
> >   This is probably less clean, but perhaps a bit more flexible because
> >   the queue and its lock are decoupled. Also it permits to deal
> >   with the 'Giant' case easily.
> >
> > Other ideas ?
> 
> In the network stack work, we started out with locks tightly coupled with 
> the queues they protected as part of an early design decision to embed 
> mutexes in ifqueue's, one of the widely used queueing structures.  We're 
> actually exploring backing off that decision now such that components use 
> queues as a "library", and use their own synchronization to protect the 
> queue.  This allows, for example, lock coalescing across multiple queues, 
> or combining of queue mutexes with larger component locks.  It also allows 
> queues to be agnostic of the lock type that is used to protect them, so a 
> consumer could use an ex/rwlock or the like, or for that matter a spin 
> lock.  It also allows lock-free access to the queue where that is 
> appropriate.
> 
> FYI, one interesting point regarding lock order: in the ifqueue lock 
> model, ifqueue locks were leaf mutexes.  However, if the scope of locks 
> protection queues expands, the replacement locks may well not be leaf 
> mutexes.  This is relevant in "hand-off" scenarios, where before 
> contention on a queue mutex was very unlikely during a "grab, insert, 
> drop" scenario, contention chances are increased as the lock might also 
> cover other significantly time-consuming things, such as data copies, 
> hardware I/O interactions, and so on.  This might, or might not, outweight 
> the overhead of the additional locking, and is worth keeping in mind.
> 
> So based on that experience, my suggestion is to make locking a property 
> of the consumer of the API, not the provider, and to create macros or 
> queue wrapper functions in the consumer that *are* aware of the locking 
> semantics, for the purposes of code simplification, assertion placement, 
> and so on.
> 
> However, the network stack is fairly different from the storage I/O stack, 
> so the lessons (while interesting) might well not hold there.
> 
> Robert N M Watson
> _______________________________________________
> freebsd-current_at_freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe_at_freebsd.org"