On Wed, Apr 05, 2017 at 02:04:19PM -0700, Liu Bo wrote:
> When doing directIO repair, we have this oops
> 
> [ 1458.532816] general protection fault: 0000 [#1] SMP
> ...
> [ 1458.536291] Workqueue: btrfs-endio-repair btrfs_endio_repair_helper [btrfs]
> [ 1458.536893] task: ffff88082a42d100 task.stack: ffffc90002b3c000
> [ 1458.537499] RIP: 0010:btrfs_retry_endio+0x7e/0x1a0 [btrfs]
> ...
> [ 1458.543261] Call Trace:
> [ 1458.543958]  ? rcu_read_lock_sched_held+0xc4/0xd0
> [ 1458.544374]  bio_endio+0xed/0x100
> [ 1458.544750]  end_workqueue_fn+0x3c/0x40 [btrfs]
> [ 1458.545257]  normal_work_helper+0x9f/0x900 [btrfs]
> [ 1458.545762]  btrfs_endio_repair_helper+0x12/0x20 [btrfs]
> [ 1458.546224]  process_one_work+0x34d/0xb70
> [ 1458.546570]  ? process_one_work+0x29e/0xb70
> [ 1458.546938]  worker_thread+0x1cf/0x960
> [ 1458.547263]  ? process_one_work+0xb70/0xb70
> [ 1458.547624]  kthread+0x17d/0x180
> [ 1458.547909]  ? kthread_create_on_node+0x70/0x70
> [ 1458.548300]  ret_from_fork+0x31/0x40
> 
> It turns out that btrfs_retry_endio is trying to get inode from a directIO
> page.
> 
> This fixes the problem by using the preservd inode pointer, done->inode.
> btrfs_retry_endio_nocsum has the same problem, and it's fixed as well.
> 
> Also cleanup unused @start(which is too trivial for a separate patch).
> 
> Cc: David Sterba <dste...@suse.cz>
> Signed-off-by: Liu Bo <bo.li....@oracle.com>

Reviewed-by: David Sterba <dste...@suse.com>

> ---
>  fs/btrfs/inode.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 3ec5a05..8e71ed7 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -7910,7 +7910,6 @@ struct btrfs_retry_complete {
>  static void btrfs_retry_endio_nocsum(struct bio *bio)
>  {
>       struct btrfs_retry_complete *done = bio->bi_private;
> -     struct inode *inode;
>       struct bio_vec *bvec;
>       int i;
>  
> @@ -7918,12 +7917,12 @@ static void btrfs_retry_endio_nocsum(struct bio *bio)
>               goto end;
>  
>       ASSERT(bio->bi_vcnt == 1);
> -     inode = bio->bi_io_vec->bv_page->mapping->host;
> -     ASSERT(bio->bi_io_vec->bv_len == btrfs_inode_sectorsize(inode));
> +     ASSERT(bio->bi_io_vec->bv_len == btrfs_inode_sectorsize(done->inode));

I was interested how it got here in the first place, inode from mapping
hasn't been used in the initial support for DIO repair. So it was added
in the subpage blocksize preparation patchset (2dabb3248453b), slipped
through the review.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to