Re: uvm: Consider BUFPAGES_DEFICIT in swap_shortage
On 05/05/22(Thu) 10:56, Bob Beck wrote:
> On Thu, May 05, 2022 at 10:16:23AM -0600, Bob Beck wrote:
> > Ugh. You???re digging in the most perilous parts of the pile.
> >
> > I will go look with you??? sigh. (This is not yet an ok for that.)
> >
> > > On May 5, 2022, at 7:53 AM, Martin Pieuchot wrote:
> > >
> > > When considering the amount of free pages in the page daemon a small
> > > amount is always kept for the buffer cache... except in one place.
> > >
> > > The diff below gets rid of this exception. This is important because
> > > uvmpd_scan() is called conditionally using the following check:
> > >
> > > if (uvmexp.free - BUFPAGES_DEFICIT) < uvmexp.freetarg) {
> > >...
> > > }
> > >
> > > So in case of swap shortage we might end up freeing fewer pages than
> > > wanted.
>
> So a bit of backgroud.
>
> I am pretty much of the belief that this "low water mark" for pages is
> nonsense now. I was in the midst of trying to prove that
> to myself and therefore rip down some of the crazy accounting and
> very arbitrary limits in the buffer cache and got distracted.
>
> Maybe something like this to start? (buf failing that I think
> your current diff is probably ok).
Thanks. I'll commit my diff then to make the current code coherent and
let me progress in my refactoring. Then we can consider changing this
magic.
> Index: sys/sys/mount.h
> ===
> RCS file: /cvs/src/sys/sys/mount.h,v
> retrieving revision 1.148
> diff -u -p -u -p -r1.148 mount.h
> --- sys/sys/mount.h 6 Apr 2021 14:17:35 - 1.148
> +++ sys/sys/mount.h 5 May 2022 16:50:50 -
> @@ -488,10 +488,8 @@ struct bcachestats {
> #ifdef _KERNEL
> extern struct bcachestats bcstats;
> extern long buflowpages, bufhighpages, bufbackpages;
> -#define BUFPAGES_DEFICIT (((buflowpages - bcstats.numbufpages) < 0) ? 0 \
> -: buflowpages - bcstats.numbufpages)
> -#define BUFPAGES_INACT (((bcstats.numcleanpages - buflowpages) < 0) ? 0 \
> -: bcstats.numcleanpages - buflowpages)
> +#define BUFPAGES_DEFICIT 0
> +#define BUFPAGES_INACT bcstats.numcleanpages
> extern int bufcachepercent;
> extern void bufadjust(int);
> struct uvm_constraint_range;
>
>
> > >
> > > ok?
> > >
> > > Index: uvm/uvm_pdaemon.c
> > > ===
> > > RCS file: /cvs/src/sys/uvm/uvm_pdaemon.c,v
> > > retrieving revision 1.98
> > > diff -u -p -r1.98 uvm_pdaemon.c
> > > --- uvm/uvm_pdaemon.c 4 May 2022 14:58:26 - 1.98
> > > +++ uvm/uvm_pdaemon.c 5 May 2022 13:40:28 -
> > > @@ -923,12 +923,13 @@ uvmpd_scan(void)
> > >* detect if we're not going to be able to page anything out
> > >* until we free some swap resources from active pages.
> > >*/
> > > + free = uvmexp.free - BUFPAGES_DEFICIT;
> > > swap_shortage = 0;
> > > - if (uvmexp.free < uvmexp.freetarg &&
> > > + if (free < uvmexp.freetarg &&
> > > uvmexp.swpginuse == uvmexp.swpages &&
> > > !uvm_swapisfull() &&
> > > pages_freed == 0) {
> > > - swap_shortage = uvmexp.freetarg - uvmexp.free;
> > > + swap_shortage = uvmexp.freetarg - free;
> > > }
> > >
> > > for (p = TAILQ_FIRST(&uvm.page_active);
> > >
> >
Re: uvm: Consider BUFPAGES_DEFICIT in swap_shortage
On Thu, May 05, 2022 at 10:16:23AM -0600, Bob Beck wrote:
> Ugh. You???re digging in the most perilous parts of the pile.
>
> I will go look with you??? sigh. (This is not yet an ok for that.)
>
> > On May 5, 2022, at 7:53 AM, Martin Pieuchot wrote:
> >
> > When considering the amount of free pages in the page daemon a small
> > amount is always kept for the buffer cache... except in one place.
> >
> > The diff below gets rid of this exception. This is important because
> > uvmpd_scan() is called conditionally using the following check:
> >
> > if (uvmexp.free - BUFPAGES_DEFICIT) < uvmexp.freetarg) {
> >...
> > }
> >
> > So in case of swap shortage we might end up freeing fewer pages than
> > wanted.
So a bit of backgroud.
I am pretty much of the belief that this "low water mark" for pages is
nonsense now. I was in the midst of trying to prove that
to myself and therefore rip down some of the crazy accounting and
very arbitrary limits in the buffer cache and got distracted.
Maybe something like this to start? (buf failing that I think
your current diff is probably ok).
Index: sys/sys/mount.h
===
RCS file: /cvs/src/sys/sys/mount.h,v
retrieving revision 1.148
diff -u -p -u -p -r1.148 mount.h
--- sys/sys/mount.h 6 Apr 2021 14:17:35 - 1.148
+++ sys/sys/mount.h 5 May 2022 16:50:50 -
@@ -488,10 +488,8 @@ struct bcachestats {
#ifdef _KERNEL
extern struct bcachestats bcstats;
extern long buflowpages, bufhighpages, bufbackpages;
-#define BUFPAGES_DEFICIT (((buflowpages - bcstats.numbufpages) < 0) ? 0 \
-: buflowpages - bcstats.numbufpages)
-#define BUFPAGES_INACT (((bcstats.numcleanpages - buflowpages) < 0) ? 0 \
-: bcstats.numcleanpages - buflowpages)
+#define BUFPAGES_DEFICIT 0
+#define BUFPAGES_INACT bcstats.numcleanpages
extern int bufcachepercent;
extern void bufadjust(int);
struct uvm_constraint_range;
> >
> > ok?
> >
> > Index: uvm/uvm_pdaemon.c
> > ===
> > RCS file: /cvs/src/sys/uvm/uvm_pdaemon.c,v
> > retrieving revision 1.98
> > diff -u -p -r1.98 uvm_pdaemon.c
> > --- uvm/uvm_pdaemon.c 4 May 2022 14:58:26 - 1.98
> > +++ uvm/uvm_pdaemon.c 5 May 2022 13:40:28 -
> > @@ -923,12 +923,13 @@ uvmpd_scan(void)
> > * detect if we're not going to be able to page anything out
> > * until we free some swap resources from active pages.
> > */
> > + free = uvmexp.free - BUFPAGES_DEFICIT;
> > swap_shortage = 0;
> > - if (uvmexp.free < uvmexp.freetarg &&
> > + if (free < uvmexp.freetarg &&
> > uvmexp.swpginuse == uvmexp.swpages &&
> > !uvm_swapisfull() &&
> > pages_freed == 0) {
> > - swap_shortage = uvmexp.freetarg - uvmexp.free;
> > + swap_shortage = uvmexp.freetarg - free;
> > }
> >
> > for (p = TAILQ_FIRST(&uvm.page_active);
> >
>
uvm: Consider BUFPAGES_DEFICIT in swap_shortage
When considering the amount of free pages in the page daemon a small
amount is always kept for the buffer cache... except in one place.
The diff below gets rid of this exception. This is important because
uvmpd_scan() is called conditionally using the following check:
if (uvmexp.free - BUFPAGES_DEFICIT) < uvmexp.freetarg) {
...
}
So in case of swap shortage we might end up freeing fewer pages than
wanted.
ok?
Index: uvm/uvm_pdaemon.c
===
RCS file: /cvs/src/sys/uvm/uvm_pdaemon.c,v
retrieving revision 1.98
diff -u -p -r1.98 uvm_pdaemon.c
--- uvm/uvm_pdaemon.c 4 May 2022 14:58:26 - 1.98
+++ uvm/uvm_pdaemon.c 5 May 2022 13:40:28 -
@@ -923,12 +923,13 @@ uvmpd_scan(void)
* detect if we're not going to be able to page anything out
* until we free some swap resources from active pages.
*/
+ free = uvmexp.free - BUFPAGES_DEFICIT;
swap_shortage = 0;
- if (uvmexp.free < uvmexp.freetarg &&
+ if (free < uvmexp.freetarg &&
uvmexp.swpginuse == uvmexp.swpages &&
!uvm_swapisfull() &&
pages_freed == 0) {
- swap_shortage = uvmexp.freetarg - uvmexp.free;
+ swap_shortage = uvmexp.freetarg - free;
}
for (p = TAILQ_FIRST(&uvm.page_active);
