Re: [PATCH] mm: vmscan: do not pass reclaimed slab to vmpressure

[zhong jiang]

On 2017/6/7 10:53, Minchan Kim wrote:
> Hi,
>
> On Tue, Jun 06, 2017 at 09:00:55PM +0800, zhong jiang wrote:
>> On 2017/1/31 7:40, Minchan Kim wrote:
>>> Hi Vinayak,
>>> Sorry for late response. It was Lunar New Year holidays.
>>>
>>> On Fri, Jan 27, 2017 at 01:43:23PM +0530, vinayak menon wrote:
> Thanks for the explain. However, such case can happen with THP page
> as well as slab. In case of THP page, nr_scanned is 1 but nr_reclaimed
> could be 512 so I think vmpressure should have a logic to prevent undeflow
> regardless of slab shrinking.
>
 I see. Going to send a vmpressure fix. But, wouldn't the THP case
 result in incorrect
 vmpressure reporting even if we fix the vmpressure underflow problem ?
>>> If a THP page is reclaimed, it reports lower pressure due to bigger
>>> reclaim ratio(ie, reclaimed/scanned) compared to normal pages but
>>> it's not a problem, is it? Because VM reclaimed more memory than
>>> expected so memory pressure isn't severe now.
>>   Hi, Minchan
>>
>>   THP lru page is reclaimed, reclaim ratio bigger make sense. but I read the 
>> code, I found
>>   THP is split to normal pages and loop again.  reclaimed pages should not 
>> be bigger
>>than nr_scan.  because of each loop will increase nr_scan counter.
>>  
>>It is likely  I miss something.  you can point out the point please.
> You are absolutely right.
>
> I got confused by nr_scanned from isolate_lru_pages and sc->nr_scanned
> from shrink_page_list.
>
> Thanks.
>
>
> .
>
 Hi, Minchan

 I will send the revert patch shortly. how do you think?

 Thanks
 zhongjiang

Re: [PATCH] mm: vmscan: do not pass reclaimed slab to vmpressure

[zhong jiang]

On 2017/6/7 10:53, Minchan Kim wrote:
> Hi,
>
> On Tue, Jun 06, 2017 at 09:00:55PM +0800, zhong jiang wrote:
>> On 2017/1/31 7:40, Minchan Kim wrote:
>>> Hi Vinayak,
>>> Sorry for late response. It was Lunar New Year holidays.
>>>
>>> On Fri, Jan 27, 2017 at 01:43:23PM +0530, vinayak menon wrote:
> Thanks for the explain. However, such case can happen with THP page
> as well as slab. In case of THP page, nr_scanned is 1 but nr_reclaimed
> could be 512 so I think vmpressure should have a logic to prevent undeflow
> regardless of slab shrinking.
>
 I see. Going to send a vmpressure fix. But, wouldn't the THP case
 result in incorrect
 vmpressure reporting even if we fix the vmpressure underflow problem ?
>>> If a THP page is reclaimed, it reports lower pressure due to bigger
>>> reclaim ratio(ie, reclaimed/scanned) compared to normal pages but
>>> it's not a problem, is it? Because VM reclaimed more memory than
>>> expected so memory pressure isn't severe now.
>>   Hi, Minchan
>>
>>   THP lru page is reclaimed, reclaim ratio bigger make sense. but I read the 
>> code, I found
>>   THP is split to normal pages and loop again.  reclaimed pages should not 
>> be bigger
>>than nr_scan.  because of each loop will increase nr_scan counter.
>>  
>>It is likely  I miss something.  you can point out the point please.
> You are absolutely right.
>
> I got confused by nr_scanned from isolate_lru_pages and sc->nr_scanned
> from shrink_page_list.
>
> Thanks.
>
>
> .
>
 Hi, Minchan

 I will send the revert patch shortly. how do you think?

 Thanks
 zhongjiang

Re: [PATCH] mm: vmscan: do not pass reclaimed slab to vmpressure

[Minchan Kim]

Hi,

On Tue, Jun 06, 2017 at 09:00:55PM +0800, zhong jiang wrote:
> On 2017/1/31 7:40, Minchan Kim wrote:
> > Hi Vinayak,
> > Sorry for late response. It was Lunar New Year holidays.
> >
> > On Fri, Jan 27, 2017 at 01:43:23PM +0530, vinayak menon wrote:
> >>> Thanks for the explain. However, such case can happen with THP page
> >>> as well as slab. In case of THP page, nr_scanned is 1 but nr_reclaimed
> >>> could be 512 so I think vmpressure should have a logic to prevent undeflow
> >>> regardless of slab shrinking.
> >>>
> >> I see. Going to send a vmpressure fix. But, wouldn't the THP case
> >> result in incorrect
> >> vmpressure reporting even if we fix the vmpressure underflow problem ?
> > If a THP page is reclaimed, it reports lower pressure due to bigger
> > reclaim ratio(ie, reclaimed/scanned) compared to normal pages but
> > it's not a problem, is it? Because VM reclaimed more memory than
> > expected so memory pressure isn't severe now.
>   Hi, Minchan
> 
>   THP lru page is reclaimed, reclaim ratio bigger make sense. but I read the 
> code, I found
>   THP is split to normal pages and loop again.  reclaimed pages should not be 
> bigger
>than nr_scan.  because of each loop will increase nr_scan counter.
>  
>It is likely  I miss something.  you can point out the point please.

You are absolutely right.

I got confused by nr_scanned from isolate_lru_pages and sc->nr_scanned
from shrink_page_list.

Thanks.

Re: [PATCH] mm: vmscan: do not pass reclaimed slab to vmpressure

[Minchan Kim]

Hi,

