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 -- To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html