[linux-lvm] LVM2 modifies the buffer_head struct?

[linux-lvm] LVM2 modifies the buffer_head struct?

[Tom Walcott]

Hello,

Browsing the patch submitted for 2.4 inclusion, I noticed that LVM2 modifies 
the buffer_head struct. Why does LVM2 require the addition of it's own 
private field in the buffer_head? It seems that it should be able to use the 
existing b_private field.

How does that extra field affect performance relative to the cache? Won't 
any negative effects be seen by everything that uses buffer_heads? Also, as 
I understand the slab code and hardware cache alignment, won't the addition 
of the new field cause the each buffer_head to consume 128 bytes instead of 
96?

Just curious to know why this field was added, when the buffer_head already 
had a field for this purpose.

-Tom



_________________________________________________________________
Chat with friends online, try MSN Messenger: http://messenger.msn.com

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Joe Thornber]

Tom,

On Tue, Jul 02, 2002 at 09:40:56AM -0400, Tom Walcott wrote:
> Hello,
> 
> Browsing the patch submitted for 2.4 inclusion, I noticed that LVM2 
> modifies the buffer_head struct. Why does LVM2 require the addition of it's 
> own private field in the buffer_head? It seems that it should be able to 
> use the existing b_private field.

This is a horrible hack to get around the fact that ext3 uses the
b_private field for its own purposes after the buffer_head has been
handed to the block layer (it doesn't just use b_private when in the
b_end_io function).  Is this acceptable behaviour ?  Other filesystems
do not have similar problems as far as I know.

device-mapper uses the b_private field to 'hook' the buffer_heads so
it can keep track of in flight ios (essential for implementing
suspend/resume correctly).  See dm.c:dec_pending()

As a simple fix I added the b_bdev_private field with the intention
that this is the private field for use by the block layer, and
b_private then effectively becomes b_fs_private.  I wont pretend to be
remotely happy with it.

I would love any suggestions of how else I can implement this, it
seems unreasonable to penalise everybody - not just those using ext3.

> How does that extra field affect performance relative to the cache? Won't 
> any negative effects be seen by everything that uses buffer_heads? Also, as 
> I understand the slab code and hardware cache alignment, won't the addition 
> of the new field cause the each buffer_head to consume 128 bytes instead of 
> 96?

Obviously there will be some negative effect, though don't have a feel
for how significant it would be.  I'm not even sure of what the best
way to measure this would be; if people can point me towards the most
suitable benchmark I'll be happy to do some testing for the list.

- Joe

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Joe Thornber]

Tom,

On Tue, Jul 02, 2002 at 09:40:56AM -0400, Tom Walcott wrote:
> Hello,
> 
> Browsing the patch submitted for 2.4 inclusion, I noticed that LVM2 
> modifies the buffer_head struct. Why does LVM2 require the addition of it's 
> own private field in the buffer_head? It seems that it should be able to 
> use the existing b_private field.

This is a horrible hack to get around the fact that ext3 uses the
b_private field for its own purposes after the buffer_head has been
handed to the block layer (it doesn't just use b_private when in the
b_end_io function).  Is this acceptable behaviour ?  Other filesystems
do not have similar problems as far as I know.

device-mapper uses the b_private field to 'hook' the buffer_heads so
it can keep track of in flight ios (essential for implementing
suspend/resume correctly).  See dm.c:dec_pending()

As a simple fix I added the b_bdev_private field with the intention
that this is the private field for use by the block layer, and
b_private then effectively becomes b_fs_private.  I wont pretend to be
remotely happy with it.

I would love any suggestions of how else I can implement this, it
seems unreasonable to penalise everybody - not just those using ext3.

> How does that extra field affect performance relative to the cache? Won't 
> any negative effects be seen by everything that uses buffer_heads? Also, as 
> I understand the slab code and hardware cache alignment, won't the addition 
> of the new field cause the each buffer_head to consume 128 bytes instead of 
> 96?

Obviously there will be some negative effect, though don't have a feel
for how significant it would be.  I'm not even sure of what the best
way to measure this would be; if people can point me towards the most
suitable benchmark I'll be happy to do some testing for the list.

- Joe

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Jens Axboe]

On Tue, Jul 02 2002, Joe Thornber wrote:
> Tom,
> 
> On Tue, Jul 02, 2002 at 09:40:56AM -0400, Tom Walcott wrote:
> > Hello,
> > 
> > Browsing the patch submitted for 2.4 inclusion, I noticed that LVM2 
> > modifies the buffer_head struct. Why does LVM2 require the addition of it's 
> > own private field in the buffer_head? It seems that it should be able to 
> > use the existing b_private field.
> 
> This is a horrible hack to get around the fact that ext3 uses the
> b_private field for its own purposes after the buffer_head has been
> handed to the block layer (it doesn't just use b_private when in the
> b_end_io function).  Is this acceptable behaviour ?  Other filesystems
> do not have similar problems as far as I know.
> 
> device-mapper uses the b_private field to 'hook' the buffer_heads so
> it can keep track of in flight ios (essential for implementing
> suspend/resume correctly).  See dm.c:dec_pending()

Your driver is required to properly stack b_private uses, however if
ext3 (well jbd really) over writes b_private after bh i/o submission I
would say that it is broken. That breaks more than just device mapper,
that will break any stacked driver (such as loop, for instance).

-- 
Jens Axboe

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Jens Axboe]

On Tue, Jul 02 2002, Joe Thornber wrote:
> Tom,
> 
> On Tue, Jul 02, 2002 at 09:40:56AM -0400, Tom Walcott wrote:
> > Hello,
> > 
> > Browsing the patch submitted for 2.4 inclusion, I noticed that LVM2 
> > modifies the buffer_head struct. Why does LVM2 require the addition of it's 
> > own private field in the buffer_head? It seems that it should be able to 
> > use the existing b_private field.
> 
> This is a horrible hack to get around the fact that ext3 uses the
> b_private field for its own purposes after the buffer_head has been
> handed to the block layer (it doesn't just use b_private when in the
> b_end_io function).  Is this acceptable behaviour ?  Other filesystems
> do not have similar problems as far as I know.
> 
> device-mapper uses the b_private field to 'hook' the buffer_heads so
> it can keep track of in flight ios (essential for implementing
> suspend/resume correctly).  See dm.c:dec_pending()

Your driver is required to properly stack b_private uses, however if
ext3 (well jbd really) over writes b_private after bh i/o submission I
would say that it is broken. That breaks more than just device mapper,
that will break any stacked driver (such as loop, for instance).

-- 
Jens Axboe

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Andrew Morton]

