[PATCH 0/2][SRU][ARTFUL] Fix VFS warning and file system resize failure (LP: #1726818)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/2][SRU][ARTFUL] Fix VFS warning and file system resize failure (LP: #1726818)

Colin Ian King-2
From: Colin Ian King <[hidden email]>

The following two patches fix a VFS warning and a file system resize failure
on Artful when booting a virtual image using a vagrant/VirtualBox combo.

[SRU Justfication, Artful]

[Impact]

Booting the 4.13 Artful kernel with vagrant using VirtualBox trips the warning:

[ 61.010337] VFS: brelse: Trying to free free buffer
[ 61.114875] ------------[ cut here ]------------
[ 61.114886] WARNING: CPU: 0 PID: 683 at /build/linux-XO_uEE/linux-4.13.0/fs/buffer.c:1205 __brelse+0x21/0x30

and a failed resize of a partition. The root cause has been bisected down
to the following commmit:

commit c20cfc27a47307e811346f85959cf3cc07ae42f9
Author: Christoph Hellwig <[hidden email]>
Date: Wed Apr 5 19:21:07 2017 +0200

    block: stop using blkdev_issue_write_same for zeroing

[Fix]
The Upstream commit directly fixes this issue:

commit d5ce4c31d6df518dd8f63bbae20d7423c5018a6c
Author: Ilya Dryomov <[hidden email]>
Date: Mon Oct 16 15:59:10 2017 +0200

    block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()

..however we also require a backport of the following upstream commit to apply
the above commit cleanly:

commit 425a4dba7953e35ffd096771973add6d2f40d2ed
Author: Ilya Dryomov <[hidden email]>
Date: Mon Oct 16 15:59:09 2017 +0200

    block: factor out __blkdev_issue_zero_pages()

[Testscase]

On Ubuntu Xenial:

1. sudo apt-get install virtualbox vagrant
2. edit /etc/group and add one's user name to the vboxusers group
3. log out log back
4. vagrant init ubuntu/artful64
5. vagrant up
6. vagrant ssh
7. dmesg | grep "VFS: brelse"

without the fix one will see the VFS brelse warning message and the /
partition will not have been resized.

with a fixed system there is is no VFS vbrelse warning and / as been
resized as expected.

[Regresion potential]
These patches touch the blk library so potentially it could break the block
layer and corrupt data on disk. However these are upstream fixes that address
the buggy commit c20cfc27a47307e811346f85959cf3cc07ae42f9 and are known to
address the bug.

Colin Ian King (1):
  block: factor out __blkdev_issue_zero_pages()

Ilya Dryomov (1):
  block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()

 block/blk-lib.c | 108 +++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 72 insertions(+), 36 deletions(-)

--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2][SRU][ARTFUL] block: factor out __blkdev_issue_zero_pages()

Colin Ian King-2
From: Colin Ian King <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1726818

blkdev_issue_zeroout() will use this in !BLKDEV_ZERO_NOFALLBACK case.

Reviewed-by: Christoph Hellwig <[hidden email]>
Reviewed-by: Martin K. Petersen <[hidden email]>
Signed-off-by: Ilya Dryomov <[hidden email]>
Signed-off-by: Jens Axboe <[hidden email]>
(backported from upstream commit 425a4dba7953e35ffd096771973add6d2f40d2ed)
Signed-off-by: Colin Ian King <[hidden email]>
---
 block/blk-lib.c | 63 +++++++++++++++++++++++++++++++++------------------------
 1 file changed, 37 insertions(+), 26 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 3fe0aec..4ef942c 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -274,6 +274,40 @@ static unsigned int __blkdev_sectors_to_bio_pages(sector_t nr_sects)
  return min(bytes >> PAGE_SHIFT, (sector_t)BIO_MAX_PAGES);
 }
 
