Bug 192863

Summary: Data race caused by double increment of pq->pq_cnt
Product: Base System Reporter: pfonseca
Component: kernAssignee: freebsd-bugs (Nobody) <bugs>
Status: Closed Works As Intended    
Severity: Affects Only Me    
Priority: ---    
Version: 10.0-RELEASE   
Hardware: Any   
OS: Any   

Description pfonseca 2014-08-20 18:42:02 UTC
I've found that there are two accesses to "pq->pq_cnt" that are not synchronized and that can race in FreeBSD 10.0. One of the accesses increments the variable in function "vm_pagequeue_cnt_add()" while the other reads its value in function "vm_pageout_scan()"

Strangely "pq->pq_cnt" is currently incremented twice in the function "vm_pagequeue_cnt_add()" (in one instance this is done atomically, in the other it's not).

Racing accesses:


       240  vm_pagequeue_cnt_add(struct vm_pagequeue *pq, int addend)
       241  {
       243  #ifdef notyet
       244      vm_pagequeue_assert_locked(pq);
       245  #endif
==>    246      pq->pq_cnt += addend;
       247      atomic_add_int(pq->pq_vcnt, addend);
       248  }

/usr/src/sys/vm/vm_pageout.c:962 (vm_pageout_scan)

       961      pq = &vmd->vmd_pagequeues[PQ_INACTIVE];
==>    962      maxscan = pq->pq_cnt;
       963      vm_pagequeue_lock(pq);
       964      queues_locked = TRUE;
       965      for (m = TAILQ_FIRST(&pq->pq_pl);
       966           m != NULL && maxscan-- > 0 && page_shortage > 0;
Comment 1 pfonseca 2014-09-12 15:51:30 UTC
After some disussion with developers, the conclussion was that the data race is benign:

"The pageout code does not care about exact queue length, it only takes
it as an advise to not over-scan the queue. The fact that read gets a
snapshot value which might be outdated immediately does not matter in
this situation. At worst, scan would fall off the end of queue, if it is
drained by other means in parallel."