On Tue, Jun 06, 2017 at 09:00:55PM +0800, zhong jiang wrote:
> On 2017/1/31 7:40, Minchan Kim wrote:
> > Hi Vinayak,
> > Sorry for late response. It was Lunar New Year holidays.
> >
> > On Fri, Jan 27, 2017 at 01:43:23PM +0530, vinayak menon wrote:
> >>> Thanks for the explain. However, such case can happen with THP page
> >>> as well as slab. In case of THP page, nr_scanned is 1 but nr_reclaimed
> >>> could be 512 so I think vmpressure should have a logic to prevent undeflow
> >>> regardless of slab shrinking.
> >>>
> >> I see. Going to send a vmpressure fix. But, wouldn't the THP case
> >> result in incorrect
> >> vmpressure reporting even if we fix the vmpressure underflow problem ?
> > If a THP page is reclaimed, it reports lower pressure due to bigger
> > reclaim ratio(ie, reclaimed/scanned) compared to normal pages but
> > it's not a problem, is it? Because VM reclaimed more memory than
> > expected so memory pressure isn't severe now.
>   Hi, Minchan
> 
>   THP lru page is reclaimed, reclaim ratio bigger make sense. but I read the 
> code, I found
>   THP is split to normal pages and loop again.  reclaimed pages should not be 
> bigger
>than nr_scan.  because of each loop will increase nr_scan counter.
>  
>It is likely  I miss something.  you can point out the point please.

You are absolutely right.

I got confused by nr_scanned from isolate_lru_pages and sc->nr_scanned
from shrink_page_list.

Thanks.

Re: [PATCH] mm: vmscan: do not pass reclaimed slab to vmpressure

[zhong jiang]

On 2017/1/31 7:40, Minchan Kim wrote:
> Hi Vinayak,
> Sorry for late response. It was Lunar New Year holidays.
>
> On Fri, Jan 27, 2017 at 01:43:23PM +0530, vinayak menon wrote:
>>> Thanks for the explain. However, such case can happen with THP page
>>> as well as slab. In case of THP page, nr_scanned is 1 but nr_reclaimed
>>> could be 512 so I think vmpressure should have a logic to prevent undeflow
>>> regardless of slab shrinking.
>>>
>> I see. Going to send a vmpressure fix. But, wouldn't the THP case
>> result in incorrect
>> vmpressure reporting even if we fix the vmpressure underflow problem ?
> If a THP page is reclaimed, it reports lower pressure due to bigger
> reclaim ratio(ie, reclaimed/scanned) compared to normal pages but
> it's not a problem, is it? Because VM reclaimed more memory than
> expected so memory pressure isn't severe now.
  Hi, Minchan

  THP lru page is reclaimed, reclaim ratio bigger make sense. but I read the 
code, I found
  THP is split to normal pages and loop again.  reclaimed pages should not be 
bigger
   than nr_scan.  because of each loop will increase nr_scan counter.
 
   It is likely  I miss something.  you can point out the point please.
 
  Thanks
  zhongjiang
>> unsigned arithmetic results in the pressure value to be
>> huge, thus resulting in a critical event being sent to
>> root cgroup. Fix this by not passing the reclaimed slab
>> count to vmpressure, with the assumption that vmpressure
>> should show the actual pressure on LRU which is now
>> diluted by adding reclaimed slab without a corresponding
>> scanned value.
> I can't guess justfication of your assumption from the description.
> Why do we consider only LRU pages for vmpressure? Could you elaborate
> a bit?
>
 When we encountered the false events from vmpressure, thought the problem
 could be that slab scanned is not included in sc->nr_scanned, like it is 
 done
 for reclaimed. But later thought vmpressure works only on the scanned and
 reclaimed from LRU. I can explain what I understand, let me know if this is
 incorrect.
 vmpressure is an index which tells the pressure on LRU, and thus an
 indicator of thrashing. In shrink_node when we come out of the inner 
 do-while
 loop after shrinking the lruvec, the scanned and reclaimed corresponds to 
 the
 pressure felt on the LRUs which in turn indicates the pressure on VM. The
 moment we add the slab reclaimed pages to the reclaimed, we dilute the
 actual pressure felt on LRUs. When slab scanned/reclaimed is not included
 in the vmpressure, the values will indicate the actual pressure and if 
 there
 were a lot of slab reclaimed pages it will result in lesser pressure
 on LRUs in the next run which will again be indicated by vmpressure. i.e. 
 the
