Re: [PATCH 2/2] drivers/dax: Changing RC value
On Tue, Apr 11, 2017 at 1:58 PM, Andrew Mortonwrote: > On Tue, 11 Apr 2017 10:56:45 +0530 Pushkar Jambhlekar > wrote: > >> Changing rc value from VM_FAULT_FALLBACK to VM_FAULT_SIGBUS for an unknown / >> unsupported fault size. >> >> ... >> >> --- a/drivers/dax/dax.c >> +++ b/drivers/dax/dax.c >> @@ -590,7 +590,7 @@ static int dax_dev_huge_fault(struct vm_fault *vmf, >> rc = __dax_dev_pud_fault(dax_dev, vmf); >> break; >> default: >> - rc = VM_FAULT_FALLBACK; >> + rc = VM_FAULT_SIGBUS; >> } >> rcu_read_unlock(); > > The change seems to make sense but more info would be helpful. What > is wrong with the current code? ie, what goes wrong if we return > VM_FAULT_FALLBACK here? Hi Andrew, I took these two patches into my for-4.12/dax branch with the following fixed up changelog: commit 17756a4c681c320712798b783eb40006de9b4cc3 Author: Pushkar Jambhlekar Date: Tue Apr 11 09:12:25 2017 -0700 device-dax: fix dax_dev_huge_fault() unknown fault size handling The default case for dax_dev_huge_fault() fault size handling mistakenly returns when it should unlock. This is not a problem in practice since the only three possible fault sizes are handled. Going forward, if the core mm adds a new fault size beyond pte, pmd, or pud device-dax should abort VM_FAULT_SIGBUS requests not VM_FAULT_FALLBACK since device-dax guarantees a configured fault granularity for all faults. Signed-off-by: Pushkar Jambhlekar Signed-off-by: Dan Williams diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c index 19795eb35579..94036d92ed16 100644 --- a/drivers/dax/dax.c +++ b/drivers/dax/dax.c @@ -591,7 +591,7 @@ static int dax_dev_huge_fault(struct vm_fault *vmf, rc = __dax_dev_pud_fault(dax_dev, vmf); break; default: - return VM_FAULT_FALLBACK; + rc = VM_FAULT_SIGBUS; } srcu_read_unlock(_srcu, id);
Re: [PATCH 2/2] drivers/dax: Changing RC value
On Tue, Apr 11, 2017 at 1:58 PM, Andrew Morton wrote: > On Tue, 11 Apr 2017 10:56:45 +0530 Pushkar Jambhlekar > wrote: > >> Changing rc value from VM_FAULT_FALLBACK to VM_FAULT_SIGBUS for an unknown / >> unsupported fault size. >> >> ... >> >> --- a/drivers/dax/dax.c >> +++ b/drivers/dax/dax.c >> @@ -590,7 +590,7 @@ static int dax_dev_huge_fault(struct vm_fault *vmf, >> rc = __dax_dev_pud_fault(dax_dev, vmf); >> break; >> default: >> - rc = VM_FAULT_FALLBACK; >> + rc = VM_FAULT_SIGBUS; >> } >> rcu_read_unlock(); > > The change seems to make sense but more info would be helpful. What > is wrong with the current code? ie, what goes wrong if we return > VM_FAULT_FALLBACK here? Hi Andrew, I took these two patches into my for-4.12/dax branch with the following fixed up changelog: commit 17756a4c681c320712798b783eb40006de9b4cc3 Author: Pushkar Jambhlekar Date: Tue Apr 11 09:12:25 2017 -0700 device-dax: fix dax_dev_huge_fault() unknown fault size handling The default case for dax_dev_huge_fault() fault size handling mistakenly returns when it should unlock. This is not a problem in practice since the only three possible fault sizes are handled. Going forward, if the core mm adds a new fault size beyond pte, pmd, or pud device-dax should abort VM_FAULT_SIGBUS requests not VM_FAULT_FALLBACK since device-dax guarantees a configured fault granularity for all faults. Signed-off-by: Pushkar Jambhlekar Signed-off-by: Dan Williams diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c index 19795eb35579..94036d92ed16 100644 --- a/drivers/dax/dax.c +++ b/drivers/dax/dax.c @@ -591,7 +591,7 @@ static int dax_dev_huge_fault(struct vm_fault *vmf, rc = __dax_dev_pud_fault(dax_dev, vmf); break; default: - return VM_FAULT_FALLBACK; + rc = VM_FAULT_SIGBUS; } srcu_read_unlock(_srcu, id);
Re: [PATCH 2/2] drivers/dax: Changing RC value
On Tue, 11 Apr 2017 10:56:45 +0530 Pushkar Jambhlekarwrote: > Changing rc value from VM_FAULT_FALLBACK to VM_FAULT_SIGBUS for an unknown / > unsupported fault size. > > ... > > --- a/drivers/dax/dax.c > +++ b/drivers/dax/dax.c > @@ -590,7 +590,7 @@ static int dax_dev_huge_fault(struct vm_fault *vmf, > rc = __dax_dev_pud_fault(dax_dev, vmf); > break; > default: > - rc = VM_FAULT_FALLBACK; > + rc = VM_FAULT_SIGBUS; > } > rcu_read_unlock(); The change seems to make sense but more info would be helpful. What is wrong with the current code? ie, what goes wrong if we return VM_FAULT_FALLBACK here?
Re: [PATCH 2/2] drivers/dax: Changing RC value
On Tue, 11 Apr 2017 10:56:45 +0530 Pushkar Jambhlekar wrote: > Changing rc value from VM_FAULT_FALLBACK to VM_FAULT_SIGBUS for an unknown / > unsupported fault size. > > ... > > --- a/drivers/dax/dax.c > +++ b/drivers/dax/dax.c > @@ -590,7 +590,7 @@ static int dax_dev_huge_fault(struct vm_fault *vmf, > rc = __dax_dev_pud_fault(dax_dev, vmf); > break; > default: > - rc = VM_FAULT_FALLBACK; > + rc = VM_FAULT_SIGBUS; > } > rcu_read_unlock(); The change seems to make sense but more info would be helpful. What is wrong with the current code? ie, what goes wrong if we return VM_FAULT_FALLBACK here?
[PATCH 2/2] drivers/dax: Changing RC value
Changing rc value from VM_FAULT_FALLBACK to VM_FAULT_SIGBUS for an unknown / unsupported fault size. Signed-off-by: Pushkar Jambhlekar--- drivers/dax/dax.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c index fd9c4db..6156fdc 100644 --- a/drivers/dax/dax.c +++ b/drivers/dax/dax.c @@ -590,7 +590,7 @@ static int dax_dev_huge_fault(struct vm_fault *vmf, rc = __dax_dev_pud_fault(dax_dev, vmf); break; default: - rc = VM_FAULT_FALLBACK; + rc = VM_FAULT_SIGBUS; } rcu_read_unlock(); -- 2.7.4
[PATCH 2/2] drivers/dax: Changing RC value
Changing rc value from VM_FAULT_FALLBACK to VM_FAULT_SIGBUS for an unknown / unsupported fault size. Signed-off-by: Pushkar Jambhlekar --- drivers/dax/dax.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c index fd9c4db..6156fdc 100644 --- a/drivers/dax/dax.c +++ b/drivers/dax/dax.c @@ -590,7 +590,7 @@ static int dax_dev_huge_fault(struct vm_fault *vmf, rc = __dax_dev_pud_fault(dax_dev, vmf); break; default: - rc = VM_FAULT_FALLBACK; + rc = VM_FAULT_SIGBUS; } rcu_read_unlock(); -- 2.7.4
[PATCH 2/2] drivers/dax: Changing RC value
Changing rc value from VM_FAULT_FALLBACK to VM_FAULT_SIGBUS for an unknown / unsupported fault size. Signed-off-by: Pushkar Jambhlekar--- drivers/dax/dax.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c index fd9c4db..6156fdc 100644 --- a/drivers/dax/dax.c +++ b/drivers/dax/dax.c @@ -590,7 +590,7 @@ static int dax_dev_huge_fault(struct vm_fault *vmf, rc = __dax_dev_pud_fault(dax_dev, vmf); break; default: - rc = VM_FAULT_FALLBACK; + rc = VM_FAULT_SIGBUS; } rcu_read_unlock(); -- 2.7.4
[PATCH 2/2] drivers/dax: Changing RC value
Changing rc value from VM_FAULT_FALLBACK to VM_FAULT_SIGBUS for an unknown / unsupported fault size. Signed-off-by: Pushkar Jambhlekar --- drivers/dax/dax.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c index fd9c4db..6156fdc 100644 --- a/drivers/dax/dax.c +++ b/drivers/dax/dax.c @@ -590,7 +590,7 @@ static int dax_dev_huge_fault(struct vm_fault *vmf, rc = __dax_dev_pud_fault(dax_dev, vmf); break; default: - rc = VM_FAULT_FALLBACK; + rc = VM_FAULT_SIGBUS; } rcu_read_unlock(); -- 2.7.4