Dead code in uvm_pageout()

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Dead code in uvm_pageout()

Martin Pieuchot
As soon as an entry is found on `pmr_control.allocs' the boolean
`work_done' will be set to true.  So it is impossible to reach the
case below that sets UVM_PMA_FAIL.

CID 1453061 Logically dead code

Ok?

Index: uvm/uvm_pdaemon.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_pdaemon.c,v
retrieving revision 1.85
diff -u -p -u -4 -r1.85 uvm_pdaemon.c
--- uvm/uvm_pdaemon.c 30 Dec 2019 23:58:38 -0000 1.85
+++ uvm/uvm_pdaemon.c 24 Mar 2020 14:12:22 -0000
@@ -209,9 +209,8 @@ void
 uvm_pageout(void *arg)
 {
  struct uvm_constraint_range constraint;
  struct uvm_pmalloc *pma;
- int work_done;
  int npages = 0;
 
  /* ensure correct priority and set paging parameters... */
  uvm.pagedaemon_proc = curproc;
@@ -222,9 +221,8 @@ uvm_pageout(void *arg)
  uvm_unlock_pageq();
 
  for (;;) {
  long size;
-   work_done = 0; /* No work done this iteration. */
 
  uvm_lock_fpageq();
  if (!uvm_nowait_failed && TAILQ_EMPTY(&uvm.pmr_control.allocs)) {
  msleep_nsec(&uvm.pagedaemon, &uvm.fpageqlock, PVM,
@@ -281,9 +279,8 @@ uvm_pageout(void *arg)
  if (pma != NULL ||
     ((uvmexp.free - BUFPAGES_DEFICIT) < uvmexp.freetarg) ||
     ((uvmexp.inactive + BUFPAGES_INACT) < uvmexp.inactarg)) {
  uvmpd_scan();
- work_done = 1; /* XXX we hope... */
  }
 
  /*
  * if there's any free memory to be had,
@@ -296,10 +293,8 @@ uvm_pageout(void *arg)
  }
 
  if (pma != NULL) {
  pma->pm_flags &= ~UVM_PMA_BUSY;
- if (!work_done)
- pma->pm_flags |= UVM_PMA_FAIL;
  if (pma->pm_flags & (UVM_PMA_FAIL | UVM_PMA_FREED)) {
  pma->pm_flags &= ~UVM_PMA_LINKED;
  TAILQ_REMOVE(&uvm.pmr_control.allocs, pma,
     pmq);

Reply | Threaded
Open this post in threaded view
|

Re: Dead code in uvm_pageout()

Mark Kettenis
> Date: Tue, 24 Mar 2020 15:18:13 +0100
> From: Martin Pieuchot <[hidden email]>
>
> As soon as an entry is found on `pmr_control.allocs' the boolean
> `work_done' will be set to true.  So it is impossible to reach the
> case below that sets UVM_PMA_FAIL.
>
> CID 1453061 Logically dead code
>
> Ok?

Almost certainly not.

> Index: uvm/uvm_pdaemon.c
> ===================================================================
> RCS file: /cvs/src/sys/uvm/uvm_pdaemon.c,v
> retrieving revision 1.85
> diff -u -p -u -4 -r1.85 uvm_pdaemon.c
> --- uvm/uvm_pdaemon.c 30 Dec 2019 23:58:38 -0000 1.85
> +++ uvm/uvm_pdaemon.c 24 Mar 2020 14:12:22 -0000
> @@ -209,9 +209,8 @@ void
>  uvm_pageout(void *arg)
>  {
>   struct uvm_constraint_range constraint;
>   struct uvm_pmalloc *pma;
> - int work_done;
>   int npages = 0;
>  
>   /* ensure correct priority and set paging parameters... */
>   uvm.pagedaemon_proc = curproc;
> @@ -222,9 +221,8 @@ uvm_pageout(void *arg)
>   uvm_unlock_pageq();
>  
>   for (;;) {
>   long size;
> -   work_done = 0; /* No work done this iteration. */
>  
>   uvm_lock_fpageq();
>   if (!uvm_nowait_failed && TAILQ_EMPTY(&uvm.pmr_control.allocs)) {
>   msleep_nsec(&uvm.pagedaemon, &uvm.fpageqlock, PVM,
> @@ -281,9 +279,8 @@ uvm_pageout(void *arg)
>   if (pma != NULL ||
>      ((uvmexp.free - BUFPAGES_DEFICIT) < uvmexp.freetarg) ||
>      ((uvmexp.inactive + BUFPAGES_INACT) < uvmexp.inactarg)) {
>   uvmpd_scan();
> - work_done = 1; /* XXX we hope... */
>   }
>  
>   /*
>   * if there's any free memory to be had,
> @@ -296,10 +293,8 @@ uvm_pageout(void *arg)
>   }
>  
>   if (pma != NULL) {
>   pma->pm_flags &= ~UVM_PMA_BUSY;
> - if (!work_done)
> - pma->pm_flags |= UVM_PMA_FAIL;
>   if (pma->pm_flags & (UVM_PMA_FAIL | UVM_PMA_FREED)) {
>   pma->pm_flags &= ~UVM_PMA_LINKED;
>   TAILQ_REMOVE(&uvm.pmr_control.allocs, pma,
>      pmq);
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Dead code in uvm_pageout()

Martin Pieuchot
On 24/03/20(Tue) 15:55, Mark Kettenis wrote:

> > Date: Tue, 24 Mar 2020 15:18:13 +0100
> > From: Martin Pieuchot <[hidden email]>
> >
> > As soon as an entry is found on `pmr_control.allocs' the boolean
> > `work_done' will be set to true.  So it is impossible to reach the
> > case below that sets UVM_PMA_FAIL.
> >
> > CID 1453061 Logically dead code
> >
> > Ok?
>
> Almost certainly not.

Could you elaborate?  Are you implying this is the symptom of a deeper
bug?

Reply | Threaded
Open this post in threaded view
|

Re: Dead code in uvm_pageout()

Mark Kettenis
> Date: Tue, 24 Mar 2020 16:11:48 +0100
> From: Martin Pieuchot <[hidden email]>
>
> On 24/03/20(Tue) 15:55, Mark Kettenis wrote:
> > > Date: Tue, 24 Mar 2020 15:18:13 +0100
> > > From: Martin Pieuchot <[hidden email]>
> > >
> > > As soon as an entry is found on `pmr_control.allocs' the boolean
> > > `work_done' will be set to true.  So it is impossible to reach the
> > > case below that sets UVM_PMA_FAIL.
> > >
> > > CID 1453061 Logically dead code
> > >
> > > Ok?
> >
> > Almost certainly not.
>
> Could you elaborate?  Are you implying this is the symptom of a deeper
> bug?

Yes.