* [PATCH] fs: guard_bio_eod() needs to consider partitions
@ 2017-10-24 1:10 Greg Edwards
2017-10-24 2:16 ` Al Viro
2017-10-24 17:21 ` [PATCH v2] " Greg Edwards
0 siblings, 2 replies; 8+ messages in thread
From: Greg Edwards @ 2017-10-24 1:10 UTC (permalink / raw)
To: linux-fsdevel
Cc: Christoph Hellwig, Alexander Viro, linux-kernel, Greg Edwards
guard_bio_eod() needs to look at the partition capacity, not just the
capacity of the whole device, when determining if truncation is
necessary.
[ 60.268688] attempt to access beyond end of device
[ 60.268690] unknown-block(9,1): rw=0, want=67103509, limit=67103506
[ 60.268693] buffer_io_error: 2 callbacks suppressed
[ 60.268696] Buffer I/O error on dev md1p7, logical block 4524305, async page read
Fixes: 74d46992e0d9 ("block: replace bi_bdev with a gendisk pointer and partitions index")
Signed-off-by: Greg Edwards <gedwards@ddn.com>
---
fs/buffer.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/fs/buffer.c b/fs/buffer.c
index 170df856bdb9..989d3b085efe 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -3055,8 +3055,15 @@ void guard_bio_eod(int op, struct bio *bio)
sector_t maxsector;
struct bio_vec *bvec = &bio->bi_io_vec[bio->bi_vcnt - 1];
unsigned truncated_bytes;
+ struct hd_struct *part;
+
+ part = disk_get_part(bio->bi_disk, bio->bi_partno);
+ if (part)
+ maxsector = part_nr_sects_read(part);
+ else
+ maxsector = get_capacity(bio->bi_disk);
+ disk_put_part(part);
- maxsector = get_capacity(bio->bi_disk);
if (!maxsector)
return;
--
2.13.6
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] fs: guard_bio_eod() needs to consider partitions
2017-10-24 1:10 [PATCH] fs: guard_bio_eod() needs to consider partitions Greg Edwards
@ 2017-10-24 2:16 ` Al Viro
2017-10-24 7:46 ` Christoph Hellwig
2017-10-24 17:21 ` [PATCH v2] " Greg Edwards
1 sibling, 1 reply; 8+ messages in thread
From: Al Viro @ 2017-10-24 2:16 UTC (permalink / raw)
To: Greg Edwards; +Cc: linux-fsdevel, Christoph Hellwig, linux-kernel
On Mon, Oct 23, 2017 at 07:10:07PM -0600, Greg Edwards wrote:
> guard_bio_eod() needs to look at the partition capacity, not just the
> capacity of the whole device, when determining if truncation is
> necessary.
>
> [ 60.268688] attempt to access beyond end of device
> [ 60.268690] unknown-block(9,1): rw=0, want=67103509, limit=67103506
> [ 60.268693] buffer_io_error: 2 callbacks suppressed
> [ 60.268696] Buffer I/O error on dev md1p7, logical block 4524305, async page read
>
> Fixes: 74d46992e0d9 ("block: replace bi_bdev with a gendisk pointer and partitions index")
> Signed-off-by: Greg Edwards <gedwards@ddn.com>
That smells like a nasty source of overhead, especially on SMP boxen...
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] fs: guard_bio_eod() needs to consider partitions
2017-10-24 2:16 ` Al Viro
@ 2017-10-24 7:46 ` Christoph Hellwig
2017-10-24 15:26 ` Greg Edwards
0 siblings, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2017-10-24 7:46 UTC (permalink / raw)
To: Al Viro; +Cc: Greg Edwards, linux-fsdevel, Christoph Hellwig, linux-kernel
On Tue, Oct 24, 2017 at 03:16:36AM +0100, Al Viro wrote:
> > Fixes: 74d46992e0d9 ("block: replace bi_bdev with a gendisk pointer and partitions index")
> > Signed-off-by: Greg Edwards <gedwards@ddn.com>
>
> That smells like a nasty source of overhead, especially on SMP boxen...
We could just use __disk_get_part similar to the partition remapping,
or just pass the block_device to guard_bio_eod.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] fs: guard_bio_eod() needs to consider partitions
2017-10-24 7:46 ` Christoph Hellwig
@ 2017-10-24 15:26 ` Greg Edwards
0 siblings, 0 replies; 8+ messages in thread
From: Greg Edwards @ 2017-10-24 15:26 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Al Viro, linux-fsdevel, linux-kernel
On Tue, Oct 24, 2017 at 09:46:15AM +0200, Christoph Hellwig wrote:
> On Tue, Oct 24, 2017 at 03:16:36AM +0100, Al Viro wrote:
>>> Fixes: 74d46992e0d9 ("block: replace bi_bdev with a gendisk pointer and partitions index")
>>> Signed-off-by: Greg Edwards <gedwards@ddn.com>
>>
>> That smells like a nasty source of overhead, especially on SMP boxen...
>
> We could just use __disk_get_part similar to the partition remapping,
> or just pass the block_device to guard_bio_eod.
Do you have a preference? __disk_get_part would be isolated to
guard_bio_eod. If we pass in the block_device, it would also touch all
the mpage_bio_submit callers.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] fs: guard_bio_eod() needs to consider partitions
2017-10-24 1:10 [PATCH] fs: guard_bio_eod() needs to consider partitions Greg Edwards
2017-10-24 2:16 ` Al Viro
@ 2017-10-24 17:21 ` Greg Edwards
2017-11-01 17:49 ` Greg Edwards
2017-11-10 8:46 ` Christoph Hellwig
1 sibling, 2 replies; 8+ messages in thread
From: Greg Edwards @ 2017-10-24 17:21 UTC (permalink / raw)
To: linux-fsdevel
Cc: Christoph Hellwig, Al Viro, Jens Axboe, linux-kernel, Greg Edwards
guard_bio_eod() needs to look at the partition capacity, not just the
capacity of the whole device, when determining if truncation is
necessary.
[ 60.268688] attempt to access beyond end of device
[ 60.268690] unknown-block(9,1): rw=0, want=67103509, limit=67103506
[ 60.268693] buffer_io_error: 2 callbacks suppressed
[ 60.268696] Buffer I/O error on dev md1p7, logical block 4524305, async page read
Fixes: 74d46992e0d9 ("block: replace bi_bdev with a gendisk pointer and partitions index")
Signed-off-by: Greg Edwards <gedwards@ddn.com>
---
Changes from v1:
* use __disk_get_part instead of disk_get_part, similar to what
blk_partition_remap does
fs/buffer.c | 10 +++++++++-
include/linux/genhd.h | 1 +
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/fs/buffer.c b/fs/buffer.c
index 170df856bdb9..b96f3b98a6ef 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -3055,8 +3055,16 @@ void guard_bio_eod(int op, struct bio *bio)
sector_t maxsector;
struct bio_vec *bvec = &bio->bi_io_vec[bio->bi_vcnt - 1];
unsigned truncated_bytes;
+ struct hd_struct *part;
+
+ rcu_read_lock();
+ part = __disk_get_part(bio->bi_disk, bio->bi_partno);
+ if (part)
+ maxsector = part_nr_sects_read(part);
+ else
+ maxsector = get_capacity(bio->bi_disk);
+ rcu_read_unlock();
- maxsector = get_capacity(bio->bi_disk);
if (!maxsector)
return;
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index ea652bfcd675..cb8c5fd74b71 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -242,6 +242,7 @@ static inline dev_t part_devt(struct hd_struct *part)
return part_to_dev(part)->devt;
}
+extern struct hd_struct *__disk_get_part(struct gendisk *disk, int partno);
extern struct hd_struct *disk_get_part(struct gendisk *disk, int partno);
static inline void disk_put_part(struct hd_struct *part)
--
2.13.6
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] fs: guard_bio_eod() needs to consider partitions
2017-10-24 17:21 ` [PATCH v2] " Greg Edwards
@ 2017-11-01 17:49 ` Greg Edwards
2017-11-10 8:46 ` Christoph Hellwig
1 sibling, 0 replies; 8+ messages in thread
From: Greg Edwards @ 2017-11-01 17:49 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Christoph Hellwig, Al Viro, Jens Axboe, linux-kernel
On Tue, Oct 24, 2017 at 11:21:48AM -0600, Greg Edwards wrote:
> guard_bio_eod() needs to look at the partition capacity, not just the
> capacity of the whole device, when determining if truncation is
> necessary.
>
> [ 60.268688] attempt to access beyond end of device
> [ 60.268690] unknown-block(9,1): rw=0, want=67103509, limit=67103506
> [ 60.268693] buffer_io_error: 2 callbacks suppressed
> [ 60.268696] Buffer I/O error on dev md1p7, logical block 4524305, async page read
>
> Fixes: 74d46992e0d9 ("block: replace bi_bdev with a gendisk pointer and partitions index")
> Signed-off-by: Greg Edwards <gedwards@ddn.com>
> ---
> Changes from v1:
> * use __disk_get_part instead of disk_get_part, similar to what
> blk_partition_remap does
Al, Christoph,
Any thoughts on this version?
It would be nice to get this fixed before 4.14 goes out, as this is a
regression from previous releases.
Greg
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] fs: guard_bio_eod() needs to consider partitions
2017-10-24 17:21 ` [PATCH v2] " Greg Edwards
2017-11-01 17:49 ` Greg Edwards
@ 2017-11-10 8:46 ` Christoph Hellwig
2017-11-10 15:25 ` Jens Axboe
1 sibling, 1 reply; 8+ messages in thread
From: Christoph Hellwig @ 2017-11-10 8:46 UTC (permalink / raw)
To: Greg Edwards
Cc: linux-fsdevel, Christoph Hellwig, Al Viro, Jens Axboe, linux-kernel
This looks good to me:
Reviewed-by: Christoph Hellwig <hch@lst.de>
Jens, can you pick it up through the block tree given that is where
the issue was introduced?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] fs: guard_bio_eod() needs to consider partitions
2017-11-10 8:46 ` Christoph Hellwig
@ 2017-11-10 15:25 ` Jens Axboe
0 siblings, 0 replies; 8+ messages in thread
From: Jens Axboe @ 2017-11-10 15:25 UTC (permalink / raw)
To: Christoph Hellwig, Greg Edwards; +Cc: linux-fsdevel, Al Viro, linux-kernel
On 11/10/2017 01:46 AM, Christoph Hellwig wrote:
> This looks good to me:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
> Jens, can you pick it up through the block tree given that is where
> the issue was introduced?
I picked it up, and marked it for 4.13 stable as well.
--
Jens Axboe
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-11-10 15:25 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-24 1:10 [PATCH] fs: guard_bio_eod() needs to consider partitions Greg Edwards
2017-10-24 2:16 ` Al Viro
2017-10-24 7:46 ` Christoph Hellwig
2017-10-24 15:26 ` Greg Edwards
2017-10-24 17:21 ` [PATCH v2] " Greg Edwards
2017-11-01 17:49 ` Greg Edwards
2017-11-10 8:46 ` Christoph Hellwig
2017-11-10 15:25 ` Jens Axboe
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.