| Message ID | 1608186532-81218-1-git-send-email-alex.shi@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] mm/memcg: revise the using condition of lock_page_lruvec function series | expand |
On Thu, 17 Dec 2020, Alex Shi wrote: > The series function could be used under lock_page_memcg(), add this and > a bit style changes following commit_charge(). > > Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com> > Cc: Hugh Dickins <hughd@google.com> This patch, or its intention, Acked-by: Hugh Dickins <hughd@google.com> but rewording suggested below, and requested above - which left me very puzzled before eventually I understood it. I don't think we need to talk about "a bit style changes", but the cross-reference to commit_charge() is helpful. " lock_page_lruvec() and its variants are safe to use under the same conditions as commit_charge(): add lock_page_memcg() to the comment. " > Cc: Johannes Weiner <hannes@cmpxchg.org> > Cc: Michal Hocko <mhocko@kernel.org> > Cc: Vladimir Davydov <vdavydov.dev@gmail.com> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: cgroups@vger.kernel.org > Cc: linux-mm@kvack.org > Cc: linux-kernel@vger.kernel.org > --- > mm/memcontrol.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index b80328f52fb4..e6b50d068b2f 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -1345,10 +1345,11 @@ void lruvec_memcg_debug(struct lruvec *lruvec, struct page *page) > * lock_page_lruvec - lock and return lruvec for a given page. > * @page: the page > * > - * This series functions should be used in either conditions: > - * PageLRU is cleared or unset > - * or page->_refcount is zero > - * or page is locked. > + * This series functions should be used in any one of following conditions: These functions are safe to use under any of the following conditions: > + * - PageLRU is cleared or unset > + * - page->_refcount is zero > + * - page is locked. Remove that full stop... > + * - lock_page_memcg() ... and, if you wish (I don't care), add full stop at the end of that line. Maybe reorder those to the same order as listed in commit_charge(). Copy its text exactly? I don't think so, actually, I find your wording (e.g. _refcount is zero) more explicit: good to have both descriptions. > */ > struct lruvec *lock_page_lruvec(struct page *page) > { > -- > 2.29.GIT
在 2020/12/22 上午11:01, Hugh Dickins 写道: > On Thu, 17 Dec 2020, Alex Shi wrote: > >> The series function could be used under lock_page_memcg(), add this and >> a bit style changes following commit_charge(). >> >> Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com> >> Cc: Hugh Dickins <hughd@google.com> > > This patch, or its intention, > Acked-by: Hugh Dickins <hughd@google.com> > but rewording suggested below, and requested above - > which left me very puzzled before eventually I understood it. > I don't think we need to talk about "a bit style changes", > but the cross-reference to commit_charge() is helpful. > > " > lock_page_lruvec() and its variants are safe to use under the same > conditions as commit_charge(): add lock_page_memcg() to the comment. > " Thanks a lot, Hugh. Yes, your commit log are far more better than mine. :) I will resent with your changes and Ack. Thanks! Alex > >> Cc: Johannes Weiner <hannes@cmpxchg.org> >> Cc: Michal Hocko <mhocko@kernel.org> >> Cc: Vladimir Davydov <vdavydov.dev@gmail.com> >> Cc: Andrew Morton <akpm@linux-foundation.org> >> Cc: cgroups@vger.kernel.org >> Cc: linux-mm@kvack.org >> Cc: linux-kernel@vger.kernel.org >> --- >> mm/memcontrol.c | 9 +++++---- >> 1 file changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c >> index b80328f52fb4..e6b50d068b2f 100644 >> --- a/mm/memcontrol.c >> +++ b/mm/memcontrol.c >> @@ -1345,10 +1345,11 @@ void lruvec_memcg_debug(struct lruvec *lruvec, struct page *page) >> * lock_page_lruvec - lock and return lruvec for a given page. >> * @page: the page >> * >> - * This series functions should be used in either conditions: >> - * PageLRU is cleared or unset >> - * or page->_refcount is zero >> - * or page is locked. >> + * This series functions should be used in any one of following conditions: > > These functions are safe to use under any of the following conditions: > >> + * - PageLRU is cleared or unset >> + * - page->_refcount is zero >> + * - page is locked. > > Remove that full stop... > >> + * - lock_page_memcg() > > ... and, if you wish (I don't care), add full stop at the end of that line. > > Maybe reorder those to the same order as listed in commit_charge(). > Copy its text exactly? I don't think so, actually, I find your wording > (e.g. _refcount is zero) more explicit: good to have both descriptions. > >> */ >> struct lruvec *lock_page_lruvec(struct page *page) >> { >> -- >> 2.29.GIT
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index b80328f52fb4..e6b50d068b2f 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -1345,10 +1345,11 @@ void lruvec_memcg_debug(struct lruvec *lruvec, struct page *page) * lock_page_lruvec - lock and return lruvec for a given page. * @page: the page * - * This series functions should be used in either conditions: - * PageLRU is cleared or unset - * or page->_refcount is zero - * or page is locked. + * This series functions should be used in any one of following conditions: + * - PageLRU is cleared or unset + * - page->_refcount is zero + * - page is locked. + * - lock_page_memcg() */ struct lruvec *lock_page_lruvec(struct page *page) {
The series function could be used under lock_page_memcg(), add this and a bit style changes following commit_charge(). Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com> Cc: Hugh Dickins <hughd@google.com> Cc: Johannes Weiner <hannes@cmpxchg.org> Cc: Michal Hocko <mhocko@kernel.org> Cc: Vladimir Davydov <vdavydov.dev@gmail.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: cgroups@vger.kernel.org Cc: linux-mm@kvack.org Cc: linux-kernel@vger.kernel.org --- mm/memcontrol.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)