Merge pull request #1 from osvunikernel/thread_safe

make it thread-safe

include/ext4_bcache.h

@@ -65,10 +65,26 @@ struct ext4_block {
 
 struct ext4_bcache;
 
+/**@brief buffer state bits
+ *
+ *  - BC_UPTODATE: Buffer contains valid data.
+ *  - BC_DIRTY: Buffer is dirty.
+ *  - BC_FLUSH: Buffer will be immediately flushed,
+ *              when no one references it.
+ *  - BC_TMP: Buffer will be dropped once its refctr
+ *            reaches zero.
+ */
+enum bcache_state_bits {
+	BC_UPTODATE, // Mostly set in ext4_block_get() after reading from disk
+	BC_DIRTY,    // Most importantly set in ext4_bcache_set_dirty()
+	BC_FLUSH,    // Set in journal code
+	BC_TMP       // Set in journal code
+};
+
 /**@brief   Single block descriptor*/
 struct ext4_buf {
 	/**@brief   Flags*/
-	int flags;
+	char flags[BC_TMP + 1];
 
 	/**@brief   Logical block address*/
 	uint64_t lba;
@@ -148,30 +164,14 @@ struct ext4_bcache {
 	SLIST_HEAD(ext4_buf_dirty, ext4_buf) dirty_list;
 };
 
-/**@brief buffer state bits
- *
- *  - BC♡UPTODATE: Buffer contains valid data.
- *  - BC_DIRTY: Buffer is dirty.
- *  - BC_FLUSH: Buffer will be immediately flushed,
- *              when no one references it.
- *  - BC_TMP: Buffer will be dropped once its refctr
- *            reaches zero.
- */
-enum bcache_state_bits {
-	BC_UPTODATE,
-	BC_DIRTY,
-	BC_FLUSH,
-	BC_TMP
-};
-
 #define ext4_bcache_set_flag(buf, b)    \
-	(buf)->flags |= 1 << (b)
+	(buf)->flags[b] = 1
 
 #define ext4_bcache_clear_flag(buf, b)    \
-	(buf)->flags &= ~(1 << (b))
+	(buf)->flags[b] = 0
 
 #define ext4_bcache_test_flag(buf, b)    \
-	(((buf)->flags & (1 << (b))) >> (b))
+	(((buf)->flags[b] == 1 ))
 
 static inline void ext4_bcache_set_dirty(struct ext4_buf *buf) {
 	ext4_bcache_set_flag(buf, BC_UPTODATE);
@@ -239,12 +239,6 @@ struct ext4_buf *ext4_buf_lowest_lru(struct ext4_bcache *bc);
  * @param   buf buffer*/
 void ext4_bcache_drop_buf(struct ext4_bcache *bc, struct ext4_buf *buf);
 
-/**@brief   Invalidate a buffer.
- * @param   bc block cache descriptor
- * @param   buf buffer*/
-void ext4_bcache_invalidate_buf(struct ext4_bcache *bc,
-				struct ext4_buf *buf);
-
 /**@brief   Invalidate a range of buffers.
  * @param   bc block cache descriptor
  * @param   from starting lba

include/ext4_fs.h

@@ -67,6 +67,15 @@ struct ext4_fs {
 	struct jbd_fs *jbd_fs;
 	struct jbd_journal *jbd_journal;
 	struct jbd_trans *curr_trans;
+
+	void (*inode_alloc_lock)();
+	void (*inode_alloc_unlock)();
+
+	void (*block_alloc_lock)();
+	void (*block_alloc_unlock)();
+
+	void (*bcache_lock)();
+	void (*bcache_unlock)();
 };
 
 struct ext4_block_group_ref {

src/ext4_balloc.c

@@ -149,7 +149,8 @@ ext4_balloc_verify_bitmap_csum(struct ext4_sblock *sb,
 #define ext4_balloc_verify_bitmap_csum(...) true
 #endif
 
-int ext4_balloc_free_block(struct ext4_inode_ref *inode_ref, ext4_fsblk_t baddr)
+static int
+__ext4_balloc_free_block(struct ext4_inode_ref *inode_ref, ext4_fsblk_t baddr)
 {
 	struct ext4_fs *fs = inode_ref->fs;
 	struct ext4_sblock *sb = &fs->sb;
@@ -223,14 +224,25 @@ int ext4_balloc_free_block(struct ext4_inode_ref *inode_ref, ext4_fsblk_t baddr)
 		ext4_fs_put_block_group_ref(&bg_ref);
 		return rc;
 	}
+	fs->bcache_lock();
 	ext4_bcache_invalidate_lba(fs->bdev->bc, baddr, 1);
+	fs->bcache_unlock();
 	/* Release block group reference */
 	rc = ext4_fs_put_block_group_ref(&bg_ref);
 
 	return rc;
 }
 
-int ext4_balloc_free_blocks(struct ext4_inode_ref *inode_ref,
+int ext4_balloc_free_block(struct ext4_inode_ref *inode_ref, ext4_fsblk_t baddr)
+{
+	inode_ref->fs->block_alloc_lock();
+	int rc = __ext4_balloc_free_block(inode_ref, baddr);
+	inode_ref->fs->block_alloc_unlock();
+	return rc;
+}
+
+static int
+__ext4_balloc_free_blocks(struct ext4_inode_ref *inode_ref,
 			    ext4_fsblk_t first, uint32_t count)
 {
 	int rc = EOK;
@@ -342,14 +354,26 @@ int ext4_balloc_free_blocks(struct ext4_inode_ref *inode_ref,
 
 	}
 
+	fs->bcache_lock();
 	ext4_bcache_invalidate_lba(fs->bdev->bc, start_block, blk_cnt);
+	fs->bcache_unlock();
 	/*All blocks should be released*/
 	ext4_assert(count == 0);
 
 	return rc;
 }
 
-int ext4_balloc_alloc_block(struct ext4_inode_ref *inode_ref,
+int ext4_balloc_free_blocks(struct ext4_inode_ref *inode_ref,
+                          ext4_fsblk_t first, uint32_t count)
+{
+	inode_ref->fs->block_alloc_lock();
+	int rc = __ext4_balloc_free_blocks(inode_ref, first, count);
+	inode_ref->fs->block_alloc_unlock();
+	return rc;
+}
+
+static int
+__ext4_balloc_alloc_block(struct ext4_inode_ref *inode_ref,
 			    ext4_fsblk_t goal,
 			    ext4_fsblk_t *fblock)
 {
@@ -582,6 +606,16 @@ int ext4_balloc_alloc_block(struct ext4_inode_ref *inode_ref,
 	return r;
 }
 
+int ext4_balloc_alloc_block(struct ext4_inode_ref *inode_ref,
+                        ext4_fsblk_t goal,
+                        ext4_fsblk_t *fblock)
+{
+	inode_ref->fs->block_alloc_lock();
+	int rc = __ext4_balloc_alloc_block(inode_ref, goal, fblock);
+	inode_ref->fs->block_alloc_unlock();
+	return rc;
+}
+
 int ext4_balloc_try_alloc_block(struct ext4_inode_ref *inode_ref,
 				ext4_fsblk_t baddr, bool *free)
 {

src/ext4_bcache.c

@@ -153,11 +153,19 @@ ext4_buf_lookup(struct ext4_bcache *bc, uint64_t lba)
 	return RB_FIND(ext4_buf_lba, &bc->lba_root, &tmp);
 }
 
+// No need for locking
+// Called by ext4_block_cache_shake() ONLY
+// PROTECTED in ext4_block_get_noread() because the only
+// caller - ext4_block_cache_shake() - is protected
 struct ext4_buf *ext4_buf_lowest_lru(struct ext4_bcache *bc)
 {
 	return RB_MIN(ext4_buf_lru, &bc->lru_root);
 }
 
+// No need for locking
+// Called by ext4_block_cache_shake() and functions in this file
+// PROTECTED in ext4_block_get_noread() because the
+// caller - ext4_block_cache_shake() - is protected
 void ext4_bcache_drop_buf(struct ext4_bcache *bc, struct ext4_buf *buf)
 {
 	/* Warn on dropping any referenced buffers.*/
@@ -178,6 +186,10 @@ void ext4_bcache_drop_buf(struct ext4_bcache *bc, struct ext4_buf *buf)
 	bc->ref_blocks--;
 }
 
+/**@brief   Invalidate a buffer.
+ * @param   bc block cache descriptor
+ * @param   buf buffer*/
+static
 void ext4_bcache_invalidate_buf(struct ext4_bcache *bc,
 				struct ext4_buf *buf)
 {
@@ -191,6 +203,9 @@ void ext4_bcache_invalidate_buf(struct ext4_bcache *bc,
 	ext4_bcache_clear_dirty(buf);
 }
 
+// Called by ext4_balloc_free_block/s()
+// Protect by LOCK
+// PROTECTED in the callers
 void ext4_bcache_invalidate_lba(struct ext4_bcache *bc,
 				uint64_t from,
 				uint32_t cnt)
@@ -205,6 +220,10 @@ void ext4_bcache_invalidate_lba(struct ext4_bcache *bc,
 	}
 }
 
+// Called by:
+// - ext4_bcache_alloc() - No need to lock (see below)
+// - ext4_block_flush_lba() - probably need to lock there - PROTECTED
+// - 2 methods in ext4_journal() not used right now - ignore
 struct ext4_buf *
 ext4_bcache_find_get(struct ext4_bcache *bc, struct ext4_block *b,
 		     uint64_t lba)
@@ -231,6 +250,10 @@ ext4_bcache_find_get(struct ext4_bcache *bc, struct ext4_block *b,
 	return buf;
 }
 
+// Called by ext4_block_get_noread() ONLY which also calls ext4_block_cache_shake()
+// which calls numbers of function here. So no need for locking,
+// and maybe we should lock in ext4_block_get_noread()?
+// PROTECTED in ext4_block_get_noread()
 int ext4_bcache_alloc(struct ext4_bcache *bc, struct ext4_block *b,
 		      bool *is_new)
 {
@@ -267,6 +290,8 @@ int ext4_bcache_alloc(struct ext4_bcache *bc, struct ext4_block *b,
 	return EOK;
 }
 
+// Called by 3 functions - ext4_block_flush_lba(), ext4_block_get() and ext4_block_set()
+// Protected there
 int ext4_bcache_free(struct ext4_bcache *bc, struct ext4_block *b)
 {
 	struct ext4_buf *buf = b->buf;

src/ext4_blockdev.c

@@ -170,16 +170,19 @@ int ext4_block_flush_buf(struct ext4_blockdev *bdev, struct ext4_buf *buf)
 	return EOK;
 }
 
+//Used indirectly by journal-ing code
 int ext4_block_flush_lba(struct ext4_blockdev *bdev, uint64_t lba)
 {
 	int r = EOK;
 	struct ext4_buf *buf;
 	struct ext4_block b;
+	bdev->fs->bcache_lock();
 	buf = ext4_bcache_find_get(bdev->bc, &b, lba);
 	if (buf) {
 		r = ext4_block_flush_buf(bdev, buf);
 		ext4_bcache_free(bdev->bc, &b);
 	}
+	bdev->fs->bcache_unlock();
 	return r;
 }
 
@@ -244,26 +247,32 @@ int ext4_block_get_noread(struct ext4_blockdev *bdev, struct ext4_block *b,
 int ext4_block_get(struct ext4_blockdev *bdev, struct ext4_block *b,
 		   uint64_t lba)
 {
+	bdev->fs->bcache_lock();
 	int r = ext4_block_get_noread(bdev, b, lba);
-	if (r != EOK)
+	if (r != EOK) {
+		bdev->fs->bcache_unlock();
 		return r;
+	}
 
 	if (ext4_bcache_test_flag(b->buf, BC_UPTODATE)) {
 		/* Data in the cache is up-to-date.
 		 * Reading from physical device is not required */
+		bdev->fs->bcache_unlock();
 		return EOK;
 	}
 
 	r = ext4_blocks_get_direct(bdev, b->data, lba, 1);
 	if (r != EOK) {
 		ext4_bcache_free(bdev->bc, b);
+		bdev->fs->bcache_unlock();
 		b->lb_id = 0;
 		return r;
 	}
 
 	/* Mark buffer up-to-date, since
 	 * fresh data is read from physical device just now. */
 	ext4_bcache_set_flag(b->buf, BC_UPTODATE);
+	bdev->fs->bcache_unlock();
 	return EOK;
 }
 
@@ -275,7 +284,10 @@ int ext4_block_set(struct ext4_blockdev *bdev, struct ext4_block *b)
 	if (!bdev->bdif->ph_refctr)
 		return EIO;
 
-	return ext4_bcache_free(bdev->bc, b);
+	bdev->fs->bcache_lock();
+	int rc = ext4_bcache_free(bdev->bc, b);
+	bdev->fs->bcache_unlock();
+	return rc;
 }
 
 int ext4_blocks_get_direct(struct ext4_blockdev *bdev, void *buf, uint64_t lba,

src/ext4_fs.c

@@ -1657,8 +1657,10 @@ int ext4_fs_append_inode_dblk(struct ext4_inode_ref *inode_ref,
 		if (rc != EOK)
 			return rc;
 
-		*fblock = current_fsblk;
-		ext4_assert(*fblock);
+		if (fblock) {
+			*fblock = current_fsblk;
+			ext4_assert(*fblock);
+		}
 
 		ext4_inode_set_size(inode_ref->inode, inode_size + block_size);
 		inode_ref->dirty = true;
@@ -1702,7 +1704,8 @@ int ext4_fs_append_inode_dblk(struct ext4_inode_ref *inode_ref,
 	ext4_inode_set_size(inode_ref->inode, inode_size + block_size);
 	inode_ref->dirty = true;
 
-	*fblock = phys_block;
+	if (fblock)
+		*fblock = phys_block;
 	*iblock = new_block_idx;
 
 	return EOK;

src/ext4_ialloc.c

@@ -110,6 +110,7 @@ static uint32_t ext4_ialloc_bitmap_csum(struct ext4_sblock *sb,	void *bitmap)
 #define ext4_ialloc_bitmap_csum(...) 0
 #endif
 
+//TODO: Check this one when called by ext4_fs_get_block_group_ref()
 void ext4_ialloc_set_bitmap_csum(struct ext4_sblock *sb, struct ext4_bgroup *bg,
 				 void *bitmap __unused)
 {
@@ -155,7 +156,8 @@ ext4_ialloc_verify_bitmap_csum(struct ext4_sblock *sb, struct ext4_bgroup *bg,
 #define ext4_ialloc_verify_bitmap_csum(...) true
 #endif
 
-int ext4_ialloc_free_inode(struct ext4_fs *fs, uint32_t index, bool is_dir)
+static int
+__ext4_ialloc_free_inode(struct ext4_fs *fs, uint32_t index, bool is_dir)
 {
 	struct ext4_sblock *sb = &fs->sb;
 
@@ -226,7 +228,16 @@ int ext4_ialloc_free_inode(struct ext4_fs *fs, uint32_t index, bool is_dir)
 	return EOK;
 }
 
-int ext4_ialloc_alloc_inode(struct ext4_fs *fs, uint32_t *idx, bool is_dir)
+int ext4_ialloc_free_inode(struct ext4_fs *fs, uint32_t index, bool is_dir)
+{
+	fs->inode_alloc_lock();
+	int rc = __ext4_ialloc_free_inode(fs, index, is_dir);
+	fs->inode_alloc_unlock();
+	return rc;
+}
+
+static int
+__ext4_ialloc_alloc_inode(struct ext4_fs *fs, uint32_t *idx, bool is_dir)
 {
 	struct ext4_sblock *sb = &fs->sb;
 
@@ -365,6 +376,14 @@ int ext4_ialloc_alloc_inode(struct ext4_fs *fs, uint32_t *idx, bool is_dir)
 	return ENOSPC;
 }
 
+int ext4_ialloc_alloc_inode(struct ext4_fs *fs, uint32_t *idx, bool is_dir)
+{
+	fs->inode_alloc_lock();
+	int rc = __ext4_ialloc_alloc_inode(fs, idx, is_dir);
+	fs->inode_alloc_unlock();
+	return rc;
+}
+
 /**
  * @}
  */

src/ext4_trans.c

@@ -68,10 +68,9 @@ int ext4_trans_block_get_noread(struct ext4_blockdev *bdev,
 			  struct ext4_block *b,
 			  uint64_t lba)
 {
+	bdev->fs->bcache_lock();
 	int r = ext4_block_get_noread(bdev, b, lba);
-	if (r != EOK)
-		return r;
-
+	bdev->fs->bcache_unlock();
 	return r;
 }