Re: [stable][PATCH < 2.6.19] Fix data corruption with dm-crypt over RAID5

2007-02-22 Thread Piet Delaney 
Christophe Saout wrote:

> Fix corruption issue with dm-crypt on top of software raid5. Cancelled
> readahead bio's that report no error, just have BIO_UPTODATE cleared
> were reported as successful reads to the higher layers (and leaving
> random content in the buffer cache). Already fixed in 2.6.19.
> 
> Signed-off-by: Christophe Saout <[EMAIL PROTECTED]>
> 
> 
> --- linux-2.6.18.orig/drivers/md/dm-crypt.c   2006-09-20 05:42:06.0
> +0200
> +++ linux-2.6.18/drivers/md/dm-crypt.c2006-12-02 03:03:36.0 
> +0100
> @@ -717,13 +717,15 @@
>  if (bio->bi_size)
>  return 1;
>  
> + if (!bio_flagged(bio, BIO_UPTODATE) && !error)
> + error = -EIO;
> +
>  bio_put(bio);
>  
>  /*
>  * successful reads are decrypted by the worker thread
>  */
> - if ((bio_data_dir(bio) == READ)
> - && bio_flagged(bio, BIO_UPTODATE)) {
> + if (bio_data_dir(io->bio) == READ && !error) {
>  kcryptd_queue_io(io);

Why doesn't kcryptd_queue_io() check the return value from queue_work()?


476 static void kcryptd_queue_io(struct crypt_io *io)
477 {
478 INIT_WORK(>work, kcryptd_do_work, io);
479 queue_work(_kcryptd_workqueue, >work);
480 }

-piet

>  return 0;
>  }


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [stable][PATCH 2.6.19] Fix data corruption with dm-crypt over RAID5

2007-02-22 Thread Piet Delaney 
Christophe Saout wrote:

 Fix corruption issue with dm-crypt on top of software raid5. Cancelled
 readahead bio's that report no error, just have BIO_UPTODATE cleared
 were reported as successful reads to the higher layers (and leaving
 random content in the buffer cache). Already fixed in 2.6.19.
 
 Signed-off-by: Christophe Saout [EMAIL PROTECTED]
 
 
 --- linux-2.6.18.orig/drivers/md/dm-crypt.c   2006-09-20 05:42:06.0
 +0200
 +++ linux-2.6.18/drivers/md/dm-crypt.c2006-12-02 03:03:36.0 
 +0100
 @@ -717,13 +717,15 @@
  if (bio-bi_size)
  return 1;
  
 + if (!bio_flagged(bio, BIO_UPTODATE)  !error)
 + error = -EIO;
 +
  bio_put(bio);
  
  /*
  * successful reads are decrypted by the worker thread
  */
 - if ((bio_data_dir(bio) == READ)
 -  bio_flagged(bio, BIO_UPTODATE)) {
 + if (bio_data_dir(io-bio) == READ  !error) {
  kcryptd_queue_io(io);

Why doesn't kcryptd_queue_io() check the return value from queue_work()?


476 static void kcryptd_queue_io(struct crypt_io *io)
477 {
478 INIT_WORK(io-work, kcryptd_do_work, io);
479 queue_work(_kcryptd_workqueue, io-work);
480 }

-piet

  return 0;
  }


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [dm-devel] [stable][PATCH < 2.6.19] Fix data corruption with dm-crypt over RAID5

2007-01-17 Thread Piet Delaney 
On Sat, 2006-12-02 at 03:27 +0100, Christophe Saout wrote:

Hi Christophe:

I'm wondering about trying out your patch with dm-crypt on 2.6.12. 
The code in drivers/md/dm-crypt.c`crypt_endio() appears to be the same.

Is there a reason that this isn't necessary or would be a bad idea.
Looks like the existing code isn't checking the BIO_UPTODATE flag
before doing the bio_put(). Looks the the second part of not calling
kcryptd_queue_io() and forwarding the processing to the cryptd is
effectively the same. The 1st change will set error if BIO_UPTODATE
isn't set and that will cause the 2nd change to skip calling 
kcryptd_queue_io().

I'm not sure about the change in the arg to bio_data_dir() 
changing from bio to io->bio. Perhaps they are equivalent;
care to comment on that.

Unless I hear otherwise I'll try it out Tomorrow.

-piet

> Fix corruption issue with dm-crypt on top of software raid5. Cancelled
> readahead bio's that report no error, just have BIO_UPTODATE cleared
> were reported as successful reads to the higher layers (and leaving
> random content in the buffer cache). Already fixed in 2.6.19.
> 
> Signed-off-by: Christophe Saout <[EMAIL PROTECTED]>
> 
> 
> --- linux-2.6.18.orig/drivers/md/dm-crypt.c   2006-09-20 05:42:06.0 
> +0200
> +++ linux-2.6.18/drivers/md/dm-crypt.c2006-12-02 03:03:36.0 
> +0100
> @@ -717,13 +717,15 @@
>   if (bio->bi_size)
>   return 1;
>  
> + if (!bio_flagged(bio, BIO_UPTODATE) && !error)
> + error = -EIO;
> +
>   bio_put(bio);
>  
>   /*
>* successful reads are decrypted by the worker thread
>*/
> - if ((bio_data_dir(bio) == READ)
> - && bio_flagged(bio, BIO_UPTODATE)) {
> + if (bio_data_dir(io->bio) == READ && !error) {
>   kcryptd_queue_io(io);
>   return 0;
>   }
> 
> 
> --
> dm-devel mailing list
> [EMAIL PROTECTED]
> https://www.redhat.com/mailman/listinfo/dm-devel
-- 
Piet DelaneyPhone: (408) 200-5256
Blue Lane Technologies  Fax:   (408) 200-5299
10450 Bubb Rd.
Cupertino, Ca. 95014Email: [EMAIL PROTECTED]


signature.asc
Description: This is a digitally signed message part

Re: [dm-devel] [stable][PATCH 2.6.19] Fix data corruption with dm-crypt over RAID5

2007-01-17 Thread Piet Delaney 
On Sat, 2006-12-02 at 03:27 +0100, Christophe Saout wrote:

Hi Christophe:

I'm wondering about trying out your patch with dm-crypt on 2.6.12. 
The code in drivers/md/dm-crypt.c`crypt_endio() appears to be the same.

Is there a reason that this isn't necessary or would be a bad idea.
Looks like the existing code isn't checking the BIO_UPTODATE flag
before doing the bio_put(). Looks the the second part of not calling
kcryptd_queue_io() and forwarding the processing to the cryptd is
effectively the same. The 1st change will set error if BIO_UPTODATE
isn't set and that will cause the 2nd change to skip calling 
kcryptd_queue_io().

I'm not sure about the change in the arg to bio_data_dir() 
changing from bio to io-bio. Perhaps they are equivalent;
care to comment on that.

Unless I hear otherwise I'll try it out Tomorrow.

-piet

 Fix corruption issue with dm-crypt on top of software raid5. Cancelled
 readahead bio's that report no error, just have BIO_UPTODATE cleared
 were reported as successful reads to the higher layers (and leaving
 random content in the buffer cache). Already fixed in 2.6.19.
 
 Signed-off-by: Christophe Saout [EMAIL PROTECTED]
 
 
 --- linux-2.6.18.orig/drivers/md/dm-crypt.c   2006-09-20 05:42:06.0 
 +0200
 +++ linux-2.6.18/drivers/md/dm-crypt.c2006-12-02 03:03:36.0 
 +0100
 @@ -717,13 +717,15 @@
   if (bio-bi_size)
   return 1;
  
 + if (!bio_flagged(bio, BIO_UPTODATE)  !error)
 + error = -EIO;
 +
   bio_put(bio);
  
   /*
* successful reads are decrypted by the worker thread
*/
 - if ((bio_data_dir(bio) == READ)
 -  bio_flagged(bio, BIO_UPTODATE)) {
 + if (bio_data_dir(io-bio) == READ  !error) {
   kcryptd_queue_io(io);
   return 0;
   }
 
 
 --
 dm-devel mailing list
 [EMAIL PROTECTED]
 https://www.redhat.com/mailman/listinfo/dm-devel
-- 
Piet DelaneyPhone: (408) 200-5256
Blue Lane Technologies  Fax:   (408) 200-5299
10450 Bubb Rd.
Cupertino, Ca. 95014Email: [EMAIL PROTECTED]


signature.asc
Description: This is a digitally signed message part

Re: [stable][PATCH < 2.6.19] Fix data corruption with dm-crypt over RAID5

2006-12-02 Thread Christophe Saout 
Am Freitag, den 01.12.2006, 19:49 -0800 schrieb Chris Wright:

> * Christophe Saout ([EMAIL PROTECTED]) wrote:
> > Fix corruption issue with dm-crypt on top of software raid5. Cancelled
> > readahead bio's that report no error, just have BIO_UPTODATE cleared
> > were reported as successful reads to the higher layers (and leaving
> > random content in the buffer cache). Already fixed in 2.6.19.
> 
> I take it this is fixed a different way in 2.6.19?  Mind clarifying the
> difference?

It's more or less fixed the same way in 2.6.19. The function was
reordered by Milan Broz to accommodate the changed write path (commit
8b004457168995f2ae2a35327f885183a9e74141):

> [PATCH] dm crypt: restructure for workqueue change
>
> Restructure part of the dm-crypt code in preparation for workqueue
> changes.
>
> Use 'base_bio' or 'clone' variable names consistently throughout.  No
> functional changes are included in this patch.

"No functional changes" actually included the correct fix, accidental or
not.

> Minor nit:  introduces trailing whitespaces, cleaned it up locally.

Ouch, sorry.



signature.asc
Description: Dies ist ein digital signierter Nachrichtenteil

Re: [stable][PATCH 2.6.19] Fix data corruption with dm-crypt over RAID5

2006-12-02 Thread Christophe Saout 
Am Freitag, den 01.12.2006, 19:49 -0800 schrieb Chris Wright:

 * Christophe Saout ([EMAIL PROTECTED]) wrote:
  Fix corruption issue with dm-crypt on top of software raid5. Cancelled
  readahead bio's that report no error, just have BIO_UPTODATE cleared
  were reported as successful reads to the higher layers (and leaving
  random content in the buffer cache). Already fixed in 2.6.19.
 
 I take it this is fixed a different way in 2.6.19?  Mind clarifying the
 difference?

It's more or less fixed the same way in 2.6.19. The function was
reordered by Milan Broz to accommodate the changed write path (commit
8b004457168995f2ae2a35327f885183a9e74141):

 [PATCH] dm crypt: restructure for workqueue change

 Restructure part of the dm-crypt code in preparation for workqueue
 changes.

 Use 'base_bio' or 'clone' variable names consistently throughout.  No
 functional changes are included in this patch.

No functional changes actually included the correct fix, accidental or
not.

 Minor nit:  introduces trailing whitespaces, cleaned it up locally.

Ouch, sorry.



signature.asc
Description: Dies ist ein digital signierter Nachrichtenteil

Re: [stable][PATCH < 2.6.19] Fix data corruption with dm-crypt over RAID5

2006-12-01 Thread Chris Wright 
[Note: please Cc: [EMAIL PROTECTED] on -stable patches]

* Christophe Saout ([EMAIL PROTECTED]) wrote:
> Fix corruption issue with dm-crypt on top of software raid5. Cancelled
> readahead bio's that report no error, just have BIO_UPTODATE cleared
> were reported as successful reads to the higher layers (and leaving
> random content in the buffer cache). Already fixed in 2.6.19.

I take it this is fixed a different way in 2.6.19?  Mind clarifying the
difference?

> Signed-off-by: Christophe Saout <[EMAIL PROTECTED]>
> 
> 
> --- linux-2.6.18.orig/drivers/md/dm-crypt.c   2006-09-20 05:42:06.0 
> +0200
> +++ linux-2.6.18/drivers/md/dm-crypt.c2006-12-02 03:03:36.0 
> +0100
> @@ -717,13 +717,15 @@
>   if (bio->bi_size)
>   return 1;
>  
> + if (!bio_flagged(bio, BIO_UPTODATE) && !error)
> + error = -EIO;
> +

Minor nit:  introduces trailing whitespaces, cleaned it up locally.

thanks,
-chris
--

This is a note to let you know that we have just queued up the patch titled

Subject: dm crypt: Fix data corruption with dm-crypt over RAID5

to the 2.6.18-stable tree.  Its filename is

dm-crypt-fix-data-corruption-with-dm-crypt-over-raid5.patch

A git repo of this tree can be found at 

http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary


>From [EMAIL PROTECTED]  Fri Dec  1 18:36:19 2006
Date:   Sat, 02 Dec 2006 03:27:56 +0100
From: Christophe Saout <[EMAIL PROTECTED]>
To: linux-kernel@vger.kernel.org
Cc: [EMAIL PROTECTED], Andrey <[EMAIL PROTECTED]>, Andrew Morton <[EMAIL 
PROTECTED]>, [EMAIL PROTECTED], Neil Brown <[EMAIL PROTECTED]>, Jens Axboe 
<[EMAIL PROTECTED]>, Chris Wright <[EMAIL PROTECTED]>
Subject: dm crypt: Fix data corruption with dm-crypt over RAID5

Fix corruption issue with dm-crypt on top of software raid5. Cancelled
readahead bio's that report no error, just have BIO_UPTODATE cleared
were reported as successful reads to the higher layers (and leaving
random content in the buffer cache). Already fixed in 2.6.19.

Signed-off-by: Christophe Saout <[EMAIL PROTECTED]>
Signed-off-by: Chris Wright <[EMAIL PROTECTED]>
---
 drivers/md/dm-crypt.c |6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

--- linux-2.6.18.5.orig/drivers/md/dm-crypt.c
+++ linux-2.6.18.5/drivers/md/dm-crypt.c
@@ -717,13 +717,15 @@ static int crypt_endio(struct bio *bio, 
if (bio->bi_size)
return 1;
 
+   if (!bio_flagged(bio, BIO_UPTODATE) && !error)
+   error = -EIO;
+
bio_put(bio);
 
/*
 * successful reads are decrypted by the worker thread
 */
-   if ((bio_data_dir(bio) == READ)
-   && bio_flagged(bio, BIO_UPTODATE)) {
+   if (bio_data_dir(io->bio) == READ && !error) {
kcryptd_queue_io(io);
return 0;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[stable][PATCH < 2.6.19] Fix data corruption with dm-crypt over RAID5

2006-12-01 Thread Christophe Saout 
Fix corruption issue with dm-crypt on top of software raid5. Cancelled
readahead bio's that report no error, just have BIO_UPTODATE cleared
were reported as successful reads to the higher layers (and leaving
random content in the buffer cache). Already fixed in 2.6.19.

Signed-off-by: Christophe Saout <[EMAIL PROTECTED]>


--- linux-2.6.18.orig/drivers/md/dm-crypt.c 2006-09-20 05:42:06.0 
+0200
+++ linux-2.6.18/drivers/md/dm-crypt.c  2006-12-02 03:03:36.0 +0100
@@ -717,13 +717,15 @@
if (bio->bi_size)
return 1;
 
+   if (!bio_flagged(bio, BIO_UPTODATE) && !error)
+   error = -EIO;
+
bio_put(bio);
 
/*
 * successful reads are decrypted by the worker thread
 */
-   if ((bio_data_dir(bio) == READ)
-   && bio_flagged(bio, BIO_UPTODATE)) {
+   if (bio_data_dir(io->bio) == READ && !error) {
kcryptd_queue_io(io);
return 0;
}


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[stable][PATCH 2.6.19] Fix data corruption with dm-crypt over RAID5

2006-12-01 Thread Christophe Saout 
Fix corruption issue with dm-crypt on top of software raid5. Cancelled
readahead bio's that report no error, just have BIO_UPTODATE cleared
were reported as successful reads to the higher layers (and leaving
random content in the buffer cache). Already fixed in 2.6.19.

Signed-off-by: Christophe Saout [EMAIL PROTECTED]


--- linux-2.6.18.orig/drivers/md/dm-crypt.c 2006-09-20 05:42:06.0 
+0200
+++ linux-2.6.18/drivers/md/dm-crypt.c  2006-12-02 03:03:36.0 +0100
@@ -717,13 +717,15 @@
if (bio-bi_size)
return 1;
 
+   if (!bio_flagged(bio, BIO_UPTODATE)  !error)
+   error = -EIO;
+
bio_put(bio);
 
/*
 * successful reads are decrypted by the worker thread
 */
-   if ((bio_data_dir(bio) == READ)
-bio_flagged(bio, BIO_UPTODATE)) {
+   if (bio_data_dir(io-bio) == READ  !error) {
kcryptd_queue_io(io);
return 0;
}


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [stable][PATCH 2.6.19] Fix data corruption with dm-crypt over RAID5

2006-12-01 Thread Chris Wright 
[Note: please Cc: [EMAIL PROTECTED] on -stable patches]

* Christophe Saout ([EMAIL PROTECTED]) wrote:
 Fix corruption issue with dm-crypt on top of software raid5. Cancelled
 readahead bio's that report no error, just have BIO_UPTODATE cleared
 were reported as successful reads to the higher layers (and leaving
 random content in the buffer cache). Already fixed in 2.6.19.

I take it this is fixed a different way in 2.6.19?  Mind clarifying the
difference?

 Signed-off-by: Christophe Saout [EMAIL PROTECTED]
 
 
 --- linux-2.6.18.orig/drivers/md/dm-crypt.c   2006-09-20 05:42:06.0 
 +0200
 +++ linux-2.6.18/drivers/md/dm-crypt.c2006-12-02 03:03:36.0 
 +0100
 @@ -717,13 +717,15 @@
   if (bio-bi_size)
   return 1;
  
 + if (!bio_flagged(bio, BIO_UPTODATE)  !error)
 + error = -EIO;
 +

Minor nit:  introduces trailing whitespaces, cleaned it up locally.

thanks,
-chris
--

This is a note to let you know that we have just queued up the patch titled

Subject: dm crypt: Fix data corruption with dm-crypt over RAID5

to the 2.6.18-stable tree.  Its filename is

dm-crypt-fix-data-corruption-with-dm-crypt-over-raid5.patch

A git repo of this tree can be found at 

http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary


From [EMAIL PROTECTED]  Fri Dec  1 18:36:19 2006
Date:   Sat, 02 Dec 2006 03:27:56 +0100
From: Christophe Saout [EMAIL PROTECTED]
To: linux-kernel@vger.kernel.org
Cc: [EMAIL PROTECTED], Andrey [EMAIL PROTECTED], Andrew Morton [EMAIL 
PROTECTED], [EMAIL PROTECTED], Neil Brown [EMAIL PROTECTED], Jens Axboe 
[EMAIL PROTECTED], Chris Wright [EMAIL PROTECTED]
Subject: dm crypt: Fix data corruption with dm-crypt over RAID5

Fix corruption issue with dm-crypt on top of software raid5. Cancelled
readahead bio's that report no error, just have BIO_UPTODATE cleared
were reported as successful reads to the higher layers (and leaving
random content in the buffer cache). Already fixed in 2.6.19.

Signed-off-by: Christophe Saout [EMAIL PROTECTED]
Signed-off-by: Chris Wright [EMAIL PROTECTED]
---
 drivers/md/dm-crypt.c |6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

--- linux-2.6.18.5.orig/drivers/md/dm-crypt.c
+++ linux-2.6.18.5/drivers/md/dm-crypt.c
@@ -717,13 +717,15 @@ static int crypt_endio(struct bio *bio, 
if (bio-bi_size)
return 1;
 
+   if (!bio_flagged(bio, BIO_UPTODATE)  !error)
+   error = -EIO;
+
bio_put(bio);
 
/*
 * successful reads are decrypted by the worker thread
 */
-   if ((bio_data_dir(bio) == READ)
-bio_flagged(bio, BIO_UPTODATE)) {
+   if (bio_data_dir(io-bio) == READ  !error) {
kcryptd_queue_io(io);
return 0;
}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/