Re: block/bsg.c

From: Jens Axboe <jens.axboe@xxxxxxxxxx>
Subject: Re: block/bsg.c
Date: Tue, 17 Jul 2007 08:59:40 +0200

On Tue, Jul 17 2007, FUJITA Tomonori wrote:
From: Jens Axboe <jens.axboe@xxxxxxxxxx>
Subject: Re: block/bsg.c
Date: Tue, 17 Jul 2007 08:38:11 +0200

On Mon, Jul 16 2007, Andrew Morton wrote:

A belated review (I've never seen this before and there it is in mainline)

static char bsg_version[] = "block layer sg (bsg) 0.4";

`const' would be better. That moves it into a write-protected memory section.

Agree

#define list_entry_bc(entry) list_entry((entry), struct bsg_command, list)

This makes the code easier to write but harder to read. We should optimise
for readers. Please open-code this at callsites.

Or at least convert it into a (commented) (possibly inlined) C function.

list_entry_to_bc(), then? The main objective is to save on typing, and
(just as important) make sure we don't bump over the 80 chars per line.

/*
* just for testing
*/
#define BSG_MAJOR (240)

What's this doing in mainline? 240 is a "reserved for local use" major.
This will cause collisions. This code should be using dynamic major
assignment.

Yeah, that's a big error on my part. Will get that fixed up right away.

I've been testing the patchset to fix the issues that Andrew pointed
out. I can send it soon.

Jens, have you already fixed some?

I already pushed some of the changes locally, just the idr change is
missing. See the #bsg branch:

http://git.kernel.dk/?p=linux-2.6-block.git;a=shortlog;h=bsg

I see.


I'll ask for that to get pulled, and can I then assume that you will
look after bsg from now on?

I'm happy to do that.
-
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/

Relevant Pages

  • Re: block/bsg.c
    ... This makes the code easier to write but harder to read. ... This code should be using dynamic major ... I'll pull them into the bsg branch. ... Jens Axboe ...
    (Linux-Kernel)
  • Re: block/bsg.c
    ... A belated review (I've never seen this before and there it is in mainline) ... This makes the code easier to write but harder to read. ... This code should be using dynamic major ... See the #bsg branch: ...
    (Linux-Kernel)
  • Re: block/bsg.c
    ... A belated review (I've never seen this before and there it is in mainline) ... This code should be using dynamic major ... See the #bsg branch: ... Jens Axboe ...
    (Linux-Kernel)
  • Re: block/bsg.c
    ... A belated review (I've never seen this before and there it is in mainline) ... This makes the code easier to write but harder to read. ... This code should be using dynamic major ... Yeah, I'll do and send patches later if necessary. ...
    (Linux-Kernel)
  • Re: block/bsg.c
    ... A belated review (I've never seen this before and there it is in mainline) ... This code should be using dynamic major ... See the #bsg branch: ... Jens Axboe ...
    (Linux-Kernel)