Jens Axboe wrote:
> 
> On Tue, Jul 02 2002, Joe Thornber wrote:
> > Tom,
> >
> > On Tue, Jul 02, 2002 at 09:40:56AM -0400, Tom Walcott wrote:
> > > Hello,
> > >
> > > Browsing the patch submitted for 2.4 inclusion, I noticed that LVM2
> > > modifies the buffer_head struct. Why does LVM2 require the addition of it's
> > > own private field in the buffer_head? It seems that it should be able to
> > > use the existing b_private field.
> >
> > This is a horrible hack to get around the fact that ext3 uses the
> > b_private field for its own purposes after the buffer_head has been
> > handed to the block layer (it doesn't just use b_private when in the
> > b_end_io function).  Is this acceptable behaviour ?  Other filesystems
> > do not have similar problems as far as I know.
> >
> > device-mapper uses the b_private field to 'hook' the buffer_heads so
> > it can keep track of in flight ios (essential for implementing
> > suspend/resume correctly).  See dm.c:dec_pending()
> 
> Your driver is required to properly stack b_private uses, however if
> ext3 (well jbd really) over writes b_private after bh i/o submission I
> would say that it is broken. That breaks more than just device mapper,
> that will break any stacked driver (such as loop, for instance).

It requires that b_private be stable across the lifetime of the buffer.

hmm.

-

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Andrew Morton]

Jens Axboe wrote:
> 
> On Tue, Jul 02 2002, Joe Thornber wrote:
> > Tom,
> >
> > On Tue, Jul 02, 2002 at 09:40:56AM -0400, Tom Walcott wrote:
> > > Hello,
> > >
> > > Browsing the patch submitted for 2.4 inclusion, I noticed that LVM2
> > > modifies the buffer_head struct. Why does LVM2 require the addition of it's
> > > own private field in the buffer_head? It seems that it should be able to
> > > use the existing b_private field.
> >
> > This is a horrible hack to get around the fact that ext3 uses the
> > b_private field for its own purposes after the buffer_head has been
> > handed to the block layer (it doesn't just use b_private when in the
> > b_end_io function).  Is this acceptable behaviour ?  Other filesystems
> > do not have similar problems as far as I know.
> >
> > device-mapper uses the b_private field to 'hook' the buffer_heads so
> > it can keep track of in flight ios (essential for implementing
> > suspend/resume correctly).  See dm.c:dec_pending()
> 
> Your driver is required to properly stack b_private uses, however if
> ext3 (well jbd really) over writes b_private after bh i/o submission I
> would say that it is broken. That breaks more than just device mapper,
> that will break any stacked driver (such as loop, for instance).

It requires that b_private be stable across the lifetime of the buffer.

hmm.

-

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Joe Thornber]

On Wed, Jul 03, 2002 at 12:08:38PM +0200, Jens Axboe wrote:
> On Tue, Jul 02 2002, Joe Thornber wrote:
> > Tom,
> > 
> > On Tue, Jul 02, 2002 at 09:40:56AM -0400, Tom Walcott wrote:
> > > Hello,
> > > 
> > > Browsing the patch submitted for 2.4 inclusion, I noticed that LVM2 
> > > modifies the buffer_head struct. Why does LVM2 require the addition of it's 
> > > own private field in the buffer_head? It seems that it should be able to 
> > > use the existing b_private field.
> > 
> > This is a horrible hack to get around the fact that ext3 uses the
> > b_private field for its own purposes after the buffer_head has been
> > handed to the block layer (it doesn't just use b_private when in the
> > b_end_io function).  Is this acceptable behaviour ?  Other filesystems
> > do not have similar problems as far as I know.
> > 
> > device-mapper uses the b_private field to 'hook' the buffer_heads so
> > it can keep track of in flight ios (essential for implementing
> > suspend/resume correctly).  See dm.c:dec_pending()
> 
> Your driver is required to properly stack b_private uses, however if
> ext3 (well jbd really) over writes b_private after bh i/o submission I
> would say that it is broken.

AFAIK ext3 doesn't overwrite b_private after submission, but does
expect the value not to change (ie. no stacking to be taking place).

- Joe

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Joe Thornber]

On Wed, Jul 03, 2002 at 12:08:38PM +0200, Jens Axboe wrote:
> On Tue, Jul 02 2002, Joe Thornber wrote:
> > Tom,
> > 
> > On Tue, Jul 02, 2002 at 09:40:56AM -0400, Tom Walcott wrote:
> > > Hello,
> > > 
> > > Browsing the patch submitted for 2.4 inclusion, I noticed that LVM2 
> > > modifies the buffer_head struct. Why does LVM2 require the addition of it's 
> > > own private field in the buffer_head? It seems that it should be able to 
> > > use the existing b_private field.
> > 
> > This is a horrible hack to get around the fact that ext3 uses the
> > b_private field for its own purposes after the buffer_head has been
> > handed to the block layer (it doesn't just use b_private when in the
> > b_end_io function).  Is this acceptable behaviour ?  Other filesystems
> > do not have similar problems as far as I know.
> > 
> > device-mapper uses the b_private field to 'hook' the buffer_heads so
> > it can keep track of in flight ios (essential for implementing
> > suspend/resume correctly).  See dm.c:dec_pending()
> 
> Your driver is required to properly stack b_private uses, however if
> ext3 (well jbd really) over writes b_private after bh i/o submission I
> would say that it is broken.

AFAIK ext3 doesn't overwrite b_private after submission, but does
expect the value not to change (ie. no stacking to be taking place).

- Joe

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Jens Axboe]

On Wed, Jul 03 2002, Joe Thornber wrote:
> On Wed, Jul 03, 2002 at 12:08:38PM +0200, Jens Axboe wrote:
> > On Tue, Jul 02 2002, Joe Thornber wrote:
> > > Tom,
> > > 
> > > On Tue, Jul 02, 2002 at 09:40:56AM -0400, Tom Walcott wrote:
> > > > Hello,
> > > > 
> > > > Browsing the patch submitted for 2.4 inclusion, I noticed that LVM2 
> > > > modifies the buffer_head struct. Why does LVM2 require the addition of it's 
> > > > own private field in the buffer_head? It seems that it should be able to 
> > > > use the existing b_private field.
> > > 
> > > This is a horrible hack to get around the fact that ext3 uses the
> > > b_private field for its own purposes after the buffer_head has been
> > > handed to the block layer (it doesn't just use b_private when in the
> > > b_end_io function).  Is this acceptable behaviour ?  Other filesystems
> > > do not have similar problems as far as I know.
> > > 
> > > device-mapper uses the b_private field to 'hook' the buffer_heads so
> > > it can keep track of in flight ios (essential for implementing
> > > suspend/resume correctly).  See dm.c:dec_pending()
> > 
> > Your driver is required to properly stack b_private uses, however if
> > ext3 (well jbd really) over writes b_private after bh i/o submission I
> > would say that it is broken.
> 
> AFAIK ext3 doesn't overwrite b_private after submission, but does
> expect the value not to change (ie. no stacking to be taking place).

