Hi, > -----Original Message----- > From: Oliver Neukum [mailto:oliver@xxxxxxxxxx] > Sent: Friday, February 26, 2010 4:33 PM > To: Gupta, Ajay Kumar > Cc: linux-usb@xxxxxxxxxxxxxxx; linux-omap@xxxxxxxxxxxxxxx; > stern@xxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH] musb: Add workqueue for URB giveback > > Am Freitag, 26. Februar 2010 11:39:06 schrieb Ajay Kumar Gupta: > > +struct queue *create(void) > > +{ > > + struct queue *new; > > + new = kmalloc(sizeof(struct queue), GFP_ATOMIC); > > + if (!new) > > + return NULL; > > + new->next = NULL; > > + return new; > > +} > > +void push_queue(struct musb *musb, struct urb *urb) > > +{ > > + struct queue *new, *temp; > > + > > + new = create(); > > + new->urb = urb; > > And you happily follow the NULL pointer in the error case. I will put the NULL check here and also during push_queue(). In case Of memory allocation failure I can giveback the URB right away. > > > + > > + temp = musb->qhead; > > + > > + spin_lock(&musb->qlock); > > + while (temp->next != NULL) > > + temp = temp->next; > > + temp->next = new; > > + spin_unlock(&musb->qlock); > > +} > > A design allocating memory in giveback is problematic. At least you need > to handle a failure to allocate memory. But you'd better handle this by > putting the list into the private part of the URB. This would require a change in urb structure which would face more resistance as it will only be used by MUSB HCD. Thanks, Ajay > > Regards > Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
