Messages in this thread | ![/](/images/icornerl.gif) | | Date | Mon, 13 Oct 2003 09:56:52 +0100 | From | Christoph Hellwig <> | Subject | Re: [PATCH] Altix I/O code cleanup |
| |
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@vger.kernel.org
> > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| ![\](/images/icornerr.gif) |