Re: [PATCH] Altix I/O code cleanup

[Christoph Hellwig]

On Fri, Oct 10, 2003 at 04:49:57PM -0500, Patrick Gefre wrote:
> This is my first patch for this - more to come ....

It would be nice to give those credits who submitted those patches.
And life would be a lot simpler if you wouldn't submit my individual
patches instead of putting them into a big one - this gives useful
entries in the revision history and allows for easier binary search
if something goes wrong.

p.s. the right list for this would probably be linux-ia64@xxxxxxxxxxxxxxx

>  
>  void *
> -snia_kmem_zalloc(size_t size, int flag)
> +snia_kmem_zalloc(size_t size)
>  {
>          void *ptr = kmalloc(size, GFP_KERNEL);

passing a gfp_mask down here would make sense..

> - * the alloc/free_node routines do a simple kmalloc for now ..
> - */
> -void *
> -snia_kmem_alloc_node(register size_t size, register int flags, cnodeid_t node)
> -{
> -	/* someday will Allocate on node 'node' */
> -	return(kmalloc(size, GFP_KERNEL));
> -}
> -
> -void *
> -snia_kmem_zalloc_node(register size_t size, register int flags, cnodeid_t node)
> -{
> -	void *ptr = kmalloc(size, GFP_KERNEL);
> -	if ( ptr )
> -		BZERO(ptr, size);
> -        return(ptr);
> -}

Why do you remove the per-nod wrappers?  Unlike the other these actually
had some use as preparation for a node-aware kmalloc..

>  	int rc;
> -	extern void * snia_kmem_zalloc(size_t size, int flag);
> +	extern void * snia_kmem_zalloc(size_t size);

This is in a header, isn't it?

>  
> -	xvolinfo = snia_kmem_zalloc(sizeof(struct xswitch_vol_s), GFP_KERNEL);
> +	xvolinfo = snia_kmem_zalloc(sizeof(struct xswitch_vol_s));

You still need to handle a NULL return here.

>  
> -	intr_hdl = snia_kmem_alloc_node(sizeof(struct hub_intr_s), KM_NOSLEEP, cnode);
> +	intr_hdl = kmalloc(sizeof(struct hub_intr_s), GFP_KERNEL);
>  	ASSERT_ALWAYS(intr_hdl);

NULL return not handled again (and the assert is totally useless)

> -#define NEWAf(ptr,n,f)	(ptr = snia_kmem_zalloc((n)*sizeof (*(ptr)), (f&PCIIO_NOSLEEP)?KM_NOSLEEP:KM_SLEEP))
> -#define NEWA(ptr,n)	(ptr = snia_kmem_zalloc((n)*sizeof (*(ptr)), KM_SLEEP))
> +#define NEWAf(ptr,n,f)	(ptr = snia_kmem_zalloc((n)*sizeof (*(ptr))))
> +#define NEWA(ptr,n)	(ptr = snia_kmem_zalloc((n)*sizeof (*(ptr))))
>  #define DELA(ptr,n)	(kfree(ptr))

What about killing this stupid wrappers while you're at it?
 Also PCIIO_NOSLEEP is never set.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/