<div dir="ltr"><div><div><div><div>Hi Kinglong<br></div></div>I was reading about makecontext/swapcontext as well, <br></div>I did find an article that suggested to use mprotect and force a segfault to check if we have a stack space issue here.<br>here is the link. <a href="http://www.evanjones.ca/software/threading.html">http://www.evanjones.ca/software/threading.html</a>.<br><br></div><div>I don't think i can try this until tomorrow.<br></div><div>thanks and regards,<br></div><div>Sanoj<br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jun 13, 2017 at 5:58 AM, Kinglong Mee <span dir="ltr"><<a href="mailto:kinglongmee@gmail.com" target="_blank">kinglongmee@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Sanoj,<br>
<br>
What's your opinion about this problem?<br>
<br>
thanks,<br>
Kinglong Mee<br>
<div class="HOEnZb"><div class="h5"><br>
On 6/9/2017 17:20, Kinglong Mee wrote:<br>
> Hi Sanoj,<br>
><br>
> On 6/9/2017 15:48, Sanoj Unnikrishnan wrote:<br>
>> I have not used valgrind before, so I may be wrong here.<br>
>><br>
>> I think the valgrind_stack_deregister should have been after GF_FREE_STACK.<br>
>> That may explain the instance of invalid_write during stack_destroy calls in after.log.<br>
><br>
> No. I move it, but the instance of invalid_write also exist.<br>
><br>
>><br>
>> There seems to be numerous issues reported in before.log (I am assuming, you did not have the valgrind_stack_register call in it),<br>
><br>
> Yes, the before.log is test without any code change(but without io-threads).<br>
><br>
>> From <a href="http://valgrind.org/docs/manual/manual-core.html" rel="noreferrer" target="_blank">http://valgrind.org/docs/<wbr>manual/manual-core.html</a> <<a href="http://valgrind.org/docs/manual/manual-core.html" rel="noreferrer" target="_blank">http://valgrind.org/docs/<wbr>manual/manual-core.html</a>>, looks like valgrind detects client switching stack only If a memory of > 2MB change in Stack pointer register.<br>
><br>
> I test with a larger max-stackframe as,<br>
> valgrind --leak-check=full --max-stackframe=242293216<br>
><br>
>> Is it possible that since marker is only using 16k, the stack pointer could have been in less than 2MB offset from current Stack Pointer?<br>
><br>
> Maybe.<br>
> But with io-threads (with adding valgrind_stack_deregister), the valgrind only show some<br>
> "Invalid read/write" about __gf_mem_invalidate.<br>
> The only reason here I think is the stack size (16K) of marker using.<br>
><br>
> I have not used makecontext/swapcontext before, am i right?<br>
> 1. without swapconext, the stack maybe (just an example)<br>
> --> io_stats-> quota-> marker-> io-threads ->.....<br>
><br>
> 2. with swapcontext,<br>
> --> io_stats-> quota-> marker<br>
> swithto new stack -> io-threads<br>
><br>
> After switchto new stack, the new stack size is 16K,<br>
> Does it enough without io-threads?<br>
><br>
> I don't know what's the behave of io-threads, does it call all sub-xlator using the 16K ? or others?<br>
><br>
>> It seems unlikely to me since we are allocating the stack from heap.<br>
>><br>
>> Did you try a run with the valgrind instrumentation, without changing stack size ?<br>
><br>
> OK.<br>
> The following valgrind-without-stack-change.<wbr>log is test with adding valgrind_stack_deregister<br>
> (and without io-threads).<br>
><br>
> thanks,<br>
> Kinglong Mee<br>
><br>
>> None of this explains the crash though.. We had seen a memory overrun crash in same code path on netbsd earlier but did not follow up then.<br>
>> Will look further into it.<br>
>><br>
>><br>
>><br>
>> On Thu, Jun 8, 2017 at 4:51 PM, Kinglong Mee <<a href="mailto:kinglongmee@gmail.com">kinglongmee@gmail.com</a> <mailto:<a href="mailto:kinglongmee@gmail.com">kinglongmee@gmail.com</a>><wbr>> wrote:<br>
>><br>
>> Maybe it's my fault, I found valgrind can't parse context switch(makecontext/<wbr>swapcontext) by default.<br>
>> So, I test with the following patch (tells valgrind new stack by VALGRIND_STACK_DEREGISTER).<br>
>> With it, only some "Invalid read/write" by __gf_mem_invalidate, Does it right ??<br>
>> So, there is only one problem, if without io-threads, the stack size is small for marker.<br>
>> Am I right?<br>
>><br>
>> Ps:<br>
>> valgrind-before.log is the log without the following patch, the valgrind-after.log is with the patch.<br>
>><br>
>> ==35656== Invalid write of size 8<br>
>> ==35656== at 0x4E8FFD4: __gf_mem_invalidate (mem-pool.c:278)<br>
>> ==35656== by 0x4E90313: __gf_free (mem-pool.c:334)<br>
>> ==35656== by 0x4EA4E5B: synctask_destroy (syncop.c:394)<br>
>> ==35656== by 0x4EA4EDF: synctask_done (syncop.c:412)<br>
>> ==35656== by 0x4EA58B3: synctask_switchto (syncop.c:673)<br>
>> ==35656== by 0x4EA596B: syncenv_processor (syncop.c:704)<br>
>> ==35656== by 0x60B2DC4: start_thread (in /usr/lib64/<a href="http://libpthread-2.17.so" rel="noreferrer" target="_blank">libpthread-2.17.so</a> <<a href="http://libpthread-2.17.so" rel="noreferrer" target="_blank">http://libpthread-2.17.so</a>>)<br>
>> ==35656== by 0x67A873C: clone (in /usr/lib64/<a href="http://libc-2.17.so" rel="noreferrer" target="_blank">libc-2.17.so</a> <<a href="http://libc-2.17.so" rel="noreferrer" target="_blank">http://libc-2.17.so</a>>)<br>
>> ==35656== Address 0x1b104931 is 2,068,017 bytes inside a block of size 2,097,224 alloc'd<br>
>> ==35656== at 0x4C29975: calloc (vg_replace_malloc.c:711)<br>
>> ==35656== by 0x4E8FA5E: __gf_calloc (mem-pool.c:117)<br>
>> ==35656== by 0x4EA52F5: synctask_create (syncop.c:500)<br>
>> ==35656== by 0x4EA55AE: synctask_new1 (syncop.c:576)<br>
>> ==35656== by 0x143AE0D7: mq_synctask1 (marker-quota.c:1078)<br>
>> ==35656== by 0x143AE199: mq_synctask (marker-quota.c:1097)<br>
>> ==35656== by 0x143AE6F6: _mq_create_xattrs_txn (marker-quota.c:1236)<br>
>> ==35656== by 0x143AE82D: mq_create_xattrs_txn (marker-quota.c:1253)<br>
>> ==35656== by 0x143B0DCB: mq_inspect_directory_xattr (marker-quota.c:2027)<br>
>> ==35656== by 0x143B13A8: mq_xattr_state (marker-quota.c:2117)<br>
>> ==35656== by 0x143A6E80: marker_lookup_cbk (marker.c:2961)<br>
>> ==35656== by 0x141811E0: up_lookup_cbk (upcall.c:753)<br>
>><br>
>> ----------------------- valgrind ------------------------------<wbr>------------<br>
>><br>
>> Don't forget install valgrind-devel.<br>
>><br>
>> diff --git a/libglusterfs/src/syncop.c b/libglusterfs/src/syncop.c<br>
>> index 00a9b57..97b1de1 100644<br>
>> --- a/libglusterfs/src/syncop.c<br>
>> +++ b/libglusterfs/src/syncop.c<br>
>> @@ -10,6 +10,7 @@<br>
>><br>
>> #include "syncop.h"<br>
>> #include "libglusterfs-messages.h"<br>
>> +#include <valgrind/valgrind.h><br>
>><br>
>> int<br>
>> syncopctx_setfsuid (void *uid)<br>
>> @@ -388,6 +389,8 @@ synctask_destroy (struct synctask *task)<br>
>> if (!task)<br>
>> return;<br>
>><br>
>> +VALGRIND_STACK_DEREGISTER(<wbr>task->valgrind_ret);<br>
>> +<br>
>> GF_FREE (task->stack);<br>
>><br>
>> if (task->opframe)<br>
>> @@ -509,6 +512,8 @@ synctask_create (struct syncenv *env, size_t stacksize, sync<br>
>><br>
>> newtask->ctx.uc_stack.ss_sp = newtask->stack;<br>
>><br>
>> + newtask->valgrind_ret = VALGRIND_STACK_REGISTER(<wbr>newtask->stack, newtask-<br>
>> +<br>
>> makecontext (&newtask->ctx, (void (*)(void)) synctask_wrap, 2, newtask)<br>
>><br>
>> newtask->state = SYNCTASK_INIT;<br>
>> diff --git a/libglusterfs/src/syncop.h b/libglusterfs/src/syncop.h<br>
>> index c2387e6..247325b 100644<br>
>> --- a/libglusterfs/src/syncop.h<br>
>> +++ b/libglusterfs/src/syncop.h<br>
>> @@ -63,6 +63,7 @@ struct synctask {<br>
>> int woken;<br>
>> int slept;<br>
>> int ret;<br>
>> + int valgrind_ret;<br>
>><br>
>> uid_t uid;<br>
>> gid_t gid;<br>
>> diff --git a/xlators/features/marker/src/<wbr>marker-quota.c b/xlators/features/marke<br>
>> index 902b8e5..f3d2507 100644<br>
>> --- a/xlators/features/marker/src/<wbr>marker-quota.c<br>
>> +++ b/xlators/features/marker/src/<wbr>marker-quota.c<br>
>> @@ -1075,7 +1075,7 @@ mq_synctask1 (xlator_t *this, synctask_fn_t task, gf_boole<br>
>> }<br>
>><br>
>> if (spawn) {<br>
>> - ret = synctask_new1 (this->ctx->env, 1024 * 16, task,<br>
>> + ret = synctask_new1 (this->ctx->env, 0, task,<br>
>> mq_synctask_cleanup, NULL, args);<br>
>> if (ret) {<br>
>> gf_log (this->name, GF_LOG_ERROR, "Failed to spawn "<br>
>><br>
>><br>
>> On 6/8/2017 19:02, Sanoj Unnikrishnan wrote:<br>
>> > I would still be worried about the Invalid read/write. IMO whether an illegal access causes a crash depends on whether the page is currently mapped.<br>
>> > So, it could so happen that there is a use after free / use outside of bounds happening in the code and it turns out that this location gets mapped in a different (unmapped) page when IO threads is not loaded.<br>
>> ><br>
>> > Could you please share the valgrind logs as well.<br>
>> ><br>
>> > On Wed, Jun 7, 2017 at 8:22 PM, Kinglong Mee <<a href="mailto:kinglongmee@gmail.com">kinglongmee@gmail.com</a> <mailto:<a href="mailto:kinglongmee@gmail.com">kinglongmee@gmail.com</a>> <mailto:<a href="mailto:kinglongmee@gmail.com">kinglongmee@gmail.com</a> <mailto:<a href="mailto:kinglongmee@gmail.com">kinglongmee@gmail.com</a>><wbr>>> wrote:<br>
>> ><br>
>> > After deleting io-threads from the vols, quota operates (list/set/modify) lets glusterfsd crash.<br>
>> > I use it at CentOS 7 (CentOS Linux release 7.3.1611) with glusterfs 3.8.12.<br>
>> > It seems the stack corrupt, when testing with the following diff, glusterfsd runs correctly.<br>
>> ><br>
>> > There are two questions as,<br>
>> > 1. When using valgrind, it shows there are many "Invalid read/write" when with io-threads.<br>
>> > Why glusterfsd runs correctly with io-threads? but crash without io-threads?<br>
>> ><br>
>> > 2. With the following diff, valgrind also shows many "Invalid read/write" when without io-threads?<br>
>> > but no any crash.<br>
>> ><br>
>> > Any comments are welcome.<br>
>> ><br>
>> > Revert <a href="http://review.gluster.org/11499" rel="noreferrer" target="_blank">http://review.gluster.org/<wbr>11499</a> <<a href="http://review.gluster.org/11499" rel="noreferrer" target="_blank">http://review.gluster.org/<wbr>11499</a>> <<a href="http://review.gluster.org/11499" rel="noreferrer" target="_blank">http://review.gluster.org/<wbr>11499</a> <<a href="http://review.gluster.org/11499" rel="noreferrer" target="_blank">http://review.gluster.org/<wbr>11499</a>>> seems better than the diff.<br>
>> ><br>
>> > diff --git a/xlators/features/marker/src/<wbr>marker-quota.c b/xlators/features/marke<br>
>> > index 902b8e5..f3d2507 100644<br>
>> > --- a/xlators/features/marker/src/<wbr>marker-quota.c<br>
>> > +++ b/xlators/features/marker/src/<wbr>marker-quota.c<br>
>> > @@ -1075,7 +1075,7 @@ mq_synctask1 (xlator_t *this, synctask_fn_t task, gf_boole<br>
>> > }<br>
>> ><br>
>> > if (spawn) {<br>
>> > - ret = synctask_new1 (this->ctx->env, 1024 * 16, task,<br>
>> > + ret = synctask_new1 (this->ctx->env, 0, task,<br>
>> > mq_synctask_cleanup, NULL, args);<br>
>> > if (ret) {<br>
>> > gf_log (this->name, GF_LOG_ERROR, "Failed to spawn "<br>
>> ><br>
>> > ------------------------------<wbr>-----test steps ------------------------------<wbr>----<br>
>> > 1. gluster volume create gvtest node1:/test/ node2:/test/<br>
>> > 2. gluster volume start gvtest<br>
>> > 3. gluster volume quota enable gvtest<br>
>> ><br>
>> > 4. "deletes io-threads from all vols"<br>
>> > 5. reboot node1 and node2.<br>
>> > 6. sh quota-set.sh<br>
>> ><br>
>> > # cat quota-set.sh<br>
>> > gluster volume quota gvtest list<br>
>> > gluster volume quota gvtest limit-usage / 10GB<br>
>> > gluster volume quota gvtest limit-usage /1234 1GB<br>
>> > gluster volume quota gvtest limit-usage /hello 1GB<br>
>> > gluster volume quota gvtest limit-usage /test 1GB<br>
>> > gluster volume quota gvtest limit-usage /xyz 1GB<br>
>> > gluster volume quota gvtest list<br>
>> > gluster volume quota gvtest remove /hello<br>
>> > gluster volume quota gvtest remove /test<br>
>> > gluster volume quota gvtest list<br>
>> > gluster volume quota gvtest limit-usage /test 1GB<br>
>> > gluster volume quota gvtest remove /xyz<br>
>> > gluster volume quota gvtest list<br>
>> ><br>
>> > -----------------------<wbr>glusterfsd crash without the diff--------------------------<wbr>------<br>
>> ><br>
>> > /usr/local/lib/libglusterfs.<wbr>so.0(_gf_msg_backtrace_nomem+<wbr>0xf5)[0x7f6e1e950af1]<br>
>> > /usr/local/lib/libglusterfs.<wbr>so.0(gf_print_trace+0x21f)[<wbr>0x7f6e1e956943]<br>
>> > /usr/local/sbin/glusterfsd(<wbr>glusterfsd_print_trace+0x1f)[<wbr>0x409c83]<br>
>> > /lib64/libc.so.6(+0x35250)[<wbr>0x7f6e1d025250]<br>
>> > /lib64/libc.so.6(gsignal+0x37)<wbr>[0x7f6e1d0251d7]<br>
>> > /lib64/libc.so.6(abort+0x148)[<wbr>0x7f6e1d0268c8]<br>
>> > /lib64/libc.so.6(+0x74f07)[<wbr>0x7f6e1d064f07]<br>
>> > /lib64/libc.so.6(+0x7baf5)[<wbr>0x7f6e1d06baf5]<br>
>> > /lib64/libc.so.6(+0x7c3e6)[<wbr>0x7f6e1d06c3e6]<br>
>> > /usr/local/lib/libglusterfs.<wbr>so.0(__gf_free+0x311)[<wbr>0x7f6e1e981327]<br>
>> > /usr/local/lib/libglusterfs.<wbr>so.0(synctask_destroy+0x82)[<wbr>0x7f6e1e995c20]<br>
>> > /usr/local/lib/libglusterfs.<wbr>so.0(synctask_done+0x25)[<wbr>0x7f6e1e995c47]<br>
>> > /usr/local/lib/libglusterfs.<wbr>so.0(synctask_switchto+0xcf)[<wbr>0x7f6e1e996585]<br>
>> > /usr/local/lib/libglusterfs.<wbr>so.0(syncenv_processor+0x60)[<wbr>0x7f6e1e99663d]<br>
>> > /lib64/libpthread.so.0(+<wbr>0x7dc5)[0x7f6e1d7a2dc5]<br>
>> > /lib64/libc.so.6(clone+0x6d)[<wbr>0x7f6e1d0e773d]<br>
>> ><br>
>> > or<br>
>> ><br>
>> > package-string: glusterfs 3.8.12<br>
>> > /usr/local/lib/libglusterfs.<wbr>so.0(_gf_msg_backtrace_nomem+<wbr>0xf5)[0x7fa15e623af1]<br>
>> > /usr/local/lib/libglusterfs.<wbr>so.0(gf_print_trace+0x21f)[<wbr>0x7fa15e629943]<br>
>> > /usr/local/sbin/glusterfsd(<wbr>glusterfsd_print_trace+0x1f)[<wbr>0x409c83]<br>
>> > /lib64/libc.so.6(+0x35250)[<wbr>0x7fa15ccf8250]<br>
>> > /lib64/libc.so.6(gsignal+0x37)<wbr>[0x7fa15ccf81d7]<br>
>> > /lib64/libc.so.6(abort+0x148)[<wbr>0x7fa15ccf98c8]<br>
>> > /lib64/libc.so.6(+0x74f07)[<wbr>0x7fa15cd37f07]<br>
>> > /lib64/libc.so.6(+0x7dd4d)[<wbr>0x7fa15cd40d4d]<br>
>> > /lib64/libc.so.6(__libc_<wbr>calloc+0xb4)[0x7fa15cd43a14]<br>
>> > /usr/local/lib/libglusterfs.<wbr>so.0(__gf_calloc+0xa7)[<wbr>0x7fa15e653a5f]<br>
>> > /usr/local/lib/libglusterfs.<wbr>so.0(iobref_new+0x2b)[<wbr>0x7fa15e65875a]<br>
>> > /usr/local/lib/glusterfs/3.8.<wbr>12/rpc-transport/socket.so(+<wbr>0xa98c)[0x7fa153a8398c]<br>
>> > /usr/local/lib/glusterfs/3.8.<wbr>12/rpc-transport/socket.so(+<wbr>0xacbc)[0x7fa153a83cbc]<br>
>> > /usr/local/lib/glusterfs/3.8.<wbr>12/rpc-transport/socket.so(+<wbr>0xad10)[0x7fa153a83d10]<br>
>> > /usr/local/lib/glusterfs/3.8.<wbr>12/rpc-transport/socket.so(+<wbr>0xb2a7)[0x7fa153a842a7]<br>
>> > /usr/local/lib/libglusterfs.<wbr>so.0(+0x97ea9)[0x7fa15e68eea9]<br>
>> > /usr/local/lib/libglusterfs.<wbr>so.0(+0x982c6)[0x7fa15e68f2c6]<br>
>> > /lib64/libpthread.so.0(+<wbr>0x7dc5)[0x7fa15d475dc5]<br>
>> > /lib64/libc.so.6(clone+0x6d)[<wbr>0x7fa15cdba73d]<br>
>> ><br>
>> > ______________________________<wbr>_________________<br>
>> > Gluster-devel mailing list<br>
>> > <a href="mailto:Gluster-devel@gluster.org">Gluster-devel@gluster.org</a> <mailto:<a href="mailto:Gluster-devel@gluster.org">Gluster-devel@gluster.<wbr>org</a>> <mailto:<a href="mailto:Gluster-devel@gluster.org">Gluster-devel@gluster.<wbr>org</a> <mailto:<a href="mailto:Gluster-devel@gluster.org">Gluster-devel@gluster.<wbr>org</a>>><br>
>> > <a href="http://lists.gluster.org/mailman/listinfo/gluster-devel" rel="noreferrer" target="_blank">http://lists.gluster.org/<wbr>mailman/listinfo/gluster-devel</a> <<a href="http://lists.gluster.org/mailman/listinfo/gluster-devel" rel="noreferrer" target="_blank">http://lists.gluster.org/<wbr>mailman/listinfo/gluster-devel</a><wbr>> <<a href="http://lists.gluster.org/mailman/listinfo/gluster-devel" rel="noreferrer" target="_blank">http://lists.gluster.org/<wbr>mailman/listinfo/gluster-devel</a> <<a href="http://lists.gluster.org/mailman/listinfo/gluster-devel" rel="noreferrer" target="_blank">http://lists.gluster.org/<wbr>mailman/listinfo/gluster-devel</a><wbr>>><br>
>> ><br>
>> ><br>
>><br>
>><br>
><br>
</div></div></blockquote></div><br></div>