[ofw] Patch: Replace memory allocator with a memory allocator that works natively with 0 bytes allocations
Hefty, Sean
sean.hefty at intel.com
Tue Aug 31 09:30:59 PDT 2010
There are several unrelated changes in this patch, and I personally still don't like the idea. First, it adds overhead that's not needed in most cases. Second, it uses a function name that looks like a Windows call, but isn't. Someone reading the code has to discover that the call is some macro. And in order to make this change, you already have to look at and touch everywhere that the size could be 0. Why not just fix those places already?
There is already WAY too much abstraction in the code base. Adding more does not fix things, it only makes it worse. This can end up hiding bugs. Upper level code should not work around problems in other code, especially drivers.
> -----Original Message-----
> From: ofw-bounces at lists.openfabrics.org [mailto:ofw-
> bounces at lists.openfabrics.org] On Behalf Of Tzachi Dar
> Sent: Tuesday, August 31, 2010 8:45 AM
> To: ofw at lists.openfabrics.org
> Subject: [ofw] Patch: Replace memory allocator with a memory allocator that
> works natively with 0 bytes allocations
>
> Signed off by: xalex
>
>
>
> This patch was already posted to the community in the past and was not
> fully accepted. I would like to resend it again.
>
>
>
> The main goal of this fix was to allow calling ExAllocatePoolWithTag with a
> value of zero. There have been cases that we have noticed in which
> ExAllocatePoolWithTag was called with a value of zero, and this has caused
> BSOD later.
>
>
>
> Instead of looking for all places in the code where we should fix, we have
> added this save macro that will return null in the case of zero size. All
> callers are handling the return of NULL already.
>
>
>
> Thanks
>
> Tzachi
>
>
>
>
>
>
>
> --- core/bus/kernel/bus_port_mgr.c Tue Apr 27 18:39:46 2010
>
> +++ core/bus/kernel/bus_port_mgr.c Mon Jun 28 10:52:13 2010
>
> @@ -27,7 +27,7 @@
>
> * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>
> * SOFTWARE.
>
> *
>
> - * $Id: bus_port_mgr.c 2510 2009-10-26 10:05:59Z leonidk $
>
> + * $Id: bus_port_mgr.c 5186 2009-11-29 11:58:37Z leonid $
>
> */
>
>
>
>
>
> @@ -1599,7 +1599,7 @@
>
> dev_id_size = p_ext->pdo.p_pdo_device_info->device_id_size;
>
>
>
> /* Device ID is "IBA\SID_<sid> where <sid> is the IO device
> Service ID. */
>
> - p_string = ExAllocatePoolWithTag( NonPagedPool, dev_id_size,
> 'vedq' );
>
> + p_string = ExAllocatePoolWithTagSafeEx( NonPagedPool,
> dev_id_size, 'vedq' );
>
> if( !p_string )
>
> {
>
> BUS_TRACE_EXIT( BUS_DBG_ERROR,
>
> @@ -1635,7 +1635,7 @@
>
>
>
> dev_id_size = p_ext->pdo.p_pdo_device_info-
> >hardware_id_size;
>
>
>
> - p_string = ExAllocatePoolWithTag( NonPagedPool, dev_id_size,
> 'ihqp' );
>
> + p_string = ExAllocatePoolWithTagSafeEx( NonPagedPool,
> dev_id_size, 'ihqp' );
>
> if( !p_string )
>
> {
>
> BUS_TRACE_EXIT( BUS_DBG_ERROR,
>
> @@ -1669,8 +1669,8 @@
>
> p_ext = (bus_port_ext_t*)p_dev_obj->DeviceExtension;
>
>
>
> dev_id_size = p_ext->pdo.p_pdo_device_info-
> >compatible_id_size;
>
> -
>
> - p_string = ExAllocatePoolWithTag( NonPagedPool, dev_id_size,
> 'ihqp' );
>
> +
>
> + p_string = ExAllocatePoolWithTagSafeEx( NonPagedPool,
> dev_id_size, 'ihqp' );
>
> if( !p_string )
>
> {
>
> BUS_TRACE_EXIT( BUS_DBG_ERROR,
>
> @@ -1753,8 +1753,7 @@
>
> return STATUS_NO_SUCH_DEVICE;
>
> }
>
>
>
> -
>
> - p_string = ExAllocatePoolWithTag( NonPagedPool, p_ext-
> >pdo.p_pdo_device_info->description_size, 'edqp' );
>
> + p_string = ExAllocatePoolWithTagSafeEx( NonPagedPool, p_ext-
> >pdo.p_pdo_device_info->description_size, 'edqp' );
>
>
>
> if( !p_string )
>
> {
>
> @@ -1796,6 +1795,12 @@
>
> return STATUS_NO_SUCH_DEVICE;
>
> }
>
>
>
> + if (!p_ext->pdo.h_ca)
>
> + {
>
> + BUS_TRACE_EXIT( BUS_DBG_ERROR, ("HCA is still
> or already not acquired !\n") );
>
> + return STATUS_DEVICE_NOT_READY;
>
> + }
>
> +
>
> p_hca_dev = p_ext->pdo.h_ca->p_hca_dev;
>
>
>
> /* Get the length of the HCA's location. */
>
> --- core/complib/kernel/cl_memory_osd.c Tue Apr 27 18:39:18 2010
>
> +++ core/complib/kernel/cl_memory_osd.c Mon Jun 28 10:52:13 2010
>
> @@ -26,7 +26,7 @@
>
> * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>
> * SOFTWARE.
>
> *
>
> - * $Id: cl_memory_osd.c 920 2007-12-18 17:39:47Z stansmith $
>
> + * $Id: cl_memory_osd.c 4989 2009-10-22 16:04:40Z xalex $
>
> */
>
>
>
>
>
> @@ -38,6 +38,7 @@
>
> IN const size_t size,
>
> IN const boolean_t pageable )
>
> {
>
> +
>
> if( pageable )
>
> {
>
> CL_ASSERT( KeGetCurrentIrql() <
> DISPATCH_LEVEL );
>
> @@ -46,7 +47,7 @@
>
> else
>
> {
>
> CL_ASSERT( KeGetCurrentIrql() <=
> DISPATCH_LEVEL );
>
> - return( ExAllocatePoolWithTag( NonPagedPool,
> size, 'virp' ) );
>
> + return( ExAllocatePoolWithTagSafeEx(
> NonPagedPool, size, 'virp' ) );
>
> }
>
> }
>
>
>
> --- core/complib/cl_map.c Mon Aug 09 11:55:34 2010
>
> +++ core/complib/cl_map.c Mon Jun 28 10:52:13 2010
>
> @@ -27,7 +27,7 @@
>
> * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>
> * SOFTWARE.
>
> *
>
> - * $Id: cl_map.c 2799 2010-05-21 19:10:08Z stansmith $
>
> + * $Id: cl_map.c 5147 2009-11-17 15:42:34Z leonid $
>
> */
>
>
>
>
>
> @@ -703,6 +703,7 @@
>
> return( p_item );
>
> }
>
>
>
> +
>
> void
>
> cl_qmap_apply_func(
>
> IN const cl_qmap_t* const p_map,
>
> @@ -1706,67 +1707,6 @@
>
> p_map->state = CL_INITIALIZED;
>
>
>
> cl_fmap_remove_all( p_map );
>
> -}
>
> -
>
> -
>
> -cl_fmap_item_t*
>
> -cl_fmap_match(
>
> - IN const cl_fmap_t *const p_map,
>
> - IN const void *const p_key,
>
> - IN cl_pfn_fmap_cmp_t pfn_compare)
>
> -{
>
> - cl_fmap_item_t *p_item;
>
> - int cmp;
>
> -
>
> - CL_ASSERT(p_map);
>
> - CL_ASSERT(p_map->state == CL_INITIALIZED);
>
> -
>
> - p_item = __cl_fmap_root(p_map);
>
> -
>
> - while (p_item != &p_map->nil) {
>
> - cmp = pfn_compare ? pfn_compare(p_key,
> p_item->p_key) :
>
> - p_map->pfn_compare(p_key,
> p_item->p_key);
>
> -
>
> - if (!cmp)
>
> - break; /* just right */
>
> -
>
> - if (cmp < 0)
>
> - p_item = p_item->p_left;
> /* too small */
>
> - else
>
> - p_item = p_item->p_right;
> /* too big */
>
> - }
>
> -
>
> - return (p_item);
>
> -}
>
> -
>
> -
>
> -cl_fmap_item_t*
>
> -cl_fmap_get_next(
>
> - IN const cl_fmap_t * const p_map,
>
> - IN const void *const p_key)
>
> -{
>
> - cl_fmap_item_t *p_item;
>
> - cl_fmap_item_t *p_item_found;
>
> - int cmp;
>
> -
>
> - CL_ASSERT(p_map);
>
> - CL_ASSERT(p_map->state == CL_INITIALIZED);
>
> -
>
> - p_item = __cl_fmap_root(p_map);
>
> - p_item_found = (cl_fmap_item_t *) & p_map->nil;
>
> -
>
> - while (p_item != &p_map->nil) {
>
> - cmp = p_map->pfn_compare(p_key, p_item-
> >p_key);
>
> -
>
> - if (cmp < 0) {
>
> - p_item_found = p_item;
>
> - p_item = p_item->p_left;
> /* too small */
>
> - } else {
>
> - p_item = p_item->p_right;
> /* too big or match */
>
> - }
>
> - }
>
> -
>
> - return (p_item_found);
>
> }
>
>
>
>
>
> --- core/iou/kernel/iou_ioc_mgr.c Tue Apr 27 18:41:52 2010
>
> +++ core/iou/kernel/iou_ioc_mgr.c Mon Jun 28 10:52:15 2010
>
> @@ -27,7 +27,7 @@
>
> * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>
> * SOFTWARE.
>
> *
>
> - * $Id: iou_ioc_mgr.c 2510 2009-10-26 10:05:59Z leonidk $
>
> + * $Id: iou_ioc_mgr.c 4989 2009-10-22 16:04:40Z xalex $
>
> */
>
>
>
>
>
> @@ -952,7 +952,8 @@
>
> {
>
>
>
> dev_id_size = (p_ext-
> >pdo.p_pdo_device_info)->device_id_size;
>
> - p_string = ExAllocatePoolWithTag(
> NonPagedPool, dev_id_size, 'didq' );
>
> +
>
> + p_string = ExAllocatePoolWithTagSafeEx(
> NonPagedPool, dev_id_size, 'didq' );
>
>
>
> if( !p_string )
>
> {
>
> @@ -1027,7 +1028,7 @@
>
> {
>
> hw_id_size = p_ext->pdo.p_pdo_device_info-
> >hardware_id_size;
>
>
>
> - p_string = ExAllocatePoolWithTag(
> NonPagedPool, hw_id_size, 'ihqi' );
>
> + p_string = ExAllocatePoolWithTagSafeEx(
> NonPagedPool, hw_id_size, 'ihqi' );
>
> if( !p_string )
>
> {
>
> IOU_PRINT_EXIT(
> TRACE_LEVEL_ERROR, IOU_DBG_ERROR,
>
> @@ -1142,8 +1143,8 @@
>
> {
>
>
>
> compat_id_size = p_ext-
> >pdo.p_pdo_device_info->compatible_id_size;
>
> -
>
> - p_string = ExAllocatePoolWithTag(
> NonPagedPool, compat_id_size, 'icqi' );
>
> +
>
> + p_string = ExAllocatePoolWithTagSafeEx(
> NonPagedPool, compat_id_size, 'icqi' );
>
>
>
> if( !p_string )
>
> {
>
> @@ -1302,7 +1303,7 @@
>
>
>
> if ( p_ext->pdo.p_pdo_device_info )
>
> {
>
> - p_string = ExAllocatePoolWithTag(
> NonPagedPool, p_ext->pdo.p_pdo_device_info->description_size,
>
> + p_string = ExAllocatePoolWithTagSafeEx(
> NonPagedPool, p_ext->pdo.p_pdo_device_info->description_size,
>
>
> 'edqi' );
>
> if( !p_string )
>
> {
>
> --- core/winmad/kernel/wm_driver.c Tue Apr 27 18:38:36 2010
>
> +++ core/winmad/kernel/wm_driver.c Mon Jun 28 10:52:13 2010
>
> @@ -238,8 +238,8 @@
>
> attr = NULL;
>
> goto out;
>
> }
>
> -
>
> - attr = ExAllocatePoolWithTag(PagedPool, size, 'acmw');
>
> +
>
> + attr = ExAllocatePoolWithTagSafeEx(PagedPool, size, 'acmw');
>
> if (attr == NULL) {
>
> goto out;
>
> }
>
> @@ -269,7 +269,8 @@
>
> }
>
>
>
> size = sizeof(WM_IB_PORT) * attr->num_ports;
>
> - pDevice->pPortArray = ExAllocatePoolWithTag(PagedPool, size,
> 'pimw');
>
> +
>
> + pDevice->pPortArray = ExAllocatePoolWithTagSafeEx(PagedPool,
> size, 'pimw') ;
>
> if (pDevice->pPortArray == NULL) {
>
> status = STATUS_NO_MEMORY;
>
> goto out;
>
> --- core/winverbs/kernel/wv_device.c Tue Apr 27 18:38:56 2010
>
> +++ core/winverbs/kernel/wv_device.c Mon Jun 28 10:52:13 2010
>
> @@ -178,8 +178,8 @@
>
> attr = NULL;
>
> goto out;
>
> }
>
> -
>
> - attr = ExAllocatePoolWithTag(PagedPool, size, 'acvw');
>
> +
>
> + attr = ExAllocatePoolWithTagSafeEx(PagedPool, size, 'acvw');
>
> if (attr == NULL) {
>
> goto out;
>
> }
>
> @@ -210,7 +210,7 @@
>
> pDevice->PortCount = attr->num_ports;
>
> ExFreePoolWithTag(attr, 'acvw');
>
>
>
> - pDevice->pPorts = ExAllocatePoolWithTag(NonPagedPool,
> sizeof(WV_PORT) *
>
> + pDevice->pPorts = ExAllocatePoolWithTagSafeEx(NonPagedPool,
> sizeof(WV_PORT) *
>
>
> pDevice->PortCount, 'cpvw');
>
> if (pDevice->pPorts == NULL) {
>
> return STATUS_NO_MEMORY;
>
> --- inc/complib/cl_memory.h Tue Apr 27 18:44:09 2010
>
> +++ inc/complib/cl_memory.h Mon Jun 28 10:52:18 2010
>
> @@ -27,7 +27,7 @@
>
> * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
>
> * SOFTWARE.
>
> *
>
> - * $Id: cl_memory.h 10 2005-05-24 00:33:03Z ftillier $
>
> + * $Id: cl_memory.h 4991 2009-10-22 16:21:47Z xalex $
>
> */
>
>
>
>
>
> @@ -919,6 +919,21 @@
>
> /*
>
> * Define allocation macro.
>
> */
>
> +
>
> +/* From WinDDK: "Avoid calling ExAllocatePoolWithTag with memory
> size == 0.
>
> + Doing so will result in pool header wastage"
>
> + Verifier with low mem simulation will crash with memory size
> == 0
>
> +*/
>
> +#define ExAllocatePoolWithTagSafeEx( pageable, size, tag ) \
>
> + (size == 0 ? NULL :
> ExAllocatePoolWithTag(pageable, size, tag))
>
> +
>
> +#define ExAllocatePoolWithTagSafeExNonPaged(size, tag ) \
>
> + (size == 0 ? NULL :
> ExAllocatePoolWithTag(NonPagedPool, size, tag ))
>
> +
>
> +#define ExAllocatePoolWithTagSafeExPaged(size, tag ) \
>
> + (size == 0 ?
> NULL : ExAllocatePoolWithTag(PagedPool, size, tag ))
>
> +
>
> +
>
> #if defined( CL_TRACK_MEM )
>
>
>
> #define cl_malloc( a ) \
More information about the ofw
mailing list