+static int __blkdev_issue_zero_pages(struct block_device *bdev,
+ sector_t sector, sector_t nr_sects, gfp_t gfp_mask,
+ struct bio **biop)
+{
+ struct request_queue *q = bdev_get_queue(bdev);
+ struct bio *bio = *biop;
+ int bi_size = 0;
+ unsigned int sz;
+
+ if (!q)
+ return -ENXIO;
+
+ while (nr_sects != 0) {
+ bio = next_bio(bio, __blkdev_sectors_to_bio_pages(nr_sects),
+       gfp_mask);
+ bio->bi_iter.bi_sector = sector;
+ bio->bi_bdev = bdev;
+ bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
+
+ while (nr_sects != 0) {
+ sz = min((sector_t) PAGE_SIZE, nr_sects << 9);
+ bi_size = bio_add_page(bio, ZERO_PAGE(0), sz, 0);
+ nr_sects -= bi_size >> 9;
+ sector += bi_size >> 9;
+ if (bi_size < sz)
+ break;
+ }
+ cond_resched();
+ }
+
+ *biop = bio;
+ return 0;
+}
+
 /**
  * __blkdev_issue_zeroout - generate number of zero filed write bios
  * @bdev: blockdev to issue
@@ -304,9 +338,6 @@ int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
  unsigned flags)
 {
  int ret;
- int bi_size = 0;
- struct bio *bio = *biop;
- unsigned int sz;
  sector_t bs_mask;
 
  bs_mask = (bdev_logical_block_size(bdev) >> 9) - 1;
@@ -316,30 +347,10 @@ int __blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
  ret = __blkdev_issue_write_zeroes(bdev, sector, nr_sects, gfp_mask,
  biop, flags);
  if (ret != -EOPNOTSUPP || (flags & BLKDEV_ZERO_NOFALLBACK))
- goto out;
-
- ret = 0;
- while (nr_sects != 0) {
- bio = next_bio(bio, __blkdev_sectors_to_bio_pages(nr_sects),
-       gfp_mask);
- bio->bi_iter.bi_sector = sector;
- bio->bi_bdev   = bdev;
- bio_set_op_attrs(bio, REQ_OP_WRITE, 0);
+ return ret;
 
- while (nr_sects != 0) {
- sz = min((sector_t) PAGE_SIZE, nr_sects << 9);
- bi_size = bio_add_page(bio, ZERO_PAGE(0), sz, 0);
- nr_sects -= bi_size >> 9;
- sector += bi_size >> 9;
- if (bi_size < sz)
- break;
- }
- cond_resched();
- }
-
- *biop = bio;
-out:
- return ret;
+ return __blkdev_issue_zero_pages(bdev, sector, nr_sects, gfp_mask,
+ biop);
 }
 EXPORT_SYMBOL(__blkdev_issue_zeroout);
 
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2][SRU][ARTFUL] block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()

Colin Ian King-2
In reply to this post by Colin Ian King-2
From: Ilya Dryomov <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1726818

sd_config_write_same() ignores ->max_ws_blocks == 0 and resets it to
permit trying WRITE SAME on older SCSI devices, unless ->no_write_same
is set.  Because REQ_OP_WRITE_ZEROES is implemented in terms of WRITE
SAME, blkdev_issue_zeroout() may fail with -EREMOTEIO:

  $ fallocate -zn -l 1k /dev/sdg
  fallocate: fallocate failed: Remote I/O error
  $ fallocate -zn -l 1k /dev/sdg  # OK
  $ fallocate -zn -l 1k /dev/sdg  # OK

The following calls succeed because sd_done() sets ->no_write_same in
response to a sense that would become BLK_STS_TARGET/-EREMOTEIO, causing
__blkdev_issue_zeroout() to fall back to generating ZERO_PAGE bios.

This means blkdev_issue_zeroout() must cope with WRITE ZEROES failing
and fall back to manually zeroing, unless BLKDEV_ZERO_NOFALLBACK is
specified.  For BLKDEV_ZERO_NOFALLBACK case, return -EOPNOTSUPP if
sd_done() has just set ->no_write_same thus indicating lack of offload
support.

Fixes: c20cfc27a473 ("block: stop using blkdev_issue_write_same for zeroing")
Cc: Hannes Reinecke <[hidden email]>
Reviewed-by: Christoph Hellwig <[hidden email]>
Reviewed-by: Martin K. Petersen <[hidden email]>
Signed-off-by: Ilya Dryomov <[hidden email]>
Signed-off-by: Jens Axboe <[hidden email]>
---
 block/blk-lib.c | 45 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 35 insertions(+), 10 deletions(-)

diff --git a/block/blk-lib.c b/block/blk-lib.c
index 4ef942c..595517c 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -321,12 +321,6 @@ static int __blkdev_issue_zero_pages(struct block_device *bdev,
  *  Zero-fill a block range, either using hardware offload or by explicitly
  *  writing zeroes to the device.
  *
- *  Note that this function may fail with -EOPNOTSUPP if the driver signals
- *  zeroing offload support, but the device fails to process the command (for
- *  some devices there is no non-destructive way to verify whether this
- *  operation is actually supported).  In this case the caller should call
- *  retry the call to blkdev_issue_zeroout() and the fallback path will be used.
- *
  *  If a device is using logical block provisioning, the underlying space will
  *  not be released if %flags contains BLKDEV_ZERO_NOUNMAP.
  *
@@ -370,18 +364,49 @@ EXPORT_SYMBOL(__blkdev_issue_zeroout);
 int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
  sector_t nr_sects, gfp_t gfp_mask, unsigned flags)
 {
- int ret;
- struct bio *bio = NULL;
+ int ret = 0;
+ sector_t bs_mask;
+ struct bio *bio;
  struct blk_plug plug;
+ bool try_write_zeroes = !!bdev_write_zeroes_sectors(bdev);
 
+ bs_mask = (bdev_logical_block_size(bdev) >> 9) - 1;
+ if ((sector | nr_sects) & bs_mask)
+ return -EINVAL;
+
+retry:
+ bio = NULL;
  blk_start_plug(&plug);
- ret = __blkdev_issue_zeroout(bdev, sector, nr_sects, gfp_mask,
- &bio, flags);
+ if (try_write_zeroes) {
+ ret = __blkdev_issue_write_zeroes(bdev, sector, nr_sects,
+  gfp_mask, &bio, flags);
+ } else if (!(flags & BLKDEV_ZERO_NOFALLBACK)) {
+ ret = __blkdev_issue_zero_pages(bdev, sector, nr_sects,
+ gfp_mask, &bio);
+ } else {
+ /* No zeroing offload support */
+ ret = -EOPNOTSUPP;
+ }
  if (ret == 0 && bio) {
  ret = submit_bio_wait(bio);
  bio_put(bio);
  }
  blk_finish_plug(&plug);
+ if (ret && try_write_zeroes) {
+ if (!(flags & BLKDEV_ZERO_NOFALLBACK)) {
+ try_write_zeroes = false;
+ goto retry;
+ }
+ if (!bdev_write_zeroes_sectors(bdev)) {
+ /*
+ * Zeroing offload support was indicated, but the
+ * device reported ILLEGAL REQUEST (for some devices
+ * there is no non-destructive way to verify whether
+ * WRITE ZEROES is actually supported).
+ */
+ ret = -EOPNOTSUPP;
+ }
+ }
 
  return ret;
 }
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2][SRU][ARTFUL] block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()

Kleber Sacilotto de Souza
On 11/21/17 18:05, Colin King wrote:

