[Openib-windows] a possible bug

[Fabian Tillier]

Hi Leo,

On 6/7/06, Leonid Keller <leonid at mellanox.co.il> wrote:
>
> Hi Fab,
>
> DDK states in IoSkipCurrentIrpStackLocation() description:
>
>
> If your driver calls IoSkipCurrentIrpStackLocation, be careful not to modify
> the IO_STACK_LOCATION structure in a way that could inadvertently affect the
> lower driver or the system's behavior with respect to that driver. Examples
> include modifying the IO_STACK_LOCATION structure's Parameters union or
> calling IoMarkIrpPending.
>
> Look at hca_set_power(), case DevicePowerState:
>
>     we call IoMarkIrpPending(), then IoQueueWorkItem( .., __PowerDownCb,
> ..);
>
> Then __PowerDownCb calls IoSkipCurrentIrpStackLocation() and PoCallDriver().
>
> Looks like it's a wrong behaviour.

I think you're right.

> But what's the right one ?
>
> What do you think about such idea ?
>
>     -- set a completion routine in __PowerDownCb();
>
>     -- call PoStartNextPowerIrp() in te completion routine.

Yes, that sounds like the right thing to do.

> I.e., something like the following:
>
> static void __PowerDownCb(
>  IN    DEVICE_OBJECT*    p_dev_obj,
>  IN    void*      context )
> {
>
>     ...    // current stuff
>     mthca_remove_one( p_ext );
>
>     IoCopyCurrentIrpStackLocationToNext( p_irp );
>     IoSetCompletionRoutine( p_irp, __DevicePowerCbCompletion, NULL, TRUE,
> TRUE, TRUE );
>     PoCallDriver( p_ext->cl_ext.p_next_do, p_irp );
>     IoReleaseRemoveLock( &p_ext->cl_ext.remove_lock, p_irp );

Note that you also release the remove lock in the completion routine.
You probably only need one release between the two functions, likely
in the completion routine.

- Fab

>
> }
>
>
> static NTSTATUS __DevicePowerCbCompletion(
>  IN    DEVICE_OBJECT    *p_dev_obj,
>  IN    IRP       *p_irp,
>  IN    void      *context )
> {
>  hca_dev_ext_t  *p_ext  =
> (hca_dev_ext_t*)p_dev_obj->DeviceExtension;
>   PoStartNextPowerIrp( p_irp );
>   IoCompleteRequest( p_irp, IO_NO_INCREMENT );
>   IoReleaseRemoveLock( &p_ext->cl_ext.remove_lock, p_irp );
>   return STATUS_SUCCESS;
> }
>
> _______________________________________________
> openib-windows mailing list
> openib-windows at openib.org
> http://openib.org/mailman/listinfo/openib-windows
>
>
>