| Message ID | 1559816080-26405-1-git-send-email-laoar.shao@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | mm: improvements in shrink slab | expand |
On Thu 06-06-19 18:14:37, Yafang Shao wrote: > In the past few days, I found an issue in shrink slab. > We I was trying to fix it, I find there are something in shrink slab need > to be improved. > > - #1 is to expose the min_slab_pages to help us analyze shrink slab. > > - #2 is an code improvement. > > - #3 is a fix to a issue. This issue is very easy to produce. > In the zone reclaim mode. > First you continuously cat a random non-exist file to produce > more and more dentry, then you read big file to produce page cache. > Finally you will find that the denty will never be shrunk. > In order to fix this issue, a new bitmask no_pagecache is introduce, > which is 0 by defalt. Node reclaim mode is quite special and rarely used these days. Could you be more specific on how did you get to see the above problems? Do you really need node reclaim in your usecases or is this more about a testing and seeing what happens. Not that I am against these changes but I would like to understand the motivation. Especially because you are exposing some internal implementation details of the node reclaim to the userspace.
On Thu, Jun 6, 2019 at 7:17 PM Michal Hocko <mhocko@suse.com> wrote: > On Thu 06-06-19 18:14:37, Yafang Shao wrote: > > In the past few days, I found an issue in shrink slab. > > We I was trying to fix it, I find there are something in shrink slab need > > to be improved. > > > > - #1 is to expose the min_slab_pages to help us analyze shrink slab. > > > > - #2 is an code improvement. > > > > - #3 is a fix to a issue. This issue is very easy to produce. > > In the zone reclaim mode. > > First you continuously cat a random non-exist file to produce > > more and more dentry, then you read big file to produce page cache. > > Finally you will find that the denty will never be shrunk. > > In order to fix this issue, a new bitmask no_pagecache is introduce, > > which is 0 by defalt. > > Node reclaim mode is quite special and rarely used these days. Could you > be more specific on how did you get to see the above problems? Do you > really need node reclaim in your usecases or is this more about a > testing and seeing what happens. Not that I am against these changes but > I would like to understand the motivation. Especially because you are > exposing some internal implementation details of the node reclaim to the > userspace. > > The slab issue we found on our server is on old kernel (kernel-3.10). We found that the dentry was continuesly growing without shrinking in one container on a server, so I read slab code and found that memcg relcaim can't shrink slab in this old kenrel, but this issue was aready fixed in upstream. When I was reading the shrink slab code in the upstream kernel, I found the slab can't be shrinked in node reclaim. So I did some test to produce this issue and post this patchset to fix it. With my patch, the issue produced by me disapears. But this is only a beginning in the node reclaim path... Then I found another issue when I implemented a memory pressure monitor for out containers, which is vmpressure_prio() is missed in the node reclaim path. Well, seems when we introduce new feature for page relciam, we always ignore the node reclaim path. Regarding node reclaim path, we always turn it off on our servers, because we really found some latency spike caused by node reclaim (the reason why node reclaim is turned on is not clear). The reason I expose node reclaim details to userspace is because the user can set node reclaim details now. Thanks Yafang <div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jun 6, 2019 at 7:17 PM Michal Hocko <<a href="mailto:mhocko@suse.com">mhocko@suse.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Thu 06-06-19 18:14:37, Yafang Shao wrote:<br> > In the past few days, I found an issue in shrink slab.<br> > We I was trying to fix it, I find there are something in shrink slab need<br> > to be improved.<br> > <br> > - #1 is to expose the min_slab_pages to help us analyze shrink slab.<br> > <br> > - #2 is an code improvement.<br> > <br> > - #3 is a fix to a issue. This issue is very easy to produce.<br> > In the zone reclaim mode.<br> > First you continuously cat a random non-exist file to produce<br> > more and more dentry, then you read big file to produce page cache.<br> > Finally you will find that the denty will never be shrunk.<br> > In order to fix this issue, a new bitmask no_pagecache is introduce,<br> > which is 0 by defalt.<br> <br> Node reclaim mode is quite special and rarely used these days. Could you<br> be more specific on how did you get to see the above problems? Do you<br> really need node reclaim in your usecases or is this more about a<br> testing and seeing what happens. Not that I am against these changes but<br> I would like to understand the motivation. Especially because you are<br> exposing some internal implementation details of the node reclaim to the<br> userspace.<br><br></blockquote><div><br></div><div>The slab issue we found on our server is on old kernel (kernel-3.10).</div><div>We found that the dentry was continuesly growing without shrinking in one container on a server,</div><div>so I read slab code and found that memcg relcaim can't shrink slab in this old kenrel, </div><div>but this issue was aready fixed in upstream.</div><div><br></div><div>When I was reading the shrink slab code in the upstream kernel,</div><div>I found the slab can't be shrinked in node reclaim.</div><div>So I did some test to produce this issue and post this patchset to fix it.</div><div>With my patch, the issue produced by me disapears. </div><div><br></div><div>But this is only a beginning in the node reclaim path...</div><div>Then I found another issue when I implemented a memory pressure monitor for out containers,</div><div>which is vmpressure_prio() is missed in the node reclaim path.</div><div><br></div><div>Well, seems when we introduce new feature for page relciam, we always ignore the node reclaim path.</div><div><br></div><div>Regarding node reclaim path, we always turn it off on our servers,</div><div>because we really found some latency spike caused by node reclaim</div><div>(the reason why node reclaim is turned on is not clear).</div><div><br></div><div>The reason I expose node reclaim details to userspace is because the user can set node reclaim details now.</div><div><br></div><div>Thanks</div><div>Yafang</div></div></div>
On Thu 06-06-19 22:18:41, Yafang Shao wrote: [...] > Well, seems when we introduce new feature for page relciam, we always > ignore the node reclaim path. Yes, node reclaim is quite weird and I am not really sure whether we still have many users these days. It used to be mostly driven by artificial benchmarks which highly benefit from the local node access. We have turned off its automatic enabling when there are nodes with higher access latency quite some time ago without anybody noticing actually. > Regarding node reclaim path, we always turn it off on our servers, > because we really found some latency spike caused by node reclaim > (the reason why node reclaim is turned on is not clear). Yes, that was the case and the reason it is not enabled by default. > The reason I expose node reclaim details to userspace is because the user > can set node reclaim details now. Well, just because somebody _can_ enable it doesn't sound like a sufficient justification to expose even more implementation details of this feature. I am not really sure there is a strong reason to touch the code without a real usecase behind.
On Thu, Jun 6, 2019 at 10:44 PM Michal Hocko <mhocko@suse.com> wrote: > On Thu 06-06-19 22:18:41, Yafang Shao wrote: > [...] > > Well, seems when we introduce new feature for page relciam, we always > > ignore the node reclaim path. > > Yes, node reclaim is quite weird and I am not really sure whether we > still have many users these days. It used to be mostly driven by > artificial benchmarks which highly benefit from the local node access. > We have turned off its automatic enabling when there are nodes with > higher access latency quite some time ago without anybody noticing > actually. > > > Regarding node reclaim path, we always turn it off on our servers, > > because we really found some latency spike caused by node reclaim > > (the reason why node reclaim is turned on is not clear). > > Yes, that was the case and the reason it is not enabled by default. > > > The reason I expose node reclaim details to userspace is because the user > > can set node reclaim details now. > > Well, just because somebody _can_ enable it doesn't sound like a > sufficient justification to expose even more implementation details of > this feature. I am not really sure there is a strong reason to touch the > code without a real usecase behind. > > Got it. So should we fix the bugs in node reclaim path then? Thanks Yafang <div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jun 6, 2019 at 10:44 PM Michal Hocko <<a href="mailto:mhocko@suse.com">mhocko@suse.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Thu 06-06-19 22:18:41, Yafang Shao wrote:<br> [...]<br> > Well, seems when we introduce new feature for page relciam, we always<br> > ignore the node reclaim path.<br> <br> Yes, node reclaim is quite weird and I am not really sure whether we<br> still have many users these days. It used to be mostly driven by<br> artificial benchmarks which highly benefit from the local node access.<br> We have turned off its automatic enabling when there are nodes with<br> higher access latency quite some time ago without anybody noticing<br> actually.<br> <br> > Regarding node reclaim path, we always turn it off on our servers,<br> > because we really found some latency spike caused by node reclaim<br> > (the reason why node reclaim is turned on is not clear).<br> <br> Yes, that was the case and the reason it is not enabled by default.<br> <br> > The reason I expose node reclaim details to userspace is because the user<br> > can set node reclaim details now.<br> <br> Well, just because somebody _can_ enable it doesn't sound like a<br> sufficient justification to expose even more implementation details of<br> this feature. I am not really sure there is a strong reason to touch the<br> code without a real usecase behind.<br> <br></blockquote><div><br></div><div>Got it.</div><div><br></div><div>So should we fix the bugs in node reclaim path then?</div><div><br></div><div>Thanks</div><div>Yafang </div></div></div>
On Thu 06-06-19 23:03:12, Yafang Shao wrote: > On Thu, Jun 6, 2019 at 10:44 PM Michal Hocko <mhocko@suse.com> wrote: > > > On Thu 06-06-19 22:18:41, Yafang Shao wrote: [...] > > > The reason I expose node reclaim details to userspace is because the user > > > can set node reclaim details now. > > > > Well, just because somebody _can_ enable it doesn't sound like a > > sufficient justification to expose even more implementation details of > > this feature. I am not really sure there is a strong reason to touch the > > code without a real usecase behind. > > > > > Got it. > > So should we fix the bugs in node reclaim path then? I am all for fixing bugs, I am just nervous to expose even more internal implementation details to the userspace if there is no _real_ usecase requiring it.