> From: Ilya Dryomov <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1726818
>
> sd_config_write_same() ignores ->max_ws_blocks == 0 and resets it to
> permit trying WRITE SAME on older SCSI devices, unless ->no_write_same
> is set.  Because REQ_OP_WRITE_ZEROES is implemented in terms of WRITE
> SAME, blkdev_issue_zeroout() may fail with -EREMOTEIO:
>
>   $ fallocate -zn -l 1k /dev/sdg
>   fallocate: fallocate failed: Remote I/O error
>   $ fallocate -zn -l 1k /dev/sdg  # OK
>   $ fallocate -zn -l 1k /dev/sdg  # OK
>
> The following calls succeed because sd_done() sets ->no_write_same in
> response to a sense that would become BLK_STS_TARGET/-EREMOTEIO, causing
> __blkdev_issue_zeroout() to fall back to generating ZERO_PAGE bios.
>
> This means blkdev_issue_zeroout() must cope with WRITE ZEROES failing
> and fall back to manually zeroing, unless BLKDEV_ZERO_NOFALLBACK is
> specified.  For BLKDEV_ZERO_NOFALLBACK case, return -EOPNOTSUPP if
> sd_done() has just set ->no_write_same thus indicating lack of offload
> support.
>
> Fixes: c20cfc27a473 ("block: stop using blkdev_issue_write_same for zeroing")
> Cc: Hannes Reinecke <[hidden email]>
> Reviewed-by: Christoph Hellwig <[hidden email]>
> Reviewed-by: Martin K. Petersen <[hidden email]>
> Signed-off-by: Ilya Dryomov <[hidden email]>
> Signed-off-by: Jens Axboe <[hidden email]>

Hi Colin,

This patch is missing the "backported/cherry picked from" tag and your SOB.

The first patch of the series has you as the author, but mainline commit
425a4dba7953e35ffd096771973add6d2f40d2ed has "Ilya Dryomov
<[hidden email]>" as author. This we can fix when applying the
patch, but the SOB for the second one needs to come from you.

Thanks,
Kleber

> ---
>  block/blk-lib.c | 45 +++++++++++++++++++++++++++++++++++----------
>  1 file changed, 35 insertions(+), 10 deletions(-)
>
> diff --git a/block/blk-lib.c b/block/blk-lib.c
> index 4ef942c..595517c 100644
> --- a/block/blk-lib.c
> +++ b/block/blk-lib.c
> @@ -321,12 +321,6 @@ static int __blkdev_issue_zero_pages(struct block_device *bdev,
>   *  Zero-fill a block range, either using hardware offload or by explicitly
>   *  writing zeroes to the device.
>   *
> - *  Note that this function may fail with -EOPNOTSUPP if the driver signals
> - *  zeroing offload support, but the device fails to process the command (for
> - *  some devices there is no non-destructive way to verify whether this
> - *  operation is actually supported).  In this case the caller should call
> - *  retry the call to blkdev_issue_zeroout() and the fallback path will be used.
> - *
>   *  If a device is using logical block provisioning, the underlying space will
>   *  not be released if %flags contains BLKDEV_ZERO_NOUNMAP.
>   *
> @@ -370,18 +364,49 @@ EXPORT_SYMBOL(__blkdev_issue_zeroout);
>  int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
>   sector_t nr_sects, gfp_t gfp_mask, unsigned flags)
>  {
> - int ret;
> - struct bio *bio = NULL;
> + int ret = 0;
> + sector_t bs_mask;
> + struct bio *bio;
>   struct blk_plug plug;
> + bool try_write_zeroes = !!bdev_write_zeroes_sectors(bdev);
>  
> + bs_mask = (bdev_logical_block_size(bdev) >> 9) - 1;
> + if ((sector | nr_sects) & bs_mask)
> + return -EINVAL;
> +
> +retry:
> + bio = NULL;
>   blk_start_plug(&plug);
> - ret = __blkdev_issue_zeroout(bdev, sector, nr_sects, gfp_mask,
> - &bio, flags);
> + if (try_write_zeroes) {
> + ret = __blkdev_issue_write_zeroes(bdev, sector, nr_sects,
> +  gfp_mask, &bio, flags);
> + } else if (!(flags & BLKDEV_ZERO_NOFALLBACK)) {
> + ret = __blkdev_issue_zero_pages(bdev, sector, nr_sects,
> + gfp_mask, &bio);
> + } else {
> + /* No zeroing offload support */
> + ret = -EOPNOTSUPP;
> + }
>   if (ret == 0 && bio) {
>   ret = submit_bio_wait(bio);
>   bio_put(bio);
>   }
>   blk_finish_plug(&plug);
> + if (ret && try_write_zeroes) {
> + if (!(flags & BLKDEV_ZERO_NOFALLBACK)) {
> + try_write_zeroes = false;
> + goto retry;
> + }
> + if (!bdev_write_zeroes_sectors(bdev)) {
> + /*
> + * Zeroing offload support was indicated, but the
> + * device reported ILLEGAL REQUEST (for some devices
> + * there is no non-destructive way to verify whether
> + * WRITE ZEROES is actually supported).
> + */
> + ret = -EOPNOTSUPP;
> + }
> + }
>  
>   return ret;
>  }
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2][SRU][ARTFUL] block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()

Colin Ian King-2
On 05/01/18 15:22, Kleber Souza wrote:

