Commit 64467825 authored by Ziye Yang's avatar Ziye Yang
Browse files

blobfs: move the location of next buffer check. 

This patch address the issue:
https://github.com/spdk/spdk/issues/151

.

For cache_append_no_cache in cache_ut testcase,
there is resource contention for buffer among two
threads in the following two functions.
Thread 0: cache_free_buffers
Thread1: __file_flush_done

When the thread1 execuctes __file_flush_done,
it calls the call back: __sem_post defined in
following statement in spdk_file_sync

_file_sync(file, channel, __sem_post, &channel->sem);

Thus Thread 0 will execute next function
cache_buffers, and it frees the buffer.

Then Thread 1 continues executing the remaining statements
in __file_flush_done with the assert function, and touches
the space already freed.

So it will be safe to move ahead the next buffer check.

Change-Id: Ic007b3481f4e3a17d47eeca5c9c802001949a5ab
Signed-off-by: default avatarZiye Yang <ziye.yang@intel.com>
parent 4eafea03

lib/blobfs/blobfs.c

+7 −7
Original line number Diff line number Diff line
@@ -1622,6 +1622,13 @@ __file_flush_done(void *arg, int bserrno) 
		}

	}



	/*

	 * Assert that there is no cached data that extends past the end of the underlying

	 *  blob.

	 */

	assert(next == NULL || next->offset < __file_get_blob_size(file) ||

	       next->bytes_filled == 0);



	if (sync_req != NULL) {

		BLOBFS_TRACE(file, "set xattr length 0x%jx\n", file->length_flushed);

		spdk_blob_md_set_xattr(file->blob, "length", &file->length_flushed,
@@ -1634,13 +1641,6 @@ __file_flush_done(void *arg, int bserrno) 
		__file_cache_finish_sync(file);

	}



	/*

	 * Assert that there is no cached data that extends past the end of the underlying

	 *  blob.

	 */

	assert(next == NULL || next->offset < __file_get_blob_size(file) ||

	       next->bytes_filled == 0);



	__file_flush(args);

}