Messages in this thread |  | | | Subject | Re: [PATCH] ATA over Ethernet driver for 2.6.9 | | From | Arjan van de Ven <> | | Date | Mon, 06 Dec 2004 11:14:06 -0500 |
| |
> +#include "u.h"
can you give this header a more human friendly name ?
> +#include "if_aoe.h" > + > +#define VER 2 > +#define nil NULL
please just use NULL
> +#define nelem(A) (sizeof (A) / sizeof (A)[0]) > +#define AOE_MAJOR 152 > +#define ROOT_PATH "/dev/etherd/"
ehhhh device drivers really should be agnostic to device node paths! > +typedef struct Bufq Bufq; > +struct Bufq { > + Buf *head, *tail; > +};
is there a good reason to not use list.h lists ??
> +struct Aoedev { > + Aoedev *next; > + uchar addr[6]; /* remote mac addr */ > + ushort flags; > + ulong sysminor; > + ulong aoemajor; > + ulong aoeminor;
sounds like the wrong type, why not use dev_t ?
> + ulong nopen; /* user count */
why do you need this ?
> +static int > +aoeblk_release(struct inode *inode, struct file *filp) > +{ > + Aoedev *d; > + ulong flags; > + > + d = (Aoedev *) inode->i_bdev->bd_disk->private_data; > + > + spin_lock_irqsave(&d->lock, flags); > + if (--d->nopen == 0)
eh why not just a ->release function instead that uses the blocklayer refcounting instead of doing your own ?
> +int > +aoeblk_make_request(request_queue_t *q, struct bio *bio) > +{ > + Aoedev *d; > + Buf *buf; > + struct sk_buff *sl; > + ulong flags; > + > + blk_queue_bounce(q, &bio); > + > + buf = kallocz(sizeof *buf, GFP_KERNEL);
this is deadlocky; you HAVE to use a mempool for allocations here!
- 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/
|  |