> On 11/21/17 18:05, Colin King wrote:
>> From: Ilya Dryomov <[hidden email]>
>>
>> BugLink: https://bugs.launchpad.net/bugs/1726818
>>
>> sd_config_write_same() ignores ->max_ws_blocks == 0 and resets it to
>> permit trying WRITE SAME on older SCSI devices, unless ->no_write_same
>> is set.  Because REQ_OP_WRITE_ZEROES is implemented in terms of WRITE
>> SAME, blkdev_issue_zeroout() may fail with -EREMOTEIO:
>>
>>   $ fallocate -zn -l 1k /dev/sdg
>>   fallocate: fallocate failed: Remote I/O error
>>   $ fallocate -zn -l 1k /dev/sdg  # OK
>>   $ fallocate -zn -l 1k /dev/sdg  # OK
>>
>> The following calls succeed because sd_done() sets ->no_write_same in
>> response to a sense that would become BLK_STS_TARGET/-EREMOTEIO, causing
>> __blkdev_issue_zeroout() to fall back to generating ZERO_PAGE bios.
>>
>> This means blkdev_issue_zeroout() must cope with WRITE ZEROES failing
>> and fall back to manually zeroing, unless BLKDEV_ZERO_NOFALLBACK is
>> specified.  For BLKDEV_ZERO_NOFALLBACK case, return -EOPNOTSUPP if
>> sd_done() has just set ->no_write_same thus indicating lack of offload
>> support.
>>
>> Fixes: c20cfc27a473 ("block: stop using blkdev_issue_write_same for zeroing")
>> Cc: Hannes Reinecke <[hidden email]>
>> Reviewed-by: Christoph Hellwig <[hidden email]>
>> Reviewed-by: Martin K. Petersen <[hidden email]>
>> Signed-off-by: Ilya Dryomov <[hidden email]>
>> Signed-off-by: Jens Axboe <[hidden email]>
>
> Hi Colin,
>
> This patch is missing the "backported/cherry picked from" tag and your SOB.

Do you mind adding the following lines before applying patch:

(cherry picked from upstream commit
d5ce4c31d6df518dd8f63bbae20d7423c5018a6c)
Signed-off-by: Colin Ian King <[hidden email]>

