Re: [PATCH] try_to_unuse : remove redundant swap_count()

[Hugh Dickins <hugh.dickins@tiscali.co.uk>]

On Wed, 21 Oct 2009, Bob Liu wrote:
> >>
> >> While comparing with swcount,it's no need to
> >> call swap_count(). Just as int set_start_mm =
> >> (*swap_map>= swcount) is ok.
> >>
> > Hmm ?
> > *swap_map = (SWAP_HAS_CACHE) | count. What this change means ?
> >
> 
> Sorry for the wrong format, I changed to gmail.
> Because swcount is assigned value *swap_map not swap_count(*swap_map).
> So I think here should compare with *swap_map not swap_count(*swap_map).
> 
> And refer to variable set_start_mm, it is inited also by comparing
> *swap_map and swcount not swap_count(*swap_map) and swcount.
> So I submited this patch.

Thanks a lot for the fuller description: I mistakenly dismissed
your patch the first time, misunderstanding what you had found.

As I remarked in private mail (being smtp-challenged last week),
what you found was worse than a redundant use of swap_count(): it
was a wrong use of swap_count(), and caused an (easily overlooked)
regression in swapoff's (never wonderful) performance.

> 
> > Anyway, swap_count() macro is removed by Hugh's patch (queued in -mm)

Actually no: I removed some of the other wrappers, which were obscuring
things for me; but swap_count() still seemed useful, so I left it.

> >
> I am sorry for not notice that. So just forget about this patch.

No, let's not forget it at all, it was a good find, thank you.
Updated version of the patch comes in following mail.

Hugh

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>