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

Reply via email to