>
> The first patch of the series has you as the author, but mainline commit
> 425a4dba7953e35ffd096771973add6d2f40d2ed has "Ilya Dryomov
> <[hidden email]>" as author. This we can fix when applying the
> patch, but the SOB for the second one needs to come from you.
>
> Thanks,
> Kleber
>
>> ---
>>  block/blk-lib.c | 45 +++++++++++++++++++++++++++++++++++----------
>>  1 file changed, 35 insertions(+), 10 deletions(-)
>>
>> diff --git a/block/blk-lib.c b/block/blk-lib.c
>> index 4ef942c..595517c 100644
>> --- a/block/blk-lib.c
>> +++ b/block/blk-lib.c
>> @@ -321,12 +321,6 @@ static int __blkdev_issue_zero_pages(struct block_device *bdev,
>>   *  Zero-fill a block range, either using hardware offload or by explicitly
>>   *  writing zeroes to the device.
>>   *
>> - *  Note that this function may fail with -EOPNOTSUPP if the driver signals
>> - *  zeroing offload support, but the device fails to process the command (for
>> - *  some devices there is no non-destructive way to verify whether this
>> - *  operation is actually supported).  In this case the caller should call
>> - *  retry the call to blkdev_issue_zeroout() and the fallback path will be used.
>> - *
>>   *  If a device is using logical block provisioning, the underlying space will
>>   *  not be released if %flags contains BLKDEV_ZERO_NOUNMAP.
>>   *
>> @@ -370,18 +364,49 @@ EXPORT_SYMBOL(__blkdev_issue_zeroout);
>>  int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
>>   sector_t nr_sects, gfp_t gfp_mask, unsigned flags)
>>  {
>> - int ret;
>> - struct bio *bio = NULL;
>> + int ret = 0;
>> + sector_t bs_mask;
>> + struct bio *bio;
>>   struct blk_plug plug;
>> + bool try_write_zeroes = !!bdev_write_zeroes_sectors(bdev);
>>  
>> + bs_mask = (bdev_logical_block_size(bdev) >> 9) - 1;
>> + if ((sector | nr_sects) & bs_mask)
>> + return -EINVAL;
>> +
>> +retry:
>> + bio = NULL;
>>   blk_start_plug(&plug);
>> - ret = __blkdev_issue_zeroout(bdev, sector, nr_sects, gfp_mask,
>> - &bio, flags);
>> + if (try_write_zeroes) {
>> + ret = __blkdev_issue_write_zeroes(bdev, sector, nr_sects,
>> +  gfp_mask, &bio, flags);
>> + } else if (!(flags & BLKDEV_ZERO_NOFALLBACK)) {
>> + ret = __blkdev_issue_zero_pages(bdev, sector, nr_sects,
>> + gfp_mask, &bio);
>> + } else {
>> + /* No zeroing offload support */
>> + ret = -EOPNOTSUPP;
>> + }
>>   if (ret == 0 && bio) {
>>   ret = submit_bio_wait(bio);
>>   bio_put(bio);
>>   }
>>   blk_finish_plug(&plug);
>> + if (ret && try_write_zeroes) {
>> + if (!(flags & BLKDEV_ZERO_NOFALLBACK)) {
>> + try_write_zeroes = false;
>> + goto retry;
>> + }
>> + if (!bdev_write_zeroes_sectors(bdev)) {
>> + /*
>> + * Zeroing offload support was indicated, but the
>> + * device reported ILLEGAL REQUEST (for some devices
>> + * there is no non-destructive way to verify whether
>> + * WRITE ZEROES is actually supported).
>> + */
>> + ret = -EOPNOTSUPP;
>> + }
>> + }
>>  
>>   return ret;
>>  }
>>
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/cmnt: [PATCH 0/2][SRU][ARTFUL] Fix VFS warning and file system resize failure (LP: #1726818)

Kleber Sacilotto de Souza
In reply to this post by Colin Ian King-2
On 11/21/17 18:05, Colin King wrote:

> From: Colin Ian King <[hidden email]>
>
> The following two patches fix a VFS warning and a file system resize failure
> on Artful when booting a virtual image using a vagrant/VirtualBox combo.
>
> [SRU Justfication, Artful]
>
> [Impact]
>
> Booting the 4.13 Artful kernel with vagrant using VirtualBox trips the warning:
>
> [ 61.010337] VFS: brelse: Trying to free free buffer
> [ 61.114875] ------------[ cut here ]------------
> [ 61.114886] WARNING: CPU: 0 PID: 683 at /build/linux-XO_uEE/linux-4.13.0/fs/buffer.c:1205 __brelse+0x21/0x30
>
> and a failed resize of a partition. The root cause has been bisected down
> to the following commmit:
>
> commit c20cfc27a47307e811346f85959cf3cc07ae42f9
> Author: Christoph Hellwig <[hidden email]>
> Date: Wed Apr 5 19:21:07 2017 +0200
>
>     block: stop using blkdev_issue_write_same for zeroing
>
> [Fix]
> The Upstream commit directly fixes this issue:
>
> commit d5ce4c31d6df518dd8f63bbae20d7423c5018a6c
> Author: Ilya Dryomov <[hidden email]>
> Date: Mon Oct 16 15:59:10 2017 +0200
>
>     block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()
>
> ..however we also require a backport of the following upstream commit to apply
> the above commit cleanly:
>
> commit 425a4dba7953e35ffd096771973add6d2f40d2ed
> Author: Ilya Dryomov <[hidden email]>
> Date: Mon Oct 16 15:59:09 2017 +0200
>
>     block: factor out __blkdev_issue_zero_pages()
>
> [Testscase]
>
> On Ubuntu Xenial:
>
> 1. sudo apt-get install virtualbox vagrant
> 2. edit /etc/group and add one's user name to the vboxusers group
> 3. log out log back
> 4. vagrant init ubuntu/artful64
> 5. vagrant up
> 6. vagrant ssh
> 7. dmesg | grep "VFS: brelse"
>
> without the fix one will see the VFS brelse warning message and the /
> partition will not have been resized.
>
> with a fixed system there is is no VFS vbrelse warning and / as been
> resized as expected.
>
> [Regresion potential]
> These patches touch the blk library so potentially it could break the block
> layer and corrupt data on disk. However these are upstream fixes that address
> the buggy commit c20cfc27a47307e811346f85959cf3cc07ae42f9 and are known to
> address the bug.
>
> Colin Ian King (1):
>   block: factor out __blkdev_issue_zero_pages()
>
> Ilya Dryomov (1):
>   block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()
>
>  block/blk-lib.c | 108 +++++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 72 insertions(+), 36 deletions(-)
>

Backports look good:
Acked-by: Kleber Sacilotto de Souza <[hidden email]>

Please note that the following changes are needed when applying the patches:
- Fix the original author of the first patch.
- Add SOB and upstream sha1 lines on the second patch.


Thank,
Kleber

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [PATCH 0/2][SRU][ARTFUL] Fix VFS warning and file system resize failure (LP: #1726818)

Khalid Elmously
In reply to this post by Colin Ian King-2

On 2017-11-21 17:05:26 , Colin King wrote:

> From: Colin Ian King <[hidden email]>
>
> The following two patches fix a VFS warning and a file system resize failure
> on Artful when booting a virtual image using a vagrant/VirtualBox combo.
>
> [SRU Justfication, Artful]
>
> [Impact]
>
> Booting the 4.13 Artful kernel with vagrant using VirtualBox trips the warning:
>
> [ 61.010337] VFS: brelse: Trying to free free buffer
> [ 61.114875] ------------[ cut here ]------------
> [ 61.114886] WARNING: CPU: 0 PID: 683 at /build/linux-XO_uEE/linux-4.13.0/fs/buffer.c:1205 __brelse+0x21/0x30
>
> and a failed resize of a partition. The root cause has been bisected down
> to the following commmit:
>
> commit c20cfc27a47307e811346f85959cf3cc07ae42f9
> Author: Christoph Hellwig <[hidden email]>
> Date: Wed Apr 5 19:21:07 2017 +0200
>
>     block: stop using blkdev_issue_write_same for zeroing
>
> [Fix]
> The Upstream commit directly fixes this issue:
>
> commit d5ce4c31d6df518dd8f63bbae20d7423c5018a6c
> Author: Ilya Dryomov <[hidden email]>
> Date: Mon Oct 16 15:59:10 2017 +0200
>
>     block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()
>
> ..however we also require a backport of the following upstream commit to apply
> the above commit cleanly:
>
> commit 425a4dba7953e35ffd096771973add6d2f40d2ed
> Author: Ilya Dryomov <[hidden email]>
> Date: Mon Oct 16 15:59:09 2017 +0200
>
>     block: factor out __blkdev_issue_zero_pages()
>
> [Testscase]
>
> On Ubuntu Xenial:
>
> 1. sudo apt-get install virtualbox vagrant
> 2. edit /etc/group and add one's user name to the vboxusers group
> 3. log out log back
> 4. vagrant init ubuntu/artful64
> 5. vagrant up
> 6. vagrant ssh
> 7. dmesg | grep "VFS: brelse"
>
> without the fix one will see the VFS brelse warning message and the /
> partition will not have been resized.
>
> with a fixed system there is is no VFS vbrelse warning and / as been
> resized as expected.
>
> [Regresion potential]
> These patches touch the blk library so potentially it could break the block
> layer and corrupt data on disk. However these are upstream fixes that address
> the buggy commit c20cfc27a47307e811346f85959cf3cc07ae42f9 and are known to
> address the bug.
>
> Colin Ian King (1):
>   block: factor out __blkdev_issue_zero_pages()
>
> Ilya Dryomov (1):
>   block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()
>
>  block/blk-lib.c | 108 +++++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 72 insertions(+), 36 deletions(-)
>
Acked-by: Khalid Elmously <[hidden email]>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED: [PATCH 0/2][SRU][ARTFUL] Fix VFS warning and file system resize failure (LP: #1726818)

Khalid Elmously
In reply to this post by Colin Ian King-2
Applied to artful

On 2017-11-21 17:05:26 , Colin King wrote:

> From: Colin Ian King <[hidden email]>
>
> The following two patches fix a VFS warning and a file system resize failure
> on Artful when booting a virtual image using a vagrant/VirtualBox combo.
>
> [SRU Justfication, Artful]
>
> [Impact]
>
> Booting the 4.13 Artful kernel with vagrant using VirtualBox trips the warning:
>
> [ 61.010337] VFS: brelse: Trying to free free buffer
> [ 61.114875] ------------[ cut here ]------------
> [ 61.114886] WARNING: CPU: 0 PID: 683 at /build/linux-XO_uEE/linux-4.13.0/fs/buffer.c:1205 __brelse+0x21/0x30
>
> and a failed resize of a partition. The root cause has been bisected down
> to the following commmit:
>
> commit c20cfc27a47307e811346f85959cf3cc07ae42f9
> Author: Christoph Hellwig <[hidden email]>
> Date: Wed Apr 5 19:21:07 2017 +0200
>
>     block: stop using blkdev_issue_write_same for zeroing
>
> [Fix]
> The Upstream commit directly fixes this issue:
>
> commit d5ce4c31d6df518dd8f63bbae20d7423c5018a6c
> Author: Ilya Dryomov <[hidden email]>
> Date: Mon Oct 16 15:59:10 2017 +0200
>
>     block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()
>
> ..however we also require a backport of the following upstream commit to apply
> the above commit cleanly:
>
> commit 425a4dba7953e35ffd096771973add6d2f40d2ed
> Author: Ilya Dryomov <[hidden email]>
> Date: Mon Oct 16 15:59:09 2017 +0200
>
>     block: factor out __blkdev_issue_zero_pages()
>
> [Testscase]
>
> On Ubuntu Xenial:
>
> 1. sudo apt-get install virtualbox vagrant
> 2. edit /etc/group and add one's user name to the vboxusers group
> 3. log out log back
> 4. vagrant init ubuntu/artful64
> 5. vagrant up
> 6. vagrant ssh
> 7. dmesg | grep "VFS: brelse"
>
> without the fix one will see the VFS brelse warning message and the /
> partition will not have been resized.
>
> with a fixed system there is is no VFS vbrelse warning and / as been
> resized as expected.
>
> [Regresion potential]
> These patches touch the blk library so potentially it could break the block
> layer and corrupt data on disk. However these are upstream fixes that address
> the buggy commit c20cfc27a47307e811346f85959cf3cc07ae42f9 and are known to
> address the bug.
>
> Colin Ian King (1):
>   block: factor out __blkdev_issue_zero_pages()
>
> Ilya Dryomov (1):
>   block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()
>
>  block/blk-lib.c | 108 +++++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 72 insertions(+), 36 deletions(-)
>
> --
> 2.7.4
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: APPLIED: [PATCH 0/2][SRU][ARTFUL] Fix VFS warning and file system resize failure (LP: #1726818)

Kleber Sacilotto de Souza
Hi Khaled,

Commit for patch 1/2 is missing the fix for the original author. Commit
for patch 2/2 is missing the SOB from Colin and the upstream sha1 line.
Both commits are missing your ACK (the commit should contain all the
ACK's from the mailing-list plus the SOB from the person applying it,
even if the same person also ACK'ed or is the original author or
submitter). The patches were applied in the wrong order.


Thanks,
Kleber

On 02/01/18 21:26, Khaled Elmously wrote:

> Applied to artful
>
> On 2017-11-21 17:05:26 , Colin King wrote:
>> From: Colin Ian King <[hidden email]>
>>
>> The following two patches fix a VFS warning and a file system resize failure
>> on Artful when booting a virtual image using a vagrant/VirtualBox combo.
>>
>> [SRU Justfication, Artful]
>>
>> [Impact]
>>
>> Booting the 4.13 Artful kernel with vagrant using VirtualBox trips the warning:
>>
>> [ 61.010337] VFS: brelse: Trying to free free buffer
>> [ 61.114875] ------------[ cut here ]------------
>> [ 61.114886] WARNING: CPU: 0 PID: 683 at /build/linux-XO_uEE/linux-4.13.0/fs/buffer.c:1205 __brelse+0x21/0x30
>>
>> and a failed resize of a partition. The root cause has been bisected down
>> to the following commmit:
>>
>> commit c20cfc27a47307e811346f85959cf3cc07ae42f9
>> Author: Christoph Hellwig <[hidden email]>
>> Date: Wed Apr 5 19:21:07 2017 +0200
>>
>>     block: stop using blkdev_issue_write_same for zeroing
>>
>> [Fix]
>> The Upstream commit directly fixes this issue:
>>
>> commit d5ce4c31d6df518dd8f63bbae20d7423c5018a6c
>> Author: Ilya Dryomov <[hidden email]>
>> Date: Mon Oct 16 15:59:10 2017 +0200
>>
>>     block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()
>>
>> ..however we also require a backport of the following upstream commit to apply
>> the above commit cleanly:
>>
>> commit 425a4dba7953e35ffd096771973add6d2f40d2ed
>> Author: Ilya Dryomov <[hidden email]>
>> Date: Mon Oct 16 15:59:09 2017 +0200
>>
>>     block: factor out __blkdev_issue_zero_pages()
>>
>> [Testscase]
>>
>> On Ubuntu Xenial:
>>
>> 1. sudo apt-get install virtualbox vagrant
>> 2. edit /etc/group and add one's user name to the vboxusers group
>> 3. log out log back
>> 4. vagrant init ubuntu/artful64
>> 5. vagrant up
>> 6. vagrant ssh
>> 7. dmesg | grep "VFS: brelse"
>>
>> without the fix one will see the VFS brelse warning message and the /
>> partition will not have been resized.
>>
>> with a fixed system there is is no VFS vbrelse warning and / as been
>> resized as expected.
>>
>> [Regresion potential]
>> These patches touch the blk library so potentially it could break the block
>> layer and corrupt data on disk. However these are upstream fixes that address
>> the buggy commit c20cfc27a47307e811346f85959cf3cc07ae42f9 and are known to
>> address the bug.
>>
>> Colin Ian King (1):
>>   block: factor out __blkdev_issue_zero_pages()
>>
>> Ilya Dryomov (1):
>>   block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()
>>
>>  block/blk-lib.c | 108 +++++++++++++++++++++++++++++++++++++-------------------
>>  1 file changed, 72 insertions(+), 36 deletions(-)
>>
>> --
>> 2.7.4
>>
>>
>> --
>> kernel-team mailing list
>> [hidden email]
>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: APPLIED: [PATCH 0/2][SRU][ARTFUL] Fix VFS warning and file system resize failure (LP: #1726818)

Khalid Elmously
On 2018-02-02 12:00:34 , Kleber Souza wrote:
> Hi Khaled,
>
> Commit for patch 1/2 is missing the fix for the original author. Commit
> for patch 2/2 is missing the SOB from Colin and the upstream sha1 line.
> Both commits are missing your ACK (the commit should contain all the
> ACK's from the mailing-list plus the SOB from the person applying it,
> even if the same person also ACK'ed or is the original author or
> submitter).

"..even if the same person also ACK'ed" I think this is the part where I may have received conflicting information. I will try to lean more towards your method of doing things since it appears you feel more strongly about it than Kamal does :) (also more information is generally better)


> The patches were applied in the wrong order

This is also something that I had discussed with Kamal. I had to go back in my git history and amend a few commits so the order of commits changed. I had asked if order is important and he stated it's not. However, now that I think about it, he may have been referring to orders of *patchsets*, not order of patches _within_ a patchset.

I will be more mindful of that in the future, thanks.


-Khaled


>
>
> Thanks,
> Kleber
>
> On 02/01/18 21:26, Khaled Elmously wrote:
> > Applied to artful
> >
> > On 2017-11-21 17:05:26 , Colin King wrote:
> >> From: Colin Ian King <[hidden email]>
> >>
> >> The following two patches fix a VFS warning and a file system resize failure
> >> on Artful when booting a virtual image using a vagrant/VirtualBox combo.
> >>
> >> [SRU Justfication, Artful]
> >>
> >> [Impact]
> >>
> >> Booting the 4.13 Artful kernel with vagrant using VirtualBox trips the warning:
> >>
> >> [ 61.010337] VFS: brelse: Trying to free free buffer
> >> [ 61.114875] ------------[ cut here ]------------
> >> [ 61.114886] WARNING: CPU: 0 PID: 683 at /build/linux-XO_uEE/linux-4.13.0/fs/buffer.c:1205 __brelse+0x21/0x30
> >>
> >> and a failed resize of a partition. The root cause has been bisected down
> >> to the following commmit:
> >>
> >> commit c20cfc27a47307e811346f85959cf3cc07ae42f9
> >> Author: Christoph Hellwig <[hidden email]>
> >> Date: Wed Apr 5 19:21:07 2017 +0200
> >>
> >>     block: stop using blkdev_issue_write_same for zeroing
> >>
> >> [Fix]
> >> The Upstream commit directly fixes this issue:
> >>
> >> commit d5ce4c31d6df518dd8f63bbae20d7423c5018a6c
> >> Author: Ilya Dryomov <[hidden email]>
> >> Date: Mon Oct 16 15:59:10 2017 +0200
> >>
> >>     block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()
> >>
> >> ..however we also require a backport of the following upstream commit to apply
> >> the above commit cleanly:
> >>
> >> commit 425a4dba7953e35ffd096771973add6d2f40d2ed
> >> Author: Ilya Dryomov <[hidden email]>
> >> Date: Mon Oct 16 15:59:09 2017 +0200
> >>
> >>     block: factor out __blkdev_issue_zero_pages()
> >>
> >> [Testscase]
> >>
> >> On Ubuntu Xenial:
> >>
> >> 1. sudo apt-get install virtualbox vagrant
> >> 2. edit /etc/group and add one's user name to the vboxusers group
> >> 3. log out log back
> >> 4. vagrant init ubuntu/artful64
> >> 5. vagrant up
> >> 6. vagrant ssh
> >> 7. dmesg | grep "VFS: brelse"
> >>
> >> without the fix one will see the VFS brelse warning message and the /
> >> partition will not have been resized.
> >>
> >> with a fixed system there is is no VFS vbrelse warning and / as been
> >> resized as expected.
> >>
> >> [Regresion potential]
> >> These patches touch the blk library so potentially it could break the block
> >> layer and corrupt data on disk. However these are upstream fixes that address
> >> the buggy commit c20cfc27a47307e811346f85959cf3cc07ae42f9 and are known to
> >> address the bug.
> >>
> >> Colin Ian King (1):
> >>   block: factor out __blkdev_issue_zero_pages()
> >>
> >> Ilya Dryomov (1):
> >>   block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()
> >>
> >>  block/blk-lib.c | 108 +++++++++++++++++++++++++++++++++++++-------------------
> >>  1 file changed, 72 insertions(+), 36 deletions(-)
> >>
> >> --
> >> 2.7.4
> >>
> >>
> >> --
> >> kernel-team mailing list
> >> [hidden email]
> >> https://lists.ubuntu.com/mailman/listinfo/kernel-team
> >

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: APPLIED: [PATCH 0/2][SRU][ARTFUL] Fix VFS warning and file system resize failure (LP: #1726818)

Khalid Elmously
On 2018-02-02 10:28:43 , Khaled Elmously wrote:

> On 2018-02-02 12:00:34 , Kleber Souza wrote:
> > Hi Khaled,
> >
> > Commit for patch 1/2 is missing the fix for the original author. Commit
> > for patch 2/2 is missing the SOB from Colin and the upstream sha1 line.
> > Both commits are missing your ACK (the commit should contain all the
> > ACK's from the mailing-list plus the SOB from the person applying it,
> > even if the same person also ACK'ed or is the original author or
> > submitter).
>
> "..even if the same person also ACK'ed" I think this is the part where I may have received conflicting information. I will try to lean more towards your method of doing things since it appears you feel more strongly about it than Kamal does :) (also more information is generally better)
>
>
> > The patches were applied in the wrong order
>
> This is also something that I had discussed with Kamal. I had to go back in my git history and amend a few commits so the order of commits changed. I had asked if order is important and he stated it's not. However, now that I think about it, he may have been referring to orders of *patchsets*, not order of patches _within_ a patchset.
>

I've fixed the order of these 2 patches (and Kamal pushed it to master-next-backlog), so at least the order should be good now.

Thanks
-Khaled

> I will be more mindful of that in the future, thanks.
>
>
> -Khaled
>
>
> >
> >
> > Thanks,
> > Kleber
> >
> > On 02/01/18 21:26, Khaled Elmously wrote:
> > > Applied to artful
> > >
> > > On 2017-11-21 17:05:26 , Colin King wrote:
> > >> From: Colin Ian King <[hidden email]>
> > >>
> > >> The following two patches fix a VFS warning and a file system resize failure
> > >> on Artful when booting a virtual image using a vagrant/VirtualBox combo.
> > >>
> > >> [SRU Justfication, Artful]
> > >>
> > >> [Impact]
> > >>
> > >> Booting the 4.13 Artful kernel with vagrant using VirtualBox trips the warning:
> > >>
> > >> [ 61.010337] VFS: brelse: Trying to free free buffer
> > >> [ 61.114875] ------------[ cut here ]------------
> > >> [ 61.114886] WARNING: CPU: 0 PID: 683 at /build/linux-XO_uEE/linux-4.13.0/fs/buffer.c:1205 __brelse+0x21/0x30
> > >>
> > >> and a failed resize of a partition. The root cause has been bisected down
> > >> to the following commmit:
> > >>
> > >> commit c20cfc27a47307e811346f85959cf3cc07ae42f9
> > >> Author: Christoph Hellwig <[hidden email]>
> > >> Date: Wed Apr 5 19:21:07 2017 +0200
> > >>
> > >>     block: stop using blkdev_issue_write_same for zeroing
> > >>
> > >> [Fix]
> > >> The Upstream commit directly fixes this issue:
> > >>
> > >> commit d5ce4c31d6df518dd8f63bbae20d7423c5018a6c
> > >> Author: Ilya Dryomov <[hidden email]>
> > >> Date: Mon Oct 16 15:59:10 2017 +0200
> > >>
> > >>     block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()
> > >>
> > >> ..however we also require a backport of the following upstream commit to apply
> > >> the above commit cleanly:
> > >>
> > >> commit 425a4dba7953e35ffd096771973add6d2f40d2ed
> > >> Author: Ilya Dryomov <[hidden email]>
> > >> Date: Mon Oct 16 15:59:09 2017 +0200
> > >>
> > >>     block: factor out __blkdev_issue_zero_pages()
> > >>
> > >> [Testscase]
> > >>
> > >> On Ubuntu Xenial:
> > >>
> > >> 1. sudo apt-get install virtualbox vagrant
> > >> 2. edit /etc/group and add one's user name to the vboxusers group
> > >> 3. log out log back
> > >> 4. vagrant init ubuntu/artful64
> > >> 5. vagrant up
> > >> 6. vagrant ssh
> > >> 7. dmesg | grep "VFS: brelse"
> > >>
> > >> without the fix one will see the VFS brelse warning message and the /
> > >> partition will not have been resized.
> > >>
> > >> with a fixed system there is is no VFS vbrelse warning and / as been
> > >> resized as expected.
> > >>
> > >> [Regresion potential]
> > >> These patches touch the blk library so potentially it could break the block
> > >> layer and corrupt data on disk. However these are upstream fixes that address
> > >> the buggy commit c20cfc27a47307e811346f85959cf3cc07ae42f9 and are known to
> > >> address the bug.
> > >>
> > >> Colin Ian King (1):
> > >>   block: factor out __blkdev_issue_zero_pages()
> > >>
> > >> Ilya Dryomov (1):
> > >>   block: cope with WRITE ZEROES failing in blkdev_issue_zeroout()
> > >>
> > >>  block/blk-lib.c | 108 +++++++++++++++++++++++++++++++++++++-------------------
> > >>  1 file changed, 72 insertions(+), 36 deletions(-)
> > >>
> > >> --
> > >> 2.7.4
> > >>
> > >>
> > >> --
> > >> kernel-team mailing list
> > >> [hidden email]
> > >> https://lists.ubuntu.com/mailman/listinfo/kernel-team
> > >

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team