[PATCH 0/9] added helper macros to remove duplicate code from probe functions of the platform drivers
1. For most of the platform drivers's probe include following steps -memory allocation for driver's private structure -getting io resources -io remapping resources -getting irq number -registering irq -setting driver's private data -getting clock -preparing and enabling clock 2. We have defined a set of macros to combine some or all of the above mentioned steps. This will remove redundant/duplicate code in drivers' probe functions of platform drivers. devm_platform_probe_helper(pdev, priv, clk_name); devm_platform_probe_helper_clk(pdev, priv, clk_name); devm_platform_probe_helper_irq(pdev, priv, clk_name, irq_hndlr, irq_flags, irq_name, irq_devid); devm_platform_probe_helper_all(pdev, priv, clk_name, irq_hndlr, irq_flags, irq_name, irq_devid); devm_platform_probe_helper_all_data(pdev, priv, clk_name, irq_hndlr, irq_flags, irq_name, irq_devid); 3. Code is made devres compatible (wherever required) The functions: clk_get, request_irq, kzalloc, platform_get_resource are replaced with their devm_* counterparts. 4. Few bugs are also fixed. Satendra Singh Thakur (9): probe/dma : added helper macros to remove redundant/duplicate code from probe functions of the dma controller drivers probe/dma/jz4740: removed redundant code from jz4740 dma controller's probe function probe/dma/zx: removed redundant code from zx dma controller's probe function probe/dma/qcom-bam: removed redundant code from qcom bam dma controller's probe function probe/dma/mtk-hs: removed redundant code from mediatek hs dma controller's probe function probe/dma/sun6i: removed redundant code from sun6i dma controller's probe function probe/dma/sun4i: removed redundant code from sun4i dma controller's probe function probe/dma/axi: removed redundant code from axi dma controller's probe function probe/dma/owl: removed redundant code from owl dma controller's probe function drivers/dma/dma-axi-dmac.c | 28 ++--- drivers/dma/dma-jz4740.c | 33 +++--- drivers/dma/mediatek/mtk-hsdma.c | 38 +++ drivers/dma/owl-dma.c| 29 ++--- drivers/dma/qcom/bam_dma.c | 71 +--- drivers/dma/sun4i-dma.c | 30 ++ drivers/dma/sun6i-dma.c | 30 ++ drivers/dma/zx_dma.c | 35 ++ include/linux/probe-helper.h | 179 +++ 9 files changed, 280 insertions(+), 193 deletions(-) create mode 100644 include/linux/probe-helper.h -- 2.17.1
Re: Linux 5.3-rc8
On Sun, Sep 15, 2019 at 08:56:55AM +0200, Lennart Poettering wrote: > There's benefit in being able to wait until the pool is initialized > before we update the random seed stored on disk with a new one, And what exactly makes you think that waiting with arms crossed not doing anything else has any chance to make the situation change if you already had no such entropy available when reaching that first call, especially during early boot ? Willy
Re: Linux 5.3-rc8
On So, 15.09.19 09:01, Willy Tarreau (w...@1wt.eu) wrote: > On Sun, Sep 15, 2019 at 08:56:55AM +0200, Lennart Poettering wrote: > > There's benefit in being able to wait until the pool is initialized > > before we update the random seed stored on disk with a new one, > > And what exactly makes you think that waiting with arms crossed not > doing anything else has any chance to make the situation change if > you already had no such entropy available when reaching that first > call, especially during early boot ? That code can finish 5h after boot, it's entirely fine with this specific usecase. Again: we don't delay "the boot" for this. We just delay "writing a new seed to disk" for this. And if that is 5h later, then that's totally fine, because in the meantime it's just one bg process more that hangs around waiting to do what it needs to do. Lennart -- Lennart Poettering, Berlin
Re: Linux 5.3-rc8
On Sun, Sep 15, 2019 at 09:05:41AM +0200, Lennart Poettering wrote: > On So, 15.09.19 09:01, Willy Tarreau (w...@1wt.eu) wrote: > > > On Sun, Sep 15, 2019 at 08:56:55AM +0200, Lennart Poettering wrote: > > > There's benefit in being able to wait until the pool is initialized > > > before we update the random seed stored on disk with a new one, > > > > And what exactly makes you think that waiting with arms crossed not > > doing anything else has any chance to make the situation change if > > you already had no such entropy available when reaching that first > > call, especially during early boot ? > > That code can finish 5h after boot, it's entirely fine with this > specific usecase. > > Again: we don't delay "the boot" for this. We just delay "writing a > new seed to disk" for this. And if that is 5h later, then that's > totally fine, because in the meantime it's just one bg process more that > hangs around waiting to do what it needs to do. Didn't you say it could also happen when using encrypted swap ? If so I suspect this could happen very early during boot, before any services may be started ? Willy
[PATCH 1/9] probe/dma : added helper macros to remove redundant/duplicate code from probe functions of the dma controller drivers
1. For most of the drivers probe include following steps
a) memory allocation for driver's private structure
b) getting io resources
c) io remapping resources
d) getting clock
e) getting irq number
f) registering irq
g) preparing and enabling clock
i) setting platform's drv data
2. We have defined a set of macros to combine above mentioned
steps.
This will remove redundant/duplicate code in drivers' probe
functions.
3. This macro combines all the steps except f), g) and i).
devm_platform_probe_helper(pdev, priv, clk_name);
4. This macro combines all the steps except f) and i).
devm_platform_probe_helper_clk(pdev, priv, clk_name);
5. This macro combines all the steps except g) and i).
devm_platform_probe_helper_irq(pdev, priv, clk_name,
irq_hndlr, irq_flags, irq_name, irq_devid);
6. This is because, some drivers perform step f) and g)
after hw init or subsys registration at very different points
in the probe function. The step i) is called at the end of
probe function by several drivers; while other drivers call it at
different points in probe function.
7. This macro combines above mentioned steps a) to g).
devm_platform_probe_helper_all(pdev, priv, clk_name,
irq_hndlr, irq_flags, irq_name, irq_devid);
8. This macro combines all of the above mentioned steps a) to i).
devm_platform_probe_helper_all_data(pdev, priv, clk_name,
irq_hndlr, irq_flags, irq_name, irq_devid);
9. Above macros will be useful for wide variety of probe
functions of different drivers.
Signed-off-by: Satendra Singh Thakur
Signed-off-by: Satendra Singh Thakur
---
include/linux/probe-helper.h | 179 +++
1 file changed, 179 insertions(+)
create mode 100644 include/linux/probe-helper.h
diff --git a/include/linux/probe-helper.h b/include/linux/probe-helper.h
new file mode 100644
index ..7baa468509e3
--- /dev/null
+++ b/include/linux/probe-helper.h
@@ -0,0 +1,179 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ *
+ * probe_helper.h - helper functions for platform drivers' probe
+ * function
+ * Author: Satendra Singh Thakur Sep 2019
+ *
+ */
+#ifndef _PROBE_HELPER_H_
+#define _PROBE_HELPER_H_
+
+#include
+#include
+
+/* devm_platform_probe_helper - Macro for helping probe method
+ * of platform drivers
+ * This macro combines the functions:
+ * devm_kzalloc, platform_get_resource, devm_ioremap_resource,
+ * devm_clk_get, platform_get_irq
+ * @pdev platform device
+ * @priv driver's private object for memory allocation
+ * @clk_name clock name as in DT
+ */
+#define devm_platform_probe_helper(pdev, priv, clk_name) \
+({ \
+ __label__ __out;\
+ int __ret = 0; \
+ priv = devm_kzalloc(&(pdev)->dev, sizeof(*priv), GFP_KERNEL); \
+ if (!(priv)) { \
+ dev_err(&(pdev)->dev, "devm_kzalloc failed\n"); \
+ __ret = -ENOMEM;\
+ goto __out; \
+ } \
+ (priv)->base = devm_platform_ioremap_resource(pdev, 0); \
+ if (IS_ERR((priv)->base)) { \
+ dev_err(&(pdev)->dev, \
+ "devm_platform_ioremap_resource failed\n"); \
+ __ret = PTR_ERR((priv)->base); \
+ goto __out; \
+ } \
+ (priv)->clk = devm_clk_get(&(pdev)->dev, clk_name); \
+ if (IS_ERR((priv)->clk)) { \
+ dev_err(&(pdev)->dev, "devm_clk_get failed\n"); \
+ __ret = PTR_ERR((priv)->clk); \
+ goto __out; \
+ } \
+ (priv)->irq = platform_get_irq(pdev, 0);\
+ if ((priv)->irq < 0) { \
+ dev_err(&(pdev)->dev, "platform_get_irq failed\n"); \
+ __ret = (priv)->irq;\
+ goto __out; \
+ } \
+__out: \
+ __ret; \
+})
+
+/* devm_platform_probe_helper_irq - Macro for helping probe method
+ * of platform drivers
+ * This macro combines the functions:
+ * devm_kzalloc, platform_get_resource, devm_ioremap_resource,
+ * devm_clk_get, platform_get_irq, devm_request_irq
+ * @pdev platform device
+ * @priv driver's private object for memory allocation
+ * @clk_name clock name as in DT
+ * @irq_hndlr interrupt handler function (isr)
+ * @irq_flags flags for interrupt registration
+ * @irq_name name of the interrupt handler
+ * @irq_devid device identifier for irq
+ */
+#define devm_platform_probe_helper_irq(pdev, priv, clk_name, \
+ irq_hndlr, irq_flags, irq_name, irq_devid) \
+({ \
+ __label__ __out;\
+ int __ret = 0; \
+ __ret = devm_platform_probe_helper(pdev, priv, clk_name); \
+ if (__ret < 0) \
+ goto __out; \
+ __ret = devm_request_irq(&(pdev)->dev, (priv)->irq, irq_hndlr, \
+ irq_flags, irq_name, irq_devid);\
+ if (__ret < 0) {\
+ dev_err(&(pdev)->dev, \
+
Re: Linux 5.3-rc8
On Sun, Sep 15, 2019 at 08:51:42AM +0200, Lennart Poettering wrote: > On Sa, 14.09.19 09:30, Linus Torvalds (torva...@linux-foundation.org) wrote: [...] > > And please don't break /dev/urandom again. The above code is the ony > way I see how we can make /dev/urandom-derived swap encryption safe, > and the only way I can see how we can sanely write a valid random seed > to disk after boot. > Any hope in making systemd-random-seed(8) credit that "random seed from previous boot" file, through RNDADDENTROPY, *by default*? Because of course this makes the problem reliably go away on my system too (as discussed in the original bug report, but you were not CCed). I know that by v243, just released 12 days ago, this can be optionally done through SYSTEMD_RANDOM_SEED_CREDIT=1. I wonder though if it can ever be done by default, just like what the BSDs does... This would solve a big part of the current problem. > Lennart thanks, -- darwi http://darwish.chasingpointers.com
[PATCH 2/9] probe/dma/jz4740: removed redundant code from jz4740 dma controller's probe function
1. In order to remove duplicate code, following functions:
devm_kzalloc
platform_get_resource
devm_ioremap_resource
clk_get
clk_prepare_enable
platform_get_irq
are replaced with a macro devm_platform_probe_helper_clk.
2. Added irq field in the struct jz4740_dma_dev.
Removed platform_get_irq from remove method.
3. This patch depends on the file include/linux/probe-helper.h
which is pushed in previous patch [01/09].
Signed-off-by: Satendra Singh Thakur
Signed-off-by: Satendra Singh Thakur
---
drivers/dma/dma-jz4740.c | 33 +
1 file changed, 13 insertions(+), 20 deletions(-)
diff --git a/drivers/dma/dma-jz4740.c b/drivers/dma/dma-jz4740.c
index 39c676c47082..b012896d02bb 100644
--- a/drivers/dma/dma-jz4740.c
+++ b/drivers/dma/dma-jz4740.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include "virt-dma.h"
@@ -121,6 +122,7 @@ struct jz4740_dma_dev {
struct dma_device ddev;
void __iomem *base;
struct clk *clk;
+ int irq;
struct jz4740_dmaengine_chan chan[JZ_DMA_NR_CHANS];
};
@@ -519,27 +521,19 @@ static int jz4740_dma_probe(struct platform_device *pdev)
struct jz4740_dma_dev *dmadev;
struct dma_device *dd;
unsigned int i;
- struct resource *res;
int ret;
- int irq;
- dmadev = devm_kzalloc(&pdev->dev, sizeof(*dmadev), GFP_KERNEL);
- if (!dmadev)
- return -EINVAL;
+ /*
+* This macro internally combines following functions:
+* devm_kzalloc, platform_get_resource, devm_ioremap_resource,
+* devm_clk_get, platform_get_irq, clk_prepare_enable
+*/
+ ret = devm_platform_probe_helper_clk(pdev, dmadev, "dma");
+ if (ret < 0)
+ return ret;
dd = &dmadev->ddev;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- dmadev->base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(dmadev->base))
- return PTR_ERR(dmadev->base);
-
- dmadev->clk = clk_get(&pdev->dev, "dma");
- if (IS_ERR(dmadev->clk))
- return PTR_ERR(dmadev->clk);
-
- clk_prepare_enable(dmadev->clk);
-
dma_cap_set(DMA_SLAVE, dd->cap_mask);
dma_cap_set(DMA_CYCLIC, dd->cap_mask);
dd->device_free_chan_resources = jz4740_dma_free_chan_resources;
@@ -567,8 +561,8 @@ static int jz4740_dma_probe(struct platform_device *pdev)
if (ret)
goto err_clk;
- irq = platform_get_irq(pdev, 0);
- ret = request_irq(irq, jz4740_dma_irq, 0, dev_name(&pdev->dev), dmadev);
+ ret = request_irq(dmadev->irq, jz4740_dma_irq, 0,
+ dev_name(&pdev->dev), dmadev);
if (ret)
goto err_unregister;
@@ -598,9 +592,8 @@ static void jz4740_cleanup_vchan(struct dma_device *dmadev)
static int jz4740_dma_remove(struct platform_device *pdev)
{
struct jz4740_dma_dev *dmadev = platform_get_drvdata(pdev);
- int irq = platform_get_irq(pdev, 0);
- free_irq(irq, dmadev);
+ free_irq(dmadev->irq, dmadev);
jz4740_cleanup_vchan(&dmadev->ddev);
dma_async_device_unregister(&dmadev->ddev);
--
2.17.1
[PATCH 3/9] probe/dma/zx: removed redundant code from zx dma controller's probe function
1. In order to remove duplicate code, following functions:
platform_get_resource
devm_kzalloc
devm_ioremap_resource
devm_clk_get
platform_get_irq
devm_request_irq
are replaced with a macro devm_platform_probe_helper_irq.
2. Removed dmam_pool_destroy from remove method as dmam_pool_create
is already used in probe function.
3. This patch depends on the file include/linux/probe-helper.h
which is pushed in previous patch [01/09].
Signed-off-by: Satendra Singh Thakur
Signed-off-by: Satendra Singh Thakur
---
drivers/dma/zx_dma.c | 35 ++-
1 file changed, 10 insertions(+), 25 deletions(-)
diff --git a/drivers/dma/zx_dma.c b/drivers/dma/zx_dma.c
index 9f4436f7c914..d8c2fbe9766c 100644
--- a/drivers/dma/zx_dma.c
+++ b/drivers/dma/zx_dma.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include "virt-dma.h"
@@ -754,20 +755,17 @@ static struct dma_chan *zx_of_dma_simple_xlate(struct
of_phandle_args *dma_spec,
static int zx_dma_probe(struct platform_device *op)
{
struct zx_dma_dev *d;
- struct resource *iores;
int i, ret = 0;
- iores = platform_get_resource(op, IORESOURCE_MEM, 0);
- if (!iores)
- return -EINVAL;
-
- d = devm_kzalloc(&op->dev, sizeof(*d), GFP_KERNEL);
- if (!d)
- return -ENOMEM;
-
- d->base = devm_ioremap_resource(&op->dev, iores);
- if (IS_ERR(d->base))
- return PTR_ERR(d->base);
+ /*
+* This macro internally combines following functions:
+* devm_kzalloc, platform_get_resource, devm_ioremap_resource,
+* devm_clk_get, platform_get_irq, devm_request_irq,
+*/
+ ret = devm_platform_probe_helper_irq(op, d, NULL,
+ zx_dma_int_handler, 0, DRIVER_NAME, d);
+ if (ret < 0)
+ return ret;
of_property_read_u32((&op->dev)->of_node,
"dma-channels", &d->dma_channels);
@@ -776,18 +774,6 @@ static int zx_dma_probe(struct platform_device *op)
if (!d->dma_requests || !d->dma_channels)
return -EINVAL;
- d->clk = devm_clk_get(&op->dev, NULL);
- if (IS_ERR(d->clk)) {
- dev_err(&op->dev, "no dma clk\n");
- return PTR_ERR(d->clk);
- }
-
- d->irq = platform_get_irq(op, 0);
- ret = devm_request_irq(&op->dev, d->irq, zx_dma_int_handler,
- 0, DRIVER_NAME, d);
- if (ret)
- return ret;
-
/* A DMA memory pool for LLIs, align on 32-byte boundary */
d->pool = dmam_pool_create(DRIVER_NAME, &op->dev,
LLI_BLOCK_SIZE, 32, 0);
@@ -894,7 +880,6 @@ static int zx_dma_remove(struct platform_device *op)
list_del(&c->vc.chan.device_node);
}
clk_disable_unprepare(d->clk);
- dmam_pool_destroy(d->pool);
return 0;
}
--
2.17.1
[PATCH 5/9] probe/dma/mtk-hs: removed redundant code from mediatek hs dma controller's probe function
1. In order to remove duplicate code, following functions:
platform_get_resource
devm_kzalloc
devm_ioremap_resource
devm_clk_get
platform_get_irq
are replaced with a macro devm_platform_probe_helper.
2. Fixed a memory leak when devm_request_irq fails,
Called of_dma_controller_free in such case.
3. This patch depends on the file include/linux/probe-helper.h
which is pushed in previous patch [01/09].
Signed-off-by: Satendra Singh Thakur
Signed-off-by: Satendra Singh Thakur
---
drivers/dma/mediatek/mtk-hsdma.c | 38 ++--
1 file changed, 12 insertions(+), 26 deletions(-)
diff --git a/drivers/dma/mediatek/mtk-hsdma.c b/drivers/dma/mediatek/mtk-hsdma.c
index 1a2028e1c29e..6fc01093aeea 100644
--- a/drivers/dma/mediatek/mtk-hsdma.c
+++ b/drivers/dma/mediatek/mtk-hsdma.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include "../virt-dma.h"
@@ -896,41 +897,24 @@ static int mtk_hsdma_probe(struct platform_device *pdev)
struct mtk_hsdma_device *hsdma;
struct mtk_hsdma_vchan *vc;
struct dma_device *dd;
- struct resource *res;
int i, err;
- hsdma = devm_kzalloc(&pdev->dev, sizeof(*hsdma), GFP_KERNEL);
- if (!hsdma)
- return -ENOMEM;
-
+ /*
+* This macro internally combines following functions:
+* devm_kzalloc, platform_get_resource, devm_ioremap_resource,
+* devm_clk_get, platform_get_irq
+*/
+ err = devm_platform_probe_helper(pdev, hsdma, "hsdma");
+ if (err < 0)
+ return err;
dd = &hsdma->ddev;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- hsdma->base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(hsdma->base))
- return PTR_ERR(hsdma->base);
-
hsdma->soc = of_device_get_match_data(&pdev->dev);
if (!hsdma->soc) {
dev_err(&pdev->dev, "No device match found\n");
return -ENODEV;
}
- hsdma->clk = devm_clk_get(&pdev->dev, "hsdma");
- if (IS_ERR(hsdma->clk)) {
- dev_err(&pdev->dev, "No clock for %s\n",
- dev_name(&pdev->dev));
- return PTR_ERR(hsdma->clk);
- }
-
- res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
- if (!res) {
- dev_err(&pdev->dev, "No irq resource for %s\n",
- dev_name(&pdev->dev));
- return -EINVAL;
- }
- hsdma->irq = res->start;
-
refcount_set(&hsdma->pc_refcnt, 0);
spin_lock_init(&hsdma->lock);
@@ -997,7 +981,7 @@ static int mtk_hsdma_probe(struct platform_device *pdev)
if (err) {
dev_err(&pdev->dev,
"request_irq failed with err %d\n", err);
- goto err_unregister;
+ goto err_free;
}
platform_set_drvdata(pdev, hsdma);
@@ -1006,6 +990,8 @@ static int mtk_hsdma_probe(struct platform_device *pdev)
return 0;
+err_free:
+ of_dma_controller_free(pdev->dev.of_node);
err_unregister:
dma_async_device_unregister(dd);
--
2.17.1
[PATCH 4/9] probe/dma/qcom-bam: removed redundant code from qcom bam dma controller's probe function
1. In order to remove duplicate code, following functions:
platform_get_resource
devm_kzalloc
devm_ioremap_resource
devm_clk_get
platform_get_irq
clk_prepare_enable
are replaced with a macro devm_platform_probe_helper_clk.
2. Renamed variables regs and bamclk so that helper macro can
be applied.
3. This patch depends on the file include/linux/probe-helper.h
which is pushed in previous patch [01/09].
Signed-off-by: Satendra Singh Thakur
Signed-off-by: Satendra Singh Thakur
---
drivers/dma/qcom/bam_dma.c | 71 --
1 file changed, 29 insertions(+), 42 deletions(-)
diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 8e90a405939d..06c136ca8e40 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -41,6 +41,7 @@
#include
#include
#include
+#include
#include "../dmaengine.h"
#include "../virt-dma.h"
@@ -378,7 +379,7 @@ static inline struct bam_chan *to_bam_chan(struct dma_chan
*common)
}
struct bam_device {
- void __iomem *regs;
+ void __iomem *base;
struct device *dev;
struct dma_device common;
struct device_dma_parameters dma_parms;
@@ -392,7 +393,7 @@ struct bam_device {
const struct reg_offset_data *layout;
- struct clk *bamclk;
+ struct clk *clk;
int irq;
/* dma start transaction tasklet */
@@ -410,7 +411,7 @@ static inline void __iomem *bam_addr(struct bam_device
*bdev, u32 pipe,
{
const struct reg_offset_data r = bdev->layout[reg];
- return bdev->regs + r.base_offset +
+ return bdev->base + r.base_offset +
r.pipe_mult * pipe +
r.evnt_mult * pipe +
r.ee_mult * bdev->ee;
@@ -1209,41 +1210,41 @@ static int bam_dma_probe(struct platform_device *pdev)
{
struct bam_device *bdev;
const struct of_device_id *match;
- struct resource *iores;
int ret, i;
-
- bdev = devm_kzalloc(&pdev->dev, sizeof(*bdev), GFP_KERNEL);
- if (!bdev)
- return -ENOMEM;
+ /*
+* This macro internally combines following functions:
+* devm_kzalloc, platform_get_resource, devm_ioremap_resource,
+* devm_clk_get, platform_get_irq, clk_prepare_enable
+*/
+ ret = devm_platform_probe_helper_clk(pdev, bdev, "bam_clk");
+ bdev->controlled_remotely = of_property_read_bool(pdev->dev.of_node,
+ "qcom,controlled-remotely");
+ if (ret < 0) {
+ if (IS_ERR(bdev->clk)) {
+ if (!bdev->controlled_remotely)
+ return ret;
+ bdev->clk = NULL;
+ } else
+ return ret;
+ }
bdev->dev = &pdev->dev;
match = of_match_node(bam_of_match, pdev->dev.of_node);
if (!match) {
dev_err(&pdev->dev, "Unsupported BAM module\n");
- return -ENODEV;
+ ret = -ENODEV;
+ goto err_disable_clk;
}
bdev->layout = match->data;
- iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- bdev->regs = devm_ioremap_resource(&pdev->dev, iores);
- if (IS_ERR(bdev->regs))
- return PTR_ERR(bdev->regs);
-
- bdev->irq = platform_get_irq(pdev, 0);
- if (bdev->irq < 0)
- return bdev->irq;
-
ret = of_property_read_u32(pdev->dev.of_node, "qcom,ee", &bdev->ee);
if (ret) {
dev_err(bdev->dev, "Execution environment unspecified\n");
- return ret;
+ goto err_disable_clk;
}
- bdev->controlled_remotely = of_property_read_bool(pdev->dev.of_node,
- "qcom,controlled-remotely");
-
if (bdev->controlled_remotely) {
ret = of_property_read_u32(pdev->dev.of_node, "num-channels",
&bdev->num_channels);
@@ -1256,20 +1257,6 @@ static int bam_dma_probe(struct platform_device *pdev)
dev_err(bdev->dev, "num-ees unspecified in dt\n");
}
- bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");
- if (IS_ERR(bdev->bamclk)) {
- if (!bdev->controlled_remotely)
- return PTR_ERR(bdev->bamclk);
-
- bdev->bamclk = NULL;
- }
-
- ret = clk_prepare_enable(bdev->bamclk);
- if (ret) {
- dev_err(bdev->dev, "failed to prepare/enable clock\n");
- return ret;
- }
-
ret = bam_init(bdev);
if (ret)
goto err_disable_clk;
@@ -1359,7 +1346,7 @@ static int bam_dma_probe(struct platform_device *pdev)
err_tasklet_kill:
tasklet_kill(&bdev->task);
err_disable_clk:
- clk_disable_unprepare(bdev->bamclk);
+ clk_disable_unprepare(bdev->clk);
return ret;
}
@@ -1393,7 +1380,7 @@ sta
[PATCH 6/9] probe/dma/sun6i: removed redundant code from sun6i dma controller's probe function
1. In order to remove duplicate code, following functions:
platform_get_resource
devm_kzalloc
devm_ioremap_resource
devm_clk_get
platform_get_irq
are replaced with a macro devm_platform_probe_helper.
2. This patch depends on the file include/linux/probe-helper.h
which is pushed in previous patch [01/09].
Signed-off-by: Satendra Singh Thakur
Signed-off-by: Satendra Singh Thakur
---
drivers/dma/sun6i-dma.c | 30 +-
1 file changed, 9 insertions(+), 21 deletions(-)
diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index ed5b68dcfe50..41ee054bbeeb 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include "virt-dma.h"
@@ -1234,34 +1235,21 @@ static int sun6i_dma_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
struct sun6i_dma_dev *sdc;
- struct resource *res;
int ret, i;
- sdc = devm_kzalloc(&pdev->dev, sizeof(*sdc), GFP_KERNEL);
- if (!sdc)
- return -ENOMEM;
+ /*
+* This macro internally combines following functions:
+* devm_kzalloc, platform_get_resource, devm_ioremap_resource,
+* devm_clk_get, platform_get_irq
+*/
+ ret = devm_platform_probe_helper(pdev, sdc, NULL);
+ if (ret < 0)
+ return ret;
sdc->cfg = of_device_get_match_data(&pdev->dev);
if (!sdc->cfg)
return -ENODEV;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- sdc->base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(sdc->base))
- return PTR_ERR(sdc->base);
-
- sdc->irq = platform_get_irq(pdev, 0);
- if (sdc->irq < 0) {
- dev_err(&pdev->dev, "Cannot claim IRQ\n");
- return sdc->irq;
- }
-
- sdc->clk = devm_clk_get(&pdev->dev, NULL);
- if (IS_ERR(sdc->clk)) {
- dev_err(&pdev->dev, "No clock specified\n");
- return PTR_ERR(sdc->clk);
- }
-
if (sdc->cfg->has_mbus_clk) {
sdc->clk_mbus = devm_clk_get(&pdev->dev, "mbus");
if (IS_ERR(sdc->clk_mbus)) {
--
2.17.1
[PATCH 7/9] probe/dma/sun4i: removed redundant code from sun4i dma controller's probe function
1. In order to remove duplicate code, following functions:
platform_get_resource
devm_kzalloc
devm_ioremap_resource
devm_clk_get
platform_get_irq
are replaced with a macro devm_platform_probe_helper.
2. This patch depends on the file include/linux/probe-helper.h
which is pushed in previous patch [01/09].
Signed-off-by: Satendra Singh Thakur
Signed-off-by: Satendra Singh Thakur
---
drivers/dma/sun4i-dma.c | 30 +-
1 file changed, 9 insertions(+), 21 deletions(-)
diff --git a/drivers/dma/sun4i-dma.c b/drivers/dma/sun4i-dma.c
index 1f80568b2613..5db139ff43ac 100644
--- a/drivers/dma/sun4i-dma.c
+++ b/drivers/dma/sun4i-dma.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include "virt-dma.h"
@@ -1119,29 +1120,16 @@ static irqreturn_t sun4i_dma_interrupt(int irq, void
*dev_id)
static int sun4i_dma_probe(struct platform_device *pdev)
{
struct sun4i_dma_dev *priv;
- struct resource *res;
int i, j, ret;
- priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
- if (!priv)
- return -ENOMEM;
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- priv->base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(priv->base))
- return PTR_ERR(priv->base);
-
- priv->irq = platform_get_irq(pdev, 0);
- if (priv->irq < 0) {
- dev_err(&pdev->dev, "Cannot claim IRQ\n");
- return priv->irq;
- }
-
- priv->clk = devm_clk_get(&pdev->dev, NULL);
- if (IS_ERR(priv->clk)) {
- dev_err(&pdev->dev, "No clock specified\n");
- return PTR_ERR(priv->clk);
- }
+ /*
+* This macro internally combines following functions:
+* devm_kzalloc, platform_get_resource, devm_ioremap_resource,
+* devm_clk_get, platform_get_irq
+*/
+ ret = devm_platform_probe_helper(pdev, priv, NULL);
+ if (ret < 0)
+ return ret;
platform_set_drvdata(pdev, priv);
spin_lock_init(&priv->lock);
--
2.17.1
[PATCH 9/9] probe/dma/owl: removed redundant code from owl dma controller's probe function
1. In order to remove duplicate code, following functions:
platform_get_resource
devm_kzalloc
devm_ioremap_resource
devm_clk_get
platform_get_irq
are replaced with a macro devm_platform_probe_helper.
2. This patch depends on the file include/linux/probe-helper.h
which is pushed in previous patch [01/09].
Signed-off-by: Satendra Singh Thakur
Signed-off-by: Satendra Singh Thakur
---
drivers/dma/owl-dma.c | 29 +
1 file changed, 9 insertions(+), 20 deletions(-)
diff --git a/drivers/dma/owl-dma.c b/drivers/dma/owl-dma.c
index 90bbcef99ef8..03e692fc25a1 100644
--- a/drivers/dma/owl-dma.c
+++ b/drivers/dma/owl-dma.c
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include "virt-dma.h"
#define OWL_DMA_FRAME_MAX_LENGTH 0xf
@@ -1045,20 +1046,15 @@ static int owl_dma_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
struct owl_dma *od;
- struct resource *res;
int ret, i, nr_channels, nr_requests;
-
- od = devm_kzalloc(&pdev->dev, sizeof(*od), GFP_KERNEL);
- if (!od)
- return -ENOMEM;
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res)
- return -EINVAL;
-
- od->base = devm_ioremap_resource(&pdev->dev, res);
- if (IS_ERR(od->base))
- return PTR_ERR(od->base);
+ /*
+* This macro internally combines following functions:
+* devm_kzalloc, platform_get_resource, devm_ioremap_resource,
+* devm_clk_get, platform_get_irq
+*/
+ ret = devm_platform_probe_helper(pdev, od, NULL);
+ if (ret < 0)
+ return ret;
ret = of_property_read_u32(np, "dma-channels", &nr_channels);
if (ret) {
@@ -1105,18 +1101,11 @@ static int owl_dma_probe(struct platform_device *pdev)
INIT_LIST_HEAD(&od->dma.channels);
- od->clk = devm_clk_get(&pdev->dev, NULL);
- if (IS_ERR(od->clk)) {
- dev_err(&pdev->dev, "unable to get clock\n");
- return PTR_ERR(od->clk);
- }
-
/*
* Eventhough the DMA controller is capable of generating 4
* IRQ's for DMA priority feature, we only use 1 IRQ for
* simplification.
*/
- od->irq = platform_get_irq(pdev, 0);
ret = devm_request_irq(&pdev->dev, od->irq, owl_dma_interrupt, 0,
dev_name(&pdev->dev), od);
if (ret) {
--
2.17.1
[PATCH 8/9] probe/dma/axi: removed redundant code from axi dma controller's probe function
1. In order to remove duplicate code, following functions: platform_get_resource devm_kzalloc devm_ioremap_resource devm_clk_get platform_get_irq clk_prepare_enable are replaced with a macro devm_platform_probe_helper. 2. This patch depends on the file include/linux/probe-helper.h which is pushed in previous patch [01/09]. Signed-off-by: Satendra Singh Thakur Signed-off-by: Satendra Singh Thakur --- drivers/dma/dma-axi-dmac.c | 28 ++-- 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c index a0ee404b736e..ac8a2355b299 100644 --- a/drivers/dma/dma-axi-dmac.c +++ b/drivers/dma/dma-axi-dmac.c @@ -21,6 +21,7 @@ #include #include #include +#include #include @@ -829,28 +830,19 @@ static int axi_dmac_probe(struct platform_device *pdev) struct device_node *of_channels, *of_chan; struct dma_device *dma_dev; struct axi_dmac *dmac; - struct resource *res; int ret; - dmac = devm_kzalloc(&pdev->dev, sizeof(*dmac), GFP_KERNEL); - if (!dmac) - return -ENOMEM; - - dmac->irq = platform_get_irq(pdev, 0); - if (dmac->irq < 0) - return dmac->irq; - if (dmac->irq == 0) + /* +* This macro internally combines following functions: +* devm_kzalloc, platform_get_resource, devm_ioremap_resource, +* devm_clk_get, platform_get_irq +*/ + ret = devm_platform_probe_helper(pdev, dmac, NULL); + if (ret < 0) + return ret; + else if (!dmac->irq) return -EINVAL; - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - dmac->base = devm_ioremap_resource(&pdev->dev, res); - if (IS_ERR(dmac->base)) - return PTR_ERR(dmac->base); - - dmac->clk = devm_clk_get(&pdev->dev, NULL); - if (IS_ERR(dmac->clk)) - return PTR_ERR(dmac->clk); - INIT_LIST_HEAD(&dmac->chan.active_descs); of_channels = of_get_child_by_name(pdev->dev.of_node, "adi,channels"); -- 2.17.1
Re: [PATCH 0/2] pseries/hotplug: Change the default behaviour of cede_offline
Hello Nathan, On Thu, Sep 12, 2019 at 10:39:45AM -0500, Nathan Lynch wrote: > "Gautham R. Shenoy" writes: > > The patchset also defines a new sysfs attribute > > "/sys/device/system/cpu/cede_offline_enabled" on PSeries Linux guests > > to allow userspace programs to change the state into which the > > offlined CPU need to be put to at runtime. > > A boolean sysfs interface will become awkward if we need to add another > mode in the future. > > What do you think about naming the attribute something like > 'offline_mode', with the possible values 'extended-cede' and > 'rtas-stopped'? We can do that. However, IMHO in the longer term, on PSeries guests, we should have only one offline state - rtas-stopped. The reason for this being, that on Linux, SMT switch is brought into effect through the CPU Hotplug interface. The only state in which the SMT switch will recognized by the hypervisors such as PHYP is rtas-stopped. All other states (such as extended-cede) should in the long-term be exposed via the cpuidle interface. With this in mind, I made the sysfs interface boolean to mirror the current "cede_offline" commandline parameter. Eventually when we have only one offline-state, we can deprecate the commandline parameter as well as the sysfs interface. Thoughts? -- Thanks and Regards gautham.
Fwd: RRurgent
-- Forwarded message -- From: Date: Sun, 15 Sep 2019 00:49:57 -0700 Subject: RRurgent To: TTHANKS.docx Description: MS-Word 2007 document
Re: pivot_root(".", ".") and the fchdir() dance
Hello Eric,
On 9/11/19 1:06 AM, Eric W. Biederman wrote:
> "Michael Kerrisk (man-pages)" writes:
>
>> Hello Christian,
>>
All: I plan to add the following text to the manual page:
new_root and put_old may be the same directory. In particular,
the following sequence allows a pivot-root operation without need‐
ing to create and remove a temporary directory:
chdir(new_root);
pivot_root(".", ".");
umount2(".", MNT_DETACH);
>>>
>>> Hm, should we mention that MS_PRIVATE or MS_SLAVE is usually needed
>>> before the umount2()? Especially for the container case... I think we
>>> discussed this briefly yesterday in person.
>> Thanks for noticing. That detail (more precisely: not MS_SHARED) is
>> already covered in the numerous other changes that I have pending
>> for this page:
>>
>>The following restrictions apply:
>>...
>>- The propagation type of new_root and its parent mount must not
>> be MS_SHARED; similarly, if put_old is an existing mount point,
>> its propagation type must not be MS_SHARED.
>
> Ugh. That is close but not quite correct.
>
> A better explanation:
>
> The pivot_root system call will never propagate any changes it makes.
> The pivot_root system call ensures this is safe by verifying that
> none of put_old, the parent of new_root, and parent of the root directory
> have a propagation type of MS_SHARED.
Thanks for that. However, another question. You text has two changes.
First, I understand why you reword the discussion to indicate the
_purpose_ of the rules. However, you also, AFAICS, list a different set of
of directories that can't be MS_SHARED:
I said: new_root, the parent of new_root, and put_old
You said: the parent of new_root, and put_old, and parent of the
root directory.
Was I wrong on this detail also?
> The concern from our conversation at the container mini-summit was that
> there is a pathology if in your initial mount namespace all of the
> mounts are marked MS_SHARED like systemd does (and is almost necessary
> if you are going to use mount propagation), that if new_root itself
> is MS_SHARED then unmounting the old_root could propagate.
>
> So I believe the desired sequence is:
>
chdir(new_root);
> +++mount("", ".", MS_SLAVE | MS_REC, NULL);
pivot_root(".", ".");
umount2(".", MNT_DETACH);
>
> The change to new new_root could be either MS_SLAVE or MS_PRIVATE. So
> long as it is not MS_SHARED the mount won't propagate back to the
> parent mount namespace.
Thanks. I made that change.
Cheers,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
[PATCH RFC v3] random: getrandom(2): optionally block when CRNG is uninitialized
Since Linux v3.17, getrandom() has been created as a new and more
secure interface for pseudorandom data requests. It attempted to solve
three problems as compared to /dev/urandom:
1. the need to access filesystem paths, which can fail, e.g. under a
chroot
2. the need to open a file descriptor, which can fail under file
descriptor exhaustion attacks
3. the possibility to get not-so-random data from /dev/urandom, due to
an incompletely initialized kernel entropy pool
To solve the third problem, getrandom(2) was made to block until a
proper amount of entropy has been accumulated. This basically made the
system call have no guaranteed upper-bound for its waiting time.
As was said in c6e9d6f38894 (random: introduce getrandom(2) system
call): "Any userspace program which uses this new functionality must
take care to assure that if it is used during the boot process, that it
will not cause the init scripts or other portions of the system startup
to hang indefinitely."
Meanwhile, user-facing Linux documentation, e.g. the urandom(4) and
getrandom(2) manpages, didn't add such explicit warnings. It didn't
also help that glibc, since v2.25, implemented an "OpenBSD-like"
getentropy(3) in terms of getrandom(2). OpenBSD getentropy(2) never
blocked though, while linux-glibc version did, possibly indefinitely.
Since that glibc change, even more applications at the boot-path began
to implicitly reques randomness through getrandom(2); e.g., for an
Xorg/Xwayland MIT cookie.
OpenBSD genentropy(2) never blocked because, as stated in its rnd(4)
manpages, it saves entropy to disk on shutdown and restores it on boot.
Moreover, the NetBSD bootloader, as shown in its boot(8), even have
special commands to load a random seed file and pass it to the kernel.
Meanwhile on a Linux systemd userland, systemd-random-seed(8) preserved
a random seed across reboots at /var/lib/systemd/random-seed, but it
never had the actual code, until very recently at v243, to ask the
kernel to credit such entropy through an RNDADDENTROPY ioctl.
>From a mix of the above factors, it began to be common for Embedded
Linux systems to "get stuck at boot" unless a daemon like haveged is
installed, or the BSP provider enabling the necessary hwrng driver in
question and crediting its entropy; e.g. 62f95ae805fa (hwrng: omap - Set
default quality). Over time, the issue began to even creep into
consumer-level x86 laptops: mainstream distributions, like debian
buster, began to recommend installing haveged as a workaround.
Thus, on certain setups where there is no hwrng (embedded systems or VMs
on a host lacking virtio-rng), or the hwrng is not trusted by some users
(intel RDRAND), or sometimes it's just broken (amd RDRAND), the system
boot can be *reliably* blocked.
It can therefore be argued that there is no way to use getrandom() on
Linux correctly, especially from shared libraries: GRND_NONBLOCK has
to be used, and a fallback to some other interface like /dev/urandom
is required, thus making the net result no better than just using
/dev/urandom unconditionally.
The issue is further exaggerated by recent file-system optimizations,
e.g. b03755ad6f33 (ext4: make __ext4_get_inode_loc plug), which merges
directory lookup code inode table IO, and thus minimizes the number of
disk interrupts and entropy during boot. After that commit, a blocked
boot can be reliably reproduced on a Thinkpad E480 laptop with
standard ArchLinux user-space.
Thus, don't trust user-space on calling getrandom(2) from the right
context. Never block, by default, and just return data from the
urandom source if entropy is not yet available. This is an explicit
decision not to let user-space work around this through busy loops on
error-codes.
Note: this lowers the quality of random data returned by getrandom(2)
to the level of randomness returned by /dev/urandom, with all the
original security implications coming out of that, as discussed in
problem "3." at the top of this commit log. If this is not desirable,
offer users a fallback to old behavior, by CONFIG_RANDOM_BLOCK=y, or
random.getrandom_block=true bootparam.
[ty...@mit.edu: make the change to a non-blocking getrandom(2) optional]
Link: https://lkml.kernel.org/r/20190914222432.gc19...@mit.edu
Link: https://lkml.kernel.org/r/20190911173624.gi2...@mit.edu
Link: https://factorable.net ("Widespread Weak Keys in Network Devices")
Suggested-by: Linus Torvalds
Link:
https://lkml.kernel.org/r/CAHk-=wjyH910+JRBdZf_Y9G54c1M=lbf8nkxb6vjcm9xjln...@mail.gmail.com
Rreported-by: Ahmed S. Darwish
Link: https://lkml.kernel.org/r/20190912034421.GA2085@darwi-home-pc
Signed-off-by: Ahmed S. Darwish
---
Notes:
changelog-v2:
- tytso: make blocking optional
changelog-v3:
- more detailed commit log + historical context (thanks patrakov)
- remove WARN_ON_ONCE. It's pretty excessive, and the first caller
is systemd-random-seed(8), which we know it will not change.
Just print errors i
Re: Re: Re: [PATCH] sched: fix migration to invalid cpu in __set_cpus_allowed_ptr
On 15/09/2019 6:13,shikemeng wrote:
>>> From 089dbf0216628ac6ae98742ab90725ca9c2bf201 Mon Sep 17 00:00:00 2001
>>> From:
>>> Date: Tue, 10 Sep 2019 09:44:58 -0400
>>> Subject: [PATCH] sched: fix migration to invalid cpu in
>>> __set_cpus_allowed_ptr
>>>
>>> reason: migration to invalid cpu in __set_cpus_allowed_ptr archive
>>> path: patches/euleros/sched
>>>
>>
>>The above should probably be trimmed from the log.
>>
>
>Thanks for reminding me. I will remove this part in a new patch.
>
>>> Oops occur when running qemu on arm64:
>>> Unable to handle kernel paging request at virtual address
>>> 08effe40 Internal error: Oops: 9607 [#1] SMP Process
>>> migration/0 (pid: 12, stack limit = 0x084e3736)
>>> pstate: 2085 (nzCv daIf -PAN -UAO) pc :
>>> __ll_sc___cmpxchg_case_acq_4+0x4/0x20
>>> lr : move_queued_task.isra.21+0x124/0x298
>>> ...
>>> Call trace:
>>> __ll_sc___cmpxchg_case_acq_4+0x4/0x20
>>> __migrate_task+0xc8/0xe0
>>> migration_cpu_stop+0x170/0x180
>>> cpu_stopper_thread+0xec/0x178
>>> smpboot_thread_fn+0x1ac/0x1e8
>>> kthread+0x134/0x138
>>> ret_from_fork+0x10/0x18
>>>
>>> __set_cpus_allowed_ptr will choose an active dest_cpu in affinity mask
>>> to migrage the process if process is not currently running on any one
>>> of the CPUs specified in affinity mask.__set_cpus_allowed_ptr will
>>> choose an invalid dest_cpu(>= nr_cpu_ids, 1024 in my virtual machine) if
>>> CPUS in affinity mask are deactived by cpu_down after cpumask_intersects
>>> check.
>>
>>Right, cpumask_any_and() returns >= nr_cpu_ids when there isn't any valid CPU
>>bit set.
>>
>>> Cpumask_test_cpu of dest_cpu afterwards is overflow and may passes if
>>> corresponding bit is coincidentally set.
>>
>>Ouch. I was going to say the cpu_active_mask check from is_cpu_allowed()
>>should've stopped the whole thing there, but AFAICT there's no safeguard
>>against > nr_cpu_ids bit accesses. I see CONFIG_DEBUG_PER_CPU_MAPS should
>>fire a warning for such accesses, but we don't enable it by default.
>>
>>Would it make sense to do something like
>>
>> return test_bit(...) && cpu < nr_cpu_ids;
>>
>>for cpumask_test_cpu()? We still get the warn with the right config, but we
>>prevent sneaky mistakes like this one. And it seems it's not the only one
>>according to:
>>
>>--
>>virtual patch
>>virtual report
>>
>>@nocheck@
>>expression E;
>>identifier any_func =~ "^cpumask_any";
>>position pos;
>>@@
>>
>>E@pos = any_func(...);
>>... when != E >= nr_cpu_ids
>>when != E < nr_cpu_ids
>>
>>@script:python depends on nocheck && report@ p << nocheck.pos; @@
>>coccilib.report.print_report(p[0], "Missing cpumask_any_*() return value
>>check!")
>>---
>>
>>Some of those seem benign since they are on e.g. cpu_online_mask, some other
>>are somewhat dubious (e.g. deadline.c::dl_task_can_attach()).
>>
>>As a consequence, kernel will access a invalid rq address associate with the
>>invalid cpu in
>
>It's more thoughtful to add check in cpumask_test_cpu.It can solve this
>problem and can prevent other potential bugs.I will test it and resend
>a new patch.
>
Think again and again. As cpumask_check will fire a warning if cpu >=
nr_cpu_ids, it seems that cpumask_check only expects cpu < nr_cpu_ids and it's
caller's responsibility to very cpu is in valid range. Interfaces like
cpumask_test_and_set_cpu, cpumask_test_and_clear_cpu and so on are not checking
cpu < nr_cpu_ids either and may cause demage if cpu is out of range.
>>> migration_cpu_stop->__migrate_task->move_queued_task and the Oops occurs.
>>> Process as follows may trigger the Oops:
>>> 1) A process repeatedly bind itself to cpu0 and cpu1 in turn by
>>> calling sched_setaffinity
>>> 2) A shell script repeatedly "echo 0 >
>>> /sys/devices/system/cpu/cpu1/online" and "echo 1 >
>>> /sys/devices/system/cpu/cpu1/online" in turn
>>> 3) Oops appears if the invalid cpu is set in memory after tested cpumask.
>>>
>>>
>>> Change-Id: I9c2f95aecd3da568991b7408397215f26c990e40
>>
>>- This doesn't respect the 75 char per line limit
>>- Change IDs don't belong here (we're not using Gerrit)
>>- You're missing a Signed-off-by.
>>
>>You'll find all the guidelines you need for formatting patches in
>>Documentation/process/submitting-patches.rst.
>>
>
>Thanks for the guide.I will read it carefully before send next patch.
>
>>> ---
>>> kernel/sched/core.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c index
>>> 4b63fef..5181ea9 100644
>>> --- a/kernel/sched/core.c
>>> +++ b/kernel/sched/core.c
>>> @@ -1112,7 +1112,8 @@ static int __set_cpus_allowed_ptr(struct task_struct
>>> *p,
>>> if (cpumask_equal(&p->cpus_allowed, new_mask))
>>> goto out;
>>>
>>> - if (!cpumask_intersects(new_mask, cpu_valid_mask)) {
>>> + dest_cpu = cpumask_any_and(cpu_valid_mask, new_mask);
>>> + if (dest_cpu >= nr_cpu_ids) {
>>> ret = -EINVAL;
>>> goto out;
Re: [PATCH] igb/igc: Don't warn on fatal read failures when the device is removed
On Fri, Aug 23, 2019 at 02:33:18AM +0800, Lyude Paul wrote:
> Fatal read errors are worth warning about, unless of course the device
> was just unplugged from the machine - something that's a rather normal
> occurence when the igb/igc adapter is located on a Thunderbolt dock. So,
> let's only WARN() if there's a fatal read error while the device is
> still present.
>
> This fixes the following WARN splat that's been appearing whenever I
> unplug my Caldigit TS3 Thunderbolt dock from my laptop:
>
> igb :09:00.0 enp9s0: PCIe link lost
> [ cut here ]
> igb: Failed to read reg 0x18!
> WARNING: CPU: 7 PID: 516 at
> drivers/net/ethernet/intel/igb/igb_main.c:756 igb_rd32+0x57/0x6a [igb]
> Modules linked in: igb dca thunderbolt fuse vfat fat elan_i2c mei_wdt
> mei_hdcp i915 wmi_bmof intel_wmi_thunderbolt iTCO_wdt
> iTCO_vendor_support x86_pkg_temp_thermal intel_powerclamp joydev
> coretemp crct10dif_pclmul crc32_pclmul i2c_algo_bit ghash_clmulni_intel
> intel_cstate drm_kms_helper intel_uncore syscopyarea sysfillrect
> sysimgblt fb_sys_fops intel_rapl_perf intel_xhci_usb_role_switch mei_me
> drm roles idma64 i2c_i801 ucsi_acpi typec_ucsi mei intel_lpss_pci
> processor_thermal_device typec intel_pch_thermal intel_soc_dts_iosf
> intel_lpss int3403_thermal thinkpad_acpi wmi int340x_thermal_zone
> ledtrig_audio int3400_thermal acpi_thermal_rel acpi_pad video
> pcc_cpufreq ip_tables serio_raw nvme nvme_core crc32c_intel uas
> usb_storage e1000e i2c_dev
> CPU: 7 PID: 516 Comm: kworker/u16:3 Not tainted 5.2.0-rc1Lyude-Test+ #14
> Hardware name: LENOVO 20L8S2N800/20L8S2N800, BIOS N22ET35W (1.12 )
> 04/09/2018
> Workqueue: kacpi_hotplug acpi_hotplug_work_fn
> RIP: 0010:igb_rd32+0x57/0x6a [igb]
> Code: 87 b8 fc ff ff 48 c7 47 08 00 00 00 00 48 c7 c6 33 42 9b c0 4c 89
> c7 e8 47 45 cd dc 89 ee 48 c7 c7 43 42 9b c0 e8 c1 94 71 dc <0f> 0b eb
> 08 8b 00 ff c0 75 b0 eb c8 44 89 e0 5d 41 5c c3 0f 1f 44
> RSP: 0018:ba5801cf7c48 EFLAGS: 00010286
> RAX: RBX: 9e7956608840 RCX: 0007
> RDX: RSI: ba5801cf7b24 RDI: 9e795e3d6a00
> RBP: 0018 R08: 9dec4a01 R09: 9e61018f
> R10: R11: ba5801cf7ae5 R12:
> R13: 9e7956608840 R14: 9e795a6f10b0 R15:
> FS: () GS:9e795e3c() knlGS:
> CS: 0010 DS: ES: CR0: 80050033
> CR2: 564317bc4088 CR3: 00010e00a006 CR4: 003606e0
> DR0: DR1: DR2:
> DR3: DR6: fffe0ff0 DR7: 0400
> Call Trace:
>igb_release_hw_control+0x1a/0x30 [igb]
>igb_remove+0xc5/0x14b [igb]
>pci_device_remove+0x3b/0x93
>device_release_driver_internal+0xd7/0x17e
>pci_stop_bus_device+0x36/0x75
>pci_stop_bus_device+0x66/0x75
>pci_stop_bus_device+0x66/0x75
>pci_stop_and_remove_bus_device+0xf/0x19
>trim_stale_devices+0xc5/0x13a
>? __pm_runtime_resume+0x6e/0x7b
>trim_stale_devices+0x103/0x13a
>? __pm_runtime_resume+0x6e/0x7b
>trim_stale_devices+0x103/0x13a
>acpiphp_check_bridge+0xd8/0xf5
>acpiphp_hotplug_notify+0xf7/0x14b
>? acpiphp_check_bridge+0xf5/0xf5
>acpi_device_hotplug+0x357/0x3b5
>acpi_hotplug_work_fn+0x1a/0x23
>process_one_work+0x1a7/0x296
>worker_thread+0x1a8/0x24c
>? process_scheduled_works+0x2c/0x2c
>kthread+0xe9/0xee
>? kthread_destroy_worker+0x41/0x41
>ret_from_fork+0x35/0x40
> ---[ end trace 252bf10352c63d22 ]---
>
Thanks for the fix.
Acked-by: Feng Tang
>
> Signed-off-by: Lyude Paul
> Fixes: 47e16692b26b ("igb/igc: warn when fatal read failure happens")
> Cc: Feng Tang
> Cc: Sasha Neftin
> Cc: Jeff Kirsher
> Cc: intel-wired-...@lists.osuosl.org
> ---
> drivers/net/ethernet/intel/igb/igb_main.c | 3 ++-
> drivers/net/ethernet/intel/igc/igc_main.c | 3 ++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c
> b/drivers/net/ethernet/intel/igb/igb_main.c
> index e5b7e638df28..1a7f7cd28df9 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -753,7 +753,8 @@ u32 igb_rd32(struct e1000_hw *hw, u32 reg)
> struct net_device *netdev = igb->netdev;
> hw->hw_addr = NULL;
> netdev_err(netdev, "PCIe link lost\n");
> - WARN(1, "igb: Failed to read reg 0x%x!\n", reg);
> + WARN(pci_device_is_present(igb->pdev),
> + "igb: Failed to read reg 0x%x!\n", reg);
> }
>
> return value;
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
> b/drivers/net/ethernet/intel/igc/igc_main.c
> index 28072b9aa932..f873a4b35eaf 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
Re: Linux 5.3-rc8
On So, 15.09.19 09:07, Willy Tarreau (w...@1wt.eu) wrote: > > That code can finish 5h after boot, it's entirely fine with this > > specific usecase. > > > > Again: we don't delay "the boot" for this. We just delay "writing a > > new seed to disk" for this. And if that is 5h later, then that's > > totally fine, because in the meantime it's just one bg process more that > > hangs around waiting to do what it needs to do. > > Didn't you say it could also happen when using encrypted swap ? If so > I suspect this could happen very early during boot, before any services > may be started ? Depends on the deps, and what options are used in /etc/crypttab. If people hard rely on swap to be enabled for boot to proceed and also use one-time passwords from /dev/urandom they better provide some form of hw rng, too. Otherwise the boot will block, yes. Basically, just add "nofail" to a line in /etc/crypttab, and the entry will be activated at boot, but we won't delay boot for it. It's going to be activated as soon as the deps are fulfilled (and thus the pool initialized), but that may well be 5h after boot, and that's totally OK as long as nothing else hard depends on it. Lennart -- Lennart Poettering, Berlin
Re: Linux 5.3-rc8
On So, 15.09.19 09:27, Ahmed S. Darwish (darwish...@gmail.com) wrote: > On Sun, Sep 15, 2019 at 08:51:42AM +0200, Lennart Poettering wrote: > > On Sa, 14.09.19 09:30, Linus Torvalds (torva...@linux-foundation.org) wrote: > [...] > > > > And please don't break /dev/urandom again. The above code is the ony > > way I see how we can make /dev/urandom-derived swap encryption safe, > > and the only way I can see how we can sanely write a valid random seed > > to disk after boot. > > > > Any hope in making systemd-random-seed(8) credit that "random seed > from previous boot" file, through RNDADDENTROPY, *by default*? No. For two reasons: a) It's way too late. We shouldn't credit entropy from the disk seed if we cannot update the disk seed with a new one at the same time, otherwise we might end up crediting the same seed twice on subsequent reboots (think: user hard powers off a system after we credited but before we updated), in which case there would not be a point in doing that at all. Hence, we have to wait until /var is writable, but that's relatively late during boot. Long afer the initrd ran, long after iscsi and what not ran. Long after the network stack is up and so on. In a time where people load root images from the initrd via HTTPS thats's generally too late to be useful. b) Golden images are a problem. There are probably more systems running off golden images in the wild, than those not running off them. This means: a random seed on disk is only safe to credit if it gets purged when the image is distributed to the systems it's supposed to be used on, because otherwise these systems will all come up with the very same seed, which makes it useless. So, by requesting people to explicitly acknowledge that they are aware of this problem (and either don't use golden images, or safely wipe the seed off the image before shipping it), by setting the env var, we protect ourselves from this. Last time I looked at it most popular distro's live images didn't wipe the random seed properly before distributing it to users... This is all documented btw: https://systemd.io/RANDOM_SEEDS#systemds-support-for-filling-the-kernel-entropy-pool See point #2. > I know that by v243, just released 12 days ago, this can be optionally > done through SYSTEMD_RANDOM_SEED_CREDIT=1. I wonder though if it can > ever be done by default, just like what the BSDs does... This would > solve a big part of the current problem. I think the best approach would be to do this in the boot loader. In fact systemd does this in its own boot loader (sd-boot): it reads a seed off the ESP, updates it (via a SHA256 hashed from the old one) and passes that to the OS. PID 1 very early on then credits this to the kernel's pool (ideally the kernel would just do this on its own btw). The trick we employ to make this generally safe is that we persistently store a "system token" as EFI var too, and include it in the SHA sum. The "system token" is a per-system random blob. It is created the first time it's needed and a good random source exists, and then stays on the system, for all future live images to use. This makes sure that even if sloppily put together live images are used (which do not reset any random seed) every system will use a different series of RNG seeds. This then solves both problems: the golden image problem, and the early-on problem. But of course only on ESP. Other systems should be able to provide similar mechanisms though, it's not rocket science. This is also documented here: https://systemd.io/RANDOM_SEEDS#systemds-support-for-filling-the-kernel-entropy-pool See point #3... Ideally other boot loaders (grub, …) would support the same scheme, but I am not sure the problem set is known to them. Lennart -- Lennart Poettering, Berlin
Re: [PATCH v6 1/5] dt-bindings: media: Add Allwinner A10 CSI binding
On Thu, Aug 15, 2019 at 4:34 PM Chen-Yu Tsai wrote: > > Hi, > > Sorry for chiming in so late. > > On Thu, Jul 11, 2019 at 8:15 PM Maxime Ripard > wrote: > > > > The Allwinner A10 CMOS Sensor Interface is a camera capture interface also > > used in later (A10s, A13, A20, R8 and GR8) SoCs. > > > > On some SoCs, like the A10, there's multiple instances of that controller, > > with one instance supporting more channels and having an ISP. > > > > Reviewed-by: Rob Herring > > Signed-off-by: Maxime Ripard > > --- > > Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml | 94 > > - > > 1 file changed, 94 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml > > b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml > > new file mode 100644 > > index ..97c9fc3b5050 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml > > @@ -0,0 +1,94 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/arm/allwinner,sun4i-a10-csi.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Allwinner A10 CMOS Sensor Interface (CSI) Device Tree Bindings > > + > > +maintainers: > > + - Chen-Yu Tsai > > + - Maxime Ripard > > + > > +description: |- > > + The Allwinner A10 and later has a CMOS Sensor Interface to retrieve > > + frames from a parallel or BT656 sensor. > > + > > + > > +properties: > > + compatible: > > +oneOf: > > + - items: > > + - enum: > > + - allwinner,sun7i-a20-csi0 > > + - const: allwinner,sun4i-a10-csi0 > > CSI0 on the A10 has an ISP. Do we know if the one in the A20 does > as well? It certainly doesn't say so in the user manual. If not, > then we can't claim that A20 CSI0 is compatible with A10 CSI0. > > > + > > + - items: > > + - const: allwinner,sun4i-a10-csi0 > > + > > + reg: > > +maxItems: 1 > > + > > + interrupts: > > +maxItems: 1 > > + > > + clocks: > > +items: > > + - description: The CSI interface clock > > + - description: The CSI module clock > > + - description: The CSI ISP clock > > + - description: The CSI DRAM clock > > + > > + clock-names: > > +items: > > + - const: bus > > + - const: mod > > I doubt this actually is a module clock. Based on the usage in your > device tree patch, and the csi driver in the old linux-sunxi kernel, > the clock rate is set to 24 MHz, or whatever the sensor requires for > MCLK. I'm working on adding support for this on the R40, and it seems with this SoC the picture is much clearer. It has the same CSI interface block, but the CCU has the clocks correctly named. We have: - CSI_MCLK0 - CSI_MCLK1 - CSI_SCLK in addition to the bus clocks. The CSI section also explains the clock signals: 6.1.3.2. Clock Sources Two Clocks need to be configured for CSI controller. CSI0/1_MCLK provides the master clock for sensor and other devices. CSI_SCLK is the top clock for the whole CSI module. So it would seem the ISP clock we currently have in the DT is simply the module clock shared by all CSI-related hardware blocks, and the module clock is bogus. ChenYu > ChenYu > > > + - const: isp > > + - const: ram > > + > > + resets: > > +description: The reset line driver this IP > > +maxItems: 1 > > + > > + pinctrl-0: > > +minItems: 1 > > + > > + pinctrl-names: > > +const: default > > + > > + port: > > +type: object > > +additionalProperties: false > > + > > +properties: > > + endpoint: > > +properties: > > + bus-width: > > +const: 8 > > +description: Number of data lines actively used. > > + > > + data-active: true > > + hsync-active: true > > + pclk-sample: true > > + remote-endpoint: true > > + vsync-active: true > > + > > +required: > > + - bus-width > > + - data-active > > + - hsync-active > > + - pclk-sample > > + - remote-endpoint > > + - vsync-active > > + > > +required: > > + - endpoint > > + > > +required: > > + - compatible > > + - reg > > + - interrupts > > + - clocks > > + > > +additionalProperties: false > > +... > > -- > > git-series 0.9.1
Re: [PATCH RFC v3] random: getrandom(2): optionally block when CRNG is uninitialized
On So, 15.09.19 10:17, Ahmed S. Darwish (darwish...@gmail.com) wrote: > Thus, don't trust user-space on calling getrandom(2) from the right > context. Never block, by default, and just return data from the > urandom source if entropy is not yet available. This is an explicit > decision not to let user-space work around this through busy loops on > error-codes. > > Note: this lowers the quality of random data returned by getrandom(2) > to the level of randomness returned by /dev/urandom, with all the > original security implications coming out of that, as discussed in > problem "3." at the top of this commit log. If this is not desirable, > offer users a fallback to old behavior, by CONFIG_RANDOM_BLOCK=y, or > random.getrandom_block=true bootparam. This is an awful idea. It just means that all crypto that needs entropy doing during early boot will now be using weak keys, and doesn't even know it. Yeah, it's a bad situation, but I am very sure that failing loudly in this case is better than just sticking your head in the sand and ignoring the issue without letting userspace know is an exceptionally bad idea. We live in a world where people run HTTPS, SSH, and all that stuff in the initrd already. It's where SSH host keys are generated, and plenty session keys. If Linux lets all that stuff run with awful entropy then you pretend things where secure while they actually aren't. It's much better to fail loudly in that case, I am sure. Quite frankly, I don't think this is something to fix in the kernel. Let the people putting together systems deal with this. Let them provide a creditable hw rng, and let them pay the price if they don't. Lennart -- Lennart Poettering, Berlin
[PATCH] x86: intel_tlb_table: small cleanups
Remove the unneeded backslash at EOL: that's not a macro.
And let's please checkpatch by aligning to open parenthesis.
For 0x4f descriptor, remove " */" from the info field.
For 0xc2 descriptor, sync the beginning of info to match the tlb_type.
(The value of info fields could be made more regular, but it's unused by
the code and will be read only by developers, so don't bother.)
Signed-off-by: Sylvain 'ythier' Hitier
---
arch/x86/kernel/cpu/intel.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 8d6d92e..24e619d 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -813,7 +813,7 @@ static unsigned int intel_size_cache(struct cpuinfo_x86 *c,
unsigned int size)
{ 0x04, TLB_DATA_4M,8, " TLB_DATA 4 MByte pages, 4-way
set associative" },
{ 0x05, TLB_DATA_4M,32, " TLB_DATA 4 MByte pages, 4-way
set associative" },
{ 0x0b, TLB_INST_4M,4, " TLB_INST 4 MByte pages, 4-way
set associative" },
- { 0x4f, TLB_INST_4K,32, " TLB_INST 4 KByte pages */" },
+ { 0x4f, TLB_INST_4K,32, " TLB_INST 4 KByte pages" },
{ 0x50, TLB_INST_ALL, 64, " TLB_INST 4 KByte and 2-MByte
or 4-MByte pages" },
{ 0x51, TLB_INST_ALL, 128," TLB_INST 4 KByte and 2-MByte
or 4-MByte pages" },
{ 0x52, TLB_INST_ALL, 256," TLB_INST 4 KByte and 2-MByte
or 4-MByte pages" },
@@ -841,7 +841,7 @@ static unsigned int intel_size_cache(struct cpuinfo_x86 *c,
unsigned int size)
{ 0xba, TLB_DATA_4K,64, " TLB_DATA 4 KByte pages, 4-way
associative" },
{ 0xc0, TLB_DATA_4K_4M, 8, " TLB_DATA 4 KByte and 4 MByte
pages, 4-way associative" },
{ 0xc1, STLB_4K_2M, 1024, " STLB 4 KByte and 2 MByte
pages, 8-way associative" },
- { 0xc2, TLB_DATA_2M_4M, 16, " DTLB 2 MByte/4MByte pages,
4-way associative" },
+ { 0xc2, TLB_DATA_2M_4M, 16, " TLB_DATA 2 MByte/4MByte
pages, 4-way associative" },
{ 0xca, STLB_4K,512," STLB 4 KByte pages, 4-way
associative" },
{ 0x00, 0, 0 }
};
@@ -853,8 +853,8 @@ static void intel_tlb_lookup(const unsigned char desc)
return;
/* look up this descriptor in the table */
- for (k = 0; intel_tlb_table[k].descriptor != desc && \
- intel_tlb_table[k].descriptor != 0; k++)
+ for (k = 0; intel_tlb_table[k].descriptor != desc &&
+intel_tlb_table[k].descriptor != 0; k++)
;
if (intel_tlb_table[k].tlb_type == 0)
--
1.7.10.4
Regards,
Sylvain "ythier" HITIER
--
Business is about being busy, not being rich...
Lived 777 days in a Debian package => http://en.wikipedia.org/wiki/Apt,_Vaucluse
There's THE room for ideals in this mechanical place!
Re: [PATCH RFC v3] random: getrandom(2): optionally block when CRNG is uninitialized
On Sun, Sep 15, 2019 at 10:59:07AM +0200, Lennart Poettering wrote: > We live in a world where people run HTTPS, SSH, and all that stuff in > the initrd already. It's where SSH host keys are generated, and plenty > session keys. It is exactly the type of crap that create this situation : making people developing such scripts believe that any random source was OK to generate these, and as such forcing urandom to produce crypto-solid randoms! No, distro developers must know that it's not acceptable to generate lifetime crypto keys from the early boot when no entropy is available. At least with this change they will get an error returned from getrandom() and will be able to ask the user to feed entropy, or be able to say "it was impossible to generate the SSH key right now, the daemon will only be started once it's possible", or "the SSH key we produced will not be saved because it's not safe and is only usable for this recovery session". > If Linux lets all that stuff run with awful entropy then > you pretend things where secure while they actually aren't. It's much > better to fail loudly in that case, I am sure. This is precisely what this change permits : fail instead of block by default, and let applications decide based on the use case. > Quite frankly, I don't think this is something to fix in the > kernel. As long as it offers a single API to return randoms, and that it is not possible not to block for low-quality randoms, it needs to be at least addressed there. Then userspace can adapt. For now userspace does not have this option just due to the kernel's way of exposing randoms. Willy
Applied "ASoC: sdm845: remove unneeded semicolon" to the asoc tree
The patch ASoC: sdm845: remove unneeded semicolon has been applied to the asoc tree at https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.4 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark >From fca11622d600228bec405456f41590155b3a3eca Mon Sep 17 00:00:00 2001 From: Saiyam Doshi Date: Sat, 14 Sep 2019 08:41:33 +0530 Subject: [PATCH] ASoC: sdm845: remove unneeded semicolon Remove excess semicolon after closing parenthesis. Signed-off-by: Saiyam Doshi Link: https://lore.kernel.org/r/20190914031133.GA28447@SD Signed-off-by: Mark Brown --- sound/soc/qcom/sdm845.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/qcom/sdm845.c b/sound/soc/qcom/sdm845.c index 882f52ed8231..28f3cef696e6 100644 --- a/sound/soc/qcom/sdm845.c +++ b/sound/soc/qcom/sdm845.c @@ -319,7 +319,7 @@ static void sdm845_snd_shutdown(struct snd_pcm_substream *substream) snd_soc_dai_set_sysclk(cpu_dai, Q6AFE_LPASS_CLK_ID_PRI_MI2S_IBIT, 0, SNDRV_PCM_STREAM_PLAYBACK); - }; + } break; case SECONDARY_MI2S_TX: -- 2.20.1
Re: [PATCH 5.2 36/37] vhost: block speculation of translated descriptors
Den 14-09-2019 kl. 11:08, skrev Greg Kroah-Hartman: On Sat, Sep 14, 2019 at 09:15:48AM +0200, Stefan Lippers-Hollmann wrote: Hi On 2019-09-14, Greg Kroah-Hartman wrote: On Sat, Sep 14, 2019 at 02:54:11AM +0200, Stefan Lippers-Hollmann wrote: On 2019-09-13, Greg Kroah-Hartman wrote: From: Michael S. Tsirkin commit a89db445fbd7f1f8457b03759aa7343fa530ef6b upstream. iovec addresses coming from vhost are assumed to be pre-validated, but in fact can be speculated to a value out of range. Userspace address are later validated with array_index_nospec so we can be sure kernel info does not leak through these addresses, but vhost must also not leak userspace info outside the allowed memory table to guests. Following the defence in depth principle, make sure the address is not validated out of node range. [...] Do you have the same problem with Linus's tree right now? Actually, yes I do (I had not tested i386 for 5.3~ within the last ~2 weeks, only amd64). Very similar kernel config, same compiler versions but built in a slightly different environment (built directly on the bare iron, in a amd64 multilib userspace, rather than a pure-i386 chroot on an amd64 kernel). $ git describe v5.3-rc8-36-ga7f89616b737 $ LANG= make ARCH=x86 -j1 bzImage modules CALLscripts/checksyscalls.sh CALLscripts/atomic/check-atomics.sh CHK include/generated/compile.h CHK kernel/kheaders_data.tar.xz CC [M] drivers/vhost/vhost.o In file included from ./include/linux/export.h:45, from ./include/linux/linkage.h:7, from ./include/linux/kernel.h:8, from ./include/linux/list.h:9, from ./include/linux/wait.h:7, from ./include/linux/eventfd.h:13, from drivers/vhost/vhost.c:13: drivers/vhost/vhost.c: In function 'translate_desc': ./include/linux/compiler.h:350:38: error: call to '__compiletime_assert_2076' declared with attribute error: BUILD_BUG_ON failed: sizeof(_s) > sizeof(long) 350 | _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) | ^ ./include/linux/compiler.h:331:4: note: in definition of macro '__compiletime_assert' 331 |prefix ## suffix();\ |^~ ./include/linux/compiler.h:350:2: note: in expansion of macro '_compiletime_assert' 350 | _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__) | ^~~ ./include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert' 39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg) | ^~ ./include/linux/build_bug.h:50:2: note: in expansion of macro 'BUILD_BUG_ON_MSG' 50 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition) | ^~~~ ./include/linux/nospec.h:56:2: note: in expansion of macro 'BUILD_BUG_ON' 56 | BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \ | ^~~~ drivers/vhost/vhost.c:2076:5: note: in expansion of macro 'array_index_nospec' 2076 | array_index_nospec((unsigned long)(addr - node->start), | ^~ make[2]: *** [scripts/Makefile.build:281: drivers/vhost/vhost.o] Error 1 make[1]: *** [scripts/Makefile.build:497: drivers/vhost] Error 2 make: *** [Makefile:1083: drivers] Error 2 $ git revert a89db445fbd7f1f8457b03759aa7343fa530ef6b $ LANG= make ARCH=x86 -j16 bzImage modules CALLscripts/atomic/check-atomics.sh CALLscripts/checksyscalls.sh CHK include/generated/compile.h CHK kernel/kheaders_data.tar.xz Building modules, stage 2. Kernel: arch/x86/boot/bzImage is ready (#1) MODPOST 3464 modules $ echo $? 0 $ find . -name vhost\\.ko ./drivers/vhost/vhost.ko I've attached the affected kernel config for v5.3~/ i386. Ok, good, we will be "bug compatible" at the very least now :) When this gets fixed in Linus's tree we can backport the fix here as well. The number of users of that compiler version/configuration is probably pretty low at the moment to want to hold off on this fix. This is now reverted upstream: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0d4a3f2abbef73b9e5bb5f12213c275565473588 -- Thomas
Re: [PATCH 2/2] staging: iio: accel: adis16240: move out of staging
On Thu, 12 Sep 2019 16:59:10 +0300 Alexandru Ardelean wrote: > On Wed, Sep 11, 2019 at 7:21 PM Rodrigo Carvalho > wrote: > > > > Hi, > > > > Em seg, 9 de set de 2019 às 02:53, Ardelean, Alexandru > > escreveu: > > > > > > On Sun, 2019-09-08 at 12:09 +0100, Jonathan Cameron wrote: > > > > On Mon, 2 Sep 2019 13:26:02 + > > > > "Ardelean, Alexandru" wrote: > > > > > > > > > On Sun, 2019-09-01 at 21:59 -0300, Rodrigo Carvalho wrote: > > > > > > Move ADIS16240 driver from staging to mainline. > > > > > > > > > > > > The ADIS16240 is a fully integrated digital shock detection > > > > > > and recorder system. > > > > > > > > > > Hey, > > > > > > > > > > Comments inline. > > > > > > > > > > I'll probably take a look in the next days again. > > > > > There seem to be some ABI/sysfs attributes that need to be resolved > > > > > before moving this out of staging. > > > > > > > > Absolutely. It is a 'new' type of device so there are definitely some > > > > corners that need discussing before we move out of staging and commit > > > > to maintaining the ABI moving forwards. > > > > > > > > That is the real reason this driver was still in staging! No one > > > > had been through the process of proposing the ABI and responding to > > > > questions etc. > > > > > > > > The issue with impact sensors has always been that they don't really fit > > > > our normal model for buffers or triggers. > > > > > > > > So normally a trigger (if exposed in IIO) is used as one trigger > > > > causes 1 set of samples (so like a frame trigger for a camera). > > > > > > > > These devices tend to work in a mode where one trigger causes data > > > > to be captured for a period of time. In this part that's the event > > > > recorder function > > > > > > > > No one is realistically going to buy an impact sensor to just use it > > > > as an accelerometer which is what this driver is currently doing. > > > > I suppose we could just leave support in that form for now, but > > > > I'm no sure how much use it is to anyone. > > > > > > > > Analog Devices people, worth working out how to support the event > > > > recorder? For that someone needs to have hardware as it is complex > > > > to say the least! > > > > > > Worth it: yes. > > > But we don't have any resources to allocate for this [at this point in > > > time]. > > > > > > > > > > > We could move it out but might be worth adding a comment somewhere > > > > saying this only really supports direct access to channels, and > > > > not the event recorder functionality. > > > > > > > > > > I guess, I would vote for leaving it in staging. > > > It's also a way to mark it as a > > > work-in-progress/not-done/still-needs-something kind of thing. > > > If we move it now, it gets the status of "everything-resolved" which is > > > not yet the case. > > > > > > Thanks for the insight/background info. > > > Much of it was discussed before my time. > > > > How about adis16203? Is it more simple to move this driver out of staging? > > Replying from my personal email, as I seem to have my periodical > issues with the work-email. > > At first glance I can't see any obvious blockers for this. > > I think Jonathan may need to provide some feedback here. There is a 'fixme' in the channels which I suspect is the remaining blocker on this one. The current y axis is actually just a different representation of the value on the x axis. I 'think' the only reason to support both of these is the interaction with the threshold detectors. Those are rather simple, so if you have -10 on the second channel which is 350 on the first, it would detected as < 10 on the second but not the first. It is ugly, but we may be better just treating these as two separate channels on the same axis - similar to the way we would deal with the case of two accelerometers in a package, with different ranges. Also a rather odd bit of abstraction around _addresses given there is only one specified. Might be nice to clean out some of the less useful comments for registers where the register name makes it clear what they are. It's a mixture of some useful ones and some pointless ones at the moment. So a few bits to tidy up and may more come up in a thorough review. Thanks, Jonathan > > Thanks > Alex > > > > > > > > > > > Jonathan > > > > > > Alex > > > > > > > > > > > > > > > > > Signed-off-by: Rodrigo Ribeiro Carvalho > > > > > > --- > > > > > > drivers/iio/accel/Kconfig | 12 + > > > > > > drivers/iio/accel/Makefile| 1 + > > > > > > drivers/iio/accel/adis16240.c | 454 > > > > > > ++ > > > > > > drivers/staging/iio/accel/Kconfig | 12 - > > > > > > drivers/staging/iio/accel/Makefile| 1 - > > > > > > drivers/staging/iio/accel/adis16240.c | 454 > > > > > > -- > > > > > > 6 files changed, 467 insertions(+), 467 deletions(-) > > > > > > create mode 100644 drivers/iio/accel/adis16240.c > >
Re: [PATCH] staging: iio: ADIS16240: Remove unused include
On Sat, 14 Sep 2019 02:06:27 +0530 Rohit Sarkar wrote: > Bcc: > Subject: [PATCH] staging: iio: adis16240: remove unused include > Reply-To: Something odd happened here with patch formatting. I fixed it up and applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. Thanks, Jonathan > > '#include' isn't being used anywhere. Remove it. > > Signed-off-by: Rohit Sarkar > --- > drivers/staging/iio/accel/adis16240.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/staging/iio/accel/adis16240.c > b/drivers/staging/iio/accel/adis16240.c > index 82099db4bf0c..a480409090c0 100644 > --- a/drivers/staging/iio/accel/adis16240.c > +++ b/drivers/staging/iio/accel/adis16240.c > @@ -7,7 +7,6 @@ > > #include > #include > -#include > #include > #include > #include
Re: [PATCH RFC v3] random: getrandom(2): optionally block when CRNG is uninitialized
On Sun, Sep 15, 2019 at 11:30:57AM +0200, Willy Tarreau wrote: > On Sun, Sep 15, 2019 at 10:59:07AM +0200, Lennart Poettering wrote: > > We live in a world where people run HTTPS, SSH, and all that stuff in > > the initrd already. It's where SSH host keys are generated, and plenty > > session keys. > > It is exactly the type of crap that create this situation : making > people developing such scripts believe that any random source was OK > to generate these, and as such forcing urandom to produce crypto-solid > randoms! Willy, let's tone it down please... the thread is already getting a bit toxic. > No, distro developers must know that it's not acceptable to > generate lifetime crypto keys from the early boot when no entropy is > available. At least with this change they will get an error returned > from getrandom() and will be able to ask the user to feed entropy, or > be able to say "it was impossible to generate the SSH key right now, > the daemon will only be started once it's possible", or "the SSH key > we produced will not be saved because it's not safe and is only usable > for this recovery session". > > > If Linux lets all that stuff run with awful entropy then > > you pretend things where secure while they actually aren't. It's much > > better to fail loudly in that case, I am sure. > > This is precisely what this change permits : fail instead of block > by default, and let applications decide based on the use case. > Unfortunately, not exactly. Linus didn't want getrandom to return an error code / "to fail" in that case, but to silently return CRNG-uninitialized /dev/urandom data, to avoid user-space even working around the error code through busy-loops. I understand the rationale behind that, of course, and this is what I've done so far in the V3 RFC. Nonetheless, this _will_, for example, make systemd-random-seed(8) save week seeds under /var/lib/systemd/random-seed, since the kernel didn't inform it about such weakness at all.. The situation is so bad now, that it's more of "some user-space are more equal than others".. Let's just at least admit this while discussing the RFC patch in question. thanks, > > Quite frankly, I don't think this is something to fix in the > > kernel. > > As long as it offers a single API to return randoms, and that it is > not possible not to block for low-quality randoms, it needs to be > at least addressed there. Then userspace can adapt. For now userspace > does not have this option just due to the kernel's way of exposing > randoms. > > Willy
Re: [PATCH] iio: adc: stm32-adc: fix a race when using several adcs with dma and irq
On Fri, 13 Sep 2019 15:21:30 +0200
Fabrice Gasnier wrote:
> End of conversion may be handled by using IRQ or DMA. There may be a
> race when two conversions complete at the same time on several ADCs.
> EOC can be read as 'set' for several ADCs, with:
> - an ADC configured to use IRQs. EOCIE bit is set. The handler is normally
> called in this case.
> - an ADC configured to use DMA. EOCIE bit isn't set. EOC triggers the DMA
> request instead. It's then automatically cleared by DMA read. But the
> handler gets called due to status bit is temporarily set (IRQ triggered
> by the other ADC).
> So both EOC status bit in CSR and EOCIE control bit must be checked
> before invoking the interrupt handler (e.g. call ISR only for
> IRQ-enabled ADCs).
>
> Fixes: 2763ea0585c9 ("iio: adc: stm32: add optional dma support")
>
> Signed-off-by: Fabrice Gasnier
Fix looks fine to me, but I'm not keen on splitting out individual bits from
register defines. That's a long term readability nightmare.
See below,
Jonathan
> ---
> drivers/iio/adc/stm32-adc-core.c | 43
> +---
> drivers/iio/adc/stm32-adc-core.h | 13
> drivers/iio/adc/stm32-adc.c | 6 --
> 3 files changed, 53 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/iio/adc/stm32-adc-core.c
> b/drivers/iio/adc/stm32-adc-core.c
> index 9b85fef..7297396 100644
> --- a/drivers/iio/adc/stm32-adc-core.c
> +++ b/drivers/iio/adc/stm32-adc-core.c
> @@ -71,6 +71,8 @@
> * @eoc1:adc1 end of conversion flag in @csr
> * @eoc2:adc2 end of conversion flag in @csr
> * @eoc3:adc3 end of conversion flag in @csr
> + * @ier: interrupt enable register offset for each adc
> + * @eocie_msk: end of conversion interrupt enable mask in @ier
> */
> struct stm32_adc_common_regs {
> u32 csr;
> @@ -78,6 +80,8 @@ struct stm32_adc_common_regs {
> u32 eoc1_msk;
> u32 eoc2_msk;
> u32 eoc3_msk;
> + u32 ier;
> + u32 eocie_msk;
> };
>
> struct stm32_adc_priv;
> @@ -303,6 +307,8 @@ static const struct stm32_adc_common_regs
> stm32f4_adc_common_regs = {
> .eoc1_msk = STM32F4_EOC1,
> .eoc2_msk = STM32F4_EOC2,
> .eoc3_msk = STM32F4_EOC3,
> + .ier = STM32F4_ADC_CR1,
> + .eocie_msk = STM32F4_EOCIE,
> };
>
> /* STM32H7 common registers definitions */
> @@ -311,8 +317,24 @@ static const struct stm32_adc_common_regs
> stm32h7_adc_common_regs = {
> .ccr = STM32H7_ADC_CCR,
> .eoc1_msk = STM32H7_EOC_MST,
> .eoc2_msk = STM32H7_EOC_SLV,
> + .ier = STM32H7_ADC_IER,
> + .eocie_msk = STM32H7_EOCIE,
> };
>
> +static const unsigned int stm32_adc_offset[STM32_ADC_MAX_ADCS] = {
> + 0, STM32_ADC_OFFSET, STM32_ADC_OFFSET * 2,
> +};
> +
> +static unsigned int stm32_adc_eoc_enabled(struct stm32_adc_priv *priv,
> + unsigned int adc)
> +{
> + u32 ier, offset = stm32_adc_offset[adc];
> +
> + ier = readl_relaxed(priv->common.base + offset + priv->cfg->regs->ier);
> +
> + return ier & priv->cfg->regs->eocie_msk;
> +}
> +
> /* ADC common interrupt for all instances */
> static void stm32_adc_irq_handler(struct irq_desc *desc)
> {
> @@ -323,13 +345,28 @@ static void stm32_adc_irq_handler(struct irq_desc *desc)
> chained_irq_enter(chip, desc);
> status = readl_relaxed(priv->common.base + priv->cfg->regs->csr);
>
> - if (status & priv->cfg->regs->eoc1_msk)
> + /*
> + * End of conversion may be handled by using IRQ or DMA. There may be a
> + * race here when two conversions complete at the same time on several
> + * ADCs. EOC may be read 'set' for several ADCs, with:
> + * - an ADC configured to use DMA (EOC triggers the DMA request, and
> + * is then automatically cleared by DR read in hardware)
> + * - an ADC configured to use IRQs (EOCIE bit is set. The handler must
> + * be called in this case)
> + * So both EOC status bit in CSR and EOCIE control bit must be checked
> + * before invoking the interrupt handler (e.g. call ISR only for
> + * IRQ-enabled ADCs).
> + */
> + if (status & priv->cfg->regs->eoc1_msk &&
> + stm32_adc_eoc_enabled(priv, 0))
> generic_handle_irq(irq_find_mapping(priv->domain, 0));
>
> - if (status & priv->cfg->regs->eoc2_msk)
> + if (status & priv->cfg->regs->eoc2_msk &&
> + stm32_adc_eoc_enabled(priv, 1))
> generic_handle_irq(irq_find_mapping(priv->domain, 1));
>
> - if (status & priv->cfg->regs->eoc3_msk)
> + if (status & priv->cfg->regs->eoc3_msk &&
> + stm32_adc_eoc_enabled(priv, 2))
> generic_handle_irq(irq_find_mapping(priv->domain, 2));
>
> chained_irq_exit(chip, desc);
> diff --git a/drivers/iio/adc/stm32-adc-core.h
> b/drivers/iio/adc/stm32-adc-core.h
> index 8af507b..8dc936b 100644
> --- a/drivers/iio/adc/stm32-adc-core.h
> +++ b/drivers/iio/adc/stm32-adc-core.h
> @@ -25
Re: [PATCH v2 4/4] iio: imu: adis: convert cs_change_delay to spi_delay struct
On Fri, 13 Sep 2019 14:55:49 +0300
Alexandru Ardelean wrote:
> The ADIS library is one of the few users of the new `cs_change_delay`
> parameter for an spi_transfer.
>
> The introduction of the `spi_delay` struct, requires that the users of of
> `cs_change_delay` get an update. This change updates the ADIS library.
>
> Signed-off-by: Alexandru Ardelean
Looks to me like the build is broken between patches 3 and 4.
Don't do that as it breaks bisectability.
If you are changing an interface like this it has to occur in one patch,
of you have to have intermediate code that deals with the smooth transition.
Otherwise, looks like a sensible bit of rework to me.
Jonathan
> ---
> drivers/iio/imu/adis.c | 24
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
> index 1631c255deab..2cd2cc2316c6 100644
> --- a/drivers/iio/imu/adis.c
> +++ b/drivers/iio/imu/adis.c
> @@ -39,24 +39,24 @@ int adis_write_reg(struct adis *adis, unsigned int reg,
> .len = 2,
> .cs_change = 1,
> .delay_usecs = adis->data->write_delay,
> - .cs_change_delay = adis->data->cs_change_delay,
> - .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
> + .cs_change_delay.value = adis->data->cs_change_delay,
> + .cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
> }, {
> .tx_buf = adis->tx + 2,
> .bits_per_word = 8,
> .len = 2,
> .cs_change = 1,
> .delay_usecs = adis->data->write_delay,
> - .cs_change_delay = adis->data->cs_change_delay,
> - .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
> + .cs_change_delay.value = adis->data->cs_change_delay,
> + .cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
> }, {
> .tx_buf = adis->tx + 4,
> .bits_per_word = 8,
> .len = 2,
> .cs_change = 1,
> .delay_usecs = adis->data->write_delay,
> - .cs_change_delay = adis->data->cs_change_delay,
> - .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
> + .cs_change_delay.value = adis->data->cs_change_delay,
> + .cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
> }, {
> .tx_buf = adis->tx + 6,
> .bits_per_word = 8,
> @@ -139,16 +139,16 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
> .len = 2,
> .cs_change = 1,
> .delay_usecs = adis->data->write_delay,
> - .cs_change_delay = adis->data->cs_change_delay,
> - .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
> + .cs_change_delay.value = adis->data->cs_change_delay,
> + .cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
> }, {
> .tx_buf = adis->tx + 2,
> .bits_per_word = 8,
> .len = 2,
> .cs_change = 1,
> .delay_usecs = adis->data->read_delay,
> - .cs_change_delay = adis->data->cs_change_delay,
> - .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
> + .cs_change_delay.value = adis->data->cs_change_delay,
> + .cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
> }, {
> .tx_buf = adis->tx + 4,
> .rx_buf = adis->rx,
> @@ -156,8 +156,8 @@ int adis_read_reg(struct adis *adis, unsigned int reg,
> .len = 2,
> .cs_change = 1,
> .delay_usecs = adis->data->read_delay,
> - .cs_change_delay = adis->data->cs_change_delay,
> - .cs_change_delay_unit = SPI_DELAY_UNIT_USECS,
> + .cs_change_delay.value = adis->data->cs_change_delay,
> + .cs_change_delay.unit = SPI_DELAY_UNIT_USECS,
> }, {
> .rx_buf = adis->rx + 2,
> .bits_per_word = 8,
Re: [RFC PATCH 00/15] Unify SPI delays into an `struct spi_delay`
On Fri, 13 Sep 2019 14:45:35 +0300 Alexandru Ardelean wrote: > Initially, I started this patchset thinking: "we need a new delay for > something-something" (in case someone is curios, we need a CS-hold-time for > the first transfer, because the CS wakes a chip from sleep-mode). > > Then I added the delay, and felt a bit dirty-inside about adding a new one > (just like that), and decided to look at maybe cleaning things up a bit, > and a few days later, I got here. > > Full disclaimer: this patchset is not complete. It's an RFC. > It's based on top of Jonathan's `iio/togreg` branch which also includes the > ADIS driver library changes and also includes `cs_change_delay`. > > I'll send a V2 patchset, which just the first 4 patches, since I feel that > those are a bit more complete. > > I thought about just sending the first 4 patches on-their-own, but I > figured that the whole series (even if not complete) serves as a better > explanation about the whole "why?". > > Hopefully, this can sort-of-explain things. > I'll reference this RFC on the next series. General approach looks sensible to me. Over to SPI specialists on whether this is a sensible bit of unification to do. Jonathan > > Thanks > > Alexandru Ardelean (15): > spi: move `cs_change_delay` backwards compat logic outside switch > spi: introduce spi_delay struct as "value + unit" & spi_delay_exec() > spi: make `cs_change_delay` the first user of the `spi_delay` logic > iio: imu: adis: convert cs_change_delay to spi_delay struct > spi: sprd: convert transfer word delay to spi_delay struct > spi: orion: use new `word_delay` field for SPI transfers > spi: spidev: use new `word_delay` field for spi transfers > spi: core,atmel: convert `word_delay_usecs` -> `word_delay` for > spi_device > spi: introduce `delay` field for `spi_transfer` + spi_transfer_exec() > spi: use new `spi_transfer_delay` helper where straightforward > spi: tegra114: use `spi_transfer_delay` helper > spi: spi-loopback-test: use new `delay` field > spi: spidev: use new `delay` field for spi transfers > spi: tegra114: change format for `spi_set_cs_timing()` function > spi: implement SW control for CS times > > drivers/iio/imu/adis.c | 24 ++--- > drivers/spi/spi-atmel.c | 29 +- > drivers/spi/spi-bcm63xx-hsspi.c | 3 +- > drivers/spi/spi-cavium.c | 3 +- > drivers/spi/spi-fsl-dspi.c | 3 +- > drivers/spi/spi-fsl-espi.c | 3 +- > drivers/spi/spi-fsl-spi.c| 3 +- > drivers/spi/spi-loopback-test.c | 12 ++- > drivers/spi/spi-mpc512x-psc.c| 3 +- > drivers/spi/spi-mpc52xx-psc.c| 3 +- > drivers/spi/spi-omap-100k.c | 3 +- > drivers/spi/spi-orion.c | 6 +- > drivers/spi/spi-pl022.c | 25 +++-- > drivers/spi/spi-sc18is602.c | 3 +- > drivers/spi/spi-sh-hspi.c| 3 +- > drivers/spi/spi-sprd.c | 11 ++- > drivers/spi/spi-tegra114.c | 39 +--- > drivers/spi/spi-tegra20-sflash.c | 2 +- > drivers/spi/spi-topcliff-pch.c | 7 +- > drivers/spi/spi-txx9.c | 3 +- > drivers/spi/spi-xcomm.c | 3 +- > drivers/spi/spi.c| 162 +-- > drivers/spi/spidev.c | 6 +- > include/linux/spi/spi.h | 65 ++--- > 24 files changed, 293 insertions(+), 131 deletions(-) >
[PATCH] perf: add support for logging debug messages to file
When in TUI mode, it is impossible to show all the debug messages to
console. This make it hard to debug perf issues using debug messages.
This patch adds support for logging debug messages to file to resolve
this problem.
The usage is:
perf -debug verbose=2 --debug file=1 COMMAND
And the path of log file is '~/perf.log'.
Signed-off-by: Changbin Du
---
tools/perf/Documentation/perf.txt | 4 +++-
tools/perf/util/debug.c | 20
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/tools/perf/Documentation/perf.txt
b/tools/perf/Documentation/perf.txt
index 401f0ed67439..45db7b22d1a5 100644
--- a/tools/perf/Documentation/perf.txt
+++ b/tools/perf/Documentation/perf.txt
@@ -16,7 +16,8 @@ OPTIONS
Setup debug variable (see list below) in value
range (0, 10). Use like:
--debug verbose # sets verbose = 1
- --debug verbose=2 # sets verbose = 2
+ --debug verbose=2 --debug file=1
+ # sets verbose = 2 and save log to file
List of debug variables allowed to set:
verbose - general debug messages
@@ -24,6 +25,7 @@ OPTIONS
data-convert - data convert command debug messages
stderr - write debug output (option -v) to stderr
in browser mode
+ file - write debug output to log file ~/perf.log
--buildid-dir::
Setup buildid cache directory. It has higher priority than
diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
index 3780fe42453b..f0b4346a0efa 100644
--- a/tools/perf/util/debug.c
+++ b/tools/perf/util/debug.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -28,6 +29,8 @@ int verbose;
bool dump_trace = false, quiet = false;
int debug_ordered_events;
static int redirect_to_stderr;
+static int log_to_file;
+static FILE *log_file;
int debug_data_convert;
int veprintf(int level, int var, const char *fmt, va_list args)
@@ -39,6 +42,9 @@ int veprintf(int level, int var, const char *fmt, va_list
args)
ui_helpline__vshow(fmt, args);
else
ret = vfprintf(stderr, fmt, args);
+
+ if (log_file)
+ vfprintf(log_file, fmt, args);
}
return ret;
@@ -180,6 +186,7 @@ static struct debug_variable {
{ .name = "verbose",.ptr = &verbose },
{ .name = "ordered-events", .ptr = &debug_ordered_events},
{ .name = "stderr", .ptr = &redirect_to_stderr},
+ { .name = "file", .ptr = &log_to_file},
{ .name = "data-convert", .ptr = &debug_data_convert },
{ .name = NULL, }
};
@@ -219,6 +226,19 @@ int perf_debug_option(const char *str)
v = -1;
*var->ptr = v;
+
+ if (log_to_file) {
+ char log_path[PATH_MAX];
+
+ strcat(strcpy(log_path, getenv("HOME")), "/perf.log");
+ log_file = fopen(log_path, "a");
+ if (!log_file) {
+ pr_err("Can not create log file: %s\n",
strerror(errno));
+ free(s);
+ return -1;
+ }
+ }
+
free(s);
return 0;
}
--
2.20.1
Re: pull-request: wireless-drivers-next 2019-09-14
David Miller writes: > From: Kalle Valo > Date: Sat, 14 Sep 2019 13:14:40 +0300 > >> here's a pull request to net-next tree for v5.4, more info below. Please >> let me know if there are any problems. > > Pulled, thanks Kalle. Thanks for pulling this but I don't see it in net-next, maybe you forgot to push? Nothing important, just making sure it didn't get lost. -- https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH RFC v3] random: getrandom(2): optionally block when CRNG is uninitialized
On Sun, Sep 15, 2019 at 12:02:01PM +0200, Ahmed S. Darwish wrote: > On Sun, Sep 15, 2019 at 11:30:57AM +0200, Willy Tarreau wrote: > > On Sun, Sep 15, 2019 at 10:59:07AM +0200, Lennart Poettering wrote: > > > We live in a world where people run HTTPS, SSH, and all that stuff in > > > the initrd already. It's where SSH host keys are generated, and plenty > > > session keys. > > > > It is exactly the type of crap that create this situation : making > > people developing such scripts believe that any random source was OK > > to generate these, and as such forcing urandom to produce crypto-solid > > randoms! > > Willy, let's tone it down please... the thread is already getting a > bit toxic. I don't see what's wrong in my tone above, I'm sorry if it can be perceived as such. My point was that things such as creating lifetime keys while there's no entropy is the wrong thing to do and what progressively led to this situation. > > > If Linux lets all that stuff run with awful entropy then > > > you pretend things where secure while they actually aren't. It's much > > > better to fail loudly in that case, I am sure. > > > > This is precisely what this change permits : fail instead of block > > by default, and let applications decide based on the use case. > > > > Unfortunately, not exactly. > > Linus didn't want getrandom to return an error code / "to fail" in > that case, but to silently return CRNG-uninitialized /dev/urandom > data, to avoid user-space even working around the error code through > busy-loops. But with this EINVAL you have the information that it only filled the buffer with whatever it could, right ? At least that was the last point I manage to catch in the discussion. Otherwise if it's totally silent, I fear that it will reintroduce the problem in a different form (i.e. libc will say "our randoms are not reliable anymore, let us work around this and produce blocking, solid randoms again to help all our users"). > I understand the rationale behind that, of course, and this is what > I've done so far in the V3 RFC. > > Nonetheless, this _will_, for example, make systemd-random-seed(8) > save week seeds under /var/lib/systemd/random-seed, since the kernel > didn't inform it about such weakness at all.. Then I am confused because I understood that the goal was to return EINVAL or anything equivalent in which case the userspace knows what it has to deal with :-/ Regards, Willy
Re: [PATCH RFC v3] random: getrandom(2): optionally block when CRNG is uninitialized
On Sun, Sep 15, 2019 at 12:40:27PM +0200, Willy Tarreau wrote: > On Sun, Sep 15, 2019 at 12:02:01PM +0200, Ahmed S. Darwish wrote: > > On Sun, Sep 15, 2019 at 11:30:57AM +0200, Willy Tarreau wrote: > > > On Sun, Sep 15, 2019 at 10:59:07AM +0200, Lennart Poettering wrote: [...] > > > > If Linux lets all that stuff run with awful entropy then > > > > you pretend things where secure while they actually aren't. It's much > > > > better to fail loudly in that case, I am sure. > > > > > > This is precisely what this change permits : fail instead of block > > > by default, and let applications decide based on the use case. > > > > > > > Unfortunately, not exactly. > > > > Linus didn't want getrandom to return an error code / "to fail" in > > that case, but to silently return CRNG-uninitialized /dev/urandom > > data, to avoid user-space even working around the error code through > > busy-loops. > > But with this EINVAL you have the information that it only filled > the buffer with whatever it could, right ? At least that was the > last point I manage to catch in the discussion. Otherwise if it's > totally silent, I fear that it will reintroduce the problem in a > different form (i.e. libc will say "our randoms are not reliable > anymore, let us work around this and produce blocking, solid randoms > again to help all our users"). > V1 of the patch I posted did indeed return -EINVAL. Linus then suggested that this might make still some user-space act smart and just busy-loop around that, basically blocking the boot again: https://lkml.kernel.org/r/CAHk-=wiB0e_uGpidYHf+dV4eeT+XmG-+rQBx=jj110r48qf...@mail.gmail.com https://lkml.kernel.org/r/CAHk-=whSbo=dbiqozloa6tfmmgbeb8d9krxxvxbktprwkg0...@mail.gmail.com So it was then requested to actually return what /dev/urandom would return, so that user-space has no way whatsoever in knowing if getrandom has failed. Then, it's the job of system integratos / BSP builders to fix the inspect the big fat WARN on the kernel and fix that. This is the core of Lennart's critqueue of V3 above. > > I understand the rationale behind that, of course, and this is what > > I've done so far in the V3 RFC. > > > > Nonetheless, this _will_, for example, make systemd-random-seed(8) > > save week seeds under /var/lib/systemd/random-seed, since the kernel > > didn't inform it about such weakness at all.. > > Then I am confused because I understood that the goal was to return > EINVAL or anything equivalent in which case the userspace knows what > it has to deal with :-/ > Yeah, the discussion moved a bit beyond that. thanks, --darwi
Re: [PATCH 3/3] spi: dw: Add compatible string for Renesas RZ/N1 SPI Controller
On Fri, Sep 13, 2019 at 3:14 PM Gareth Williams
wrote:
>
> From: Phil Edworthy
>
> The Renesas RZ/N1 SPI Controller is based on the Synopsys DW SSI, but has
> additional registers for software CS control and DMA. This patch does not
> address the changes required for DMA support, it simply adds the compatible
> string. The CS registers are not needed as Linux can use gpios for the CS
> signals.
> + { .compatible = "renesas,rzn1-spi", },
Can't you simple use in DT something like
compatible = "renesas,rzn1-spi", "snps,dw-apb-ssi"
?
--
With Best Regards,
Andy Shevchenko
[PATCH] media: rc: Use the correct style for SPDX License Identifier
This patch corrects the SPDX License Identifier style in header file related to Remote Controller Driver for Linux. For C header files Documentation/process/license-rules.rst mandates C-like comments (opposed to C source files where C++ style should be used) Changes made by using a script provided by Joe Perches here: https://lkml.org/lkml/2019/2/7/46. Suggested-by: Joe Perches Signed-off-by: Nishad Kamdar --- drivers/media/rc/rc-core-priv.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h index 9f21b3e8b377..5f36244cc34f 100644 --- a/drivers/media/rc/rc-core-priv.h +++ b/drivers/media/rc/rc-core-priv.h @@ -1,5 +1,5 @@ +/* SPDX-License-Identifier: GPL-2.0 */ /* - * SPDX-License-Identifier: GPL-2.0 * Remote Controller core raw events header * * Copyright (C) 2010 by Mauro Carvalho Chehab -- 2.17.1
Re: [PATCH v2] net: mdio: switch to using gpiod_get_optional()
On Sun, Sep 15, 2019 at 9:26 AM Dmitry Torokhov wrote: > On Sat, Sep 14, 2019 at 08:09:33PM +0300, Andy Shevchenko wrote: > > On Fri, Sep 13, 2019 at 03:55:47PM -0700, Dmitry Torokhov wrote: > > > + mdiodev->reset_gpio = gpiod_get_optional(&mdiodev->dev, > > > +"reset", GPIOD_OUT_LOW); > > > + error = PTR_ERR_OR_ZERO(mdiodev->reset_gpio); > > > + if (error) > > > + return error; > > > + if (mdiodev->reset_gpio) > > > > This is redundant check. > > I see that gpiod_* API handle NULL desc and usually return immediately, > but frankly I am not that comfortable with it. I'm OK with functions > that free/destroy objects that recognize NULL resources, > but it is > unusual for other types of APIs. CLK, reset, ... frameworks do check for NULL pointer since they provide an *_optional() getters. So, it's not unusual. -- With Best Regards, Andy Shevchenko
[PATCH] media: tuners: Use the correct style for SPDX License Identifier
This patch corrects the SPDX License Identifier style in header file related to media Drivers for Analog TV Tuners. For C header files Documentation/process/license-rules.rst mandates C-like comments (opposed to C source files where C++ style should be used) Changes made by using a script provided by Joe Perches here: https://lkml.org/lkml/2019/2/7/46. Suggested-by: Joe Perches Signed-off-by: Nishad Kamdar --- drivers/media/tuners/tuner-xc2028-types.h | 2 +- drivers/media/tuners/tuner-xc2028.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/tuners/tuner-xc2028-types.h b/drivers/media/tuners/tuner-xc2028-types.h index 50d017a4822a..fcca39d3e006 100644 --- a/drivers/media/tuners/tuner-xc2028-types.h +++ b/drivers/media/tuners/tuner-xc2028-types.h @@ -1,5 +1,5 @@ +/* SPDX-License-Identifier: GPL-2.0 */ /* - * SPDX-License-Identifier: GPL-2.0 * tuner-xc2028_types * * This file includes internal tipes to be used inside tuner-xc2028. diff --git a/drivers/media/tuners/tuner-xc2028.h b/drivers/media/tuners/tuner-xc2028.h index 7b58bc06e35c..2dd45d0765d7 100644 --- a/drivers/media/tuners/tuner-xc2028.h +++ b/drivers/media/tuners/tuner-xc2028.h @@ -1,5 +1,5 @@ +/* SPDX-License-Identifier: GPL-2.0 */ /* - * SPDX-License-Identifier: GPL-2.0 * tuner-xc2028 * * Copyright (c) 2007-2008 Mauro Carvalho Chehab -- 2.17.1
Re: [PATCH RFC v3] random: getrandom(2): optionally block when CRNG is uninitialized
On Sun, Sep 15, 2019 at 12:55:39PM +0200, Ahmed S. Darwish wrote: > On Sun, Sep 15, 2019 at 12:40:27PM +0200, Willy Tarreau wrote: > > On Sun, Sep 15, 2019 at 12:02:01PM +0200, Ahmed S. Darwish wrote: > > > On Sun, Sep 15, 2019 at 11:30:57AM +0200, Willy Tarreau wrote: > > > > On Sun, Sep 15, 2019 at 10:59:07AM +0200, Lennart Poettering wrote: > [...] > > > > > If Linux lets all that stuff run with awful entropy then > > > > > you pretend things where secure while they actually aren't. It's much > > > > > better to fail loudly in that case, I am sure. > > > > > > > > This is precisely what this change permits : fail instead of block > > > > by default, and let applications decide based on the use case. > > > > > > > > > > Unfortunately, not exactly. > > > > > > Linus didn't want getrandom to return an error code / "to fail" in > > > that case, but to silently return CRNG-uninitialized /dev/urandom > > > data, to avoid user-space even working around the error code through > > > busy-loops. > > > > But with this EINVAL you have the information that it only filled > > the buffer with whatever it could, right ? At least that was the > > last point I manage to catch in the discussion. Otherwise if it's > > totally silent, I fear that it will reintroduce the problem in a > > different form (i.e. libc will say "our randoms are not reliable > > anymore, let us work around this and produce blocking, solid randoms > > again to help all our users"). > > > > V1 of the patch I posted did indeed return -EINVAL. Linus then > suggested that this might make still some user-space act smart and > just busy-loop around that, basically blocking the boot again: > > > https://lkml.kernel.org/r/CAHk-=wiB0e_uGpidYHf+dV4eeT+XmG-+rQBx=jj110r48qf...@mail.gmail.com > > https://lkml.kernel.org/r/CAHk-=whSbo=dbiqozloa6tfmmgbeb8d9krxxvxbktprwkg0...@mail.gmail.com > > So it was then requested to actually return what /dev/urandom would > return, so that user-space has no way whatsoever in knowing if > getrandom has failed. Then, it's the job of system integratos / BSP > builders to fix the inspect the big fat WARN on the kernel and fix > that. Then I was indeed a bit confused in the middle of the discussion as I didn't understand exactly this, thanks for the clarifying :-) But does it still block when called with GRND_RANDOM ? If so I guess I'm fine as it translates exactly the previous behavior of random vs urandom, and that GRND_NONBLOCK allows the application to fall back to reliable sources if needed (typically human interactions). Thanks, Willy
Re: [PATCH 3/3] spi: dw: Add compatible string for Renesas RZ/N1 SPI Controller
On Sun, Sep 15, 2019 at 02:00:33PM +0300, Andy Shevchenko wrote:
> On Fri, Sep 13, 2019 at 3:14 PM Gareth Williams
> > The Renesas RZ/N1 SPI Controller is based on the Synopsys DW SSI, but has
> > additional registers for software CS control and DMA. This patch does not
> > address the changes required for DMA support, it simply adds the compatible
> > string. The CS registers are not needed as Linux can use gpios for the CS
> > signals.
> > + { .compatible = "renesas,rzn1-spi", },
> Can't you simple use in DT something like
> compatible = "renesas,rzn1-spi", "snps,dw-apb-ssi"
> ?
Yes, you can and should do that but it's still nice to list the
compatibles explicitly in the driver in case someone leaves out the
fallback compatible for whatever reason - if both the driver and the DT
list things then there's a bit more robustness.
signature.asc
Description: PGP signature
Re: pull-request: wireless-drivers-next 2019-09-14
From: Kalle Valo Date: Sun, 15 Sep 2019 13:32:49 +0300 > David Miller writes: > >> From: Kalle Valo >> Date: Sat, 14 Sep 2019 13:14:40 +0300 >> >>> here's a pull request to net-next tree for v5.4, more info below. Please >>> let me know if there are any problems. >> >> Pulled, thanks Kalle. > > Thanks for pulling this but I don't see it in net-next, maybe you forgot > to push? Nothing important, just making sure it didn't get lost. I feel asleep while the build test was running, it should be there now :-)
Re: [PATCH] ARM: dts: imx6dl: SolidRun: add phy node with 100Mb/s max-speed
On Sun, Sep 15, 2019 at 09:30:00AM +0300, Baruch Siach wrote:
> Hi Andrew,
>
> On Tue, Sep 10 2019, Andrew Lunn wrote:
> > On Tue, Sep 10, 2019 at 06:55:07PM +0300, tinywrkb wrote:
> >> Cubox-i Solo/DualLite carrier board has 100Mb/s magnetics while the
> >> Atheros AR8035 PHY on the MicroSoM v1.3 CPU module is a 1GbE PHY device.
> >>
> >> Since commit 5502b218e001 ("net: phy: use phy_resolve_aneg_linkmode in
> >> genphy_read_status") ethernet is broken on Cubox-i Solo/DualLite devices.
> >
> > Hi Tinywrkb
> >
> > You emailed lots of people, but missed the PHY maintainers :-(
> >
> > Are you sure this is the patch which broken it? Did you do a git
> > bisect.
>
> Tinywrkb confirmed to me in private communication that revert of
> 5502b218e001 fixes Ethernet for him on effected system.
>
> He also referred me to an old Cubox-i spec that lists 10/100 Ethernet
> only for i.MX6 Solo/DualLite variants of Cubox-i. It turns out that
> there was a plan to use a different 10/100 PHY for Solo/DualLite
> SOMs. This plan never materialized. All SolidRun i.MX6 SOMs use the same
> AR8035 PHY that supports 1Gb.
>
> Commit 5502b218e001 might be triggering a hardware issue on the affected
> Cubox-i. I could not reproduce the issue here with Cubox-i and a Dual
> SOM variant running v5.3-rc8. I have no Solo/DualLite variant handy at
> the moment.
With 5.3 due out today, I'll be updating my systems to that, which will
include quite a few variants of the Hummingboard.
It looks like one of my Solo Hummingboards (running a fully up to date
Fedora 28) has encountered a problem, so needs a reboot...
systemd-journald[436]: Failed to retrieve credentials for PID 17906, ignoring:
Cannot allocate memory
systemd-journald[436]: Failed to open runtime journal: Cannot allocate memory
# ps aux
USER PID %CPU %MEMVSZ RSS TTY STAT START TIME COMMAND
...
root 436 0.0 5.2 3128140 26392 ? Ss Aug03 1:20
/usr/lib/systemd/systemd-journald
# uptime
13:28:41 up 42 days, 19:13, 1 user, load average: 0.00, 0.03, 0.00
Looks like systemd-journald has a rather bad memory leak...
#include
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
[PATCH] ALSA: usb-audio: Add Hiby device family to quirks for native DSD support
From: Sudokamikaze This patch adds quirk VID ID for Hiby portable players family with native DSD playback support Signed-off-by: Sudokamikaze --- sound/usb/quirks.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c index 78858918cbc1..64a8d73972e3 100644 --- a/sound/usb/quirks.c +++ b/sound/usb/quirks.c @@ -1655,6 +1655,7 @@ u64 snd_usb_interface_dsd_format_quirks(struct snd_usb_audio *chip, case 0x152a: /* Thesycon devices */ case 0x25ce: /* Mytek devices */ case 0x2ab6: /* T+A devices */ + case 0xc502: /* HiBy devices */ if (fp->dsd_raw) return SNDRV_PCM_FMTBIT_DSD_U32_BE; break; -- 2.23.0
Re: [PATCH v5 2/9] documention: leds: Add multicolor class documentation
On 9/12/19 10:55 PM, Pavel Machek wrote: > Hi! > >> +Directory Layout Example >> + >> +root:/sys/class/leds/rgb:grouped_leds# ls -lR colors/ >> +colors/: >> +drwxr-xr-x2 root root 0 Jun 28 20:21 blue >> +drwxr-xr-x2 root root 0 Jun 28 20:21 green >> +drwxr-xr-x2 root root 0 Jun 28 20:21 red >> +-rw---1 root root 4096 Jun 28 20:21 color_mix >> + >> +colors/blue: >> +-rw---1 root root 4096 Jun 28 20:21 intensity >> +-r1 root root 4096 Jun 28 20:27 max_intensity >> +-r1 root root 4096 Jun 28 20:21 color_id > > I don't really like the directories... A bit too much complexity, and > it will have a memory footprint, too. We will need separate intensity files for each color anyway so separate max_brightness can go along in the sub-dir - it will be cleaner this way and will leave some flexibility regarding max intensity supported by the LED. And I'm saying about the need for separate intensity files because I've changed my mind regarding color_mix file, basing on the recent experience with trigger file PAGE_SIZE limit rework. That said, I am no longer eager to accept any exception for one-value-per-file rule. This of course render color_mix file format unacceptable. In a new approach intensity files will only cache the values and actual write to registers will occur on write to the brightness file. > I'd expect max_intensity to be same for all the leds in > rgb:grouped_leds... Could we simply rely on max_brightness file? This may not be necessarily true if one will add gpio LED to the cluster of PWM LEDs. > [If not, would one "max_intensity" file in rgb:grouped_leds be > enough?] > > Best regards, > Pavel > > -- Best regards, Jacek Anaszewski
Re: [PATCH] usbip: vhci_hcd indicate failed message
On Sun, Sep 15, 2019 at 11:43:32AM +0800, Mao Wenan wrote:
> If the return value of vhci_init_attr_group and
> sysfs_create_group is non-zero, which mean they failed
> to init attr_group and create sysfs group, so it would
> better add 'failed' message to indicate that.
>
> Fixes: 0775a9cbc694 ("usbip: vhci extension: modifications to vhci driver")
> Signed-off-by: Mao Wenan
> ---
> drivers/usb/usbip/vhci_hcd.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
> index 000ab7225717..dd54c95d2498 100644
> --- a/drivers/usb/usbip/vhci_hcd.c
> +++ b/drivers/usb/usbip/vhci_hcd.c
> @@ -1185,12 +1185,12 @@ static int vhci_start(struct usb_hcd *hcd)
> if (id == 0 && usb_hcd_is_primary_hcd(hcd)) {
> err = vhci_init_attr_group();
> if (err) {
> - pr_err("init attr group\n");
> + pr_err("init attr group failed\n");
dev_err() would also be good to use here.
> return err;
> }
> err = sysfs_create_group(&hcd_dev(hcd)->kobj, &vhci_attr_group);
> if (err) {
> - pr_err("create sysfs files\n");
> + pr_err("create sysfs failed\n");
Same here.
thanks,
greg k-h
Re: [PATCH v2 1/3] iio: adc: hx711: fix bug in sampling of data
On Mon, 9 Sep 2019 14:37:21 +0200
Andreas Klinger wrote:
> Fix bug in sampling function hx711_cycle() when interrupt occures while
> PD_SCK is high. If PD_SCK is high for at least 60 us power down mode of
> the sensor is entered which in turn leads to a wrong measurement.
>
> Switch off interrupts during a PD_SCK high period and move query of DOUT
> to the latest point of time which is at the end of PD_SCK low period.
>
> This bug exists in the driver since it's initial addition. The more
> interrupts on the system the higher is the probability that it happens.
>
> Fixes: c3b2fdd0ea7e ("iio: adc: hx711: Add IIO driver for AVIA HX711")
>
> Signed-off-by: Andreas Klinger
Applied to the fixes-togreg branch of iio.git and marked for stable.
Thanks,
Jonathan
> ---
> drivers/iio/adc/hx711.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/adc/hx711.c b/drivers/iio/adc/hx711.c
> index 88c7fe15003b..0678964dbd21 100644
> --- a/drivers/iio/adc/hx711.c
> +++ b/drivers/iio/adc/hx711.c
> @@ -101,13 +101,14 @@ struct hx711_data {
> static int hx711_cycle(struct hx711_data *hx711_data)
> {
> int val;
> + unsigned long flags;
>
> /*
>* if preempted for more then 60us while PD_SCK is high:
>* hx711 is going in reset
>* ==> measuring is false
>*/
> - preempt_disable();
> + local_irq_save(flags);
> gpiod_set_value(hx711_data->gpiod_pd_sck, 1);
>
> /*
> @@ -117,7 +118,6 @@ static int hx711_cycle(struct hx711_data *hx711_data)
>*/
> ndelay(hx711_data->data_ready_delay_ns);
>
> - val = gpiod_get_value(hx711_data->gpiod_dout);
> /*
>* here we are not waiting for 0.2 us as suggested by the datasheet,
>* because the oscilloscope showed in a test scenario
> @@ -125,7 +125,7 @@ static int hx711_cycle(struct hx711_data *hx711_data)
>* and 0.56 us for PD_SCK low on TI Sitara with 800 MHz
>*/
> gpiod_set_value(hx711_data->gpiod_pd_sck, 0);
> - preempt_enable();
> + local_irq_restore(flags);
>
> /*
>* make it a square wave for addressing cases with capacitance on
> @@ -133,7 +133,8 @@ static int hx711_cycle(struct hx711_data *hx711_data)
>*/
> ndelay(hx711_data->data_ready_delay_ns);
>
> - return val;
> + /* sample as late as possible */
> + return gpiod_get_value(hx711_data->gpiod_dout);
> }
>
> static int hx711_read(struct hx711_data *hx711_data)
Re: [PATCH v2 1/3] iio: adc: hx711: fix bug in sampling of data
On Sun, 15 Sep 2019 13:53:26 +0100
Jonathan Cameron wrote:
> On Mon, 9 Sep 2019 14:37:21 +0200
> Andreas Klinger wrote:
>
> > Fix bug in sampling function hx711_cycle() when interrupt occures while
> > PD_SCK is high. If PD_SCK is high for at least 60 us power down mode of
> > the sensor is entered which in turn leads to a wrong measurement.
> >
> > Switch off interrupts during a PD_SCK high period and move query of DOUT
> > to the latest point of time which is at the end of PD_SCK low period.
> >
> > This bug exists in the driver since it's initial addition. The more
> > interrupts on the system the higher is the probability that it happens.
> >
> > Fixes: c3b2fdd0ea7e ("iio: adc: hx711: Add IIO driver for AVIA HX711")
> >
> > Signed-off-by: Andreas Klinger
>
> Applied to the fixes-togreg branch of iio.git and marked for stable.
>
> Thanks,
I should have waited for my local tests to finish. This result in
val being unused, so I've removed it.
Thanks,
Jonathan
>
> Jonathan
>
> > ---
> > drivers/iio/adc/hx711.c | 9 +
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/iio/adc/hx711.c b/drivers/iio/adc/hx711.c
> > index 88c7fe15003b..0678964dbd21 100644
> > --- a/drivers/iio/adc/hx711.c
> > +++ b/drivers/iio/adc/hx711.c
> > @@ -101,13 +101,14 @@ struct hx711_data {
> > static int hx711_cycle(struct hx711_data *hx711_data)
> > {
> > int val;
> > + unsigned long flags;
> >
> > /*
> > * if preempted for more then 60us while PD_SCK is high:
> > * hx711 is going in reset
> > * ==> measuring is false
> > */
> > - preempt_disable();
> > + local_irq_save(flags);
> > gpiod_set_value(hx711_data->gpiod_pd_sck, 1);
> >
> > /*
> > @@ -117,7 +118,6 @@ static int hx711_cycle(struct hx711_data *hx711_data)
> > */
> > ndelay(hx711_data->data_ready_delay_ns);
> >
> > - val = gpiod_get_value(hx711_data->gpiod_dout);
> > /*
> > * here we are not waiting for 0.2 us as suggested by the datasheet,
> > * because the oscilloscope showed in a test scenario
> > @@ -125,7 +125,7 @@ static int hx711_cycle(struct hx711_data *hx711_data)
> > * and 0.56 us for PD_SCK low on TI Sitara with 800 MHz
> > */
> > gpiod_set_value(hx711_data->gpiod_pd_sck, 0);
> > - preempt_enable();
> > + local_irq_restore(flags);
> >
> > /*
> > * make it a square wave for addressing cases with capacitance on
> > @@ -133,7 +133,8 @@ static int hx711_cycle(struct hx711_data *hx711_data)
> > */
> > ndelay(hx711_data->data_ready_delay_ns);
> >
> > - return val;
> > + /* sample as late as possible */
> > + return gpiod_get_value(hx711_data->gpiod_dout);
> > }
> >
> > static int hx711_read(struct hx711_data *hx711_data)
>
Re: [PATCH v2 3/3] iio: adc: hx711: remove unnecessary returns
On Mon, 9 Sep 2019 14:38:08 +0200
Andreas Klinger wrote:
> Optimize use of return in hx711_set_gain_for_channel().
>
> Signed-off-by: Andreas Klinger
I agree with Joe on this. Minor reduction in code, but hurts
readability so a no on this one.
thanks,
Jonathan
> ---
> drivers/iio/adc/hx711.c | 8 ++--
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/adc/hx711.c b/drivers/iio/adc/hx711.c
> index c8686558429b..20c249f502c0 100644
> --- a/drivers/iio/adc/hx711.c
> +++ b/drivers/iio/adc/hx711.c
> @@ -213,7 +213,7 @@ static int hx711_reset(struct hx711_data *hx711_data)
>
> static int hx711_set_gain_for_channel(struct hx711_data *hx711_data, int
> chan)
> {
> - int ret;
> + int ret = 0;
>
> if (chan == 0) {
> if (hx711_data->gain_set == 32) {
> @@ -224,8 +224,6 @@ static int hx711_set_gain_for_channel(struct hx711_data
> *hx711_data, int chan)
> return ret;
>
> ret = hx711_wait_for_ready(hx711_data);
> - if (ret)
> - return ret;
> }
> } else {
> if (hx711_data->gain_set != 32) {
> @@ -236,12 +234,10 @@ static int hx711_set_gain_for_channel(struct hx711_data
> *hx711_data, int chan)
> return ret;
>
> ret = hx711_wait_for_ready(hx711_data);
> - if (ret)
> - return ret;
> }
> }
>
> - return 0;
> + return ret;
> }
>
> static int hx711_reset_read(struct hx711_data *hx711_data, int chan)
Re: [PATCH 4.9 00/14] 4.9.193-stable review
On Sat, Sep 14, 2019 at 05:49:32PM -0700, Guenter Roeck wrote:
> Hi Greg,
>
> On 9/14/19 1:31 AM, Greg Kroah-Hartman wrote:
> > On Sat, Sep 14, 2019 at 01:28:39AM -0700, Guenter Roeck wrote:
> > > On 9/13/19 6:06 AM, Greg Kroah-Hartman wrote:
> > > > This is the start of the stable review cycle for the 4.9.193 release.
> > > > There are 14 patches in this series, all will be posted as a response
> > > > to this one. If anyone has any issues with these being applied, please
> > > > let me know.
> > > >
> > > > Responses should be made by Sun 15 Sep 2019 01:03:32 PM UTC.
> > > > Anything received after that time might be too late.
> > > >
> > >
> > > Is it really only me seeing this ?
> > >
> > > drivers/vhost/vhost.c: In function 'translate_desc':
> > > include/linux/compiler.h:549:38: error: call to
> > > '__compiletime_assert_1879' declared with attribute error: BUILD_BUG_ON
> > > failed: sizeof(_s) > sizeof(long)
> > >
> > > i386:allyesconfig, mips:allmodconfig and others, everywhere including
> > > mainline. Culprit is commit a89db445fbd7f1 ("vhost: block speculation
> > > of translated descriptors").
> >
> > Nope, I just got another report of this on 5.2.y. Problem is also in
> > Linus's tree :(
> >
>
> Please apply upstream commit 0d4a3f2abbef ("Revert "vhost: block speculation
> of translated descriptors") to v4.9.y-queue and later to fix the build
> problems.
> Or maybe just drop the offending commit from the stable release queues.
I'm just going to drop the offending commit from everywhere and push out
new -rcs in a bit...
thanks,
greg k-h
Re: [PATCH v2 2/3] iio: adc: hx711: optimize performance in read cycle
On Mon, 9 Sep 2019 14:37:48 +0200
Andreas Klinger wrote:
> Set gain in hx711_reset() to its default value after a reset cycle. This
> omits one precautionary read cycle, because the read is performed in
> hx711_set_gain_for_channel() anyway if gain has changed.
>
> Check for DOUT low and if its high wait some time if it goes down
> instead of doing a blind reset cycle when DOUT is not down.
>
> This is a performance optimization which allows to query the sensor with
> a higher frequency.
>
> Signed-off-by: Andreas Klinger
Hi Andreas,
This one seems to have a slightly non trivial interaction with patch 1
so will have to wait until the fixes-togreg branch works it's way back
around to the upstream for the togreg branch. Give me a poke if I seem
to have lost it!
Thanks,
Jonathan
> ---
> drivers/iio/adc/hx711.c | 23 +--
> 1 file changed, 5 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/iio/adc/hx711.c b/drivers/iio/adc/hx711.c
> index 0678964dbd21..c8686558429b 100644
> --- a/drivers/iio/adc/hx711.c
> +++ b/drivers/iio/adc/hx711.c
> @@ -23,6 +23,7 @@
>
> /* gain to pulse and scale conversion */
> #define HX711_GAIN_MAX 3
> +#define HX711_RESET_GAIN 128
>
> struct hx711_gain_to_scale {
> int gain;
> @@ -100,7 +101,6 @@ struct hx711_data {
>
> static int hx711_cycle(struct hx711_data *hx711_data)
> {
> - int val;
> unsigned long flags;
>
> /*
> @@ -186,8 +186,7 @@ static int hx711_wait_for_ready(struct hx711_data
> *hx711_data)
>
> static int hx711_reset(struct hx711_data *hx711_data)
> {
> - int ret;
> - int val = gpiod_get_value(hx711_data->gpiod_dout);
> + int val = hx711_wait_for_ready(hx711_data);
>
> if (val) {
> /*
> @@ -203,22 +202,10 @@ static int hx711_reset(struct hx711_data *hx711_data)
> msleep(10);
> gpiod_set_value(hx711_data->gpiod_pd_sck, 0);
>
> - ret = hx711_wait_for_ready(hx711_data);
> - if (ret)
> - return ret;
> - /*
> - * after a reset the gain is 128 so we do a dummy read
> - * to set the gain for the next read
> - */
> - ret = hx711_read(hx711_data);
> - if (ret < 0)
> - return ret;
> -
> - /*
> - * after a dummy read we need to wait vor readiness
> - * for not mixing gain pulses with the clock
> - */
> val = hx711_wait_for_ready(hx711_data);
> +
> + /* after a reset the gain is 128 */
> + hx711_data->gain_set = HX711_RESET_GAIN;
> }
>
> return val;
Re: [PATCH v2 1/3] iio: accel: adxl372: Fix/remove limitation for FIFO samples
On Tue, 10 Sep 2019 17:43:32 +0300
Stefan Popa wrote:
> Currently, the driver sets the FIFO_SAMPLES register with the number of
> sample sets (maximum of 170 for 3 axis data, 256 for 2-axis and 512 for
> single axis). However, the FIFO_SAMPLES register should store the number
> of samples, regardless of how the FIFO format is configured.
>
> Signed-off-by: Stefan Popa
Fixes tags? I think it's
Fixes: f4f55ce38e5f ("iio:adxl372: Add FIFO and interrupts support")
Check I got that right though.
One trivial inline that I have tidied up whilst applying.
Applied to the fixes-togreg branch of iio.git.
Thanks,
Jonathan
> ---
> Changes in v2:
> - st->watermark needs to store the number of sample sets,
> the total number of samples is computed in
> adxl372_configure_fifo() func.
>
> drivers/iio/accel/adxl372.c | 11 ---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> index 055227cb..7de5e1b 100644
> --- a/drivers/iio/accel/adxl372.c
> +++ b/drivers/iio/accel/adxl372.c
> @@ -474,12 +474,17 @@ static int adxl372_configure_fifo(struct adxl372_state
> *st)
> if (ret < 0)
> return ret;
>
> - fifo_samples = st->watermark & 0xFF;
> + /*
> + * watermak stores the number of sets; we need to write the FIFO
watermark
> + * registers with the number of samples
> + */
> + fifo_samples = (st->watermark * st->fifo_set_size);
> fifo_ctl = ADXL372_FIFO_CTL_FORMAT_MODE(st->fifo_format) |
> ADXL372_FIFO_CTL_MODE_MODE(st->fifo_mode) |
> -ADXL372_FIFO_CTL_SAMPLES_MODE(st->watermark);
> +ADXL372_FIFO_CTL_SAMPLES_MODE(fifo_samples);
>
> - ret = regmap_write(st->regmap, ADXL372_FIFO_SAMPLES, fifo_samples);
> + ret = regmap_write(st->regmap,
> +ADXL372_FIFO_SAMPLES, fifo_samples & 0xFF);
> if (ret < 0)
> return ret;
>
[GIT PULL] Kbuild updates for v5.4-rc1
Hi Linus,
This is a Kbuild pull request for v5.4-rc1.
I am sending this a bit earlier.
Please pull it in when you open the merge window.
Thanks.
The following changes since commit d45331b00ddb179e291766617259261c112db872:
Linux 5.3-rc4 (2019-08-11 13:26:41 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git
tags/kbuild-v5.4
for you to fetch changes up to 77564a4829ef6d309331d443ea6ceb065f3dc371:
genksyms: convert to SPDX License Identifier for lex.l and parse.y
(2019-09-14 11:40:13 +0900)
Kbuild updates for v5.4
- add modpost warn exported symbols marked as 'static' because 'static'
and EXPORT_SYMBOL is an odd combination
- break the build early if gold linker is used
- optimize the Bison rule to produce .c and .h files by a single
pattern rule
- handle PREEMPT_RT in the module vermagic and UTS_VERSION
- warn CONFIG options leaked to the user-space except existing ones
- make single targets work properly
- rebuild modules when module linker scripts are updated
- split the module final link stage into scripts/Makefile.modfinal
- fix the missed error code in merge_config.sh
- improve the error message displayed on the attempt of the O= build
in unclean source tree
- remove 'clean-dirs' syntax
- disable -Wimplicit-fallthrough warning for Clang
- add CONFIG_CC_OPTIMIZE_FOR_SIZE_O3 for ARC
- remove ARCH_{CPP,A,C}FLAGS variables
- add $(BASH) to run bash scripts
- change *CFLAGS_.o to take the relative path to $(obj)
instead of the basename
- stop suppressing Clang's -Wunused-function warnings when W=1
- fix linux/export.h to avoid genksyms calculating CRC of trimmed
exported symbols
- misc cleanups
Denis Efremov (2):
modpost: check for static EXPORT_SYMBOL* functions
modpost: add NOFAIL to strndup
Guillaume Tucker (1):
merge_config.sh: ignore unwanted grep errors
Heikki Krogerus (1):
modpost: add guid_t type definition
Kees Cook (1):
kbuild: Parameterize kallsyms generation and correct reporting
Mark Brown (1):
merge_config.sh: Check error codes from make
Masahiro Yamada (54):
kbuild: use $(basename ...) for cmd_asn1_compiler
kbuild: make bison create C file and header in a single pattern rule
kbuild: move flex and bison rules to Makefile.host
kbuild: add [M] marker for build log of *.mod.o
kbuild: treat an object as multi-used when $(foo-) is set
kbuild: move the Module.symvers check for external module build
kbuild: refactor part-of-module more
kbuild: fix modkern_aflags implementation
kbuild: remove 'make /' support
kbuild: remove meaningless 'targets' in ./Kbuild
kbuild: do not descend to ./Kbuild when cleaning
kbuild: unset variables in top Makefile instead of setting 0
kbuild: unify vmlinux-dirs and module-dirs rules
kbuild: unify clean-dirs rule for in-kernel and external module
kbuild: re-implement detection of CONFIG options leaked to user-space
kbuild: make single targets work more correctly
treewide: remove dummy Makefiles for single targets
kbuild: move KBUILD_LDS, KBUILD_VMLINUX_{OBJS,LIBS} to makefiles.rst
kbuild: rebuild modules when module linker scripts are updated
kbuild: split final module linking out into Makefile.modfinal
.gitignore: ignore modules.order explicitly
kbuild: add CONFIG_ASM_MODVERSIONS
kbuild: move modkern_{c,a}flags to Makefile.lib from Makefile.build
kbuild: pkg: clean up package files/dirs from the top Makefile
kbuild: pkg: add package targets to PHONY instead of FORCE
kbuild: pkg: rename scripts/package/Makefile to scripts/Makefile.package
kbuild: remove unneeded '+' marker from kselftest-merge
docs: kbuild: fix invalid ReST syntax
docs: kbuild: remove cc-ldoption from document again
init/Kconfig: rework help of CONFIG_CC_OPTIMIZE_FOR_SIZE
kbuild: remove unneeded comments and code from scripts/basic/Makefile
kbuild: remove unneeded dependency for $(DOC_TARGETS)
kbuild: get rid of $(realpath ...) from scripts/mkmakefile
kbuild: remove 'Using ... as source for kernel' message
kbuild: Inform user to pass ARCH= for make mrproper only when necessary
kbuild: clarify where to run make mrproper when out-of-tree fails
kbuild: move the clean srctree check to the outputmakefile target
kbuild: remove prepare3 target
kbuild: check clean srctree even earlier
kbuild: remove clean-dirs syntax
kbuild: remove unneeded '+' marker from cmd_clean
kbuild: clean up subdir-ymn calculation in Makefile.clean
kbuild,arc: add CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE_O3 for ARC
kbuild: remove ARCH_{CPP,A,C}FLAGS
kbuild: add $(BASH) to run
Re: [PATCH v2 2/3] iio: accel: adxl372: Fix push to buffers lost samples
On Tue, 10 Sep 2019 17:44:21 +0300 Stefan Popa wrote: > One in two sample sets was lost by multiplying fifo_set_size with > sizeof(u16). Also, the double number of available samples were pushed to > the iio buffers. > > Signed-off-by: Stefan Popa Applied with same fixes tag as previous and cc stable. > --- > Changes in v2: > - Nothing changed. > > drivers/iio/accel/adxl372.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c > index 7de5e1b..33edca8 100644 > --- a/drivers/iio/accel/adxl372.c > +++ b/drivers/iio/accel/adxl372.c > @@ -553,8 +553,7 @@ static irqreturn_t adxl372_trigger_handler(int irq, void > *p) > goto err; > > /* Each sample is 2 bytes */ > - for (i = 0; i < fifo_entries * sizeof(u16); > - i += st->fifo_set_size * sizeof(u16)) > + for (i = 0; i < fifo_entries; i += st->fifo_set_size) > iio_push_to_buffers(indio_dev, &st->fifo_buf[i]); > } > err:
Re: [PATCH] usbip: vhci_hcd indicate failed message
Am 15.09.2019 05:43, schrieb Mao Wenan:
> If the return value of vhci_init_attr_group and
> sysfs_create_group is non-zero, which mean they failed
> to init attr_group and create sysfs group, so it would
> better add 'failed' message to indicate that.
>
> Fixes: 0775a9cbc694 ("usbip: vhci extension: modifications to vhci driver")
> Signed-off-by: Mao Wenan
> ---
> drivers/usb/usbip/vhci_hcd.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
> index 000ab7225717..dd54c95d2498 100644
> --- a/drivers/usb/usbip/vhci_hcd.c
> +++ b/drivers/usb/usbip/vhci_hcd.c
> @@ -1185,12 +1185,12 @@ static int vhci_start(struct usb_hcd *hcd)
> if (id == 0 && usb_hcd_is_primary_hcd(hcd)) {
> err = vhci_init_attr_group();
> if (err) {
> - pr_err("init attr group\n");
> + pr_err("init attr group failed\n");
> return err;
> }
> err = sysfs_create_group(&hcd_dev(hcd)->kobj, &vhci_attr_group);
> if (err) {
> - pr_err("create sysfs files\n");
> + pr_err("create sysfs failed\n");
I guess "sysfs files" is here intended ?
re,
wh
> vhci_finish_attr_group();
> return err;
> }
Re: [PATCH v2 3/3] iio: accel: adxl372: Perform a reset at start up
On Tue, 10 Sep 2019 17:44:46 +0300 Stefan Popa wrote: > We need to perform a reset a start up to make sure that the chip is in a > consistent state. This reset also disables all the interrupts which > should only be enabled together with the iio buffer. Not doing this, was > sometimes causing unwanted interrupts to trigger. > > Signed-off-by: Stefan Popa Added the same fixes tag, and cc for stable. We'll have to keep an eye on this though as there are other patches after the one hightlighted so they may not go on cleanly. Thanks, Jonathan > --- > Changes in v2: > - Instead of disabling the interrupts, now this patch performs > a software reset. > > drivers/iio/accel/adxl372.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c > index 33edca8..8a00528 100644 > --- a/drivers/iio/accel/adxl372.c > +++ b/drivers/iio/accel/adxl372.c > @@ -575,6 +575,14 @@ static int adxl372_setup(struct adxl372_state *st) > return -ENODEV; > } > > + /* > + * Perform a software reset to make sure the device is in a consistent > + * state after start up. > + */ > + ret = regmap_write(st->regmap, ADXL372_RESET, ADXL372_RESET_CODE); > + if (ret < 0) > + return ret; > + > ret = adxl372_set_op_mode(st, ADXL372_STANDBY); > if (ret < 0) > return ret;
Re: [PATCH 4.19 000/190] 4.19.73-stable review
On Fri, Sep 13, 2019 at 02:04:15PM +0100, Greg Kroah-Hartman wrote: > This is the start of the stable review cycle for the 4.19.73 release. > There are 190 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Sun 15 Sep 2019 01:03:32 PM UTC. > Anything received after that time might be too late. > > The whole patch series can be found in one patch at: > > https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.19.73-rc1.gz > or in the git tree and branch at: > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > linux-4.19.y > and the diffstat can be found below. I have released -rc2 to resolve a build issue: https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.19.73-rc2.gz
Re: [PATCH 5.2 00/37] 5.2.15-stable review
On Fri, Sep 13, 2019 at 02:07:05PM +0100, Greg Kroah-Hartman wrote: > This is the start of the stable review cycle for the 5.2.15 release. > There are 37 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Sun 15 Sep 2019 01:03:32 PM UTC. > Anything received after that time might be too late. > > The whole patch series can be found in one patch at: > > https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.2.15-rc1.gz > or in the git tree and branch at: > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > linux-5.2.y > and the diffstat can be found below. I have released -rc2 to resolve a build issue: https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.2.15-rc2.gz
Re: [PATCH 4.14 00/21] 4.14.144-stable review
On Fri, Sep 13, 2019 at 02:06:53PM +0100, Greg Kroah-Hartman wrote: > This is the start of the stable review cycle for the 4.14.144 release. > There are 21 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Sun 15 Sep 2019 01:03:32 PM UTC. > Anything received after that time might be too late. > > The whole patch series can be found in one patch at: > > https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.14.144-rc1.gz > or in the git tree and branch at: > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > linux-4.14.y > and the diffstat can be found below. I have released -rc2 to resolve a build issue: https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.14.144-rc2.gz
Re: [PATCH 4.9 00/14] 4.9.193-stable review
On Fri, Sep 13, 2019 at 02:06:53PM +0100, Greg Kroah-Hartman wrote: > This is the start of the stable review cycle for the 4.9.193 release. > There are 14 patches in this series, all will be posted as a response > to this one. If anyone has any issues with these being applied, please > let me know. > > Responses should be made by Sun 15 Sep 2019 01:03:32 PM UTC. > Anything received after that time might be too late. > > The whole patch series can be found in one patch at: > > https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.9.193-rc1.gz > or in the git tree and branch at: > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > linux-4.9.y > and the diffstat can be found below. I have released -rc2 to resolve a reported build issue: https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.9.193-rc2.gz
Re: [PATCH 5.2 36/37] vhost: block speculation of translated descriptors
On Sun, Sep 15, 2019 at 12:34:57PM +0300, Thomas Backlund wrote: > Den 14-09-2019 kl. 11:08, skrev Greg Kroah-Hartman: > > On Sat, Sep 14, 2019 at 09:15:48AM +0200, Stefan Lippers-Hollmann wrote: > > > Hi > > > > > > On 2019-09-14, Greg Kroah-Hartman wrote: > > > > On Sat, Sep 14, 2019 at 02:54:11AM +0200, Stefan Lippers-Hollmann wrote: > > > > > On 2019-09-13, Greg Kroah-Hartman wrote: > > > > > > From: Michael S. Tsirkin > > > > > > > > > > > > commit a89db445fbd7f1f8457b03759aa7343fa530ef6b upstream. > > > > > > > > > > > > iovec addresses coming from vhost are assumed to be > > > > > > pre-validated, but in fact can be speculated to a value > > > > > > out of range. > > > > > > > > > > > > Userspace address are later validated with array_index_nospec so we > > > > > > can > > > > > > be sure kernel info does not leak through these addresses, but vhost > > > > > > must also not leak userspace info outside the allowed memory table > > > > > > to > > > > > > guests. > > > > > > > > > > > > Following the defence in depth principle, make sure > > > > > > the address is not validated out of node range. > > > [...] > > > > Do you have the same problem with Linus's tree right now? > > > > > > Actually, yes I do (I had not tested i386 for 5.3~ within the last ~2 > > > weeks, only amd64). Very similar kernel config, same compiler versions > > > but built in a slightly different environment (built directly on the bare > > > iron, in a amd64 multilib userspace, rather than a pure-i386 chroot on an > > > amd64 kernel). > > > > > > $ git describe > > > v5.3-rc8-36-ga7f89616b737 > > > > > > $ LANG= make ARCH=x86 -j1 bzImage modules > > >CALLscripts/checksyscalls.sh > > >CALLscripts/atomic/check-atomics.sh > > >CHK include/generated/compile.h > > >CHK kernel/kheaders_data.tar.xz > > >CC [M] drivers/vhost/vhost.o > > > In file included from ./include/linux/export.h:45, > > > from ./include/linux/linkage.h:7, > > > from ./include/linux/kernel.h:8, > > > from ./include/linux/list.h:9, > > > from ./include/linux/wait.h:7, > > > from ./include/linux/eventfd.h:13, > > > from drivers/vhost/vhost.c:13: > > > drivers/vhost/vhost.c: In function 'translate_desc': > > > ./include/linux/compiler.h:350:38: error: call to > > > '__compiletime_assert_2076' declared with attribute error: BUILD_BUG_ON > > > failed: sizeof(_s) > sizeof(long) > > >350 | _compiletime_assert(condition, msg, __compiletime_assert_, > > > __LINE__) > > >| ^ > > > ./include/linux/compiler.h:331:4: note: in definition of macro > > > '__compiletime_assert' > > >331 |prefix ## suffix();\ > > >|^~ > > > ./include/linux/compiler.h:350:2: note: in expansion of macro > > > '_compiletime_assert' > > >350 | _compiletime_assert(condition, msg, __compiletime_assert_, > > > __LINE__) > > >| ^~~ > > > ./include/linux/build_bug.h:39:37: note: in expansion of macro > > > 'compiletime_assert' > > > 39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), > > > msg) > > >| ^~ > > > ./include/linux/build_bug.h:50:2: note: in expansion of macro > > > 'BUILD_BUG_ON_MSG' > > > 50 | BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition) > > >| ^~~~ > > > ./include/linux/nospec.h:56:2: note: in expansion of macro 'BUILD_BUG_ON' > > > 56 | BUILD_BUG_ON(sizeof(_s) > sizeof(long)); \ > > >| ^~~~ > > > drivers/vhost/vhost.c:2076:5: note: in expansion of macro > > > 'array_index_nospec' > > > 2076 | array_index_nospec((unsigned long)(addr - node->start), > > >| ^~ > > > make[2]: *** [scripts/Makefile.build:281: drivers/vhost/vhost.o] Error 1 > > > make[1]: *** [scripts/Makefile.build:497: drivers/vhost] Error 2 > > > make: *** [Makefile:1083: drivers] Error 2 > > > > > > $ git revert a89db445fbd7f1f8457b03759aa7343fa530ef6b > > > > > > $ LANG= make ARCH=x86 -j16 bzImage modules > > >CALLscripts/atomic/check-atomics.sh > > >CALLscripts/checksyscalls.sh > > >CHK include/generated/compile.h > > >CHK kernel/kheaders_data.tar.xz > > >Building modules, stage 2. > > > Kernel: arch/x86/boot/bzImage is ready (#1) > > >MODPOST 3464 modules > > > > > > $ echo $? > > > 0 > > > > > > $ find . -name vhost\\.ko > > > ./drivers/vhost/vhost.ko > > > > > > I've attached the affected kernel config for v5.3~/ i386. > > > > Ok, good, we will be "bug compatible" at the very least now :) > > > > When this gets fixed in Linus's tree we can backport the fix here as > > well. The number of users of that compiler version/configuration is > > probably pretty low at the moment to want to hold off on this fix. > > > > This is now re
Re: [RFC 1/4] counter: Simplify the count_read and count_write callbacks
On Sun, 15 Sep 2019 14:57:56 +0900
William Breathitt Gray wrote:
> The count_read and count_write callbacks are simplified to pass val as
> unsigned long rather than as an opaque data structure. The opaque
> counter_count_read_value and counter_count_write_value structures,
> counter_count_value_type enum, and relevant counter_count_read_value_set
> and counter_count_write_value_get functions, are removed as they are no
> longer used.
>
> Signed-off-by: William Breathitt Gray
Seems like a sensible bit of excessive abstraction removal to me. I'm not
totally sure why these got so complex in the first place though.
Can you recall the reason as it might help to judge why we no longer
think the same?
Thanks,
Jonathan
> ---
> drivers/counter/counter.c | 66 +--
> include/linux/counter.h | 43 +++--
> 2 files changed, 12 insertions(+), 97 deletions(-)
>
> diff --git a/drivers/counter/counter.c b/drivers/counter/counter.c
> index 106bc7180cd8..1d08f1437b1b 100644
> --- a/drivers/counter/counter.c
> +++ b/drivers/counter/counter.c
> @@ -246,60 +246,6 @@ void counter_signal_read_value_set(struct
> counter_signal_read_value *const val,
> }
> EXPORT_SYMBOL_GPL(counter_signal_read_value_set);
>
> -/**
> - * counter_count_read_value_set - set counter_count_read_value data
> - * @val: counter_count_read_value structure to set
> - * @type:property Count data represents
> - * @data:Count data
> - *
> - * This function sets an opaque counter_count_read_value structure with the
> - * provided Count data.
> - */
> -void counter_count_read_value_set(struct counter_count_read_value *const val,
> - const enum counter_count_value_type type,
> - void *const data)
> -{
> - switch (type) {
> - case COUNTER_COUNT_POSITION:
> - val->len = sprintf(val->buf, "%lu\n", *(unsigned long *)data);
> - break;
> - default:
> - val->len = 0;
> - }
> -}
> -EXPORT_SYMBOL_GPL(counter_count_read_value_set);
> -
> -/**
> - * counter_count_write_value_get - get counter_count_write_value data
> - * @data:Count data
> - * @type:property Count data represents
> - * @val: counter_count_write_value structure containing data
> - *
> - * This function extracts Count data from the provided opaque
> - * counter_count_write_value structure and stores it at the address provided
> by
> - * @data.
> - *
> - * RETURNS:
> - * 0 on success, negative error number on failure.
> - */
> -int counter_count_write_value_get(void *const data,
> - const enum counter_count_value_type type,
> - const struct counter_count_write_value *const
> val)
> -{
> - int err;
> -
> - switch (type) {
> - case COUNTER_COUNT_POSITION:
> - err = kstrtoul(val->buf, 0, data);
> - if (err)
> - return err;
> - break;
> - }
> -
> - return 0;
> -}
> -EXPORT_SYMBOL_GPL(counter_count_write_value_get);
> -
> struct counter_attr_parm {
> struct counter_device_attr_group *group;
> const char *prefix;
> @@ -788,13 +734,13 @@ static ssize_t counter_count_show(struct device *dev,
> const struct counter_count_unit *const component = devattr->component;
> struct counter_count *const count = component->count;
> int err;
> - struct counter_count_read_value val = { .buf = buf };
> + unsigned long val;
>
> err = counter->ops->count_read(counter, count, &val);
> if (err)
> return err;
>
> - return val.len;
> + return sprintf(buf, "%lu\n", val);
> }
>
> static ssize_t counter_count_store(struct device *dev,
> @@ -806,9 +752,13 @@ static ssize_t counter_count_store(struct device *dev,
> const struct counter_count_unit *const component = devattr->component;
> struct counter_count *const count = component->count;
> int err;
> - struct counter_count_write_value val = { .buf = buf };
> + unsigned long val;
> +
> + err = kstrtoul(buf, 0, &val);
> + if (err)
> + return err;
>
> - err = counter->ops->count_write(counter, count, &val);
> + err = counter->ops->count_write(counter, count, val);
> if (err)
> return err;
>
> diff --git a/include/linux/counter.h b/include/linux/counter.h
> index a061cdcdef7c..7e40796598a6 100644
> --- a/include/linux/counter.h
> +++ b/include/linux/counter.h
> @@ -300,24 +300,6 @@ struct counter_signal_read_value {
> size_t len;
> };
>
> -/**
> - * struct counter_count_read_value - Opaque Count read value
> - * @buf: string representation of Count read value
> - * @len: length of string in @buf
> - */
> -struct counter_count_read_value {
> - char *buf;
> - size_t len;
> -};
> -
> -/**
> - * struct counter_count_write_value - Opaque Count write value
> - * @buf: string r
Re: [PATCH 4.4 0/9] 4.4.193-stable review
On Sat, Sep 14, 2019 at 06:55:34AM -0700, Guenter Roeck wrote: > On 9/13/19 6:06 AM, Greg Kroah-Hartman wrote: > > This is the start of the stable review cycle for the 4.4.193 release. > > There are 9 patches in this series, all will be posted as a response > > to this one. If anyone has any issues with these being applied, please > > let me know. > > > > Responses should be made by Sun 15 Sep 2019 01:03:32 PM UTC. > > Anything received after that time might be too late. > > > > Build results: > total: 170 pass: 170 fail: 0 > Qemu test results: > total: 324 pass: 324 fail: 0 Yeah this one worked! I've done a -rc2 for all of the other trees, hopefully that resolves all of the issues you found in them. thanks, greg k-h
Re: [HELP REQUESTED from the community] Was: Staging status of speakup
On Sat, Sep 14, 2019 at 10:08:35PM +0100, Okash Khawaja wrote: > On Mon, Sep 9, 2019 at 3:55 AM Gregory Nowak wrote: > > > > On Sun, Sep 08, 2019 at 10:43:02AM +0100, Okash Khawaja wrote: > > > Sorry, I have only now got round to working on this. It's not complete > > > yet but I have assimilated the feedback and converted subjective > > > phrases, like "I think..." into objective statements or put them in > > > TODO: so that someone else may verify. I have attached it to this > > > email. > > > > I think bleeps needs a TODO, since we don't know what values it accepts, or > > what difference those values make. Also, to keep things uniform, we > > should replace my "don't know" for trigger_time with a TODO. Looks > > good to me otherwise. Thanks. > > Great thanks. I have updated. > > I have two questions: > > 1. Is it okay for these descriptions to go inside > Documentation/ABI/stable? They have been around since 2.6 (2010). Or > would we prefer Documentation/ABI/testing/? stable is fine. thanks, greg k-h
Re: [RFC 1/4] counter: Simplify the count_read and count_write callbacks
On Sun, 15 Sep 2019 14:39:17 +0100
Jonathan Cameron wrote:
> On Sun, 15 Sep 2019 14:57:56 +0900
> William Breathitt Gray wrote:
>
> > The count_read and count_write callbacks are simplified to pass val as
> > unsigned long rather than as an opaque data structure. The opaque
> > counter_count_read_value and counter_count_write_value structures,
> > counter_count_value_type enum, and relevant counter_count_read_value_set
> > and counter_count_write_value_get functions, are removed as they are no
> > longer used.
> >
> > Signed-off-by: William Breathitt Gray
>
> Seems like a sensible bit of excessive abstraction removal to me. I'm not
> totally sure why these got so complex in the first place though.
Ah. I should have read the cover letter rather than just diving in the code :)
All explained there I see.
>
> Can you recall the reason as it might help to judge why we no longer
> think the same?
>
> Thanks,
>
> Jonathan
> > ---
> > drivers/counter/counter.c | 66 +--
> > include/linux/counter.h | 43 +++--
> > 2 files changed, 12 insertions(+), 97 deletions(-)
> >
> > diff --git a/drivers/counter/counter.c b/drivers/counter/counter.c
> > index 106bc7180cd8..1d08f1437b1b 100644
> > --- a/drivers/counter/counter.c
> > +++ b/drivers/counter/counter.c
> > @@ -246,60 +246,6 @@ void counter_signal_read_value_set(struct
> > counter_signal_read_value *const val,
> > }
> > EXPORT_SYMBOL_GPL(counter_signal_read_value_set);
> >
> > -/**
> > - * counter_count_read_value_set - set counter_count_read_value data
> > - * @val: counter_count_read_value structure to set
> > - * @type: property Count data represents
> > - * @data: Count data
> > - *
> > - * This function sets an opaque counter_count_read_value structure with the
> > - * provided Count data.
> > - */
> > -void counter_count_read_value_set(struct counter_count_read_value *const
> > val,
> > - const enum counter_count_value_type type,
> > - void *const data)
> > -{
> > - switch (type) {
> > - case COUNTER_COUNT_POSITION:
> > - val->len = sprintf(val->buf, "%lu\n", *(unsigned long *)data);
> > - break;
> > - default:
> > - val->len = 0;
> > - }
> > -}
> > -EXPORT_SYMBOL_GPL(counter_count_read_value_set);
> > -
> > -/**
> > - * counter_count_write_value_get - get counter_count_write_value data
> > - * @data: Count data
> > - * @type: property Count data represents
> > - * @val: counter_count_write_value structure containing data
> > - *
> > - * This function extracts Count data from the provided opaque
> > - * counter_count_write_value structure and stores it at the address
> > provided by
> > - * @data.
> > - *
> > - * RETURNS:
> > - * 0 on success, negative error number on failure.
> > - */
> > -int counter_count_write_value_get(void *const data,
> > - const enum counter_count_value_type type,
> > - const struct counter_count_write_value *const
> > val)
> > -{
> > - int err;
> > -
> > - switch (type) {
> > - case COUNTER_COUNT_POSITION:
> > - err = kstrtoul(val->buf, 0, data);
> > - if (err)
> > - return err;
> > - break;
> > - }
> > -
> > - return 0;
> > -}
> > -EXPORT_SYMBOL_GPL(counter_count_write_value_get);
> > -
> > struct counter_attr_parm {
> > struct counter_device_attr_group *group;
> > const char *prefix;
> > @@ -788,13 +734,13 @@ static ssize_t counter_count_show(struct device *dev,
> > const struct counter_count_unit *const component = devattr->component;
> > struct counter_count *const count = component->count;
> > int err;
> > - struct counter_count_read_value val = { .buf = buf };
> > + unsigned long val;
> >
> > err = counter->ops->count_read(counter, count, &val);
> > if (err)
> > return err;
> >
> > - return val.len;
> > + return sprintf(buf, "%lu\n", val);
> > }
> >
> > static ssize_t counter_count_store(struct device *dev,
> > @@ -806,9 +752,13 @@ static ssize_t counter_count_store(struct device *dev,
> > const struct counter_count_unit *const component = devattr->component;
> > struct counter_count *const count = component->count;
> > int err;
> > - struct counter_count_write_value val = { .buf = buf };
> > + unsigned long val;
> > +
> > + err = kstrtoul(buf, 0, &val);
> > + if (err)
> > + return err;
> >
> > - err = counter->ops->count_write(counter, count, &val);
> > + err = counter->ops->count_write(counter, count, val);
> > if (err)
> > return err;
> >
> > diff --git a/include/linux/counter.h b/include/linux/counter.h
> > index a061cdcdef7c..7e40796598a6 100644
> > --- a/include/linux/counter.h
> > +++ b/include/linux/counter.h
> > @@ -300,24 +300,6 @@ struct counter_signal_read_value {
> > size_t len;
> > };
> >
> > -/**
> > - * st
Re: printk meeting at LPC
On 2019-09-13, Daniel Vetter wrote:
>> 2. A kernel thread will be created for each registered console, each
>> responsible for being the sole printers to their respective
>> consoles. With this, console printing is _fully_ decoupled from
>> printk() callers.
>
> Is the plan to split the console_lock up into a per-console thing? Or
> postponed for later on?
AFAICT, the only purpose for a console_lock would be to synchronize
between the console printing kthread and some other component that wants
to write to that same device. So a per-console console_lock should be
the proper solution. However, I will look into the details. My main
concerns about this are the suspend/resume logic and the code sitting
behind /dev/console. I will share details once I've sorted it all out.
>> 6. A new may-sleep function pr_flush() will be made available to wait
>> for all previously printk'd messages to be output on all consoles
>> before proceeding. For example:
>>
>> pr_cont("Running test ABC... ");
>> pr_flush();
>>
>> do_test();
>>
>> pr_cont("PASSED\n");
>> pr_flush();
>
> Just crossed my mind: Could/should we lockdep-annotate pr_flush (take
> a lockdep map in there that we also take around the calls down into
> console drivers in each of the console printing kthreads or something
> like that)? Just to avoid too many surprises when people call pr_flush
> from within gpu drivers and wonder why it doesn't work so well.
Why would it not work so well? Basically the task calling pr_flush()
will monitor the lockless iterators of the various consoles until _all_
have hit/passed the latest sequence number from the time of the call.
> Although with this nice plan we'll take the modeset paths fully out of
> the printk paths (even for normal outputs) I hope, so should be a lot
> more reasonable.
You will be running in your own preemptible kthread, so any paths you
take should be safe.
> From gpu perspective this all sounds extremely good and first
> realistic plan that might lead us to an actually working bsod on
> linux.
Are you planning on basing the bsod stuff on write_atomic() (which is
used after entering an emergency state) or on the kmsg_dump facility? I
would expect kmsg_dump might be more appropriate, unless there are
concerns that the machine will die before getting that far (i.e. there
is a lot that happens between when an OOPS begins and when kmsg_dumpers
are invoked).
John Ogness
Re: [PATCH] staging: exfat: add exfat filesystem code to
On Sat, Sep 14, 2019 at 10:39:51PM +0900, Park Ju Hyung wrote: > Hi. > > I just noticed that this exfat-staging drivers are based on the old > Samsung's 1.x exFAT drivers. > > I've been working to get the newer Samsung's driver(now named "sdFAT") > to fit better for general Linux users, and I believe it can provide a > better base for the community to work on(and hopefully complies better > to the mainline coding standard). > > GitHub link > https://github.com/arter97/exfat-linux > > I also included some rudimentary benchmark results. > > I encourage mainline developers to explore this driver base and see if > it's worth to switch, since it's the early days of exfat-staging. Note, this just showed up publically on August 12, where were you with all of this new code before then? :) > To others watching this thread: > It's more than likely that you can start using exFAT reliably right > away by following the link above. It's tested on all major LTS kernels > ranging from 3.4 to 4.19 and the ones Canonical uses for Ubuntu: 3.4, > 3.10, 3.18, 4.1, 4.4, 4.9, 4.14, 4.19 and 4.15, 5.0, 5.2, and 5.3-rc. For the in-kernel code, we would have to rip out all of the work you did for all older kernels, so that's a non-starter right there. As for what codebase to work off of, I don't want to say it is too late, but really, this shows up from nowhere and we had to pick something so we found the best we could at that point in time. Is there anything specific in the codebase you have now, that is lacking in the in-kernel code? Old-kernel-support doesn't count here, as we don't care about that as it is not applicable. But functionality does matter, what has been added here that we can make use of? And do you have any "real" development history to look at instead of the "one giant commit" of the initial code drop? That is where we could actually learn what has changed over time. Your repo as-is shows none of the interesting bits :( thanks, greg kh
Re: [PATCH v2 1/4] task: Add a count of task rcu users
On Sat, Sep 14, 2019 at 07:33:34AM -0500, Eric W. Biederman wrote:
>
> Add a count of the number of rcu users (currently 1) of the task
> struct so that we can later add the scheduler case and get rid of the
> very subtle task_rcu_dereference, and just use rcu_dereference.
>
> As suggested by Oleg have the count overlap rcu_head so that no
> additional space in task_struct is required.
>
> Inspired-by: Linus Torvalds
> Inspired-by: Oleg Nesterov
> Signed-off-by: "Eric W. Biederman"
> ---
> include/linux/sched.h | 5 -
> include/linux/sched/task.h | 1 +
> kernel/exit.c | 7 ++-
> kernel/fork.c | 7 +++
> 4 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 9f51932bd543..99a4518b9b17 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1142,7 +1142,10 @@ struct task_struct {
>
> struct tlbflush_unmap_batch tlb_ubc;
>
> - struct rcu_head rcu;
> + union {
> + refcount_t rcu_users;
> + struct rcu_head rcu;
> + };
>
> /* Cache last used pipe for splice(): */
> struct pipe_inode_info *splice_pipe;
> diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h
> index 0497091e40c1..4c44c37236b2 100644
> --- a/include/linux/sched/task.h
> +++ b/include/linux/sched/task.h
> @@ -116,6 +116,7 @@ static inline void put_task_struct(struct task_struct *t)
> }
>
> struct task_struct *task_rcu_dereference(struct task_struct **ptask);
> +void put_task_struct_rcu_user(struct task_struct *task);
>
> #ifdef CONFIG_ARCH_WANTS_DYNAMIC_TASK_STRUCT
> extern int arch_task_struct_size __read_mostly;
> diff --git a/kernel/exit.c b/kernel/exit.c
> index 5b4a5dcce8f8..2e259286f4e7 100644
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -182,6 +182,11 @@ static void delayed_put_task_struct(struct rcu_head *rhp)
> put_task_struct(tsk);
> }
>
> +void put_task_struct_rcu_user(struct task_struct *task)
> +{
> + if (refcount_dec_and_test(&task->rcu_users))
> + call_rcu(&task->rcu, delayed_put_task_struct);
We "instantly" transition from the union being ->rcu_user to being ->rcu,
so there needs to be some mechanism that has previously made sure that
nothing is going to attempt to use ->rcu on this task.
We cannot be relying solely on something like atomic_add_unless(),
because call_rcu() will likely result in ->rcu being non-zero.
> +}
>
> void release_task(struct task_struct *p)
> {
> @@ -222,7 +227,7 @@ void release_task(struct task_struct *p)
>
> write_unlock_irq(&tasklist_lock);
> release_thread(p);
> - call_rcu(&p->rcu, delayed_put_task_struct);
> + put_task_struct_rcu_user(p);
This, along with the pre-existing initialization of ->rcu_user to two
below gives some hope, at least assuming that release_task() is invoked
after no one can possibly try to increment ->rcu_user. And in v5.2,
release_task() is invoked from these places:
o de_thread(). On this one, I must defer to Oleg, Peter, and crew.
It might be that the list removals while write-holding the
tasklist_lock do the trick, but that assumes that this lock is
involved in acquiring a reference.
o find_child_reaper(). This is invoked via exit_notify() from
do_exit(), just after the call to exit_tasks_rcu_start().
This is OK from a Tasks RCU perspective because it is using
separate synchornization. Something earlier must prevent
new ->rcu_user references.
o wait_task_zombie(). Here is hoping that the check for
EXIT_DEAD is helpful here.
I am not seeing how this would be safe, but then again this is only the
first patch. Plus there is much about this use case that I do not know.
OK, on to the other patches...
Thanx, Paul
>
> p = leader;
> if (unlikely(zap_leader))
> diff --git a/kernel/fork.c b/kernel/fork.c
> index 2852d0e76ea3..9f04741d5c70 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -900,10 +900,9 @@ static struct task_struct *dup_task_struct(struct
> task_struct *orig, int node)
> if (orig->cpus_ptr == &orig->cpus_mask)
> tsk->cpus_ptr = &tsk->cpus_mask;
>
> - /*
> - * One for us, one for whoever does the "release_task()" (usually
> - * parent)
> - */
> + /* One for the user space visible state that goes away when reaped. */
> + refcount_set(&tsk->rcu_users, 1);
> + /* One for the rcu users, and one for the scheduler */
> refcount_set(&tsk->usage, 2);
> #ifdef CONFIG_BLK_DEV_IO_TRACE
> tsk->btrace_seq = 0;
> --
> 2.21.0.dirty
>
Re: [PATCH] staging: bcm2835-audio: Fix draining behavior regression
Hi Takashi,
Am 14.09.19 um 17:24 schrieb Takashi Iwai:
> The PCM draining behavior got broken since the recent refactoring, and
> this turned out to be the incorrect expectation of the firmware
> behavior regarding "draining". While I expected the "drain" flag at
> the stop operation would do processing the queued samples, it seems
> rather dropping the samples.
>
> As a quick fix, just drop the SNDRV_PCM_INFO_DRAIN_TRIGGER flag, so
> that the driver uses the normal PCM draining procedure. Also, put
> some caution comment to the function for future readers not to fall
> into the same pitfall.
>
> Fixes: d7ca3a71545b ("staging: bcm2835-audio: Operate non-atomic PCM ops")
> BugLink: https://github.com/raspberrypi/linux/issues/2983
thanks for taking care of this. Wouldn't it be better to add the link to
the new comment to provide more context of the unexpected behavior?
Nevertheless:
Acked-by: Stefan Wahren
> Cc: sta...@vger.kernel.org
> Signed-off-by: Takashi Iwai
> ---
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c | 4 ++--
> drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c | 1 +
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
> b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
> index bc1eaa3a0773..826016c3431a 100644
> --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
> +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
> @@ -12,7 +12,7 @@
> static const struct snd_pcm_hardware snd_bcm2835_playback_hw = {
> .info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER |
>SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
> - SNDRV_PCM_INFO_DRAIN_TRIGGER | SNDRV_PCM_INFO_SYNC_APPLPTR),
> + SNDRV_PCM_INFO_SYNC_APPLPTR),
> .formats = SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S16_LE,
> .rates = SNDRV_PCM_RATE_CONTINUOUS | SNDRV_PCM_RATE_8000_48000,
> .rate_min = 8000,
> @@ -29,7 +29,7 @@ static const struct snd_pcm_hardware
> snd_bcm2835_playback_hw = {
> static const struct snd_pcm_hardware snd_bcm2835_playback_spdif_hw = {
> .info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER |
>SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
> - SNDRV_PCM_INFO_DRAIN_TRIGGER | SNDRV_PCM_INFO_SYNC_APPLPTR),
> + SNDRV_PCM_INFO_SYNC_APPLPTR),
> .formats = SNDRV_PCM_FMTBIT_S16_LE,
> .rates = SNDRV_PCM_RATE_CONTINUOUS | SNDRV_PCM_RATE_44100 |
> SNDRV_PCM_RATE_48000,
> diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> index 23fba01107b9..c6f9cf1913d2 100644
> --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> @@ -289,6 +289,7 @@ int bcm2835_audio_stop(struct bcm2835_alsa_stream
> *alsa_stream)
>VC_AUDIO_MSG_TYPE_STOP, false);
> }
>
> +/* FIXME: this doesn't seem working as expected for "draining" */
> int bcm2835_audio_drain(struct bcm2835_alsa_stream *alsa_stream)
> {
> struct vc_audio_msg m = {
Re: [PATCH] ARM: dts: imx6dl: SolidRun: add phy node with 100Mb/s max-speed
> Tinywrkb confirmed to me in private communication that revert of > 5502b218e001 fixes Ethernet for him on effected system. > > He also referred me to an old Cubox-i spec that lists 10/100 Ethernet > only for i.MX6 Solo/DualLite variants of Cubox-i. It turns out that > there was a plan to use a different 10/100 PHY for Solo/DualLite > SOMs. This plan never materialized. All SolidRun i.MX6 SOMs use the same > AR8035 PHY that supports 1Gb. > > Commit 5502b218e001 might be triggering a hardware issue on the affected > Cubox-i. I could not reproduce the issue here with Cubox-i and a Dual > SOM variant running v5.3-rc8. I have no Solo/DualLite variant handy at > the moment. Could somebody with an affected device show us the output of ethtool with and without 5502b218e001. Does one show 1G has been negotiated, and the other 100Mbps? If this is true, how does it get 100Mbps without that patch? We are missing a piece of the puzzle. Andrew
Re: [PATCH v5 1/9] leds: multicolor: Add sysfs interface definition
Hi Dan, On 9/11/19 8:01 PM, Dan Murphy wrote: > Add a documentation of LED Multicolor LED class specific > sysfs attributes. > > Signed-off-by: Dan Murphy > --- > .../ABI/testing/sysfs-class-led-multicolor| 73 +++ > 1 file changed, 73 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-class-led-multicolor > > diff --git a/Documentation/ABI/testing/sysfs-class-led-multicolor > b/Documentation/ABI/testing/sysfs-class-led-multicolor > new file mode 100644 > index ..4ea54c2ad4c8 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-class-led-multicolor > @@ -0,0 +1,73 @@ > +What:/sys/class/leds//brightness > +Date:Sept 2019 > +KernelVersion: 5.5 > +Contact: Dan Murphy > +Description: read/write > + The multicolor class will redirect the device drivers call back > + function for brightness control to the multicolor class > + brightness control function. > + > + Writing to this file will update all LEDs within the group to a > + calculated percentage of what each color LED in the group is set > + to. The percentage is calculated via the equation below: > + > + led_brightness = requested_value * > led_color_intensity/led_color_max_intensity > + > + For additional details please refer to > + Documentation/leds/leds-class-multicolor.rst. > + > + The value of the color is from 0 to > + /sys/class/leds//max_brightness. > + > +What:/sys/class/leds//colors/color_mix > +Date:Sept 2019 > +KernelVersion: 5.5 > +Contact: Dan Murphy > +Description: read/write > + The values written to this file should contain the intensity > + values of each multicolor LED within the colors directory. The > + index of given color is reported by the color_id file present in > + colors/ directory. The index determines the position in > + the sequence of intensities on which the related intensity > + should be passed to this file. > + > + For additional details please refer to > + Documentation/leds/leds-class-multicolor.rst. As already mentioned in the reply to Pavel - let's avoid the introduction of another sysfs file with multiple values. Color intensity files will only cache the value and actual write will be performed upon writing brightness file. > + > +What:/sys/class/leds//colors//color_id > +Date:Sept 2019 > +KernelVersion: 5.5 > +Contact: Dan Murphy > +Description: read only > + This file when read will return the index of the color in the > + color_mix. > + > + For additional details please refer to > + Documentation/leds/leds-class-multicolor.rst. > + > +What:/sys/class/leds//colors//intensity > +Date:Sept 2019 > +KernelVersion: 5.5 > +Contact: Dan Murphy > +Description: read/write > + The led_color directory is dynamically created based on the > + colors defined by the registrar of the class. > + The led_color can be but not limited to red, green, blue, > + white, amber, yellow and violet. There is one directory per > color > + presented. The brightness file is created under each > + led_color directory and controls the individual LED color > + setting. > + > + The value of the color is from 0 to > + /sys/class/leds//colors//max_intensity. > + > +What:/sys/class/leds//colors//max_intensity > +Date:Sept 2019 > +KernelVersion: 5.5 > +Contact: Dan Murphy > +Description: read only > + Maximum intensity level for the LED color, default is > + 255 (LED_FULL). > + > + If the LED does not support different intensity levels, this > + should be 1. > -- Best regards, Jacek Anaszewski
Re: [RFC 1/4] counter: Simplify the count_read and count_write callbacks
On Sun, Sep 15, 2019 at 02:47:00PM +0100, Jonathan Cameron wrote: > On Sun, 15 Sep 2019 14:39:17 +0100 > Jonathan Cameron wrote: > > > On Sun, 15 Sep 2019 14:57:56 +0900 > > William Breathitt Gray wrote: > > > > > The count_read and count_write callbacks are simplified to pass val as > > > unsigned long rather than as an opaque data structure. The opaque > > > counter_count_read_value and counter_count_write_value structures, > > > counter_count_value_type enum, and relevant counter_count_read_value_set > > > and counter_count_write_value_get functions, are removed as they are no > > > longer used. > > > > > > Signed-off-by: William Breathitt Gray > > > > Seems like a sensible bit of excessive abstraction removal to me. I'm not > > totally sure why these got so complex in the first place though. > Ah. I should have read the cover letter rather than just diving in the code :) > All explained there I see. > > > > > Can you recall the reason as it might help to judge why we no longer > > think the same? > > > > Thanks, > > > > Jonathan The cover letter probably explains it well enough, but it may be good anyway for posterity to add on a bit about the origins of the opaque structures. If I recall correctly, it was from early on when I had a dedicated set of functions for "quadrature counters" as opposed to "simple counters" -- eventually we abandoned that design and decided instead to keep the interface simple since devices could be represented robustly enough with just the core Count, Signal, and Synapse components. I decided to keep the opaque structures anyway in the hope that they could be used in the future to restrict the attributes exposed to a certain set of "counter types" defined for the Counter subsystems; i.e. "position" counters would only expose spatial coordinates, "tally" counters would expose tally units, etc. However, I've come to realized that this type of organization is best left to the userspace application and that the kernelspace code should instead be focused on maintaining a simple and robust interface for representing the core concept of a "counter" device. William Breathitt Gray
Re: [PATCH v5 3/9] dt: bindings: Add multicolor class dt bindings documention
Dan,
This patch has the same issues I mentioned in the v4 review [0].
On 9/11/19 8:01 PM, Dan Murphy wrote:
> Add DT bindings for the LEDs multicolor class framework.
>
> Signed-off-by: Dan Murphy
> ---
> .../bindings/leds/leds-class-multicolor.txt | 96 +++
> 1 file changed, 96 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/leds/leds-class-multicolor.txt
>
> diff --git a/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt
> b/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt
> new file mode 100644
> index ..5d36327b18fc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-class-multicolor.txt
> @@ -0,0 +1,96 @@
> +* Multicolor LED properties
> +
> +Multicolor LEDs can consist of a RGB, RGBW or a RGBA LED clusters. These
> devices
> +can be grouped together and also provide a modeling mechanism so that the
> +cluster LEDs can vary in hue and intensity to produce a wide range of colors.
> +
> +The nodes and properties defined in this document are unique to the
> multicolor
> +LED class. Common LED nodes and properties are inherited from the common.txt
> +within this documentation directory.
> +
> +Required LED Child properties:
> + - color : For multicolor LED support this property should be defined as
> + LED_COLOR_ID_MULTI and further definition can be found in
> + include/linux/leds/common.h.
> +
> +led-controller@30 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "ti,lp5024";
> + reg = <0x29>;
> +
> + multi-led@1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <1>;
> + color = ;
> + function = LED_FUNCTION_;
> +
> +
> + led@3 {
> + reg = <3>;
> + color = ;
> + };
> +
> + led@4 {
> + reg = <4>;
> + color = ;
> + };
> +
> + led@5 {
> + reg = <5>;
> + color = ;
> + };
> + };
> +
> + multi-led@2 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + color = ;
> + function = LED_FUNCTION_ACTIVITY;
> + reg = <2>;
> + ti,led-bank = <2 3 5>;
> +
> + led@6 {
> + reg = <0x6>;
> + color = ;
> + led-sources = <6 9 15>;
> + };
> +
> + led@7 {
> + reg = <0x7>;
> + color = ;
> + led-sources = <7 10 16>;
> + };
> +
> + led@8 {
> + reg = <0x8>;
> + color = ;
> + led-sources = <8 11 17>;
> + };
> + };
> +
> + multi-led@4 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <4>;
> + color = ;
> + function = LED_FUNCTION_ACTIVITY;
> +
> + led@12 {
> + reg = <12>;
> + color = ;
> + };
> +
> + led@13 {
> + reg = <13>;
> + color = ;
> + };
> +
> + led@14 {
> + reg = <14>;
> + color = ;
> + };
> + };
> +
> +};
>
[0]
https://lore.kernel.org/linux-leds/684010e7-a47c-2bc8-2bf6-c632be316...@gmail.com/
--
Best regards,
Jacek Anaszewski
[PATCH v2] usbip: vhci_hcd indicate failed message
If the return value of vhci_init_attr_group and
sysfs_create_group is non-zero, which mean they failed
to init attr_group and create sysfs group, so it would
better add 'failed' message to indicate that.
This patch also change pr_err to dev_err to trace which
device is failed.
Fixes: 0775a9cbc694 ("usbip: vhci extension: modifications to vhci driver")
Signed-off-by: Mao Wenan
---
v2: change pr_err to dev_err.
drivers/usb/usbip/vhci_hcd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index 000ab7225717..bea28ec846ee 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -1185,12 +1185,12 @@ static int vhci_start(struct usb_hcd *hcd)
if (id == 0 && usb_hcd_is_primary_hcd(hcd)) {
err = vhci_init_attr_group();
if (err) {
- pr_err("init attr group\n");
+ dev_err(hcd_dev(hcd), "init attr group failed\n");
return err;
}
err = sysfs_create_group(&hcd_dev(hcd)->kobj, &vhci_attr_group);
if (err) {
- pr_err("create sysfs files\n");
+ dev_err(hcd_dev(hcd), "create sysfs files failed\n");
vhci_finish_attr_group();
return err;
}
--
2.20.1
Re: [PATCH] ARM: dts: imx6dl: SolidRun: add phy node with 100Mb/s max-speed
On Sun, Sep 15, 2019 at 03:56:52PM +0200, Andrew Lunn wrote: > > Tinywrkb confirmed to me in private communication that revert of > > 5502b218e001 fixes Ethernet for him on effected system. > > > > He also referred me to an old Cubox-i spec that lists 10/100 Ethernet > > only for i.MX6 Solo/DualLite variants of Cubox-i. It turns out that > > there was a plan to use a different 10/100 PHY for Solo/DualLite > > SOMs. This plan never materialized. All SolidRun i.MX6 SOMs use the same > > AR8035 PHY that supports 1Gb. > > > > Commit 5502b218e001 might be triggering a hardware issue on the affected > > Cubox-i. I could not reproduce the issue here with Cubox-i and a Dual > > SOM variant running v5.3-rc8. I have no Solo/DualLite variant handy at > > the moment. > > Could somebody with an affected device show us the output of ethtool > with and without 5502b218e001. Does one show 1G has been negotiated, > and the other 100Mbps? If this is true, how does it get 100Mbps > without that patch? We are missing a piece of the puzzle. Hang on. 5502b218e001 is in 5.2 already - it was merged as part of the v5.1 merge window. That means my imx6 Solo Hummingboard is already running it with the AR8035 PHY, and it works fine. # dmesg ... OF: fdt: Machine model: SolidRun HummingBoard Solo/DualLite ... # ethtool eth0 Settings for eth0: Supported ports: [ TP MII ] Supported link modes: 10baseT/Half 10baseT/Full 100baseT/Half 100baseT/Full 1000baseT/Full Supported pause frame use: Symmetric Supports auto-negotiation: Yes Supported FEC modes: Not reported Advertised link modes: 10baseT/Half 10baseT/Full 100baseT/Half 100baseT/Full 1000baseT/Full Advertised pause frame use: Symmetric Advertised auto-negotiation: Yes Advertised FEC modes: Not reported Link partner advertised link modes: 10baseT/Half 10baseT/Full 100baseT/Half 100baseT/Full 1000baseT/Full Link partner advertised pause frame use: Symmetric Link partner advertised auto-negotiation: Yes Link partner advertised FEC modes: Not reported Speed: 1000Mb/s Duplex: Full Port: MII PHYAD: 0 Transceiver: internal Auto-negotiation: on Supports Wake-on: d Wake-on: d Link detected: yes -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
Re: [PATCH v2 2/4] task: Ensure tasks are available for a grace period after leaving the runqueue
On Sat, Sep 14, 2019 at 07:33:58AM -0500, Eric W. Biederman wrote: > > In the ordinary case today the rcu grace period for a task_struct is > triggered when another process wait's for it's zombine and causes the > kernel to call release_task(). As the waiting task has to receive a > signal and then act upon it before this happens, typically this will > occur after the original task as been removed from the runqueue. > > Unfortunaty in some cases such as self reaping tasks it can be shown > that release_task() will be called starting the grace period for > task_struct long before the task leaves the runqueue. > > Therefore use put_task_struct_rcu_user in finish_task_switch to > guarantee that the there is a rcu lifetime after the task > leaves the runqueue. > > Besides the change in the start of the rcu grace period for the > task_struct this change may cause perf_event_delayed_put and > trace_sched_process_free. The function perf_event_delayed_put boils > down to just a WARN_ON for cases that I assume never show happen. So > I don't see any problem with delaying it. > > The function trace_sched_process_free is a trace point and thus > visible to user space. Occassionally userspace has the strangest > dependencies so this has a miniscule chance of causing a regression. > This change only changes the timing of when the tracepoint is called. > The change in timing arguably gives userspace a more accurate picture > of what is going on. So I don't expect there to be a regression. > > In the case where a task self reaps we are pretty much guaranteed that > the rcu grace period is delayed. So we should get quite a bit of > coverage in of this worst case for the change in a normal threaded > workload. So I expect any issues to turn up quickly or not at all. > > I have lightly tested this change and everything appears to work > fine. > > Inspired-by: Linus Torvalds > Inspired-by: Oleg Nesterov > Signed-off-by: "Eric W. Biederman" > --- > kernel/fork.c | 11 +++ > kernel/sched/core.c | 2 +- > 2 files changed, 8 insertions(+), 5 deletions(-) > > diff --git a/kernel/fork.c b/kernel/fork.c > index 9f04741d5c70..7a74ade4e7d6 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -900,10 +900,13 @@ static struct task_struct *dup_task_struct(struct > task_struct *orig, int node) > if (orig->cpus_ptr == &orig->cpus_mask) > tsk->cpus_ptr = &tsk->cpus_mask; > > - /* One for the user space visible state that goes away when reaped. */ > - refcount_set(&tsk->rcu_users, 1); > - /* One for the rcu users, and one for the scheduler */ > - refcount_set(&tsk->usage, 2); > + /* > + * One for the user space visible state that goes away when reaped. > + * One for the scheduler. > + */ > + refcount_set(&tsk->rcu_users, 2); OK, this would allow us to add a later decrement-and-test of ->rcu_users ... > + /* One for the rcu users */ > + refcount_set(&tsk->usage, 1); > #ifdef CONFIG_BLK_DEV_IO_TRACE > tsk->btrace_seq = 0; > #endif > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 2b037f195473..69015b7c28da 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -3135,7 +3135,7 @@ static struct rq *finish_task_switch(struct task_struct > *prev) > /* Task is done with its stack. */ > put_task_stack(prev); > > - put_task_struct(prev); > + put_task_struct_rcu_user(prev); ... which is here. And this looks to be invoked from the __schedule() called from do_task_dead() at the very end of do_exit(). This looks plausible, but still requires that it no longer be possible to enter an RCU read-side critical section that might increment ->rcu_users after this point in time. This might be enforced by a grace period between the time that the task was removed from its lists and the current time (seems unlikely, though, in that case why bother with call_rcu()?) or by some other synchronization. On to the next patch! Thanx, Paul > } > > tick_nohz_task_switch(); > -- > 2.21.0.dirty >
Re: [PATCH v2 2/4] task: Ensure tasks are available for a grace period after leaving the runqueue
On Sun, Sep 15, 2019 at 07:07:52AM -0700, Paul E. McKenney wrote: > On Sat, Sep 14, 2019 at 07:33:58AM -0500, Eric W. Biederman wrote: > > > > In the ordinary case today the rcu grace period for a task_struct is > > triggered when another process wait's for it's zombine and causes the Oh, and "waits for its", just to hit the grammar en passant... ;-) Thanx, Paul > > kernel to call release_task(). As the waiting task has to receive a > > signal and then act upon it before this happens, typically this will > > occur after the original task as been removed from the runqueue. > > > > Unfortunaty in some cases such as self reaping tasks it can be shown > > that release_task() will be called starting the grace period for > > task_struct long before the task leaves the runqueue. > > > > Therefore use put_task_struct_rcu_user in finish_task_switch to > > guarantee that the there is a rcu lifetime after the task > > leaves the runqueue. > > > > Besides the change in the start of the rcu grace period for the > > task_struct this change may cause perf_event_delayed_put and > > trace_sched_process_free. The function perf_event_delayed_put boils > > down to just a WARN_ON for cases that I assume never show happen. So > > I don't see any problem with delaying it. > > > > The function trace_sched_process_free is a trace point and thus > > visible to user space. Occassionally userspace has the strangest > > dependencies so this has a miniscule chance of causing a regression. > > This change only changes the timing of when the tracepoint is called. > > The change in timing arguably gives userspace a more accurate picture > > of what is going on. So I don't expect there to be a regression. > > > > In the case where a task self reaps we are pretty much guaranteed that > > the rcu grace period is delayed. So we should get quite a bit of > > coverage in of this worst case for the change in a normal threaded > > workload. So I expect any issues to turn up quickly or not at all. > > > > I have lightly tested this change and everything appears to work > > fine. > > > > Inspired-by: Linus Torvalds > > Inspired-by: Oleg Nesterov > > Signed-off-by: "Eric W. Biederman" > > --- > > kernel/fork.c | 11 +++ > > kernel/sched/core.c | 2 +- > > 2 files changed, 8 insertions(+), 5 deletions(-) > > > > diff --git a/kernel/fork.c b/kernel/fork.c > > index 9f04741d5c70..7a74ade4e7d6 100644 > > --- a/kernel/fork.c > > +++ b/kernel/fork.c > > @@ -900,10 +900,13 @@ static struct task_struct *dup_task_struct(struct > > task_struct *orig, int node) > > if (orig->cpus_ptr == &orig->cpus_mask) > > tsk->cpus_ptr = &tsk->cpus_mask; > > > > - /* One for the user space visible state that goes away when reaped. */ > > - refcount_set(&tsk->rcu_users, 1); > > - /* One for the rcu users, and one for the scheduler */ > > - refcount_set(&tsk->usage, 2); > > + /* > > +* One for the user space visible state that goes away when reaped. > > +* One for the scheduler. > > +*/ > > + refcount_set(&tsk->rcu_users, 2); > > OK, this would allow us to add a later decrement-and-test of > ->rcu_users ... > > > + /* One for the rcu users */ > > + refcount_set(&tsk->usage, 1); > > #ifdef CONFIG_BLK_DEV_IO_TRACE > > tsk->btrace_seq = 0; > > #endif > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > > index 2b037f195473..69015b7c28da 100644 > > --- a/kernel/sched/core.c > > +++ b/kernel/sched/core.c > > @@ -3135,7 +3135,7 @@ static struct rq *finish_task_switch(struct > > task_struct *prev) > > /* Task is done with its stack. */ > > put_task_stack(prev); > > > > - put_task_struct(prev); > > + put_task_struct_rcu_user(prev); > > ... which is here. And this looks to be invoked from the __schedule() > called from do_task_dead() at the very end of do_exit(). > > This looks plausible, but still requires that it no longer be possible to > enter an RCU read-side critical section that might increment ->rcu_users > after this point in time. This might be enforced by a grace period > between the time that the task was removed from its lists and the current > time (seems unlikely, though, in that case why bother with call_rcu()?) or > by some other synchronization. > > On to the next patch! > > Thanx, Paul > > > } > > > > tick_nohz_task_switch(); > > -- > > 2.21.0.dirty > >
Re: [RFC PATCH v3 00/16] Core scheduling v3
On Fri, Sep 13, 2019 at 07:12:52AM +0800, Aubrey Li wrote: > On Thu, Sep 12, 2019 at 8:04 PM Aaron Lu wrote: > > > > On Wed, Sep 11, 2019 at 09:19:02AM -0700, Tim Chen wrote: > > > On 9/11/19 7:02 AM, Aaron Lu wrote: > > > I think Julien's result show that my patches did not do as well as > > > your patches for fairness. Aubrey did some other testing with the same > > > conclusion. So I think keeping the forced idle time balanced is not > > > enough for maintaining fairness. > > > > Well, I have done following tests: > > 1 Julien's test script: https://paste.debian.net/plainh/834cf45c > > 2 start two tagged will-it-scale/page_fault1, see how each performs; > > 3 Aubrey's mysql test: https://github.com/aubreyli/coresched_bench.git > > > > They all show your patchset performs equally well...And consider what > > the patch does, I think they are really doing the same thing in > > different ways. > > It looks like we are not on the same page, if you don't mind, can both of > you rebase your patchset onto v5.3-rc8 and provide a public branch so I > can fetch and test it at least by my benchmark? I'm using the following branch as base which is v5.1.5 based: https://github.com/digitalocean/linux-coresched coresched-v3-v5.1.5-test And I have pushed Tim's branch to: https://github.com/aaronlu/linux coresched-v3-v5.1.5-test-tim Mine: https://github.com/aaronlu/linux coresched-v3-v5.1.5-test-core_vruntime The two branches both have two patches I have sent previouslly: https://lore.kernel.org/lkml/20190810141556.GA73644@aaronlu/ Although it has some potential performance loss as pointed out by Vineeth, I haven't got time to rework it yet.
Re: [PATCH] ARM: dts: imx6dl: SolidRun: add phy node with 100Mb/s max-speed
On Sun, Sep 15, 2019 at 03:06:39PM +0100, Russell King - ARM Linux admin wrote: > On Sun, Sep 15, 2019 at 03:56:52PM +0200, Andrew Lunn wrote: > > > Tinywrkb confirmed to me in private communication that revert of > > > 5502b218e001 fixes Ethernet for him on effected system. > > > > > > He also referred me to an old Cubox-i spec that lists 10/100 Ethernet > > > only for i.MX6 Solo/DualLite variants of Cubox-i. It turns out that > > > there was a plan to use a different 10/100 PHY for Solo/DualLite > > > SOMs. This plan never materialized. All SolidRun i.MX6 SOMs use the same > > > AR8035 PHY that supports 1Gb. > > > > > > Commit 5502b218e001 might be triggering a hardware issue on the affected > > > Cubox-i. I could not reproduce the issue here with Cubox-i and a Dual > > > SOM variant running v5.3-rc8. I have no Solo/DualLite variant handy at > > > the moment. > > > > Could somebody with an affected device show us the output of ethtool > > with and without 5502b218e001. Does one show 1G has been negotiated, > > and the other 100Mbps? If this is true, how does it get 100Mbps > > without that patch? We are missing a piece of the puzzle. > > Hang on. 5502b218e001 is in 5.2 already - it was merged as part of the > v5.1 merge window. That means my imx6 Solo Hummingboard is already > running it with the AR8035 PHY, and it works fine. > > # dmesg > ... > OF: fdt: Machine model: SolidRun HummingBoard Solo/DualLite > ... > # ethtool eth0 > Settings for eth0: > Supported ports: [ TP MII ] > Supported link modes: 10baseT/Half 10baseT/Full > 100baseT/Half 100baseT/Full > 1000baseT/Full > Supported pause frame use: Symmetric > Supports auto-negotiation: Yes > Supported FEC modes: Not reported > Advertised link modes: 10baseT/Half 10baseT/Full > 100baseT/Half 100baseT/Full > 1000baseT/Full > Advertised pause frame use: Symmetric > Advertised auto-negotiation: Yes > Advertised FEC modes: Not reported > Link partner advertised link modes: 10baseT/Half 10baseT/Full > 100baseT/Half 100baseT/Full > 1000baseT/Full > Link partner advertised pause frame use: Symmetric > Link partner advertised auto-negotiation: Yes > Link partner advertised FEC modes: Not reported > Speed: 1000Mb/s > Duplex: Full > Port: MII > PHYAD: 0 > Transceiver: internal > Auto-negotiation: on > Supports Wake-on: d > Wake-on: d > Link detected: yes For some further testing, by changing the advertisment on the DSA switch (other end of this platform's link): Link partner advertised link modes: 10baseT/Half 10baseT/Full 100baseT/Half 100baseT/Full ... Speed: 100Mb/s Duplex: Full === Link partner advertised link modes: 10baseT/Half 10baseT/Full 100baseT/Half ... Speed: 100Mb/s Duplex: Half === Link partner advertised link modes: 10baseT/Half 10baseT/Full ... Speed: 10Mb/s Duplex: Full === Link partner advertised link modes: 10baseT/Half ... Speed: 10Mb/s Duplex: Half So it looks like the commit works as it should. So there's something else going on. Note that the FEC does *not* support 1000baseT/Half. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
Re: [PATCH] irqchip/sifive-plic: add irq_mask and irq_unmask
On Thu, 12 Sep 2019 22:40:34 +0100,
Darius Rad wrote:
Hi Darius,
>
> As per the existing comment, irq_mask and irq_unmask do not need
> to do anything for the PLIC. However, the functions must exist
> (the pointers cannot be NULL) as they are not optional, based on
> the documentation (Documentation/core-api/genericirq.rst) as well
> as existing usage (e.g., include/linux/irqchip/chained_irq.h).
>
> Signed-off-by: Darius Rad
> ---
> drivers/irqchip/irq-sifive-plic.c | 13 +
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/irqchip/irq-sifive-plic.c
> b/drivers/irqchip/irq-sifive-plic.c
> index cf755964f2f8..52d5169f924f 100644
> --- a/drivers/irqchip/irq-sifive-plic.c
> +++ b/drivers/irqchip/irq-sifive-plic.c
> @@ -111,6 +111,13 @@ static void plic_irq_disable(struct irq_data *d)
> plic_irq_toggle(cpu_possible_mask, d->hwirq, 0);
> }
>
> +/*
> + * There is no need to mask/unmask PLIC interrupts. They are "masked"
> + * by reading claim and "unmasked" when writing it back.
> + */
> +static void plic_irq_mask(struct irq_data *d) { }
> +static void plic_irq_unmask(struct irq_data *d) { }
This outlines a bigger issue. If your irqchip doesn't require
mask/unmask, you're probably not using the right interrupt
flow. Looking at the code, I see you're using handle_simple_irq, which
is almost universally wrong.
As per the description above, these interrupts should be using the
fasteoi flow, which is designed for this exact behaviour (the
interrupt controller knows which interrupt is in flight and doesn't
require SW to do anything bar signalling the EOI).
Another thing is that mask/unmask tends to be a requirement, while
enable/disable tends to be optional. There is no hard line here, but
the expectations are that:
(a) A disabled line can drop interrupts
(b) A masked line cannot drop interrupts
Depending what the PLIC architecture mandates, you'll need to
implement one and/or the other. Having just (a) is indicative of a HW
bug, and I'm not assuming that this is the case. (b) only is pretty
common, and (a)+(b) has a few adepts. My bet is that it requires (b)
only.
> +
> #ifdef CONFIG_SMP
> static int plic_set_affinity(struct irq_data *d,
>const struct cpumask *mask_val, bool force)
> @@ -138,12 +145,10 @@ static int plic_set_affinity(struct irq_data *d,
>
> static struct irq_chip plic_chip = {
> .name = "SiFive PLIC",
> - /*
> - * There is no need to mask/unmask PLIC interrupts. They are "masked"
> - * by reading claim and "unmasked" when writing it back.
> - */
> .irq_enable = plic_irq_enable,
> .irq_disable= plic_irq_disable,
> + .irq_mask = plic_irq_mask,
> + .irq_unmask = plic_irq_unmask,
> #ifdef CONFIG_SMP
> .irq_set_affinity = plic_set_affinity,
> #endif
Can you give the following patch a go? It brings the irq flow in line
with what the HW can do. It is of course fully untested (not even
compile tested...).
Thanks,
M.
>From c0ce33a992ec18f5d3bac7f70de62b1ba2b42090 Mon Sep 17 00:00:00 2001
From: Marc Zyngier
Date: Sun, 15 Sep 2019 15:17:45 +0100
Subject: [PATCH] irqchip/sifive-plic: Switch to fasteoi flow
The SiFive PLIC interrupt controller seems to have all the HW
features to support the fasteoi flow, but the driver seems to be
stuck in a distant past. Bring it into the 21st century.
Signed-off-by: Marc Zyngier
---
drivers/irqchip/irq-sifive-plic.c | 29 +++--
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/irqchip/irq-sifive-plic.c
b/drivers/irqchip/irq-sifive-plic.c
index cf755964f2f8..8fea384d392b 100644
--- a/drivers/irqchip/irq-sifive-plic.c
+++ b/drivers/irqchip/irq-sifive-plic.c
@@ -97,7 +97,7 @@ static inline void plic_irq_toggle(const struct cpumask *mask,
}
}
-static void plic_irq_enable(struct irq_data *d)
+static void plic_irq_mask(struct irq_data *d)
{
unsigned int cpu = cpumask_any_and(irq_data_get_affinity_mask(d),
cpu_online_mask);
@@ -106,7 +106,7 @@ static void plic_irq_enable(struct irq_data *d)
plic_irq_toggle(cpumask_of(cpu), d->hwirq, 1);
}
-static void plic_irq_disable(struct irq_data *d)
+static void plic_irq_unmask(struct irq_data *d)
{
plic_irq_toggle(cpu_possible_mask, d->hwirq, 0);
}
@@ -125,10 +125,8 @@ static int plic_set_affinity(struct irq_data *d,
if (cpu >= nr_cpu_ids)
return -EINVAL;
- if (!irqd_irq_disabled(d)) {
- plic_irq_toggle(cpu_possible_mask, d->hwirq, 0);
- plic_irq_toggle(cpumask_of(cpu), d->hwirq, 1);
- }
+ plic_irq_toggle(cpu_possible_mask, d->hwirq, 0);
+ plic_irq_toggle(cpumask_of(cpu), d->hwirq, 1);
irq_data_update_effective_affinity(d, cpumask_of(cpu));
@@ -136,14 +134,18 @@ static int plic_set_affinity(struct irq_data *d,
}
#endif
+static void
Re: [PATCH v2 3/4] task: With a grace period after finish_task_switch, remove unnecessary code
On Sat, Sep 14, 2019 at 07:34:30AM -0500, Eric W. Biederman wrote:
>
> Remove work arounds that were written before there was a grace period
> after tasks left the runqueue in finish_task_switch.
>
> In particular now that there tasks exiting the runqueue exprience
> a rcu grace period none of the work performed by task_rcu_dereference
> excpet the rcu_dereference is necessary so replace task_rcu_dereference
> with rcu_dereference.
>
> Remove the code in rcuwait_wait_event that checks to ensure the current
> task has not exited. It is no longer necessary as it is guaranteed
> that any running task will experience a rcu grace period after it
> leaves the run queueue.
>
> Remove the comment in rcuwait_wake_up as it is no longer relevant.
>
> Cc: Davidlohr Bueso
> Cc: Peter Zijlstra (Intel)
> Cc: Oleg Nesterov
> Ref: 8f95c90ceb54 ("sched/wait, RCU: Introduce rcuwait machinery")
> Ref: 150593bf8693 ("sched/api: Introduce task_rcu_dereference() and
> try_get_task_struct()")
> Signed-off-by: "Eric W. Biederman"
First, what am I looking for?
I am looking for something that prevents the following:
o Task A acquires a reference to Task B's task_struct while
protected only by RCU, and is just about to increment ->rcu_users
when it is delayed. Maybe its vCPU is preempted or something.
o Task B begins exiting.
o Task B removes itself from all the lists. (Which doesn't
matter to Task A, which already has an RCU-protected reference
to Task B's task_struct structure.
o Task B does a bunch of stuff protected by various locks and atomic
operations, but does not wait for an RCU grace period to elapse.
(Or am I wrong about that?)
o Task B does its final context switch, invoking finish_task_switch(),
in turn invoking put_task_struct_rcu_user(), which does the
final decrement of ->rcu_user to zero. And then immediately
overwrites that value by invoking call_rcu().
o Task A resumes, sees a (bogus) non-zero ->rcu_user and proceeds
to mangle the freelist. Or worse yet, something just now
allocated as some other data structure.
(If this really can happen, one easy fix is of course to remove the
union so that ->rcu and ->rcu_users don't sit on top of each other.)
With that, on to the patch!
Which does not do anything that I can see to prevent the above sequence
of events.
On to the next patch!
Thanx, Paul
> ---
> include/linux/rcuwait.h| 20 +++-
> include/linux/sched/task.h | 1 -
> kernel/exit.c | 67 --
> kernel/sched/fair.c| 2 +-
> kernel/sched/membarrier.c | 4 +--
> 5 files changed, 7 insertions(+), 87 deletions(-)
>
> diff --git a/include/linux/rcuwait.h b/include/linux/rcuwait.h
> index 563290fc194f..75c97e4bbc57 100644
> --- a/include/linux/rcuwait.h
> +++ b/include/linux/rcuwait.h
> @@ -6,16 +6,11 @@
>
> /*
> * rcuwait provides a way of blocking and waking up a single
> - * task in an rcu-safe manner; where it is forbidden to use
> - * after exit_notify(). task_struct is not properly rcu protected,
> - * unless dealing with rcu-aware lists, ie: find_task_by_*().
> + * task in an rcu-safe manner.
> *
> - * Alternatively we have task_rcu_dereference(), but the return
> - * semantics have different implications which would break the
> - * wakeup side. The only time @task is non-nil is when a user is
> - * blocked (or checking if it needs to) on a condition, and reset
> - * as soon as we know that the condition has succeeded and are
> - * awoken.
> + * The only time @task is non-nil is when a user is blocked (or
> + * checking if it needs to) on a condition, and reset as soon as we
> + * know that the condition has succeeded and are awoken.
> */
> struct rcuwait {
> struct task_struct __rcu *task;
> @@ -37,13 +32,6 @@ extern void rcuwait_wake_up(struct rcuwait *w);
> */
> #define rcuwait_wait_event(w, condition) \
> ({ \
> - /* \
> - * Complain if we are called after do_exit()/exit_notify(), \
> - * as we cannot rely on the rcu critical region for the \
> - * wakeup side. \
> - */ \
> - WARN_ON(current->exit_state); \
> - \
> rcu_assign_pointer((w)->task, current); \
> for (;;) { \
> /* \
> diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h
> index 4c44c37236
Re: [PATCH] sched: fix migration to invalid cpu in __set_cpus_allowed_ptr
On 15/09/2019 09:21, shikemeng wrote: >> It's more thoughtful to add check in cpumask_test_cpu.It can solve this >> problem and can prevent other potential bugs.I will test it and resend >> a new patch. >> > > Think again and again. As cpumask_check will fire a warning if cpu >= > nr_cpu_ids, it seems that cpumask_check only expects cpu < nr_cpu_ids and it's > caller's responsibility to very cpu is in valid range. Interfaces like > cpumask_test_and_set_cpu, cpumask_test_and_clear_cpu and so on are not > checking > cpu < nr_cpu_ids either and may cause demage if cpu is out of range. > cpumask operations clearly should never be fed CPU numbers > nr_cpu_ids, but we can get some sneaky mishaps like the one you're fixing. The answer might just be to have more folks turn on DEBUG_PER_CPU_MAPS in their test runs (I don't for instance - will do from now on), since I get the feeling people like to be able to disable these checks for producty kernels. In any case, don't feel like you have to fix this globally - your fix is fine on its own.
Re: [PATCH v2 4/4] task: RCUify the assignment of rq->curr
On Sat, Sep 14, 2019 at 07:35:02AM -0500, Eric W. Biederman wrote:
>
> The current task on the runqueue is currently read with rcu_dereference().
>
> To obtain ordinary rcu semantics for an rcu_dereference of rq->curr it needs
> to be paird with rcu_assign_pointer of rq->curr. Which provides the
> memory barrier necessary to order assignments to the task_struct
> and the assignment to rq->curr.
>
> Unfortunately the assignment of rq->curr in __schedule is a hot path,
> and it has already been show that additional barriers in that code
> will reduce the performance of the scheduler. So I will attempt to
> describe below why you can effectively have ordinary rcu semantics
> without any additional barriers.
>
> The assignment of rq->curr in init_idle is a slow path called once
> per cpu and that can use rcu_assign_pointer() without any concerns.
>
> As I write this there are effectively two users of rcu_dereference on
> rq->curr. There is the membarrier code in kernel/sched/membarrier.c
> that only looks at "->mm" after the rcu_dereference. Then there is
> task_numa_compare() in kernel/sched/fair.c. My best reading of the
> code shows that task_numa_compare only access: "->flags",
> "->cpus_ptr", "->numa_group", "->numa_faults[]",
> "->total_numa_faults", and "->se.cfs_rq".
>
> The code in __schedule() essentially does:
> rq_lock(...);
> smp_mb__after_spinlock();
>
> next = pick_next_task(...);
> rq->curr = next;
>
> context_switch(prev, next);
>
> At the start of the function the rq_lock/smp_mb__after_spinlock
> pair provides a full memory barrier. Further there is a full memory barrier
> in context_switch().
>
> This means that any task that has already run and modified itself (the
> common case) has already seen two memory barriers before __schedule()
> runs and begins executing. A task that modifies itself then sees a
> third full memory barrier pair with the rq_lock();
>
> For a brand new task that is enqueued with wake_up_new_task() there
> are the memory barriers present from the taking and release the
> pi_lock and the rq_lock as the processes is enqueued as well as the
> full memory barrier at the start of __schedule() assuming __schedule()
> happens on the same cpu.
>
> This means that by the time we reach the assignment of rq->curr
> except for values on the task struct modified in pick_next_task
> the code has the same guarantees as if it used rcu_assign_pointer.
>
> Reading through all of the implementations of pick_next_task it
> appears pick_next_task is limited to modifying the task_struct fields
> "->se", "->rt", "->dl". These fields are the sched_entity structures
> of the varies schedulers.
s/varies/various/ for whatever that is worth.
> Further "->se.cfs_rq" is only changed in cgroup attach/move operations
> initialized by userspace.
>
> Unless I have missed something this means that in practice that the
> users of "rcu_dereerence(rq->curr)" get normal rcu semantics of
> rcu_dereference() for the fields the care about, despite the
> assignment of rq->curr in __schedule() ot using rcu_assign_pointer.
The reasoning makes sense. I have not double-checked all the code.
> Link:
> https://lore.kernel.org/r/20190903200603.gw2...@hirez.programming.kicks-ass.net
> Signed-off-by: "Eric W. Biederman"
> ---
> kernel/sched/core.c | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 69015b7c28da..668262806942 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3857,7 +3857,11 @@ static void __sched notrace __schedule(bool preempt)
>
> if (likely(prev != next)) {
> rq->nr_switches++;
> - rq->curr = next;
> + /*
> + * RCU users of rcu_dereference(rq->curr) may not see
> + * changes to task_struct made by pick_next_task().
> + */
> + RCU_INIT_POINTER(rq->curr, next);
> /*
>* The membarrier system call requires each architecture
>* to have a full memory barrier after updating
> @@ -5863,7 +5867,8 @@ void init_idle(struct task_struct *idle, int cpu)
> __set_task_cpu(idle, cpu);
> rcu_read_unlock();
>
> - rq->curr = rq->idle = idle;
> + rq->idle = idle;
> + rcu_assign_pointer(rq->curr, idle);
> idle->on_rq = TASK_ON_RQ_QUEUED;
> #ifdef CONFIG_SMP
> idle->on_cpu = 1;
> --
> 2.21.0.dirty
So this looks good in and of itself, but I still do not see what prevents
the unfortunate sequence of events called out in my previous email.
On the other hand, if ->rcu and ->rcu_users were not allocated on top
of each other by a union, I would be happy to provide a Reviewed-by.
And I am fundamentally distrusting of a refcount_dec_and_test() that
is immediately followed by code that clobbers the now-zero value.
Yes, this does have valid use cases, but it has a lot more invalid
use cases. Th
Re: [PATCH] ARM: dts: imx6dl: SolidRun: add phy node with 100Mb/s max-speed
> > OF: fdt: Machine model: SolidRun HummingBoard Solo/DualLite > > ... > > # ethtool eth0 > > Settings for eth0: > > Supported ports: [ TP MII ] > > Supported link modes: 10baseT/Half 10baseT/Full > > 100baseT/Half 100baseT/Full > > 1000baseT/Full > > Supported pause frame use: Symmetric > > Supports auto-negotiation: Yes > > Supported FEC modes: Not reported > > Advertised link modes: 10baseT/Half 10baseT/Full > > 100baseT/Half 100baseT/Full > > 1000baseT/Full > > Advertised pause frame use: Symmetric > > Advertised auto-negotiation: Yes > > Advertised FEC modes: Not reported > > Link partner advertised link modes: 10baseT/Half 10baseT/Full > > 100baseT/Half 100baseT/Full > > 1000baseT/Full > > Link partner advertised pause frame use: Symmetric > > Link partner advertised auto-negotiation: Yes > > Link partner advertised FEC modes: Not reported > > Speed: 1000Mb/s > > Duplex: Full > > Port: MII > > PHYAD: 0 > > Transceiver: internal > > Auto-negotiation: on > > Supports Wake-on: d > > Wake-on: d > > Link detected: yes > Note that the FEC does *not* support 1000baseT/Half. Hi Russell fec_main.c has code to mask it out. And it is not listed in the modes you have above. So as you say, this all looks to be working. I'm wondering if there is an older variant of the hardware with 100Mbps magnetics, and the boot loader is setting something in the PHY? It could be we are now stomping over that? We need to see output like yours, but on a device which is experiencing the problem. Andrew
D;
-- Hi , can we talk about this please?
Re: [RFC][PATCH] pipe: Convert ring to head/tail
Hi David, [+Peter]
I have a few drive-by comments on the ordering side of things. See below.
On Fri, Sep 13, 2019 at 02:00:39PM +0100, David Howells wrote:
> Convert pipes to use head and tail pointers for the buffer ring rather than
> pointer and length as the latter requires two atomic ops to update (or a
> combined op) whereas the former only requires one.
>
> (1) The head pointer is the point at which production occurs and points to
> the slot in which the next buffer will be placed. This is equivalent
> to pipe->curbuf + pipe->nrbufs.
>
> The head pointer belongs to the write-side.
>
> (2) The tail pointer is the point at which consumption occurs. It points
> to the next slot to be consumed. This is equivalent to pipe->curbuf.
>
> The tail pointer belongs to the read-side.
>
> (3) head and tail are allowed to run to UINT_MAX and wrap naturally. They
> are only masked off when the array is being accessed, e.g.:
>
> pipe->bufs[head & mask]
>
> This means that it is not necessary to have a dead slot in the ring as
> head == tail isn't ambiguous.
>
> (4) The ring is empty if head == tail.
>
> (5) The occupancy of the ring is head - tail.
>
> (6) The amount of space in the ring is (tail + pipe->buffers) - head.
>
> (7) The ring is full if head == (tail + pipe->buffers) or
> head - tail == pipe->buffers.
>
> Barriers are also inserted, wrapped in inline functions:
>
> (1) unsigned int tail = pipe_get_tail_for_write(pipe);
>
> Read the tail pointer from the write-side. This acts as a barrier to
> order the tail read before the data in the ring is overwritten. It
> also prevents the compiler from re-reading the pointer.
>
> (2) unsigned int head = pipe_get_head_for_read(pipe);
>
> Read the head pointer from the read-side. This acts as a barrier to
> order the head read before the data read. It also prevents the
> compiler from re-reading the pointer.
>
> (3) pipe_post_read_barrier(pipe, unsigned int tail);
>
> Update the tail pointer from the read-side. This acts as a barrier to
> order the pointer update after the data read. The consumed data slot
> must not be touched after this function.
>
> (4) pipe_post_write_barrier(pipe, unsigned int head);
>
> Update the head pointer from the write-side. This acts as a barrier
> to order the pointer update after the data write. The produced data
> slot should not normally be touched after this function[*].
>
> [*] Unless pipe->mutex is held.
> ---
> fs/fuse/dev.c | 23 ++-
> fs/pipe.c | 154 --
> fs/splice.c | 161 +--
> include/linux/pipe_fs_i.h | 76 -
> include/linux/uio.h |4
> lib/iov_iter.c| 268
> ++
> 6 files changed, 438 insertions(+), 248 deletions(-)
[...]
> diff --git a/fs/pipe.c b/fs/pipe.c
> index 8a2ab2f974bd..aa410ee0f423 100644
> --- a/fs/pipe.c
> +++ b/fs/pipe.c
> @@ -43,10 +43,10 @@ unsigned long pipe_user_pages_hard;
> unsigned long pipe_user_pages_soft = PIPE_DEF_BUFFERS * INR_OPEN_CUR;
>
> /*
> - * We use a start+len construction, which provides full use of the
> + * We use a start+len construction, which provides full use of the
> * allocated memory.
> * -- Florian Coosmann (FGC)
> - *
> + *
> * Reads with count = 0 should always return 0.
> * -- Julian Bradfield 1999-06-07.
> *
> @@ -285,10 +285,15 @@ pipe_read(struct kiocb *iocb, struct iov_iter *to)
> ret = 0;
> __pipe_lock(pipe);
> for (;;) {
> - int bufs = pipe->nrbufs;
> - if (bufs) {
> - int curbuf = pipe->curbuf;
> - struct pipe_buffer *buf = pipe->bufs + curbuf;
> + /* Barrier: head belongs to the write side, so order reading
> + * the data after reading the head pointer.
> + */
> + unsigned int head = READ_ONCE(pipe->head);
Hmm, I don't understand this. Since READ_ONCE() doesn't imply a barrier,
how are you enforcing the read-read ordering in the CPU?
> @@ -104,6 +104,76 @@ struct pipe_buf_operations {
> bool (*get)(struct pipe_inode_info *, struct pipe_buffer *);
> };
>
> +/**
> + * pipe_get_tail_for_write - Get pipe buffer tail pointer for write-side use
> + * @pipe: The pipe in question
> + *
> + * Get the tail pointer for use in the write-side code. This may need to
> + * insert a barrier against the reader to order reading the tail pointer
> + * against the reader reading the buffer.
What is the purpose of saying "This may need to insert a barrier"? Can this
function be overridden or something?
> + */
> +static inline unsigned int pipe_get_tail_for_write(struct pipe_inode_info
> *pipe)
> +{
> + return READ_ONCE(pipe->tail);
> +}
> +
> +/**
> + * pipe_post_read_barrier - Set pipe
Re: [PATCH] ARM: dts: imx6dl: SolidRun: add phy node with 100Mb/s max-speed
On Sun, Sep 15, 2019 at 04:42:52PM +0200, Andrew Lunn wrote: > > > OF: fdt: Machine model: SolidRun HummingBoard Solo/DualLite > > > ... > > > # ethtool eth0 > > > Settings for eth0: > > > Supported ports: [ TP MII ] > > > Supported link modes: 10baseT/Half 10baseT/Full > > > 100baseT/Half 100baseT/Full > > > 1000baseT/Full > > > Supported pause frame use: Symmetric > > > Supports auto-negotiation: Yes > > > Supported FEC modes: Not reported > > > Advertised link modes: 10baseT/Half 10baseT/Full > > > 100baseT/Half 100baseT/Full > > > 1000baseT/Full > > > Advertised pause frame use: Symmetric > > > Advertised auto-negotiation: Yes > > > Advertised FEC modes: Not reported > > > Link partner advertised link modes: 10baseT/Half 10baseT/Full > > > 100baseT/Half 100baseT/Full > > > 1000baseT/Full > > > Link partner advertised pause frame use: Symmetric > > > Link partner advertised auto-negotiation: Yes > > > Link partner advertised FEC modes: Not reported > > > Speed: 1000Mb/s > > > Duplex: Full > > > Port: MII > > > PHYAD: 0 > > > Transceiver: internal > > > Auto-negotiation: on > > > Supports Wake-on: d > > > Wake-on: d > > > Link detected: yes > > > Note that the FEC does *not* support 1000baseT/Half. > > Hi Russell > > fec_main.c has code to mask it out. And it is not listed in the modes > you have above. So as you say, this all looks to be working. > > I'm wondering if there is an older variant of the hardware with > 100Mbps magnetics, and the boot loader is setting something in the > PHY? It could be we are now stomping over that? Not according to Rabeeh, the SolidRun CTO: < rabeeh> all i.MX6 based machines from SolidRun are 1Gbps phys < rabeeh> i thought that we fixed that information, documentation wise; but seems not Even the Carrier1 board that pre-dates Hummingboards had the AR8035 with 1G magnetics. The schematics I have for the Cubox-i state that the RJ45 jack (which contains the magnetics) is to be "Gigabit". There was a 10/100M option for the microsom, which is selected by where a resistor pack is fitted, having the effect of configuring the AR8035 differently. I seem to recall the 10/100M option in the early days was to use a different Atheros PHY. However, I'm not aware of 10/100M option making it into production, and Rabeeh's comment (who was involved in the design) confirms that. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
Re: [PATCH v4 1/2] ethtool: implement Energy Detect Powerdown support via phy-tunable
On 9/12/2019 9:28 AM, Alexandru Ardelean wrote: > The `phy_tunable_id` has been named `ETHTOOL_PHY_EDPD` since it looks like > this feature is common across other PHYs (like EEE), and defining > `ETHTOOL_PHY_ENERGY_DETECT_POWER_DOWN` seems too long. > > The way EDPD works, is that the RX block is put to a lower power mode, > except for link-pulse detection circuits. The TX block is also put to low > power mode, but the PHY wakes-up periodically to send link pulses, to avoid > lock-ups in case the other side is also in EDPD mode. > > Currently, there are 2 PHY drivers that look like they could use this new > PHY tunable feature: the `adin` && `micrel` PHYs. > > The ADIN's datasheet mentions that TX pulses are at intervals of 1 second > default each, and they can be disabled. For the Micrel KSZ9031 PHY, the > datasheet does not mention whether they can be disabled, but mentions that > they can modified. > > The way this change is structured, is similar to the PHY tunable downshift > control: > * a `ETHTOOL_PHY_EDPD_DFLT_TX_MSECS` value is exposed to cover a default > TX interval; some PHYs could specify a certain value that makes sense > * `ETHTOOL_PHY_EDPD_NO_TX` would disable TX when EDPD is enabled > * `ETHTOOL_PHY_EDPD_DISABLE` will disable EDPD > > As noted by the `ETHTOOL_PHY_EDPD_DFLT_TX_MSECS` the interval unit is 1 > millisecond, which should cover a reasonable range of intervals: > - from 1 millisecond, which does not sound like much of a power-saver > - to ~65 seconds which is quite a lot to wait for a link to come up when >plugging a cable > > Signed-off-by: Alexandru Ardelean Reviewed-by: Florian Fainelli -- Florian
Re: [PATCH v4 2/2] net: phy: adin: implement Energy Detect Powerdown mode via phy-tunable
On 9/14/2019 8:29 AM, Andrew Lunn wrote:
> On Thu, Sep 12, 2019 at 07:28:12PM +0300, Alexandru Ardelean wrote:
>
>> +static int adin_set_edpd(struct phy_device *phydev, u16 tx_interval)
>> +{
>> +u16 val;
>> +
>> +if (tx_interval == ETHTOOL_PHY_EDPD_DISABLE)
>> +return phy_clear_bits(phydev, ADIN1300_PHY_CTRL_STATUS2,
>> +(ADIN1300_NRG_PD_EN | ADIN1300_NRG_PD_TX_EN));
>> +
>> +val = ADIN1300_NRG_PD_EN;
>> +
>> +switch (tx_interval) {
>> +case 1000: /* 1 second */
>> +/* fallthrough */
>> +case ETHTOOL_PHY_EDPD_DFLT_TX_MSECS:
>> +val |= ADIN1300_NRG_PD_TX_EN;
>> +/* fallthrough */
>> +case ETHTOOL_PHY_EDPD_NO_TX:
>> +break;
>> +default:
>> +return -EINVAL;
>> +}
>> +
>> +return phy_modify(phydev, ADIN1300_PHY_CTRL_STATUS2,
>> + (ADIN1300_NRG_PD_EN | ADIN1300_NRG_PD_TX_EN),
>> + val);
>> +}
>> +
>
>>
>> +rc = adin_set_edpd(phydev, 1);
>> +if (rc < 0)
>> +return rc;
>
> Hi Alexandru
>
> Shouldn't this be adin_set_edpd(phydev, 1000);
That does sound like the intended use, or use
ETHTOOL_PHY_EDPD_DFLT_TX_MSECS, with that fixed:
Reviewed-by: Florian Fainelli
--
Florian
Re: [PATCH AUTOSEL 4.19 126/167] tpm: Fix TPM 1.2 Shutdown sequence to prevent future TPM operations
On Mon, Sep 09, 2019 at 05:28:08PM +0100, Jarkko Sakkinen wrote:
> On Sat, Sep 07, 2019 at 06:04:48PM -0400, Sasha Levin wrote:
> > On Sat, Sep 07, 2019 at 09:55:18PM +0300, Jarkko Sakkinen wrote:
> > > On Tue, 2019-09-03 at 15:43 -0400, Sasha Levin wrote:
> > > > Right. I gave a go at backporting a few patches and this happens to be
> > > > one of them. It will be a while before it goes in a stable tree
> > > > (probably way after after LPC).
> > >
> > > It *semantically* depends on
> > >
> > > db4d8cb9c9f2 ("tpm: use tpm_try_get_ops() in tpm-sysfs.c.")
> > >
> > > I.e. can cause crashes without the above patch. As a code change your
> > > patch is fine but it needs the above patch backported to work in stable
> > > manner.
> > >
> > > So... either I can backport that one (because ultimately I have
> > > responsibility to do that as the maintainer) but if you want to finish
> > > this one that is what you need to backport in addition and then it
> > > should be fine.
> >
> > If you're ok with the backport of this commit, I can just add
> > db4d8cb9c9f2 on top.
>
> Sure, I've already gave my promise to do that :-)
I ended up with:
db4d8cb9c9f2 tpm: Fix TPM 1.2 Shutdown sequence to prevent future TPM operations
2677ca98ae37 tpm: use tpm_try_get_ops() in tpm-sysfs.c.
da379f3c1db0 tpm: migrate pubek_show to struct tpm_buf
Since some time has passed I'l just restate that the reason for
backporting 2677ca98ae37 was that tpm_class_shutdown() could pull carpet
under the TPM 1.2 code. tpm_try_get_ops() makes sure that read lock is
taken and chip->ops is not NULL if it successfully returns.
Still need to test the patches with TPM 1.2 hardware before I can send
them.
/Jarkko
Re: [PATCH] staging: bcm2835-audio: Fix draining behavior regression
On Sun, 15 Sep 2019 15:54:16 +0200,
Stefan Wahren wrote:
>
> Hi Takashi,
>
> Am 14.09.19 um 17:24 schrieb Takashi Iwai:
> > The PCM draining behavior got broken since the recent refactoring, and
> > this turned out to be the incorrect expectation of the firmware
> > behavior regarding "draining". While I expected the "drain" flag at
> > the stop operation would do processing the queued samples, it seems
> > rather dropping the samples.
> >
> > As a quick fix, just drop the SNDRV_PCM_INFO_DRAIN_TRIGGER flag, so
> > that the driver uses the normal PCM draining procedure. Also, put
> > some caution comment to the function for future readers not to fall
> > into the same pitfall.
> >
> > Fixes: d7ca3a71545b ("staging: bcm2835-audio: Operate non-atomic PCM ops")
> > BugLink: https://github.com/raspberrypi/linux/issues/2983
>
> thanks for taking care of this. Wouldn't it be better to add the link to
> the new comment to provide more context of the unexpected behavior?
Yeah, a bit more explanation would be better. Unfortunately I've been
traveling in the last week (and will be traveling again in the next
weeks), so had little time for caring and testing with any actual
hardware...
>
> Nevertheless:
>
> Acked-by: Stefan Wahren
Thanks!
Takashi
> > Cc: sta...@vger.kernel.org
> > Signed-off-by: Takashi Iwai
> > ---
> > drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c | 4 ++--
> > drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c | 1 +
> > 2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
> > b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
> > index bc1eaa3a0773..826016c3431a 100644
> > --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
> > +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c
> > @@ -12,7 +12,7 @@
> > static const struct snd_pcm_hardware snd_bcm2835_playback_hw = {
> > .info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER |
> > SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
> > -SNDRV_PCM_INFO_DRAIN_TRIGGER | SNDRV_PCM_INFO_SYNC_APPLPTR),
> > +SNDRV_PCM_INFO_SYNC_APPLPTR),
> > .formats = SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S16_LE,
> > .rates = SNDRV_PCM_RATE_CONTINUOUS | SNDRV_PCM_RATE_8000_48000,
> > .rate_min = 8000,
> > @@ -29,7 +29,7 @@ static const struct snd_pcm_hardware
> > snd_bcm2835_playback_hw = {
> > static const struct snd_pcm_hardware snd_bcm2835_playback_spdif_hw = {
> > .info = (SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER |
> > SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
> > -SNDRV_PCM_INFO_DRAIN_TRIGGER | SNDRV_PCM_INFO_SYNC_APPLPTR),
> > +SNDRV_PCM_INFO_SYNC_APPLPTR),
> > .formats = SNDRV_PCM_FMTBIT_S16_LE,
> > .rates = SNDRV_PCM_RATE_CONTINUOUS | SNDRV_PCM_RATE_44100 |
> > SNDRV_PCM_RATE_48000,
> > diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> > b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> > index 23fba01107b9..c6f9cf1913d2 100644
> > --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> > +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c
> > @@ -289,6 +289,7 @@ int bcm2835_audio_stop(struct bcm2835_alsa_stream
> > *alsa_stream)
> > VC_AUDIO_MSG_TYPE_STOP, false);
> > }
> >
> > +/* FIXME: this doesn't seem working as expected for "draining" */
> > int bcm2835_audio_drain(struct bcm2835_alsa_stream *alsa_stream)
> > {
> > struct vc_audio_msg m = {
>
Re: [PATCH 4.9 00/14] 4.9.193-stable review
On 9/15/19 5:58 AM, Greg Kroah-Hartman wrote:
On Sat, Sep 14, 2019 at 05:49:32PM -0700, Guenter Roeck wrote:
Hi Greg,
On 9/14/19 1:31 AM, Greg Kroah-Hartman wrote:
On Sat, Sep 14, 2019 at 01:28:39AM -0700, Guenter Roeck wrote:
On 9/13/19 6:06 AM, Greg Kroah-Hartman wrote:
This is the start of the stable review cycle for the 4.9.193 release.
There are 14 patches in this series, all will be posted as a response
to this one. If anyone has any issues with these being applied, please
let me know.
Responses should be made by Sun 15 Sep 2019 01:03:32 PM UTC.
Anything received after that time might be too late.
Is it really only me seeing this ?
drivers/vhost/vhost.c: In function 'translate_desc':
include/linux/compiler.h:549:38: error: call to '__compiletime_assert_1879'
declared with attribute error: BUILD_BUG_ON failed: sizeof(_s) > sizeof(long)
i386:allyesconfig, mips:allmodconfig and others, everywhere including
mainline. Culprit is commit a89db445fbd7f1 ("vhost: block speculation
of translated descriptors").
Nope, I just got another report of this on 5.2.y. Problem is also in
Linus's tree :(
Please apply upstream commit 0d4a3f2abbef ("Revert "vhost: block speculation
of translated descriptors") to v4.9.y-queue and later to fix the build problems.
Or maybe just drop the offending commit from the stable release queues.
I'm just going to drop the offending commit from everywhere and push out
new -rcs in a bit...
A quick rebuild of previously failed builds now passes, so looks like we are
good.
Guenter
Re: [PATCH] staging: exfat: add exfat filesystem code to
Hi Greg, On Sun, Sep 15, 2019 at 10:54 PM Greg KH wrote: > Note, this just showed up publically on August 12, where were you with > all of this new code before then? :) My sdFAT port, exfat-nofuse and the one on the staging tree, were all made by Samsung. And unless you guys had a chance to talk to Samsung developers directly, those all share the same faith of lacking proper development history. The source I used was from http://opensource.samsung.com, which provides kernel sources as tar.gz files. There is no code history available. > For the in-kernel code, we would have to rip out all of the work you did > for all older kernels, so that's a non-starter right there. I'm aware. I'm just letting mainline know that there is potentially another (much better) base that could be upstreamed. If you want me to rip out older kernel support for upstreaming, I'm more than happy to do so. > As for what codebase to work off of, I don't want to say it is too late, > but really, this shows up from nowhere and we had to pick something so > we found the best we could at that point in time. To be honest, whole public exFAT sources are all from nowhere unless you had internal access to Samsung's development archive. The one in the current staging tree isn't any better. I'm not even sure where the staging driver is from, actually. Samsung used the 1.2.x versioning until they switched to a new code base - sdFAT. The one in the staging tree is marked version 1.3.0(exfat_super.c). I failed to find anything 1.3.x from Samsung's public kernel sources. The last time exFAT 1.2.x was used was in Galaxy S7(released in 2016). Mine was originally based on sdFAT 2.1.10, used in Galaxy S10(released in March 2019) and it just got updated to 2.2.0, used in Galaxy Note10(released in August 2019). > Is there anything specific in the codebase you have now, that is lacking > in the in-kernel code? Old-kernel-support doesn't count here, as we > don't care about that as it is not applicable. But functionality does > matter, what has been added here that we can make use of? This is more of a suggestion of "Let's base on a *much more recent* snapshot for the community to work on", since the current one on the staging tree also lacks development history. The diff is way too big to even start understanding the difference. With that said though, I do have some vague but real reason as to why sdFAT base is better. With some major Android vendors showing interests in supporting exFAT, Motorola notably published their work on public Git repository with full development history(the only vendor to do this that I'm aware of). Commits like this: https://github.com/MotorolaMobilityLLC/kernel-msm/commit/7ab1657 is not merged to exFAT(including the current staging tree one) while it did for sdFAT. The only thing I regret is not working on porting sdFAT sooner. I definitely didn't anticipate Microsoft to suddenly lift legal issues on upstreaming exFAT just around when I happen to gain interest in porting sdFAT. If my port happened sooner, it would have been a no-brainer for it to be considered as a top candidate for upstreaming. > And do you have any "real" development history to look at instead of the > "one giant commit" of the initial code drop? That is where we could > actually learn what has changed over time. Your repo as-is shows none > of the interesting bits :( As I mentioned, development history is unobtainable, even for the current staging tree or exfat-nofuse. (If you guys took exfat-nofuse, you can also see that there's barely any real exFAT-related development done in that tree. Everything is basically fixes for newer kernel versions.) The best I could do, if someone's interested, is to diff all versions of exFAT/sdFAT throughout the Samsung's kernel versions, but that still won't give us reasons as to why the changes were made. TL;DR My suggestion - Let's base on a much newer driver that's matured more, contains more fixes, gives (slightly?) better performance and hopefully has better code quality. Both drivers are horrible. You said it yourself(for the current staging one), and even for my new sdFAT-base proposal, I'm definitely not comfortable seeing this kind of crap in mainline: https://github.com/arter97/exfat-linux/commit/0f1ddde However, it's clear to me that the sdFAT base is less-horrible. Please let me know what you think. > thanks, > > greg kh Thanks.
Re: [PATCH v5 6/9] leds: multicolor: Introduce a multicolor class definition
Dan,
On 9/11/19 8:01 PM, Dan Murphy wrote:
> Introduce a multicolor class that groups colored LEDs
> within a LED node.
>
> The framework allows for dynamically setting individual LEDs
> or setting brightness levels of LEDs and updating them virtually
> simultaneously.
>
> Signed-off-by: Dan Murphy
> ---
> drivers/leds/Kconfig | 10 +
> drivers/leds/Makefile| 1 +
> drivers/leds/led-class-multicolor.c | 387 +++
> include/linux/led-class-multicolor.h | 96 +++
> 4 files changed, 494 insertions(+)
> create mode 100644 drivers/leds/led-class-multicolor.c
> create mode 100644 include/linux/led-class-multicolor.h
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 1988de1d64c0..71e7fd4f6f15 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -30,6 +30,16 @@ config LEDS_CLASS_FLASH
> for the flash related features of a LED device. It can be built
> as a module.
>
> +config LEDS_CLASS_MULTI_COLOR
> + tristate "LED Mulit Color LED Class Support"
> + depends on LEDS_CLASS
> + help
> + This option enables the multicolor LED sysfs class in /sys/class/leds.
> + It wraps LED class and adds multicolor LED specific sysfs attributes
> + and kernel internal API to it. You'll need this to provide support
> + for multicolor LEDs that are grouped together. This class is not
> + intended for single color LEDs. It can be built as a module.
> +
> config LEDS_BRIGHTNESS_HW_CHANGED
> bool "LED Class brightness_hw_changed attribute support"
> depends on LEDS_CLASS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 41fb073a39c1..897b810257dd 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -4,6 +4,7 @@
> obj-$(CONFIG_NEW_LEDS) += led-core.o
> obj-$(CONFIG_LEDS_CLASS) += led-class.o
> obj-$(CONFIG_LEDS_CLASS_FLASH) += led-class-flash.o
> +obj-$(CONFIG_LEDS_CLASS_MULTI_COLOR) += led-class-multicolor.o
> obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o
>
> # LED Platform Drivers
> diff --git a/drivers/leds/led-class-multicolor.c
> b/drivers/leds/led-class-multicolor.c
> new file mode 100644
> index ..c733192b31fa
> --- /dev/null
> +++ b/drivers/leds/led-class-multicolor.c
> @@ -0,0 +1,387 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// LED Multi Color class interface
> +// Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "leds.h"
> +
> +struct led_classdev_mc_priv {
I'd go for some more informative struct name here.
And also this is not going to be main classdev struct
so we can drop that from the name.
struct led_mc_color_entry ?
> + struct led_classdev_mc *mcled_cdev;
> +
> + struct device_attribute max_intensity_attr;
> + struct device_attribute intensity_attr;
> + struct device_attribute color_index_attr;
> +
> + enum led_brightness max_intensity;
> + enum led_brightness intensity;
> +
> + struct list_head list;
> +
> + int led_color_id;
> + int color_seq_pos;
> +};
> +
> +static int led_mc_calculate_brightness(int brightness, int intensity,
> +int max_intensity)
> +{
> + if (brightness && intensity && max_intensity)
This check looks odd. I'd drop it entirely. One thing we could worry
about would be max_intensity == 0 but this should be secured by the
core on registration. Effectively I don't see the need for wrapping
below calculation in a function at all.
> + return brightness * intensity / max_intensity;
> +
> + return LED_OFF;
> +}
> +
> +void led_mc_set_cluster_brightness(struct led_classdev_mc *mcled_cdev,
We really don't set cluster brightness but calculate it only here
(we can skip "cluster" as well):
s/led_mc_set_cluster_brightness/led_mc_calc_brightness/
> + enum led_brightness brightness, int adj_value[])
s/adj_value/brightness_val/
> +{
> + struct led_classdev_mc_data *data = mcled_cdev->data;
> + struct led_classdev_mc_priv *priv;
> + int i = 0;
> +
> + list_for_each_entry(priv, &data->color_list, list) {
> + adj_value[i] = led_mc_calculate_brightness(brightness,
> +priv->intensity,
> +priv->max_intensity);
brightness_val[i] = brightness * priv->intensity / priv->max_intensity;
> + i++;
> + }
> +
> + data->cluster_brightness = brightness;
Why can't we use led_classdev_mc->led_cdev->brightness for storing
cluster brightness?
> +}
> +
> +void led_mc_get_cluster_brightness(struct led_classdev_mc *mcled_cdev,
> + int brightness_val[])
> +{
> + struct led_classdev_mc_data *data = mcled_cdev->data
Re: [PATCH v5 9/9] leds: Update the lp55xx to use the multi color framework
Hi Dan,
On 9/11/19 8:01 PM, Dan Murphy wrote:
> Update the lp5523 to use the multi color framework.
>
> Signed-off-by: Dan Murphy
> ---
> drivers/leds/leds-lp5523.c| 13 +++
> drivers/leds/leds-lp55xx-common.c | 131 ++
> drivers/leds/leds-lp55xx-common.h | 9 ++
> include/linux/platform_data/leds-lp55xx.h | 6 +
> modules.builtin.modinfo | Bin 0 -> 43550 bytes
> 5 files changed, 137 insertions(+), 22 deletions(-)
> create mode 100644 modules.builtin.modinfo
>
> diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c
> index d0b931a136b9..8b2cdb98fed6 100644
> --- a/drivers/leds/leds-lp5523.c
> +++ b/drivers/leds/leds-lp5523.c
> @@ -791,6 +791,18 @@ static ssize_t store_master_fader_leds(struct device
> *dev,
> return ret;
> }
[...]
> struct lp55xx_predef_pattern {
Below file seems to have been added to the index by mistake.
> diff --git a/modules.builtin.modinfo b/modules.builtin.modinfo
> new file mode 100644
> index
> ..e528d8f57796621b6cfef52ad0da44551a482481
> GIT binary patch
> literal 43550
> zcmcJ2?Q+|=(x(2ta}}twKjNIV zoV|v|YGNXqfd?pCE75g*+RpaC?x8;w!ycl>2BO|zrh@WId3MUofUG7gi1
> zeg~})3e2M*O!DyPegBfbM`0dh(V~db zWw;DG3qOmC$3=KF3Wv*yH%_x4^s>wg7R&B1P3KKT7wLVNdEs~*1raD&TW%EP{%{_8
[...]
--
Best regards,
Jacek Anaszewski
Re: Linux 5.3-rc8
On Sat, Sep 14, 2019 at 11:51 PM Lennart Poettering wrote: > > Oh man. Just spend 5min to understand the situation, before claiming > this was garbage or that was garbage. The code above does not block > boot. Yes it does. You clearly didn't read the thread. > It blocks startup of services that explicit order themselves > after the code above. There's only a few services that should do that, > and the main system boots up just fine without waiting for this. That's a nice theory, but it doesn't actually match reality. There are clearly broken setups that use this for things that it really shouldn't be used for. Asking for true randomness at boot before there is any indication that randomness exists, and then just blocking with no further action that could actually _generate_ said randomness. If your description was true that the system would come up and be usable while the blocked thread is waiting for that to happen, things would be fine. But that simply isn't the case. Linus
pci: endpoint test BUG
Kernel is 5.3-rc8 on x86_64. Loading and removing the pci-epf-test module causes a BUG. [40928.435755] calling pci_epf_test_init+0x0/0x1000 [pci_epf_test] @ 12132 [40928.436717] initcall pci_epf_test_init+0x0/0x1000 [pci_epf_test] returned 0 after 891 usecs [40936.996081] == [40936.996125] BUG: KASAN: use-after-free in pci_epf_remove_cfs+0x1ae/0x1f0 [40936.996153] Write of size 8 at addr 88810a22a068 by task rmmod/12139 [40936.996193] CPU: 2 PID: 12139 Comm: rmmod Not tainted 5.3.0-rc8 #3 [40936.996217] Hardware name: TOSHIBA PORTEGE R835/Portable PC, BIOS Version 4.10 01/08/2013 [40936.996247] Call Trace: [40936.996265] dump_stack+0x7b/0xb5 [40936.996288] print_address_description+0x6e/0x470 [40936.996316] __kasan_report+0x11a/0x198 [40936.996337] ? pci_epf_remove_cfs+0x1ae/0x1f0 [40936.996362] ? pci_epf_remove_cfs+0x1ae/0x1f0 [40936.996384] kasan_report+0x12/0x20 [40936.996404] __asan_report_store8_noabort+0x17/0x20 [40936.996427] pci_epf_remove_cfs+0x1ae/0x1f0 [40936.996452] pci_epf_unregister_driver+0xd/0x20 [40936.996476] pci_epf_test_exit+0x10/0x19 [pci_epf_test] [40936.996500] __x64_sys_delete_module+0x329/0x490 [40936.996523] ? __ia32_sys_delete_module+0x490/0x490 [40936.996549] ? _raw_spin_unlock_irq+0x22/0x40 [40936.996582] do_syscall_64+0xaa/0x380 [40936.996601] ? prepare_exit_to_usermode+0xad/0x1b0 [40936.996625] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [40936.996648] RIP: 0033:0x7fb84c88d187 [40936.996667] Code: 73 01 c3 48 8b 0d 11 ad 2b 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 b8 b0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d e1 ac 2b 00 f7 d8 64 89 01 48 [40936.996724] RSP: 002b:7ffc1c5c7b38 EFLAGS: 0206 ORIG_RAX: 00b0 [40936.996753] RAX: ffda RBX: 7ffc1c5c7b98 RCX: 7fb84c88d187 [40936.996780] RDX: 000a RSI: 0800 RDI: 556838f1c7d8 [40936.996806] RBP: 556838f1c770 R08: 7ffc1c5c6ab1 R09: [40936.996833] R10: 7fb84c8fc5e0 R11: 0206 R12: 7ffc1c5c7d60 [40936.996859] R13: 7ffc1c5c975c R14: 556838f1c260 R15: 556838f1c770 [40936.996910] Allocated by task 12132: [40936.996929] save_stack+0x21/0x90 [40936.996947] __kasan_kmalloc.constprop.8+0xa7/0xd0 [40936.996968] kasan_kmalloc+0x9/0x10 [40936.996988] configfs_register_default_group+0x63/0xe0 [40936.997010] pci_ep_cfs_add_epf_group+0x20/0x50 [40936.997031] __pci_epf_register_driver+0x2b2/0x410 [40936.997052] 0xc1c9004a [40936.997070] do_one_initcall+0xab/0x2d5 [40936.997089] do_init_module+0x1c7/0x582 [40936.997107] load_module+0x4efa/0x5f30 [40936.997126] __do_sys_finit_module+0x12a/0x1b0 [40936.997146] __x64_sys_finit_module+0x6e/0xb0 [40936.997166] do_syscall_64+0xaa/0x380 [40936.997185] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [40936.997218] Freed by task 12139: [40936.997235] save_stack+0x21/0x90 [40936.997253] __kasan_slab_free+0x137/0x190 [40936.997281] kasan_slab_free+0xe/0x10 [40936.997301] kfree+0xb8/0x210 [40936.997320] configfs_unregister_default_group+0x15/0x20 [40936.997344] pci_ep_cfs_remove_epf_group+0x17/0x20 [40936.997367] pci_epf_remove_cfs+0x8e/0x1f0 [40936.997389] pci_epf_unregister_driver+0xd/0x20 [40936.997419] pci_epf_test_exit+0x10/0x19 [pci_epf_test] [40936.997441] __x64_sys_delete_module+0x329/0x490 [40936.997462] do_syscall_64+0xaa/0x380 [40936.997480] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [40936.997513] The buggy address belongs to the object at 88810a229fe8 which belongs to the cache kmalloc-192 of size 192 [40936.997557] The buggy address is located 128 bytes inside of 192-byte region [88810a229fe8, 88810a22a0a8) [40936.997597] The buggy address belongs to the page: [40936.997619] page:ea0004288a00 refcount:1 mapcount:0 mapping:888107c10f40 index:0x0 compound_mapcount: 0 [40936.997655] flags: 0x17ffc10200(slab|head) [40936.997677] raw: 0017ffc10200 ea0004992e08 888107c036b0 888107c10f40 [40936.997706] raw: 001e001e 0001 [40936.997734] page dumped because: kasan: bad access detected [40936.997767] Memory state around the buggy address: [40936.997789] 88810a229f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc [40936.997816] 88810a229f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fb fb fb [40936.997843] >88810a22a000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [40936.997869] ^ [40936.997895] 88810a22a080: fb fb fb fb fb fc fc fc fc fc fc fc fc fc fc fc [40936.997922] 88810a22a100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc [40936.997948] == -- ~Randy
