All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drivers/dax: Avoiding potential deadlock
@ 2017-04-11  5:26 Pushkar Jambhlekar
  2017-04-11  5:26 ` [PATCH 2/2] drivers/dax: Changing RC value Pushkar Jambhlekar
  2017-04-11 16:17 ` [PATCH 1/2] drivers/dax: Avoiding potential deadlock Dan Williams
  0 siblings, 2 replies; 7+ messages in thread
From: Pushkar Jambhlekar @ 2017-04-11  5:26 UTC (permalink / raw)
  To: Dan Williams, Dave Jiang, Andrew Morton, Johannes Thumshirn,
	Ross Zwisler
  Cc: linux-kernel, Pushkar Jambhlekar

dax_dev_huge_fault returning without releasing lock. Making code change to avoid this situation

Signed-off-by: Pushkar Jambhlekar <pushkar.iit@gmail.com>
---
 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 0d1ca24..fd9c4db 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:
-		return VM_FAULT_FALLBACK;
+		rc = VM_FAULT_FALLBACK;
 	}
 	rcu_read_unlock();
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] drivers/dax: Changing RC value
  2017-04-11  5:26 [PATCH 1/2] drivers/dax: Avoiding potential deadlock Pushkar Jambhlekar
@ 2017-04-11  5:26 ` Pushkar Jambhlekar
  2017-04-11 20:58   ` Andrew Morton
  2017-04-11 16:17 ` [PATCH 1/2] drivers/dax: Avoiding potential deadlock Dan Williams
  1 sibling, 1 reply; 7+ messages in thread
From: Pushkar Jambhlekar @ 2017-04-11  5:26 UTC (permalink / raw)
  To: Dan Williams, Dave Jiang, Andrew Morton, Johannes Thumshirn,
	Ross Zwisler
  Cc: linux-kernel, Pushkar Jambhlekar

Changing rc value from VM_FAULT_FALLBACK to VM_FAULT_SIGBUS for an unknown / unsupported fault size.

Signed-off-by: Pushkar Jambhlekar <pushkar.iit@gmail.com>
---
 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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] drivers/dax: Avoiding potential deadlock
  2017-04-11  5:26 [PATCH 1/2] drivers/dax: Avoiding potential deadlock Pushkar Jambhlekar
  2017-04-11  5:26 ` [PATCH 2/2] drivers/dax: Changing RC value Pushkar Jambhlekar
@ 2017-04-11 16:17 ` Dan Williams
  2017-04-12  4:19   ` Pushkar Jambhlekar
  1 sibling, 1 reply; 7+ messages in thread
From: Dan Williams @ 2017-04-11 16:17 UTC (permalink / raw)
  To: Pushkar Jambhlekar
  Cc: Dave Jiang, Andrew Morton, Johannes Thumshirn, Ross Zwisler,
	linux-kernel

On Mon, Apr 10, 2017 at 10:26 PM, Pushkar Jambhlekar
<pushkar.iit@gmail.com> wrote:
> dax_dev_huge_fault returning without releasing lock. Making code change to avoid this situation
>
> Signed-off-by: Pushkar Jambhlekar <pushkar.iit@gmail.com>
> ---
>  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 0d1ca24..fd9c4db 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:
> -               return VM_FAULT_FALLBACK;
> +               rc = VM_FAULT_FALLBACK;
>         }
>         rcu_read_unlock();

Thanks Pushkar, I'll apply this and combine with patch 2 as well.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] drivers/dax: Changing RC value
  2017-04-11  5:26 ` [PATCH 2/2] drivers/dax: Changing RC value Pushkar Jambhlekar
@ 2017-04-11 20:58   ` Andrew Morton
  2017-04-12 16:27     ` Dan Williams
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Morton @ 2017-04-11 20:58 UTC (permalink / raw)
  To: Pushkar Jambhlekar
  Cc: Dan Williams, Dave Jiang, Johannes Thumshirn, Ross Zwisler, linux-kernel

On Tue, 11 Apr 2017 10:56:45 +0530 Pushkar Jambhlekar <pushkar.iit@gmail.com> 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?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] drivers/dax: Avoiding potential deadlock
  2017-04-11 16:17 ` [PATCH 1/2] drivers/dax: Avoiding potential deadlock Dan Williams
@ 2017-04-12  4:19   ` Pushkar Jambhlekar
  0 siblings, 0 replies; 7+ messages in thread
From: Pushkar Jambhlekar @ 2017-04-12  4:19 UTC (permalink / raw)
  To: Dan Williams
  Cc: Dave Jiang, Andrew Morton, Johannes Thumshirn, Ross Zwisler,
	linux-kernel

Thanks Dan!

On Tue, Apr 11, 2017 at 9:47 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Mon, Apr 10, 2017 at 10:26 PM, Pushkar Jambhlekar
> <pushkar.iit@gmail.com> wrote:
>> dax_dev_huge_fault returning without releasing lock. Making code change to avoid this situation
>>
>> Signed-off-by: Pushkar Jambhlekar <pushkar.iit@gmail.com>
>> ---
>>  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 0d1ca24..fd9c4db 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:
>> -               return VM_FAULT_FALLBACK;
>> +               rc = VM_FAULT_FALLBACK;
>>         }
>>         rcu_read_unlock();
>
> Thanks Pushkar, I'll apply this and combine with patch 2 as well.



-- 
Jambhlekar Pushkar Arun

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] drivers/dax: Changing RC value
  2017-04-11 20:58   ` Andrew Morton
@ 2017-04-12 16:27     ` Dan Williams
  0 siblings, 0 replies; 7+ messages in thread
From: Dan Williams @ 2017-04-12 16:27 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Pushkar Jambhlekar, Dave Jiang, Johannes Thumshirn, Ross Zwisler,
	linux-kernel

On Tue, Apr 11, 2017 at 1:58 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Tue, 11 Apr 2017 10:56:45 +0530 Pushkar Jambhlekar <pushkar.iit@gmail.com> 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 <pushkar.iit@gmail.com>
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 <pushkar.iit@gmail.com>
    Signed-off-by: Dan Williams <dan.j.williams@intel.com>

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(&dax_srcu, id);

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] drivers/dax: Changing RC value
  2017-04-11  5:26 ` Pushkar Jambhlekar
@ 2017-04-11  5:26   ` Pushkar Jambhlekar
  0 siblings, 0 replies; 7+ messages in thread
From: Pushkar Jambhlekar @ 2017-04-11  5:26 UTC (permalink / raw)
  To: Dan Williams; +Cc: linux-kernel, Pushkar Jambhlekar

Changing rc value from VM_FAULT_FALLBACK to VM_FAULT_SIGBUS for an unknown / unsupported fault size.

Signed-off-by: Pushkar Jambhlekar <pushkar.iit@gmail.com>
---
 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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-04-12 16:27 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-11  5:26 [PATCH 1/2] drivers/dax: Avoiding potential deadlock Pushkar Jambhlekar
2017-04-11  5:26 ` [PATCH 2/2] drivers/dax: Changing RC value Pushkar Jambhlekar
2017-04-11 20:58   ` Andrew Morton
2017-04-12 16:27     ` Dan Williams
2017-04-11 16:17 ` [PATCH 1/2] drivers/dax: Avoiding potential deadlock Dan Williams
2017-04-12  4:19   ` Pushkar Jambhlekar
     [not found] <Dave Jiang <dave.jiang@intel.com>
2017-04-11  5:26 ` Pushkar Jambhlekar
2017-04-11  5:26   ` [PATCH 2/2] drivers/dax: Changing RC value Pushkar Jambhlekar

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.