Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1238615 > unrolled thread
| Started by | Ross Zwisler <ross.zwisler@linux.intel.com> |
|---|---|
| First post | 2015-10-02 23:10 +0200 |
| Last post | 2015-10-05 10:50 +0200 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 1/3] Revert "dax: fix NULL pointer in __dax_pmd_fault()" Ross Zwisler <ross.zwisler@linux.intel.com> - 2015-10-02 23:10 +0200
Re: [PATCH 1/3] Revert "dax: fix NULL pointer in __dax_pmd_fault()" Dan Williams <dan.j.williams@intel.com> - 2015-10-02 23:20 +0200
Re: [PATCH 1/3] Revert "dax: fix NULL pointer in __dax_pmd_fault()" Ross Zwisler <ross.zwisler@linux.intel.com> - 2015-10-03 01:30 +0200
Re: [PATCH 1/3] Revert "dax: fix NULL pointer in __dax_pmd_fault()" Jan Kara <jack@suse.cz> - 2015-10-05 10:50 +0200
| From | Ross Zwisler <ross.zwisler@linux.intel.com> |
|---|---|
| Date | 2015-10-02 23:10 +0200 |
| Subject | [PATCH 1/3] Revert "dax: fix NULL pointer in __dax_pmd_fault()" |
| Message-ID | <qffKx-6Av-1@gated-at.bofh.it> |
This reverts commit 8346c416d17bf5b4ea1508662959bb62e73fd6a5.
This commit did fix the issue it intended to fix, but it turns out that
the locking changes introduced by these two commits:
commit 843172978bb9 ("dax: fix race between simultaneous faults")
commit 46c043ede471 ("mm: take i_mmap_lock in unmap_mapping_range() for DAX")
had other issues as well, so they need to just be reverted.
The list of issues in DAX after these commits (some newly introduced by
the commits, some preexisting) can be found here:
https://lkml.org/lkml/2015/9/25/602
Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
---
fs/dax.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/fs/dax.c b/fs/dax.c
index bcfb14b..7ae6df7 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -569,20 +569,8 @@ int __dax_pmd_fault(struct vm_area_struct *vma, unsigned long address,
if (!buffer_size_valid(&bh) || bh.b_size < PMD_SIZE)
goto fallback;
- sector = bh.b_blocknr << (blkbits - 9);
-
if (buffer_unwritten(&bh) || buffer_new(&bh)) {
int i;
-
- length = bdev_direct_access(bh.b_bdev, sector, &kaddr, &pfn,
- bh.b_size);
- if (length < 0) {
- result = VM_FAULT_SIGBUS;
- goto out;
- }
- if ((length < PMD_SIZE) || (pfn & PG_PMD_COLOUR))
- goto fallback;
-
for (i = 0; i < PTRS_PER_PMD; i++)
clear_pmem(kaddr + i * PAGE_SIZE, PAGE_SIZE);
wmb_pmem();
@@ -635,6 +623,7 @@ int __dax_pmd_fault(struct vm_area_struct *vma, unsigned long address,
result = VM_FAULT_NOPAGE;
spin_unlock(ptl);
} else {
+ sector = bh.b_blocknr << (blkbits - 9);
length = bdev_direct_access(bh.b_bdev, sector, &kaddr, &pfn,
bh.b_size);
if (length < 0) {
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Dan Williams <dan.j.williams@intel.com> |
|---|---|
| Date | 2015-10-02 23:20 +0200 |
| Message-ID | <qffUe-6LS-11@gated-at.bofh.it> |
| In reply to | #1238615 |
On Fri, Oct 2, 2015 at 2:02 PM, Ross Zwisler
<ross.zwisler@linux.intel.com> wrote:
> This reverts commit 8346c416d17bf5b4ea1508662959bb62e73fd6a5.
>
> This commit did fix the issue it intended to fix, but it turns out that
> the locking changes introduced by these two commits:
>
> commit 843172978bb9 ("dax: fix race between simultaneous faults")
> commit 46c043ede471 ("mm: take i_mmap_lock in unmap_mapping_range() for DAX")
>
> had other issues as well, so they need to just be reverted.
Wait, why introduce two points in the kernel history where we have a
known uninitialized variable? I'd say fix up the revert of "mm: take
i_mmap_lock in unmap_mapping_range() for DAX" to address the conflict
with the fix, one less patch and keeps the stability rolling forward.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Ross Zwisler <ross.zwisler@linux.intel.com> |
|---|---|
| Date | 2015-10-03 01:30 +0200 |
| Message-ID | <qfhW2-1bm-5@gated-at.bofh.it> |
| In reply to | #1238623 |
On Fri, Oct 02, 2015 at 02:11:03PM -0700, Dan Williams wrote:
> On Fri, Oct 2, 2015 at 2:02 PM, Ross Zwisler
> <ross.zwisler@linux.intel.com> wrote:
> > This reverts commit 8346c416d17bf5b4ea1508662959bb62e73fd6a5.
> >
> > This commit did fix the issue it intended to fix, but it turns out that
> > the locking changes introduced by these two commits:
> >
> > commit 843172978bb9 ("dax: fix race between simultaneous faults")
> > commit 46c043ede471 ("mm: take i_mmap_lock in unmap_mapping_range() for DAX")
> >
> > had other issues as well, so they need to just be reverted.
>
> Wait, why introduce two points in the kernel history where we have a
> known uninitialized variable? I'd say fix up the revert of "mm: take
> i_mmap_lock in unmap_mapping_range() for DAX" to address the conflict
> with the fix, one less patch and keeps the stability rolling forward.
Essentially because I wasn't sure about the rules regarding reverts, if there
are any. I assumed (perhaps incorrectly) that you'd want a 1:1 relationship
between original commits and reverts. If it's better to not have intermediate
breakage, sure, let's squash them.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Jan Kara <jack@suse.cz> |
|---|---|
| Date | 2015-10-05 10:50 +0200 |
| Message-ID | <qg9D4-2es-21@gated-at.bofh.it> |
| In reply to | #1238702 |
On Fri 02-10-15 17:28:42, Ross Zwisler wrote:
> On Fri, Oct 02, 2015 at 02:11:03PM -0700, Dan Williams wrote:
> > On Fri, Oct 2, 2015 at 2:02 PM, Ross Zwisler
> > <ross.zwisler@linux.intel.com> wrote:
> > > This reverts commit 8346c416d17bf5b4ea1508662959bb62e73fd6a5.
> > >
> > > This commit did fix the issue it intended to fix, but it turns out that
> > > the locking changes introduced by these two commits:
> > >
> > > commit 843172978bb9 ("dax: fix race between simultaneous faults")
> > > commit 46c043ede471 ("mm: take i_mmap_lock in unmap_mapping_range() for DAX")
> > >
> > > had other issues as well, so they need to just be reverted.
> >
> > Wait, why introduce two points in the kernel history where we have a
> > known uninitialized variable? I'd say fix up the revert of "mm: take
> > i_mmap_lock in unmap_mapping_range() for DAX" to address the conflict
> > with the fix, one less patch and keeps the stability rolling forward.
>
> Essentially because I wasn't sure about the rules regarding reverts, if there
> are any. I assumed (perhaps incorrectly) that you'd want a 1:1 relationship
> between original commits and reverts. If it's better to not have intermediate
> breakage, sure, let's squash them.
Well, reverts aren't any special commits after all. So if it is simple
enough to just revert part of the patch that is broken, then just reverting
that part is fine.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web