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

[nick <xerofoify@gmail.com>]

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
> 
> ------------------------------------------------------------------------------
> 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
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 
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
is that can there be many readers to one writer here as if so reader/writer
spin locks may be better.
Nick  

------------------------------------------------------------------------------
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