* [PATCH] dax: Fix xarray entry association for mixed mappings
@ 2019-06-06 9:10 ` Jan Kara
0 siblings, 0 replies; 6+ messages in thread
From: Jan Kara @ 2019-06-06 9:10 UTC (permalink / raw)
To: Dan Williams; +Cc: linux-fsdevel, Jan Kara, Goldwyn Rodrigues, linux-nvdimm
When inserting entry into xarray, we store mapping and index in
corresponding struct pages for memory error handling. When it happened
that one process was mapping file at PMD granularity while another
process at PTE granularity, we could wrongly deassociate PMD range and
then reassociate PTE range leaving the rest of struct pages in PMD range
without mapping information which could later cause missed notifications
about memory errors. Fix the problem by calling the association /
deassociation code if and only if we are really going to update the
xarray (deassociating and associating zero or empty entries is just
no-op so there's no reason to complicate the code with trying to avoid
the calls for these cases).
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/dax.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/fs/dax.c b/fs/dax.c
index f74386293632..9fd908f3df32 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -728,12 +728,11 @@ static void *dax_insert_entry(struct xa_state *xas,
xas_reset(xas);
xas_lock_irq(xas);
- if (dax_entry_size(entry) != dax_entry_size(new_entry)) {
+ if (dax_is_zero_entry(entry) || dax_is_empty_entry(entry)) {
+ void *old;
+
dax_disassociate_entry(entry, mapping, false);
dax_associate_entry(new_entry, mapping, vmf->vma, vmf->address);
- }
-
- if (dax_is_zero_entry(entry) || dax_is_empty_entry(entry)) {
/*
* Only swap our new entry into the page cache if the current
* entry is a zero page or an empty entry. If a normal PTE or
@@ -742,7 +741,7 @@ static void *dax_insert_entry(struct xa_state *xas,
* existing entry is a PMD, we will just leave the PMD in the
* tree and dirty it if necessary.
*/
- void *old = dax_lock_entry(xas, new_entry);
+ old = dax_lock_entry(xas, new_entry);
WARN_ON_ONCE(old != xa_mk_value(xa_to_value(entry) |
DAX_LOCKED));
entry = new_entry;
--
2.16.4
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] dax: Fix xarray entry association for mixed mappings
@ 2019-06-06 9:10 ` Jan Kara
0 siblings, 0 replies; 6+ messages in thread
From: Jan Kara @ 2019-06-06 9:10 UTC (permalink / raw)
To: Dan Williams; +Cc: Goldwyn Rodrigues, linux-fsdevel, linux-nvdimm, Jan Kara
When inserting entry into xarray, we store mapping and index in
corresponding struct pages for memory error handling. When it happened
that one process was mapping file at PMD granularity while another
process at PTE granularity, we could wrongly deassociate PMD range and
then reassociate PTE range leaving the rest of struct pages in PMD range
without mapping information which could later cause missed notifications
about memory errors. Fix the problem by calling the association /
deassociation code if and only if we are really going to update the
xarray (deassociating and associating zero or empty entries is just
no-op so there's no reason to complicate the code with trying to avoid
the calls for these cases).
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/dax.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/fs/dax.c b/fs/dax.c
index f74386293632..9fd908f3df32 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -728,12 +728,11 @@ static void *dax_insert_entry(struct xa_state *xas,
xas_reset(xas);
xas_lock_irq(xas);
- if (dax_entry_size(entry) != dax_entry_size(new_entry)) {
+ if (dax_is_zero_entry(entry) || dax_is_empty_entry(entry)) {
+ void *old;
+
dax_disassociate_entry(entry, mapping, false);
dax_associate_entry(new_entry, mapping, vmf->vma, vmf->address);
- }
-
- if (dax_is_zero_entry(entry) || dax_is_empty_entry(entry)) {
/*
* Only swap our new entry into the page cache if the current
* entry is a zero page or an empty entry. If a normal PTE or
@@ -742,7 +741,7 @@ static void *dax_insert_entry(struct xa_state *xas,
* existing entry is a PMD, we will just leave the PMD in the
* tree and dirty it if necessary.
*/
- void *old = dax_lock_entry(xas, new_entry);
+ old = dax_lock_entry(xas, new_entry);
WARN_ON_ONCE(old != xa_mk_value(xa_to_value(entry) |
DAX_LOCKED));
entry = new_entry;
--
2.16.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] dax: Fix xarray entry association for mixed mappings
2019-06-06 9:10 ` Jan Kara
@ 2019-06-06 17:00 ` Dan Williams
-1 siblings, 0 replies; 6+ messages in thread
From: Dan Williams @ 2019-06-06 17:00 UTC (permalink / raw)
To: Jan Kara; +Cc: linux-fsdevel, Goldwyn Rodrigues, linux-nvdimm
On Thu, Jun 6, 2019 at 2:10 AM Jan Kara <jack@suse.cz> wrote:
>
> When inserting entry into xarray, we store mapping and index in
> corresponding struct pages for memory error handling. When it happened
> that one process was mapping file at PMD granularity while another
> process at PTE granularity, we could wrongly deassociate PMD range and
> then reassociate PTE range leaving the rest of struct pages in PMD range
> without mapping information which could later cause missed notifications
> about memory errors. Fix the problem by calling the association /
> deassociation code if and only if we are really going to update the
> xarray (deassociating and associating zero or empty entries is just
> no-op so there's no reason to complicate the code with trying to avoid
> the calls for these cases).
Looks good to me, I assume this also needs:
Cc: <stable@vger.kernel.org>
Fixes: d2c997c0f145 ("fs, dax: use page->mapping to warn if truncate
collides with a busy page")
>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
> fs/dax.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/fs/dax.c b/fs/dax.c
> index f74386293632..9fd908f3df32 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -728,12 +728,11 @@ static void *dax_insert_entry(struct xa_state *xas,
>
> xas_reset(xas);
> xas_lock_irq(xas);
> - if (dax_entry_size(entry) != dax_entry_size(new_entry)) {
> + if (dax_is_zero_entry(entry) || dax_is_empty_entry(entry)) {
> + void *old;
> +
> dax_disassociate_entry(entry, mapping, false);
> dax_associate_entry(new_entry, mapping, vmf->vma, vmf->address);
> - }
> -
> - if (dax_is_zero_entry(entry) || dax_is_empty_entry(entry)) {
> /*
> * Only swap our new entry into the page cache if the current
> * entry is a zero page or an empty entry. If a normal PTE or
> @@ -742,7 +741,7 @@ static void *dax_insert_entry(struct xa_state *xas,
> * existing entry is a PMD, we will just leave the PMD in the
> * tree and dirty it if necessary.
> */
> - void *old = dax_lock_entry(xas, new_entry);
> + old = dax_lock_entry(xas, new_entry);
> WARN_ON_ONCE(old != xa_mk_value(xa_to_value(entry) |
> DAX_LOCKED));
> entry = new_entry;
> --
> 2.16.4
>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dax: Fix xarray entry association for mixed mappings
@ 2019-06-06 17:00 ` Dan Williams
0 siblings, 0 replies; 6+ messages in thread
From: Dan Williams @ 2019-06-06 17:00 UTC (permalink / raw)
To: Jan Kara; +Cc: Goldwyn Rodrigues, linux-fsdevel, linux-nvdimm
On Thu, Jun 6, 2019 at 2:10 AM Jan Kara <jack@suse.cz> wrote:
>
> When inserting entry into xarray, we store mapping and index in
> corresponding struct pages for memory error handling. When it happened
> that one process was mapping file at PMD granularity while another
> process at PTE granularity, we could wrongly deassociate PMD range and
> then reassociate PTE range leaving the rest of struct pages in PMD range
> without mapping information which could later cause missed notifications
> about memory errors. Fix the problem by calling the association /
> deassociation code if and only if we are really going to update the
> xarray (deassociating and associating zero or empty entries is just
> no-op so there's no reason to complicate the code with trying to avoid
> the calls for these cases).
Looks good to me, I assume this also needs:
Cc: <stable@vger.kernel.org>
Fixes: d2c997c0f145 ("fs, dax: use page->mapping to warn if truncate
collides with a busy page")
>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
> fs/dax.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/fs/dax.c b/fs/dax.c
> index f74386293632..9fd908f3df32 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -728,12 +728,11 @@ static void *dax_insert_entry(struct xa_state *xas,
>
> xas_reset(xas);
> xas_lock_irq(xas);
> - if (dax_entry_size(entry) != dax_entry_size(new_entry)) {
> + if (dax_is_zero_entry(entry) || dax_is_empty_entry(entry)) {
> + void *old;
> +
> dax_disassociate_entry(entry, mapping, false);
> dax_associate_entry(new_entry, mapping, vmf->vma, vmf->address);
> - }
> -
> - if (dax_is_zero_entry(entry) || dax_is_empty_entry(entry)) {
> /*
> * Only swap our new entry into the page cache if the current
> * entry is a zero page or an empty entry. If a normal PTE or
> @@ -742,7 +741,7 @@ static void *dax_insert_entry(struct xa_state *xas,
> * existing entry is a PMD, we will just leave the PMD in the
> * tree and dirty it if necessary.
> */
> - void *old = dax_lock_entry(xas, new_entry);
> + old = dax_lock_entry(xas, new_entry);
> WARN_ON_ONCE(old != xa_mk_value(xa_to_value(entry) |
> DAX_LOCKED));
> entry = new_entry;
> --
> 2.16.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dax: Fix xarray entry association for mixed mappings
2019-06-06 17:00 ` Dan Williams
@ 2019-06-06 21:28 ` Jan Kara
-1 siblings, 0 replies; 6+ messages in thread
From: Jan Kara @ 2019-06-06 21:28 UTC (permalink / raw)
To: Dan Williams; +Cc: linux-fsdevel, Jan Kara, Goldwyn Rodrigues, linux-nvdimm
On Thu 06-06-19 10:00:01, Dan Williams wrote:
> On Thu, Jun 6, 2019 at 2:10 AM Jan Kara <jack@suse.cz> wrote:
> >
> > When inserting entry into xarray, we store mapping and index in
> > corresponding struct pages for memory error handling. When it happened
> > that one process was mapping file at PMD granularity while another
> > process at PTE granularity, we could wrongly deassociate PMD range and
> > then reassociate PTE range leaving the rest of struct pages in PMD range
> > without mapping information which could later cause missed notifications
> > about memory errors. Fix the problem by calling the association /
> > deassociation code if and only if we are really going to update the
> > xarray (deassociating and associating zero or empty entries is just
> > no-op so there's no reason to complicate the code with trying to avoid
> > the calls for these cases).
>
> Looks good to me, I assume this also needs:
>
> Cc: <stable@vger.kernel.org>
> Fixes: d2c997c0f145 ("fs, dax: use page->mapping to warn if truncate
> collides with a busy page")
Yes, thanks for that.
Honza
>
> >
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> > fs/dax.c | 9 ++++-----
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/dax.c b/fs/dax.c
> > index f74386293632..9fd908f3df32 100644
> > --- a/fs/dax.c
> > +++ b/fs/dax.c
> > @@ -728,12 +728,11 @@ static void *dax_insert_entry(struct xa_state *xas,
> >
> > xas_reset(xas);
> > xas_lock_irq(xas);
> > - if (dax_entry_size(entry) != dax_entry_size(new_entry)) {
> > + if (dax_is_zero_entry(entry) || dax_is_empty_entry(entry)) {
> > + void *old;
> > +
> > dax_disassociate_entry(entry, mapping, false);
> > dax_associate_entry(new_entry, mapping, vmf->vma, vmf->address);
> > - }
> > -
> > - if (dax_is_zero_entry(entry) || dax_is_empty_entry(entry)) {
> > /*
> > * Only swap our new entry into the page cache if the current
> > * entry is a zero page or an empty entry. If a normal PTE or
> > @@ -742,7 +741,7 @@ static void *dax_insert_entry(struct xa_state *xas,
> > * existing entry is a PMD, we will just leave the PMD in the
> > * tree and dirty it if necessary.
> > */
> > - void *old = dax_lock_entry(xas, new_entry);
> > + old = dax_lock_entry(xas, new_entry);
> > WARN_ON_ONCE(old != xa_mk_value(xa_to_value(entry) |
> > DAX_LOCKED));
> > entry = new_entry;
> > --
> > 2.16.4
> >
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] dax: Fix xarray entry association for mixed mappings
@ 2019-06-06 21:28 ` Jan Kara
0 siblings, 0 replies; 6+ messages in thread
From: Jan Kara @ 2019-06-06 21:28 UTC (permalink / raw)
To: Dan Williams; +Cc: Jan Kara, Goldwyn Rodrigues, linux-fsdevel, linux-nvdimm
On Thu 06-06-19 10:00:01, Dan Williams wrote:
> On Thu, Jun 6, 2019 at 2:10 AM Jan Kara <jack@suse.cz> wrote:
> >
> > When inserting entry into xarray, we store mapping and index in
> > corresponding struct pages for memory error handling. When it happened
> > that one process was mapping file at PMD granularity while another
> > process at PTE granularity, we could wrongly deassociate PMD range and
> > then reassociate PTE range leaving the rest of struct pages in PMD range
> > without mapping information which could later cause missed notifications
> > about memory errors. Fix the problem by calling the association /
> > deassociation code if and only if we are really going to update the
> > xarray (deassociating and associating zero or empty entries is just
> > no-op so there's no reason to complicate the code with trying to avoid
> > the calls for these cases).
>
> Looks good to me, I assume this also needs:
>
> Cc: <stable@vger.kernel.org>
> Fixes: d2c997c0f145 ("fs, dax: use page->mapping to warn if truncate
> collides with a busy page")
Yes, thanks for that.
Honza
>
> >
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> > fs/dax.c | 9 ++++-----
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/dax.c b/fs/dax.c
> > index f74386293632..9fd908f3df32 100644
> > --- a/fs/dax.c
> > +++ b/fs/dax.c
> > @@ -728,12 +728,11 @@ static void *dax_insert_entry(struct xa_state *xas,
> >
> > xas_reset(xas);
> > xas_lock_irq(xas);
> > - if (dax_entry_size(entry) != dax_entry_size(new_entry)) {
> > + if (dax_is_zero_entry(entry) || dax_is_empty_entry(entry)) {
> > + void *old;
> > +
> > dax_disassociate_entry(entry, mapping, false);
> > dax_associate_entry(new_entry, mapping, vmf->vma, vmf->address);
> > - }
> > -
> > - if (dax_is_zero_entry(entry) || dax_is_empty_entry(entry)) {
> > /*
> > * Only swap our new entry into the page cache if the current
> > * entry is a zero page or an empty entry. If a normal PTE or
> > @@ -742,7 +741,7 @@ static void *dax_insert_entry(struct xa_state *xas,
> > * existing entry is a PMD, we will just leave the PMD in the
> > * tree and dirty it if necessary.
> > */
> > - void *old = dax_lock_entry(xas, new_entry);
> > + old = dax_lock_entry(xas, new_entry);
> > WARN_ON_ONCE(old != xa_mk_value(xa_to_value(entry) |
> > DAX_LOCKED));
> > entry = new_entry;
> > --
> > 2.16.4
> >
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-06-06 21:28 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-06 9:10 [PATCH] dax: Fix xarray entry association for mixed mappings Jan Kara
2019-06-06 9:10 ` Jan Kara
2019-06-06 17:00 ` Dan Williams
2019-06-06 17:00 ` Dan Williams
2019-06-06 21:28 ` Jan Kara
2019-06-06 21:28 ` Jan Kara
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.