diff mbox series

mm/vmscan: remove prefetch_prev_lru_page

Message ID 1579006500-127143-1-git-send-email-alex.shi@linux.alibaba.com (mailing list archive)
State New, archived
Headers show
Series mm/vmscan: remove prefetch_prev_lru_page | expand

Commit Message

Alex Shi Jan. 14, 2020, 12:55 p.m. UTC
This macro are never used in git history. So better to remove.

Signed-off-by: Alex Shi <alex.shi@linux.alibaba.com>
Cc: Andrew Morton <akpm@linux-foundation.org> 
Cc: linux-mm@kvack.org 
Cc: linux-kernel@vger.kernel.org 
---
 mm/vmscan.c | 14 --------------
 1 file changed, 14 deletions(-)

Comments

Qian Cai Jan. 14, 2020, 1:46 p.m. UTC | #1
> On Jan 14, 2020, at 7:55 AM, Alex Shi <alex.shi@linux.alibaba.com> wrote:
> 
> This macro are never used in git history. So better to remove.

When removing unused thingy, it is important to figure out which commit introduced it in the first place and Cc the relevant people in that commit.
Alex Shi Jan. 15, 2020, 2:31 a.m. UTC | #2
在 2020/1/14 下午9:46, Qian Cai 写道:
> 
> 
>> On Jan 14, 2020, at 7:55 AM, Alex Shi <alex.shi@linux.alibaba.com> wrote:
>>
>> This macro are never used in git history. So better to remove.
> 
> When removing unused thingy, it is important to figure out which commit introduced it in the first place and Cc the relevant people in that commit.
> 

Thanks fore reminder, Qian!

This macro was introduced in 1da177e4c3f4 Linux-2.6.12-rc2, no author or commiter could be found.

Thanks
Alex
Christoph Hellwig Jan. 15, 2020, 8:29 a.m. UTC | #3
On Tue, Jan 14, 2020 at 08:46:21AM -0500, Qian Cai wrote:
> 
> 
> > On Jan 14, 2020, at 7:55 AM, Alex Shi <alex.shi@linux.alibaba.com> wrote:
> > 
> > This macro are never used in git history. So better to remove.
> 
> When removing unused thingy, it is important to figure out which commit introduced it in the first place and Cc the relevant people in that commit.

No, it isn't.  It is at best nice to have, but for a trivial macro
really doesn't matter.
Qian Cai Jan. 15, 2020, 6:27 p.m. UTC | #4
> On Jan 15, 2020, at 3:29 AM, Christoph Hellwig <hch@infradead.org> wrote:
> 
> On Tue, Jan 14, 2020 at 08:46:21AM -0500, Qian Cai wrote:
>> 
>> 
>>> On Jan 14, 2020, at 7:55 AM, Alex Shi <alex.shi@linux.alibaba.com> wrote:
>>> 
>>> This macro are never used in git history. So better to remove.
>> 
>> When removing unused thingy, it is important to figure out which commit introduced it in the first place and Cc the relevant people in that commit.
> 
> No, it isn't.  It is at best nice to have, but for a trivial macro
> really doesn't matter.

A more of personal taste what the trivial macro is. I’d rather be on the caution side
when removing code especially nowaday developers may not even compile
test the patch properly given how many arches we have here which will only waste
time on other people when things goes wrong.
Qian Cai Jan. 16, 2020, 12:26 p.m. UTC | #5
> On Jan 14, 2020, at 9:33 PM, Alex Shi <alex.shi@linux.alibaba.com> wrote:
> 
> 
> 
>> 在 2020/1/14 下午9:46, Qian Cai 写道:
>> 
>> 
>>>> On Jan 14, 2020, at 7:55 AM, Alex Shi <alex.shi@linux.alibaba.com> wrote:
>>> 
>>> This macro are never used in git history. So better to remove.
>> 
>> When removing unused thingy, it is important to figure out which commit introduced it in the first place and Cc the relevant people in that commit.
>> 
> 
> Thanks fore reminder, Qian!
> 
> This macro was introduced in 1da177e4c3f4 Linux-2.6.12-rc2, no author or commiter could be found.

Looks a bit deeper for this, and I am not sure if it is necessary to remove it especially this does not cause any complication warning noise, because the macro looks like a part of API design to have a pair of both read and write version, even though only the write version is used at the moment.

In theory,  there could be users for the read version in the future, and then it needs to be added back.
Guoqing Jiang Jan. 16, 2020, 1:18 p.m. UTC | #6
On 1/15/20 3:31 AM, Alex Shi wrote:
> 在 2020/1/14 下午9:46, Qian Cai 写道:
>>> On Jan 14, 2020, at 7:55 AM, Alex Shi <alex.shi@linux.alibaba.com> wrote:
>>>
>>> This macro are never used in git history. So better to remove.
>> When removing unused thingy, it is important to figure out which commit introduced it in the first place and Cc the relevant people in that commit.
>>
> Thanks fore reminder, Qian!
>
> This macro was introduced in 1da177e4c3f4 Linux-2.6.12-rc2, no author or commiter could be found.

FYI, seems it was introduced in commit 3aa1dc7725 [PATCH] multithread 
page reclaim in history tree.

Thanks,
Guoqing
Andrew Morton Jan. 16, 2020, 11:37 p.m. UTC | #7
On Thu, 16 Jan 2020 07:26:23 -0500 Qian Cai <cai@lca.pw> wrote:

> 
> 
> > On Jan 14, 2020, at 9:33 PM, Alex Shi <alex.shi@linux.alibaba.com> wrote:
> > 
> > 
> > 
> >> 在 2020/1/14 下午9:46, Qian Cai 写道:
> >> 
> >> 
> >>>> On Jan 14, 2020, at 7:55 AM, Alex Shi <alex.shi@linux.alibaba.com> wrote:
> >>> 
> >>> This macro are never used in git history. So better to remove.
> >> 
> >> When removing unused thingy, it is important to figure out which commit introduced it in the first place and Cc the relevant people in that commit.
> >> 
> > 
> > Thanks fore reminder, Qian!
> > 
> > This macro was introduced in 1da177e4c3f4 Linux-2.6.12-rc2, no author or commiter could be found.
> 
> Looks a bit deeper for this, and I am not sure if it is necessary to remove it especially this does not cause any complication warning noise, because the macro looks like a part of API design to have a pair of both read and write version, even though only the write version is used at the moment.
> 
> In theory,  there could be users for the read version in the future, and then it needs to be added back.

Sure.  A problem with leaving it in place is that this leads people to
assume it is tested, which it presumably is not.

I don't think there's any particular downside either way, really.  But
it's presently cruft so I'm inclined to remove it.  If someone has a
need then they can add it back (presumbly reimplement it, actually) and
test it then.
diff mbox series

Patch

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 4e699ed3501e..033e7145061b 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -146,20 +146,6 @@  struct scan_control {
 	struct reclaim_state reclaim_state;
 };
 
-#ifdef ARCH_HAS_PREFETCH
-#define prefetch_prev_lru_page(_page, _base, _field)			\
-	do {								\
-		if ((_page)->lru.prev != _base) {			\
-			struct page *prev;				\
-									\
-			prev = lru_to_page(&(_page->lru));		\
-			prefetch(&prev->_field);			\
-		}							\
-	} while (0)
-#else
-#define prefetch_prev_lru_page(_page, _base, _field) do { } while (0)
-#endif
-
 #ifdef ARCH_HAS_PREFETCHW
 #define prefetchw_prev_lru_page(_page, _base, _field)			\
 	do {								\