linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "HORIGUCHI NAOYA(堀口 直也)" <naoya.horiguchi@nec.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: "linux-mm@kvack.org" <linux-mm@kvack.org>,
	Tony Luck <tony.luck@intel.com>, Aili Yao <yaoaili@kingsoft.com>,
	Oscar Salvador <osalvador@suse.de>,
	David Hildenbrand <david@redhat.com>,
	Borislav Petkov <bp@alien8.de>, Andy Lutomirski <luto@kernel.org>,
	Jue Wang <juew@google.com>,
	Naoya Horiguchi <nao.horiguchi@gmail.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v5 3/3] mm,hwpoison: Send SIGBUS with error virutal address
Date: Thu, 3 Jun 2021 05:10:56 +0000	[thread overview]
Message-ID: <20210603051055.GA244241@hori.linux.bs1.fc.nec.co.jp> (raw)
In-Reply-To: <20210521030156.2612074-4-nao.horiguchi@gmail.com>

On Fri, May 21, 2021 at 12:01:56PM +0900, Naoya Horiguchi wrote:
> From: Naoya Horiguchi <naoya.horiguchi@nec.com>
> 
> Now an action required MCE in already hwpoisoned address surely sends a
> SIGBUS to current process, but the SIGBUS doesn't convey error virtual
> address.  That's not optimal for hwpoison-aware applications.
> 
> To fix the issue, make memory_failure() call kill_accessing_process(),
> that does pagetable walk to find the error virtual address.  It could
> find multiple virtual addresses for the same error page, and it seems
> hard to tell which virtual address is correct one.  But that's rare
> and sending incorrect virtual address could be better than no address.
> So let's report the first found virtual address for now.
> 
> Signed-off-by: Naoya Horiguchi <naoya.horiguchi@nec.com>
> ---
> change log v4 -> v5:
> - switched to first found approach,
> - introduced check_hwpoisoned_pmd_entry() to fix build failure on arch
>   without thp support.
> 
> change log v3 -> v4:
> - refactored hwpoison_pte_range to save indentation,
> - updated patch description
> 
> change log v1 -> v2:
> - initialize local variables in check_hwpoisoned_entry() and
>   hwpoison_pte_range()
> - fix and improve logic to calculate error address offset.
> ---
...
> +static int kill_accessing_process(struct task_struct *p, unsigned long pfn,
> +				  int flags)
> +{
> +	int ret;
> +	struct hwp_walk priv = {
> +		.pfn = pfn,
> +	};
> +	priv.tk.tsk = p;
> +
> +	mmap_read_lock(p->mm);
> +	ret = walk_page_range(p->mm, 0, TASK_SIZE, &hwp_walk_ops,
> +			      (void *)&priv);
> +	if (!ret && priv.tk.addr)

Sorry, I found a silly mistake, the walk_page_range() got to return 1 when it
found at least error virtual address since v5, so this if-condition should be
like this.

@@ -691,7 +691,8 @@ static int kill_accessing_process(struct task_struct *p, unsigned long pfn,
        mmap_read_lock(p->mm);
        ret = walk_page_range(p->mm, 0, TASK_SIZE, &hwp_walk_ops,
                              (void *)&priv);
-       if (!ret && priv.tk.addr)
+       if (ret == 1 && priv.tk.addr)
                kill_proc(&priv.tk, pfn, flags);
        mmap_read_unlock(p->mm);
        return ret ? -EFAULT : -EHWPOISON;

Andrew, this patch is now in linux-mm, so could you apply this fix onto
mmhwpoison-send-sigbus-with-error-virutal-address.patch ?
Or if it's better to resend a whole patch, please let me know.

Thanks,
Naoya Horiguchi

      reply	other threads:[~2021-06-03  5:11 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-21  3:01 [PATCH v5 0/3] mm,hwpoison: fix sending SIGBUS for Action Required MCE Naoya Horiguchi
2021-05-21  3:01 ` [PATCH v5 1/3] mm/memory-failure: Use a mutex to avoid memory_failure() races Naoya Horiguchi
2021-05-22 22:09   ` Andrew Morton
2021-05-24  8:42     ` HORIGUCHI NAOYA(堀口 直也)
2021-05-26  9:47   ` Oscar Salvador
2021-05-21  3:01 ` [PATCH v5 2/3] mm,hwpoison: Return -EHWPOISON to denote that the page has already been poisoned Naoya Horiguchi
2021-05-26 10:18   ` Oscar Salvador
2021-05-21  3:01 ` [PATCH v5 3/3] mm,hwpoison: Send SIGBUS with error virutal address Naoya Horiguchi
2021-06-03  5:10   ` HORIGUCHI NAOYA(堀口 直也) [this message]

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=20210603051055.GA244241@hori.linux.bs1.fc.nec.co.jp \
    --to=naoya.horiguchi@nec.com \
    --cc=akpm@linux-foundation.org \
    --cc=bp@alien8.de \
    --cc=david@redhat.com \
    --cc=juew@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@kernel.org \
    --cc=nao.horiguchi@gmail.com \
    --cc=osalvador@suse.de \
    --cc=tony.luck@intel.com \
    --cc=yaoaili@kingsoft.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox