lkml.org 
[lkml]   [2004]   [Dec]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] ATA over Ethernet driver for 2.6.9
From
Date

> +#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/

\
 
 \ /
  Last update: 2005-03-22 14:08    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site