Re: [PATCH] f2fs crypto: add rwsem to avoid data races

[Nicholas Krause <xerofoify@gmail.com>]

On May 19, 2015 8:38:59 PM EDT, Jaegeuk Kim <jaegeuk@kernel.org> wrote:
>On Tue, May 19, 2015 at 10:35:30AM -0400, nick wrote:
>> 
>> 
>> On 2015-05-19 10:29 AM, Theodore Ts'o wrote:
>> > On Mon, May 18, 2015 at 10:36:41PM -0700, Jaegeuk Kim wrote:
>> >> Previoulsy, fi->i_crypt_info was not covered by any lock,
>resulting in
>> >> memory leak.
>> >>
>> >> This patch adds a rwsem to avoid leaking objects on i_crypt_info.
>> >>
>> >> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>> > 
>> > I'm not sure we need an rwsem to fix this issue.  In terms of
>> > serializing the creation and deletion of the structure, it should
>be
>> > possible to use an cmpxchg() on the pointer itself.  (e.g., if we
>lose
>> > the race on the creation side, we just release our structure and
>use
>> > the one that the winner allocated).
>> > 
>> > If we do end up needing to serialize access to the tfm in the
>> > i_crypt_info object for datapath reads/writes, then we might need a
>> > mutex, but I think that should be it, no?
>> > 
>> > 	       	     	   	       - Ted
>> > 
>> I have to agree with Ted here, as mutual exclusion locking is ideal
>> for the scenario here of a reader vs writer exclusion. My only
>concern
>
>What I'm saying is writer vs writer actually.
>
>> is that can there be many readers to one writer here as if so
>reader/writer
>> spin locks may be better.
>
>I could write another patch using a rwlock by removing needless
>down_write for
>f2fs_free_encryption_info.
>
That sounds good to me, I wasn't sure if it was writer vs writer or reader vs writer. However rwlocks are the easiest way to avoid dealing with semaphones as you seem interested in avoiding writing one into the f2fs code with this patch. 
Nick 
>Thanks,
>
>> Nick  

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y