Re: [PATCH v2] mm/vmscan: shrink slab in node reclaim

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

 



On Tue, Aug 6, 2019 at 7:15 PM Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Tue, Aug 06, 2019 at 05:32:54PM +0800, Yafang Shao wrote:
> > On Tue, Aug 6, 2019 at 5:25 PM Michal Hocko <mhocko@xxxxxxxxxx> wrote:
> > >
> > > On Tue 06-08-19 17:15:05, Yafang Shao wrote:
> > > > On Tue, Aug 6, 2019 at 5:05 PM Michal Hocko <mhocko@xxxxxxxxxx> wrote:
> > > [...]
> > > > > > As you said, the direct reclaim path set it to 1, but the
> > > > > > __node_reclaim() forgot to process may_shrink_slab.
> > > > >
> > > > > OK, I am blind obviously. Sorry about that. Anyway, why cannot we simply
> > > > > get back to the original behavior by setting may_shrink_slab in that
> > > > > path as well?
> > > >
> > > > You mean do it as the commit 0ff38490c836 did  before ?
> > > > I haven't check in which commit the shrink_slab() is removed from
> > >
> > > What I've had in mind was essentially this:
> > >
> > > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > > index 7889f583ced9..8011288a80e2 100644
> > > --- a/mm/vmscan.c
> > > +++ b/mm/vmscan.c
> > > @@ -4088,6 +4093,7 @@ static int __node_reclaim(struct pglist_data *pgdat, gfp_t gfp_mask, unsigned in
> > >                 .may_unmap = !!(node_reclaim_mode & RECLAIM_UNMAP),
> > >                 .may_swap = 1,
> > >                 .reclaim_idx = gfp_zone(gfp_mask),
> > > +               .may_shrinkslab = 1;
> > >         };
> > >
> > >         trace_mm_vmscan_node_reclaim_begin(pgdat->node_id, order,
> > >
> > > shrink_node path already does shrink slab when the flag allows that. In
> > > other words get us back to before 1c30844d2dfe because that has clearly
> > > changed the long term node reclaim behavior just recently.
> > > --
> >
> > If we do it like this, then vm.min_slab_ratio will not take effect if
> > there're enough relcaimable page cache.
> > Seems there're bugs in the original behavior as well.
> >
>
> Typically that would be done as a separate patch with a standalone
> justification for it. The first patch should simply restore expected
> behaviour with a Fixes: tag noting that the change in behaviour was
> unintentional.
>

Sure, I will do it.

Thanks
Yafang




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux