All of lore.kernel.org
 help / color / mirror / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: Cong Wang <xiyou.wangcong@gmail.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	linux-edac@vger.kernel.org, Tony Luck <tony.luck@intel.com>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH v2 1/2] ras: fix an off-by-one error in __find_elem()
Date: Sat, 20 Apr 2019 21:04:42 +0200	[thread overview]
Message-ID: <20190420190442.GF29704@zn.tnic> (raw)
In-Reply-To: <CAM_iQpXKWDr4AbROO=ZbkxwQHkj4anA9FJAaohOqg1SCq0L7eg@mail.gmail.com>

On Sat, Apr 20, 2019 at 11:25:43AM -0700, Cong Wang wrote:
> If you want to go that far, you can choose to use lib/bsearch.c too in
> case you want to reinvent the wheel.

Well, that doesn't give me the @to functionality which points to the
slot where the new element should be inserted, when the search was
unsuccessful.

> What's your point here?

My point is to fix it properly. Obviously.

> You know my fix is targeted for -stable,

Well, first you sent me this:

https://lkml.kernel.org/r/20190416012001.5338-1-xiyou.wangcong@gmail.com

then this:

https://lkml.kernel.org/r/20190416213351.28999-1-xiyou.wangcong@gmail.com

Tony liked this second version more and if you look at the final result of mine:

	int min = 0, max = ca->n - 1;

	...

                if (this_pfn < pfn)
                        min = i + 1;
                else if (this_pfn > pfn)
                        max = i - 1;
                else if (this_pfn == pfn) {
                        if (to)
                                *to = i;
                        return i;
                }

it has basically *both*: the correct [min:max] range *and* the return of
ithe ndex when found. But the algorithm this time is the correct one.

> I doubt your 83-line change could fit for -stable.

My 83-line change has debug output only for experimentation. It will,
*of* *course* be removed before committing it upstream. That's why I
called it "a conglomerate patch" and I said "It has some debug output
for easier debugging, that will be removed in the final version, of
course." I guess you didn't read that either.

And the sanity_check() piece will be a separate patch, of course.

In the end the diffstat will be 30-40 lines max.

> Feel free to drop my patch to favor yours. I am really tired.

Suit yourself. Thanks for the reporting.

> Good luck with that!

Ditto.

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

WARNING: multiple messages have this Message-ID (diff)
From: Borislav Petkov <bp@alien8.de>
To: Cong Wang <xiyou.wangcong@gmail.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
	linux-edac@vger.kernel.org, Tony Luck <tony.luck@intel.com>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: [v2,1/2] ras: fix an off-by-one error in __find_elem()
Date: Sat, 20 Apr 2019 21:04:42 +0200	[thread overview]
Message-ID: <20190420190442.GF29704@zn.tnic> (raw)

On Sat, Apr 20, 2019 at 11:25:43AM -0700, Cong Wang wrote:
> If you want to go that far, you can choose to use lib/bsearch.c too in
> case you want to reinvent the wheel.

Well, that doesn't give me the @to functionality which points to the
slot where the new element should be inserted, when the search was
unsuccessful.

> What's your point here?

My point is to fix it properly. Obviously.

> You know my fix is targeted for -stable,

Well, first you sent me this:

https://lkml.kernel.org/r/20190416012001.5338-1-xiyou.wangcong@gmail.com

then this:

https://lkml.kernel.org/r/20190416213351.28999-1-xiyou.wangcong@gmail.com

Tony liked this second version more and if you look at the final result of mine:

	int min = 0, max = ca->n - 1;

	...

                if (this_pfn < pfn)
                        min = i + 1;
                else if (this_pfn > pfn)
                        max = i - 1;
                else if (this_pfn == pfn) {
                        if (to)
                                *to = i;
                        return i;
                }

it has basically *both*: the correct [min:max] range *and* the return of
ithe ndex when found. But the algorithm this time is the correct one.

> I doubt your 83-line change could fit for -stable.

My 83-line change has debug output only for experimentation. It will,
*of* *course* be removed before committing it upstream. That's why I
called it "a conglomerate patch" and I said "It has some debug output
for easier debugging, that will be removed in the final version, of
course." I guess you didn't read that either.

And the sanity_check() piece will be a separate patch, of course.

In the end the diffstat will be 30-40 lines max.

> Feel free to drop my patch to favor yours. I am really tired.

Suit yourself. Thanks for the reporting.

> Good luck with that!

Ditto.

  reply	other threads:[~2019-04-20 19:04 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-16 21:33 [PATCH v2 1/2] ras: fix an off-by-one error in __find_elem() Cong Wang
2019-04-16 21:33 ` [v2,1/2] " Cong Wang
2019-04-16 21:33 ` [PATCH v2 2/2] ras: close the race condition with timer Cong Wang
2019-04-16 21:33   ` [v2,2/2] " Cong Wang
2019-06-08 15:28   ` [tip:ras/urgent] RAS/CEC: Convert the timer callback to a workqueue tip-bot for Cong Wang
2019-04-16 21:46 ` [PATCH v2 1/2] ras: fix an off-by-one error in __find_elem() Borislav Petkov
2019-04-16 21:46   ` [v2,1/2] " Borislav Petkov
2019-04-16 23:16   ` [PATCH v2 1/2] " Cong Wang
2019-04-16 23:16     ` [v2,1/2] " Cong Wang
2019-04-20 11:34     ` [PATCH v2 1/2] " Borislav Petkov
2019-04-20 11:34       ` [v2,1/2] " Borislav Petkov
2019-04-20 18:25       ` [PATCH v2 1/2] " Cong Wang
2019-04-20 18:25         ` [v2,1/2] " Cong Wang
2019-04-20 19:04         ` Borislav Petkov [this message]
2019-04-20 19:04           ` Borislav Petkov
2019-04-20 19:15           ` [PATCH v2 1/2] " Cong Wang
2019-04-20 19:15             ` [v2,1/2] " Cong Wang
2019-04-21  8:27             ` [PATCH v2 1/2] " Borislav Petkov
2019-04-21  8:27               ` [v2,1/2] " Borislav Petkov

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=20190420190442.GF29704@zn.tnic \
    --to=bp@alien8.de \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=xiyou.wangcong@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.