>>> I think there is no intention to exclude slab by design of vmpressure.
>>> Beause slab is memory consumption so freeing of slab pages really helps
>>> the memory pressure. Also, there might be slab-intensive workload rather
>>> than LRU. It would be great if vmpressure works well with that case.
>>> But the problem with involving slab for vmpressure is it's not fair with
>>> LRU pages. LRU pages are 1:1 cost model for scan:free but slab shriking
>>> depends the each slab's object population. It means it's impossible to
>>> get stable cost model with current slab shrinkg model, unfortunately.
>>> So I don't obejct this patch although I want to see slab shrink model's
>>> change which is heavy-handed work.
>>>
>> Looking at the code, the slab reclaimed pages started getting passed to
>> vmpressure after the commit ("mm: vmscan: invoke slab shrinkers from
>> shrink_zone()").
>> But as you said, this may be helpful for slab intensive workloads. But in its
>> current form I think it results in incorrect vmpressure reporting because of 
>> not
>> accounting the slab scanned pages. Resending the patch with a modified
>> commit msg
>> since the underflow issue is fixed separately. Thanks Minchan.
> Make sense.
>
> Thanks, Vinayak!
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 
>
> .
>

Re: [PATCH] mm: vmscan: do not pass reclaimed slab to vmpressure

[zhong jiang]

On 2017/1/31 7:40, Minchan Kim wrote:
> Hi Vinayak,
> Sorry for late response. It was Lunar New Year holidays.
>
> On Fri, Jan 27, 2017 at 01:43:23PM +0530, vinayak menon wrote:
>>> Thanks for the explain. However, such case can happen with THP page
>>> as well as slab. In case of THP page, nr_scanned is 1 but nr_reclaimed
>>> could be 512 so I think vmpressure should have a logic to prevent undeflow
>>> regardless of slab shrinking.
>>>
>> I see. Going to send a vmpressure fix. But, wouldn't the THP case
>> result in incorrect
>> vmpressure reporting even if we fix the vmpressure underflow problem ?
> If a THP page is reclaimed, it reports lower pressure due to bigger
> reclaim ratio(ie, reclaimed/scanned) compared to normal pages but
> it's not a problem, is it? Because VM reclaimed more memory than
> expected so memory pressure isn't severe now.
  Hi, Minchan

  THP lru page is reclaimed, reclaim ratio bigger make sense. but I read the 
code, I found
  THP is split to normal pages and loop again.  reclaimed pages should not be 
bigger
   than nr_scan.  because of each loop will increase nr_scan counter.
 
   It is likely  I miss something.  you can point out the point please.
 
  Thanks
  zhongjiang
>> unsigned arithmetic results in the pressure value to be
>> huge, thus resulting in a critical event being sent to
>> root cgroup. Fix this by not passing the reclaimed slab
>> count to vmpressure, with the assumption that vmpressure
>> should show the actual pressure on LRU which is now
>> diluted by adding reclaimed slab without a corresponding
>> scanned value.
> I can't guess justfication of your assumption from the description.
> Why do we consider only LRU pages for vmpressure? Could you elaborate
> a bit?
>
 When we encountered the false events from vmpressure, thought the problem
 could be that slab scanned is not included in sc->nr_scanned, like it is 
 done
 for reclaimed. But later thought vmpressure works only on the scanned and
 reclaimed from LRU. I can explain what I understand, let me know if this is
 incorrect.
 vmpressure is an index which tells the pressure on LRU, and thus an
 indicator of thrashing. In shrink_node when we come out of the inner 
 do-while
 loop after shrinking the lruvec, the scanned and reclaimed corresponds to 
 the
 pressure felt on the LRUs which in turn indicates the pressure on VM. The
 moment we add the slab reclaimed pages to the reclaimed, we dilute the
 actual pressure felt on LRUs. When slab scanned/reclaimed is not included
 in the vmpressure, the values will indicate the actual pressure and if 
 there
 were a lot of slab reclaimed pages it will result in lesser pressure
 on LRUs in the next run which will again be indicated by vmpressure. i.e. 
 the
>>> I think there is no intention to exclude slab by design of vmpressure.
>>> Beause slab is memory consumption so freeing of slab pages really helps
>>> the memory pressure. Also, there might be slab-intensive workload rather
>>> than LRU. It would be great if vmpressure works well with that case.
>>> But the problem with involving slab for vmpressure is it's not fair with
>>> LRU pages. LRU pages are 1:1 cost model for scan:free but slab shriking
>>> depends the each slab's object population. It means it's impossible to
>>> get stable cost model with current slab shrinkg model, unfortunately.
>>> So I don't obejct this patch although I want to see slab shrink model's
>>> change which is heavy-handed work.
>>>
>> Looking at the code, the slab reclaimed pages started getting passed to
>> vmpressure after the commit ("mm: vmscan: invoke slab shrinkers from
>> shrink_zone()").
>> But as you said, this may be helpful for slab intensive workloads. But in its
>> current form I think it results in incorrect vmpressure reporting because of 
>> not
>> accounting the slab scanned pages. Resending the patch with a modified
>> commit msg
>> since the underflow issue is fixed separately. Thanks Minchan.
> Make sense.
>
> Thanks, Vinayak!
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 
>
> .
>

Re: [PATCH] mm: vmscan: do not pass reclaimed slab to vmpressure

[Minchan Kim]

Hi Vinayak,
Sorry for late response. It was Lunar New Year holidays.

On Fri, Jan 27, 2017 at 01:43:23PM +0530, vinayak menon wrote:
> >
> > Thanks for the explain. However, such case can happen with THP page
> > as well as slab. In case of THP page, nr_scanned is 1 but nr_reclaimed
> > could be 512 so I think vmpressure should have a logic to prevent undeflow
> > regardless of slab shrinking.
> >
> I see. Going to send a vmpressure fix. But, wouldn't the THP case
> result in incorrect
> vmpressure reporting even if we fix the vmpressure underflow problem ?

If a THP page is reclaimed, it reports lower pressure due to bigger
reclaim ratio(ie, reclaimed/scanned) compared to normal pages but
it's not a problem, is it? Because VM reclaimed more memory than
expected so memory pressure isn't severe now.

> 
> >>
> >> >
> >> >> unsigned arithmetic results in the pressure value to be
> >> >> huge, thus resulting in a critical event being sent to
> >> >> root cgroup. Fix this by not passing the reclaimed slab
> >> >> count to vmpressure, with the assumption that vmpressure
> >> >> should show the actual pressure on LRU which is now
> >> >> diluted by adding reclaimed slab without a corresponding
> >> >> scanned value.
> >> >
> >> > I can't guess justfication of your assumption from the description.
> >> > Why do we consider only LRU pages for vmpressure? Could you elaborate
> >> > a bit?
> >> >
> >> When we encountered the false events from vmpressure, thought the problem
> >> could be that slab scanned is not included in sc->nr_scanned, like it is 
> >> done
> >> for reclaimed. But later thought vmpressure works only on the scanned and
> >> reclaimed from LRU. I can explain what I understand, let me know if this is
> >> incorrect.
> >> vmpressure is an index which tells the pressure on LRU, and thus an
> >> indicator of thrashing. In shrink_node when we come out of the inner 
> >> do-while
> >> loop after shrinking the lruvec, the scanned and reclaimed corresponds to 
> >> the
> >> pressure felt on the LRUs which in turn indicates the pressure on VM. The
> >> moment we add the slab reclaimed pages to the reclaimed, we dilute the
> >> actual pressure felt on LRUs. When slab scanned/reclaimed is not included
> >> in the vmpressure, the values will indicate the actual pressure and if 
> >> there
> >> were a lot of slab reclaimed pages it will result in lesser pressure
> >> on LRUs in the next run which will again be indicated by vmpressure. i.e. 
> >> the
> >
> > I think there is no intention to exclude slab by design of vmpressure.
> > Beause slab is memory consumption so freeing of slab pages really helps
> > the memory pressure. Also, there might be slab-intensive workload rather
> > than LRU. It would be great if vmpressure works well with that case.
> > But the problem with involving slab for vmpressure is it's not fair with
> > LRU pages. LRU pages are 1:1 cost model for scan:free but slab shriking
> > depends the each slab's object population. It means it's impossible to
> > get stable cost model with current slab shrinkg model, unfortunately.
> > So I don't obejct this patch although I want to see slab shrink model's
> > change which is heavy-handed work.
> >
> Looking at the code, the slab reclaimed pages started getting passed to
> vmpressure after the commit ("mm: vmscan: invoke slab shrinkers from
> shrink_zone()").
> But as you said, this may be helpful for slab intensive workloads. But in its
> current form I think it results in incorrect vmpressure reporting because of 
> not
> accounting the slab scanned pages. Resending the patch with a modified
> commit msg
> since the underflow issue is fixed separately. Thanks Minchan.

Make sense.

Thanks, Vinayak!

Re: [PATCH] mm: vmscan: do not pass reclaimed slab to vmpressure

[Minchan Kim]

Hi Vinayak,
Sorry for late response. It was Lunar New Year holidays.

On Fri, Jan 27, 2017 at 01:43:23PM +0530, vinayak menon wrote:
> >
> > Thanks for the explain. However, such case can happen with THP page
> > as well as slab. In case of THP page, nr_scanned is 1 but nr_reclaimed
> > could be 512 so I think vmpressure should have a logic to prevent undeflow
> > regardless of slab shrinking.
> >
> I see. Going to send a vmpressure fix. But, wouldn't the THP case
> result in incorrect
> vmpressure reporting even if we fix the vmpressure underflow problem ?

If a THP page is reclaimed, it reports lower pressure due to bigger
reclaim ratio(ie, reclaimed/scanned) compared to normal pages but
it's not a problem, is it? Because VM reclaimed more memory than
expected so memory pressure isn't severe now.

> 
> >>
> >> >
> >> >> unsigned arithmetic results in the pressure value to be
> >> >> huge, thus resulting in a critical event being sent to
> >> >> root cgroup. Fix this by not passing the reclaimed slab
> >> >> count to vmpressure, with the assumption that vmpressure
> >> >> should show the actual pressure on LRU which is now
> >> >> diluted by adding reclaimed slab without a corresponding
> >> >> scanned value.
> >> >
> >> > I can't guess justfication of your assumption from the description.
> >> > Why do we consider only LRU pages for vmpressure? Could you elaborate
> >> > a bit?
> >> >
> >> When we encountered the false events from vmpressure, thought the problem
> >> could be that slab scanned is not included in sc->nr_scanned, like it is 
> >> done
> >> for reclaimed. But later thought vmpressure works only on the scanned and
> >> reclaimed from LRU. I can explain what I understand, let me know if this is
> >> incorrect.
> >> vmpressure is an index which tells the pressure on LRU, and thus an
> >> indicator of thrashing. In shrink_node when we come out of the inner 
> >> do-while
> >> loop after shrinking the lruvec, the scanned and reclaimed corresponds to 
> >> the
> >> pressure felt on the LRUs which in turn indicates the pressure on VM. The
> >> moment we add the slab reclaimed pages to the reclaimed, we dilute the
> >> actual pressure felt on LRUs. When slab scanned/reclaimed is not included
> >> in the vmpressure, the values will indicate the actual pressure and if 
> >> there
> >> were a lot of slab reclaimed pages it will result in lesser pressure
> >> on LRUs in the next run which will again be indicated by vmpressure. i.e. 
> >> the
> >
> > I think there is no intention to exclude slab by design of vmpressure.
> > Beause slab is memory consumption so freeing of slab pages really helps
> > the memory pressure. Also, there might be slab-intensive workload rather
> > than LRU. It would be great if vmpressure works well with that case.
> > But the problem with involving slab for vmpressure is it's not fair with
> > LRU pages. LRU pages are 1:1 cost model for scan:free but slab shriking
> > depends the each slab's object population. It means it's impossible to
> > get stable cost model with current slab shrinkg model, unfortunately.
> > So I don't obejct this patch although I want to see slab shrink model's
> > change which is heavy-handed work.
> >
> Looking at the code, the slab reclaimed pages started getting passed to
> vmpressure after the commit ("mm: vmscan: invoke slab shrinkers from
> shrink_zone()").
> But as you said, this may be helpful for slab intensive workloads. But in its
> current form I think it results in incorrect vmpressure reporting because of 
> not
> accounting the slab scanned pages. Resending the patch with a modified
> commit msg
> since the underflow issue is fixed separately. Thanks Minchan.

Make sense.

Thanks, Vinayak!

Re: [PATCH] mm: vmscan: do not pass reclaimed slab to vmpressure

[vinayak menon]

>
> Thanks for the explain. However, such case can happen with THP page
> as well as slab. In case of THP page, nr_scanned is 1 but nr_reclaimed
> could be 512 so I think vmpressure should have a logic to prevent undeflow
> regardless of slab shrinking.
>
I see. Going to send a vmpressure fix. But, wouldn't the THP case
result in incorrect
vmpressure reporting even if we fix the vmpressure underflow problem ?

>>
>> >
>> >> unsigned arithmetic results in the pressure value to be
>> >> huge, thus resulting in a critical event being sent to
>> >> root cgroup. Fix this by not passing the reclaimed slab
>> >> count to vmpressure, with the assumption that vmpressure
>> >> should show the actual pressure on LRU which is now
>> >> diluted by adding reclaimed slab without a corresponding
>> >> scanned value.
>> >
>> > I can't guess justfication of your assumption from the description.
>> > Why do we consider only LRU pages for vmpressure? Could you elaborate
>> > a bit?
>> >
>> When we encountered the false events from vmpressure, thought the problem
>> could be that slab scanned is not included in sc->nr_scanned, like it is done
>> for reclaimed. But later thought vmpressure works only on the scanned and
>> reclaimed from LRU. I can explain what I understand, let me know if this is
>> incorrect.
>> vmpressure is an index which tells the pressure on LRU, and thus an
>> indicator of thrashing. In shrink_node when we come out of the inner do-while
>> loop after shrinking the lruvec, the scanned and reclaimed corresponds to the
>> pressure felt on the LRUs which in turn indicates the pressure on VM. The
>> moment we add the slab reclaimed pages to the reclaimed, we dilute the
>> actual pressure felt on LRUs. When slab scanned/reclaimed is not included
>> in the vmpressure, the values will indicate the actual pressure and if there
>> were a lot of slab reclaimed pages it will result in lesser pressure
>> on LRUs in the next run which will again be indicated by vmpressure. i.e. the
>
> I think there is no intention to exclude slab by design of vmpressure.
> Beause slab is memory consumption so freeing of slab pages really helps
> the memory pressure. Also, there might be slab-intensive workload rather
> than LRU. It would be great if vmpressure works well with that case.
> But the problem with involving slab for vmpressure is it's not fair with
> LRU pages. LRU pages are 1:1 cost model for scan:free but slab shriking
> depends the each slab's object population. It means it's impossible to
> get stable cost model with current slab shrinkg model, unfortunately.
> So I don't obejct this patch although I want to see slab shrink model's
> change which is heavy-handed work.
>
Looking at the code, the slab reclaimed pages started getting passed to
vmpressure after the commit ("mm: vmscan: invoke slab shrinkers from
shrink_zone()").
But as you said, this may be helpful for slab intensive workloads. But in its
current form I think it results in incorrect vmpressure reporting because of not
accounting the slab scanned pages. Resending the patch with a modified
commit msg
since the underflow issue is fixed separately. Thanks Minchan.

Vinayak

Re: [PATCH] mm: vmscan: do not pass reclaimed slab to vmpressure

[vinayak menon]

>
> Thanks for the explain. However, such case can happen with THP page
> as well as slab. In case of THP page, nr_scanned is 1 but nr_reclaimed
> could be 512 so I think vmpressure should have a logic to prevent undeflow
> regardless of slab shrinking.
>
I see. Going to send a vmpressure fix. But, wouldn't the THP case
result in incorrect
vmpressure reporting even if we fix the vmpressure underflow problem ?

>>
>> >
>> >> unsigned arithmetic results in the pressure value to be
>> >> huge, thus resulting in a critical event being sent to
>> >> root cgroup. Fix this by not passing the reclaimed slab
>> >> count to vmpressure, with the assumption that vmpressure
>> >> should show the actual pressure on LRU which is now
>> >> diluted by adding reclaimed slab without a corresponding
>> >> scanned value.
>> >
>> > I can't guess justfication of your assumption from the description.
>> > Why do we consider only LRU pages for vmpressure? Could you elaborate
>> > a bit?
>> >
>> When we encountered the false events from vmpressure, thought the problem
>> could be that slab scanned is not included in sc->nr_scanned, like it is done
>> for reclaimed. But later thought vmpressure works only on the scanned and
>> reclaimed from LRU. I can explain what I understand, let me know if this is
>> incorrect.
>> vmpressure is an index which tells the pressure on LRU, and thus an
>> indicator of thrashing. In shrink_node when we come out of the inner do-while
>> loop after shrinking the lruvec, the scanned and reclaimed corresponds to the
>> pressure felt on the LRUs which in turn indicates the pressure on VM. The
>> moment we add the slab reclaimed pages to the reclaimed, we dilute the
>> actual pressure felt on LRUs. When slab scanned/reclaimed is not included
>> in the vmpressure, the values will indicate the actual pressure and if there
>> were a lot of slab reclaimed pages it will result in lesser pressure
>> on LRUs in the next run which will again be indicated by vmpressure. i.e. the
>
> I think there is no intention to exclude slab by design of vmpressure.
> Beause slab is memory consumption so freeing of slab pages really helps
> the memory pressure. Also, there might be slab-intensive workload rather
> than LRU. It would be great if vmpressure works well with that case.
> But the problem with involving slab for vmpressure is it's not fair with
> LRU pages. LRU pages are 1:1 cost model for scan:free but slab shriking
> depends the each slab's object population. It means it's impossible to
> get stable cost model with current slab shrinkg model, unfortunately.
> So I don't obejct this patch although I want to see slab shrink model's
> change which is heavy-handed work.
>
Looking at the code, the slab reclaimed pages started getting passed to
vmpressure after the commit ("mm: vmscan: invoke slab shrinkers from
shrink_zone()").
But as you said, this may be helpful for slab intensive workloads. But in its
current form I think it results in incorrect vmpressure reporting because of not
accounting the slab scanned pages. Resending the patch with a modified
commit msg
since the underflow issue is fixed separately. Thanks Minchan.

Vinayak

Re: [PATCH] mm: vmscan: do not pass reclaimed slab to vmpressure

[Minchan Kim]

Hi Vinayak,

On Thu, Jan 26, 2017 at 10:53:38AM +0530, vinayak menon wrote:
> Hi Minchan
> 
> On Thu, Jan 26, 2017 at 4:57 AM, Minchan Kim  wrote:
> > Hello Vinayak,
> >
> > On Wed, Jan 25, 2017 at 05:08:38PM +0530, Vinayak Menon wrote:
> >> It is noticed that during a global reclaim the memory
> >> reclaimed via shrinking the slabs can sometimes result
> >> in reclaimed pages being greater than the scanned pages
> >> in shrink_node. When this is passed to vmpressure, the
> >
> > I don't know you are saying zsmalloc. Anyway, it's one of those which
> > free larger pages than requested. I should fix that but was not sent
> > yet, unfortunately.
> 
> As I understand, the problem is not related to a particular shrinker.
> In shrink_node, when subtree's reclaim efficiency is passed to vmpressure,
> the 4th parameter (sc->nr_scanned - nr_scanned) includes only the LRU
> scanned pages, but the 5th parameter (sc->nr_reclaimed - nr_reclaimed) 
> includes
> the reclaimed slab pages also since in the previous step
> "reclaimed_slab" is added
> to it. i.e the slabs scanned are not included in scanned passed to vmpressure.
> This results in reclaimed going higher than scanned in vmpressure resulting in
> false events.

Thanks for the explain. However, such case can happen with THP page
as well as slab. In case of THP page, nr_scanned is 1 but nr_reclaimed
could be 512 so I think vmpressure should have a logic to prevent undeflow
regardless of slab shrinking.

> 
> >
> >> unsigned arithmetic results in the pressure value to be
> >> huge, thus resulting in a critical event being sent to
> >> root cgroup. Fix this by not passing the reclaimed slab
> >> count to vmpressure, with the assumption that vmpressure
> >> should show the actual pressure on LRU which is now
> >> diluted by adding reclaimed slab without a corresponding
> >> scanned value.
> >
> > I can't guess justfication of your assumption from the description.
> > Why do we consider only LRU pages for vmpressure? Could you elaborate
> > a bit?
> >
> When we encountered the false events from vmpressure, thought the problem
> could be that slab scanned is not included in sc->nr_scanned, like it is done
> for reclaimed. But later thought vmpressure works only on the scanned and
> reclaimed from LRU. I can explain what I understand, let me know if this is
> incorrect.
> vmpressure is an index which tells the pressure on LRU, and thus an
> indicator of thrashing. In shrink_node when we come out of the inner do-while
> loop after shrinking the lruvec, the scanned and reclaimed corresponds to the
> pressure felt on the LRUs which in turn indicates the pressure on VM. The
> moment we add the slab reclaimed pages to the reclaimed, we dilute the
> actual pressure felt on LRUs. When slab scanned/reclaimed is not included
> in the vmpressure, the values will indicate the actual pressure and if there
> were a lot of slab reclaimed pages it will result in lesser pressure
> on LRUs in the next run which will again be indicated by vmpressure. i.e. the

I think there is no intention to exclude slab by design of vmpressure.
Beause slab is memory consumption so freeing of slab pages really helps
the memory pressure. Also, there might be slab-intensive workload rather
than LRU. It would be great if vmpressure works well with that case.
But the problem with involving slab for vmpressure is it's not fair with
LRU pages. LRU pages are 1:1 cost model for scan:free but slab shriking
depends the each slab's object population. It means it's impossible to
get stable cost model with current slab shrinkg model, unfortunately.
So I don't obejct this patch although I want to see slab shrink model's
change which is heavy-handed work.

Thanks.


> pressure on LRUs indicate actual pressure on VM even if slab reclaimed is
> not included. Moreover, what I understand from code is, the reclaimed_slab
> includes only the inodesteals and the pages freed by slab allocator, and does
> not include the pages reclaimed by other shrinkers like
> lowmemorykiller, zsmalloc
> etc. That means even now we are including only a subset of reclaimed pages
> to vmpressure. Also, considering the case of a userspace lowmemorykiller
> which works on vmpressure on root cgroup, if the slab reclaimed in included in
> vmpressure, the lowmemorykiller will wait till most of the slab is
> shrinked before
> kicking in to kill a task. No ?
> 
> Thanks,
> Vinayak
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 

Re: [PATCH] mm: vmscan: do not pass reclaimed slab to vmpressure

[Minchan Kim]

Hi Vinayak,

On Thu, Jan 26, 2017 at 10:53:38AM +0530, vinayak menon wrote:
> Hi Minchan
> 
> On Thu, Jan 26, 2017 at 4:57 AM, Minchan Kim  wrote:
> > Hello Vinayak,
> >
> > On Wed, Jan 25, 2017 at 05:08:38PM +0530, Vinayak Menon wrote:
> >> It is noticed that during a global reclaim the memory
> >> reclaimed via shrinking the slabs can sometimes result
> >> in reclaimed pages being greater than the scanned pages
> >> in shrink_node. When this is passed to vmpressure, the
> >
> > I don't know you are saying zsmalloc. Anyway, it's one of those which
> > free larger pages than requested. I should fix that but was not sent
> > yet, unfortunately.
> 
> As I understand, the problem is not related to a particular shrinker.
> In shrink_node, when subtree's reclaim efficiency is passed to vmpressure,
> the 4th parameter (sc->nr_scanned - nr_scanned) includes only the LRU
> scanned pages, but the 5th parameter (sc->nr_reclaimed - nr_reclaimed) 
> includes
> the reclaimed slab pages also since in the previous step
> "reclaimed_slab" is added
> to it. i.e the slabs scanned are not included in scanned passed to vmpressure.
> This results in reclaimed going higher than scanned in vmpressure resulting in
> false events.

Thanks for the explain. However, such case can happen with THP page
as well as slab. In case of THP page, nr_scanned is 1 but nr_reclaimed
could be 512 so I think vmpressure should have a logic to prevent undeflow
regardless of slab shrinking.

> 
> >
> >> unsigned arithmetic results in the pressure value to be
> >> huge, thus resulting in a critical event being sent to
> >> root cgroup. Fix this by not passing the reclaimed slab
> >> count to vmpressure, with the assumption that vmpressure
> >> should show the actual pressure on LRU which is now
> >> diluted by adding reclaimed slab without a corresponding
> >> scanned value.
> >
> > I can't guess justfication of your assumption from the description.
> > Why do we consider only LRU pages for vmpressure? Could you elaborate
> > a bit?
> >
> When we encountered the false events from vmpressure, thought the problem
> could be that slab scanned is not included in sc->nr_scanned, like it is done
> for reclaimed. But later thought vmpressure works only on the scanned and
> reclaimed from LRU. I can explain what I understand, let me know if this is
> incorrect.
> vmpressure is an index which tells the pressure on LRU, and thus an
> indicator of thrashing. In shrink_node when we come out of the inner do-while
> loop after shrinking the lruvec, the scanned and reclaimed corresponds to the
> pressure felt on the LRUs which in turn indicates the pressure on VM. The
> moment we add the slab reclaimed pages to the reclaimed, we dilute the
> actual pressure felt on LRUs. When slab scanned/reclaimed is not included
> in the vmpressure, the values will indicate the actual pressure and if there
> were a lot of slab reclaimed pages it will result in lesser pressure
> on LRUs in the next run which will again be indicated by vmpressure. i.e. the

I think there is no intention to exclude slab by design of vmpressure.
Beause slab is memory consumption so freeing of slab pages really helps
the memory pressure. Also, there might be slab-intensive workload rather
than LRU. It would be great if vmpressure works well with that case.
But the problem with involving slab for vmpressure is it's not fair with
LRU pages. LRU pages are 1:1 cost model for scan:free but slab shriking
depends the each slab's object population. It means it's impossible to
get stable cost model with current slab shrinkg model, unfortunately.
So I don't obejct this patch although I want to see slab shrink model's
change which is heavy-handed work.

Thanks.


> pressure on LRUs indicate actual pressure on VM even if slab reclaimed is
> not included. Moreover, what I understand from code is, the reclaimed_slab
> includes only the inodesteals and the pages freed by slab allocator, and does
> not include the pages reclaimed by other shrinkers like
> lowmemorykiller, zsmalloc
> etc. That means even now we are including only a subset of reclaimed pages
> to vmpressure. Also, considering the case of a userspace lowmemorykiller
> which works on vmpressure on root cgroup, if the slab reclaimed in included in
> vmpressure, the lowmemorykiller will wait till most of the slab is
> shrinked before
> kicking in to kill a task. No ?
> 
> Thanks,
> Vinayak
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 

Re: [PATCH] mm: vmscan: do not pass reclaimed slab to vmpressure

[vinayak menon]

Hi Minchan

On Thu, Jan 26, 2017 at 4:57 AM, Minchan Kim  wrote:
> Hello Vinayak,
>
> On Wed, Jan 25, 2017 at 05:08:38PM +0530, Vinayak Menon wrote:
>> It is noticed that during a global reclaim the memory
>> reclaimed via shrinking the slabs can sometimes result
>> in reclaimed pages being greater than the scanned pages
>> in shrink_node. When this is passed to vmpressure, the
>
> I don't know you are saying zsmalloc. Anyway, it's one of those which
> free larger pages than requested. I should fix that but was not sent
> yet, unfortunately.

As I understand, the problem is not related to a particular shrinker.
In shrink_node, when subtree's reclaim efficiency is passed to vmpressure,
the 4th parameter (sc->nr_scanned - nr_scanned) includes only the LRU
scanned pages, but the 5th parameter (sc->nr_reclaimed - nr_reclaimed) includes
the reclaimed slab pages also since in the previous step
"reclaimed_slab" is added
to it. i.e the slabs scanned are not included in scanned passed to vmpressure.
This results in reclaimed going higher than scanned in vmpressure resulting in
false events.

>
>> unsigned arithmetic results in the pressure value to be
>> huge, thus resulting in a critical event being sent to
>> root cgroup. Fix this by not passing the reclaimed slab
>> count to vmpressure, with the assumption that vmpressure
>> should show the actual pressure on LRU which is now
>> diluted by adding reclaimed slab without a corresponding
>> scanned value.
>
> I can't guess justfication of your assumption from the description.
> Why do we consider only LRU pages for vmpressure? Could you elaborate
> a bit?
>
When we encountered the false events from vmpressure, thought the problem
could be that slab scanned is not included in sc->nr_scanned, like it is done
for reclaimed. But later thought vmpressure works only on the scanned and
reclaimed from LRU. I can explain what I understand, let me know if this is
incorrect.
vmpressure is an index which tells the pressure on LRU, and thus an
indicator of thrashing. In shrink_node when we come out of the inner do-while
loop after shrinking the lruvec, the scanned and reclaimed corresponds to the
pressure felt on the LRUs which in turn indicates the pressure on VM. The
moment we add the slab reclaimed pages to the reclaimed, we dilute the
actual pressure felt on LRUs. When slab scanned/reclaimed is not included
in the vmpressure, the values will indicate the actual pressure and if there
were a lot of slab reclaimed pages it will result in lesser pressure
on LRUs in the next run which will again be indicated by vmpressure. i.e. the
pressure on LRUs indicate actual pressure on VM even if slab reclaimed is
not included. Moreover, what I understand from code is, the reclaimed_slab
includes only the inodesteals and the pages freed by slab allocator, and does
not include the pages reclaimed by other shrinkers like
lowmemorykiller, zsmalloc
etc. That means even now we are including only a subset of reclaimed pages
to vmpressure. Also, considering the case of a userspace lowmemorykiller
which works on vmpressure on root cgroup, if the slab reclaimed in included in
vmpressure, the lowmemorykiller will wait till most of the slab is
shrinked before
kicking in to kill a task. No ?

Thanks,
Vinayak

Re: [PATCH] mm: vmscan: do not pass reclaimed slab to vmpressure

[vinayak menon]

Hi Minchan

On Thu, Jan 26, 2017 at 4:57 AM, Minchan Kim  wrote:
> Hello Vinayak,
>
> On Wed, Jan 25, 2017 at 05:08:38PM +0530, Vinayak Menon wrote:
>> It is noticed that during a global reclaim the memory
>> reclaimed via shrinking the slabs can sometimes result
>> in reclaimed pages being greater than the scanned pages
>> in shrink_node. When this is passed to vmpressure, the
>
> I don't know you are saying zsmalloc. Anyway, it's one of those which
> free larger pages than requested. I should fix that but was not sent
> yet, unfortunately.

As I understand, the problem is not related to a particular shrinker.
In shrink_node, when subtree's reclaim efficiency is passed to vmpressure,
the 4th parameter (sc->nr_scanned - nr_scanned) includes only the LRU
scanned pages, but the 5th parameter (sc->nr_reclaimed - nr_reclaimed) includes
the reclaimed slab pages also since in the previous step
"reclaimed_slab" is added
to it. i.e the slabs scanned are not included in scanned passed to vmpressure.
This results in reclaimed going higher than scanned in vmpressure resulting in
false events.

>
>> unsigned arithmetic results in the pressure value to be
>> huge, thus resulting in a critical event being sent to
>> root cgroup. Fix this by not passing the reclaimed slab
>> count to vmpressure, with the assumption that vmpressure
>> should show the actual pressure on LRU which is now
>> diluted by adding reclaimed slab without a corresponding
>> scanned value.
>
> I can't guess justfication of your assumption from the description.
> Why do we consider only LRU pages for vmpressure? Could you elaborate
> a bit?
>
When we encountered the false events from vmpressure, thought the problem
could be that slab scanned is not included in sc->nr_scanned, like it is done
for reclaimed. But later thought vmpressure works only on the scanned and
reclaimed from LRU. I can explain what I understand, let me know if this is
incorrect.
vmpressure is an index which tells the pressure on LRU, and thus an
indicator of thrashing. In shrink_node when we come out of the inner do-while
loop after shrinking the lruvec, the scanned and reclaimed corresponds to the
pressure felt on the LRUs which in turn indicates the pressure on VM. The
moment we add the slab reclaimed pages to the reclaimed, we dilute the
actual pressure felt on LRUs. When slab scanned/reclaimed is not included
in the vmpressure, the values will indicate the actual pressure and if there
were a lot of slab reclaimed pages it will result in lesser pressure
on LRUs in the next run which will again be indicated by vmpressure. i.e. the
pressure on LRUs indicate actual pressure on VM even if slab reclaimed is
not included. Moreover, what I understand from code is, the reclaimed_slab
includes only the inodesteals and the pages freed by slab allocator, and does
not include the pages reclaimed by other shrinkers like
lowmemorykiller, zsmalloc
etc. That means even now we are including only a subset of reclaimed pages
to vmpressure. Also, considering the case of a userspace lowmemorykiller
which works on vmpressure on root cgroup, if the slab reclaimed in included in
vmpressure, the lowmemorykiller will wait till most of the slab is
shrinked before
kicking in to kill a task. No ?

Thanks,
Vinayak

Re: [PATCH] mm: vmscan: do not pass reclaimed slab to vmpressure

[Minchan Kim]

Hello Vinayak,

On Wed, Jan 25, 2017 at 05:08:38PM +0530, Vinayak Menon wrote:
> It is noticed that during a global reclaim the memory
> reclaimed via shrinking the slabs can sometimes result
> in reclaimed pages being greater than the scanned pages
> in shrink_node. When this is passed to vmpressure, the

I don't know you are saying zsmalloc. Anyway, it's one of those which
free larger pages than requested. I should fix that but was not sent
yet, unfortunately.

> unsigned arithmetic results in the pressure value to be
> huge, thus resulting in a critical event being sent to
> root cgroup. Fix this by not passing the reclaimed slab
> count to vmpressure, with the assumption that vmpressure
> should show the actual pressure on LRU which is now
> diluted by adding reclaimed slab without a corresponding
> scanned value.

I can't guess justfication of your assumption from the description.
Why do we consider only LRU pages for vmpressure? Could you elaborate
a bit?

Thanks.

> 
> Signed-off-by: Vinayak Menon 
> ---
>  mm/vmscan.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 947ab6f..37c4486 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2594,16 +2594,16 @@ static bool shrink_node(pg_data_t *pgdat, struct 
> scan_control *sc)
>   sc->nr_scanned - nr_scanned,
>   node_lru_pages);
>  
> - if (reclaim_state) {
> - sc->nr_reclaimed += reclaim_state->reclaimed_slab;
> - reclaim_state->reclaimed_slab = 0;
> - }
> -
>   /* Record the subtree's reclaim efficiency */
>   vmpressure(sc->gfp_mask, sc->target_mem_cgroup, true,
>  sc->nr_scanned - nr_scanned,
>  sc->nr_reclaimed - nr_reclaimed);
>  
> + if (reclaim_state) {
> + sc->nr_reclaimed += reclaim_state->reclaimed_slab;
> + reclaim_state->reclaimed_slab = 0;
> + }
> +
>   if (sc->nr_reclaimed - nr_reclaimed)
>   reclaimable = true;
>  
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
> member of the Code Aurora Forum, hosted by The Linux Foundation
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 

Re: [PATCH] mm: vmscan: do not pass reclaimed slab to vmpressure

[Minchan Kim]

Hello Vinayak,

On Wed, Jan 25, 2017 at 05:08:38PM +0530, Vinayak Menon wrote:
> It is noticed that during a global reclaim the memory
> reclaimed via shrinking the slabs can sometimes result
> in reclaimed pages being greater than the scanned pages
> in shrink_node. When this is passed to vmpressure, the

I don't know you are saying zsmalloc. Anyway, it's one of those which
free larger pages than requested. I should fix that but was not sent
yet, unfortunately.

> unsigned arithmetic results in the pressure value to be
> huge, thus resulting in a critical event being sent to
> root cgroup. Fix this by not passing the reclaimed slab
> count to vmpressure, with the assumption that vmpressure
> should show the actual pressure on LRU which is now
> diluted by adding reclaimed slab without a corresponding
> scanned value.

I can't guess justfication of your assumption from the description.
Why do we consider only LRU pages for vmpressure? Could you elaborate
a bit?

Thanks.

> 
> Signed-off-by: Vinayak Menon 
> ---
>  mm/vmscan.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 947ab6f..37c4486 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2594,16 +2594,16 @@ static bool shrink_node(pg_data_t *pgdat, struct 
> scan_control *sc)
>   sc->nr_scanned - nr_scanned,
>   node_lru_pages);
>  
> - if (reclaim_state) {
> - sc->nr_reclaimed += reclaim_state->reclaimed_slab;
> - reclaim_state->reclaimed_slab = 0;
> - }
> -
>   /* Record the subtree's reclaim efficiency */
>   vmpressure(sc->gfp_mask, sc->target_mem_cgroup, true,
>  sc->nr_scanned - nr_scanned,
>  sc->nr_reclaimed - nr_reclaimed);
>  
> + if (reclaim_state) {
> + sc->nr_reclaimed += reclaim_state->reclaimed_slab;
> + reclaim_state->reclaimed_slab = 0;
> + }
> +
>   if (sc->nr_reclaimed - nr_reclaimed)
>   reclaimable = true;
>  
> -- 
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
> member of the Code Aurora Forum, hosted by The Linux Foundation
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org