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

[Jens Axboe <axboe@kernel.dk>]

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