Hi
In bio_endio() there is the follow line:-
bio->bi_sector += (bytes_done >> 9);
and there is a similiar line assuming a 512byte sector in
__bio_add_page() .
Is this a bug as the the actual block size should be available
from bio->bi_bdev->bd_block_size surely - or is some clever code
where all block devices have to translate back to 512 byte sectors
for bio s.
TTFN
--
Roger. Home| http://www.sandman.uklinux.net/
Master of Peng Shui. (Ancient oriental art of Penguin Arranging)
Work|Independent Sys Consultant | http://www.computer-surgery.co.uk/
So what are the eigenvalues and eigenvectors of 'The Matrix'? --anon
On Thu, 28 Sep 2006 19:22:38 +0100 Roger Gammans wrote:
> Hi
>
> In bio_endio() there is the follow line:-
>
> bio->bi_sector += (bytes_done >> 9);
>
> and there is a similiar line assuming a 512byte sector in
> __bio_add_page() .
>
> Is this a bug as the the actual block size should be available
> from bio->bi_bdev->bd_block_size surely - or is some clever code
> where all block devices have to translate back to 512 byte sectors
> for bio s.
Does this answer it for you?
http://marc.theaimsgroup.com/?l=linux-kernel&m=115542872706154&w=2
---
~Randy
On Fri, Sep 29, 2006 at 11:38:14AM -0700, Randy Dunlap wrote:
> > from bio->bi_bdev->bd_block_size surely - or is some clever code
> > where all block devices have to translate back to 512 byte sectors
> Does this answer it for you?
Ahh, Yup.
Is this documented anywhere ? , I'd suggest a <para> in kernel-api.tmpl
but I'm not sure this is the right place.
TTFN
--
Roger. Home| http://www.sandman.uklinux.net/
Master of Peng Shui. (Ancient oriental art of Penguin Arranging)
Work|Independent Sys Consultant | http://www.computer-surgery.co.uk/
So what are the eigenvalues and eigenvectors of 'The Matrix'? --anon
On Thu, 28 Sep 2006 19:58:21 +0100 [email protected] wrote:
> On Fri, Sep 29, 2006 at 11:38:14AM -0700, Randy Dunlap wrote:
> > > from bio->bi_bdev->bd_block_size surely - or is some clever code
> > > where all block devices have to translate back to 512 byte sectors
>
> > Does this answer it for you?
>
> Ahh, Yup.
>
> Is this documented anywhere ? , I'd suggest a <para> in kernel-api.tmpl
> but I'm not sure this is the right place.
I don't know if or where it is documented.
You can submit a patch for it.
If you don't, I'll put it in my todo queue.
---
~Randy
On Fri, Sep 29, 2006 at 12:11:57PM -0700, Randy Dunlap wrote:
> I don't know if or where it is documented.
Well, I've spend a good chunk of time reading round this part of
the kernel's interfaces without spotting it so another note somewhere
can't help.
> You can submit a patch for it.
> If you don't, I'll put it in my todo queue.
If I find an approriate place to put such a note I'll add it and
submit a patch, but I'm not sure where to put it , atm.
Any suggestions?
TTFN
--
Roger. Home| http://www.sandman.uklinux.net/
Master of Peng Shui. (Ancient oriental art of Penguin Arranging)
Work|Independent Sys Consultant | http://www.computer-surgery.co.uk/
So what are the eigenvalues and eigenvectors of 'The Matrix'? --anon
On Thu, 28 Sep 2006 20:19:46 +0100 Roger Gammans wrote:
> On Fri, Sep 29, 2006 at 12:11:57PM -0700, Randy Dunlap wrote:
> > I don't know if or where it is documented.
>
> Well, I've spend a good chunk of time reading round this part of
> the kernel's interfaces without spotting it so another note somewhere
> can't help.
>
> > You can submit a patch for it.
> > If you don't, I'll put it in my todo queue.
>
> If I find an approriate place to put such a note I'll add it and
> submit a patch, but I'm not sure where to put it , atm.
>
> Any suggestions?
Hm, I looked thru fs/bio.c and block/*.c and Documentation/Docbook/*.tmpl.
The best place that I see to put it right now is in
include/linux/bio.h, struct bio, field: bi_sector.
What do you think of that?
---
~Randy
GPL v0: http://www.glacierparkinc.com/GlacierParkLodge.htm
On Fri, Sep 29, 2006 at 12:37:37PM -0700, Randy Dunlap wrote:
> Hm, I looked thru fs/bio.c and block/*.c and Documentation/Docbook/*.tmpl.
> The best place that I see to put it right now is in
> include/linux/bio.h, struct bio, field: bi_sector.
>
> What do you think of that?
Well, ... Um. I can't think of anywhere better either, so how about
this:-
Signed-Off-By: Roger Gammans <[email protected]>
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 76bdaea..77a8e6b 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -70,7 +70,8 @@ typedef void (bio_destructor_t) (struct
* stacking drivers)
*/
struct bio {
- sector_t bi_sector;
+ sector_t bi_sector; /* device address in 512 byte
+ sectors */
struct bio *bi_next; /* request queue link */
struct block_device *bi_bdev;
unsigned long bi_flags; /* status, command, etc
*/
--
Roger. Home| http://www.sandman.uklinux.net/
Master of Peng Shui. (Ancient oriental art of Penguin Arranging)
Work|Independent Sys Consultant | http://www.computer-surgery.co.uk/
So what are the eigenvalues and eigenvectors of 'The Matrix'? --anon
On Thu, 28 Sep 2006 20:56:27 +0100 Roger Gammans wrote:
> On Fri, Sep 29, 2006 at 12:37:37PM -0700, Randy Dunlap wrote:
> > Hm, I looked thru fs/bio.c and block/*.c and Documentation/Docbook/*.tmpl.
> > The best place that I see to put it right now is in
> > include/linux/bio.h, struct bio, field: bi_sector.
> >
> > What do you think of that?
>
> Well, ... Um. I can't think of anywhere better either, so how about
> this:-
>
> Signed-Off-By: Roger Gammans <[email protected]>
Looks OK to me. I would probably go for something a little
stronger, though, like:
sector_t bi_sector; /* block layer sector
* addresses are always in
* 512-byte units in Linux */
Jens, is something like this (above or below) OK with you?
> diff --git a/include/linux/bio.h b/include/linux/bio.h
> index 76bdaea..77a8e6b 100644
> --- a/include/linux/bio.h
> +++ b/include/linux/bio.h
> @@ -70,7 +70,8 @@ typedef void (bio_destructor_t) (struct
> * stacking drivers)
> */
> struct bio {
> - sector_t bi_sector;
> + sector_t bi_sector; /* device address in 512 byte
> + sectors */
> struct bio *bi_next; /* request queue link */
> struct block_device *bi_bdev;
> unsigned long bi_flags; /* status, command, etc
> */
---
~Randy
> sector_t bi_sector; /* block layer sector
> * addresses are always in
> * 512-byte units in Linux */
How about adding kerneldoc for sector_t itself?
- z
On Fri, 29 Sep 2006 13:22:34 -0700 Zach Brown wrote:
>
> > sector_t bi_sector; /* block layer sector
> > * addresses are always in
> > * 512-byte units in Linux */
>
> How about adding kerneldoc for sector_t itself?
Good idea, but afaik it would have to be added for the entire
struct, not just one field.
---
~Randy
On Fri, Sep 29, 2006 at 01:32:05PM -0700, Randy Dunlap wrote:
> > How about adding kerneldoc for sector_t itself?
>
> Good idea, but afaik it would have to be added for the entire
> struct, not just one field.
sector_t 's a simple typedef from unsigned long or u64 depending on
config rather than a struct - will kerneldoc still pick up the comments
on theese?
Assuming it will I suggest the following. I've kept my shorter text in
the bi_sector field as it is now more fully explained with sector_t.
Signed-Off-By: Roger Gammans <[email protected]>
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 76bdaea..77a8e6b 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -70,7 +70,8 @@ typedef void (bio_destructor_t) (struct
* stacking drivers)
*/
struct bio {
- sector_t bi_sector;
+ sector_t bi_sector; /* device address in 512 byte
+ sectors */
struct bio *bi_next; /* request queue link */
struct block_device *bi_bdev;
unsigned long bi_flags; /* status, command, etc
*/
diff --git a/include/linux/types.h b/include/linux/types.h
index 3f23566..0ddfa1a 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -127,8 +127,12 @@ #endif
/* this is a special 64bit data type that is 8-byte aligned */
#define aligned_u64 unsigned long long __attribute__((aligned(8)))
-/*
+/**
* The type used for indexing onto a disc or disc partition.
+ *
+ * Linux always considers sectors to be 512 bytes long independently
+ * of the devices real block size.
+ *
* If required, asm/types.h can override it and define
* HAVE_SECTOR_T
*/
--
Roger. Home| http://www.sandman.uklinux.net/
Master of Peng Shui. (Ancient oriental art of Penguin Arranging)
Work|Independent Sys Consultant | http://www.computer-surgery.co.uk/
So what are the eigenvalues and eigenvectors of 'The Matrix'? --anon
On Fri, Sep 29 2006, Roger Gammans wrote:
> On Fri, Sep 29, 2006 at 01:32:05PM -0700, Randy Dunlap wrote:
> > > How about adding kerneldoc for sector_t itself?
> >
> > Good idea, but afaik it would have to be added for the entire
> > struct, not just one field.
>
> sector_t 's a simple typedef from unsigned long or u64 depending on
> config rather than a struct - will kerneldoc still pick up the comments
> on theese?
>
> Assuming it will I suggest the following. I've kept my shorter text in
> the bi_sector field as it is now more fully explained with sector_t.
>
>
> Signed-Off-By: Roger Gammans <[email protected]>
>
> diff --git a/include/linux/bio.h b/include/linux/bio.h
> index 76bdaea..77a8e6b 100644
> --- a/include/linux/bio.h
> +++ b/include/linux/bio.h
> @@ -70,7 +70,8 @@ typedef void (bio_destructor_t) (struct
> * stacking drivers)
> */
> struct bio {
> - sector_t bi_sector;
> + sector_t bi_sector; /* device address in 512 byte
> + sectors */
> struct bio *bi_next; /* request queue link */
> struct block_device *bi_bdev;
> unsigned long bi_flags; /* status, command, etc
> */
> diff --git a/include/linux/types.h b/include/linux/types.h
> index 3f23566..0ddfa1a 100644
> --- a/include/linux/types.h
> +++ b/include/linux/types.h
> @@ -127,8 +127,12 @@ #endif
> /* this is a special 64bit data type that is 8-byte aligned */
> #define aligned_u64 unsigned long long __attribute__((aligned(8)))
>
> -/*
> +/**
> * The type used for indexing onto a disc or disc partition.
> + *
> + * Linux always considers sectors to be 512 bytes long independently
> + * of the devices real block size.
> + *
> * If required, asm/types.h can override it and define
> * HAVE_SECTOR_T
> */
Looks fine to me, I'll add it (although I tend to prefer disk :-)
--
Jens Axboe