* Re: [PATCH net] bonding: fix randomly populated arp target array
From: Jarod Wilson @ 2017-05-19 22:14 UTC (permalink / raw)
To: Mahesh Bandewar (महेश बंडेवार)
Cc: linux-kernel, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
linux-netdev, stable
In-Reply-To: <CAF2d9jiyCcqH=4Ls-GanJmLcMcjJzR=KEkqomFqVonrPv8QAPw@mail.gmail.com>
On 2017-05-19 4:38 PM, Mahesh Bandewar (महेश बंडेवार) wrote:
> On Fri, May 19, 2017 at 11:46 AM, Jarod Wilson <jarod@redhat.com> wrote:
>> In commit dc9c4d0fe023, the arp_target array moved from a static global
>> to a local variable. By the nature of static globals, the array used to
>> be initialized to all 0. At present, it's full of random data, which
>> that gets interpreted as arp_target values, when none have actually been
>> specified. Systems end up booting with spew along these lines:
>>
>> [ 32.161783] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
>> [ 32.168475] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
>> [ 32.175089] 8021q: adding VLAN 0 to HW filter on device lacp0
>> [ 32.193091] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
>> [ 32.204892] lacp0: Setting MII monitoring interval to 100
>> [ 32.211071] lacp0: Removing ARP target 216.124.228.17
>> [ 32.216824] lacp0: Removing ARP target 218.160.255.255
>> [ 32.222646] lacp0: Removing ARP target 185.170.136.184
>> [ 32.228496] lacp0: invalid ARP target 255.255.255.255 specified for removal
>> [ 32.236294] lacp0: option arp_ip_target: invalid value (-255.255.255.255)
>> [ 32.243987] lacp0: Removing ARP target 56.125.228.17
>> [ 32.249625] lacp0: Removing ARP target 218.160.255.255
>> [ 32.255432] lacp0: Removing ARP target 15.157.233.184
>> [ 32.261165] lacp0: invalid ARP target 255.255.255.255 specified for removal
>> [ 32.268939] lacp0: option arp_ip_target: invalid value (-255.255.255.255)
>> [ 32.276632] lacp0: Removing ARP target 16.0.0.0
>> [ 32.281755] lacp0: Removing ARP target 218.160.255.255
>> [ 32.287567] lacp0: Removing ARP target 72.125.228.17
>> [ 32.293165] lacp0: Removing ARP target 218.160.255.255
>> [ 32.298970] lacp0: Removing ARP target 8.125.228.17
>> [ 32.304458] lacp0: Removing ARP target 218.160.255.255
>>
>> None of these were actually specified as ARP targets, and the driver does
>> seem to clean up the mess okay, but it's rather noisy and confusing, leaks
>> values to userspace, and the 255.255.255.255 spew shows up even when debug
>> prints are disabled.
>>
>> The fix: just zero out arp_target at init time.
>>
>> While we're in here, init arp_all_targets_value in the right place.
>>
>> Fixes: dc9c4d0fe023 ("bonding: reduce scope of some global variables")
>> CC: Mahesh Bandewar <maheshb@google.com>
>> CC: Jay Vosburgh <j.vosburgh@gmail.com>
>> CC: Veaceslav Falico <vfalico@gmail.com>
>> CC: Andy Gospodarek <andy@greyhouse.net>
>> CC: netdev@vger.kernel.org
>> CC: stable@vger.kernel.org
>> Signed-off-by: Jarod Wilson <jarod@redhat.com>
>> ---
>> drivers/net/bonding/bond_main.c | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index 2be78807fd6e..73313318399c 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -4271,10 +4271,10 @@ static int bond_check_params(struct bond_params *params)
>> int arp_validate_value, fail_over_mac_value, primary_reselect_value, i;
>> struct bond_opt_value newval;
>> const struct bond_opt_value *valptr;
>> - int arp_all_targets_value;
>> + int arp_all_targets_value = 0;
>
> I think this is unnecessary as long as the var is initialized before it's use.
No, it's not part of the fix, it just irked me. The extra line is
completely unnecessary, less lines is good. Possibly shouldn't have
folded it into this fix, will respin and submit the cleanup another
time, if necessary.
>> u16 ad_actor_sys_prio = 0;
>> u16 ad_user_port_key = 0;
>> - __be32 arp_target[BOND_MAX_ARP_TARGETS];
>> + __be32 arp_target[BOND_MAX_ARP_TARGETS] = { 0 };
>
> this is the only change required to avoid reported error.
Absolutely correct.
--
Jarod Wilson
jarod@redhat.com
^ permalink raw reply
* Re: [PATCH net] bonding: fix randomly populated arp target array
From: Mahesh Bandewar (महेश बंडेवार) @ 2017-05-19 20:38 UTC (permalink / raw)
To: Jarod Wilson
Cc: linux-kernel, Jay Vosburgh, Veaceslav Falico, Andy Gospodarek,
linux-netdev, stable
In-Reply-To: <20170519184646.42572-1-jarod@redhat.com>
On Fri, May 19, 2017 at 11:46 AM, Jarod Wilson <jarod@redhat.com> wrote:
> In commit dc9c4d0fe023, the arp_target array moved from a static global
> to a local variable. By the nature of static globals, the array used to
> be initialized to all 0. At present, it's full of random data, which
> that gets interpreted as arp_target values, when none have actually been
> specified. Systems end up booting with spew along these lines:
>
> [ 32.161783] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
> [ 32.168475] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
> [ 32.175089] 8021q: adding VLAN 0 to HW filter on device lacp0
> [ 32.193091] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
> [ 32.204892] lacp0: Setting MII monitoring interval to 100
> [ 32.211071] lacp0: Removing ARP target 216.124.228.17
> [ 32.216824] lacp0: Removing ARP target 218.160.255.255
> [ 32.222646] lacp0: Removing ARP target 185.170.136.184
> [ 32.228496] lacp0: invalid ARP target 255.255.255.255 specified for removal
> [ 32.236294] lacp0: option arp_ip_target: invalid value (-255.255.255.255)
> [ 32.243987] lacp0: Removing ARP target 56.125.228.17
> [ 32.249625] lacp0: Removing ARP target 218.160.255.255
> [ 32.255432] lacp0: Removing ARP target 15.157.233.184
> [ 32.261165] lacp0: invalid ARP target 255.255.255.255 specified for removal
> [ 32.268939] lacp0: option arp_ip_target: invalid value (-255.255.255.255)
> [ 32.276632] lacp0: Removing ARP target 16.0.0.0
> [ 32.281755] lacp0: Removing ARP target 218.160.255.255
> [ 32.287567] lacp0: Removing ARP target 72.125.228.17
> [ 32.293165] lacp0: Removing ARP target 218.160.255.255
> [ 32.298970] lacp0: Removing ARP target 8.125.228.17
> [ 32.304458] lacp0: Removing ARP target 218.160.255.255
>
> None of these were actually specified as ARP targets, and the driver does
> seem to clean up the mess okay, but it's rather noisy and confusing, leaks
> values to userspace, and the 255.255.255.255 spew shows up even when debug
> prints are disabled.
>
> The fix: just zero out arp_target at init time.
>
> While we're in here, init arp_all_targets_value in the right place.
>
> Fixes: dc9c4d0fe023 ("bonding: reduce scope of some global variables")
> CC: Mahesh Bandewar <maheshb@google.com>
> CC: Jay Vosburgh <j.vosburgh@gmail.com>
> CC: Veaceslav Falico <vfalico@gmail.com>
> CC: Andy Gospodarek <andy@greyhouse.net>
> CC: netdev@vger.kernel.org
> CC: stable@vger.kernel.org
> Signed-off-by: Jarod Wilson <jarod@redhat.com>
> ---
> drivers/net/bonding/bond_main.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 2be78807fd6e..73313318399c 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -4271,10 +4271,10 @@ static int bond_check_params(struct bond_params *params)
> int arp_validate_value, fail_over_mac_value, primary_reselect_value, i;
> struct bond_opt_value newval;
> const struct bond_opt_value *valptr;
> - int arp_all_targets_value;
> + int arp_all_targets_value = 0;
I think this is unnecessary as long as the var is initialized before it's use.
> u16 ad_actor_sys_prio = 0;
> u16 ad_user_port_key = 0;
> - __be32 arp_target[BOND_MAX_ARP_TARGETS];
> + __be32 arp_target[BOND_MAX_ARP_TARGETS] = { 0 };
this is the only change required to avoid reported error.
> int arp_ip_count;
> int bond_mode = BOND_MODE_ROUNDROBIN;
> int xmit_hashtype = BOND_XMIT_POLICY_LAYER2;
> @@ -4501,7 +4501,6 @@ static int bond_check_params(struct bond_params *params)
> arp_validate_value = 0;
> }
>
> - arp_all_targets_value = 0;
> if (arp_all_targets) {
> bond_opt_initstr(&newval, arp_all_targets);
> valptr = bond_opt_parse(bond_opt_get(BOND_OPT_ARP_ALL_TARGETS),
> --
> 2.12.1
>
^ permalink raw reply
* Re: FAILED: patch "[PATCH] dax: fix PMD data corruption when fault races with write" failed to apply to 4.10-stable tree
From: Ross Zwisler @ 2017-05-19 19:54 UTC (permalink / raw)
To: gregkh; +Cc: ross.zwisler, akpm, dan.j.williams, jack, stable, torvalds
In-Reply-To: <149509239110625@kroah.com>
On Thu, May 18, 2017 at 09:26:31AM +0200, gregkh@linuxfoundation.org wrote:
>
> The patch below does not apply to the 4.10-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.
>
> thanks,
>
> greg k-h
The patch below is a backport of:
commit 876f29460cbd ("dax: fix PMD data corruption when fault races with write")
to v4.10.16. This was patch 5 in the series.
This patch still needs a backport of the following ext4 commit (which was
patch 3 in the series) from Jan to work correctly:
commit fb26a1cbed8c ("ext4: return to starting transaction in ext4_dax_huge_fault()")
--- >8 ---
>From eddc74225a0414af64ac59c09a44b9dae23b16b8 Mon Sep 17 00:00:00 2001
From: Ross Zwisler <ross.zwisler@linux.intel.com>
Date: Fri, 12 May 2017 15:47:00 -0700
Subject: [PATCH] dax: fix PMD data corruption when fault races with write
This is based on a patch from Jan Kara that fixed the equivalent race in
the DAX PTE fault path.
Currently DAX PMD read fault can race with write(2) in the following
way:
CPU1 - write(2) CPU2 - read fault
dax_iomap_pmd_fault()
->iomap_begin() - sees hole
dax_iomap_rw()
iomap_apply()
->iomap_begin - allocates blocks
dax_iomap_actor()
invalidate_inode_pages2_range()
- there's nothing to invalidate
grab_mapping_entry()
- we add huge zero page to the radix tree
and map it to page tables
The result is that hole page is mapped into page tables (and thus zeros
are seen in mmap) while file has data written in that place.
Fix the problem by locking exception entry before mapping blocks for the
fault. That way we are sure invalidate_inode_pages2_range() call for
racing write will either block on entry lock waiting for the fault to
finish (and unmap stale page tables after that) or read fault will see
already allocated blocks by write(2).
Fixes: 9f141d6ef6258 ("dax: Call ->iomap_begin without entry lock during dax fault")
Link: http://lkml.kernel.org/r/20170510172700.18991-1-ross.zwisler@linux.intel.com
Signed-off-by: Ross Zwisler <ross.zwisler@linux.intel.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
fs/dax.c | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/fs/dax.c b/fs/dax.c
index c83a0d8..c69ad4a 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -1343,6 +1343,16 @@ int dax_iomap_pmd_fault(struct vm_area_struct *vma, unsigned long address,
goto fallback;
/*
+ * grab_mapping_entry() will make sure we get a 2M empty entry, a DAX
+ * PMD or a HZP entry. If it can't (because a 4k page is already in
+ * the tree, for instance), it will return -EEXIST and we just fall
+ * back to 4k entries.
+ */
+ entry = grab_mapping_entry(mapping, pgoff, RADIX_DAX_PMD);
+ if (IS_ERR(entry))
+ goto fallback;
+
+ /*
* Note that we don't use iomap_apply here. We aren't doing I/O, only
* setting up a mapping, so really we're using iomap_begin() as a way
* to look up our filesystem block.
@@ -1350,20 +1360,11 @@ int dax_iomap_pmd_fault(struct vm_area_struct *vma, unsigned long address,
pos = (loff_t)pgoff << PAGE_SHIFT;
error = ops->iomap_begin(inode, pos, PMD_SIZE, iomap_flags, &iomap);
if (error)
- goto fallback;
+ goto unlock_entry;
if (iomap.offset + iomap.length < pos + PMD_SIZE)
goto finish_iomap;
- /*
- * grab_mapping_entry() will make sure we get a 2M empty entry, a DAX
- * PMD or a HZP entry. If it can't (because a 4k page is already in
- * the tree, for instance), it will return -EEXIST and we just fall
- * back to 4k entries.
- */
- entry = grab_mapping_entry(mapping, pgoff, RADIX_DAX_PMD);
- if (IS_ERR(entry))
- goto finish_iomap;
vmf.pgoff = pgoff;
vmf.flags = flags;
@@ -1377,7 +1378,7 @@ int dax_iomap_pmd_fault(struct vm_area_struct *vma, unsigned long address,
case IOMAP_UNWRITTEN:
case IOMAP_HOLE:
if (WARN_ON_ONCE(write))
- goto unlock_entry;
+ break;
result = dax_pmd_load_hole(vma, pmd, &vmf, address, &iomap,
&entry);
break;
@@ -1386,8 +1387,6 @@ int dax_iomap_pmd_fault(struct vm_area_struct *vma, unsigned long address,
break;
}
- unlock_entry:
- put_locked_mapping_entry(mapping, pgoff, entry);
finish_iomap:
if (ops->iomap_end) {
int copied = PMD_SIZE;
@@ -1403,6 +1402,8 @@ int dax_iomap_pmd_fault(struct vm_area_struct *vma, unsigned long address,
ops->iomap_end(inode, pos, PMD_SIZE, copied, iomap_flags,
&iomap);
}
+ unlock_entry:
+ put_locked_mapping_entry(mapping, pgoff, entry);
fallback:
if (result == VM_FAULT_FALLBACK) {
split_huge_pmd(vma, pmd, address);
--
2.9.4
^ permalink raw reply related
* Re: [PATCH net] bonding: fix randomly populated arp target array
From: Andy Gospodarek @ 2017-05-19 19:23 UTC (permalink / raw)
To: Jarod Wilson
Cc: linux-kernel, Mahesh Bandewar, Jay Vosburgh, Veaceslav Falico,
netdev, stable
In-Reply-To: <20170519184646.42572-1-jarod@redhat.com>
On Fri, May 19, 2017 at 02:46:46PM -0400, Jarod Wilson wrote:
> In commit dc9c4d0fe023, the arp_target array moved from a static global
> to a local variable. By the nature of static globals, the array used to
> be initialized to all 0. At present, it's full of random data, which
> that gets interpreted as arp_target values, when none have actually been
> specified. Systems end up booting with spew along these lines:
>
> [ 32.161783] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
> [ 32.168475] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
> [ 32.175089] 8021q: adding VLAN 0 to HW filter on device lacp0
> [ 32.193091] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
> [ 32.204892] lacp0: Setting MII monitoring interval to 100
> [ 32.211071] lacp0: Removing ARP target 216.124.228.17
> [ 32.216824] lacp0: Removing ARP target 218.160.255.255
> [ 32.222646] lacp0: Removing ARP target 185.170.136.184
> [ 32.228496] lacp0: invalid ARP target 255.255.255.255 specified for removal
> [ 32.236294] lacp0: option arp_ip_target: invalid value (-255.255.255.255)
> [ 32.243987] lacp0: Removing ARP target 56.125.228.17
> [ 32.249625] lacp0: Removing ARP target 218.160.255.255
> [ 32.255432] lacp0: Removing ARP target 15.157.233.184
> [ 32.261165] lacp0: invalid ARP target 255.255.255.255 specified for removal
> [ 32.268939] lacp0: option arp_ip_target: invalid value (-255.255.255.255)
> [ 32.276632] lacp0: Removing ARP target 16.0.0.0
> [ 32.281755] lacp0: Removing ARP target 218.160.255.255
> [ 32.287567] lacp0: Removing ARP target 72.125.228.17
> [ 32.293165] lacp0: Removing ARP target 218.160.255.255
> [ 32.298970] lacp0: Removing ARP target 8.125.228.17
> [ 32.304458] lacp0: Removing ARP target 218.160.255.255
>
> None of these were actually specified as ARP targets, and the driver does
> seem to clean up the mess okay, but it's rather noisy and confusing, leaks
> values to userspace, and the 255.255.255.255 spew shows up even when debug
> prints are disabled.
>
> The fix: just zero out arp_target at init time.
>
> While we're in here, init arp_all_targets_value in the right place.
>
Looks good. Thanks, Jarod!
Acked-by: Andy Gospodarek <andy@greyhouse.net>
> Fixes: dc9c4d0fe023 ("bonding: reduce scope of some global variables")
> CC: Mahesh Bandewar <maheshb@google.com>
> CC: Jay Vosburgh <j.vosburgh@gmail.com>
> CC: Veaceslav Falico <vfalico@gmail.com>
> CC: Andy Gospodarek <andy@greyhouse.net>
> CC: netdev@vger.kernel.org
> CC: stable@vger.kernel.org
> Signed-off-by: Jarod Wilson <jarod@redhat.com>
> ---
> drivers/net/bonding/bond_main.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 2be78807fd6e..73313318399c 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -4271,10 +4271,10 @@ static int bond_check_params(struct bond_params *params)
> int arp_validate_value, fail_over_mac_value, primary_reselect_value, i;
> struct bond_opt_value newval;
> const struct bond_opt_value *valptr;
> - int arp_all_targets_value;
> + int arp_all_targets_value = 0;
> u16 ad_actor_sys_prio = 0;
> u16 ad_user_port_key = 0;
> - __be32 arp_target[BOND_MAX_ARP_TARGETS];
> + __be32 arp_target[BOND_MAX_ARP_TARGETS] = { 0 };
> int arp_ip_count;
> int bond_mode = BOND_MODE_ROUNDROBIN;
> int xmit_hashtype = BOND_XMIT_POLICY_LAYER2;
> @@ -4501,7 +4501,6 @@ static int bond_check_params(struct bond_params *params)
> arp_validate_value = 0;
> }
>
> - arp_all_targets_value = 0;
> if (arp_all_targets) {
> bond_opt_initstr(&newval, arp_all_targets);
> valptr = bond_opt_parse(bond_opt_get(BOND_OPT_ARP_ALL_TARGETS),
> --
> 2.12.1
>
^ permalink raw reply
* [PATCH net] bonding: fix randomly populated arp target array
From: Jarod Wilson @ 2017-05-19 18:46 UTC (permalink / raw)
To: linux-kernel
Cc: Jarod Wilson, Mahesh Bandewar, Jay Vosburgh, Veaceslav Falico,
Andy Gospodarek, netdev, stable
In commit dc9c4d0fe023, the arp_target array moved from a static global
to a local variable. By the nature of static globals, the array used to
be initialized to all 0. At present, it's full of random data, which
that gets interpreted as arp_target values, when none have actually been
specified. Systems end up booting with spew along these lines:
[ 32.161783] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
[ 32.168475] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
[ 32.175089] 8021q: adding VLAN 0 to HW filter on device lacp0
[ 32.193091] IPv6: ADDRCONF(NETDEV_UP): lacp0: link is not ready
[ 32.204892] lacp0: Setting MII monitoring interval to 100
[ 32.211071] lacp0: Removing ARP target 216.124.228.17
[ 32.216824] lacp0: Removing ARP target 218.160.255.255
[ 32.222646] lacp0: Removing ARP target 185.170.136.184
[ 32.228496] lacp0: invalid ARP target 255.255.255.255 specified for removal
[ 32.236294] lacp0: option arp_ip_target: invalid value (-255.255.255.255)
[ 32.243987] lacp0: Removing ARP target 56.125.228.17
[ 32.249625] lacp0: Removing ARP target 218.160.255.255
[ 32.255432] lacp0: Removing ARP target 15.157.233.184
[ 32.261165] lacp0: invalid ARP target 255.255.255.255 specified for removal
[ 32.268939] lacp0: option arp_ip_target: invalid value (-255.255.255.255)
[ 32.276632] lacp0: Removing ARP target 16.0.0.0
[ 32.281755] lacp0: Removing ARP target 218.160.255.255
[ 32.287567] lacp0: Removing ARP target 72.125.228.17
[ 32.293165] lacp0: Removing ARP target 218.160.255.255
[ 32.298970] lacp0: Removing ARP target 8.125.228.17
[ 32.304458] lacp0: Removing ARP target 218.160.255.255
None of these were actually specified as ARP targets, and the driver does
seem to clean up the mess okay, but it's rather noisy and confusing, leaks
values to userspace, and the 255.255.255.255 spew shows up even when debug
prints are disabled.
The fix: just zero out arp_target at init time.
While we're in here, init arp_all_targets_value in the right place.
Fixes: dc9c4d0fe023 ("bonding: reduce scope of some global variables")
CC: Mahesh Bandewar <maheshb@google.com>
CC: Jay Vosburgh <j.vosburgh@gmail.com>
CC: Veaceslav Falico <vfalico@gmail.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: netdev@vger.kernel.org
CC: stable@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
drivers/net/bonding/bond_main.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 2be78807fd6e..73313318399c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4271,10 +4271,10 @@ static int bond_check_params(struct bond_params *params)
int arp_validate_value, fail_over_mac_value, primary_reselect_value, i;
struct bond_opt_value newval;
const struct bond_opt_value *valptr;
- int arp_all_targets_value;
+ int arp_all_targets_value = 0;
u16 ad_actor_sys_prio = 0;
u16 ad_user_port_key = 0;
- __be32 arp_target[BOND_MAX_ARP_TARGETS];
+ __be32 arp_target[BOND_MAX_ARP_TARGETS] = { 0 };
int arp_ip_count;
int bond_mode = BOND_MODE_ROUNDROBIN;
int xmit_hashtype = BOND_XMIT_POLICY_LAYER2;
@@ -4501,7 +4501,6 @@ static int bond_check_params(struct bond_params *params)
arp_validate_value = 0;
}
- arp_all_targets_value = 0;
if (arp_all_targets) {
bond_opt_initstr(&newval, arp_all_targets);
valptr = bond_opt_parse(bond_opt_get(BOND_OPT_ARP_ALL_TARGETS),
--
2.12.1
^ permalink raw reply related
* Re: [PATCH 1/3] btrfs: fix race with relocation recovery and fs_root setup
From: David Sterba @ 2017-05-19 18:10 UTC (permalink / raw)
To: Liu Bo; +Cc: jeffm, linux-btrfs, stable
In-Reply-To: <20170518235819.GD23822@lim.localdomain>
On Thu, May 18, 2017 at 04:58:19PM -0700, Liu Bo wrote:
> On Wed, May 17, 2017 at 11:38:34AM -0400, jeffm@suse.com wrote:
> > From: Jeff Mahoney <jeffm@suse.com>
> >
> > If we have to recover relocation during mount, we'll ultimately have to
> > evict the orphan inode. That goes through the reservation dance, where
> > priority_reclaim_metadata_space and flush_space expect fs_info->fs_root
> > to be valid. That's the next thing to be set up during mount, so we
> > crash, almost always in flush_space trying to join the transaction
> > but priority_reclaim_metadata_space is possible as well. This call
> > path has been problematic in the past WRT whether ->fs_root is valid
> > yet. Commit 957780eb278 (Btrfs: introduce ticketed enospc
> > infrastructure) added new users that are called in the direct path
> > instead of the async path that had already been worked around.
> >
> > The thing is that we don't actually need the fs_root, specifically, for
> > anything. We either use it to determine whether the root is the
> > chunk_root for use in choosing an allocation profile or as a root to pass
> > btrfs_join_transaction before immediately committing it. Anything that
> > isn't the chunk root works in the former case and any root works in
> > the latter.
> >
> > A simple fix is to use a root we know will always be there: the
> > extent_root.
> >
> Reviewed-by: Liu Bo <bo.li.liu@oracle.com>
2-3 added to 4.13 queue, 1 will go to 4.12. Thanks.
^ permalink raw reply
* Re: [PATCH 1/2] nvme: fix race between removing and reseting failure
From: yizhan @ 2017-05-19 16:55 UTC (permalink / raw)
To: Ming Lei, Jens Axboe
Cc: linux-block, Sagi Grimberg, linux-nvme, Keith Busch, stable,
Christoph Hellwig
In-Reply-To: <20170519164038.GB4870@ming.t460p>
On 05/20/2017 12:40 AM, Ming Lei wrote:
> On Fri, May 19, 2017 at 08:41:13AM -0600, Jens Axboe wrote:
>> On 05/16/2017 07:27 PM, Ming Lei wrote:
>>> When one NVMe PCI device is being resetted and found reset failue,
>>> nvme_remove_dead_ctrl() is called to handle the failure: blk-mq hw queues
>>> are put into stopped first, then schedule .remove_work to release the driver.
>>>
>>> Unfortunately if the driver is being released via sysfs store
>>> just before the .remove_work is run, del_gendisk() from
>>> nvme_remove() may hang forever because hw queues are stopped and
>>> the submitted writeback IOs from fsync_bdev() can't be completed at all.
>>>
>>> This patch fixes the following issue[1][2] by moving nvme_kill_queues()
>>> into nvme_remove_dead_ctrl() to avoid the issue because nvme_remove()
>>> flushs .reset_work, and this way is reasonable and safe because
>>> nvme_dev_disable() has started to suspend queues and canceled requests
>>> already.
>>>
>>> [1] test script
>>> fio -filename=$NVME_DISK -iodepth=1 -thread -rw=randwrite -ioengine=psync \
>>> -bssplit=5k/10:9k/10:13k/10:17k/10:21k/10:25k/10:29k/10:33k/10:37k/10:41k/10 \
>>> -bs_unaligned -runtime=1200 -size=-group_reporting -name=mytest -numjobs=60
>>>
>>> sleep 35
>>> echo 1 > $SYSFS_NVME_PCI_PATH/rescan
>>> echo 1 > $SYSFS_NVME_PCI_PATH/reset
>>> echo 1 > $SYSFS_NVME_PCI_PATH/remove
>>> echo 1 > /sys/bus/pci/rescan
>> The patch looks good to me. But since you have a nice reproducer, how about
>> turning that into a blktests [1] test case?
>>
>> [1] https://github.com/osandov/blktests
> Forget to mention, this test case is written by Zhang Yi.
>
> Zhang Yi, maybe you can try to integrate your NVMe test case into
> blktests if you are interested, :-)
Sure, I will have a try. :)
Thanks
Yi
> Thanks,
> Ming
>
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply
* [PATCH 1/2] ARM: dts: am335x-sl50: Fix card detect pin for mmc1
From: Enric Balletbo i Serra @ 2017-05-19 16:51 UTC (permalink / raw)
To: Tony Lindgren, stable, linux-omap; +Cc: linux-arm-kernel, linux-kernel
The second version of the hardware moved the card detect pin from gpio0_6
to gpio1_9, as we won't support the first hardware version fix the pinmux
configuration of this pin.
Fixes: 8584d4fc ("ARM: dts: am335x-sl50: Add Toby-Churchill SL50 board support.")
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
arch/arm/boot/dts/am335x-sl50.dts | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/am335x-sl50.dts b/arch/arm/boot/dts/am335x-sl50.dts
index c5d2589..da4f8e3 100644
--- a/arch/arm/boot/dts/am335x-sl50.dts
+++ b/arch/arm/boot/dts/am335x-sl50.dts
@@ -220,7 +220,7 @@
mmc1_pins: pinmux_mmc1_pins {
pinctrl-single,pins = <
- AM33XX_IOPAD(0x960, PIN_INPUT | MUX_MODE7) /* spi0_cs1.gpio0_6 */
+ AM33XX_IOPAD(0x96c, PIN_INPUT | MUX_MODE7) /* uart0_rtsn.gpio1_9 */
>;
};
@@ -384,7 +384,7 @@
pinctrl-names = "default";
pinctrl-0 = <&mmc1_pins>;
bus-width = <4>;
- cd-gpios = <&gpio0 6 GPIO_ACTIVE_LOW>;
+ cd-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;
vmmc-supply = <&vmmcsd_fixed>;
};
--
2.9.3
^ permalink raw reply related
* [PATCH 2/2] ARM: dts: am335x-sl50: Fix cannot claim requested pins for spi0
From: Enric Balletbo i Serra @ 2017-05-19 16:51 UTC (permalink / raw)
To: Tony Lindgren, stable, linux-omap; +Cc: linux-arm-kernel, linux-kernel
In-Reply-To: <20170519165144.11508-1-enric.balletbo@collabora.com>
We don't need to bitbang these pins anymore, instead we muxed these
pins as SPI, after this change, done in commit 6c69f726, we introduced
the following error:
pinctrl-single 44e10800.pinmux: pin PIN85 already requested \
by 44e10800.pinmux; cannot claim for 48030000.spi
pinctrl-single 44e10800.pinmux: pin-85 (48030000.spi) status -22
Fixes: 6c69f726 ("ARM: dts: am335x-sl50: Enable SPI0 interface and Flash Memory")
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
arch/arm/boot/dts/am335x-sl50.dts | 4 ----
1 file changed, 4 deletions(-)
diff --git a/arch/arm/boot/dts/am335x-sl50.dts b/arch/arm/boot/dts/am335x-sl50.dts
index da4f8e3..fc864a8 100644
--- a/arch/arm/boot/dts/am335x-sl50.dts
+++ b/arch/arm/boot/dts/am335x-sl50.dts
@@ -280,10 +280,6 @@
AM33XX_IOPAD(0x834, PIN_INPUT_PULLUP | MUX_MODE7) /* nKbdReset - gpmc_ad13.gpio1_13 */
AM33XX_IOPAD(0x838, PIN_INPUT_PULLUP | MUX_MODE7) /* nDispReset - gpmc_ad14.gpio1_14 */
AM33XX_IOPAD(0x844, PIN_INPUT_PULLUP | MUX_MODE7) /* USB1_enPower - gpmc_a1.gpio1_17 */
- /* AVR Programming - SPI Bus (bit bang) - Screen and Keyboard */
- AM33XX_IOPAD(0x954, PIN_INPUT_PULLUP | MUX_MODE7) /* Kbd/Disp/BattMOSI spi0_d0.gpio0_3 */
- AM33XX_IOPAD(0x958, PIN_INPUT_PULLUP | MUX_MODE7) /* Kbd/Disp/BattMISO spi0_d1.gpio0_4 */
- AM33XX_IOPAD(0x950, PIN_INPUT_PULLUP | MUX_MODE7) /* Kbd/Disp/BattSCLK spi0_clk.gpio0_2 */
/* PDI Bus - Battery system */
AM33XX_IOPAD(0x840, PIN_INPUT_PULLUP | MUX_MODE7) /* nBattReset gpmc_a0.gpio1_16 */
AM33XX_IOPAD(0x83c, PIN_INPUT_PULLUP | MUX_MODE7) /* BattPDIData gpmc_ad15.gpio1_15 */
--
2.9.3
^ permalink raw reply related
* Re: [PATCH 1/2] nvme: fix race between removing and reseting failure
From: Ming Lei @ 2017-05-19 16:40 UTC (permalink / raw)
To: Jens Axboe
Cc: Keith Busch, Christoph Hellwig, Sagi Grimberg, linux-block,
stable, linux-nvme, Zhang Yi
In-Reply-To: <d17e121f-799d-f555-8c7b-1d82ce61df53@kernel.dk>
On Fri, May 19, 2017 at 08:41:13AM -0600, Jens Axboe wrote:
> On 05/16/2017 07:27 PM, Ming Lei wrote:
> > When one NVMe PCI device is being resetted and found reset failue,
> > nvme_remove_dead_ctrl() is called to handle the failure: blk-mq hw queues
> > are put into stopped first, then schedule .remove_work to release the driver.
> >
> > Unfortunately if the driver is being released via sysfs store
> > just before the .remove_work is run, del_gendisk() from
> > nvme_remove() may hang forever because hw queues are stopped and
> > the submitted writeback IOs from fsync_bdev() can't be completed at all.
> >
> > This patch fixes the following issue[1][2] by moving nvme_kill_queues()
> > into nvme_remove_dead_ctrl() to avoid the issue because nvme_remove()
> > flushs .reset_work, and this way is reasonable and safe because
> > nvme_dev_disable() has started to suspend queues and canceled requests
> > already.
> >
> > [1] test script
> > fio -filename=$NVME_DISK -iodepth=1 -thread -rw=randwrite -ioengine=psync \
> > -bssplit=5k/10:9k/10:13k/10:17k/10:21k/10:25k/10:29k/10:33k/10:37k/10:41k/10 \
> > -bs_unaligned -runtime=1200 -size=-group_reporting -name=mytest -numjobs=60
> >
> > sleep 35
> > echo 1 > $SYSFS_NVME_PCI_PATH/rescan
> > echo 1 > $SYSFS_NVME_PCI_PATH/reset
> > echo 1 > $SYSFS_NVME_PCI_PATH/remove
> > echo 1 > /sys/bus/pci/rescan
>
> The patch looks good to me. But since you have a nice reproducer, how about
> turning that into a blktests [1] test case?
>
> [1] https://github.com/osandov/blktests
Forget to mention, this test case is written by Zhang Yi.
Zhang Yi, maybe you can try to integrate your NVMe test case into
blktests if you are interested, :-)
Thanks,
Ming
^ permalink raw reply
* [PATCH 2/3] f2fs: wake up all waiters in f2fs_submit_discard_endio
From: Chao Yu @ 2017-05-19 15:46 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu, stable
In-Reply-To: <20170519154645.20121-1-chao@kernel.org>
From: Chao Yu <yuchao0@huawei.com>
There could be more than one waiter waiting discard IO completion, so we
need use complete_all() instead of complete() in f2fs_submit_discard_endio
to avoid hungtask.
Fixes: ec9895add2c5 ("f2fs: don't hold cmd_lock during waiting discard
command")
Cc: <stable@vger.kernel.org>
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
fs/f2fs/segment.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index d310b82caef1..2b11119d9071 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -751,7 +751,7 @@ static void f2fs_submit_discard_endio(struct bio *bio)
dc->error = bio->bi_error;
dc->state = D_DONE;
- complete(&dc->wait);
+ complete_all(&dc->wait);
bio_put(bio);
}
--
2.12.2.575.gb14f27f
^ permalink raw reply related
* Re: [PATCH 1/2] nvme: fix race between removing and reseting failure
From: Ming Lei @ 2017-05-19 15:10 UTC (permalink / raw)
To: Jens Axboe
Cc: Keith Busch, Christoph Hellwig, Sagi Grimberg, linux-block,
stable, linux-nvme, Zhang Yi
In-Reply-To: <d17e121f-799d-f555-8c7b-1d82ce61df53@kernel.dk>
On Fri, May 19, 2017 at 08:41:13AM -0600, Jens Axboe wrote:
> On 05/16/2017 07:27 PM, Ming Lei wrote:
> > When one NVMe PCI device is being resetted and found reset failue,
> > nvme_remove_dead_ctrl() is called to handle the failure: blk-mq hw queues
> > are put into stopped first, then schedule .remove_work to release the driver.
> >
> > Unfortunately if the driver is being released via sysfs store
> > just before the .remove_work is run, del_gendisk() from
> > nvme_remove() may hang forever because hw queues are stopped and
> > the submitted writeback IOs from fsync_bdev() can't be completed at all.
> >
> > This patch fixes the following issue[1][2] by moving nvme_kill_queues()
> > into nvme_remove_dead_ctrl() to avoid the issue because nvme_remove()
> > flushs .reset_work, and this way is reasonable and safe because
> > nvme_dev_disable() has started to suspend queues and canceled requests
> > already.
> >
> > [1] test script
> > fio -filename=$NVME_DISK -iodepth=1 -thread -rw=randwrite -ioengine=psync \
> > -bssplit=5k/10:9k/10:13k/10:17k/10:21k/10:25k/10:29k/10:33k/10:37k/10:41k/10 \
> > -bs_unaligned -runtime=1200 -size=-group_reporting -name=mytest -numjobs=60
> >
> > sleep 35
> > echo 1 > $SYSFS_NVME_PCI_PATH/rescan
> > echo 1 > $SYSFS_NVME_PCI_PATH/reset
> > echo 1 > $SYSFS_NVME_PCI_PATH/remove
> > echo 1 > /sys/bus/pci/rescan
>
> The patch looks good to me. But since you have a nice reproducer, how about
> turning that into a blktests [1] test case?
This test has triggered several problems recently, and looks a good idea to
integrate into blktests.
I am a little busy recently, and may take a while to start that. If
anyone would like to integrate the case, please go ahead, and I am happy
to provide any details you need.
Thanks,
Ming
^ permalink raw reply
* Re: [PATCH 1/2] nvme: fix race between removing and reseting failure
From: Keith Busch @ 2017-05-19 15:15 UTC (permalink / raw)
To: Ming Lei
Cc: Jens Axboe, Christoph Hellwig, Sagi Grimberg, linux-nvme,
Zhang Yi, linux-block, stable
In-Reply-To: <20170519125244.GB22585@ming.t460p>
On Fri, May 19, 2017 at 08:52:45PM +0800, Ming Lei wrote:
> But I still think it may be better to move nvme_kill_queues() into
> nvme_remove_dead_ctrl() as an improvement because during this small
> window page cache can be used up by write application, and no writeback
> can move on meantime.
Yes, I agree that's a better placement for it. I was just concerned
about the reasoning since it would also mean we're still stuck if an
IO timeout occurs while calling del_gendisk. So I'm okay with the patch
as-is, but I'll also look into my other concern.
^ permalink raw reply
* Re: [PATCH 1/2] nvme: fix race between removing and reseting failure
From: Jens Axboe @ 2017-05-19 14:41 UTC (permalink / raw)
To: Ming Lei, Keith Busch, Christoph Hellwig, Sagi Grimberg
Cc: linux-nvme, Zhang Yi, linux-block, stable
In-Reply-To: <20170517012729.13469-2-ming.lei@redhat.com>
On 05/16/2017 07:27 PM, Ming Lei wrote:
> When one NVMe PCI device is being resetted and found reset failue,
> nvme_remove_dead_ctrl() is called to handle the failure: blk-mq hw queues
> are put into stopped first, then schedule .remove_work to release the driver.
>
> Unfortunately if the driver is being released via sysfs store
> just before the .remove_work is run, del_gendisk() from
> nvme_remove() may hang forever because hw queues are stopped and
> the submitted writeback IOs from fsync_bdev() can't be completed at all.
>
> This patch fixes the following issue[1][2] by moving nvme_kill_queues()
> into nvme_remove_dead_ctrl() to avoid the issue because nvme_remove()
> flushs .reset_work, and this way is reasonable and safe because
> nvme_dev_disable() has started to suspend queues and canceled requests
> already.
>
> [1] test script
> fio -filename=$NVME_DISK -iodepth=1 -thread -rw=randwrite -ioengine=psync \
> -bssplit=5k/10:9k/10:13k/10:17k/10:21k/10:25k/10:29k/10:33k/10:37k/10:41k/10 \
> -bs_unaligned -runtime=1200 -size=-group_reporting -name=mytest -numjobs=60
>
> sleep 35
> echo 1 > $SYSFS_NVME_PCI_PATH/rescan
> echo 1 > $SYSFS_NVME_PCI_PATH/reset
> echo 1 > $SYSFS_NVME_PCI_PATH/remove
> echo 1 > /sys/bus/pci/rescan
The patch looks good to me. But since you have a nice reproducer, how about
turning that into a blktests [1] test case?
[1] https://github.com/osandov/blktests
--
Jens Axboe
^ permalink raw reply
* RE: [PATCH 6/8] xhci: remove GFP_DMA flag from allocation
From: David Laight @ 2017-05-19 13:54 UTC (permalink / raw)
To: 'Mathias Nyman', gregkh@linuxfoundation.org
Cc: linux-usb@vger.kernel.org, Matthias Lange, stable@vger.kernel.org
In-Reply-To: <591EBF97.7010200@linux.intel.com>
From: Mathias Nyman
> Sent: 19 May 2017 10:49
> To: David Laight; gregkh@linuxfoundation.org
> Cc: linux-usb@vger.kernel.org; Matthias Lange; stable@vger.kernel.org
> Subject: Re: [PATCH 6/8] xhci: remove GFP_DMA flag from allocation
>
> On 19.05.2017 12:10, David Laight wrote:
> > From: Mathias Nyman
> >> Sent: 17 May 2017 16:32
> >> There is no reason to restrict allocations to the first 16MB ISA DMA
> >> addresses.
> >>
> >> It is causing problems in a virtualization setup with enabled IOMMU
> >> (x86_64). The result is that USB is not working in the VM.
> > ...
> >> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> >> index 12b573c..1f1687e 100644
> >> --- a/drivers/usb/host/xhci-mem.c
> >> +++ b/drivers/usb/host/xhci-mem.c
> >> @@ -56,7 +56,7 @@ static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci,
> >> }
> >>
> >> if (max_packet) {
> >> - seg->bounce_buf = kzalloc(max_packet, flags | GFP_DMA);
> >> + seg->bounce_buf = kzalloc(max_packet, flags);
> >
> > This might allocate memory that the device cannot access.
> > So can only work if dma_map_single() itself allocates a bounce buffer.
> > There must be a sane way to do this that doesn't ever require
> > double copies.
> >
>
> We are using dma_map_single()
> This allocated memory is used as the processor virtual memory required by dma_map_single()
> i.e. the "void *cpu_addr" part of dma_map_single, see DMA-API.txt:
>
>
> dma_map_single(struct device *dev, void *cpu_addr, size_t size,
> enum dma_data_direction direction)
>
> Maps a piece of processor virtual memory so it can be accessed by the
> device and returns the DMA address of the memory.
>
> ...
>
> Notes: Not all memory regions in a machine can be mapped by this API.
> Further, contiguous kernel virtual space may not be contiguous as
> physical memory. Since this API does not provide any scatter/gather
> capability, it will fail if the user tries to map a non-physically
> contiguous piece of memory. For this reason, memory to be mapped by
> this API should be obtained from sources which guarantee it to be
> physically contiguous (like kmalloc).
>
> I'm not fully sure I understand yout concern, are you thinking the driver should
> doublecheck the dma address returned by dma_map_single() and make sure it's within the
> dma mask set for the device?
The physical memory returned by kzalloc() (without GFP_DMA) can definitely be
outside the dma mask for the device.
If that happens something must allocate a dma bounce buffer, I'm guessing that
dma_map_single() does so - it certainly can since it can copy the data during
the unmap (for a read transfer).
So you really want to allocate a buffer that is within the devices dma window.
The buffer you are allocating isn't really a 'bounce' buffer at all - is it?
It is more of a de_fragmentation buffer.
It is there so that the LINK TRB will always be at a USB segment boundary.
(I'm guessing the bug I found several years ago still isn't fixed??)
David
^ permalink raw reply
* [PATCH 2/2] nohz: Fix collision between tick and other hrtimers, again
From: Frederic Weisbecker @ 2017-05-19 13:51 UTC (permalink / raw)
To: Ingo Molnar
Cc: LKML, Frederic Weisbecker, Peter Zijlstra, Rik van Riel,
James Hartsock, Thomas Gleixner, stable, Tim Wright, Pavel Machek
In-Reply-To: <1495201910-12466-1-git-send-email-fweisbec@gmail.com>
This restores commit:
24b91e360ef5: ("nohz: Fix collision between tick and other hrtimers")
... which got reverted by commit:
558e8e27e73f: ('Revert "nohz: Fix collision between tick and other hrtimers"')
... due to a regression where CPUs spuriously stopped ticking.
The bug happened when a tick fired too early past its expected expiration:
on IRQ exit the tick was scheduled again to the same deadline but skipped
reprogramming because ts->next_tick still kept in cache the deadline.
This has been fixed now with resetting ts->next_tick from the tick
itself. Extra care has also been taken to prevent from obsolete values
throughout CPU hotplug operations. Also in order to honour further
paranoia, this version of the patch takes care about tick interrupts
whose regs are NULL. It happens when hrtimer_interrupt() is called from
non-interrupt contexts (ex: hotplug cpu down).
When the tick is stopped and an interrupt occurs afterward, we check on
that interrupt exit if the next tick needs to be rescheduled. If it
doesn't need any update, we don't want to do anything.
In order to check if the tick needs an update, we compare it against the
clockevent device deadline. Now that's a problem because the clockevent
device is at a lower level than the tick itself if it is implemented
on top of hrtimer.
Every hrtimer share this clockevent device. So comparing the next tick
deadline against the clockevent device deadline is wrong because the
device may be programmed for another hrtimer whose deadline collides
with the tick. As a result we may end up not reprogramming the tick
accidentally.
In a worst case scenario under full dynticks mode, the tick stops firing
as it is supposed to every 1hz, leaving /proc/stat stalled:
Task in a full dynticks CPU
----------------------------
* hrtimer A is queued 2 seconds ahead
* the tick is stopped, scheduled 1 second ahead
* tick fires 1 second later
* on tick exit, nohz schedules the tick 1 second ahead but sees
the clockevent device is already programmed to that deadline,
fooled by hrtimer A, the tick isn't rescheduled.
* hrtimer A is cancelled before its deadline
* tick never fires again until an interrupt happens...
In order to fix this, store the next tick deadline to the tick_sched
local structure and reuse that value later to check whether we need to
reprogram the clock after an interrupt.
On the other hand, ts->sleep_length still wants to know about the next
clock event and not just the tick, so we want to improve the related
comment to avoid confusion.
Reported-and-tested-by: Tim Wright <tim@binbash.co.uk>
Reported-and-tested-by: Pavel Machek <pavel@ucw.cz>
Reported-by: James Hartsock <hartsjc@redhat.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Rik van Riel <riel@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: stable@vger.kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/time/tick-sched.c | 49 ++++++++++++++++++++++++++++++++++++++++--------
kernel/time/tick-sched.h | 2 ++
2 files changed, 43 insertions(+), 8 deletions(-)
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index d212bb6..30253ed 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -660,6 +660,12 @@ static void tick_nohz_restart(struct tick_sched *ts, ktime_t now)
hrtimer_start_expires(&ts->sched_timer, HRTIMER_MODE_ABS_PINNED);
else
tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1);
+
+ /*
+ * Reset to make sure next tick stop doesn't get fooled by past
+ * cached clock deadline.
+ */
+ ts->next_tick = 0;
}
static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
@@ -771,12 +777,15 @@ static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
tick = expires;
/* Skip reprogram of event if its not changed */
- if (ts->tick_stopped) {
- if (hrtimer_active(&ts->sched_timer))
- WARN_ON_ONCE(hrtimer_get_expires(&ts->sched_timer) < dev->next_event);
-
- if (expires == dev->next_event)
+ if (ts->tick_stopped && (expires == ts->next_tick)) {
+ /* Sanity check: make sure clockevent is actually programmed */
+ if (likely(dev->next_event <= ts->next_tick))
goto out;
+
+ WARN_ON_ONCE(1);
+ printk_once("basemono: %llu ts->next_tick: %llu dev->next_event: %llu timer->active: %d timer->expires: %llu\n",
+ basemono, ts->next_tick, dev->next_event,
+ hrtimer_active(&ts->sched_timer), hrtimer_get_expires(&ts->sched_timer));
}
/*
@@ -796,6 +805,8 @@ static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
trace_tick_stop(1, TICK_DEP_MASK_NONE);
}
+ ts->next_tick = tick;
+
/*
* If the expiration time == KTIME_MAX, then we simply stop
* the tick timer.
@@ -811,7 +822,10 @@ static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
else
tick_program_event(tick, 1);
out:
- /* Update the estimated sleep length */
+ /*
+ * Update the estimated sleep length until the next timer
+ * (not only the tick).
+ */
ts->sleep_length = ktime_sub(dev->next_event, now);
return tick;
}
@@ -869,6 +883,11 @@ static bool can_stop_idle_tick(int cpu, struct tick_sched *ts)
if (unlikely(!cpu_online(cpu))) {
if (cpu == tick_do_timer_cpu)
tick_do_timer_cpu = TICK_DO_TIMER_NONE;
+ /*
+ * Make sure the CPU doesn't get fooled by obsolete tick
+ * deadline if it comes back online later.
+ */
+ ts->next_tick = 0;
return false;
}
@@ -1078,8 +1097,15 @@ static void tick_nohz_handler(struct clock_event_device *dev)
tick_sched_handle(ts, regs);
/* No need to reprogram if we are running tickless */
- if (unlikely(ts->tick_stopped))
+ if (unlikely(ts->tick_stopped)) {
+ /*
+ * In case the current tick fired too early past its expected
+ * expiration, make sure we don't bypass the next clock reprogramming
+ * to the same deadline.
+ */
+ ts->next_tick = 0;
return;
+ }
hrtimer_forward(&ts->sched_timer, now, tick_period);
tick_program_event(hrtimer_get_expires(&ts->sched_timer), 1);
@@ -1179,8 +1205,15 @@ static enum hrtimer_restart tick_sched_timer(struct hrtimer *timer)
tick_sched_handle(ts, regs);
/* No need to reprogram if we are in idle or full dynticks mode */
- if (unlikely(ts->tick_stopped))
+ if (unlikely(ts->tick_stopped)) {
+ /*
+ * In case the current tick fired too early past its expected
+ * expiration, make sure we don't bypass the next clock reprogramming
+ * to the same deadline.
+ */
+ ts->next_tick = 0;
return HRTIMER_NORESTART;
+ }
hrtimer_forward(timer, now, tick_period);
diff --git a/kernel/time/tick-sched.h b/kernel/time/tick-sched.h
index bf38226..075444e 100644
--- a/kernel/time/tick-sched.h
+++ b/kernel/time/tick-sched.h
@@ -27,6 +27,7 @@ enum tick_nohz_mode {
* timer is modified for nohz sleeps. This is necessary
* to resume the tick timer operation in the timeline
* when the CPU returns from nohz sleep.
+ * @next_tick: Next tick to be fired when in dynticks mode.
* @tick_stopped: Indicator that the idle tick has been stopped
* @idle_jiffies: jiffies at the entry to idle for idle time accounting
* @idle_calls: Total number of idle calls
@@ -44,6 +45,7 @@ struct tick_sched {
unsigned long check_clocks;
enum tick_nohz_mode nohz_mode;
ktime_t last_tick;
+ ktime_t next_tick;
int inidle;
int tick_stopped;
unsigned long idle_jiffies;
--
2.7.4
^ permalink raw reply related
* [PATCH 1/2] nohz: Add hrtimer sanity check
From: Frederic Weisbecker @ 2017-05-19 13:51 UTC (permalink / raw)
To: Ingo Molnar
Cc: LKML, Frederic Weisbecker, Peter Zijlstra, Rik van Riel,
James Hartsock, Thomas Gleixner, stable, Tim Wright, Pavel Machek
In-Reply-To: <1495201910-12466-1-git-send-email-fweisbec@gmail.com>
Make sure that the clockevent device is never programmed to a deadline
later than the tick.
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Tim Wright <tim@binbash.co.uk>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: James Hartsock <hartsjc@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Rik van Riel <riel@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/time/tick-sched.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 64c97fc..d212bb6 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -771,8 +771,13 @@ static ktime_t tick_nohz_stop_sched_tick(struct tick_sched *ts,
tick = expires;
/* Skip reprogram of event if its not changed */
- if (ts->tick_stopped && (expires == dev->next_event))
- goto out;
+ if (ts->tick_stopped) {
+ if (hrtimer_active(&ts->sched_timer))
+ WARN_ON_ONCE(hrtimer_get_expires(&ts->sched_timer) < dev->next_event);
+
+ if (expires == dev->next_event)
+ goto out;
+ }
/*
* nohz_stop_sched_tick can be called several times before
--
2.7.4
^ permalink raw reply related
* [PATCH 0/2] nohz: Deal with clock reprogram skipping issues v3
From: Frederic Weisbecker @ 2017-05-19 13:51 UTC (permalink / raw)
To: Ingo Molnar
Cc: LKML, Frederic Weisbecker, Peter Zijlstra, Rik van Riel,
James Hartsock, Thomas Gleixner, stable, Tim Wright, Pavel Machek
v2 had issues on -tip tree and triggered a warning. It seems to have
disappeared. Perhaps it was due to another timer issue. Anyway this
version brings more debugging informations, with a layout that is more
bisection-friendly and it also handles ticks that fire outside IRQ
context and thus carry NULL irq regs. This happen when
hrtimer_interrupt() is called on hotplug cpu down for example.
We'll see if the issue arises again.
git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
nohz/fixes
HEAD: cd15f46b284f04dbedd065a9d99a4e0badae379a
Thanks,
Frederic
---
Frederic Weisbecker (2):
nohz: Add hrtimer sanity check
nohz: Fix collision between tick and other hrtimers, again
kernel/time/tick-sched.c | 48 +++++++++++++++++++++++++++++++++++++++++++-----
kernel/time/tick-sched.h | 2 ++
2 files changed, 45 insertions(+), 5 deletions(-)
^ permalink raw reply
* Re: [PATCH 2/2] nvme: avoid to hang in remove disk
From: Ming Lei @ 2017-05-19 13:19 UTC (permalink / raw)
To: Keith Busch
Cc: Christoph Hellwig, Jens Axboe, Sagi Grimberg, linux-nvme,
Zhang Yi, linux-block, stable
In-Reply-To: <20170518160624.GA6015@localhost.localdomain>
On Thu, May 18, 2017 at 12:06:24PM -0400, Keith Busch wrote:
> On Thu, May 18, 2017 at 11:35:43PM +0800, Ming Lei wrote:
> > On Thu, May 18, 2017 at 03:49:31PM +0200, Christoph Hellwig wrote:
> > > On Wed, May 17, 2017 at 09:27:29AM +0800, Ming Lei wrote:
> > > > If some writeback requests are submitted just before queue is killed,
> > > > and these requests may not be canceled in nvme_dev_disable() because
> > > > they are not started yet, it is still possible for blk-mq to hold
> > > > these requests in .requeue list.
> > > >
> > > > So we have to abort these requests first before del_gendisk(), because
> > > > del_gendisk() may wait for completion of these requests.
> > > >
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Ming Lei <ming.lei@redhat.com>
> > > > ---
> > > > drivers/nvme/host/core.c | 8 ++++++++
> > > > 1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> > > > index d5e0906262ea..8eaeea86509a 100644
> > > > --- a/drivers/nvme/host/core.c
> > > > +++ b/drivers/nvme/host/core.c
> > > > @@ -2097,6 +2097,14 @@ static void nvme_ns_remove(struct nvme_ns *ns)
> > > > &nvme_ns_attr_group);
> > > > if (ns->ndev)
> > > > nvme_nvm_unregister_sysfs(ns);
> > > > + /*
> > > > + * If queue is dead, we have to abort requests in
> > > > + * requeue list because fsync_bdev() in removing disk
> > > > + * path may wait for these IOs, which can't
> > > > + * be submitted to hardware too.
> > > > + */
> > > > + if (blk_queue_dying(ns->queue))
> > > > + blk_mq_abort_requeue_list(ns->queue);
> > > > del_gendisk(ns->disk);
> > > > blk_mq_abort_requeue_list(ns->queue);
> > >
> > > Why can't we just move the blk_mq_abort_requeue_list call before
> > > del_gendisk in general?
> >
> > That may cause data loss if queue isn't killed. Normally queue is only killed
> > when the controller is dead(such as in reset failure) or !pci_device_is_present()
> > (in nvme_remove()).
>
> But in your test, your controller isn't even dead. Why are we killing
> it when it's still functional? I think we need to first not consider
Last time, you posted one patch which looks fixes the hang, but the
NVMe device becomes not functional after rebinding later, see
the link:
http://marc.info/?l=linux-block&m=149430047402103&w=2
Also not sure it is worthy of doing that, because in my case,
nvme_dev_disable() has been called in nvme_reset_notify() before the reset,
that means requests have been canceled already. As I mentioned, the two
directions aren't contradictorily. But thinking controller as live under
this situation need big change, such as, not canceling requests in whole
reset path.
Finally this patch is not only for the case of reset failure vs. remove,
but also for hot-unplug, queues are still killed in path of hot remove.
> this perfectly functional controller to be dead under these conditions,
> and second, understand why killing the queues after del_gendisk is called
> does not allow forward progress.
I have explained this cause in last email, :-)
Thanks,
Ming
^ permalink raw reply
* Re: [PATCH v3] KVM: x86: Fix potential preemption when get the current kvmclock timestamp
From: Radim Krčmář @ 2017-05-19 13:13 UTC (permalink / raw)
To: Wanpeng Li
Cc: Paolo Bonzini, linux-kernel@vger.kernel.org, kvm, Wanpeng Li,
stable
In-Reply-To: <CANRm+CzLBNNrhcb1ssQgZze30sEPG56HPXZBrirnODsms2GjUg@mail.gmail.com>
2017-05-19 05:53+0800, Wanpeng Li:
> Ping,
Applying for 4.12-rc2, thanks.
^ permalink raw reply
* Re: [PATCH 1/2] nvme: fix race between removing and reseting failure
From: Ming Lei @ 2017-05-19 12:52 UTC (permalink / raw)
To: Keith Busch
Cc: Jens Axboe, Christoph Hellwig, Sagi Grimberg, linux-nvme,
Zhang Yi, linux-block, stable
In-Reply-To: <20170518141307.GD28520@localhost.localdomain>
On Thu, May 18, 2017 at 10:13:07AM -0400, Keith Busch wrote:
> On Wed, May 17, 2017 at 09:27:28AM +0800, Ming Lei wrote:
> > When one NVMe PCI device is being resetted and found reset failue,
> > nvme_remove_dead_ctrl() is called to handle the failure: blk-mq hw queues
> > are put into stopped first, then schedule .remove_work to release the driver.
> >
> > Unfortunately if the driver is being released via sysfs store
> > just before the .remove_work is run, del_gendisk() from
> > nvme_remove() may hang forever because hw queues are stopped and
> > the submitted writeback IOs from fsync_bdev() can't be completed at all.
> >
> > This patch fixes the following issue[1][2] by moving nvme_kill_queues()
> > into nvme_remove_dead_ctrl() to avoid the issue because nvme_remove()
> > flushs .reset_work, and this way is reasonable and safe because
> > nvme_dev_disable() has started to suspend queues and canceled requests
> > already.
>
> I'm still not sure moving where we kill the queues is the correct way
> to fix this problem. The nvme_kill_queues restarts all the hardware
> queues to force all IO to failure already, so why is this really stuck?
That is a good question.
Today I investigate further, and figured out that it is because that
blk_mq_start_stopped_hw_queues() in nvme_kill_queues() does not
run hw queues actually becasue the queues are started in
nvme_reset_work() already. Will figure out a patch later to fix the
issue.
And the reason why this patch 'fixes' the issue is that just timing,
I guess.
> We should be able to make forward progress even if we kill the queues
> while calling into del_gendisk, right? That could happen with a different
> sequence of events, so that also needs to work.
Now I am not a big fun of this patch for fixing the issue.
But I still think it may be better to move nvme_kill_queues() into
nvme_remove_dead_ctrl() as an improvement because during this small
window page cache can be used up by write application, and no writeback
can move on meantime.
Thanks,
Ming
^ permalink raw reply
* Re: [Intel-gfx] [PATCH 3/3] drm/i915: Fix 90/270 rotated coordinates for FBC
From: Tvrtko Ursulin @ 2017-05-19 11:34 UTC (permalink / raw)
To: ville.syrjala, intel-gfx; +Cc: Paulo Zanoni, stable
In-Reply-To: <20170331180056.14086-4-ville.syrjala@linux.intel.com>
On 31/03/2017 19:00, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The clipped src coordinates have already been rotated by 270 degrees for
> when the plane rotation is 90/270 degrees, hence the FBC code should no
> longer swap the width and height.
>
> Cc: stable@vger.kernel.org
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Fixes: b63a16f6cd89 ("drm/i915: Compute display surface offset in the plane check hook for SKL+")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_fbc.c | 19 +++++++------------
> 1 file changed, 7 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index ded2add18b26..d93c58410bff 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -82,20 +82,10 @@ static unsigned int get_crtc_fence_y_offset(struct intel_crtc *crtc)
> static void intel_fbc_get_plane_source_size(struct intel_fbc_state_cache *cache,
> int *width, int *height)
> {
> - int w, h;
> -
> - if (drm_rotation_90_or_270(cache->plane.rotation)) {
> - w = cache->plane.src_h;
> - h = cache->plane.src_w;
> - } else {
> - w = cache->plane.src_w;
> - h = cache->plane.src_h;
> - }
> -
> if (width)
> - *width = w;
> + *width = cache->plane.src_w;
> if (height)
> - *height = h;
> + *height = cache->plane.src_h;
> }
>
> static int intel_fbc_calculate_cfb_size(struct drm_i915_private *dev_priv,
> @@ -746,6 +736,11 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
> cache->crtc.hsw_bdw_pixel_rate = crtc_state->pixel_rate;
>
> cache->plane.rotation = plane_state->base.rotation;
> + /*
> + * Src coordinates are already rotated by 270 degrees for
> + * the 90/270 degree plane rotation cases (to match the
> + * GTT mapping), hence no need to account for rotation here.
> + */
> cache->plane.src_w = drm_rect_width(&plane_state->base.src) >> 16;
> cache->plane.src_h = drm_rect_height(&plane_state->base.src) >> 16;
> cache->plane.visible = plane_state->base.visible;
>
For the series:
Tested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
^ permalink raw reply
* Re: [PATCH 4.4-only] openvswitch: clear sender cpu before forwarding packets
From: Anoob Soman @ 2017-05-19 10:28 UTC (permalink / raw)
To: Greg KH; +Cc: stable, pshelar, davem, netdev, dev, linux-kernel, Eric Dumazet
In-Reply-To: <20170518081149.GA31512@kroah.com>
On 18/05/17 09:11, Greg KH wrote:
> So backporting that one patch solves the issue here? Can you please
> verify it, and let me know before I apply it?
>
> thanks,
>
> greg k-h
yes, I can do that.
^ permalink raw reply
* Re: [PATCH 6/8] xhci: remove GFP_DMA flag from allocation
From: Mathias Nyman @ 2017-05-19 9:49 UTC (permalink / raw)
To: David Laight, gregkh@linuxfoundation.org
Cc: linux-usb@vger.kernel.org, Matthias Lange, stable@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DCFFFA8F7@AcuExch.aculab.com>
On 19.05.2017 12:10, David Laight wrote:
> From: Mathias Nyman
>> Sent: 17 May 2017 16:32
>> There is no reason to restrict allocations to the first 16MB ISA DMA
>> addresses.
>>
>> It is causing problems in a virtualization setup with enabled IOMMU
>> (x86_64). The result is that USB is not working in the VM.
> ...
>> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
>> index 12b573c..1f1687e 100644
>> --- a/drivers/usb/host/xhci-mem.c
>> +++ b/drivers/usb/host/xhci-mem.c
>> @@ -56,7 +56,7 @@ static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci,
>> }
>>
>> if (max_packet) {
>> - seg->bounce_buf = kzalloc(max_packet, flags | GFP_DMA);
>> + seg->bounce_buf = kzalloc(max_packet, flags);
>
> This might allocate memory that the device cannot access.
> So can only work if dma_map_single() itself allocates a bounce buffer.
> There must be a sane way to do this that doesn't ever require
> double copies.
>
We are using dma_map_single()
This allocated memory is used as the processor virtual memory required by dma_map_single()
i.e. the "void *cpu_addr" part of dma_map_single, see DMA-API.txt:
dma_map_single(struct device *dev, void *cpu_addr, size_t size,
enum dma_data_direction direction)
Maps a piece of processor virtual memory so it can be accessed by the
device and returns the DMA address of the memory.
...
Notes: Not all memory regions in a machine can be mapped by this API.
Further, contiguous kernel virtual space may not be contiguous as
physical memory. Since this API does not provide any scatter/gather
capability, it will fail if the user tries to map a non-physically
contiguous piece of memory. For this reason, memory to be mapped by
this API should be obtained from sources which guarantee it to be
physically contiguous (like kmalloc).
I'm not fully sure I understand yout concern, are you thinking the driver should
doublecheck the dma address returned by dma_map_single() and make sure it's within the
dma mask set for the device?
-Mathias
^ permalink raw reply
* RE: [PATCH 6/8] xhci: remove GFP_DMA flag from allocation
From: David Laight @ 2017-05-19 9:10 UTC (permalink / raw)
To: 'Mathias Nyman', gregkh@linuxfoundation.org
Cc: linux-usb@vger.kernel.org, Matthias Lange, stable@vger.kernel.org
In-Reply-To: <1495035126-29091-7-git-send-email-mathias.nyman@linux.intel.com>
From: Mathias Nyman
> Sent: 17 May 2017 16:32
> There is no reason to restrict allocations to the first 16MB ISA DMA
> addresses.
>
> It is causing problems in a virtualization setup with enabled IOMMU
> (x86_64). The result is that USB is not working in the VM.
...
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 12b573c..1f1687e 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -56,7 +56,7 @@ static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci,
> }
>
> if (max_packet) {
> - seg->bounce_buf = kzalloc(max_packet, flags | GFP_DMA);
> + seg->bounce_buf = kzalloc(max_packet, flags);
This might allocate memory that the device cannot access.
So can only work if dma_map_single() itself allocates a bounce buffer.
There must be a sane way to do this that doesn't ever require
double copies.
David
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox