Bug 192863 - Data race caused by double increment of pq->pq_cnt
Summary: Data race caused by double increment of pq->pq_cnt
Status: Closed Works As Intended
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 10.0-RELEASE
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-08-20 18:42 UTC by pfonseca
Modified: 2014-09-12 15:51 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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:

/usr/src/sys/vm/vm_page.h:246

       240  vm_pagequeue_cnt_add(struct vm_pagequeue *pq, int addend)
       241  {
       242
       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."