[Gluster-devel] crypt xlator bug

[Pranith Kumar Karampuri]

On 04/02/2015 12:27 AM, Raghavendra Talur wrote:
>
>
> On Wed, Apr 1, 2015 at 10:34 PM, Justin Clift <justin at gluster.org 
> <mailto:justin at gluster.org>> wrote:
>
>     On 1 Apr 2015, at 10:57, Emmanuel Dreyfus <manu at netbsd.org
>     <mailto:manu at netbsd.org>> 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
>         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 at gluster.org <mailto:Gluster-devel at gluster.org>
>     http://www.gluster.org/mailman/listinfo/gluster-devel
>
>
>
>
> -- 
> *Raghavendra Talur *
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.gluster.org/pipermail/gluster-devel/attachments/20150402/02830a0a/attachment-0001.html>