/dev/loop0 over lvm... leading to d-state :-(

/dev/loop0 over lvm... leading to d-state :-(

[Herbert Valerio Riedel]

fyi, loop devices over lvm LV's dont work for me...

I've tested with 2.4.3final (and some other 2.4.3 derivates) and two
lvm'ized partitions with a size of about 1gig each; mke2fs
just goes into D-state and stays there when applying it to /dev/loop0,
running it directly on the LV-device works...

greetings,
-- 

Re: /dev/loop0 over lvm... leading to d-state :-(

[Jens Axboe]

[-- Attachment #1: Type: text/plain, Size: 523 bytes --]

On Wed, Apr 04 2001, Herbert Valerio Riedel wrote:
> 
> fyi, loop devices over lvm LV's dont work for me...
> 
> I've tested with 2.4.3final (and some other 2.4.3 derivates) and two
> lvm'ized partitions with a size of about 1gig each; mke2fs
> just goes into D-state and stays there when applying it to /dev/loop0,
> running it directly on the LV-device works...

this would appear to be an lvm bug, could you try this patch? it's
untested, let me know if it doesn't work and I'll try and reproduce
here.

-- 
Jens Axboe


[-- Attachment #2: lvm-rdev-1 --]
[-- Type: text/plain, Size: 570 bytes --]

--- /opt/kernel/linux-2.4.3/drivers/md/lvm.c	Mon Jan 29 01:11:20 2001
+++ drivers/md/lvm.c	Thu Apr  5 07:12:14 2001
@@ -1480,14 +1480,14 @@
  */
 static int lvm_map(struct buffer_head *bh, int rw)
 {
-	int minor = MINOR(bh->b_dev);
+	int minor = MINOR(bh->b_rdev);
 	int ret = 0;
 	ulong index;
 	ulong pe_start;
 	ulong size = bh->b_size >> 9;
 	ulong rsector_tmp = bh->b_blocknr * size;
 	ulong rsector_sav;
-	kdev_t rdev_tmp = bh->b_dev;
+	kdev_t rdev_tmp = bh->b_rdev;
 	kdev_t rdev_sav;
 	vg_t *vg_this = vg[VG_BLK(minor)];
 	lv_t *lv = vg_this->lv[LV_BLK(minor)];

Re: /dev/loop0 over lvm... leading to d-state :-(

[Jens Axboe]

[-- Attachment #1: Type: text/plain, Size: 906 bytes --]

On Thu, Apr 05 2001, Jens Axboe wrote:
> On Wed, Apr 04 2001, Herbert Valerio Riedel wrote:
> > 
> > fyi, loop devices over lvm LV's dont work for me...
> > 
> > I've tested with 2.4.3final (and some other 2.4.3 derivates) and two
> > lvm'ized partitions with a size of about 1gig each; mke2fs
> > just goes into D-state and stays there when applying it to /dev/loop0,
> > running it directly on the LV-device works...
> 
> this would appear to be an lvm bug, could you try this patch? it's
> untested, let me know if it doesn't work and I'll try and reproduce
> here.

What do you know, there was one more in there. Even visible in
the original hunk :-). And this time it wasn't a loop bug (the
crowd goes bezerk).

To the LVM folks: you can't use b_dev or b_blocknr inside your
make_request_fn, it destroys stacking drivers such as loop. And
is just plain wrong in the general case too.

-- 
Jens Axboe


[-- Attachment #2: lvm-stack-1 --]
[-- Type: text/plain, Size: 952 bytes --]

--- /opt/kernel/linux-2.4.3/drivers/md/lvm.c	Mon Jan 29 01:11:20 2001
+++ drivers/md/lvm.c	Thu Apr  5 16:20:12 2001
@@ -148,6 +148,8 @@
  *                 procfs is always supported now. (JT)
  *    12/01/2001 - avoided flushing logical volume in case of shrinking
  *                 because of unecessary overhead in case of heavy updates
+ *    05/04/2001 - don't use b_blocknr/b_dev in lvm_map, it destroys
+ *		   stacking devices (Jens Axboe)
  *
  */
 
@@ -1480,14 +1482,14 @@
  */
 static int lvm_map(struct buffer_head *bh, int rw)
 {
-	int minor = MINOR(bh->b_dev);
+	int minor = MINOR(bh->b_rdev);
 	int ret = 0;
 	ulong index;
 	ulong pe_start;
 	ulong size = bh->b_size >> 9;
-	ulong rsector_tmp = bh->b_blocknr * size;
+	ulong rsector_tmp = bh->b_rsector;
 	ulong rsector_sav;
-	kdev_t rdev_tmp = bh->b_dev;
+	kdev_t rdev_tmp = bh->b_rdev;
 	kdev_t rdev_sav;
 	vg_t *vg_this = vg[VG_BLK(minor)];
 	lv_t *lv = vg_this->lv[LV_BLK(minor)];

Re: /dev/loop0 over lvm... leading to d-state :-(

[Jens Axboe]

[-- Attachment #1: Type: text/plain, Size: 390 bytes --]

On Thu, Apr 05 2001, Jens Axboe wrote:
> To the LVM folks: you can't use b_dev or b_blocknr inside your
> make_request_fn, it destroys stacking drivers such as loop. And
> is just plain wrong in the general case too.

Irks, another one. lvm_make_request_fn also needs to call b_end_io
if a map fails. Updated patch attached, I'll collate further
patches if I find more :-)

-- 
Jens Axboe


[-- Attachment #2: lvm-stack-2 --]
[-- Type: text/plain, Size: 1338 bytes --]

--- /opt/kernel/linux-2.4.3/drivers/md/lvm.c	Mon Jan 29 01:11:20 2001
+++ drivers/md/lvm.c	Thu Apr  5 16:48:52 2001
@@ -148,6 +148,9 @@
  *                 procfs is always supported now. (JT)
  *    12/01/2001 - avoided flushing logical volume in case of shrinking
  *                 because of unecessary overhead in case of heavy updates
+ *    05/04/2001 - lvm_map bugs: don't use b_blocknr/b_dev in lvm_map, it
+ *		   destroys stacking devices. call b_end_io on failed maps.
+ *		   (Jens Axboe)
  *
  */
 
@@ -1480,14 +1483,14 @@
  */
 static int lvm_map(struct buffer_head *bh, int rw)
 {
-	int minor = MINOR(bh->b_dev);
+	int minor = MINOR(bh->b_rdev);
 	int ret = 0;
 	ulong index;
 	ulong pe_start;
 	ulong size = bh->b_size >> 9;
-	ulong rsector_tmp = bh->b_blocknr * size;
+	ulong rsector_tmp = bh->b_rsector;
 	ulong rsector_sav;
-	kdev_t rdev_tmp = bh->b_dev;
+	kdev_t rdev_tmp = bh->b_rdev;
 	kdev_t rdev_sav;
 	vg_t *vg_this = vg[VG_BLK(minor)];
 	lv_t *lv = vg_this->lv[LV_BLK(minor)];
@@ -1676,8 +1679,12 @@
  */
 static int lvm_make_request_fn(request_queue_t *q,
 			       int rw,
-			       struct buffer_head *bh) {
-	return (lvm_map(bh, rw) < 0) ? 0 : 1;
+			       struct buffer_head *bh)
+{
+	int ret = lvm_map(bh, rw);
+	if (ret)
+		bh->b_end_io(bh, test_bit(BH_Uptodate, &bh->b_state));
+	return ret;
 }
 
 

Re: /dev/loop0 over lvm... leading to d-state :-(

[Jens Axboe]

On Thu, Apr 05 2001, Heinz J. Mauelshagen wrote:
> > Irks, another one. lvm_make_request_fn also needs to call b_end_io
> > if a map fails.
> 
> This is wrong.
> 
> In case of an io error we do already call buffer_IO_error() on 2.4 in
> lvm_map().

Where? Calling buffer_IO_error would be ok, but there are no such calls
in 2.4.3. I just stated elsewhere that submit_bh should probably be
clearing the dirty bit, not ll_rw_block, in which case the b_end_io
is fine. But buffer_IO_error is probably more clear. I trust you'll
take care of that part then.

-- 
Jens Axboe

Re: /dev/loop0 over lvm... leading to d-state :-(

[Jens Axboe]

On Thu, Apr 05 2001, Heinz J. Mauelshagen wrote:
> > Where? Calling buffer_IO_error would be ok, but there are no such calls
> > in 2.4.3. I just stated elsewhere that submit_bh should probably be
> > clearing the dirty bit, not ll_rw_block, in which case the b_end_io
> > is fine. But buffer_IO_error is probably more clear. I trust you'll
> > take care of that part then.
> 
> Sorry, didn't mention that you need to patch the driver with the recent
> LVM software in order to get it.

Ah ok, so the b_dev/b_blocknr is all then. Good.

> I've send the patch a while ago to Linus to get it into 2.4.3
> but he obviously didn't include it (likely because he thought it was too
> large ;-)

Maybe you're hanging on to fixes and submitting huge chunks of them?

> He didn't comment back to me at all though :-(
> Maybe this will help.

Two things that usually help -- submit small and often, then resubmit,
resubmit, resubmit until he takes it or complains loudly :-)

-- 
Jens Axboe

Re: /dev/loop0 over lvm... leading to d-state :-(

[Heinz J. Mauelshagen]

Jens,

thanks for the b_dev hint you provided.

On Thu, Apr 05, 2001 at 04:49:42PM +0200, Jens Axboe wrote:
> On Thu, Apr 05 2001, Jens Axboe wrote:
> > To the LVM folks: you can't use b_dev or b_blocknr inside your
> > make_request_fn, it destroys stacking drivers such as loop. And
> > is just plain wrong in the general case too.
> 
> Irks, another one. lvm_make_request_fn also needs to call b_end_io
> if a map fails.

This is wrong.

In case of an io error we do already call buffer_IO_error() on 2.4 in lvm_map().

In 2.2 ll_rw_block() does change the state of the buffer and calls
b_end_io() for us at the end of the function:

      sorry:
        for (i = 0; i < nr; i++) {
                if (bh[i]) {
                        clear_bit(BH_Dirty, &bh[i]->b_state);
                        clear_bit(BH_Uptodate, &bh[i]->b_state);
                        bh[i]->b_end_io(bh[i], 0);
                }
        }


> Updated patch attached, I'll collate further
> patches if I find more :-)

Please do that. Thanks again.

Regards,
Heinz    -- The LVM Guy --

>
> 
> -- 
> Jens Axboe
> 

> --- /opt/kernel/linux-2.4.3/drivers/md/lvm.c	Mon Jan 29 01:11:20 2001
> +++ drivers/md/lvm.c	Thu Apr  5 16:48:52 2001
> @@ -148,6 +148,9 @@
>   *                 procfs is always supported now. (JT)
>   *    12/01/2001 - avoided flushing logical volume in case of shrinking
>   *                 because of unecessary overhead in case of heavy updates
> + *    05/04/2001 - lvm_map bugs: don't use b_blocknr/b_dev in lvm_map, it
> + *		   destroys stacking devices. call b_end_io on failed maps.
> + *		   (Jens Axboe)
>   *
>   */
>  
> @@ -1480,14 +1483,14 @@
>   */
>  static int lvm_map(struct buffer_head *bh, int rw)
>  {
> -	int minor = MINOR(bh->b_dev);
> +	int minor = MINOR(bh->b_rdev);
>  	int ret = 0;
>  	ulong index;
>  	ulong pe_start;
>  	ulong size = bh->b_size >> 9;
> -	ulong rsector_tmp = bh->b_blocknr * size;
> +	ulong rsector_tmp = bh->b_rsector;
>  	ulong rsector_sav;
> -	kdev_t rdev_tmp = bh->b_dev;
> +	kdev_t rdev_tmp = bh->b_rdev;
>  	kdev_t rdev_sav;
>  	vg_t *vg_this = vg[VG_BLK(minor)];
>  	lv_t *lv = vg_this->lv[LV_BLK(minor)];
> @@ -1676,8 +1679,12 @@
>   */
>  static int lvm_make_request_fn(request_queue_t *q,
>  			       int rw,
> -			       struct buffer_head *bh) {
> -	return (lvm_map(bh, rw) < 0) ? 0 : 1;
> +			       struct buffer_head *bh)
> +{
> +	int ret = lvm_map(bh, rw);
> +	if (ret)
> +		bh->b_end_io(bh, test_bit(BH_Uptodate, &bh->b_state));
> +	return ret;
>  }
>  
>  

*** Software bugs are stupid.
    Nevertheless it needs not so stupid people to solve them ***

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

Heinz Mauelshagen                                 Sistina Software Inc.
Senior Consultant/Developer                       Am Sonnenhang 11
                                                  56242 Marienrachdorf
                                                  Germany
Mauelshagen@Sistina.com                           +49 2626 141200
                                                       FAX 924446
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

Re: /dev/loop0 over lvm... leading to d-state :-(

[Heinz J. Mauelshagen]

On Thu, Apr 05, 2001 at 05:37:31PM +0200, Jens Axboe wrote:
> On Thu, Apr 05 2001, Heinz J. Mauelshagen wrote:
> > > Irks, another one. lvm_make_request_fn also needs to call b_end_io
> > > if a map fails.
> > 
> > This is wrong.
> > 
> > In case of an io error we do already call buffer_IO_error() on 2.4 in
> > lvm_map().
> 
> Where? Calling buffer_IO_error would be ok, but there are no such calls
> in 2.4.3. I just stated elsewhere that submit_bh should probably be
> clearing the dirty bit, not ll_rw_block, in which case the b_end_io
> is fine. But buffer_IO_error is probably more clear. I trust you'll
> take care of that part then.

Sorry, didn't mention that you need to patch the driver with the recent
LVM software in order to get it.

I've send the patch a while ago to Linus to get it into 2.4.3
but he obviously didn't include it (likely because he thought it was too
large ;-)

He didn't comment back to me at all though :-(
Maybe this will help.

Linus?

> 
> -- 
> Jens Axboe
> 

-- 

Regards,
Heinz    -- The LVM Guy --

*** Software bugs are stupid.
    Nevertheless it needs not so stupid people to solve them ***

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

Heinz Mauelshagen                                 Sistina Software Inc.
Senior Consultant/Developer                       Am Sonnenhang 11
                                                  56242 Marienrachdorf
                                                  Germany
Mauelshagen@Sistina.com                           +49 2626 141200
                                                       FAX 924446
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

Re: /dev/loop0 over lvm... leading to d-state :-(

[Heinz J. Mauelshagen]

On Thu, Apr 05, 2001 at 05:54:30PM +0200, Jens Axboe wrote:
> On Thu, Apr 05 2001, Heinz J. Mauelshagen wrote:
> > > Where? Calling buffer_IO_error would be ok, but there are no such calls
> > > in 2.4.3. I just stated elsewhere that submit_bh should probably be
> > > clearing the dirty bit, not ll_rw_block, in which case the b_end_io
> > > is fine. But buffer_IO_error is probably more clear. I trust you'll
> > > take care of that part then.
> > 
> > Sorry, didn't mention that you need to patch the driver with the recent
> > LVM software in order to get it.
> 
> Ah ok, so the b_dev/b_blocknr is all then. Good.
> 
> > I've send the patch a while ago to Linus to get it into 2.4.3
> > but he obviously didn't include it (likely because he thought it was too
> > large ;-)
> 
> Maybe you're hanging on to fixes and submitting huge chunks of them?

We want to change that, sorry :)

> 
> > He didn't comment back to me at all though :-(
> > Maybe this will help.
> 
> Two things that usually help -- submit small and often, then resubmit,
> resubmit, resubmit until he takes it or complains loudly :-)

I know, I know, I know ;-)

> 
> -- 
> Jens Axboe
> 

-- 

Regards,
Heinz    -- The LVM Guy --

*** Software bugs are stupid.
    Nevertheless it needs not so stupid people to solve them ***

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-

Heinz Mauelshagen                                 Sistina Software Inc.
Senior Consultant/Developer                       Am Sonnenhang 11
                                                  56242 Marienrachdorf
                                                  Germany
Mauelshagen@Sistina.com                           +49 2626 141200
                                                       FAX 924446
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-