Re: [PATCH v2 opensm] Add support for synchronizing in memory files with storage

[Hal Rosenstock <hal-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org>]

On 12/21/2013 8:57 AM, Bart Van Assche wrote:
> On 12/21/13 13:54, Hal Rosenstock wrote:
>> -int osm_db_store(IN osm_db_domain_t * p_domain)
>> +int osm_db_store(IN osm_db_domain_t * p_domain,
>> +		 IN boolean_t fsync_high_avail_files)
> 
> Version two of this patch looks fine to me, but while reviewing this
> patch I noticed two issues in osm_db_store() that might need to be
> addressed:
> * With p_domain_imp->dirty == FALSE cl_spinlock_release() is called
> without having invoked cl_spinlock_acquire() first. Can the 'dirty' flag
> be modified concurrently with the test of the 'dirty' flag ? If so, the
> test of this flag probably has to occur after the spinlock has been
> acquired.

Yes; I'll fix this in a subsequent patch.

> * If the malloc() call for allocating the temporary file name fails then
> strcpy() will be called with NULL as first argument. Shouldn't the
> return value of malloc() be checked ?

Ditto.

Thanks.

-- Hal

> Thanks,
> 
> Bart.
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html