On Sun, Apr 25, 2021 at 01:42:59PM -0700, Bart Van Assche wrote:
> On 4/25/21 1:57 AM, Ming Lei wrote:
> > + spin_lock_irqsave(&drv_tags->lock, flags);
> > + list_for_each_entry(page, &tags->page_list, lru) {
> > + unsigned long start = (unsigned long)page_address(page);
> > + unsigned long end = start + order_to_size(page->private);
> > + int i;
> > +
> > + for (i = 0; i < set->queue_depth; i++) {
> > + struct request *rq = drv_tags->rqs[i];
> > + unsigned long rq_addr = (unsigned long)rq;
> > +
> > + if (rq_addr >= start && rq_addr < end) {
> > + WARN_ON_ONCE(refcount_read(&rq->ref) != 0);
> > + cmpxchg(&drv_tags->rqs[i], rq, NULL);
> > + }
> > + }
> > + }
> > + spin_unlock_irqrestore(&drv_tags->lock, flags);
>
> Using cmpxchg() on set->tags[] is only safe if all other set->tags[]
> accesses are changed into WRITE_ONCE() or READ_ONCE().
Why?
Semantic of cmpxchg() is to modify value pointed by the address if its
old value is same with passed 'rq'. That is exactly what we need.
writting 'void *' is always atomic. if someone has touched
'->rqs[tag]', cmpxchg() won't modify the value.
Thanks,
Ming