Now we are in a grey area. The 'usual' stacked drivers work like this:

some fs path
	submit_bh(bh_orig);

...

stacked driver make_request_fn:
	bh_new = alloc_bh
	bh_new->b_private = bh_orig;
	...
	submit_bh(bh_new);

if you are just modifying b_private, how exactly is your stacking
working? ie what about lvm2 on lvm2?

-- 
Jens Axboe

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Jens Axboe]

On Wed, Jul 03 2002, Joe Thornber wrote:
> On Wed, Jul 03, 2002 at 12:08:38PM +0200, Jens Axboe wrote:
> > On Tue, Jul 02 2002, Joe Thornber wrote:
> > > Tom,
> > > 
> > > On Tue, Jul 02, 2002 at 09:40:56AM -0400, Tom Walcott wrote:
> > > > Hello,
> > > > 
> > > > Browsing the patch submitted for 2.4 inclusion, I noticed that LVM2 
> > > > modifies the buffer_head struct. Why does LVM2 require the addition of it's 
> > > > own private field in the buffer_head? It seems that it should be able to 
> > > > use the existing b_private field.
> > > 
> > > This is a horrible hack to get around the fact that ext3 uses the
> > > b_private field for its own purposes after the buffer_head has been
> > > handed to the block layer (it doesn't just use b_private when in the
> > > b_end_io function).  Is this acceptable behaviour ?  Other filesystems
> > > do not have similar problems as far as I know.
> > > 
> > > device-mapper uses the b_private field to 'hook' the buffer_heads so
> > > it can keep track of in flight ios (essential for implementing
> > > suspend/resume correctly).  See dm.c:dec_pending()
> > 
> > Your driver is required to properly stack b_private uses, however if
> > ext3 (well jbd really) over writes b_private after bh i/o submission I
> > would say that it is broken.
> 
> AFAIK ext3 doesn't overwrite b_private after submission, but does
> expect the value not to change (ie. no stacking to be taking place).

Now we are in a grey area. The 'usual' stacked drivers work like this:

some fs path
	submit_bh(bh_orig);

...

stacked driver make_request_fn:
	bh_new = alloc_bh
	bh_new->b_private = bh_orig;
	...
	submit_bh(bh_new);

if you are just modifying b_private, how exactly is your stacking
working? ie what about lvm2 on lvm2?

-- 
Jens Axboe

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Neil Brown]

On Wednesday July 3, axboe@suse.de wrote:
> 
> Now we are in a grey area. The 'usual' stacked drivers work like this:
> 
> some fs path
> 	submit_bh(bh_orig);
> 
> ...
> 
> stacked driver make_request_fn:
> 	bh_new = alloc_bh
> 	bh_new->b_private = bh_orig;
> 	...
> 	submit_bh(bh_new);
> 
> if you are just modifying b_private, how exactly is your stacking
> working? ie what about lvm2 on lvm2?

I think this can work sanely and is something I have considered for
raid1-read and multipath in md.

struct privatebit {
  bio_end_io_t  *oldend;
  void          *oldprivate;
  ...other...stuff;
};

make_request(struct request_queue_t *q, struct buffer_head *bh, int rw)
{

 struct privatebit *pb = kmalloc(...);

  pb->oldend = bh->b_end_io;
  pb->oldprivate = bh->b_private;
  bh->b_private = pb;
  bh->b_end_io = my_end_handler;

  ..remap b_rdev, b_rsector ...

  generic_make_request(bh, rw);

}

Then my_end_handler have do some local cleanup,
re-instate oldend and oldprivate, and pass the bh back up.
For raid1/multipath it would arrange to resubmit the request if there
as an error.

This stacks nicely and allows for the extra bit to be alloced to be
minimal.

We just want ext3/jbd to make sure that it only calls bh2jh on
an unlocked buffer... is that easy?

Ofcourse this ceases to be an issue in 2.5 because the filesys uses 
pages or buffer_heads and the device driver uses bios.

NeilBrown

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Neil Brown]

On Wednesday July 3, axboe@suse.de wrote:
> 
> Now we are in a grey area. The 'usual' stacked drivers work like this:
> 
> some fs path
> 	submit_bh(bh_orig);
> 
> ...
> 
> stacked driver make_request_fn:
> 	bh_new = alloc_bh
> 	bh_new->b_private = bh_orig;
> 	...
> 	submit_bh(bh_new);
> 
> if you are just modifying b_private, how exactly is your stacking
> working? ie what about lvm2 on lvm2?

I think this can work sanely and is something I have considered for
raid1-read and multipath in md.

struct privatebit {
  bio_end_io_t  *oldend;
  void          *oldprivate;
  ...other...stuff;
};

make_request(struct request_queue_t *q, struct buffer_head *bh, int rw)
{

 struct privatebit *pb = kmalloc(...);

  pb->oldend = bh->b_end_io;
  pb->oldprivate = bh->b_private;
  bh->b_private = pb;
  bh->b_end_io = my_end_handler;

  ..remap b_rdev, b_rsector ...

  generic_make_request(bh, rw);

}

Then my_end_handler have do some local cleanup,
re-instate oldend and oldprivate, and pass the bh back up.
For raid1/multipath it would arrange to resubmit the request if there
as an error.

This stacks nicely and allows for the extra bit to be alloced to be
minimal.

We just want ext3/jbd to make sure that it only calls bh2jh on
an unlocked buffer... is that easy?

Ofcourse this ceases to be an issue in 2.5 because the filesys uses 
pages or buffer_heads and the device driver uses bios.

NeilBrown

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Andrew Morton]

Neil Brown wrote:
> 
> ...
> We just want ext3/jbd to make sure that it only calls bh2jh on
> an unlocked buffer... is that easy?

It's feasible, but a downright pita.

> Ofcourse this ceases to be an issue in 2.5 because the filesys uses
> pages or buffer_heads and the device driver uses bios.

Well, for 2.4 I'd be inclined to just add the extra field to 
struct buffer_head and be done with it.

Current sizeof(buffer_head) is 96 bytes, and making that 100 would
be a nuisance, but its quite easy to shrink the buffer_head a
bit (replace b_inode with BH_inode, say...).

-

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Andrew Morton]

Neil Brown wrote:
> 
> ...
> We just want ext3/jbd to make sure that it only calls bh2jh on
> an unlocked buffer... is that easy?

It's feasible, but a downright pita.

> Ofcourse this ceases to be an issue in 2.5 because the filesys uses
> pages or buffer_heads and the device driver uses bios.

Well, for 2.4 I'd be inclined to just add the extra field to 
struct buffer_head and be done with it.

