https://tracker.ceph.com/https://tracker.ceph.com/favicon.ico2015-06-21T14:19:45ZCeph CephFS - Bug #12088: cephfs client crash after enable readahead mechanism through setting conf option 'client_readahead_max_bytes'https://tracker.ceph.com/issues/12088?journal_id=539162015-06-21T14:19:45ZWenjun Huanghuangwenjun310@gmail.com
<ul></ul><p>While diving into the source code, we found the code path which will cause the crash. Described as the following steps:<br />In the Client::_read_async method:<br />1. The <strong>Fh</strong> is read for some data, which will trigger an read Op to be sent.<br />2. If the <strong>client_readahead_max_bytes</strong> option is set, it will trigger an additional readahead Op to be sent, and the Fh is handled by the Op finisher <strong>C_Readahead</strong>.</p>
<p>3. For some reason, the client close the fh quickly, but the response of the readahead Op has not reached to the client at that time.<br />4. In the above condition, the Client::_release_fh method did not have an additional check before deleting the Fh object. But the pointer to the Fh object is still handled by the finisher C_Readahead.<br />5. At last, the response of the readahead Op reached, it will execute its <strong>C_Readahead::finish</strong> method, but the pointer to Fh it handled points to an undefined memory area. So when it operates on the pointer, the program will crash.</p>
<p>A way I can come up with to fix this bug is to set a ref count for the Fh object. Then we can have a check before deleting the Fh object, if the ref count is larger than zero, we can postpone the deleting operation.</p> CephFS - Bug #12088: cephfs client crash after enable readahead mechanism through setting conf option 'client_readahead_max_bytes'https://tracker.ceph.com/issues/12088?journal_id=542952015-06-30T14:45:25ZSamuel Justsjust@redhat.com
<ul><li><strong>Project</strong> changed from <i>Ceph</i> to <i>CephFS</i></li></ul> CephFS - Bug #12088: cephfs client crash after enable readahead mechanism through setting conf option 'client_readahead_max_bytes'https://tracker.ceph.com/issues/12088?journal_id=544252015-07-02T08:47:59ZZheng Yanukernel@gmail.com
<ul><li><strong>File</strong> <a href="/attachments/download/1853/client.patch">client.patch</a> <a class="icon-only icon-magnifier" title="View" href="/attachments/1853/client.patch">View</a> added</li><li><strong>Status</strong> changed from <i>New</i> to <i>7</i></li></ul><p>please try the attached patch</p> CephFS - Bug #12088: cephfs client crash after enable readahead mechanism through setting conf option 'client_readahead_max_bytes'https://tracker.ceph.com/issues/12088?journal_id=544282015-07-02T09:30:50ZZhi Zhangzhangz.david@outlook.com
<ul></ul><p>hi Yan,</p>
<p>Thanks for the patch. We also have a similar patch tested internally, which uses readahead's pending counter as a ref, but your fix looks more concise. One question is, do you think a lock is needed when modifying _ref in Fh?</p> CephFS - Bug #12088: cephfs client crash after enable readahead mechanism through setting conf option 'client_readahead_max_bytes'https://tracker.ceph.com/issues/12088?journal_id=544292015-07-02T10:48:53ZZheng Yanukernel@gmail.com
<ul></ul><p>no extra lock is needed. the async readahead context is called while client_lock is locked.</p> CephFS - Bug #12088: cephfs client crash after enable readahead mechanism through setting conf option 'client_readahead_max_bytes'https://tracker.ceph.com/issues/12088?journal_id=547832015-07-13T12:40:18ZJohn Sprayjcspray@gmail.com
<ul><li><strong>Status</strong> changed from <i>7</i> to <i>Pending Backport</i></li></ul><p>I imagine we probably want backports to firefly + hammer?</p> CephFS - Bug #12088: cephfs client crash after enable readahead mechanism through setting conf option 'client_readahead_max_bytes'https://tracker.ceph.com/issues/12088?journal_id=547842015-07-13T12:40:55ZJohn Sprayjcspray@gmail.com
<ul></ul><pre>
commit 34b939a81d38173b882c429b28dedce778504ba8
Author: Yan, Zheng <zyan@redhat.com>
Date: Wed Jul 8 10:11:43 2015 +0800
client: reference counting 'struct Fh'
The async readahead finisher needs to reference 'struct Fh'. But
it's possible user closes FD and free the corresponding 'struct Fh'
before async readahead finishes.
Fixes: #12088
Signed-off-by: Yan, Zheng <zyan@redhat.com>
</pre> CephFS - Bug #12088: cephfs client crash after enable readahead mechanism through setting conf option 'client_readahead_max_bytes'https://tracker.ceph.com/issues/12088?journal_id=548372015-07-14T06:45:15ZZheng Yanukernel@gmail.com
<ul></ul><p><a class="external" href="https://github.com/ceph/ceph/pull/5222">https://github.com/ceph/ceph/pull/5222</a></p> CephFS - Bug #12088: cephfs client crash after enable readahead mechanism through setting conf option 'client_readahead_max_bytes'https://tracker.ceph.com/issues/12088?journal_id=548382015-07-14T07:04:58ZKefu Chaitchaikov@gmail.com
<ul><li><strong>Assignee</strong> set to <i>Zheng Yan</i></li><li><strong>Backport</strong> set to <i>hammer</i></li></ul> CephFS - Bug #12088: cephfs client crash after enable readahead mechanism through setting conf option 'client_readahead_max_bytes'https://tracker.ceph.com/issues/12088?journal_id=548792015-07-14T10:45:21ZGreg Farnumgfarnum@redhat.com
<ul></ul><p>Backports team: Zheng created a backport PR at <a class="external" href="https://github.com/ceph/ceph/pull/5222">https://github.com/ceph/ceph/pull/5222</a>, but let's let it bake a bit in master.</p> CephFS - Bug #12088: cephfs client crash after enable readahead mechanism through setting conf option 'client_readahead_max_bytes'https://tracker.ceph.com/issues/12088?journal_id=550412015-07-16T02:22:51ZYan Shenshenyan_001@126.com
<ul></ul><p>Hi, Yan. I think it is also unsafe in the _read_async code when this patch applied. When thread1 wait on read, thread2 may release this fh (close fh immediately). If I enable client_readahead_max_bytes, f->readahead.update(off, len, in->size) may access freed fh probably. <br /> <pre>
if (r == 0) {
get_cap_ref(in, CEPH_CAP_FILE_CACHE);
client_lock.Unlock();
flock.Lock();
while (!done)
cond.Wait(flock);<---------unsafe wait?
flock.Unlock();
client_lock.Lock();
put_cap_ref(in, CEPH_CAP_FILE_CACHE);
r = rvalue;
} else {
// it was cached.
delete onfinish;
}
if(conf->client_readahead_max_bytes > 0) {
pair<uint64_t, uint64_t> readahead_extent = f->readahead.update(off, len, in->size);
if (readahead_extent.second > 0) {
</pre></p>
<p>When I apply this patch in ceph-dokan, the following bug happend at some times. It is really hard to reproduce. Do you think we should reproduce this patch to handle it?</p>
<pre>
18 Id: 1eb0.1a6c Suspend: 0 Teb: 7ef70000 Unfrozen
ChildEBP RetAddr Args to Child
04e4f1dc 75640bdd 00000002 04e4f22c 00000001 ntdll_776c0000!ZwWaitForMultipleObjects+0x15
04e4f278 76bb1a2c 04e4f22c 04e4f2a0 00000000 KERNELBASE!WaitForMultipleObjectsEx+0x100
04e4f2c0 76bb4208 00000002 7efde000 00000000 kernel32!WaitForMultipleObjectsExImplementation+0xe0
04e4f2dc 76bd80a4 00000002 04e4f310 00000000 kernel32!WaitForMultipleObjects+0x18
04e4f348 76bd7f63 04e4f428 00000001 00000001 kernel32!WerpReportFaultInternal+0x186
04e4f35c 76bd7858 04e4f428 00000001 04e4f3f8 kernel32!WerpReportFault+0x70
04e4f36c 76bd77d7 04e4f428 00000001 e294106f kernel32!BasepReportFault+0x20
04e4f3f8 777374df 00000000 777373bc 00000000 kernel32!UnhandledExceptionFilter+0x1af
04e4f400 777373bc 00000000 04e5ffd4 776ec530 ntdll_776c0000!__RtlUserThreadStart+0x62
04e4f414 77737261 00000000 00000000 00000000 ntdll_776c0000!_EH4_CallFilterFunc+0x12
04e4f43c 7771b459 fffffffe 04e5ffc4 04e4f578 ntdll_776c0000!_except_handler4+0x8e
04e4f460 7771b42b 04e4f528 04e5ffc4 04e4f578 ntdll_776c0000!ExecuteHandler2+0x26
04e4f484 7771b3ce 04e4f528 04e5ffc4 04e4f578 ntdll_776c0000!ExecuteHandler+0x24
04e4f510 776d0133 01e4f528 04e4f578 04e4f528 ntdll_776c0000!RtlDispatchException+0x127
04e4f51c 04e4f528 04e4f578 c0000005 00000000 ntdll_776c0000!KiUserExceptionDispatcher+0xf
WARNING: Frame IP not in any known module. Following frames may be wrong.
04e4f8d8 64e9fa26 00000000 00000000 00000008 0x4e4f528
04e4f928 64e88ea7 00000000 04e4f9cc 6fcc3a00 libcephfs!ZN5Mutex4LockEb+0x4c
04e4f968 64fdaf3f 101140a0 02c32000 00000000 libcephfs!ZN9Readahead6updateEyyy+0x41
04e4fc18 64fda419 10114078 02c32000 00000000 libcephfs!ZN6Client11_read_asyncEP2FhyyPN4ceph6buffer4listE+0x5ff
04e4fce8 64fd9d22 10114078 02c32000 00000000 libcephfs!ZN6Client5_readEP2FhxyPN4ceph6buffer4listE+0x497
</pre> CephFS - Bug #12088: cephfs client crash after enable readahead mechanism through setting conf option 'client_readahead_max_bytes'https://tracker.ceph.com/issues/12088?journal_id=553672015-07-22T03:22:31ZZhi Zhangzhangz.david@outlook.com
<ul></ul><p>Zheng Yan wrote:</p>
<blockquote>
<p><a class="external" href="https://github.com/ceph/ceph/pull/5222">https://github.com/ceph/ceph/pull/5222</a></p>
</blockquote>
<p>Hi Yan,</p>
<p>We found a serious problem with this patch when doing massive testing recently. Fh's ref will be increased when initializing C_Readahead, but if readahead does not need to do read from osd, readahead finisher will not be called and onfinish2 will be deleted. So Fh's ref can not be decreased. Then closing this file later, its inode's ref will miss the window to be decreased as well.</p>
<p>When this happened, the bad thing we met was that if we tried to delete those files, they could not be deleted physically from OSD because their inode's ref was not zero. The cluster is telling it is nearfull, while only some files are in cephfs.</p>
<p>How about we put _put_fh in C_Readahead's deconstructor, then either case will decrease Fh's ref?</p>
<p>Please check this PR: <a class="external" href="https://github.com/ceph/ceph/pull/5311">https://github.com/ceph/ceph/pull/5311</a></p>
<p>Thanks.</p> CephFS - Bug #12088: cephfs client crash after enable readahead mechanism through setting conf option 'client_readahead_max_bytes'https://tracker.ceph.com/issues/12088?journal_id=559772015-07-31T07:52:53ZLoïc Dacharyloic@dachary.org
<ul><li><strong>Status</strong> changed from <i>Pending Backport</i> to <i>Resolved</i></li></ul>