From: Michal Hocko <mhocko@suse.com>
To: Wei Yang <richard.weiyang@gmail.com>
Cc: akpm@linux-foundation.org, mgorman@techsingularity.net,
linux-mm@kvack.org
Subject: Re: [PATCH] mm: get pfn by page_to_pfn() instead of save in page->private
Date: Thu, 18 Oct 2018 18:30:39 +0200 [thread overview]
Message-ID: <20181018163039.GF18839@dhcp22.suse.cz> (raw)
In-Reply-To: <20181018141008.lcyttmp7bb42uigi@master>
On Thu 18-10-18 14:10:08, Wei Yang wrote:
> On Thu, Oct 18, 2018 at 03:15:04PM +0200, Michal Hocko wrote:
> >On Thu 18-10-18 21:04:29, Wei Yang wrote:
> >> This is not necessary to save the pfn to page->private.
> >>
> >> The pfn could be retrieved by page_to_pfn() directly.
> >
> >Yes it can, but a cursory look at the commit which has introduced this
> >suggests that this is a micro-optimization. Mel would know more of
> >course. There are some memory models where page_to_pfn is close to free.
> >
> >If that is the case I am not really sure it is measurable or worth it.
> >In any case any change to this code should have a proper justification.
> >In other words, is this change really needed? Does it help in any
> >aspect? Possibly readability? The only thing I can guess from this
> >changelog is that you read the code and stumble over this. If that is
> >the case I would recommend asking author for the motivation and
> >potentially add a comment to explain it better rather than shoot a patch
> >rightaway.
> >
>
> Your are right. I am really willing to understand why we want to use
> this mechanisum.
I am happy to hear that.
> So the correct procedure is to send a mail to the mail list to query the
> reason?
It is certainly better to ask a question than send a patch without a
proper justification. I would also encourage to use git blame to see
which patch has introduced the specific piece of code. Many times it
helps to understand the motivation. I would also encourage to go back to
the mailing list archives and the associate discussion to the specific
patch. In many cases there is Link: tag which can help you to find the
respective discussion.
Thanks!
--
Michal Hocko
SUSE Labs
next prev parent reply other threads:[~2018-10-18 16:30 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-18 13:04 [PATCH] mm: get pfn by page_to_pfn() instead of save in page->private Wei Yang
2018-10-18 13:10 ` Matthew Wilcox
2018-10-18 13:15 ` Michal Hocko
2018-10-18 14:10 ` Wei Yang
2018-10-18 16:30 ` Michal Hocko [this message]
2018-10-19 3:32 ` Wei Yang
2018-10-18 13:39 ` Mel Gorman
2018-10-18 14:19 ` Wei Yang
2018-10-18 14:53 ` Mel Gorman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181018163039.GF18839@dhcp22.suse.cz \
--to=mhocko@suse.com \
--cc=akpm@linux-foundation.org \
--cc=linux-mm@kvack.org \
--cc=mgorman@techsingularity.net \
--cc=richard.weiyang@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.