On Thu, May 17, 2018 at 4:07 PM, Yan, Zheng <zyan@xxxxxxxxxx> wrote:
>
>
>> On May 17, 2018, at 20:27, Ilya Dryomov <idryomov@xxxxxxxxx> wrote:
>>
>> On Thu, May 17, 2018 at 1:40 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>>> On Thu, 2018-05-17 at 11:32 +0200, Ilya Dryomov wrote:
>>>> On Thu, May 17, 2018 at 5:29 AM, Yan, Zheng <zyan@xxxxxxxxxx> wrote:
>>>>> In the case of -ENOSPC, writeback thread may wait on itself. The call
>>>>> stack looks like:
>>>>>
>>>>> inode_wait_for_writeback+0x26/0x40
>>>>> evict+0xb5/0x1a0
>>>>> iput+0x1d2/0x220
>>>>> ceph_put_wrbuffer_cap_refs+0xe0/0x2c0 [ceph]
>>>>> writepages_finish+0x2d3/0x410 [ceph]
>>>>> __complete_request+0x26/0x60 [libceph]
>>>>> complete_request+0x2e/0x70 [libceph]
>>>>> __submit_request+0x256/0x330 [libceph]
>>>>> submit_request+0x2b/0x30 [libceph]
>>>>> ceph_osdc_start_request+0x25/0x40 [libceph]
>>>>> ceph_writepages_start+0xdfe/0x1320 [ceph]
>>>>> do_writepages+0x1f/0x70
>>>>> __writeback_single_inode+0x45/0x330
>>>>> writeback_sb_inodes+0x26a/0x600
>>>>> __writeback_inodes_wb+0x92/0xc0
>>>>> wb_writeback+0x274/0x330
>>>>> wb_workfn+0x2d5/0x3b0
>>>>
>>>> This is exactly what I was worried about when Jeff introduced the
>>>> possibility of complete_request() on the submit thread. Do you think
>>>> this is the only such case or there may be others?
>>>>
>>>> Another related issue is that normally ->r_callback is invoked
>>>> without any libceph locks held -- handle_reply() drops both osd->lock
>>>> and osdc->lock before calling __complete_request(). In this case it
>>>> is called with both of these locks held.
>>>>
>>>
>>> Not in the "fail_request" case. The lack of clear locking rules with
>>> these callbacks makes it really difficult to suss out these problems.
>>
>> Yeah, it was (is?) pretty much the same with Objecter in userspace.
>> The locking issue is old and I guess we have learned to be careful
>> there. Calling the callback from the submit thread is new.
>>
>>>
>>>> Given that umount -f will use the same mechanism, could you please
>>>> double check all fs/ceph callbacks? I wonder if we should maybe do
>>>> something different in libceph...
>>>
>>> Might a simpler fix be to just have __submit_request queue the
>>> complete_request callback to a workqueue in the ENOSPC case? That should
>>> be a rare thing in most cases.
>>
>> That was my thought as well, but it needs to be justified and this
>> stack trace is actually a bad example. In the common case the callback
>> is invoked by the messenger, so blocking is undesirable. Blocking on
>> writeback is particularly so -- unless I'm misunderstanding something,
>> that can deadlock even under normal conditions.
>
> It can’t happen on normal condition. writepages_finish() drops inode’s last reference only when there is no more dirty/writeback page. Writeback should be already done or be about to done.
I see, so at most it will wait for the writeback thread to get to
inode_sync_complete()?
While this patch isn't too ugly, I'm leaning towards adding a finisher
for all complete_request() special cases. I'm not convinced this is the
only problematic site in fs/ceph and there is a patch pending that makes
blocking optional in rbd, so the space is about to grow.
I have put this on my list for next week.
Thanks,
Ilya
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
