On Mon, 21 May 2001, Manuel Bouyer wrote:
> > I think you're missing the point. The only mechanism with which
> > scsipi_run_queue can be called now after a thaw is via the indirect softcall
> > mechanism in scsipi_channel_thaw (which is exposed). I don't care whether
> > scsipi_run_queue itself is private. I *do* care that you can't restart except
> > via a softcall. That's just plain wrong when you've already got a per-HBA
> > thread.
> 
> This is why I propose to make scsipi_channel_thaw public, this once can be
> called from your thread. But I don't think we should allow HBA drivers to
> bypass the freeze/thaw mechanism; this will break if we want to change the
> internals of scsipi.

Oops- sorry! It's scspi_channel_timed_thaw that does the softcall...

But scsipi_channel_thaw (which is public- I'm assuming that if it's in
scsipiconf.h it's public) doesn't call scsipi_run_queue- all it does is
decrement the qfreeze count.

The freeze/thaw mechanism may not be entirely complete yet anyway-

This is all coming up because I'm finally implementing a kthread to manage
loop events. What I'm forced to do now because I do not know how many channel
'freeze' events I get but I do know when they should all thaw, is to do the
following:

        case ISPASYNC_LIP:
                if (isp->isp_osinfo.blocked == 0) {
                        isp->isp_osinfo.blocked = 1;
                        scsipi_channel_freeze(&isp->isp_chanA, 1);
                }
                isp_prt(isp, ISP_LOGINFO, "LIP Received");
                break; 
        case ISPASYNC_LOOP_RESET:
                if (isp->isp_osinfo.blocked == 0) {
                        isp->isp_osinfo.blocked = 1;
                        scsipi_channel_freeze(&isp->isp_chanA, 1);
                }
                isp_prt(isp, ISP_LOGINFO, "Loop Reset Received");
                break;
        case ISPASYNC_LOOP_DOWN:
                if (isp->isp_osinfo.blocked == 0) {
                        isp->isp_osinfo.blocked = 1;
                        scsipi_channel_freeze(&isp->isp_chanA, 1);
                }
                isp_prt(isp, ISP_LOGINFO, "Loop DOWN");
                break;  
...
        case ISPASYNC_CHANGE_NOTIFY:
                if (isp->isp_osinfo.blocked == 0) {
                        isp->isp_osinfo.blocked = 1;
                        scsipi_channel_freeze(&isp->isp_chanA, 1);
                }
                if (arg == ISPASYNC_CHANGE_PDB) {
                        isp_prt(isp, ISP_LOGINFO, "Port Database Changed");  
                } else if (arg == ISPASYNC_CHANGE_SNS) {
                        isp_prt(isp, ISP_LOGINFO,
                            "Name Server Database Changed");
                }       
                /*      
                 * Note that we have work for the thread to do, and
                 * if the thread is here already, wake it up.
                 */
                isp->isp_osinfo.threadwork++;
                if (isp->isp_osinfo.thread) {
                        wakeup(&isp->isp_osinfo.thread);
                }       
                break;

and then the last event is the one which allows me to fire up the loop cleanup
thread, which when it gets done rebuilding loop state and (re)finding all
devices, wants to thaw everything and call scsipi_run_queue directly so that
things run now, darn it, not off of a softcall later:

                if (isp->isp_osinfo.blocked) {
                        isp->isp_osinfo.blocked = 0;
                        isp_prt(isp, ISP_LOGINFO, "restarting queues");
                        isp->isp_chanA.chan_qfreeze = 0;
                        scsipi_run_queue(&isp->isp_chanA);
                }

The two latter line there are both layering violations. What I'd *really* like
to see is:

	scsipi_channel_thaw(&isp->isp_chanA, -1);

to be taken as:

	zero freeze count (-1 count argument)

and to have scsipi_channel_thaw call scsipi_run_queue.


-matt