Re: [PATCH 2/5] testb: implement block device operations

On 08/05/2017 09:51 AM, Shaohua Li wrote:

+static struct testb_page *testb_insert_page(struct testb *testb,
+	sector_t sector, unsigned long *lock_flag)
+{
+	u64 idx;
+	struct testb_page *t_page;
+
+	assert_spin_locked(&testb->t_dev->lock);
+
+	t_page = testb_lookup_page(testb, sector, true);
+	if (t_page)
+		return t_page;
+
+	spin_unlock_irqrestore(&testb->t_dev->lock, *lock_flag);

Passing in lock flags through several functions is kind of iffy. It also
used to break on some architectures, though I don't think that's the
case anymore. The problem is that it usually ends up being a code
locking, instead of a data structure locking. The latter is much
cleaner.

+static int copy_from_testb(struct testb *testb, struct page *dest,
+	unsigned int off, sector_t sector, size_t n, unsigned long *lock_flag)

This also gets the lock flags passed in, but doesn't use it.

-- 
Jens Axboe