NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: kern/60584: vfs_bio: getnewbuf() can deadlock when fresh allocation is refused and no buffer is reclaimable



The following reply was made to PR kern/60584; it has been noted by GNATS.

From: Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: tsutsui%ceres.dti.ne.jp@localhost
Subject: Re: kern/60584: vfs_bio: getnewbuf() can deadlock when fresh allocation
	 is refused and no buffer is reclaimable
Date: Fri, 21 Aug 2026 04:42:09 +0900

 I wrote:
 
 > >Number:         60584
 > >Category:       kern
 > >Synopsis:       vfs_bio: getnewbuf() can deadlock when fresh allocation is refused and no buffer is reclaimable
 
 I looked further into the history of buf_lotsfree().
 
 The probabilistic allocation policy was introduced in vfs_bio.c
 r1.114 to control buffer cache growth and avoid thrashing between
 the high and low water marks.
 
  https://github.com/NetBSD/src/commit/3e8ad532aa3cba09a96fa5ddfa876318a5133107
 
 The next r1.115 made BQ_AGE take precedence over fresh allocation,
 
  https://github.com/NetBSD/src/commit/249b0cda71b6c885c87eee8f0e7d3ad9ca203ef1
 
 and r1.136 adjusted the probability curve to start at the low water mark.
 
  https://github.com/NetBSD/src/commit/249b0cda71b6c885c87eee8f0e7d3ad9ca203ef1
 
 
 >From these changes, my understanding is that buf_lotsfree() is mainly
 intended to control buffer cache growth.  A false return seems to mean
 that recycling an existing buffer is preferable to growing the cache.
 
 The current getnewbuf() path can be summarized roughly as follows:
 
 ```
         if (!from_bufq && buf_lotsfree()) {
                 /*
                  * Try a fresh allocation according to the existing
                  * probabilistic growth policy.
                  */
                 ...
         }
 
         /*
          * Look for a buffer which can actually be recycled from
          * BQ_AGE or BQ_LRU.
          */
         ...
 
         if (bp != NULL) {
                 /*
                  * Recycle the buffer as before.
                  */
                 ...
         } else {
                 /*
                  * At this point getnewbuf() has not only preferred
                  * recycling as a policy decision, but has also found
                  * that there is in fact no buffer which can be recycled.
                  */
 
                 /*
                  * Sleep on needbuffer_cv,
                  * but no forward path to wake up this in case of this PR
                  */
                 ...
         }
 ```
 
 The problem reported in this PR seems to occur in the last
 "Sleep on needbuffer_cv" case as noted above.
 
 There is no reusable buffer, but getnewbuf() still waits for one to
 become available.  If releasing such a buffer itself requires another
 buffer allocation, there is no way to make progress.
 
 I think one possible way to avoid this would be to add another
 PR_NOWAIT allocation attempt before sleeping, while bufmem is still
 below bufmem_hiwater.
 
 The resulting path would look roughly like this:
 
 ```
         if (!from_bufq && buf_lotsfree()) {
                 /*
                  * Existing probabilistic fresh allocation.
                  */
                 ...
         }
 
         /*
          * Existing BQ_AGE/BQ_LRU scan.
          */
         ...
 
         if (bp != NULL) {
                 /*
                  * Existing recycle path.
                  */
                 ...
         } else {
 -->             /*
 -->              * No buffer can actually be recycled.
 -->              */
 -->             if (!from_bufq && bufmem < bufmem_hiwater) {
 -->                     /*
 -->                      * Try one fresh allocation for forward
 -->                      * progress.
 -->                      */
 -->                     ...
 -->             }
 
                 /*
                  * Sleep on needbuffer_cv only if that also fails.
                  */
                 ...
         }
 ```
 
 This would leave the existing buf_lotsfree() policy in place for the
 normal case.  The additional allocation would only be attempted after
 getnewbuf() has actually found that recycling is not possible, and it
 would not allow the cache to grow past bufmem_hiwater.
 
 I have not tested this change yet, and I may be missing some reason
 why a fresh allocation should not be attempted at this point.
 
 It would also not address the case where bufmem is already at or
 above the high water mark.
 
 Comments from people more familiar with the buffer cache and UVM
 would be appreciated.
 
 Thanks,
 ---
 Izumi Tsutsui
 



Home | Main Index | Thread Index | Old Index