Current sizeof(buffer_head) is 96 bytes, and making that 100 would
be a nuisance, but its quite easy to shrink the buffer_head a
bit (replace b_inode with BH_inode, say...).

-

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Joe Thornber]

On Thu, Jul 04, 2002 at 02:46:20PM +1000, Neil Brown wrote:
> I think this can work sanely and is something I have considered for
> raid1-read and multipath in md.
> 
> struct privatebit {
>   bio_end_io_t  *oldend;
>   void          *oldprivate;
>   ...other...stuff;
> };
> 
> make_request(struct request_queue_t *q, struct buffer_head *bh, int rw)
> {
> 
>  struct privatebit *pb = kmalloc(...);
> 
>   pb->oldend = bh->b_end_io;
>   pb->oldprivate = bh->b_private;
>   bh->b_private = pb;
>   bh->b_end_io = my_end_handler;
> 
>   ..remap b_rdev, b_rsector ...
> 
>   generic_make_request(bh, rw);
> 
> }
> 
> Then my_end_handler have do some local cleanup,
> re-instate oldend and oldprivate, and pass the bh back up.
> For raid1/multipath it would arrange to resubmit the request if there
> as an error.
> 
> This stacks nicely and allows for the extra bit to be alloced to be
> minimal.

This is exactly what I'm doing in device-mapper :)

> Ofcourse this ceases to be an issue in 2.5 because the filesys uses 
> pages or buffer_heads and the device driver uses bios.

y, 2.5 is fine.

- Joe

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Joe Thornber]

On Thu, Jul 04, 2002 at 02:46:20PM +1000, Neil Brown wrote:
> I think this can work sanely and is something I have considered for
> raid1-read and multipath in md.
> 
> struct privatebit {
>   bio_end_io_t  *oldend;
>   void          *oldprivate;
>   ...other...stuff;
> };
> 
> make_request(struct request_queue_t *q, struct buffer_head *bh, int rw)
> {
> 
>  struct privatebit *pb = kmalloc(...);
> 
>   pb->oldend = bh->b_end_io;
>   pb->oldprivate = bh->b_private;
>   bh->b_private = pb;
>   bh->b_end_io = my_end_handler;
> 
>   ..remap b_rdev, b_rsector ...
> 
>   generic_make_request(bh, rw);
> 
> }
> 
> Then my_end_handler have do some local cleanup,
> re-instate oldend and oldprivate, and pass the bh back up.
> For raid1/multipath it would arrange to resubmit the request if there
> as an error.
> 
> This stacks nicely and allows for the extra bit to be alloced to be
> minimal.

This is exactly what I'm doing in device-mapper :)

> Ofcourse this ceases to be an issue in 2.5 because the filesys uses 
> pages or buffer_heads and the device driver uses bios.

y, 2.5 is fine.

- Joe

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Jens Axboe]

On Thu, Jul 04 2002, Neil Brown wrote:
> On Wednesday July 3, axboe@suse.de wrote:
> > 
> > Now we are in a grey area. The 'usual' stacked drivers work like this:
> > 
> > some fs path
> > 	submit_bh(bh_orig);
> > 
> > ...
> > 
> > stacked driver make_request_fn:
> > 	bh_new = alloc_bh
> > 	bh_new->b_private = bh_orig;
> > 	...
> > 	submit_bh(bh_new);
> > 
> > if you are just modifying b_private, how exactly is your stacking
> > working? ie what about lvm2 on lvm2?
> 
> I think this can work sanely and is something I have considered for
> raid1-read and multipath in md.
> 
> struct privatebit {
>   bio_end_io_t  *oldend;
>   void          *oldprivate;
>   ...other...stuff;
> };
> 
> make_request(struct request_queue_t *q, struct buffer_head *bh, int rw)
> {
> 
>  struct privatebit *pb = kmalloc(...);
> 
>   pb->oldend = bh->b_end_io;
>   pb->oldprivate = bh->b_private;
>   bh->b_private = pb;
>   bh->b_end_io = my_end_handler;
> 
>   ..remap b_rdev, b_rsector ...
> 
>   generic_make_request(bh, rw);
> 
> }

Yes this works fine, as long as you assert that stacking is an entity of
the b_end_io functions, ie you dont rely on the state of the buffer_head
before i/o completion. But it breaks for ext3.

I'm inclined to say that I think 2.4 mappers should go the full length
like I did in loop, which is always safe (although it does eat more
memory, of course, buffer_heads aren't exactly lite :)

int make_request(struct request_queue_t *q, struct buffer_head *bh, int rw)
{
	struct buffer_head *stack_bh = some_pool_alloc(...);

	stack_bh->b_private = bh;
	stack_bh->b_end_io = my_end_handler;

	/* setup b_rdev and b_rsector, etc */
	generic_make_request(stack_bh, rw);
	return 0;
}

> We just want ext3/jbd to make sure that it only calls bh2jh on
> an unlocked buffer... is that easy?

That's the question indeed, someone with a good grasp of jbd should make
that call. If that is the only 'violator' (depending on your point of
view), then yes lets just fix that up and say that the above is pb
private is valid.

> Ofcourse this ceases to be an issue in 2.5 because the filesys uses 
> pages or buffer_heads and the device driver uses bios.

Yep, no such problem in 2.5

-- 
Jens Axboe

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Jens Axboe]

On Thu, Jul 04 2002, Neil Brown wrote:
> On Wednesday July 3, axboe@suse.de wrote:
> > 
> > Now we are in a grey area. The 'usual' stacked drivers work like this:
> > 
> > some fs path
> > 	submit_bh(bh_orig);
> > 
> > ...
> > 
> > stacked driver make_request_fn:
> > 	bh_new = alloc_bh
> > 	bh_new->b_private = bh_orig;
> > 	...
> > 	submit_bh(bh_new);
> > 
> > if you are just modifying b_private, how exactly is your stacking
> > working? ie what about lvm2 on lvm2?
> 
> I think this can work sanely and is something I have considered for
> raid1-read and multipath in md.
> 
> struct privatebit {
>   bio_end_io_t  *oldend;
>   void          *oldprivate;
>   ...other...stuff;
> };
> 
> make_request(struct request_queue_t *q, struct buffer_head *bh, int rw)
> {
> 
>  struct privatebit *pb = kmalloc(...);
> 
>   pb->oldend = bh->b_end_io;
>   pb->oldprivate = bh->b_private;
>   bh->b_private = pb;
>   bh->b_end_io = my_end_handler;
> 
>   ..remap b_rdev, b_rsector ...
> 
>   generic_make_request(bh, rw);
> 
> }

Yes this works fine, as long as you assert that stacking is an entity of
the b_end_io functions, ie you dont rely on the state of the buffer_head
before i/o completion. But it breaks for ext3.

