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.
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
