Discussion:
[Openib-windows] [patch] cl_signal_osd.h
(too old to reply)
Yossi Leybovich
2005-07-11 11:07:40 UTC
Permalink
Fab

Attached patch file for cl_signal_osd.h (for registering to OSs signals.)
OpenSM need it.
We would be happy if you review it, and then CI this file to complib.

Thanks
Yossi
Fab Tillier
2005-07-12 00:17:31 UTC
Permalink
Sent: Monday, July 11, 2005 4:08 AM
Attached patch file for cl_signal_osd.h (for registering to OSs signals.)
OpenSM need it.
We would be happy if you review it, and then CI this file to complib.
What does cl_reg_sig_hdl map to for other platforms? Why not just call signal
directly? Does it ever resolve to anything but signal or a __noop macro? As it
stands, it doesn't seem like a very useful abstraction, as cl_reg_sig_hdl has
the same prototype as signal, and cl_sig_mask_sigint is a no-op.

Thanks,

- Fab
Yossi Leybovich
2005-07-12 05:44:32 UTC
Permalink
What does cl_reg_sig_hdl map to for other platforms?
[YL] We have implementation of this function in our Linux compilb for both
Windows and Linux, because our complib does not include any Linux
implementation I remove the code and left only direct call to signal .
But still to keep the same code base we need this function.

Why not just call signal directly?
[YL] as I explain in Linux complib we have other implementation.
We need it in order to keep the same code base

-----Original Message-----
From: Fab Tillier [mailto:***@silverstorm.com]
Sent: Tuesday, July 12, 2005 3:18 AM
To: 'Yossi Leybovich'
Cc: openib-***@openib.org; Windows Group
Subject: RE: [patch] cl_signal_osd.h
Sent: Monday, July 11, 2005 4:08 AM
Attached patch file for cl_signal_osd.h (for registering to OSs
signals.) OpenSM need it. We would be happy if you review it, and then
CI this file to complib.
What does cl_reg_sig_hdl map to for other platforms? Why not just call
signal directly? Does it ever resolve to anything but signal or a __noop
macro? As it stands, it doesn't seem like a very useful abstraction, as
cl_reg_sig_hdl has the same prototype as signal, and cl_sig_mask_sigint is a
no-op.

Thanks,

- Fab
Fab Tillier
2005-07-12 17:15:41 UTC
Permalink
Sent: Monday, July 11, 2005 10:45 PM
Post by Fab Tillier
What does cl_reg_sig_hdl map to for other platforms?
[YL] We have implementation of this function in our Linux compilb for both
Post by Fab Tillier
Windows and Linux, because our complib does not include any Linux
implementation I remove the code and left only direct call to signal .
But still to keep the same code base we need this function.
Why not just call signal directly?
[YL] as I explain in Linux complib we have other implementation.
We need it in order to keep the same code base
Is there a cl_signal.h file too, or just the cl_signal_osd.h file? If it's just
the cl_signal_osd.h file, I'll rename it to cl_signal.h, and put it in
inc\user\complib.

You do realize that ctrl-C signal handling has totally different semantics in
Windows - there's a note in the MSDN docs warning about this:

"Note SIGINT is not supported for any Win32 application, including Windows
98/Me and Windows NT/2000/XP. When a CTRL+C interrupt occurs, Win32 operating
systems generate a new thread to specifically handle that interrupt."

Take a look at SetConsoleCtrlHandler, as it should allow you to actually
implement the mask_sigint functionality by passing a NULL handler. The
SetConsoleCtrlHandler callback function also allows the application to control
the behavior in response to the signal, which is probably something you want to
do (to prevent the app from aborting, for example).

There doesn't be a function to unmask sigint - should there be one? If OSM
masks CTRL-C, why does it register for the SIGINT signal?

And lastly, please try to keep lines to 80 characters or less.

Thanks,

- Fab
Eitan Zahavi
2005-07-13 06:39:28 UTC
Permalink
Hi Fab,

Thanks for the useful note.
We currently catch ^C for OpenSM and OSMV to cleanup resources that would be
lost or create kernel oops. I guess that in Windows environment this is not
a must.

Also please note that we do use HUP signal to force a full sweep (will be
made available next OpenSM for Windoes release).

Your proposal of renaming the osm_signal_osd.h to osm_signal.h is fine.

Eitan


-----Original Message-----
From: Fab Tillier [mailto:***@silverstorm.com]
Sent: Tuesday, July 12, 2005 8:16 PM
To: 'Yossi Leybovich'
Cc: openib-***@openib.org; Windows Group; Eitan Zahavi; Aviram
Gutman
Subject: RE: [patch] cl_signal_osd.h
Sent: Monday, July 11, 2005 10:45 PM
Post by Fab Tillier
What does cl_reg_sig_hdl map to for other platforms?
[YL] We have implementation of this function in our Linux compilb for both
Post by Fab Tillier
Windows and Linux, because our complib does not include any Linux
implementation I remove the code and left only direct call to signal .
But still to keep the same code base we need this function.
Why not just call signal directly?
[YL] as I explain in Linux complib we have other implementation.
We need it in order to keep the same code base
Is there a cl_signal.h file too, or just the cl_signal_osd.h file? If it's
just
the cl_signal_osd.h file, I'll rename it to cl_signal.h, and put it in
inc\user\complib.

You do realize that ctrl-C signal handling has totally different semantics
in
Windows - there's a note in the MSDN docs warning about this:

"Note SIGINT is not supported for any Win32 application, including Windows
98/Me and Windows NT/2000/XP. When a CTRL+C interrupt occurs, Win32
operating
systems generate a new thread to specifically handle that interrupt."

Take a look at SetConsoleCtrlHandler, as it should allow you to actually
implement the mask_sigint functionality by passing a NULL handler. The
SetConsoleCtrlHandler callback function also allows the application to
control
the behavior in response to the signal, which is probably something you want
to
do (to prevent the app from aborting, for example).

There doesn't be a function to unmask sigint - should there be one? If OSM
masks CTRL-C, why does it register for the SIGINT signal?

And lastly, please try to keep lines to 80 characters or less.

Thanks,

- Fab
Sean Hefty
2005-07-13 16:38:33 UTC
Permalink
Thanks for the useful note.
We currently catch ^C for OpenSM and OSMV to cleanup resources that would be
lost or create kernel oops. I guess that in Windows environment this is not a
must.


If you need to catch ctrl-C to prevent a kernel oops, then something is wrong
with the implementation. I would think the same thing about cleanup.

- Sean
Eitan Zahavi
2005-07-14 06:13:58 UTC
Permalink
Hi

Clarification: The ^C was introduced long ago (~2years) into the code in
order to avoid resource loss that happened in IBAL. Later it proved itself
useful when the Gen1 was introduced. Today, I am not aware of any issue with
resources leaks etc. So practically one could stop catching ^C and probably
everything will work right in the OpenIB access layer.

Sorry about my improper misleading previous mail.

Eitan


-----Original Message-----
From: Sean Hefty [mailto:***@intel.com]
Sent: Wednesday, July 13, 2005 7:39 PM
To: 'Eitan Zahavi'; 'Fab Tillier'; Yossi Leybovich
Cc: Aviram Gutman; openib-***@openib.org; Windows Group
Subject: RE: [Openib-windows] RE: [patch] cl_signal_osd.h

Thanks for the useful note.
We currently catch ^C for OpenSM and OSMV to cleanup resources that would be
lost or create kernel oops. I guess that in Windows environment this is not
a
must.


If you need to catch ctrl-C to prevent a kernel oops, then something is
wrong
with the implementation. I would think the same thing about cleanup.

- Sean

Loading...