I'm inclined to say that I think 2.4 mappers should go the full length
like I did in loop, which is always safe (although it does eat more
memory, of course, buffer_heads aren't exactly lite :)

int make_request(struct request_queue_t *q, struct buffer_head *bh, int rw)
{
	struct buffer_head *stack_bh = some_pool_alloc(...);

	stack_bh->b_private = bh;
	stack_bh->b_end_io = my_end_handler;

	/* setup b_rdev and b_rsector, etc */
	generic_make_request(stack_bh, rw);
	return 0;
}

> We just want ext3/jbd to make sure that it only calls bh2jh on
> an unlocked buffer... is that easy?

That's the question indeed, someone with a good grasp of jbd should make
that call. If that is the only 'violator' (depending on your point of
view), then yes lets just fix that up and say that the above is pb
private is valid.

> Ofcourse this ceases to be an issue in 2.5 because the filesys uses 
> pages or buffer_heads and the device driver uses bios.

Yep, no such problem in 2.5

-- 
Jens Axboe

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Andrew Morton]

Jens Axboe wrote:
> 
> ...
> > We just want ext3/jbd to make sure that it only calls bh2jh on
> > an unlocked buffer... is that easy?
> 
> That's the question indeed, someone with a good grasp of jbd should make
> that call. If that is the only 'violator' (depending on your point of
> view), then yes lets just fix that up and say that the above is pb
> private is valid.

We really don't want to do this, please.  Changing things so
that we can only run bh2jh() and, particularly, journal_add_journal_head()
on a locked buffer would involve fairly unpleasant surgery against
parts of ext3 which are already prone to exploding.  Like
do_get_write_access().

If it was needed for 2.5 then hmm, maybe.  But as this is only a
2.4 problem then I really don't think we should risk breaking
or slowing down the filesystem for this.

Look, it's easy: delete buffer_head.b_inode (which is only used as 
a boolean), move its function to a b_state bit.  Add a new
buffer_head.ext3_hack and we can use that for pointing at the journal_head.

<insert "stable kernel" mantra here ;)>

-

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Andrew Morton]

Jens Axboe wrote:
> 
> ...
> > We just want ext3/jbd to make sure that it only calls bh2jh on
> > an unlocked buffer... is that easy?
> 
> That's the question indeed, someone with a good grasp of jbd should make
> that call. If that is the only 'violator' (depending on your point of
> view), then yes lets just fix that up and say that the above is pb
> private is valid.

We really don't want to do this, please.  Changing things so
that we can only run bh2jh() and, particularly, journal_add_journal_head()
on a locked buffer would involve fairly unpleasant surgery against
parts of ext3 which are already prone to exploding.  Like
do_get_write_access().

If it was needed for 2.5 then hmm, maybe.  But as this is only a
2.4 problem then I really don't think we should risk breaking
or slowing down the filesystem for this.

Look, it's easy: delete buffer_head.b_inode (which is only used as 
a boolean), move its function to a b_state bit.  Add a new
buffer_head.ext3_hack and we can use that for pointing at the journal_head.

<insert "stable kernel" mantra here ;)>

-

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Jens Axboe]

On Thu, Jul 04 2002, Andrew Morton wrote:
> Jens Axboe wrote:
> > 
> > ...
> > > We just want ext3/jbd to make sure that it only calls bh2jh on
> > > an unlocked buffer... is that easy?
> > 
> > That's the question indeed, someone with a good grasp of jbd should make
> > that call. If that is the only 'violator' (depending on your point of
> > view), then yes lets just fix that up and say that the above is pb
> > private is valid.
> 
> We really don't want to do this, please.  Changing things so
> that we can only run bh2jh() and, particularly, journal_add_journal_head()
> on a locked buffer would involve fairly unpleasant surgery against
> parts of ext3 which are already prone to exploding.  Like
> do_get_write_access().
> 
> If it was needed for 2.5 then hmm, maybe.  But as this is only a
> 2.4 problem then I really don't think we should risk breaking
> or slowing down the filesystem for this.
> 
> Look, it's easy: delete buffer_head.b_inode (which is only used as 
> a boolean), move its function to a b_state bit.  Add a new
> buffer_head.ext3_hack and we can use that for pointing at the journal_head.

Thank you, this is what I was looking for (if you look further up, I was
advocating this very thing). Slimming down buffer_head and just add the
ext3 hack is perfectly acceptable to me.

Which just means that device mapper needs to do the stacking properly,
EOD.

-- 
Jens Axboe

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Jens Axboe]

On Thu, Jul 04 2002, Andrew Morton wrote:
> Jens Axboe wrote:
> > 
> > ...
> > > We just want ext3/jbd to make sure that it only calls bh2jh on
> > > an unlocked buffer... is that easy?
> > 
> > That's the question indeed, someone with a good grasp of jbd should make
> > that call. If that is the only 'violator' (depending on your point of
> > view), then yes lets just fix that up and say that the above is pb
> > private is valid.
> 
> We really don't want to do this, please.  Changing things so
> that we can only run bh2jh() and, particularly, journal_add_journal_head()
> on a locked buffer would involve fairly unpleasant surgery against
> parts of ext3 which are already prone to exploding.  Like
> do_get_write_access().
> 
> If it was needed for 2.5 then hmm, maybe.  But as this is only a
> 2.4 problem then I really don't think we should risk breaking
> or slowing down the filesystem for this.
> 
> Look, it's easy: delete buffer_head.b_inode (which is only used as 
> a boolean), move its function to a b_state bit.  Add a new
> buffer_head.ext3_hack and we can use that for pointing at the journal_head.

Thank you, this is what I was looking for (if you look further up, I was
advocating this very thing). Slimming down buffer_head and just add the
ext3 hack is perfectly acceptable to me.

Which just means that device mapper needs to do the stacking properly,
EOD.

-- 
Jens Axboe

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Joe Thornber]

On Thu, Jul 04, 2002 at 10:39:41AM +0200, Jens Axboe wrote:
> Which just means that device mapper needs to do the stacking properly,
> EOD.

You don't think it does it properly ATM ?

- Joe

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Joe Thornber]

On Thu, Jul 04, 2002 at 10:39:41AM +0200, Jens Axboe wrote:
> Which just means that device mapper needs to do the stacking properly,
> EOD.

You don't think it does it properly ATM ?

- Joe

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Jens Axboe]

On Thu, Jul 04 2002, Joe Thornber wrote:
> On Thu, Jul 04, 2002 at 10:39:41AM +0200, Jens Axboe wrote:
> > Which just means that device mapper needs to do the stacking properly,
> > EOD.
> 
> You don't think it does it properly ATM ?

Well as I said, it's in the grey zone. If you look at stacking from a
submission and end_io point of view, then yes it works. Anywhere in
between, no it doesn't.

See my previous mails in this thread :-)

-- 
Jens Axboe

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Jens Axboe]

