DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 2A52561E51
Date: Thu, 20 Jul 2017 15:23:09 -0500
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Joe Lawrence <joe.lawrence@redhat.com>
Cc: Miroslav Benes <mbenes@suse.cz>, live-patching@vger.kernel.org,
        linux-kernel@vger.kernel.org, Jessica Yu <jeyu@redhat.com>,
        Jiri Kosina <jikos@kernel.org>, Petr Mladek <pmladek@suse.com>
Subject: Re: [PATCH v2 1/2] livepatch: introduce shadow variable API
Message-ID: <20170720202309.rzwfzazpzyb4doco@treble>
References: <1498664247-12296-1-git-send-email-joe.lawrence@redhat.com>
 <1498664247-12296-2-git-send-email-joe.lawrence@redhat.com>
 <alpine.LSU.2.20.1707171709370.22628@pobox.suse.cz>
 <20170718202107.3hsptpdspr26snxc@redhat.com>
 <alpine.LSU.2.20.1707192055461.15946@pobox.suse.cz>
 <alpine.LSU.2.20.1707201644460.30401@pobox.suse.cz>
 <164836e1-2d33-ec32-46ac-d6360f671b17@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <164836e1-2d33-ec32-46ac-d6360f671b17@redhat.com>
User-Agent: Mutt/1.6.0.1 (2016-04-01)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1090
Lines: 37

On Thu, Jul 20, 2017 at 11:48:41AM -0400, Joe Lawrence wrote:
> On a related note, if we keep the allocations and memcpy, how about I
> shift around the attach/get calls like so:
> 
>   __klp_shadow_attach
>     set shadow variable member values
>     memcpy
>     add to hash
> 
>   klp_shadow_attach
>     alloc new shadow var
>     lock
>     call __klp_shadow_attach with new alloc
>     unlock
> 
>   klp_shadow_get_or_attach
>     be optimistic, call klp_shadow_get (if found, return it)
>     be pessimistic, alloc new shadow var
>     lock
>       call klp_shadow_get again
>       if unlikely found
>         kfree unneeded alloc
>       else
>         call __klp_shadow_attach with new alloc
>     unlock
>     return whichever shadow var we used
> 
> This way both calls can accept gfp_flags that may sleep, with the only
> downside that klp_shadow_get_or_attach may allocate an unnecessary
> shadow variable in the unlikely case that it's found on the second
> klp_shadow_get attempt (under the lock).  No more clunky "bool lock"
> flag either. :)

Sounds good to me!

-- 
Josh