Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1634147

Re: [PATCH -mm -v3] mm, swap: Sort swap entries before free

From Minchan Kim <minchan@kernel.org>
Newsgroups linux.kernel
Subject Re: [PATCH -mm -v3] mm, swap: Sort swap entries before free
Date 2017-05-02 07:50 +0200
Message-ID <tCyRb-2bw-11@gated-at.bofh.it> (permalink)
References (5 earlier) <tBaed-5ps-1@gated-at.bofh.it> <tBczn-6JZ-17@gated-at.bofh.it> <tBehR-7VW-41@gated-at.bofh.it> <tCyet-1Ys-5@gated-at.bofh.it> <tCyHw-27S-13@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


Hi Huang,

On Tue, May 02, 2017 at 01:35:24PM +0800, Huang, Ying wrote:
> Hi, Minchan,
> 
> Minchan Kim <minchan@kernel.org> writes:
> 
> > On Fri, Apr 28, 2017 at 09:35:37PM +0800, Huang, Ying wrote:
> >> In fact, during the test, I found the overhead of sort() is comparable
> >> with the performance difference of adding likely()/unlikely() to the
> >> "if" in the function.
> >
> > Huang,
> >
> > This discussion is started from your optimization code:
> >
> >         if (nr_swapfiles > 1)
> >                 sort();
> >
> > I don't have such fast machine so cannot test it. However, you added
> > such optimization code in there so I guess it's *worth* to review so
> > with spending my time, I pointed out what you are missing and
> > suggested a idea to find a compromise.
> 
> Sorry for wasting your time and Thanks a lot for your review and
> suggestion!
> 
> When I started talking this with you, I found there is some measurable
> overhead of sort().  But later when I done more tests, I found the
> measurable overhead is at the same level of likely()/unlikely() compiler
> notation.  So you help me to find that, Thanks again!
> 
> > Now you are saying sort is so fast so no worth to add more logics
> > to avoid the overhead?
> > Then, please just drop that if condition part and instead, sort
> > it unconditionally.
> 
> Now, because we found the overhead of sort() is low, I suggest to put
> minimal effort to avoid it.  Like the original implementation,
> 
>          if (nr_swapfiles > 1)
>                  sort();

It might confuse someone in future and would make him/her send a patch
to fix like we discussed. If the logic is not clear and doesn't have
measureable overhead, just leave it which is more simple/clear.

> 
> Or, we can make nr_swapfiles more correct as Tim suggested (tracking
> the number of the swap devices during swap on/off).

It might be better option but it's still hard to justify the patch
because you said it's hard to measure. Such optimiztion patch should
be from numbers.

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [PATCH -mm -v3] mm, swap: Sort swap entries before free "Huang\, Ying" <ying.huang@intel.com> - 2017-04-26 14:50 +0200
  Re: [PATCH -mm -v3] mm, swap: Sort swap entries before free Tim Chen <tim.c.chen@linux.intel.com> - 2017-04-26 22:20 +0200
    Re: [PATCH -mm -v3] mm, swap: Sort swap entries before free "Huang\, Ying" <ying.huang@intel.com> - 2017-04-27 03:30 +0200
      Re: [PATCH -mm -v3] mm, swap: Sort swap entries before free Tim Chen <tim.c.chen@linux.intel.com> - 2017-04-27 18:50 +0200
  Re: [PATCH -mm -v3] mm, swap: Sort swap entries before free Minchan Kim <minchan@kernel.org> - 2017-04-27 06:50 +0200
    Re: [PATCH -mm -v3] mm, swap: Sort swap entries before free "Huang\, Ying" <ying.huang@intel.com> - 2017-04-28 03:20 +0200
      Re: [PATCH -mm -v3] mm, swap: Sort swap entries before free Minchan Kim <minchan@kernel.org> - 2017-04-28 09:50 +0200
        Re: [PATCH -mm -v3] mm, swap: Sort swap entries before free "Huang\, Ying" <ying.huang@intel.com> - 2017-04-28 10:10 +0200
          Re: [PATCH -mm -v3] mm, swap: Sort swap entries before free Minchan Kim <minchan@kernel.org> - 2017-04-28 11:20 +0200
            Re: [PATCH -mm -v3] mm, swap: Sort swap entries before free "Huang\, Ying" <ying.huang@intel.com> - 2017-04-28 13:50 +0200
              Re: [PATCH -mm -v3] mm, swap: Sort swap entries before free "Huang\, Ying" <ying.huang@intel.com> - 2017-04-28 15:40 +0200
                Re: [PATCH -mm -v3] mm, swap: Sort swap entries before free Minchan Kim <minchan@kernel.org> - 2017-05-02 07:10 +0200
                Re: [PATCH -mm -v3] mm, swap: Sort swap entries before free "Huang\, Ying" <ying.huang@intel.com> - 2017-05-02 07:40 +0200
                Re: [PATCH -mm -v3] mm, swap: Sort swap entries before free Minchan Kim <minchan@kernel.org> - 2017-05-02 07:50 +0200
                Re: [PATCH -mm -v3] mm, swap: Sort swap entries before free "Huang\, Ying" <ying.huang@intel.com> - 2017-05-02 08:10 +0200

csiph-web