On Thu, Jul 04 2002, Joe Thornber wrote:
> On Thu, Jul 04, 2002 at 10:39:41AM +0200, Jens Axboe wrote:
> > Which just means that device mapper needs to do the stacking properly,
> > EOD.
> 
> You don't think it does it properly ATM ?

Well as I said, it's in the grey zone. If you look at stacking from a
submission and end_io point of view, then yes it works. Anywhere in
between, no it doesn't.

See my previous mails in this thread :-)

-- 
Jens Axboe

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Andrew Morton]

Jens Axboe wrote:
> 
> ...
> Thank you, this is what I was looking for (if you look further up, I was
> advocating this very thing). Slimming down buffer_head and just add the
> ext3 hack is perfectly acceptable to me.
> 
> Which just means that device mapper needs to do the stacking properly,
> EOD.
> 

Here it is.  We can slot this into 2.4.20-pre, or Joe can own
it.  Any preferences?

 fs/buffer.c         |   21 ++++++++++++---------
 fs/jbd/journal.c    |   14 +++++++-------
 include/linux/fs.h  |   19 +++++++++++++++++--
 include/linux/jbd.h |    2 +-
 4 files changed, 37 insertions(+), 19 deletions(-)

--- 2.4.19-rc1/fs/buffer.c~ext3-journal-head	Thu Jul  4 02:25:17 2002
+++ 2.4.19-rc1-akpm/fs/buffer.c	Thu Jul  4 02:36:06 2002
@@ -587,9 +587,10 @@ struct buffer_head * get_hash_table(kdev
 void buffer_insert_inode_queue(struct buffer_head *bh, struct inode *inode)
 {
 	spin_lock(&lru_list_lock);
-	if (bh->b_inode)
+	if (buffer_inode(bh))
 		list_del(&bh->b_inode_buffers);
-	bh->b_inode = inode;
+	else
+		set_buffer_inode(bh);
 	list_add(&bh->b_inode_buffers, &inode->i_dirty_buffers);
 	spin_unlock(&lru_list_lock);
 }
@@ -597,9 +598,10 @@ void buffer_insert_inode_queue(struct bu
 void buffer_insert_inode_data_queue(struct buffer_head *bh, struct inode *inode)
 {
 	spin_lock(&lru_list_lock);
-	if (bh->b_inode)
+	if (buffer_inode(bh))
 		list_del(&bh->b_inode_buffers);
-	bh->b_inode = inode;
+	else
+		set_buffer_inode(bh);
 	list_add(&bh->b_inode_buffers, &inode->i_dirty_data_buffers);
 	spin_unlock(&lru_list_lock);
 }
@@ -608,13 +610,13 @@ void buffer_insert_inode_data_queue(stru
    remove_inode_queue functions.  */
 static void __remove_inode_queue(struct buffer_head *bh)
 {
-	bh->b_inode = NULL;
+	clear_buffer_inode(bh);
 	list_del(&bh->b_inode_buffers);
 }
 
 static inline void remove_inode_queue(struct buffer_head *bh)
 {
-	if (bh->b_inode)
+	if (buffer_inode(bh))
 		__remove_inode_queue(bh);
 }
 
@@ -746,6 +748,7 @@ void init_buffer(struct buffer_head *bh,
 	bh->b_list = BUF_CLEAN;
 	bh->b_end_io = handler;
 	bh->b_private = private;
+	bh->b_journal_head = NULL;
 }
 
 static void end_buffer_io_async(struct buffer_head * bh, int uptodate)
@@ -843,9 +846,9 @@ int fsync_buffers_list(struct list_head 
 		bh = BH_ENTRY(list->next);
 		list_del(&bh->b_inode_buffers);
 		if (!buffer_dirty(bh) && !buffer_locked(bh))
-			bh->b_inode = NULL;
+			clear_buffer_inode(bh);
 		else {
-			bh->b_inode = &tmp;
+			set_buffer_inode(bh);
 			list_add(&bh->b_inode_buffers, &tmp.i_dirty_buffers);
 			if (buffer_dirty(bh)) {
 				get_bh(bh);
@@ -1129,7 +1132,7 @@ struct buffer_head * bread(kdev_t dev, i
  */
 static void __put_unused_buffer_head(struct buffer_head * bh)
 {
-	if (bh->b_inode)
+	if (buffer_inode(bh))
 		BUG();
 	if (nr_unused_buffer_heads >= MAX_UNUSED_BUFFERS) {
 		kmem_cache_free(bh_cachep, bh);
--- 2.4.19-rc1/include/linux/fs.h~ext3-journal-head	Thu Jul  4 02:25:17 2002
+++ 2.4.19-rc1-akpm/include/linux/fs.h	Thu Jul  4 02:25:17 2002
@@ -219,6 +219,7 @@ enum bh_state_bits {
 	BH_Wait_IO,	/* 1 if we should write out this buffer */
 	BH_Launder,	/* 1 if we can throttle on this buffer */
 	BH_JBD,		/* 1 if it has an attached journal_head */
+	BH_Inode,	/* 1 if it is attached to i_dirty[_data]_buffers */
 
 	BH_PrivateStart,/* not a state bit, but the first bit available
 			 * for private allocation by other entities
@@ -261,11 +262,10 @@ struct buffer_head {
 	struct page *b_page;		/* the page this bh is mapped to */
 	void (*b_end_io)(struct buffer_head *bh, int uptodate); /* I/O completion */
  	void *b_private;		/* reserved for b_end_io */
-
+	void *b_journal_head;		/* ext3 journal_heads */
 	unsigned long b_rsector;	/* Real buffer location on disk */
 	wait_queue_head_t b_wait;
 
-	struct inode *	     b_inode;
 	struct list_head     b_inode_buffers;	/* doubly linked list of inode dirty buffers */
 };
 
@@ -1181,6 +1181,21 @@ static inline void mark_buffer_async(str
 		clear_bit(BH_Async, &bh->b_state);
 }
 
+static inline void set_buffer_inode(struct buffer_head *bh)
+{
+	set_bit(BH_Inode, &bh->b_state);
+}
+
+static inline void clear_buffer_inode(struct buffer_head *bh)
+{
+	clear_bit(BH_Inode, &bh->b_state);
+}
+
+static inline int buffer_inode(struct buffer_head *bh)
+{
+	return test_bit(BH_Inode, &bh->b_state);
+}
+
 /*
  * If an error happens during the make_request, this function
  * has to be recalled. It marks the buffer as clean and not
--- 2.4.19-rc1/fs/jbd/journal.c~ext3-journal-head	Thu Jul  4 02:25:17 2002
+++ 2.4.19-rc1-akpm/fs/jbd/journal.c	Thu Jul  4 02:25:17 2002
@@ -1625,8 +1625,8 @@ static void journal_free_journal_head(st
  *
  * Whenever a buffer has an attached journal_head, its ->b_state:BH_JBD bit
  * is set.  This bit is tested in core kernel code where we need to take
- * JBD-specific actions.  Testing the zeroness of ->b_private is not reliable
- * there.
+ * JBD-specific actions.  Testing the zeroness of ->b_journal_head is not
+ * reliable there.
  *
  * When a buffer has its BH_JBD bit set, its ->b_count is elevated by one.
  *
@@ -1681,9 +1681,9 @@ struct journal_head *journal_add_journal
 
 		if (buffer_jbd(bh)) {
 			/* Someone did it for us! */
-			J_ASSERT_BH(bh, bh->b_private != NULL);
+			J_ASSERT_BH(bh, bh->b_journal_head != NULL);
 			journal_free_journal_head(jh);
-			jh = bh->b_private;
+			jh = bh->b_journal_head;
 		} else {
 			/*
 			 * We actually don't need jh_splice_lock when
@@ -1691,7 +1691,7 @@ struct journal_head *journal_add_journal
 			 */
 			spin_lock(&jh_splice_lock);
 			set_bit(BH_JBD, &bh->b_state);
-			bh->b_private = jh;
+			bh->b_journal_head = jh;
 			jh->b_bh = bh;
 			atomic_inc(&bh->b_count);
 			spin_unlock(&jh_splice_lock);
@@ -1700,7 +1700,7 @@ struct journal_head *journal_add_journal
 	}
 	jh->b_jcount++;
 	spin_unlock(&journal_datalist_lock);
-	return bh->b_private;
+	return bh->b_journal_head;
 }
 
 /*
@@ -1733,7 +1733,7 @@ void __journal_remove_journal_head(struc
 			J_ASSERT_BH(bh, jh2bh(jh) == bh);
 			BUFFER_TRACE(bh, "remove journal_head");
 			spin_lock(&jh_splice_lock);
-			bh->b_private = NULL;
+			bh->b_journal_head = NULL;
 			jh->b_bh = NULL;	/* debug, really */
 			clear_bit(BH_JBD, &bh->b_state);
 			__brelse(bh);
--- 2.4.19-rc1/include/linux/jbd.h~ext3-journal-head	Thu Jul  4 02:25:17 2002
+++ 2.4.19-rc1-akpm/include/linux/jbd.h	Thu Jul  4 02:26:29 2002
@@ -246,7 +246,7 @@ static inline struct buffer_head *jh2bh(
 
 static inline struct journal_head *bh2jh(struct buffer_head *bh)
 {
-	return bh->b_private;
+	return bh->b_journal_head;
 }
 
 struct jbd_revoke_table_s;

-

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Andrew Morton]

Jens Axboe wrote:
> 
> ...
> Thank you, this is what I was looking for (if you look further up, I was
> advocating this very thing). Slimming down buffer_head and just add the
> ext3 hack is perfectly acceptable to me.
> 
> Which just means that device mapper needs to do the stacking properly,
> EOD.
> 

Here it is.  We can slot this into 2.4.20-pre, or Joe can own
it.  Any preferences?

 fs/buffer.c         |   21 ++++++++++++---------
 fs/jbd/journal.c    |   14 +++++++-------
 include/linux/fs.h  |   19 +++++++++++++++++--
 include/linux/jbd.h |    2 +-
 4 files changed, 37 insertions(+), 19 deletions(-)

--- 2.4.19-rc1/fs/buffer.c~ext3-journal-head	Thu Jul  4 02:25:17 2002
+++ 2.4.19-rc1-akpm/fs/buffer.c	Thu Jul  4 02:36:06 2002
@@ -587,9 +587,10 @@ struct buffer_head * get_hash_table(kdev
 void buffer_insert_inode_queue(struct buffer_head *bh, struct inode *inode)
 {
 	spin_lock(&lru_list_lock);
-	if (bh->b_inode)
+	if (buffer_inode(bh))
 		list_del(&bh->b_inode_buffers);
-	bh->b_inode = inode;
+	else
+		set_buffer_inode(bh);
 	list_add(&bh->b_inode_buffers, &inode->i_dirty_buffers);
 	spin_unlock(&lru_list_lock);
 }
@@ -597,9 +598,10 @@ void buffer_insert_inode_queue(struct bu
 void buffer_insert_inode_data_queue(struct buffer_head *bh, struct inode *inode)
 {
 	spin_lock(&lru_list_lock);
-	if (bh->b_inode)
+	if (buffer_inode(bh))
 		list_del(&bh->b_inode_buffers);
-	bh->b_inode = inode;
+	else
+		set_buffer_inode(bh);
 	list_add(&bh->b_inode_buffers, &inode->i_dirty_data_buffers);
 	spin_unlock(&lru_list_lock);
 }
@@ -608,13 +610,13 @@ void buffer_insert_inode_data_queue(stru
    remove_inode_queue functions.  */
 static void __remove_inode_queue(struct buffer_head *bh)
 {
-	bh->b_inode = NULL;
+	clear_buffer_inode(bh);
 	list_del(&bh->b_inode_buffers);
 }
 
 static inline void remove_inode_queue(struct buffer_head *bh)
 {
-	if (bh->b_inode)
+	if (buffer_inode(bh))
 		__remove_inode_queue(bh);
 }
 
@@ -746,6 +748,7 @@ void init_buffer(struct buffer_head *bh,
 	bh->b_list = BUF_CLEAN;
 	bh->b_end_io = handler;
 	bh->b_private = private;
+	bh->b_journal_head = NULL;
 }
 
 static void end_buffer_io_async(struct buffer_head * bh, int uptodate)
@@ -843,9 +846,9 @@ int fsync_buffers_list(struct list_head 
 		bh = BH_ENTRY(list->next);
 		list_del(&bh->b_inode_buffers);
 		if (!buffer_dirty(bh) && !buffer_locked(bh))
-			bh->b_inode = NULL;
+			clear_buffer_inode(bh);
 		else {
-			bh->b_inode = &tmp;
+			set_buffer_inode(bh);
 			list_add(&bh->b_inode_buffers, &tmp.i_dirty_buffers);
 			if (buffer_dirty(bh)) {
 				get_bh(bh);
@@ -1129,7 +1132,7 @@ struct buffer_head * bread(kdev_t dev, i
  */
 static void __put_unused_buffer_head(struct buffer_head * bh)
 {
-	if (bh->b_inode)
+	if (buffer_inode(bh))
 		BUG();
 	if (nr_unused_buffer_heads >= MAX_UNUSED_BUFFERS) {
 		kmem_cache_free(bh_cachep, bh);
--- 2.4.19-rc1/include/linux/fs.h~ext3-journal-head	Thu Jul  4 02:25:17 2002
+++ 2.4.19-rc1-akpm/include/linux/fs.h	Thu Jul  4 02:25:17 2002
@@ -219,6 +219,7 @@ enum bh_state_bits {
 	BH_Wait_IO,	/* 1 if we should write out this buffer */
 	BH_Launder,	/* 1 if we can throttle on this buffer */
 	BH_JBD,		/* 1 if it has an attached journal_head */
+	BH_Inode,	/* 1 if it is attached to i_dirty[_data]_buffers */
 
 	BH_PrivateStart,/* not a state bit, but the first bit available
 			 * for private allocation by other entities
@@ -261,11 +262,10 @@ struct buffer_head {
 	struct page *b_page;		/* the page this bh is mapped to */
 	void (*b_end_io)(struct buffer_head *bh, int uptodate); /* I/O completion */
  	void *b_private;		/* reserved for b_end_io */
-
+	void *b_journal_head;		/* ext3 journal_heads */
 	unsigned long b_rsector;	/* Real buffer location on disk */
 	wait_queue_head_t b_wait;
 
-	struct inode *	     b_inode;
 	struct list_head     b_inode_buffers;	/* doubly linked list of inode dirty buffers */
 };
 
@@ -1181,6 +1181,21 @@ static inline void mark_buffer_async(str
 		clear_bit(BH_Async, &bh->b_state);
 }
 
+static inline void set_buffer_inode(struct buffer_head *bh)
+{
+	set_bit(BH_Inode, &bh->b_state);
+}
+
+static inline void clear_buffer_inode(struct buffer_head *bh)
+{
+	clear_bit(BH_Inode, &bh->b_state);
+}
+
+static inline int buffer_inode(struct buffer_head *bh)
+{
+	return test_bit(BH_Inode, &bh->b_state);
+}
+
 /*
  * If an error happens during the make_request, this function
  * has to be recalled. It marks the buffer as clean and not
--- 2.4.19-rc1/fs/jbd/journal.c~ext3-journal-head	Thu Jul  4 02:25:17 2002
+++ 2.4.19-rc1-akpm/fs/jbd/journal.c	Thu Jul  4 02:25:17 2002
@@ -1625,8 +1625,8 @@ static void journal_free_journal_head(st
  *
  * Whenever a buffer has an attached journal_head, its ->b_state:BH_JBD bit
  * is set.  This bit is tested in core kernel code where we need to take
- * JBD-specific actions.  Testing the zeroness of ->b_private is not reliable
- * there.
+ * JBD-specific actions.  Testing the zeroness of ->b_journal_head is not
+ * reliable there.
  *
  * When a buffer has its BH_JBD bit set, its ->b_count is elevated by one.
  *
@@ -1681,9 +1681,9 @@ struct journal_head *journal_add_journal
 
 		if (buffer_jbd(bh)) {
 			/* Someone did it for us! */
-			J_ASSERT_BH(bh, bh->b_private != NULL);
+			J_ASSERT_BH(bh, bh->b_journal_head != NULL);
 			journal_free_journal_head(jh);
-			jh = bh->b_private;
+			jh = bh->b_journal_head;
 		} else {
 			/*
 			 * We actually don't need jh_splice_lock when
@@ -1691,7 +1691,7 @@ struct journal_head *journal_add_journal
 			 */
 			spin_lock(&jh_splice_lock);
 			set_bit(BH_JBD, &bh->b_state);
-			bh->b_private = jh;
+			bh->b_journal_head = jh;
 			jh->b_bh = bh;
 			atomic_inc(&bh->b_count);
 			spin_unlock(&jh_splice_lock);
@@ -1700,7 +1700,7 @@ struct journal_head *journal_add_journal
 	}
 	jh->b_jcount++;
 	spin_unlock(&journal_datalist_lock);
-	return bh->b_private;
+	return bh->b_journal_head;
 }
 
 /*
@@ -1733,7 +1733,7 @@ void __journal_remove_journal_head(struc
 			J_ASSERT_BH(bh, jh2bh(jh) == bh);
 			BUFFER_TRACE(bh, "remove journal_head");
 			spin_lock(&jh_splice_lock);
-			bh->b_private = NULL;
+			bh->b_journal_head = NULL;
 			jh->b_bh = NULL;	/* debug, really */
 			clear_bit(BH_JBD, &bh->b_state);
 			__brelse(bh);
--- 2.4.19-rc1/include/linux/jbd.h~ext3-journal-head	Thu Jul  4 02:25:17 2002
+++ 2.4.19-rc1-akpm/include/linux/jbd.h	Thu Jul  4 02:26:29 2002
@@ -246,7 +246,7 @@ static inline struct buffer_head *jh2bh(
 
 static inline struct journal_head *bh2jh(struct buffer_head *bh)
 {
-	return bh->b_private;
+	return bh->b_journal_head;
 }
 
 struct jbd_revoke_table_s;

-

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Joe Thornber]

Andrew,

On Thu, Jul 04, 2002 at 02:44:30AM -0700, Andrew Morton wrote:
> Here it is.  We can slot this into 2.4.20-pre, or Joe can own
> it.  Any preferences?

Thanks for doing this, I'll keep this with the device-mapper patches
for now.

- Joe

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Joe Thornber]

Andrew,

On Thu, Jul 04, 2002 at 02:44:30AM -0700, Andrew Morton wrote:
> Here it is.  We can slot this into 2.4.20-pre, or Joe can own
> it.  Any preferences?

Thanks for doing this, I'll keep this with the device-mapper patches
for now.

- Joe

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Mark Peloquin]

On 2002-07-02 14:17:02, Joe wrote:

>This is a horrible hack to get around the fact that ext3 uses the
>b_private field for its own purposes after the buffer_head has been
>handed to the block layer (it doesn't just use b_private when in the
>b_end_io function).  Is this acceptable behaviour ?  Other > filesystems
>do not have similar problems as far as I know.

Under what conditions does ext3 exhibit this behaviour? In EVMS, we have 
been stacking the b_private field (for many months), and have not seen any 
problems or have had any problems reported with ext3.

Mark



_________________________________________________________________
Join the world�s largest e-mail service with MSN Hotmail. 
http://www.hotmail.com

Re: [linux-lvm] LVM2 modifies the buffer_head struct?

[Mark Peloquin]

On 2002-07-02 14:17:02, Joe wrote:

>This is a horrible hack to get around the fact that ext3 uses the
>b_private field for its own purposes after the buffer_head has been
>handed to the block layer (it doesn't just use b_private when in the
>b_end_io function).  Is this acceptable behaviour ?  Other > filesystems
>do not have similar problems as far as I know.

Under what conditions does ext3 exhibit this behaviour? In EVMS, we have
been stacking the b_private field (for many months), and have not seen any
problems or have had any problems reported with ext3.

Mark