On Thu, Apr 02, 2015 at 01:58:39PM +0530, Raghavendra Bhat wrote: > On Thursday 02 April 2015 01:00 PM, Pranith Kumar Karampuri wrote: > > > >On 04/02/2015 12:27 AM, Raghavendra Talur wrote: > >> > >> > >>On Wed, Apr 1, 2015 at 10:34 PM, Justin Clift <justin@xxxxxxxxxxx > >><mailto:justin@xxxxxxxxxxx>> wrote: > >> > >> On 1 Apr 2015, at 10:57, Emmanuel Dreyfus <manu@xxxxxxxxxx > >> <mailto:manu@xxxxxxxxxx>> wrote: > >> > Hi > >> > > >> > crypt.t was recently broken in NetBSD regression. The glusterfs > >> returns > >> > a node with file type invalid to FUSE, and that breaks the test. > >> > > >> > After running a git bisect, I found the offending commit after > >> which > >> > this behavior appeared: > >> > 8a2e2b88fc21dc7879f838d18cd0413dd88023b7 > >> > mem-pool: invalidate memory on GF_FREE to aid debugging > >> > > >> > This means the bug has always been there, but this debugging aid > >> > caused it to be reliable. > >> > >> Sounds like that commit is a good win then. :) > >> > >> Harsha/Pranith/Lala, your names are on the git blame for crypt.c... > >> any ideas? :) > >> > >> > >>I found one issue that local is not allocated using GF_CALLOC and with a > >>mem-type. > >>This is a patch which *might* fix it. > >> > >>diff --git a/xlators/encryption/crypt/src/crypt-mem-types.h > >>b/xlators/encryption/crypt/src/crypt-mem-types.h > >>index 2eab921..c417b67 100644 > >>--- a/xlators/encryption/crypt/src/crypt-mem-types.h > >>+++ b/xlators/encryption/crypt/src/crypt-mem-types.h > >>@@ -24,6 +24,7 @@ enum gf_crypt_mem_types_ { > >> gf_crypt_mt_key, > >> gf_crypt_mt_iovec, > >> gf_crypt_mt_char, > >>+ gf_crypt_mt_local, > >> gf_crypt_mt_end, > >> }; > >>diff --git a/xlators/encryption/crypt/src/crypt.c > >>b/xlators/encryption/crypt/src/crypt.c > >>index ae8cdb2..63c0977 100644 > >>--- a/xlators/encryption/crypt/src/crypt.c > >>+++ b/xlators/encryption/crypt/src/crypt.c > >>@@ -48,7 +48,7 @@ static crypt_local_t *crypt_alloc_local(call_frame_t > >>*frame, xlator_t *this, > >> { > >> crypt_local_t *local = NULL; > >>- local = mem_get0(this->local_pool); > >>+ local = GF_CALLOC (sizeof (*local), 1, gf_crypt_mt_local); > >local is using the memory from pool earlier(i.e. with mem_get0()). Which > >seems ok to me. Changing it this way will include memory allocation in fop > >I/O path which is why xlators generally use the mem-pool approach. > > > >Pranith > > I think, crypt xlator should do a mem_put of local after doing STACK_UNWIND > like other xlators which also use mem_get for local (such as AFR). I am > suspecting crypt not doing mem_put might be the reason for the bug > mentioned. I've looked at this now too. The use of mem_get0() seems fine to me. But indeed, calling mem_put() is missing. Whenever the crypt_local_t should be released, mem_put() should get called, just like any GF_CALLOC/GF_FREE combinations. HTH, Niels > > Regards, > Raghavendra Bat > > >> if (!local) { > >> gf_log(this->name, GF_LOG_ERROR, "out of memory"); > >> return NULL; > >> > >> > >>Niels should be able to recognize if this is sufficient fix or not. > >> > >>Thanks, > >>Raghavendra Talur > >> > >> + Justin > >> > >> -- > >> GlusterFS - http://www.gluster.org > >> > >> An open source, distributed file system scaling to several > >> petabytes, and handling thousands of clients. > >> > >> My personal twitter: twitter.com/realjustinclift > >> <http://twitter.com/realjustinclift> > >> > >> _______________________________________________ > >> Gluster-devel mailing list > >> Gluster-devel@xxxxxxxxxxx <mailto:Gluster-devel@xxxxxxxxxxx> > >> http://www.gluster.org/mailman/listinfo/gluster-devel > >> > >> > >> > >> > >>-- > >>*Raghavendra Talur * > >> > > > > > > > >_______________________________________________ > >Gluster-devel mailing list > >Gluster-devel@xxxxxxxxxxx > >http://www.gluster.org/mailman/listinfo/gluster-devel > > _______________________________________________ > Gluster-devel mailing list > Gluster-devel@xxxxxxxxxxx > http://www.gluster.org/mailman/listinfo/gluster-devel
Attachment:
pgpSdmEG2cezI.pgp
Description: PGP signature
_______________________________________________ Gluster-devel mailing list Gluster-devel@xxxxxxxxxxx http://www.gluster.org/mailman/listinfo/gluster-devel
