* [PATCH v3 20/24] wireless: Remove call to memset after dma_alloc_coherent
From: Fuqian Huang @ 2019-07-15 3:19 UTC (permalink / raw)
Cc: Kalle Valo, David S . Miller, Arend van Spriel, Franky Lin,
Hante Meuleman, Chi-Hsien Lin, Wright Feng, Igor Mitsyanko,
Avinash Patil, Sergey Matyukevich, ath10k, linux-wireless, netdev,
linux-kernel, brcm80211-dev-list.pdl, brcm80211-dev-list,
Fuqian Huang
In commit 518a2f1925c3
("dma-mapping: zero memory returned from dma_alloc_*"),
dma_alloc_coherent has already zeroed the memory.
So memset is not needed.
Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
---
Changes in v3:
- Use actual commit rather than the merge commit in the commit message
drivers/net/wireless/ath/ath10k/ce.c | 5 -----
drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 2 --
drivers/net/wireless/quantenna/qtnfmac/pcie/pearl_pcie.c | 2 --
drivers/net/wireless/quantenna/qtnfmac/pcie/topaz_pcie.c | 2 --
4 files changed, 11 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
index eca87f7c5b6c..294fbc1e89ab 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -1704,9 +1704,6 @@ ath10k_ce_alloc_dest_ring_64(struct ath10k *ar, unsigned int ce_id,
/* Correctly initialize memory to 0 to prevent garbage
* data crashing system when download firmware
*/
- memset(dest_ring->base_addr_owner_space_unaligned, 0,
- nentries * sizeof(struct ce_desc_64) + CE_DESC_RING_ALIGN);
-
dest_ring->base_addr_owner_space =
PTR_ALIGN(dest_ring->base_addr_owner_space_unaligned,
CE_DESC_RING_ALIGN);
@@ -2019,8 +2016,6 @@ void ath10k_ce_alloc_rri(struct ath10k *ar)
value |= ar->hw_ce_regs->upd->mask;
ath10k_ce_write32(ar, ce_base_addr + ctrl1_regs, value);
}
-
- memset(ce->vaddr_rri, 0, CE_COUNT * sizeof(u32));
}
EXPORT_SYMBOL(ath10k_ce_alloc_rri);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index 4ea5401c4d6b..5f0437a3fd82 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -1023,8 +1023,6 @@ brcmf_pcie_init_dmabuffer_for_device(struct brcmf_pciedev_info *devinfo,
address & 0xffffffff);
brcmf_pcie_write_tcm32(devinfo, tcm_dma_phys_addr + 4, address >> 32);
- memset(ring, 0, size);
-
return (ring);
}
diff --git a/drivers/net/wireless/quantenna/qtnfmac/pcie/pearl_pcie.c b/drivers/net/wireless/quantenna/qtnfmac/pcie/pearl_pcie.c
index 3aa3714d4dfd..5ec1c9bc1612 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/pcie/pearl_pcie.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/pcie/pearl_pcie.c
@@ -244,8 +244,6 @@ static int pearl_alloc_bd_table(struct qtnf_pcie_pearl_state *ps)
/* tx bd */
- memset(vaddr, 0, len);
-
ps->bd_table_vaddr = vaddr;
ps->bd_table_paddr = paddr;
ps->bd_table_len = len;
diff --git a/drivers/net/wireless/quantenna/qtnfmac/pcie/topaz_pcie.c b/drivers/net/wireless/quantenna/qtnfmac/pcie/topaz_pcie.c
index 9a4380ed7f1b..1f91088e3dff 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/pcie/topaz_pcie.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/pcie/topaz_pcie.c
@@ -199,8 +199,6 @@ static int topaz_alloc_bd_table(struct qtnf_pcie_topaz_state *ts,
if (!vaddr)
return -ENOMEM;
- memset(vaddr, 0, len);
-
/* tx bd */
ts->tx_bd_vbase = vaddr;
--
2.11.0
^ permalink raw reply related
* [PATCH v3 19/24] vmxnet3: Remove call to memset after dma_alloc_coherent
From: Fuqian Huang @ 2019-07-15 3:21 UTC (permalink / raw)
Cc: Ronak Doshi, VMware Inc ., David S . Miller, netdev, linux-kernel,
Fuqian Huang
In commit 518a2f1925c3
("dma-mapping: zero memory returned from dma_alloc_*"),
dma_alloc_coherent has already zeroed the memory.
So memset is not needed.
Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
---
Changes in v3:
- Use actual commit rather than the merge commit in the commit message
drivers/net/vmxnet3/vmxnet3_drv.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 3f48f05dd2a6..2a1918f25e47 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -3430,7 +3430,6 @@ vmxnet3_probe_device(struct pci_dev *pdev,
err = -ENOMEM;
goto err_ver;
}
- memset(adapter->coal_conf, 0, sizeof(*adapter->coal_conf));
adapter->coal_conf->coalMode = VMXNET3_COALESCE_DISABLED;
adapter->default_coal_mode = true;
}
--
2.11.0
^ permalink raw reply related
* [PATCH net] caif-hsi: fix possible deadlock in cfhsi_exit_module()
From: Taehee Yoo @ 2019-07-15 5:10 UTC (permalink / raw)
To: davem, netdev; +Cc: ap420073
cfhsi_exit_module() calls unregister_netdev() under rtnl_lock().
but unregister_netdev() internally calls rtnl_lock().
So deadlock would occur.
Fixes: c41254006377 ("caif-hsi: Add rtnl support")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
I did only compile test because I don't have the testbed.
Could anyone test this patch?
drivers/net/caif/caif_hsi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/caif/caif_hsi.c b/drivers/net/caif/caif_hsi.c
index b2f10b6ad6e5..bbb2575d4728 100644
--- a/drivers/net/caif/caif_hsi.c
+++ b/drivers/net/caif/caif_hsi.c
@@ -1455,7 +1455,7 @@ static void __exit cfhsi_exit_module(void)
rtnl_lock();
list_for_each_safe(list_node, n, &cfhsi_list) {
cfhsi = list_entry(list_node, struct cfhsi, list);
- unregister_netdev(cfhsi->ndev);
+ unregister_netdevice(cfhsi->ndev);
}
rtnl_unlock();
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v3 0/3] kernel/notifier.c: avoid duplicate registration
From: Vasily Averin @ 2019-07-15 5:38 UTC (permalink / raw)
To: Xiaoming Ni, gregkh@linuxfoundation.org
Cc: adobriyan@gmail.com, akpm@linux-foundation.org,
anna.schumaker@netapp.com, arjan@linux.intel.com,
bfields@fieldses.org, chuck.lever@oracle.com, davem@davemloft.net,
jlayton@kernel.org, luto@kernel.org, mingo@kernel.org,
Nadia.Derbey@bull.net, paulmck@linux.vnet.ibm.com,
semen.protsenko@linaro.org, stable@kernel.org,
stern@rowland.harvard.edu, tglx@linutronix.de,
torvalds@linux-foundation.org, trond.myklebust@hammerspace.com,
viresh.kumar@linaro.org, Huangjianhui (Alex), Dailei,
linux-kernel@vger.kernel.org, linux-nfs@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <65f50cf2-3051-ab55-078f-30930fe0c9bc@huawei.com>
On 7/14/19 5:45 AM, Xiaoming Ni wrote:
> On 2019/7/12 22:07, gregkh@linuxfoundation.org wrote:
>> On Fri, Jul 12, 2019 at 09:11:57PM +0800, Xiaoming Ni wrote:
>>> On 2019/7/11 21:57, Vasily Averin wrote:
>>>> On 7/11/19 4:55 AM, Nixiaoming wrote:
>>>>> On Wed, July 10, 2019 1:49 PM Vasily Averin wrote:
>>>>>> On 7/10/19 6:09 AM, Xiaoming Ni wrote:
>>>>>>> Registering the same notifier to a hook repeatedly can cause the hook
>>>>>>> list to form a ring or lose other members of the list.
>>>>>>
>>>>>> I think is not enough to _prevent_ 2nd register attempt,
>>>>>> it's enough to detect just attempt and generate warning to mark host in bad state.
>>>>>>
>>>>>
>>>>> Duplicate registration is prevented in my patch, not just "mark host in bad state"
>>>>>
>>>>> Duplicate registration is checked and exited in notifier_chain_cond_register()
>>>>>
>>>>> Duplicate registration was checked in notifier_chain_register() but only
>>>>> the alarm was triggered without exiting. added by commit 831246570d34692e
>>>>> ("kernel/notifier.c: double register detection")
>>>>>
>>>>> My patch is like a combination of 831246570d34692e and notifier_chain_cond_register(),
>>>>> which triggers an alarm and exits when a duplicate registration is detected.
>>>>>
>>>>>> Unexpected 2nd register of the same hook most likely will lead to 2nd unregister,
>>>>>> and it can lead to host crash in any time:
>>>>>> you can unregister notifier on first attempt it can be too early, it can be still in use.
>>>>>> on the other hand you can never call 2nd unregister at all.
>>>>>
>>>>> Since the member was not added to the linked list at the time of the second registration,
>>>>> no linked list ring was formed.
>>>>> The member is released on the first unregistration and -ENOENT on the second unregistration.
>>>>> After patching, the fault has been alleviated
>>>>
>>>> You are wrong here.
>>>> 2nd notifier's registration is a pure bug, this should never happen.
>>>> If you know the way to reproduce this situation -- you need to fix it.
>>>>
>>>> 2nd registration can happen in 2 cases:
>>>> 1) missed rollback, when someone forget to call unregister after successfull registration,
>>>> and then tried to call register again. It can lead to crash for example when according module will be unloaded.
>>>> 2) some subsystem is registered twice, for example from different namespaces.
>>>> in this case unregister called during sybsystem cleanup in first namespace will incorrectly remove notifier used
>>>> in second namespace, it also can lead to unexpacted behaviour.
>>>>
>>> So in these two cases, is it more reasonable to trigger BUG() directly when checking for duplicate registration ?
>>> But why does current notifier_chain_register() just trigger WARN() without exiting ?
>>> notifier_chain_cond_register() direct exit without triggering WARN() ?
>>
>> It should recover from this, if it can be detected. The main point is
>> that not all apis have to be this "robust" when used within the kernel
>> as we do allow for the callers to know what they are doing :)
>>
> In the notifier_chain_register(), the condition ( (*nl) == n) is the same registration of the same hook.
> We can intercept this situation and avoid forming a linked list ring to make the API more rob
Once again -- yes, you CAN prevent list corruption, but you CANNOT recover the host and return it back to safe state.
If double register event was detected -- it means you have bug in kernel.
Yes, you can add BUG here and crash the host immediately, but I prefer to use warning in such situations.
>> If this does not cause any additional problems or slow downs, it's
>> probably fine to add.
>>
> Notifier_chain_register() is not a system hotspot function.
> At the same time, there is already a WARN_ONCE judgment. There is no new judgment in the new patch.
> It only changes the processing under the condition of (*nl) == n, which will not cause performance problems.
> At the same time, avoiding the formation of a link ring can make the system more robust.
I disagree,
yes, node will have correct list, but anyway node will work wrong and can crash the host in any time.
>> thanks,
>>
>> greg k-h
>>
>> .
>>
> Thanks
>
> Xiaoming Ni
>
>
>
^ permalink raw reply
* Re: [PATCH 8/8] docs: remove extra conf.py files
From: Markus Heiser @ 2019-07-15 6:16 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Linux Doc Mailing List
Cc: Mauro Carvalho Chehab, linux-kernel, Jonathan Corbet, Herbert Xu,
David S. Miller, David Airlie, Daniel Vetter, Maarten Lankhorst,
Maxime Ripard, Sean Paul, Dmitry Torokhov, Yoshinori Sato,
Rich Felker, Jaroslav Kysela, Takashi Iwai, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, H. Peter Anvin, x86, linux-crypto,
dri-devel, linux-input, netdev, linux-sh, alsa-devel
In-Reply-To: <12a160afc9e70156f671010bd4ccff9311acdc5e.1563115732.git.mchehab+samsung@kernel.org>
Hi Mauro,
sorry, I havn't tested your patch, but one question ...
Am 14.07.19 um 17:10 schrieb Mauro Carvalho Chehab:
> Now that the latex_documents are handled automatically, we can
> remove those extra conf.py files.
We need these conf.py also for compiling books' into HTML. For this
the tags.add("subproject") is needed. Should we realy drop this feature?
-- Markus --
^ permalink raw reply
* [PATCH] ethtool: igb: dump RR2DCDELAY register
From: Artem Bityutskiy @ 2019-07-15 6:52 UTC (permalink / raw)
To: John W . Linville; +Cc: netdev
Decode 'RR2DCDELAY' register which Linux kernel provides starting from version
5.3. The corresponding commit in the Linux kernel is:
cd502a7f7c9c igb: add RR2DCDELAY to ethtool registers dump
The RR2DCDELAY register is present in I210 and I211 Intel Gigabit Ethernet
chips and it stands for "Read Request To Data Completion Delay". Here is how
this register is described in the I210 datasheet:
"This field captures the maximum PCIe split time in 16 ns units, which is the
maximum delay between the read request to the first data completion. This is
giving an estimation of the PCIe round trip time."
In practice, this register can be used to measure the time it takes the NIC to
read data from the host memory.
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
igb.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/igb.c b/igb.c
index e0ccef9..ab0896f 100644
--- a/igb.c
+++ b/igb.c
@@ -859,6 +859,18 @@ igb_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs)
"0x03430: TDFPC (Tx data FIFO packet count) 0x%08X\n",
regs_buff[550]);
+ /*
+ * Starting from kernel version 5.3 the registers dump buffer grew from
+ * 739 4-byte words to 740 words, and word 740 contains the RR2DCDLAY
+ * register.
+ */
+ if (regs->len < 740)
+ return 0;
+
+ fprintf(stdout,
+ "0x05BF4: RR2DCDELAY (Max. DMA read delay) 0x%08X\n",
+ regs_buff[739]);
+
return 0;
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] mm/gup: Use put_user_page*() instead of put_page*()
From: Bharath Vedartham @ 2019-07-15 6:56 UTC (permalink / raw)
To: John Hubbard
Cc: akpm, ira.weiny, Mauro Carvalho Chehab, Dimitri Sivanich,
Arnd Bergmann, Greg Kroah-Hartman, Alex Williamson, Cornelia Huck,
Jens Axboe, Alexander Viro, Björn Töpel,
Magnus Karlsson, David S. Miller, Alexei Starovoitov,
Daniel Borkmann, Jakub Kicinski, Jesper Dangaard Brouer,
John Fastabend, Enrico Weigelt, Thomas Gleixner, Alexios Zavras,
Dan Carpenter, Max Filippov, Matt Sickler, Kirill A. Shutemov,
Keith Busch, YueHaibing, linux-media, linux-kernel, devel, kvm,
linux-block, linux-fsdevel, linux-mm, netdev, bpf, xdp-newbies,
Jason Gunthorpe
In-Reply-To: <deea584f-2da2-8e1f-5a07-e97bf32c63bb@nvidia.com>
On Sun, Jul 14, 2019 at 04:33:42PM -0700, John Hubbard wrote:
> On 7/14/19 12:08 PM, Bharath Vedartham wrote:
> > This patch converts all call sites of get_user_pages
> > to use put_user_page*() instead of put_page*() functions to
> > release reference to gup pinned pages.
Hi John,
> Hi Bharath,
>
> Thanks for jumping in to help, and welcome to the party!
>
> You've caught everyone in the middle of a merge window, btw. As a
> result, I'm busy rebasing and reworking the get_user_pages call sites,
> and gup tracking, in the wake of some semi-traumatic changes to bio
> and gup and such. I plan to re-post right after 5.3-rc1 shows up, from
> here:
>
> https://github.com/johnhubbard/linux/commits/gup_dma_core
>
> ...which you'll find already covers the changes you've posted, except for:
>
> drivers/misc/sgi-gru/grufault.c
> drivers/staging/kpc2000/kpc_dma/fileops.c
>
> ...and this one, which is undergoing to larger local changes, due to
> bvec, so let's leave it out of the choices:
>
> fs/io_uring.c
>
> Therefore, until -rc1, if you'd like to help, I'd recommend one or more
> of the following ideas:
>
> 1. Pull down https://github.com/johnhubbard/linux/commits/gup_dma_core
> and find missing conversions: look for any additional missing
> get_user_pages/put_page conversions. You've already found a couple missing
> ones. I haven't re-run a search in a long time, so there's probably even more.
> a) And find more, after I rebase to 5.3-rc1: people probably are adding
> get_user_pages() calls as we speak. :)
Shouldn't this be documented then? I don't see any docs for using
put_user_page*() in v5.2.1 in the memory management API section?
> 2. Patches: Focus on just one subsystem at a time, and perfect the patch for
> it. For example, I think this the staging driver would be perfect to start with:
>
> drivers/staging/kpc2000/kpc_dma/fileops.c
>
> a) verify that you've really, corrected converted the whole
> driver. (Hint: I think you might be overlooking a put_page call.)
Yup. I did see that! Will fix it!
> b) Attempt to test it if you can (I'm being hypocritical in
> the extreme here, but one of my problems is that testing
> has been light, so any help is very valuable). qemu...?
> OTOH, maybe even qemu cannot easily test a kpc2000, but
> perhaps `git blame` and talking to the authors would help
> figure out a way to validate the changes.
Great! I ll do that, I ll mail the patch authors and ask them for help
in testing.
> Thinking about whether you can run a test that would prove or
> disprove my claim in (a), above, could be useful in coming up
> with tests to run.
> In other words, a few very high quality conversions (even just one) that
> we can really put our faith in, is what I value most here. Tested patches
> are awesome.
I understand that!
> 3. Once I re-post, turn on the new CONFIG_DEBUG_GET_USER_PAGES_REFERENCES
> and run things such as xfstest/fstest. (Again, doing so would be going
> further than I have yet--very helpful). Help clarify what conversions have
> actually been tested and work, and which ones remain unvalidated.
> Other: Please note that this:
Yup will do that.
> https://github.com/johnhubbard/linux/commits/gup_dma_core
>
> a) gets rebased often, and
>
> b) has a bunch of commits (iov_iter and related) that conflict
> with the latest linux.git,
>
> c) has some bugs in the bio area, that I'm fixing, so I don't trust
> that's it's safely runnable, for a few more days.
I assume your repo contains only work related to fixing gup issues and
not the main repo for gup development? i.e where gup changes are merged?
Also are release_pages and put_user_pages interchangable?
> One note below, for the future:
>
> >
> > This is a bunch of trivial conversions which is a part of an effort
> > by John Hubbard to solve issues with gup pinned pages and
> > filesystem writeback.
> >
> > The issue is more clearly described in John Hubbard's patch[1] where
> > put_user_page*() functions are introduced.
> >
> > Currently put_user_page*() simply does put_page but future implementations
> > look to change that once treewide change of put_page callsites to
> > put_user_page*() is finished.
> >
> > The lwn article describing the issue with gup pinned pages and filesystem
> > writeback [2].
> >
> > This patch has been tested by building and booting the kernel as I don't
> > have the required hardware to test the device drivers.
> >
> > I did not modify gpu/drm drivers which use release_pages instead of
> > put_page() to release reference of gup pinned pages as I am not clear
> > whether release_pages and put_page are interchangable.
> >
> > [1] https://lkml.org/lkml/2019/3/26/1396
>
> When referring to patches in a commit description, please use the
> commit hash, not an external link. See Submitting Patches [1] for details.
>
> Also, once you figure out the right maintainers and other involved people,
> putting Cc: in the commit description is common practice, too.
>
> [1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html
Will work on that! Thanks!
> thanks,
> --
> John Hubbard
> NVIDIA
>
> >
> > [2] https://lwn.net/Articles/784574/
> >
> > Signed-off-by: Bharath Vedartham <linux.bhar@gmail.com>
> > ---
> > drivers/media/v4l2-core/videobuf-dma-sg.c | 3 +--
> > drivers/misc/sgi-gru/grufault.c | 2 +-
> > drivers/staging/kpc2000/kpc_dma/fileops.c | 4 +---
> > drivers/vfio/vfio_iommu_type1.c | 2 +-
> > fs/io_uring.c | 7 +++----
> > mm/gup_benchmark.c | 6 +-----
> > net/xdp/xdp_umem.c | 7 +------
> > 7 files changed, 9 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/media/v4l2-core/videobuf-dma-sg.c b/drivers/media/v4l2-core/videobuf-dma-sg.c
> > index 66a6c6c..d6eeb43 100644
> > --- a/drivers/media/v4l2-core/videobuf-dma-sg.c
> > +++ b/drivers/media/v4l2-core/videobuf-dma-sg.c
> > @@ -349,8 +349,7 @@ int videobuf_dma_free(struct videobuf_dmabuf *dma)
> > BUG_ON(dma->sglen);
> >
> > if (dma->pages) {
> > - for (i = 0; i < dma->nr_pages; i++)
> > - put_page(dma->pages[i]);
> > + put_user_pages(dma->pages, dma->nr_pages);
> > kfree(dma->pages);
> > dma->pages = NULL;
> > }
> > diff --git a/drivers/misc/sgi-gru/grufault.c b/drivers/misc/sgi-gru/grufault.c
> > index 4b713a8..61b3447 100644
> > --- a/drivers/misc/sgi-gru/grufault.c
> > +++ b/drivers/misc/sgi-gru/grufault.c
> > @@ -188,7 +188,7 @@ static int non_atomic_pte_lookup(struct vm_area_struct *vma,
> > if (get_user_pages(vaddr, 1, write ? FOLL_WRITE : 0, &page, NULL) <= 0)
> > return -EFAULT;
> > *paddr = page_to_phys(page);
> > - put_page(page);
> > + put_user_page(page);
> > return 0;
> > }
> >
> > diff --git a/drivers/staging/kpc2000/kpc_dma/fileops.c b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > index 6166587..26dceed 100644
> > --- a/drivers/staging/kpc2000/kpc_dma/fileops.c
> > +++ b/drivers/staging/kpc2000/kpc_dma/fileops.c
> > @@ -198,9 +198,7 @@ int kpc_dma_transfer(struct dev_private_data *priv, struct kiocb *kcb, unsigned
> > sg_free_table(&acd->sgt);
> > err_dma_map_sg:
> > err_alloc_sg_table:
> > - for (i = 0 ; i < acd->page_count ; i++){
> > - put_page(acd->user_pages[i]);
> > - }
> > + put_user_pages(acd->user_pages, acd->page_count);
> > err_get_user_pages:
> > kfree(acd->user_pages);
> > err_alloc_userpages:
> > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> > index add34ad..c491524 100644
> > --- a/drivers/vfio/vfio_iommu_type1.c
> > +++ b/drivers/vfio/vfio_iommu_type1.c
> > @@ -369,7 +369,7 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned long vaddr,
> > */
> > if (ret > 0 && vma_is_fsdax(vmas[0])) {
> > ret = -EOPNOTSUPP;
> > - put_page(page[0]);
> > + put_user_page(page[0]);
> > }
> > }
> > up_read(&mm->mmap_sem);
> > diff --git a/fs/io_uring.c b/fs/io_uring.c
> > index 4ef62a4..b4a4549 100644
> > --- a/fs/io_uring.c
> > +++ b/fs/io_uring.c
> > @@ -2694,10 +2694,9 @@ static int io_sqe_buffer_register(struct io_ring_ctx *ctx, void __user *arg,
> > * if we did partial map, or found file backed vmas,
> > * release any pages we did get
> > */
> > - if (pret > 0) {
> > - for (j = 0; j < pret; j++)
> > - put_page(pages[j]);
> > - }
> > + if (pret > 0)
> > + put_user_pages(pages, pret);
> > +
> > if (ctx->account_mem)
> > io_unaccount_mem(ctx->user, nr_pages);
> > kvfree(imu->bvec);
> > diff --git a/mm/gup_benchmark.c b/mm/gup_benchmark.c
> > index 7dd602d..15fc7a2 100644
> > --- a/mm/gup_benchmark.c
> > +++ b/mm/gup_benchmark.c
> > @@ -76,11 +76,7 @@ static int __gup_benchmark_ioctl(unsigned int cmd,
> > gup->size = addr - gup->addr;
> >
> > start_time = ktime_get();
> > - for (i = 0; i < nr_pages; i++) {
> > - if (!pages[i])
> > - break;
> > - put_page(pages[i]);
> > - }
> > + put_user_pages(pages, nr_pages);
> > end_time = ktime_get();
> > gup->put_delta_usec = ktime_us_delta(end_time, start_time);
> >
> > diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
> > index 9c6de4f..6103e19 100644
> > --- a/net/xdp/xdp_umem.c
> > +++ b/net/xdp/xdp_umem.c
> > @@ -173,12 +173,7 @@ static void xdp_umem_unpin_pages(struct xdp_umem *umem)
> > {
> > unsigned int i;
> >
> > - for (i = 0; i < umem->npgs; i++) {
> > - struct page *page = umem->pgs[i];
> > -
> > - set_page_dirty_lock(page);
> > - put_page(page);
> > - }
> > + put_user_pages_dirty_lock(umem->pgs, umem->npgs);
> >
> > kfree(umem->pgs);
> > umem->pgs = NULL;
> >
^ permalink raw reply
* Re: [PATCH] mm/gup: Use put_user_page*() instead of put_page*()
From: Bharath Vedartham @ 2019-07-15 6:59 UTC (permalink / raw)
To: Jens Axboe
Cc: akpm, ira.weiny, jhubbard, Mauro Carvalho Chehab,
Dimitri Sivanich, Arnd Bergmann, Greg Kroah-Hartman,
Alex Williamson, Cornelia Huck, Alexander Viro,
Björn Töpel, Magnus Karlsson, David S. Miller,
Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski,
Jesper Dangaard Brouer, John Fastabend, Enrico Weigelt,
Thomas Gleixner, Alexios Zavras, Dan Carpenter, Max Filippov,
Matt Sickler, Kirill A. Shutemov, Keith Busch, YueHaibing,
linux-media, linux-kernel, devel, kvm, linux-block, linux-fsdevel,
linux-mm, netdev, bpf, xdp-newbies
In-Reply-To: <018ee3d1-e2f0-ca12-9f63-945056c09985@kernel.dk>
On Sun, Jul 14, 2019 at 08:33:57PM -0600, Jens Axboe wrote:
> On 7/14/19 1:08 PM, Bharath Vedartham wrote:
> > diff --git a/fs/io_uring.c b/fs/io_uring.c
> > index 4ef62a4..b4a4549 100644
> > --- a/fs/io_uring.c
> > +++ b/fs/io_uring.c
> > @@ -2694,10 +2694,9 @@ static int io_sqe_buffer_register(struct io_ring_ctx *ctx, void __user *arg,
> > * if we did partial map, or found file backed vmas,
> > * release any pages we did get
> > */
> > - if (pret > 0) {
> > - for (j = 0; j < pret; j++)
> > - put_page(pages[j]);
> > - }
> > + if (pret > 0)
> > + put_user_pages(pages, pret);
> > +
> > if (ctx->account_mem)
> > io_unaccount_mem(ctx->user, nr_pages);
> > kvfree(imu->bvec);
>
> You handled just the failure case of the buffer registration, but not
> the actual free in io_sqe_buffer_unregister().
>
> --
> Jens Axboe
Yup got it! Thanks! I won't be sending a patch again as fs/io_uring.c
may have larger local changes for put_user_pages.
Thanks
^ permalink raw reply
* Re: [PATCH v2 3/9] rcu/sync: Remove custom check for reader-section
From: Oleg Nesterov @ 2019-07-15 7:26 UTC (permalink / raw)
To: Joel Fernandes
Cc: Paul E. McKenney, linux-kernel, Alexey Kuznetsov, Bjorn Helgaas,
Borislav Petkov, c0d1n61at3, David S. Miller, edumazet,
Greg Kroah-Hartman, Hideaki YOSHIFUJI, H. Peter Anvin,
Ingo Molnar, Jonathan Corbet, Josh Triplett, keescook,
kernel-hardening, kernel-team, Lai Jiangshan, Len Brown,
linux-acpi, linux-doc, linux-pci, linux-pm, Mathieu Desnoyers,
neilb, netdev, Pavel Machek, peterz, Rafael J. Wysocki,
Rasmus Villemoes, rcu, Steven Rostedt, Tejun Heo, Thomas Gleixner,
will, maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)
In-Reply-To: <20190713031008.GA248225@google.com>
On 07/12, Joel Fernandes wrote:
>
> static inline bool rcu_sync_is_idle(struct rcu_sync *rsp)
> {
> - RCU_LOCKDEP_WARN(!rcu_read_lock_held() &&
> - !rcu_read_lock_bh_held() &&
> - !rcu_read_lock_sched_held(),
> + RCU_LOCKDEP_WARN(!rcu_read_lock_any_held(),
Yes, this is what I meant.
Sorry for confusion, I should have mentioned that rcu_sync_is_idle()
was recently updated when I suggested to use the new helper.
Acked-by: Oleg Nesterov <oleg@redhat.com>
^ permalink raw reply
* Re: [RFC] virtio-net: share receive_*() and add_recvbuf_*() with virtio-vsock
From: Stefano Garzarella @ 2019-07-15 7:44 UTC (permalink / raw)
To: Jason Wang; +Cc: Michael S. Tsirkin, Stefan Hajnoczi, virtualization, netdev
In-Reply-To: <a514d8a4-3a12-feeb-4467-af7a9fbf5183@redhat.com>
On Fri, Jul 12, 2019 at 06:14:39PM +0800, Jason Wang wrote:
>
> On 2019/7/12 下午6:00, Stefano Garzarella wrote:
> > On Thu, Jul 11, 2019 at 03:52:21PM -0400, Michael S. Tsirkin wrote:
> > > On Thu, Jul 11, 2019 at 01:41:34PM +0200, Stefano Garzarella wrote:
> > > > On Thu, Jul 11, 2019 at 03:37:00PM +0800, Jason Wang wrote:
> > > > > On 2019/7/10 下午11:37, Stefano Garzarella wrote:
> > > > > > Hi,
> > > > > > as Jason suggested some months ago, I looked better at the virtio-net driver to
> > > > > > understand if we can reuse some parts also in the virtio-vsock driver, since we
> > > > > > have similar challenges (mergeable buffers, page allocation, small
> > > > > > packets, etc.).
> > > > > >
> > > > > > Initially, I would add the skbuff in the virtio-vsock in order to re-use
> > > > > > receive_*() functions.
> > > > >
> > > > > Yes, that will be a good step.
> > > > >
> > > > Okay, I'll go on this way.
> > > >
> > > > > > Then I would move receive_[small, big, mergeable]() and
> > > > > > add_recvbuf_[small, big, mergeable]() outside of virtio-net driver, in order to
> > > > > > call them also from virtio-vsock. I need to do some refactoring (e.g. leave the
> > > > > > XDP part on the virtio-net driver), but I think it is feasible.
> > > > > >
> > > > > > The idea is to create a virtio-skb.[h,c] where put these functions and a new
> > > > > > object where stores some attributes needed (e.g. hdr_len ) and status (e.g.
> > > > > > some fields of struct receive_queue).
> > > > >
> > > > > My understanding is we could be more ambitious here. Do you see any blocker
> > > > > for reusing virtio-net directly? It's better to reuse not only the functions
> > > > > but also the logic like NAPI to avoid re-inventing something buggy and
> > > > > duplicated.
> > > > >
> > > > These are my concerns:
> > > > - virtio-vsock is not a "net_device", so a lot of code related to
> > > > ethtool, net devices (MAC address, MTU, speed, VLAN, XDP, offloading) will be
> > > > not used by virtio-vsock.
>
>
> Linux support device other than ethernet, so it should not be a problem.
>
>
> > > >
> > > > - virtio-vsock has a different header. We can consider it as part of
> > > > virtio_net payload, but it precludes the compatibility with old hosts. This
> > > > was one of the major doubts that made me think about using only the
> > > > send/recv skbuff functions, that it shouldn't break the compatibility.
>
>
> We can extend the current vnet header helper for it to work for vsock.
Okay, I'll do it.
>
>
> > > >
> > > > > > This is an idea of virtio-skb.h that
> > > > > > I have in mind:
> > > > > > struct virtskb;
> > > > >
> > > > > What fields do you want to store in virtskb? It looks to be exist sk_buff is
> > > > > flexible enough to us?
> > > > My idea is to store queues information, like struct receive_queue or
> > > > struct send_queue, and some device attributes (e.g. hdr_len ).
>
>
> If you reuse skb or virtnet_info, there is not necessary.
>
Okay.
>
> > > >
> > > > >
> > > > > > struct sk_buff *virtskb_receive_small(struct virtskb *vs, ...);
> > > > > > struct sk_buff *virtskb_receive_big(struct virtskb *vs, ...);
> > > > > > struct sk_buff *virtskb_receive_mergeable(struct virtskb *vs, ...);
> > > > > >
> > > > > > int virtskb_add_recvbuf_small(struct virtskb*vs, ...);
> > > > > > int virtskb_add_recvbuf_big(struct virtskb *vs, ...);
> > > > > > int virtskb_add_recvbuf_mergeable(struct virtskb *vs, ...);
> > > > > >
> > > > > > For the Guest->Host path it should be easier, so maybe I can add a
> > > > > > "virtskb_send(struct virtskb *vs, struct sk_buff *skb)" with a part of the code
> > > > > > of xmit_skb().
> > > > >
> > > > > I may miss something, but I don't see any thing that prevents us from using
> > > > > xmit_skb() directly.
> > > > >
> > > > Yes, but my initial idea was to make it more parametric and not related to the
> > > > virtio_net_hdr, so the 'hdr_len' could be a parameter and the
> > > > 'num_buffers' should be handled by the caller.
> > > >
> > > > > > Let me know if you have in mind better names or if I should put these function
> > > > > > in another place.
> > > > > >
> > > > > > I would like to leave the control part completely separate, so, for example,
> > > > > > the two drivers will negotiate the features independently and they will call
> > > > > > the right virtskb_receive_*() function based on the negotiation.
> > > > >
> > > > > If it's one the issue of negotiation, we can simply change the
> > > > > virtnet_probe() to deal with different devices.
> > > > >
> > > > >
> > > > > > I already started to work on it, but before to do more steps and send an RFC
> > > > > > patch, I would like to hear your opinion.
> > > > > > Do you think that makes sense?
> > > > > > Do you see any issue or a better solution?
> > > > >
> > > > > I still think we need to seek a way of adding some codes on virtio-net.c
> > > > > directly if there's no huge different in the processing of TX/RX. That would
> > > > > save us a lot time.
> > > > After the reading of the buffers from the virtqueue I think the process
> > > > is slightly different, because virtio-net will interface with the network
> > > > stack, while virtio-vsock will interface with the vsock-core (socket).
> > > > So the virtio-vsock implements the following:
> > > > - control flow mechanism to avoid to loose packets, informing the peer
> > > > about the amount of memory available in the receive queue using some
> > > > fields in the virtio_vsock_hdr
> > > > - de-multiplexing parsing the virtio_vsock_hdr and choosing the right
> > > > socket depending on the port
> > > > - socket state handling
>
>
> I think it's just a branch, for ethernet, go for networking stack. otherwise
> go for vsock core?
>
Yes, that should work.
So, I should refactor the functions that can be called also from the vsock
core, in order to remove "struct net_device *dev" parameter.
Maybe creating some wrappers for the network stack.
Otherwise I should create a fake net_device for vsock_core.
What do you suggest?
>
> > > >
> > > > We can use the virtio-net as transport, but we should add a lot of
> > > > code to skip "net device" stuff when it is used by the virtio-vsock.
>
>
> This could be another choice, but consider it was not transparent to the
> admin and require new features, we may seek a transparent solution here.
>
>
> > > > This could break something in virtio-net, for this reason, I thought to reuse
> > > > only the send/recv functions starting from the idea to split the virtio-net
> > > > driver in two parts:
> > > > a. one with all stuff related to the network stack
> > > > b. one with the stuff needed to communicate with the host
> > > >
> > > > And use skbuff to communicate between parts. In this way, virtio-vsock
> > > > can use only the b part.
> > > >
> > > > Maybe we can do this split in a better way, but I'm not sure it is
> > > > simple.
> > > >
> > > > Thanks,
> > > > Stefano
> > > Frankly, skb is a huge structure which adds a lot of
> > > overhead. I am not sure that using it is such a great idea
> > > if building a device that does not have to interface
> > > with the networking stack.
>
>
> I believe vsock is mainly used for stream performance not for PPS. So the
> impact should be minimal. We can use other metadata, just need branch in
> recv_xxx().
>
Yes, I think stream performance is the case.
>
> > Thanks for the advice!
> >
> > > So I agree with Jason in theory. To clarify, he is basically saying
> > > current implementation is all wrong, it should be a protocol and we
> > > should teach networking stack that there are reliable net devices that
> > > handle just this protocol. We could add a flag in virtio net that
> > > will say it's such a device.
> > >
> > > Whether it's doable, I don't know, and it's definitely not simple - in
> > > particular you will have to also re-implement existing devices in these
> > > terms, and not just virtio - vmware vsock too.
>
>
> Merging vsock protocol to exist networking stack could be a long term goal,
> I believe for the first phase, we can seek to use virtio-net first.
>
Yes, I agree.
>
> > >
> > > If you want to do a POC you can add a new address family,
> > > that's easier.
> > Very interesting!
> > I agree with you. In this way we can completely split the protocol
> > logic, from the device.
> >
> > As you said, it will not simple to do, but can be an opportunity to learn
> > better the Linux networking stack!
> > I'll try to do a PoC with AF_VSOCK2 that will use the virtio-net.
>
>
> I suggest to do this step by step:
>
> 1) use virtio-net but keep some protocol logic
>
> 2) separate protocol logic and merge it to exist Linux networking stack
Make sense, thanks for the suggestions, I'll try to do these steps!
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH] ipvs: remove unnecessary space
From: Pablo Neira Ayuso @ 2019-07-15 7:57 UTC (permalink / raw)
To: yangxingwu
Cc: wensong, horms, ja, kadlec, fw, davem, netdev, lvs-devel,
netfilter-devel, coreteam, linux-kernel, joe
In-Reply-To: <20190712130721.7168-1-xingwu.yang@gmail.com>
On Fri, Jul 12, 2019 at 09:07:21PM +0800, yangxingwu wrote:
> this patch removes the extra space and use bitmap_zalloc instead
>
> Signed-off-by: yangxingwu <xingwu.yang@gmail.com>
> ---
> net/netfilter/ipvs/ip_vs_mh.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_mh.c b/net/netfilter/ipvs/ip_vs_mh.c
> index 94d9d34..3229867 100644
> --- a/net/netfilter/ipvs/ip_vs_mh.c
> +++ b/net/netfilter/ipvs/ip_vs_mh.c
> @@ -174,8 +174,7 @@ static int ip_vs_mh_populate(struct ip_vs_mh_state *s,
> return 0;
> }
>
> - table = kcalloc(BITS_TO_LONGS(IP_VS_MH_TAB_SIZE),
> - sizeof(unsigned long), GFP_KERNEL);
> + table = bitmap_zalloc(IP_VS_MH_TAB_SIZE, GFP_KERNEL);
By doing:
git grep "= " ...
on the netfilter folders, I see more of these, it would be good if you
fix them at once or, probably, you want to use coccinelle for this.
Thanks.
^ permalink raw reply
* Re: [PATCH 8/8] docs: remove extra conf.py files
From: Mauro Carvalho Chehab @ 2019-07-15 7:57 UTC (permalink / raw)
To: Markus Heiser
Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
Jonathan Corbet, Herbert Xu, David S. Miller, David Airlie,
Daniel Vetter, Maarten Lankhorst, Maxime Ripard, Sean Paul,
Dmitry Torokhov, Yoshinori Sato, Rich Felker, Jaroslav Kysela,
Takashi Iwai, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
H. Peter Anvin, x86, linux-crypto, dri-devel, linux-input, netdev,
linux-sh, alsa-devel
In-Reply-To: <e3ff0a8a-6956-3855-07be-9c126df2da2d@darmarit.de>
Em Mon, 15 Jul 2019 08:16:54 +0200
Markus Heiser <markus.heiser@darmarit.de> escreveu:
> Hi Mauro,
>
> sorry, I havn't tested your patch, but one question ...
>
> Am 14.07.19 um 17:10 schrieb Mauro Carvalho Chehab:
> > Now that the latex_documents are handled automatically, we can
> > remove those extra conf.py files.
>
> We need these conf.py also for compiling books' into HTML. For this
> the tags.add("subproject") is needed. Should we realy drop this feature?
>
> -- Markus --
You're right: adding "subproject" tags is needed for html. Folding this
to patch 7/8 makes both htmldocs and pdfdocs to work with SPHINXDIRS
without the need of a per-subdir conf.py.
Regards,
Mauro
diff --git a/Documentation/sphinx/load_config.py b/Documentation/sphinx/load_config.py
index 75f527ff4c95..e4a04f367b41 100644
--- a/Documentation/sphinx/load_config.py
+++ b/Documentation/sphinx/load_config.py
@@ -51,3 +51,7 @@ def loadConfig(namespace):
execfile_(config_file, config)
del config['__file__']
namespace.update(config)
+ else:
+ config = namespace.copy()
+ config['tags'].add("subproject")
+ namespace.update(config)
Thanks,
Mauro
^ permalink raw reply related
* Re: [PATCH] ipvs: remove unnecessary space
From: Pablo Neira Ayuso @ 2019-07-15 8:26 UTC (permalink / raw)
To: yangxingwu
Cc: wensong, horms, ja, kadlec, fw, davem, netdev, lvs-devel,
netfilter-devel, coreteam, linux-kernel, joe
In-Reply-To: <20190715075703.ak6nk3sbnqksjh72@salvia>
On Mon, Jul 15, 2019 at 09:57:03AM +0200, Pablo Neira Ayuso wrote:
> On Fri, Jul 12, 2019 at 09:07:21PM +0800, yangxingwu wrote:
> > this patch removes the extra space and use bitmap_zalloc instead
> >
> > Signed-off-by: yangxingwu <xingwu.yang@gmail.com>
> > ---
> > net/netfilter/ipvs/ip_vs_mh.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/net/netfilter/ipvs/ip_vs_mh.c b/net/netfilter/ipvs/ip_vs_mh.c
> > index 94d9d34..3229867 100644
> > --- a/net/netfilter/ipvs/ip_vs_mh.c
> > +++ b/net/netfilter/ipvs/ip_vs_mh.c
> > @@ -174,8 +174,7 @@ static int ip_vs_mh_populate(struct ip_vs_mh_state *s,
> > return 0;
> > }
> >
> > - table = kcalloc(BITS_TO_LONGS(IP_VS_MH_TAB_SIZE),
> > - sizeof(unsigned long), GFP_KERNEL);
> > + table = bitmap_zalloc(IP_VS_MH_TAB_SIZE, GFP_KERNEL);
>
> By doing:
>
> git grep "= " ...
>
> on the netfilter folders, I see more of these, it would be good if you
> fix them at once or, probably, you want to use coccinelle for this.
If patch subject is "remove unnecessary space" then just remove
unnecessary spaces in the patch, otherwise I suggest you rename this
to "ipvs: use bitmap_zalloc()" or such, since the space removal here
is irrelevant.
^ permalink raw reply
* Re: [PATCH] ipvs: remove unnecessary space
From: Pablo Neira Ayuso @ 2019-07-15 8:27 UTC (permalink / raw)
To: Simon Horman
Cc: yangxingwu, wensong, ja, kadlec, fw, davem, netdev, lvs-devel,
netfilter-devel, coreteam, linux-kernel
In-Reply-To: <20190710080609.smxjqe2d5jyro4hv@verge.net.au>
On Wed, Jul 10, 2019 at 10:06:09AM +0200, Simon Horman wrote:
> On Wed, Jul 10, 2019 at 03:45:52PM +0800, yangxingwu wrote:
> > ---
> > net/netfilter/ipvs/ip_vs_mh.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/netfilter/ipvs/ip_vs_mh.c b/net/netfilter/ipvs/ip_vs_mh.c
> > index 94d9d34..98e358e 100644
> > --- a/net/netfilter/ipvs/ip_vs_mh.c
> > +++ b/net/netfilter/ipvs/ip_vs_mh.c
> > @@ -174,8 +174,8 @@ static int ip_vs_mh_populate(struct ip_vs_mh_state *s,
> > return 0;
> > }
> >
> > - table = kcalloc(BITS_TO_LONGS(IP_VS_MH_TAB_SIZE),
> > - sizeof(unsigned long), GFP_KERNEL);
> > + table = kcalloc(BITS_TO_LONGS(IP_VS_MH_TAB_SIZE),
> > + sizeof(unsigned long), GFP_KERNEL);
May I ask one thing? :-)
Please, remove all unnecessary spaces in one go, search for:
git grep "= "
in the netfilter tree, and send a v2 for this one.
Thanks.
^ permalink raw reply
* [PATCH] e1000e: Make speed detection on hotplugging cable more reliable
From: Kai-Heng Feng @ 2019-07-15 8:43 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: intel-wired-lan, netdev, linux-kernel, Kai-Heng Feng
After hotplugging an 1Gbps ethernet cable with 1Gbps link partner, the
MII_BMSR may reports 10Mbps, renders the network rather slow.
The issue has much lower fail rate after commit 59653e6497d1 ("e1000e:
Make watchdog use delayed work"), which esssentially introduces some
delay before running the watchdog task.
But there's still a chance that the hotplugging event and the queued
watchdog task gets run at the same time, then the original issue can be
observed once again.
So let's use mod_delayed_work() to add a deterministic 1 second delay
before running watchdog task, after an interrupt.
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index e4baa13b3cda..c83bf5349d53 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -1780,8 +1780,8 @@ static irqreturn_t e1000_intr_msi(int __always_unused irq, void *data)
}
/* guard against interrupt when we're going down */
if (!test_bit(__E1000_DOWN, &adapter->state))
- queue_delayed_work(adapter->e1000_workqueue,
- &adapter->watchdog_task, 1);
+ mod_delayed_work(adapter->e1000_workqueue,
+ &adapter->watchdog_task, HZ);
}
/* Reset on uncorrectable ECC error */
@@ -1861,8 +1861,8 @@ static irqreturn_t e1000_intr(int __always_unused irq, void *data)
}
/* guard against interrupt when we're going down */
if (!test_bit(__E1000_DOWN, &adapter->state))
- queue_delayed_work(adapter->e1000_workqueue,
- &adapter->watchdog_task, 1);
+ mod_delayed_work(adapter->e1000_workqueue,
+ &adapter->watchdog_task, HZ);
}
/* Reset on uncorrectable ECC error */
@@ -1907,8 +1907,8 @@ static irqreturn_t e1000_msix_other(int __always_unused irq, void *data)
hw->mac.get_link_status = true;
/* guard against interrupt when we're going down */
if (!test_bit(__E1000_DOWN, &adapter->state))
- queue_delayed_work(adapter->e1000_workqueue,
- &adapter->watchdog_task, 1);
+ mod_delayed_work(adapter->e1000_workqueue,
+ &adapter->watchdog_task, HZ);
}
if (!test_bit(__E1000_DOWN, &adapter->state))
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v2 1/2] rt2x00usb: fix rx queue hang
From: Kalle Valo @ 2019-07-15 8:48 UTC (permalink / raw)
To: Soeren Moch
Cc: Stanislaw Gruszka, stable, Helmut Schaa, David S. Miller,
linux-wireless, netdev, linux-kernel
In-Reply-To: <20190701105314.9707-1-smoch@web.de>
Soeren Moch <smoch@web.de> writes:
> Since commit ed194d136769 ("usb: core: remove local_irq_save() around
> ->complete() handler") the handler rt2x00usb_interrupt_rxdone() is
> not running with interrupts disabled anymore. So this completion handler
> is not guaranteed to run completely before workqueue processing starts
> for the same queue entry.
> Be sure to set all other flags in the entry correctly before marking
> this entry ready for workqueue processing. This way we cannot miss error
> conditions that need to be signalled from the completion handler to the
> worker thread.
> Note that rt2x00usb_work_rxdone() processes all available entries, not
> only such for which queue_work() was called.
>
> This patch is similar to what commit df71c9cfceea ("rt2x00: fix order
> of entry flags modification") did for TX processing.
>
> This fixes a regression on a RT5370 based wifi stick in AP mode, which
> suddenly stopped data transmission after some period of heavy load. Also
> stopping the hanging hostapd resulted in the error message "ieee80211
> phy0: rt2x00queue_flush_queue: Warning - Queue 14 failed to flush".
> Other operation modes are probably affected as well, this just was
> the used testcase.
>
> Fixes: ed194d136769 ("usb: core: remove local_irq_save() around ->complete() handler")
> Cc: stable@vger.kernel.org # 4.20+
> Signed-off-by: Soeren Moch <smoch@web.de>
I'll queue this for v5.3.
--
Kalle Valo
^ permalink raw reply
* Re: [Intel-wired-lan] [PATCH] e1000e: Make speed detection on hotplugging cable more reliable
From: Paul Menzel @ 2019-07-15 8:52 UTC (permalink / raw)
To: Kai-Heng Feng; +Cc: Jeff Kirsher, netdev, intel-wired-lan, linux-kernel
In-Reply-To: <20190715084355.9962-1-kai.heng.feng@canonical.com>
[-- Attachment #1: Type: text/plain, Size: 1158 bytes --]
Dear Kai-Heng,
Thank you for the patch.
On 7/15/19 10:43 AM, Kai-Heng Feng wrote:
> After hotplugging an 1Gbps ethernet cable with 1Gbps link partner, the
> MII_BMSR may reports 10Mbps, renders the network rather slow.
s/may reports/may report/
s/renders/rendering/
> The issue has much lower fail rate after commit 59653e6497d1 ("e1000e:
> Make watchdog use delayed work"), which esssentially introduces some
essentially
> delay before running the watchdog task.
>
> But there's still a chance that the hotplugging event and the queued
> watchdog task gets run at the same time, then the original issue can be
> observed once again.
>
> So let's use mod_delayed_work() to add a deterministic 1 second delay
> before running watchdog task, after an interrupt.
I am not clear about the effects for the user. Could you elaborate
please? Does the link now come up up to one second later?
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Any bug URL?
> ---
> drivers/net/ethernet/intel/e1000e/netdev.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
Kind regards,
Paul
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5174 bytes --]
^ permalink raw reply
* Re: [Intel-wired-lan] [PATCH] e1000e: Make speed detection on hotplugging cable more reliable
From: Kai Heng Feng @ 2019-07-15 9:00 UTC (permalink / raw)
To: Paul Menzel; +Cc: Jeff Kirsher, netdev, intel-wired-lan, linux-kernel
In-Reply-To: <017771d5-f168-493a-46a1-88e513941ba1@molgen.mpg.de>
at 4:52 PM, Paul Menzel <pmenzel@molgen.mpg.de> wrote:
> Dear Kai-Heng,
>
>
> Thank you for the patch.
>
> On 7/15/19 10:43 AM, Kai-Heng Feng wrote:
>> After hotplugging an 1Gbps ethernet cable with 1Gbps link partner, the
>> MII_BMSR may reports 10Mbps, renders the network rather slow.
>
> s/may reports/may report/
> s/renders/rendering/
Apparently English isn’t my mother tongue ;)
>
>> The issue has much lower fail rate after commit 59653e6497d1 ("e1000e:
>> Make watchdog use delayed work"), which esssentially introduces some
>
> essentially
Ok.
>
>> delay before running the watchdog task.
>>
>> But there's still a chance that the hotplugging event and the queued
>> watchdog task gets run at the same time, then the original issue can be
>> observed once again.
>>
>> So let's use mod_delayed_work() to add a deterministic 1 second delay
>> before running watchdog task, after an interrupt.
>
> I am not clear about the effects for the user. Could you elaborate
> please? Does the link now come up up to one second later?
Yes, the link will be up on a fixed one second later.
The delay varies between 0 to 2 seconds without this patch.
>
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>
> Any bug URL?
If maintainers think it’s necessary then I’ll file one.
Kai-Heng
>
>> ---
>> drivers/net/ethernet/intel/e1000e/netdev.c | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>
>
> Kind regards,
>
> Paul
^ permalink raw reply
* Re: [Intel-wired-lan] [PATCH] e1000e: Make speed detection on hotplugging cable more reliable
From: Paul Menzel @ 2019-07-15 9:06 UTC (permalink / raw)
To: Kai Heng Feng; +Cc: Jeff Kirsher, netdev, intel-wired-lan, linux-kernel
In-Reply-To: <F9859C57-4F6D-4685-B4B6-D1D7DCB50D27@canonical.com>
[-- Attachment #1: Type: text/plain, Size: 1725 bytes --]
Dear Kai Heng,
(with or without hyphen?)
On 7/15/19 11:00 AM, Kai Heng Feng wrote:
> at 4:52 PM, Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>> On 7/15/19 10:43 AM, Kai-Heng Feng wrote:
>>> After hotplugging an 1Gbps ethernet cable with 1Gbps link partner, the
>>> MII_BMSR may reports 10Mbps, renders the network rather slow.
>>
>> s/may reports/may report/
>> s/renders/rendering/
>
> Apparently English isn’t my mother tongue ;)
No problem. Mine neither.
>>> The issue has much lower fail rate after commit 59653e6497d1 ("e1000e:
>>> Make watchdog use delayed work"), which esssentially introduces some
>>
>> essentially
>
> Ok.
>
>>> delay before running the watchdog task.
>>>
>>> But there's still a chance that the hotplugging event and the queued
>>> watchdog task gets run at the same time, then the original issue can be
>>> observed once again.
>>>
>>> So let's use mod_delayed_work() to add a deterministic 1 second delay
>>> before running watchdog task, after an interrupt.
>>
>> I am not clear about the effects for the user. Could you elaborate
>> please? Does the link now come up up to one second later?
>
> Yes, the link will be up on a fixed one second later.
>
> The delay varies between 0 to 2 seconds without this patch.
Is there no other fix? Regarding booting a system fast (less than six
seconds), a fixed one second delay is quite a regression on systems where
it worked before.
>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>>
>> Any bug URL?
>
> If maintainers think it’s necessary then I’ll file one.
Not necessary, if there is none. I thought you had one in Launchpad or so.
Kind regards,
Paul
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5174 bytes --]
^ permalink raw reply
* Re: [PATCH v3 20/24] wireless: Remove call to memset after dma_alloc_coherent
From: Kalle Valo @ 2019-07-15 9:06 UTC (permalink / raw)
To: Fuqian Huang
Cc: David S . Miller, Arend van Spriel, Franky Lin, Hante Meuleman,
Chi-Hsien Lin, Wright Feng, Igor Mitsyanko, Avinash Patil,
Sergey Matyukevich, ath10k, linux-wireless, netdev, linux-kernel,
brcm80211-dev-list.pdl, brcm80211-dev-list
In-Reply-To: <20190715031941.7120-1-huangfq.daxian@gmail.com>
Fuqian Huang <huangfq.daxian@gmail.com> writes:
> In commit 518a2f1925c3
> ("dma-mapping: zero memory returned from dma_alloc_*"),
> dma_alloc_coherent has already zeroed the memory.
> So memset is not needed.
>
> Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
> ---
> Changes in v3:
> - Use actual commit rather than the merge commit in the commit message
>
> drivers/net/wireless/ath/ath10k/ce.c | 5 -----
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 2 --
> drivers/net/wireless/quantenna/qtnfmac/pcie/pearl_pcie.c | 2 --
> drivers/net/wireless/quantenna/qtnfmac/pcie/topaz_pcie.c | 2 --
> 4 files changed, 11 deletions(-)
Via which tree is this supposed to go? Can I take this to
wireless-drivers-next?
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH v3 20/24] wireless: Remove call to memset after dma_alloc_coherent
From: Kalle Valo @ 2019-07-15 9:07 UTC (permalink / raw)
To: Fuqian Huang
Cc: David S . Miller, Arend van Spriel, Franky Lin, Hante Meuleman,
Chi-Hsien Lin, Wright Feng, Igor Mitsyanko, Avinash Patil,
Sergey Matyukevich, ath10k, linux-wireless, netdev, linux-kernel,
brcm80211-dev-list.pdl, brcm80211-dev-list
In-Reply-To: <20190715031941.7120-1-huangfq.daxian@gmail.com>
Fuqian Huang <huangfq.daxian@gmail.com> writes:
> In commit 518a2f1925c3
> ("dma-mapping: zero memory returned from dma_alloc_*"),
> dma_alloc_coherent has already zeroed the memory.
> So memset is not needed.
>
> Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
> ---
> Changes in v3:
> - Use actual commit rather than the merge commit in the commit message
>
> drivers/net/wireless/ath/ath10k/ce.c | 5 -----
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c | 2 --
> drivers/net/wireless/quantenna/qtnfmac/pcie/pearl_pcie.c | 2 --
> drivers/net/wireless/quantenna/qtnfmac/pcie/topaz_pcie.c | 2 --
> 4 files changed, 11 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
> index eca87f7c5b6c..294fbc1e89ab 100644
> --- a/drivers/net/wireless/ath/ath10k/ce.c
> +++ b/drivers/net/wireless/ath/ath10k/ce.c
> @@ -1704,9 +1704,6 @@ ath10k_ce_alloc_dest_ring_64(struct ath10k *ar, unsigned int ce_id,
> /* Correctly initialize memory to 0 to prevent garbage
> * data crashing system when download firmware
> */
> - memset(dest_ring->base_addr_owner_space_unaligned, 0,
> - nentries * sizeof(struct ce_desc_64) + CE_DESC_RING_ALIGN);
Shouldn't you also remove the comment?
--
Kalle Valo
^ permalink raw reply
* [PATCH bpf] samples/bpf: build with -D__TARGET_ARCH_$(SRCARCH)
From: Ilya Leoshkevich @ 2019-07-15 9:11 UTC (permalink / raw)
To: bpf, netdev; +Cc: gor, heiko.carstens, Ilya Leoshkevich
While $ARCH can be relatively flexible (see Makefile and
tools/scripts/Makefile.arch), $SRCARCH always corresponds to a directory
name under arch/.
Therefore, build samples with -D__TARGET_ARCH_$(SRCARCH), since that
matches the expectations of bpf_helpers.h.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Acked-by: Vasily Gorbik <gor@linux.ibm.com>
---
samples/bpf/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index f90daadfbc89..1d9be26b4edd 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -284,7 +284,7 @@ $(obj)/%.o: $(src)/%.c
$(Q)$(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) -I$(obj) \
-I$(srctree)/tools/testing/selftests/bpf/ \
-D__KERNEL__ -D__BPF_TRACING__ -Wno-unused-value -Wno-pointer-sign \
- -D__TARGET_ARCH_$(ARCH) -Wno-compare-distinct-pointer-types \
+ -D__TARGET_ARCH_$(SRCARCH) -Wno-compare-distinct-pointer-types \
-Wno-gnu-variable-sized-type-not-at-end \
-Wno-address-of-packed-member -Wno-tautological-compare \
-Wno-unknown-warning-option $(CLANG_ARCH_ARGS) \
--
2.21.0
^ permalink raw reply related
* Re: [PATCH] ethtool: igb: dump RR2DCDELAY register
From: Michal Kubecek @ 2019-07-15 9:13 UTC (permalink / raw)
To: Artem Bityutskiy; +Cc: John W . Linville, netdev
In-Reply-To: <20190715065228.88377-1-artem.bityutskiy@linux.intel.com>
On Mon, Jul 15, 2019 at 09:52:28AM +0300, Artem Bityutskiy wrote:
> Decode 'RR2DCDELAY' register which Linux kernel provides starting from version
> 5.3. The corresponding commit in the Linux kernel is:
> cd502a7f7c9c igb: add RR2DCDELAY to ethtool registers dump
>
> The RR2DCDELAY register is present in I210 and I211 Intel Gigabit Ethernet
> chips and it stands for "Read Request To Data Completion Delay". Here is how
> this register is described in the I210 datasheet:
>
> "This field captures the maximum PCIe split time in 16 ns units, which is the
> maximum delay between the read request to the first data completion. This is
> giving an estimation of the PCIe round trip time."
>
> In practice, this register can be used to measure the time it takes the NIC to
> read data from the host memory.
>
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
> ---
> igb.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/igb.c b/igb.c
> index e0ccef9..ab0896f 100644
> --- a/igb.c
> +++ b/igb.c
> @@ -859,6 +859,18 @@ igb_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs)
> "0x03430: TDFPC (Tx data FIFO packet count) 0x%08X\n",
> regs_buff[550]);
>
> + /*
> + * Starting from kernel version 5.3 the registers dump buffer grew from
> + * 739 4-byte words to 740 words, and word 740 contains the RR2DCDLAY
nit: "E" missing here: ^
> + * register.
> + */
> + if (regs->len < 740)
> + return 0;
> +
> + fprintf(stdout,
> + "0x05BF4: RR2DCDELAY (Max. DMA read delay) 0x%08X\n",
> + regs_buff[739]);
Showing a delay as hex number doesn't seem very user friendly but that
also applies to many existing registers so it's probably better to be
consistent and perhaps do an overall cleanup later.
Michal
^ permalink raw reply
* Re: [RFC] virtio-net: share receive_*() and add_recvbuf_*() with virtio-vsock
From: Jason Wang @ 2019-07-15 9:16 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Michael S. Tsirkin, Stefan Hajnoczi, virtualization, netdev
In-Reply-To: <20190715074416.a3s2i5ausognotbn@steredhat>
>>>>>>> struct sk_buff *virtskb_receive_small(struct virtskb *vs, ...);
>>>>>>> struct sk_buff *virtskb_receive_big(struct virtskb *vs, ...);
>>>>>>> struct sk_buff *virtskb_receive_mergeable(struct virtskb *vs, ...);
>>>>>>>
>>>>>>> int virtskb_add_recvbuf_small(struct virtskb*vs, ...);
>>>>>>> int virtskb_add_recvbuf_big(struct virtskb *vs, ...);
>>>>>>> int virtskb_add_recvbuf_mergeable(struct virtskb *vs, ...);
>>>>>>>
>>>>>>> For the Guest->Host path it should be easier, so maybe I can add a
>>>>>>> "virtskb_send(struct virtskb *vs, struct sk_buff *skb)" with a part of the code
>>>>>>> of xmit_skb().
>>>>>> I may miss something, but I don't see any thing that prevents us from using
>>>>>> xmit_skb() directly.
>>>>>>
>>>>> Yes, but my initial idea was to make it more parametric and not related to the
>>>>> virtio_net_hdr, so the 'hdr_len' could be a parameter and the
>>>>> 'num_buffers' should be handled by the caller.
>>>>>
>>>>>>> Let me know if you have in mind better names or if I should put these function
>>>>>>> in another place.
>>>>>>>
>>>>>>> I would like to leave the control part completely separate, so, for example,
>>>>>>> the two drivers will negotiate the features independently and they will call
>>>>>>> the right virtskb_receive_*() function based on the negotiation.
>>>>>> If it's one the issue of negotiation, we can simply change the
>>>>>> virtnet_probe() to deal with different devices.
>>>>>>
>>>>>>
>>>>>>> I already started to work on it, but before to do more steps and send an RFC
>>>>>>> patch, I would like to hear your opinion.
>>>>>>> Do you think that makes sense?
>>>>>>> Do you see any issue or a better solution?
>>>>>> I still think we need to seek a way of adding some codes on virtio-net.c
>>>>>> directly if there's no huge different in the processing of TX/RX. That would
>>>>>> save us a lot time.
>>>>> After the reading of the buffers from the virtqueue I think the process
>>>>> is slightly different, because virtio-net will interface with the network
>>>>> stack, while virtio-vsock will interface with the vsock-core (socket).
>>>>> So the virtio-vsock implements the following:
>>>>> - control flow mechanism to avoid to loose packets, informing the peer
>>>>> about the amount of memory available in the receive queue using some
>>>>> fields in the virtio_vsock_hdr
>>>>> - de-multiplexing parsing the virtio_vsock_hdr and choosing the right
>>>>> socket depending on the port
>>>>> - socket state handling
>>
>> I think it's just a branch, for ethernet, go for networking stack. otherwise
>> go for vsock core?
>>
> Yes, that should work.
>
> So, I should refactor the functions that can be called also from the vsock
> core, in order to remove "struct net_device *dev" parameter.
> Maybe creating some wrappers for the network stack.
>
> Otherwise I should create a fake net_device for vsock_core.
>
> What do you suggest?
I'm not quite sure I get the question. Can you just use the one that
created by virtio_net?
Thanks
^ permalink raw reply
* Re: [Intel-wired-lan] [PATCH] e1000e: Make speed detection on hotplugging cable more reliable
From: Kai Heng Feng @ 2019-07-15 9:21 UTC (permalink / raw)
To: Paul Menzel; +Cc: Jeff Kirsher, netdev, intel-wired-lan, linux-kernel
In-Reply-To: <37a1e2af-64c6-4515-5dcc-6051e1192636@molgen.mpg.de>
at 5:06 PM, Paul Menzel <pmenzel@molgen.mpg.de> wrote:
> Dear Kai Heng,
>
>
> (with or without hyphen?)
>
> On 7/15/19 11:00 AM, Kai Heng Feng wrote:
>> at 4:52 PM, Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
>>> On 7/15/19 10:43 AM, Kai-Heng Feng wrote:
>>>> After hotplugging an 1Gbps ethernet cable with 1Gbps link partner, the
>>>> MII_BMSR may reports 10Mbps, renders the network rather slow.
>>>
>>> s/may reports/may report/
>>> s/renders/rendering/
>>
>> Apparently English isn’t my mother tongue ;)
>
> No problem. Mine neither.
>
>>>> The issue has much lower fail rate after commit 59653e6497d1 ("e1000e:
>>>> Make watchdog use delayed work"), which esssentially introduces some
>>>
>>> essentially
>>
>> Ok.
>>
>>>> delay before running the watchdog task.
>>>>
>>>> But there's still a chance that the hotplugging event and the queued
>>>> watchdog task gets run at the same time, then the original issue can be
>>>> observed once again.
>>>>
>>>> So let's use mod_delayed_work() to add a deterministic 1 second delay
>>>> before running watchdog task, after an interrupt.
>>>
>>> I am not clear about the effects for the user. Could you elaborate
>>> please? Does the link now come up up to one second later?
>>
>> Yes, the link will be up on a fixed one second later.
>>
>> The delay varies between 0 to 2 seconds without this patch.
>
> Is there no other fix? Regarding booting a system fast (less than six
> seconds), a fixed one second delay is quite a regression on systems where
> it worked before.
This only affects when ethernet cable is hot plugged.
Kai-Heng
>
>>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>>>
>>> Any bug URL?
>>
>> If maintainers think it’s necessary then I’ll file one.
>
> Not necessary, if there is none. I thought you had one in Launchpad or so.
>
>
> Kind regards,
>
> Paul
^ 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