Re: fs: GPF in locked_inode_to_wb_and_lock_list

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Tejun,

I have not seem the crash since then. Please mail the patch.


On Fri, Apr 22, 2016 at 8:55 PM, Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote:
> On Thu, Apr 21, 2016 at 7:06 PM, Tejun Heo <tj@xxxxxxxxxx> wrote:
>> Hello,
>>
>> (cc'ing Ilya, Jan and Jens)
>>
>> On Thu, Apr 21, 2016 at 12:00:38PM +0200, Dmitry Vyukov wrote:
>>> On Thu, Apr 21, 2016 at 11:45 AM, Andrey Ryabinin
>>> <ryabinin.a.a@xxxxxxxxx> wrote:
>>> > 2016-04-21 11:35 GMT+03:00 Dmitry Vyukov <dvyukov@xxxxxxxxxx>:
>>> >>
>>> >> ffffffff818884dd:       48 8b 03                mov    (%rbx),%rax
>>> >>
>>> >> So whatever load "&wb->bdi->wb" produces is a NULL deref. (is it wb
>>> >> that is NULL?)
>>> >
>>> > Yes it's NULL wb, because there is only one load:
>>> >     mov    (%rbx),%rax        =>       rax = wb->bdi
>>> >     add    $0x50,%rax         =>       rax = &bdi->wb
>>>
>>>
>>> I bet that wb becomes NULL on the second iteration of the loop. The
>>> loop loops in case of a race with another thread, so it would also
>>> explain why it is difficult to reproduce.
>>>
>>> Tejun, does it make any sense to you?
>>
>> Yeah, that makes sense.  I think the culprit is 43d1c0eb7e11 ("block:
>> detach bdev inode from its wb in __blkdev_put()") which allows inode
>> to wb association to be broken while other operations including
>> writeback are in progress.  I thought it should be okay as the inode
>> must be clean at that point but that obviously doesn't mean that there
>> can be no writeback operations in flight.
>>
>> I hope we could eventually move away from the current model where we
>> try to swap out an underlying data structure while upper layers may
>> still be referring to it in the future but for now we can make sure
>> the writeback operation is finished before detaching wb.
>>
>> Dmitry, I understand that the bug is difficult to reproduce but can
>> you please give the following patch a try?
>
>
> I've merged it into my tree and will restart the fuzzer and leave it
> running for the weekend.
> Though, yeah, it is difficult to reproduce...
>
>
>> Thanks.
>>
>> diff --git a/fs/block_dev.c b/fs/block_dev.c
>> index 20a2c02..209ea33 100644
>> --- a/fs/block_dev.c
>> +++ b/fs/block_dev.c
>> @@ -1530,12 +1530,7 @@ static void __blkdev_put(struct block_device *bdev, fmode_t mode, int for_part)
>>                 kill_bdev(bdev);
>>
>>                 bdev_write_inode(bdev);
>> -               /*
>> -                * Detaching bdev inode from its wb in __destroy_inode()
>> -                * is too late: the queue which embeds its bdi (along with
>> -                * root wb) can be gone as soon as we put_disk() below.
>> -                */
>> -               inode_detach_wb(bdev->bd_inode);
>> +               inode_detach_blkdev_wb(bdev);
>>         }
>>         if (bdev->bd_contains == bdev) {
>>                 if (disk->fops->release)
>> diff --git a/include/linux/writeback.h b/include/linux/writeback.h
>> index d0b5ca5..ec1f530 100644
>> --- a/include/linux/writeback.h
>> +++ b/include/linux/writeback.h
>> @@ -230,6 +230,25 @@ static inline void inode_detach_wb(struct inode *inode)
>>  }
>>
>>  /**
>> + * inode_detach_blkdev_wb - disassociate a bd_inode from its wb
>> + * @bdev: block_device of interest
>> + *
>> + * @bdev is being put for the last time.  Detaching bdev inode in
>> + * __destroy_inode() is too late: the queue which embeds its bdi (along
>> + * with root wb) can be gone as soon as the containing disk is put.
>> + *
>> + * This function dissociates @bdev->bd_inode from its wb.  The inode must
>> + * be clean and no further operations should be started on it.
>> + */
>> +static inline void inode_detach_blkdev_wb(struct block_device *bdev)
>> +{
>> +       if (bdev->bd_inode->i_wb) {
>> +               flush_delayed_work(&bdev->bd_inode->i_wb->dwork);
>> +               inode_detach_wb(bdev->bd_inode);
>> +       }
>> +}
>> +
>> +/**
>>   * wbc_attach_fdatawrite_inode - associate wbc and inode for fdatawrite
>>   * @wbc: writeback_control of interest
>>   * @inode: target inode
>> @@ -277,6 +296,10 @@ static inline void inode_detach_wb(struct inode *inode)
>>  {
>>  }
>>
>> +static inline void inode_detach_blkdev_wb(struct block_device *bdev)
>> +{
>> +}
>> +
>>  static inline void wbc_attach_and_unlock_inode(struct writeback_control *wbc,
>>                                                struct inode *inode)
>>         __releases(&inode->i_lock)
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux