[PATCH 01/10] ASoC: au1x: constify snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Signed-off-by: Arvind Yadav--- sound/soc/au1x/dbdma2.c | 2 +- sound/soc/au1x/dma.c| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/soc/au1x/dbdma2.c b/sound/soc/au1x/dbdma2.c index b5d1caa..6a035ca 100644 --- a/sound/soc/au1x/dbdma2.c +++ b/sound/soc/au1x/dbdma2.c @@ -304,7 +304,7 @@ static int au1xpsc_pcm_close(struct snd_pcm_substream *substream) return 0; } -static struct snd_pcm_ops au1xpsc_pcm_ops = { +static const struct snd_pcm_ops au1xpsc_pcm_ops = { .open = au1xpsc_pcm_open, .close = au1xpsc_pcm_close, .ioctl = snd_pcm_lib_ioctl, diff --git a/sound/soc/au1x/dma.c b/sound/soc/au1x/dma.c index fcf5a9a..19457e2 100644 --- a/sound/soc/au1x/dma.c +++ b/sound/soc/au1x/dma.c @@ -277,7 +277,7 @@ static snd_pcm_uframes_t alchemy_pcm_pointer(struct snd_pcm_substream *ss) return bytes_to_frames(ss->runtime, location); } -static struct snd_pcm_ops alchemy_pcm_ops = { +static const struct snd_pcm_ops alchemy_pcm_ops = { .open = alchemy_pcm_open, .close = alchemy_pcm_close, .ioctl = snd_pcm_lib_ioctl, -- 1.9.1
[PATCH 01/10] ASoC: au1x: constify snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Signed-off-by: Arvind Yadav --- sound/soc/au1x/dbdma2.c | 2 +- sound/soc/au1x/dma.c| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/soc/au1x/dbdma2.c b/sound/soc/au1x/dbdma2.c index b5d1caa..6a035ca 100644 --- a/sound/soc/au1x/dbdma2.c +++ b/sound/soc/au1x/dbdma2.c @@ -304,7 +304,7 @@ static int au1xpsc_pcm_close(struct snd_pcm_substream *substream) return 0; } -static struct snd_pcm_ops au1xpsc_pcm_ops = { +static const struct snd_pcm_ops au1xpsc_pcm_ops = { .open = au1xpsc_pcm_open, .close = au1xpsc_pcm_close, .ioctl = snd_pcm_lib_ioctl, diff --git a/sound/soc/au1x/dma.c b/sound/soc/au1x/dma.c index fcf5a9a..19457e2 100644 --- a/sound/soc/au1x/dma.c +++ b/sound/soc/au1x/dma.c @@ -277,7 +277,7 @@ static snd_pcm_uframes_t alchemy_pcm_pointer(struct snd_pcm_substream *ss) return bytes_to_frames(ss->runtime, location); } -static struct snd_pcm_ops alchemy_pcm_ops = { +static const struct snd_pcm_ops alchemy_pcm_ops = { .open = alchemy_pcm_open, .close = alchemy_pcm_close, .ioctl = snd_pcm_lib_ioctl, -- 1.9.1
[PATCH 04/10] ASoC: intel: constify snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Signed-off-by: Arvind Yadav--- sound/soc/intel/baytrail/sst-baytrail-pcm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/intel/baytrail/sst-baytrail-pcm.c b/sound/soc/intel/baytrail/sst-baytrail-pcm.c index 4765ad4..0bdc057 100644 --- a/sound/soc/intel/baytrail/sst-baytrail-pcm.c +++ b/sound/soc/intel/baytrail/sst-baytrail-pcm.c @@ -309,7 +309,7 @@ static int sst_byt_pcm_mmap(struct snd_pcm_substream *substream, return snd_pcm_lib_default_mmap(substream, vma); } -static struct snd_pcm_ops sst_byt_pcm_ops = { +static const struct snd_pcm_ops sst_byt_pcm_ops = { .open = sst_byt_pcm_open, .close = sst_byt_pcm_close, .ioctl = snd_pcm_lib_ioctl, -- 1.9.1
[PATCH 05/10] ASoC: nuc900: constify snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Signed-off-by: Arvind Yadav--- sound/soc/nuc900/nuc900-pcm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/nuc900/nuc900-pcm.c b/sound/soc/nuc900/nuc900-pcm.c index 2cca055..7f2d55d 100644 --- a/sound/soc/nuc900/nuc900-pcm.c +++ b/sound/soc/nuc900/nuc900-pcm.c @@ -271,7 +271,7 @@ static int nuc900_dma_mmap(struct snd_pcm_substream *substream, runtime->dma_addr, runtime->dma_bytes); } -static struct snd_pcm_ops nuc900_dma_ops = { +static const struct snd_pcm_ops nuc900_dma_ops = { .open = nuc900_dma_open, .close = nuc900_dma_close, .ioctl = snd_pcm_lib_ioctl, -- 1.9.1
[PATCH 04/10] ASoC: intel: constify snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Signed-off-by: Arvind Yadav --- sound/soc/intel/baytrail/sst-baytrail-pcm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/intel/baytrail/sst-baytrail-pcm.c b/sound/soc/intel/baytrail/sst-baytrail-pcm.c index 4765ad4..0bdc057 100644 --- a/sound/soc/intel/baytrail/sst-baytrail-pcm.c +++ b/sound/soc/intel/baytrail/sst-baytrail-pcm.c @@ -309,7 +309,7 @@ static int sst_byt_pcm_mmap(struct snd_pcm_substream *substream, return snd_pcm_lib_default_mmap(substream, vma); } -static struct snd_pcm_ops sst_byt_pcm_ops = { +static const struct snd_pcm_ops sst_byt_pcm_ops = { .open = sst_byt_pcm_open, .close = sst_byt_pcm_close, .ioctl = snd_pcm_lib_ioctl, -- 1.9.1
[PATCH 05/10] ASoC: nuc900: constify snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Signed-off-by: Arvind Yadav --- sound/soc/nuc900/nuc900-pcm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/nuc900/nuc900-pcm.c b/sound/soc/nuc900/nuc900-pcm.c index 2cca055..7f2d55d 100644 --- a/sound/soc/nuc900/nuc900-pcm.c +++ b/sound/soc/nuc900/nuc900-pcm.c @@ -271,7 +271,7 @@ static int nuc900_dma_mmap(struct snd_pcm_substream *substream, runtime->dma_addr, runtime->dma_bytes); } -static struct snd_pcm_ops nuc900_dma_ops = { +static const struct snd_pcm_ops nuc900_dma_ops = { .open = nuc900_dma_open, .close = nuc900_dma_close, .ioctl = snd_pcm_lib_ioctl, -- 1.9.1
[PATCH 06/10] ASoC: omap: constify snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Signed-off-by: Arvind Yadav--- sound/soc/omap/omap-pcm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c index 94e9ff7..e2f1245 100644 --- a/sound/soc/omap/omap-pcm.c +++ b/sound/soc/omap/omap-pcm.c @@ -162,7 +162,7 @@ static int omap_pcm_mmap(struct snd_pcm_substream *substream, runtime->dma_addr, runtime->dma_bytes); } -static struct snd_pcm_ops omap_pcm_ops = { +static const struct snd_pcm_ops omap_pcm_ops = { .open = omap_pcm_open, .close = snd_dmaengine_pcm_close_release_chan, .ioctl = snd_pcm_lib_ioctl, -- 1.9.1
[PATCH 06/10] ASoC: omap: constify snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Signed-off-by: Arvind Yadav --- sound/soc/omap/omap-pcm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c index 94e9ff7..e2f1245 100644 --- a/sound/soc/omap/omap-pcm.c +++ b/sound/soc/omap/omap-pcm.c @@ -162,7 +162,7 @@ static int omap_pcm_mmap(struct snd_pcm_substream *substream, runtime->dma_addr, runtime->dma_bytes); } -static struct snd_pcm_ops omap_pcm_ops = { +static const struct snd_pcm_ops omap_pcm_ops = { .open = omap_pcm_open, .close = snd_dmaengine_pcm_close_release_chan, .ioctl = snd_pcm_lib_ioctl, -- 1.9.1
[PATCH 02/10] ASoC: blackfin: constify snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Signed-off-by: Arvind Yadav--- sound/soc/blackfin/bf5xx-ac97-pcm.c | 2 +- sound/soc/blackfin/bf5xx-i2s-pcm.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/soc/blackfin/bf5xx-ac97-pcm.c b/sound/soc/blackfin/bf5xx-ac97-pcm.c index 913e292..8c1d198 100644 --- a/sound/soc/blackfin/bf5xx-ac97-pcm.c +++ b/sound/soc/blackfin/bf5xx-ac97-pcm.c @@ -308,7 +308,7 @@ static int bf5xx_pcm_copy_user(struct snd_pcm_substream *substream, } #endif -static struct snd_pcm_ops bf5xx_pcm_ac97_ops = { +static const struct snd_pcm_ops bf5xx_pcm_ac97_ops = { .open = bf5xx_pcm_open, .ioctl = snd_pcm_lib_ioctl, .hw_params = bf5xx_pcm_hw_params, diff --git a/sound/soc/blackfin/bf5xx-i2s-pcm.c b/sound/soc/blackfin/bf5xx-i2s-pcm.c index 470d99a..51cae76 100644 --- a/sound/soc/blackfin/bf5xx-i2s-pcm.c +++ b/sound/soc/blackfin/bf5xx-i2s-pcm.c @@ -318,7 +318,7 @@ static int bf5xx_pcm_silence(struct snd_pcm_substream *substream, return 0; } -static struct snd_pcm_ops bf5xx_pcm_i2s_ops = { +static const struct snd_pcm_ops bf5xx_pcm_i2s_ops = { .open = bf5xx_pcm_open, .ioctl = snd_pcm_lib_ioctl, .hw_params = bf5xx_pcm_hw_params, -- 1.9.1
[PATCH 02/10] ASoC: blackfin: constify snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Signed-off-by: Arvind Yadav --- sound/soc/blackfin/bf5xx-ac97-pcm.c | 2 +- sound/soc/blackfin/bf5xx-i2s-pcm.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/soc/blackfin/bf5xx-ac97-pcm.c b/sound/soc/blackfin/bf5xx-ac97-pcm.c index 913e292..8c1d198 100644 --- a/sound/soc/blackfin/bf5xx-ac97-pcm.c +++ b/sound/soc/blackfin/bf5xx-ac97-pcm.c @@ -308,7 +308,7 @@ static int bf5xx_pcm_copy_user(struct snd_pcm_substream *substream, } #endif -static struct snd_pcm_ops bf5xx_pcm_ac97_ops = { +static const struct snd_pcm_ops bf5xx_pcm_ac97_ops = { .open = bf5xx_pcm_open, .ioctl = snd_pcm_lib_ioctl, .hw_params = bf5xx_pcm_hw_params, diff --git a/sound/soc/blackfin/bf5xx-i2s-pcm.c b/sound/soc/blackfin/bf5xx-i2s-pcm.c index 470d99a..51cae76 100644 --- a/sound/soc/blackfin/bf5xx-i2s-pcm.c +++ b/sound/soc/blackfin/bf5xx-i2s-pcm.c @@ -318,7 +318,7 @@ static int bf5xx_pcm_silence(struct snd_pcm_substream *substream, return 0; } -static struct snd_pcm_ops bf5xx_pcm_i2s_ops = { +static const struct snd_pcm_ops bf5xx_pcm_i2s_ops = { .open = bf5xx_pcm_open, .ioctl = snd_pcm_lib_ioctl, .hw_params = bf5xx_pcm_hw_params, -- 1.9.1
[PATCH 03/10] ASoC: fsl: constify snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Signed-off-by: Arvind Yadav--- sound/soc/fsl/fsl_dma.c | 2 +- sound/soc/fsl/imx-pcm-fiq.c | 2 +- sound/soc/fsl/mpc5200_dma.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c index ccadefc..dc71942 100644 --- a/sound/soc/fsl/fsl_dma.c +++ b/sound/soc/fsl/fsl_dma.c @@ -870,7 +870,7 @@ static struct device_node *find_ssi_node(struct device_node *dma_channel_np) return NULL; } -static struct snd_pcm_ops fsl_dma_ops = { +static const struct snd_pcm_ops fsl_dma_ops = { .open = fsl_dma_open, .close = fsl_dma_close, .ioctl = snd_pcm_lib_ioctl, diff --git a/sound/soc/fsl/imx-pcm-fiq.c b/sound/soc/fsl/imx-pcm-fiq.c index 92410f7..9ce3761 100644 --- a/sound/soc/fsl/imx-pcm-fiq.c +++ b/sound/soc/fsl/imx-pcm-fiq.c @@ -227,7 +227,7 @@ static int snd_imx_pcm_mmap(struct snd_pcm_substream *substream, return ret; } -static struct snd_pcm_ops imx_pcm_ops = { +static const struct snd_pcm_ops imx_pcm_ops = { .open = snd_imx_open, .close = snd_imx_close, .ioctl = snd_pcm_lib_ioctl, diff --git a/sound/soc/fsl/mpc5200_dma.c b/sound/soc/fsl/mpc5200_dma.c index 1f7e70b..66042ce 100644 --- a/sound/soc/fsl/mpc5200_dma.c +++ b/sound/soc/fsl/mpc5200_dma.c @@ -287,7 +287,7 @@ static int psc_dma_close(struct snd_pcm_substream *substream) return 0; } -static struct snd_pcm_ops psc_dma_ops = { +static const struct snd_pcm_ops psc_dma_ops = { .open = psc_dma_open, .close = psc_dma_close, .hw_free= psc_dma_hw_free, -- 1.9.1
[PATCH 08/10] ASoC: samsung: constify snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Signed-off-by: Arvind Yadav--- sound/soc/samsung/idma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/samsung/idma.c b/sound/soc/samsung/idma.c index 3e40815..5c59c95 100644 --- a/sound/soc/samsung/idma.c +++ b/sound/soc/samsung/idma.c @@ -325,7 +325,7 @@ static int idma_close(struct snd_pcm_substream *substream) return 0; } -static struct snd_pcm_ops idma_ops = { +static const struct snd_pcm_ops idma_ops = { .open = idma_open, .close = idma_close, .ioctl = snd_pcm_lib_ioctl, -- 1.9.1
[PATCH 09/10] ASoC: sh: constify snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Signed-off-by: Arvind Yadav--- sound/soc/sh/dma-sh7760.c | 2 +- sound/soc/sh/fsi.c| 2 +- sound/soc/sh/rcar/core.c | 2 +- sound/soc/sh/siu_pcm.c| 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sound/soc/sh/dma-sh7760.c b/sound/soc/sh/dma-sh7760.c index 8fad444..df92f38 100644 --- a/sound/soc/sh/dma-sh7760.c +++ b/sound/soc/sh/dma-sh7760.c @@ -294,7 +294,7 @@ static snd_pcm_uframes_t camelot_pos(struct snd_pcm_substream *substream) return bytes_to_frames(runtime, pos); } -static struct snd_pcm_ops camelot_pcm_ops = { +static const struct snd_pcm_ops camelot_pcm_ops = { .open = camelot_pcm_open, .close = camelot_pcm_close, .ioctl = snd_pcm_lib_ioctl, diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index 7c4bdd8..094e285 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -1755,7 +1755,7 @@ static snd_pcm_uframes_t fsi_pointer(struct snd_pcm_substream *substream) return fsi_sample2frame(fsi, io->buff_sample_pos); } -static struct snd_pcm_ops fsi_pcm_ops = { +static const struct snd_pcm_ops fsi_pcm_ops = { .open = fsi_pcm_open, .ioctl = snd_pcm_lib_ioctl, .hw_params = fsi_hw_params, diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c index 3f2ced2..6f24dd1 100644 --- a/sound/soc/sh/rcar/core.c +++ b/sound/soc/sh/rcar/core.c @@ -1158,7 +1158,7 @@ static snd_pcm_uframes_t rsnd_pointer(struct snd_pcm_substream *substream) return pointer; } -static struct snd_pcm_ops rsnd_pcm_ops = { +static const struct snd_pcm_ops rsnd_pcm_ops = { .open = rsnd_pcm_open, .ioctl = snd_pcm_lib_ioctl, .hw_params = rsnd_hw_params, diff --git a/sound/soc/sh/siu_pcm.c b/sound/soc/sh/siu_pcm.c index 82902f5..3118cb0 100644 --- a/sound/soc/sh/siu_pcm.c +++ b/sound/soc/sh/siu_pcm.c @@ -593,7 +593,7 @@ static void siu_pcm_free(struct snd_pcm *pcm) dev_dbg(pcm->card->dev, "%s\n", __func__); } -static struct snd_pcm_ops siu_pcm_ops = { +static const struct snd_pcm_ops siu_pcm_ops = { .open = siu_pcm_open, .close = siu_pcm_close, .ioctl = snd_pcm_lib_ioctl, -- 1.9.1
[PATCH 10/10] ASoC: txx9: constify snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Signed-off-by: Arvind Yadav--- sound/soc/txx9/txx9aclc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/txx9/txx9aclc.c b/sound/soc/txx9/txx9aclc.c index 7912bf0..2e82d06 100644 --- a/sound/soc/txx9/txx9aclc.c +++ b/sound/soc/txx9/txx9aclc.c @@ -271,7 +271,7 @@ static int txx9aclc_pcm_close(struct snd_pcm_substream *substream) return 0; } -static struct snd_pcm_ops txx9aclc_pcm_ops = { +static const struct snd_pcm_ops txx9aclc_pcm_ops = { .open = txx9aclc_pcm_open, .close = txx9aclc_pcm_close, .ioctl = snd_pcm_lib_ioctl, -- 1.9.1
[PATCH 03/10] ASoC: fsl: constify snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Signed-off-by: Arvind Yadav --- sound/soc/fsl/fsl_dma.c | 2 +- sound/soc/fsl/imx-pcm-fiq.c | 2 +- sound/soc/fsl/mpc5200_dma.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c index ccadefc..dc71942 100644 --- a/sound/soc/fsl/fsl_dma.c +++ b/sound/soc/fsl/fsl_dma.c @@ -870,7 +870,7 @@ static struct device_node *find_ssi_node(struct device_node *dma_channel_np) return NULL; } -static struct snd_pcm_ops fsl_dma_ops = { +static const struct snd_pcm_ops fsl_dma_ops = { .open = fsl_dma_open, .close = fsl_dma_close, .ioctl = snd_pcm_lib_ioctl, diff --git a/sound/soc/fsl/imx-pcm-fiq.c b/sound/soc/fsl/imx-pcm-fiq.c index 92410f7..9ce3761 100644 --- a/sound/soc/fsl/imx-pcm-fiq.c +++ b/sound/soc/fsl/imx-pcm-fiq.c @@ -227,7 +227,7 @@ static int snd_imx_pcm_mmap(struct snd_pcm_substream *substream, return ret; } -static struct snd_pcm_ops imx_pcm_ops = { +static const struct snd_pcm_ops imx_pcm_ops = { .open = snd_imx_open, .close = snd_imx_close, .ioctl = snd_pcm_lib_ioctl, diff --git a/sound/soc/fsl/mpc5200_dma.c b/sound/soc/fsl/mpc5200_dma.c index 1f7e70b..66042ce 100644 --- a/sound/soc/fsl/mpc5200_dma.c +++ b/sound/soc/fsl/mpc5200_dma.c @@ -287,7 +287,7 @@ static int psc_dma_close(struct snd_pcm_substream *substream) return 0; } -static struct snd_pcm_ops psc_dma_ops = { +static const struct snd_pcm_ops psc_dma_ops = { .open = psc_dma_open, .close = psc_dma_close, .hw_free= psc_dma_hw_free, -- 1.9.1
[PATCH 08/10] ASoC: samsung: constify snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Signed-off-by: Arvind Yadav --- sound/soc/samsung/idma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/samsung/idma.c b/sound/soc/samsung/idma.c index 3e40815..5c59c95 100644 --- a/sound/soc/samsung/idma.c +++ b/sound/soc/samsung/idma.c @@ -325,7 +325,7 @@ static int idma_close(struct snd_pcm_substream *substream) return 0; } -static struct snd_pcm_ops idma_ops = { +static const struct snd_pcm_ops idma_ops = { .open = idma_open, .close = idma_close, .ioctl = snd_pcm_lib_ioctl, -- 1.9.1
[PATCH 09/10] ASoC: sh: constify snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Signed-off-by: Arvind Yadav --- sound/soc/sh/dma-sh7760.c | 2 +- sound/soc/sh/fsi.c| 2 +- sound/soc/sh/rcar/core.c | 2 +- sound/soc/sh/siu_pcm.c| 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sound/soc/sh/dma-sh7760.c b/sound/soc/sh/dma-sh7760.c index 8fad444..df92f38 100644 --- a/sound/soc/sh/dma-sh7760.c +++ b/sound/soc/sh/dma-sh7760.c @@ -294,7 +294,7 @@ static snd_pcm_uframes_t camelot_pos(struct snd_pcm_substream *substream) return bytes_to_frames(runtime, pos); } -static struct snd_pcm_ops camelot_pcm_ops = { +static const struct snd_pcm_ops camelot_pcm_ops = { .open = camelot_pcm_open, .close = camelot_pcm_close, .ioctl = snd_pcm_lib_ioctl, diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c index 7c4bdd8..094e285 100644 --- a/sound/soc/sh/fsi.c +++ b/sound/soc/sh/fsi.c @@ -1755,7 +1755,7 @@ static snd_pcm_uframes_t fsi_pointer(struct snd_pcm_substream *substream) return fsi_sample2frame(fsi, io->buff_sample_pos); } -static struct snd_pcm_ops fsi_pcm_ops = { +static const struct snd_pcm_ops fsi_pcm_ops = { .open = fsi_pcm_open, .ioctl = snd_pcm_lib_ioctl, .hw_params = fsi_hw_params, diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c index 3f2ced2..6f24dd1 100644 --- a/sound/soc/sh/rcar/core.c +++ b/sound/soc/sh/rcar/core.c @@ -1158,7 +1158,7 @@ static snd_pcm_uframes_t rsnd_pointer(struct snd_pcm_substream *substream) return pointer; } -static struct snd_pcm_ops rsnd_pcm_ops = { +static const struct snd_pcm_ops rsnd_pcm_ops = { .open = rsnd_pcm_open, .ioctl = snd_pcm_lib_ioctl, .hw_params = rsnd_hw_params, diff --git a/sound/soc/sh/siu_pcm.c b/sound/soc/sh/siu_pcm.c index 82902f5..3118cb0 100644 --- a/sound/soc/sh/siu_pcm.c +++ b/sound/soc/sh/siu_pcm.c @@ -593,7 +593,7 @@ static void siu_pcm_free(struct snd_pcm *pcm) dev_dbg(pcm->card->dev, "%s\n", __func__); } -static struct snd_pcm_ops siu_pcm_ops = { +static const struct snd_pcm_ops siu_pcm_ops = { .open = siu_pcm_open, .close = siu_pcm_close, .ioctl = snd_pcm_lib_ioctl, -- 1.9.1
[PATCH 10/10] ASoC: txx9: constify snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Signed-off-by: Arvind Yadav --- sound/soc/txx9/txx9aclc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/txx9/txx9aclc.c b/sound/soc/txx9/txx9aclc.c index 7912bf0..2e82d06 100644 --- a/sound/soc/txx9/txx9aclc.c +++ b/sound/soc/txx9/txx9aclc.c @@ -271,7 +271,7 @@ static int txx9aclc_pcm_close(struct snd_pcm_substream *substream) return 0; } -static struct snd_pcm_ops txx9aclc_pcm_ops = { +static const struct snd_pcm_ops txx9aclc_pcm_ops = { .open = txx9aclc_pcm_open, .close = txx9aclc_pcm_close, .ioctl = snd_pcm_lib_ioctl, -- 1.9.1
[PATCH 07/10] ASoC: pxa: constify snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Signed-off-by: Arvind Yadav--- sound/soc/pxa/mmp-pcm.c| 2 +- sound/soc/pxa/pxa2xx-pcm.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/soc/pxa/mmp-pcm.c b/sound/soc/pxa/mmp-pcm.c index 5b5f1a4..e25f1c2 100644 --- a/sound/soc/pxa/mmp-pcm.c +++ b/sound/soc/pxa/mmp-pcm.c @@ -131,7 +131,7 @@ static int mmp_pcm_mmap(struct snd_pcm_substream *substream, vma->vm_end - vma->vm_start, vma->vm_page_prot); } -static struct snd_pcm_ops mmp_pcm_ops = { +static const struct snd_pcm_ops mmp_pcm_ops = { .open = mmp_pcm_open, .close = snd_dmaengine_pcm_close_release_chan, .ioctl = snd_pcm_lib_ioctl, diff --git a/sound/soc/pxa/pxa2xx-pcm.c b/sound/soc/pxa/pxa2xx-pcm.c index b51d7a0..357981c 100644 --- a/sound/soc/pxa/pxa2xx-pcm.c +++ b/sound/soc/pxa/pxa2xx-pcm.c @@ -45,7 +45,7 @@ static int pxa2xx_pcm_hw_free(struct snd_pcm_substream *substream) return 0; } -static struct snd_pcm_ops pxa2xx_pcm_ops = { +static const struct snd_pcm_ops pxa2xx_pcm_ops = { .open = __pxa2xx_pcm_open, .close = __pxa2xx_pcm_close, .ioctl = snd_pcm_lib_ioctl, -- 1.9.1
[PATCH 00/10] constify ASoC snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Arvind Yadav (10): [PATCH 01/10] ASoC: au1x: constify snd_pcm_ops structures [PATCH 02/10] ASoC: blackfin: constify snd_pcm_ops structures [PATCH 03/10] ASoC: fsl: constify snd_pcm_ops structures [PATCH 04/10] ASoC: intel: constify snd_pcm_ops structures [PATCH 05/10] ASoC: nuc900: constify snd_pcm_ops structures [PATCH 06/10] ASoC: omap: constify snd_pcm_ops structures [PATCH 07/10] ASoC: pxa: constify snd_pcm_ops structures [PATCH 08/10] ASoC: samsung: constify snd_pcm_ops structures [PATCH 09/10] ASoC: sh: constify snd_pcm_ops structures [PATCH 10/10] ASoC: txx9: constify snd_pcm_ops structures sound/soc/au1x/dbdma2.c | 2 +- sound/soc/au1x/dma.c| 2 +- sound/soc/blackfin/bf5xx-ac97-pcm.c | 2 +- sound/soc/blackfin/bf5xx-i2s-pcm.c | 2 +- sound/soc/fsl/fsl_dma.c | 2 +- sound/soc/fsl/imx-pcm-fiq.c | 2 +- sound/soc/fsl/mpc5200_dma.c | 2 +- sound/soc/intel/baytrail/sst-baytrail-pcm.c | 2 +- sound/soc/nuc900/nuc900-pcm.c | 2 +- sound/soc/omap/omap-pcm.c | 2 +- sound/soc/pxa/mmp-pcm.c | 2 +- sound/soc/pxa/pxa2xx-pcm.c | 2 +- sound/soc/samsung/idma.c| 2 +- sound/soc/sh/dma-sh7760.c | 2 +- sound/soc/sh/fsi.c | 2 +- sound/soc/sh/rcar/core.c| 2 +- sound/soc/sh/siu_pcm.c | 2 +- sound/soc/txx9/txx9aclc.c | 2 +- 18 files changed, 18 insertions(+), 18 deletions(-) -- 1.9.1
[PATCH 07/10] ASoC: pxa: constify snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Signed-off-by: Arvind Yadav --- sound/soc/pxa/mmp-pcm.c| 2 +- sound/soc/pxa/pxa2xx-pcm.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/soc/pxa/mmp-pcm.c b/sound/soc/pxa/mmp-pcm.c index 5b5f1a4..e25f1c2 100644 --- a/sound/soc/pxa/mmp-pcm.c +++ b/sound/soc/pxa/mmp-pcm.c @@ -131,7 +131,7 @@ static int mmp_pcm_mmap(struct snd_pcm_substream *substream, vma->vm_end - vma->vm_start, vma->vm_page_prot); } -static struct snd_pcm_ops mmp_pcm_ops = { +static const struct snd_pcm_ops mmp_pcm_ops = { .open = mmp_pcm_open, .close = snd_dmaengine_pcm_close_release_chan, .ioctl = snd_pcm_lib_ioctl, diff --git a/sound/soc/pxa/pxa2xx-pcm.c b/sound/soc/pxa/pxa2xx-pcm.c index b51d7a0..357981c 100644 --- a/sound/soc/pxa/pxa2xx-pcm.c +++ b/sound/soc/pxa/pxa2xx-pcm.c @@ -45,7 +45,7 @@ static int pxa2xx_pcm_hw_free(struct snd_pcm_substream *substream) return 0; } -static struct snd_pcm_ops pxa2xx_pcm_ops = { +static const struct snd_pcm_ops pxa2xx_pcm_ops = { .open = __pxa2xx_pcm_open, .close = __pxa2xx_pcm_close, .ioctl = snd_pcm_lib_ioctl, -- 1.9.1
[PATCH 00/10] constify ASoC snd_pcm_ops structures
snd_pcm_ops are not supposed to change at runtime. All functions working with snd_pcm_ops provided by work with const snd_pcm_ops. So mark the non-const structs as const. Arvind Yadav (10): [PATCH 01/10] ASoC: au1x: constify snd_pcm_ops structures [PATCH 02/10] ASoC: blackfin: constify snd_pcm_ops structures [PATCH 03/10] ASoC: fsl: constify snd_pcm_ops structures [PATCH 04/10] ASoC: intel: constify snd_pcm_ops structures [PATCH 05/10] ASoC: nuc900: constify snd_pcm_ops structures [PATCH 06/10] ASoC: omap: constify snd_pcm_ops structures [PATCH 07/10] ASoC: pxa: constify snd_pcm_ops structures [PATCH 08/10] ASoC: samsung: constify snd_pcm_ops structures [PATCH 09/10] ASoC: sh: constify snd_pcm_ops structures [PATCH 10/10] ASoC: txx9: constify snd_pcm_ops structures sound/soc/au1x/dbdma2.c | 2 +- sound/soc/au1x/dma.c| 2 +- sound/soc/blackfin/bf5xx-ac97-pcm.c | 2 +- sound/soc/blackfin/bf5xx-i2s-pcm.c | 2 +- sound/soc/fsl/fsl_dma.c | 2 +- sound/soc/fsl/imx-pcm-fiq.c | 2 +- sound/soc/fsl/mpc5200_dma.c | 2 +- sound/soc/intel/baytrail/sst-baytrail-pcm.c | 2 +- sound/soc/nuc900/nuc900-pcm.c | 2 +- sound/soc/omap/omap-pcm.c | 2 +- sound/soc/pxa/mmp-pcm.c | 2 +- sound/soc/pxa/pxa2xx-pcm.c | 2 +- sound/soc/samsung/idma.c| 2 +- sound/soc/sh/dma-sh7760.c | 2 +- sound/soc/sh/fsi.c | 2 +- sound/soc/sh/rcar/core.c| 2 +- sound/soc/sh/siu_pcm.c | 2 +- sound/soc/txx9/txx9aclc.c | 2 +- 18 files changed, 18 insertions(+), 18 deletions(-) -- 1.9.1
Re: [PATCH v2] x86/xen/64: Rearrange the SYSCALL entries
On Sun, Aug 13, 2017 at 7:44 PM, Brian Gerstwrote: > On Mon, Aug 7, 2017 at 11:59 PM, Andy Lutomirski wrote: >> /* Normal 64-bit system call target */ >> ENTRY(xen_syscall_target) >> - undo_xen_syscall >> - jmp entry_SYSCALL_64_after_swapgs >> + popq %rcx >> + popq %r11 >> + jmp entry_SYSCALL_64_after_hwframe >> ENDPROC(xen_syscall_target) >> >> #ifdef CONFIG_IA32_EMULATION >> >> /* 32-bit compat syscall target */ >> ENTRY(xen_syscall32_target) >> - undo_xen_syscall >> - jmp entry_SYSCALL_compat >> + popq %rcx >> + popq %r11 >> + jmp entry_SYSCALL_compat_after_hwframe >> ENDPROC(xen_syscall32_target) >> >> /* 32-bit compat sysenter target */ >> ENTRY(xen_sysenter_target) >> - undo_xen_syscall >> + mov 0*8(%rsp), %rcx >> + mov 1*8(%rsp), %r11 >> + mov 5*8(%rsp), %rsp >> jmp entry_SYSENTER_compat >> ENDPROC(xen_sysenter_target) > > This patch causes the iopl_32 and ioperm_32 self-tests to fail on a > 64-bit PV kernel. The 64-bit versions pass. It gets a seg fault after > "parent: write to 0x80 (should fail)", and the fault isn't caught by > the signal handler. It just dumps back to the shell. The tests pass > after reverting this. I can reproduce it if I emulate an AMD machine. I can "fix" it like this: diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S index a8a4f4c460a6..6255e00f425e 100644 --- a/arch/x86/xen/xen-asm_64.S +++ b/arch/x86/xen/xen-asm_64.S @@ -97,6 +97,9 @@ ENDPROC(xen_syscall_target) ENTRY(xen_syscall32_target) popq %rcx popq %r11 + movq $__USER32_DS, 4*8(%rsp) + movq $__USER32_CS, 1*8(%rsp) + movq %r11, 2*8(%rsp) jmp entry_SYSCALL_compat_after_hwframe ENDPROC(xen_syscall32_target) but I haven't tried to diagnose precisely what's going on. Xen seems to be putting the 0xe0?? values in ss and cs, which oughtn't to be a problem, but it kills opportunistic sysretl. Maybe that's triggering a preexisting bug?
Re: [PATCH v2] x86/xen/64: Rearrange the SYSCALL entries
On Sun, Aug 13, 2017 at 7:44 PM, Brian Gerst wrote: > On Mon, Aug 7, 2017 at 11:59 PM, Andy Lutomirski wrote: >> /* Normal 64-bit system call target */ >> ENTRY(xen_syscall_target) >> - undo_xen_syscall >> - jmp entry_SYSCALL_64_after_swapgs >> + popq %rcx >> + popq %r11 >> + jmp entry_SYSCALL_64_after_hwframe >> ENDPROC(xen_syscall_target) >> >> #ifdef CONFIG_IA32_EMULATION >> >> /* 32-bit compat syscall target */ >> ENTRY(xen_syscall32_target) >> - undo_xen_syscall >> - jmp entry_SYSCALL_compat >> + popq %rcx >> + popq %r11 >> + jmp entry_SYSCALL_compat_after_hwframe >> ENDPROC(xen_syscall32_target) >> >> /* 32-bit compat sysenter target */ >> ENTRY(xen_sysenter_target) >> - undo_xen_syscall >> + mov 0*8(%rsp), %rcx >> + mov 1*8(%rsp), %r11 >> + mov 5*8(%rsp), %rsp >> jmp entry_SYSENTER_compat >> ENDPROC(xen_sysenter_target) > > This patch causes the iopl_32 and ioperm_32 self-tests to fail on a > 64-bit PV kernel. The 64-bit versions pass. It gets a seg fault after > "parent: write to 0x80 (should fail)", and the fault isn't caught by > the signal handler. It just dumps back to the shell. The tests pass > after reverting this. I can reproduce it if I emulate an AMD machine. I can "fix" it like this: diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S index a8a4f4c460a6..6255e00f425e 100644 --- a/arch/x86/xen/xen-asm_64.S +++ b/arch/x86/xen/xen-asm_64.S @@ -97,6 +97,9 @@ ENDPROC(xen_syscall_target) ENTRY(xen_syscall32_target) popq %rcx popq %r11 + movq $__USER32_DS, 4*8(%rsp) + movq $__USER32_CS, 1*8(%rsp) + movq %r11, 2*8(%rsp) jmp entry_SYSCALL_compat_after_hwframe ENDPROC(xen_syscall32_target) but I haven't tried to diagnose precisely what's going on. Xen seems to be putting the 0xe0?? values in ss and cs, which oughtn't to be a problem, but it kills opportunistic sysretl. Maybe that's triggering a preexisting bug?
[PATCH] misc: mic: constify virtio_config_ops structure
This virtio_config_ops structure is only stored in the config field of a virtio_device structure and this field is const, so the virtio_config_ops structure can also be const. Done with the help of Coccinelle. Signed-off-by: Julia Lawall--- drivers/misc/mic/vop/vop_main.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/misc/mic/vop/vop_main.c b/drivers/misc/mic/vop/vop_main.c index a341938..c05caa8 100644 --- a/drivers/misc/mic/vop/vop_main.c +++ b/drivers/misc/mic/vop/vop_main.c @@ -425,7 +425,7 @@ static int vop_find_vqs(struct virtio_device *dev, unsigned nvqs, /* * The config ops structure as defined by virtio config */ -static struct virtio_config_ops vop_vq_config_ops = { +static const struct virtio_config_ops vop_vq_config_ops = { .get_features = vop_get_features, .finalize_features = vop_finalize_features, .get = vop_get,
[PATCH] misc: mic: constify virtio_config_ops structure
This virtio_config_ops structure is only stored in the config field of a virtio_device structure and this field is const, so the virtio_config_ops structure can also be const. Done with the help of Coccinelle. Signed-off-by: Julia Lawall --- drivers/misc/mic/vop/vop_main.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/misc/mic/vop/vop_main.c b/drivers/misc/mic/vop/vop_main.c index a341938..c05caa8 100644 --- a/drivers/misc/mic/vop/vop_main.c +++ b/drivers/misc/mic/vop/vop_main.c @@ -425,7 +425,7 @@ static int vop_find_vqs(struct virtio_device *dev, unsigned nvqs, /* * The config ops structure as defined by virtio config */ -static struct virtio_config_ops vop_vq_config_ops = { +static const struct virtio_config_ops vop_vq_config_ops = { .get_features = vop_get_features, .finalize_features = vop_finalize_features, .get = vop_get,
Re: [PATCH 2/9] Implement containers as kernel objects
On 2017-05-22 17:22, David Howells wrote: > A container is then a kernel object that contains the following things: > > (1) Namespaces. > > (2) A root directory. > > (3) A set of processes, including one designated as the 'init' process. > > A container is created and attached to a file descriptor by: > > int cfd = container_create(const char *name, unsigned int flags); > > this inherits all the namespaces of the parent container unless otherwise > the mask calls for new namespaces. > > CONTAINER_NEW_FS_NS > CONTAINER_NEW_EMPTY_FS_NS > CONTAINER_NEW_CGROUP_NS [root only] > CONTAINER_NEW_UTS_NS > CONTAINER_NEW_IPC_NS > CONTAINER_NEW_USER_NS > CONTAINER_NEW_PID_NS > CONTAINER_NEW_NET_NS > > Other flags include: > > CONTAINER_KILL_ON_CLOSE > CONTAINER_CLOSE_ON_EXEC Hi David, I wanted to respond to this thread to attempt some constructive feedback, better late than never. I had a look at your fsopen/fsmount() patchset(s) to support this patchset which was interesting, but doesn't directly affect my work. The primary patch of interest to the audit kernel folks (Paul Moore and me) is this patch while the rest of the patchset is interesting, but not likely to directly affect us. This patch has most of what we need to solve our problem. Paul and I agree that audit is going to have a difficult time identifying containers or even namespaces without some change to the kernel. The audit subsystem in the kernel needs at least a basic clue about which container caused an event to be able to report this at the appropriate level and ignore it at other levels to avoid a DoS. We also agree that there will need to be some sort of trigger from userspace to indicate the creation of a container and its allocated resources and we're not really picky how that is done, such as a clone flag, a syscall or a sysfs write (or even a read, I suppose), but there will need to be some permission restrictions, obviously. (I'd like to see capabilities used for this by adding a specific container bit to the capabilities bitmask.) I doubt we will be able to accomodate all definitions or concepts of a container in a timely fashion. We'll need to start somewhere with a minimum definition so that we can get traction and actually move forward before another compelling shared kernel microservice method leaves our entire community behind. I'd like to declare that a container is a full set of cloned namespaces, but this is inefficient, overly constricting and unnecessary for our needs. If we could agree on a minimum definition of a container (which may have only one specific cloned namespace) then we have something on which to build. I could even see a container being defined by a trigger sent from userspace about a process (task) from which all its children are considered to be within that container, subject to further nesting. In the simplest usable model for audit, if a container (definition implies and) starts a PID namespace, then the container ID could simply be the container's "init" process PID in the initial PID namespace. This assumes that as soon as that process vanishes, that entire container and all its children are killed off (which you've done). There may be some container orchestration systems that don't use a unique PID namespace per container and that imposing this will cause them challenges. If containers have at minimum a unique mount namespace then the root path dentry inode device and inode number could be used, but there are likely better identifiers. Again, there may be container orchestrators that don't use a unique mount namespace per container and that imposing this will cause challenges. I expect there are similar examples for each of the other namespaces. If we could pick one namespace type for consensus for which each container has a unique instance of that namespace, we could use the dev/ino tuple from that namespace as had originally been suggested by Aristeu Rozanski more than 4 years ago as part of the set of namespace IDs. I had also attempted to solve this problem by using the namespace' proc inode, then switched over to generate a unique kernel serial number for each namespace and then went back to namespace proc dev/ino once Al Viro implemented nsfs: v1 https://lkml.org/lkml/2014/4/22/662 v2 https://lkml.org/lkml/2014/5/9/637 v3 https://lkml.org/lkml/2014/5/20/287 v4 https://lkml.org/lkml/2014/8/20/844 v5 https://lkml.org/lkml/2014/10/6/25 v6 https://lkml.org/lkml/2015/4/17/48 v7 https://lkml.org/lkml/2015/5/12/773 These patches don't use a container ID, but track all namespaces in use for an event. This has the benefit of punting this tracking to userspace for some other tool to analyse and determine to which container an event belongs. This will use a lot of bandwidth in audit log files when a single container ID that
Re: [PATCH 2/9] Implement containers as kernel objects
On 2017-05-22 17:22, David Howells wrote: > A container is then a kernel object that contains the following things: > > (1) Namespaces. > > (2) A root directory. > > (3) A set of processes, including one designated as the 'init' process. > > A container is created and attached to a file descriptor by: > > int cfd = container_create(const char *name, unsigned int flags); > > this inherits all the namespaces of the parent container unless otherwise > the mask calls for new namespaces. > > CONTAINER_NEW_FS_NS > CONTAINER_NEW_EMPTY_FS_NS > CONTAINER_NEW_CGROUP_NS [root only] > CONTAINER_NEW_UTS_NS > CONTAINER_NEW_IPC_NS > CONTAINER_NEW_USER_NS > CONTAINER_NEW_PID_NS > CONTAINER_NEW_NET_NS > > Other flags include: > > CONTAINER_KILL_ON_CLOSE > CONTAINER_CLOSE_ON_EXEC Hi David, I wanted to respond to this thread to attempt some constructive feedback, better late than never. I had a look at your fsopen/fsmount() patchset(s) to support this patchset which was interesting, but doesn't directly affect my work. The primary patch of interest to the audit kernel folks (Paul Moore and me) is this patch while the rest of the patchset is interesting, but not likely to directly affect us. This patch has most of what we need to solve our problem. Paul and I agree that audit is going to have a difficult time identifying containers or even namespaces without some change to the kernel. The audit subsystem in the kernel needs at least a basic clue about which container caused an event to be able to report this at the appropriate level and ignore it at other levels to avoid a DoS. We also agree that there will need to be some sort of trigger from userspace to indicate the creation of a container and its allocated resources and we're not really picky how that is done, such as a clone flag, a syscall or a sysfs write (or even a read, I suppose), but there will need to be some permission restrictions, obviously. (I'd like to see capabilities used for this by adding a specific container bit to the capabilities bitmask.) I doubt we will be able to accomodate all definitions or concepts of a container in a timely fashion. We'll need to start somewhere with a minimum definition so that we can get traction and actually move forward before another compelling shared kernel microservice method leaves our entire community behind. I'd like to declare that a container is a full set of cloned namespaces, but this is inefficient, overly constricting and unnecessary for our needs. If we could agree on a minimum definition of a container (which may have only one specific cloned namespace) then we have something on which to build. I could even see a container being defined by a trigger sent from userspace about a process (task) from which all its children are considered to be within that container, subject to further nesting. In the simplest usable model for audit, if a container (definition implies and) starts a PID namespace, then the container ID could simply be the container's "init" process PID in the initial PID namespace. This assumes that as soon as that process vanishes, that entire container and all its children are killed off (which you've done). There may be some container orchestration systems that don't use a unique PID namespace per container and that imposing this will cause them challenges. If containers have at minimum a unique mount namespace then the root path dentry inode device and inode number could be used, but there are likely better identifiers. Again, there may be container orchestrators that don't use a unique mount namespace per container and that imposing this will cause challenges. I expect there are similar examples for each of the other namespaces. If we could pick one namespace type for consensus for which each container has a unique instance of that namespace, we could use the dev/ino tuple from that namespace as had originally been suggested by Aristeu Rozanski more than 4 years ago as part of the set of namespace IDs. I had also attempted to solve this problem by using the namespace' proc inode, then switched over to generate a unique kernel serial number for each namespace and then went back to namespace proc dev/ino once Al Viro implemented nsfs: v1 https://lkml.org/lkml/2014/4/22/662 v2 https://lkml.org/lkml/2014/5/9/637 v3 https://lkml.org/lkml/2014/5/20/287 v4 https://lkml.org/lkml/2014/8/20/844 v5 https://lkml.org/lkml/2014/10/6/25 v6 https://lkml.org/lkml/2015/4/17/48 v7 https://lkml.org/lkml/2015/5/12/773 These patches don't use a container ID, but track all namespaces in use for an event. This has the benefit of punting this tracking to userspace for some other tool to analyse and determine to which container an event belongs. This will use a lot of bandwidth in audit log files when a single container ID that
Re: [PATCH 3/3] IPI: Avoid to use 2 cache lines for one call_single_data
Hi, Peter, "Huang, Ying"writes: > Peter Zijlstra writes: > >> On Sat, Aug 05, 2017 at 08:47:02AM +0800, Huang, Ying wrote: >>> Yes. That looks good. So you will prepare the final patch? Or you >>> hope me to do that? >> >> I was hoping you'd do it ;-) > > Thanks! Here is the updated patch > > Best Regards, > Huang, Ying > > -->8-- > From 957735e9ff3922368286540dab852986fc7b23b5 Mon Sep 17 00:00:00 2001 > From: Huang Ying > Date: Mon, 7 Aug 2017 16:55:33 +0800 > Subject: [PATCH -v3] IPI: Avoid to use 2 cache lines for one > call_single_data > > struct call_single_data is used in IPI to transfer information between > CPUs. Its size is bigger than sizeof(unsigned long) and less than > cache line size. Now, it is allocated with no explicit alignment > requirement. This makes it possible for allocated call_single_data to > cross 2 cache lines. So that double the number of the cache lines > that need to be transferred among CPUs. > > This is resolved by requiring call_single_data to be aligned with the > size of call_single_data. Now the size of call_single_data is the > power of 2. If we add new fields to call_single_data, we may need to > add pads to make sure the size of new definition is the power of 2. > Fortunately, this is enforced by gcc, which will report error for not > power of 2 alignment requirement. > > To set alignment requirement of call_single_data to the size of > call_single_data, a struct definition and a typedef is used. > > To test the effect of the patch, we use the vm-scalability multiple > thread swap test case (swap-w-seq-mt). The test will create multiple > threads and each thread will eat memory until all RAM and part of swap > is used, so that huge number of IPI will be triggered when unmapping > memory. In the test, the throughput of memory writing improves ~5% > compared with misaligned call_single_data because of faster IPI. What do you think about this version? Best Regards, Huang, Ying > [Add call_single_data_t and align with size of call_single_data] > Suggested-by: Peter Zijlstra > Signed-off-by: "Huang, Ying" > Cc: Peter Zijlstra > Cc: Ingo Molnar > Cc: Michael Ellerman > Cc: Borislav Petkov > Cc: Thomas Gleixner > Cc: Juergen Gross > Cc: Aaron Lu > --- > arch/mips/kernel/smp.c | 6 ++-- > block/blk-softirq.c| 2 +- > drivers/block/null_blk.c | 2 +- > drivers/cpuidle/coupled.c | 10 +++ > drivers/net/ethernet/cavium/liquidio/lio_main.c| 2 +- > drivers/net/ethernet/cavium/liquidio/octeon_droq.h | 2 +- > include/linux/blkdev.h | 2 +- > include/linux/netdevice.h | 2 +- > include/linux/smp.h| 8 -- > kernel/sched/sched.h | 2 +- > kernel/smp.c | 32 > -- > kernel/up.c| 2 +- > 12 files changed, 39 insertions(+), 33 deletions(-) > > diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c > index 770d4d1516cb..bd8ba5472bca 100644 > --- a/arch/mips/kernel/smp.c > +++ b/arch/mips/kernel/smp.c > @@ -648,12 +648,12 @@ EXPORT_SYMBOL(flush_tlb_one); > #ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST > > static DEFINE_PER_CPU(atomic_t, tick_broadcast_count); > -static DEFINE_PER_CPU(struct call_single_data, tick_broadcast_csd); > +static DEFINE_PER_CPU(call_single_data_t, tick_broadcast_csd); > > void tick_broadcast(const struct cpumask *mask) > { > atomic_t *count; > - struct call_single_data *csd; > + call_single_data_t *csd; > int cpu; > > for_each_cpu(cpu, mask) { > @@ -674,7 +674,7 @@ static void tick_broadcast_callee(void *info) > > static int __init tick_broadcast_init(void) > { > - struct call_single_data *csd; > + call_single_data_t *csd; > int cpu; > > for (cpu = 0; cpu < NR_CPUS; cpu++) { > diff --git a/block/blk-softirq.c b/block/blk-softirq.c > index 87b7df4851bf..07125e7941f4 100644 > --- a/block/blk-softirq.c > +++ b/block/blk-softirq.c > @@ -60,7 +60,7 @@ static void trigger_softirq(void *data) > static int raise_blk_irq(int cpu, struct request *rq) > { > if (cpu_online(cpu)) { > - struct call_single_data *data = >csd; > + call_single_data_t *data = >csd; > > data->func = trigger_softirq; > data->info = rq; > diff --git a/drivers/block/null_blk.c b/drivers/block/null_blk.c > index 85c24cace973..81142ce781da 100644 > --- a/drivers/block/null_blk.c > +++ b/drivers/block/null_blk.c > @@ -13,7 +13,7 @@ > struct nullb_cmd {
Re: [PATCH 3/3] IPI: Avoid to use 2 cache lines for one call_single_data
Hi, Peter, "Huang, Ying" writes: > Peter Zijlstra writes: > >> On Sat, Aug 05, 2017 at 08:47:02AM +0800, Huang, Ying wrote: >>> Yes. That looks good. So you will prepare the final patch? Or you >>> hope me to do that? >> >> I was hoping you'd do it ;-) > > Thanks! Here is the updated patch > > Best Regards, > Huang, Ying > > -->8-- > From 957735e9ff3922368286540dab852986fc7b23b5 Mon Sep 17 00:00:00 2001 > From: Huang Ying > Date: Mon, 7 Aug 2017 16:55:33 +0800 > Subject: [PATCH -v3] IPI: Avoid to use 2 cache lines for one > call_single_data > > struct call_single_data is used in IPI to transfer information between > CPUs. Its size is bigger than sizeof(unsigned long) and less than > cache line size. Now, it is allocated with no explicit alignment > requirement. This makes it possible for allocated call_single_data to > cross 2 cache lines. So that double the number of the cache lines > that need to be transferred among CPUs. > > This is resolved by requiring call_single_data to be aligned with the > size of call_single_data. Now the size of call_single_data is the > power of 2. If we add new fields to call_single_data, we may need to > add pads to make sure the size of new definition is the power of 2. > Fortunately, this is enforced by gcc, which will report error for not > power of 2 alignment requirement. > > To set alignment requirement of call_single_data to the size of > call_single_data, a struct definition and a typedef is used. > > To test the effect of the patch, we use the vm-scalability multiple > thread swap test case (swap-w-seq-mt). The test will create multiple > threads and each thread will eat memory until all RAM and part of swap > is used, so that huge number of IPI will be triggered when unmapping > memory. In the test, the throughput of memory writing improves ~5% > compared with misaligned call_single_data because of faster IPI. What do you think about this version? Best Regards, Huang, Ying > [Add call_single_data_t and align with size of call_single_data] > Suggested-by: Peter Zijlstra > Signed-off-by: "Huang, Ying" > Cc: Peter Zijlstra > Cc: Ingo Molnar > Cc: Michael Ellerman > Cc: Borislav Petkov > Cc: Thomas Gleixner > Cc: Juergen Gross > Cc: Aaron Lu > --- > arch/mips/kernel/smp.c | 6 ++-- > block/blk-softirq.c| 2 +- > drivers/block/null_blk.c | 2 +- > drivers/cpuidle/coupled.c | 10 +++ > drivers/net/ethernet/cavium/liquidio/lio_main.c| 2 +- > drivers/net/ethernet/cavium/liquidio/octeon_droq.h | 2 +- > include/linux/blkdev.h | 2 +- > include/linux/netdevice.h | 2 +- > include/linux/smp.h| 8 -- > kernel/sched/sched.h | 2 +- > kernel/smp.c | 32 > -- > kernel/up.c| 2 +- > 12 files changed, 39 insertions(+), 33 deletions(-) > > diff --git a/arch/mips/kernel/smp.c b/arch/mips/kernel/smp.c > index 770d4d1516cb..bd8ba5472bca 100644 > --- a/arch/mips/kernel/smp.c > +++ b/arch/mips/kernel/smp.c > @@ -648,12 +648,12 @@ EXPORT_SYMBOL(flush_tlb_one); > #ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST > > static DEFINE_PER_CPU(atomic_t, tick_broadcast_count); > -static DEFINE_PER_CPU(struct call_single_data, tick_broadcast_csd); > +static DEFINE_PER_CPU(call_single_data_t, tick_broadcast_csd); > > void tick_broadcast(const struct cpumask *mask) > { > atomic_t *count; > - struct call_single_data *csd; > + call_single_data_t *csd; > int cpu; > > for_each_cpu(cpu, mask) { > @@ -674,7 +674,7 @@ static void tick_broadcast_callee(void *info) > > static int __init tick_broadcast_init(void) > { > - struct call_single_data *csd; > + call_single_data_t *csd; > int cpu; > > for (cpu = 0; cpu < NR_CPUS; cpu++) { > diff --git a/block/blk-softirq.c b/block/blk-softirq.c > index 87b7df4851bf..07125e7941f4 100644 > --- a/block/blk-softirq.c > +++ b/block/blk-softirq.c > @@ -60,7 +60,7 @@ static void trigger_softirq(void *data) > static int raise_blk_irq(int cpu, struct request *rq) > { > if (cpu_online(cpu)) { > - struct call_single_data *data = >csd; > + call_single_data_t *data = >csd; > > data->func = trigger_softirq; > data->info = rq; > diff --git a/drivers/block/null_blk.c b/drivers/block/null_blk.c > index 85c24cace973..81142ce781da 100644 > --- a/drivers/block/null_blk.c > +++ b/drivers/block/null_blk.c > @@ -13,7 +13,7 @@ > struct nullb_cmd { > struct list_head list; > struct llist_node ll_list; > - struct call_single_data csd; > + call_single_data_t csd; > struct request *rq; > struct bio *bio; > unsigned int tag; > diff --git
Re: [PATCH] soc: imx: gpcv2: fix regulator deferred probe
On Sun, Aug 13, 2017 at 10:12:13PM -0700, Stefan Agner wrote: > Hi Shawn, > > On 2017-08-04 20:13, Shawn Guo wrote: > > On Wed, Aug 02, 2017 at 12:51:29PM -0700, Stefan Agner wrote: > >> If a regulator requests a deferred probe, the power domain gets > >> initialized twice. This leads to a list double add (without > >> list debugging the kernel hangs due to the double add later): > >> > >> WARNING: CPU: 0 PID: 19 at lib/list_debug.c:31 __list_add_valid+0xbc/0xc4 > >> list_add double add: new=c1229754, prev=c12383b4, next=c1229754. > >> > >> Initialize the power domain after we get the regulator. Also do > >> not print an error in case the regulator defers probing. > >> > >> Cc: Fabio Estevam> >> Cc: Andrey Smirnov > >> Cc: linux-arm-ker...@lists.infradead.org > >> Cc: linux-kernel@vger.kernel.org > >> Fixes: 03aa12629fc4 ("soc: imx: Add GPCv2 power gating driver") > >> Signed-off-by: Stefan Agner > > > > Applied, thanks. > > As of 4.13-rc5 this did not make it upstream yet, I guess still on its > way? Yes, Stefan. It's on the way to arm-soc [1], but not yet responded. Shawn [1] https://www.spinics.net/lists/arm-kernel/msg598634.html
Re: [PATCH] soc: imx: gpcv2: fix regulator deferred probe
On Sun, Aug 13, 2017 at 10:12:13PM -0700, Stefan Agner wrote: > Hi Shawn, > > On 2017-08-04 20:13, Shawn Guo wrote: > > On Wed, Aug 02, 2017 at 12:51:29PM -0700, Stefan Agner wrote: > >> If a regulator requests a deferred probe, the power domain gets > >> initialized twice. This leads to a list double add (without > >> list debugging the kernel hangs due to the double add later): > >> > >> WARNING: CPU: 0 PID: 19 at lib/list_debug.c:31 __list_add_valid+0xbc/0xc4 > >> list_add double add: new=c1229754, prev=c12383b4, next=c1229754. > >> > >> Initialize the power domain after we get the regulator. Also do > >> not print an error in case the regulator defers probing. > >> > >> Cc: Fabio Estevam > >> Cc: Andrey Smirnov > >> Cc: linux-arm-ker...@lists.infradead.org > >> Cc: linux-kernel@vger.kernel.org > >> Fixes: 03aa12629fc4 ("soc: imx: Add GPCv2 power gating driver") > >> Signed-off-by: Stefan Agner > > > > Applied, thanks. > > As of 4.13-rc5 this did not make it upstream yet, I guess still on its > way? Yes, Stefan. It's on the way to arm-soc [1], but not yet responded. Shawn [1] https://www.spinics.net/lists/arm-kernel/msg598634.html
[PATCH] swap: choose swap device according to numa node
If the system has more than one swap device and swap device has the node information, we can make use of this information to decide which swap device to use in get_swap_pages() to get better performance. The current code uses a priority based list, swap_avail_list, to decide which swap device to use and if multiple swap devices share the same priority, they are used round robin. This patch changes the previous single global swap_avail_list into a per-numa-node list, i.e. for each numa node, it sees its own priority based list of available swap devices. Swap device's priority can be promoted on its matching node's swap_avail_list. The current swap device's priority is set as: user can set a >=0 value, or the system will pick one starting from -1 then downwards. The priority value in the swap_avail_list is the negated value of the swap device's due to plist being sorted from low to high. The new policy doesn't change the semantics for priority >=0 cases, the previous starting from -1 then downwards now becomes starting from -2 then downwards and -1 is reserved as the promoted value. Take 4-node EX machine as an example, suppose 4 swap devices are available, each sit on a different node: swapA on node 0 swapB on node 1 swapC on node 2 swapD on node 3 After they are all swapped on in the sequence of ABCD. Current behaviour: their priorities will be: swapA: -1 swapB: -2 swapC: -3 swapD: -4 And their position in the global swap_avail_list will be: swapA -> swapB -> swapC -> swapD prio:1 prio:2 prio:3 prio:4 New behaviour: their priorities will be(note that -1 is skipped): swapA: -2 swapB: -3 swapC: -4 swapD: -5 And their positions in the 4 swap_avail_lists[nid] will be: swap_avail_lists[0]: /* node 0's available swap device list */ swapA -> swapB -> swapC -> swapD prio:1 prio:3 prio:4 prio:5 swap_avali_lists[1]: /* node 1's available swap device list */ swapB -> swapA -> swapC -> swapD prio:1 prio:2 prio:4 prio:5 swap_avail_lists[2]: /* node 2's available swap device list */ swapC -> swapA -> swapB -> swapD prio:1 prio:2 prio:3 prio:5 swap_avail_lists[3]: /* node 3's available swap device list */ swapD -> swapA -> swapB -> swapC prio:1 prio:2 prio:3 prio:4 To see the effect of the patch, a test that starts N process, each mmap a region of anonymous memory and then continually write to it at random position to trigger both swap in and out is used. On a 2 node Skylake EP machine with 64GiB memory, two 170GB SSD drives are used as swap devices with each attached to a different node, the result is: runtime=30m/processes=32/total test size=128G/each process mmap region=4G kernel throughput vanilla13306 auto-binding 15169 +14% runtime=30m/processes=64/total test size=128G/each process mmap region=2G kernel throughput vanilla11885 auto-binding 14879 25% Signed-off-by: Aaron Lu--- A previous version was sent without catching much attention: https://lkml.org/lkml/2016/7/4/633 I'm sending again with some minor modifications and after test showed performance gains for SSD. Documentation/vm/swap_numa.txt | 18 +++ include/linux/swap.h | 2 +- mm/swapfile.c | 113 +++-- 3 files changed, 106 insertions(+), 27 deletions(-) create mode 100644 Documentation/vm/swap_numa.txt diff --git a/Documentation/vm/swap_numa.txt b/Documentation/vm/swap_numa.txt new file mode 100644 index ..e63fe485567c --- /dev/null +++ b/Documentation/vm/swap_numa.txt @@ -0,0 +1,18 @@ +If the system has more than one swap device and swap device has the node +information, we can make use of this information to decide which swap +device to use in get_swap_pages() to get better performance. + +The current code uses a priority based list, swap_avail_list, to decide +which swap device to use and if multiple swap devices share the same +priority, they are used round robin. This change here replaces the single +global swap_avail_list with a per-numa-node list, i.e. for each numa node, +it sees its own priority based list of available swap devices. Swap +device's priority can be promoted on its matching node's swap_avail_list. + +The current swap device's priority is set as: user can set a >=0 value, +or the system will pick one starting from -1 then downwards. The priority +value in the swap_avail_list is the negated value of the swap device's +due to plist being sorted from low to high. The new policy doesn't change +the semantics for priority >=0 cases, the previous starting from -1 then +downwards now becomes starting from -2 then downwards and -1 is reserved +as the promoted value. diff --git a/include/linux/swap.h b/include/linux/swap.h index d83d28e53e62..28262fe683ad 100644 --- a/include/linux/swap.h +++ b/include/linux/swap.h @@ -211,7 +211,7 @@ struct swap_info_struct { unsigned long flags; /*
[PATCH] swap: choose swap device according to numa node
If the system has more than one swap device and swap device has the node information, we can make use of this information to decide which swap device to use in get_swap_pages() to get better performance. The current code uses a priority based list, swap_avail_list, to decide which swap device to use and if multiple swap devices share the same priority, they are used round robin. This patch changes the previous single global swap_avail_list into a per-numa-node list, i.e. for each numa node, it sees its own priority based list of available swap devices. Swap device's priority can be promoted on its matching node's swap_avail_list. The current swap device's priority is set as: user can set a >=0 value, or the system will pick one starting from -1 then downwards. The priority value in the swap_avail_list is the negated value of the swap device's due to plist being sorted from low to high. The new policy doesn't change the semantics for priority >=0 cases, the previous starting from -1 then downwards now becomes starting from -2 then downwards and -1 is reserved as the promoted value. Take 4-node EX machine as an example, suppose 4 swap devices are available, each sit on a different node: swapA on node 0 swapB on node 1 swapC on node 2 swapD on node 3 After they are all swapped on in the sequence of ABCD. Current behaviour: their priorities will be: swapA: -1 swapB: -2 swapC: -3 swapD: -4 And their position in the global swap_avail_list will be: swapA -> swapB -> swapC -> swapD prio:1 prio:2 prio:3 prio:4 New behaviour: their priorities will be(note that -1 is skipped): swapA: -2 swapB: -3 swapC: -4 swapD: -5 And their positions in the 4 swap_avail_lists[nid] will be: swap_avail_lists[0]: /* node 0's available swap device list */ swapA -> swapB -> swapC -> swapD prio:1 prio:3 prio:4 prio:5 swap_avali_lists[1]: /* node 1's available swap device list */ swapB -> swapA -> swapC -> swapD prio:1 prio:2 prio:4 prio:5 swap_avail_lists[2]: /* node 2's available swap device list */ swapC -> swapA -> swapB -> swapD prio:1 prio:2 prio:3 prio:5 swap_avail_lists[3]: /* node 3's available swap device list */ swapD -> swapA -> swapB -> swapC prio:1 prio:2 prio:3 prio:4 To see the effect of the patch, a test that starts N process, each mmap a region of anonymous memory and then continually write to it at random position to trigger both swap in and out is used. On a 2 node Skylake EP machine with 64GiB memory, two 170GB SSD drives are used as swap devices with each attached to a different node, the result is: runtime=30m/processes=32/total test size=128G/each process mmap region=4G kernel throughput vanilla13306 auto-binding 15169 +14% runtime=30m/processes=64/total test size=128G/each process mmap region=2G kernel throughput vanilla11885 auto-binding 14879 25% Signed-off-by: Aaron Lu --- A previous version was sent without catching much attention: https://lkml.org/lkml/2016/7/4/633 I'm sending again with some minor modifications and after test showed performance gains for SSD. Documentation/vm/swap_numa.txt | 18 +++ include/linux/swap.h | 2 +- mm/swapfile.c | 113 +++-- 3 files changed, 106 insertions(+), 27 deletions(-) create mode 100644 Documentation/vm/swap_numa.txt diff --git a/Documentation/vm/swap_numa.txt b/Documentation/vm/swap_numa.txt new file mode 100644 index ..e63fe485567c --- /dev/null +++ b/Documentation/vm/swap_numa.txt @@ -0,0 +1,18 @@ +If the system has more than one swap device and swap device has the node +information, we can make use of this information to decide which swap +device to use in get_swap_pages() to get better performance. + +The current code uses a priority based list, swap_avail_list, to decide +which swap device to use and if multiple swap devices share the same +priority, they are used round robin. This change here replaces the single +global swap_avail_list with a per-numa-node list, i.e. for each numa node, +it sees its own priority based list of available swap devices. Swap +device's priority can be promoted on its matching node's swap_avail_list. + +The current swap device's priority is set as: user can set a >=0 value, +or the system will pick one starting from -1 then downwards. The priority +value in the swap_avail_list is the negated value of the swap device's +due to plist being sorted from low to high. The new policy doesn't change +the semantics for priority >=0 cases, the previous starting from -1 then +downwards now becomes starting from -2 then downwards and -1 is reserved +as the promoted value. diff --git a/include/linux/swap.h b/include/linux/swap.h index d83d28e53e62..28262fe683ad 100644 --- a/include/linux/swap.h +++ b/include/linux/swap.h @@ -211,7 +211,7 @@ struct swap_info_struct { unsigned long flags; /* SWP_USED etc: see
Re: linux-next: manual merge of the akpm-current tree with the tip tree
On Mon, Aug 14, 2017 at 05:07:19AM +, Nadav Amit wrote: < snip > > Minchan, as for the solution you proposed, it seems to open again a race, > since the “pending” indication is removed before the actual TLB flush is > performed. Oops, you're right!
Re: linux-next: manual merge of the akpm-current tree with the tip tree
On Mon, Aug 14, 2017 at 05:07:19AM +, Nadav Amit wrote: < snip > > Minchan, as for the solution you proposed, it seems to open again a race, > since the “pending” indication is removed before the actual TLB flush is > performed. Oops, you're right!
Re: [PATCH 0/5] qcom-ufs: phy/hcd: Refactor phy initialization code
Vivek, On 8/11/2017 12:42 PM, Vivek Gautam wrote: On Fri, Aug 11, 2017 at 5:33 AM, Martin K. Petersenwrote: Vivek, Can you kindly review this patch series (for UFS controller changes) and consider giving your Ack so that Kishon can pull in the series through phy tree. SCSI piece looks OK. Thank you Martin for your review. Would still like Subhash to review the rest. Subhash is on vacation with limited access to emails. I will ask his team to take a look. Looks good to me. regards Vivek -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Asutosh Das (asd) Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH 0/5] qcom-ufs: phy/hcd: Refactor phy initialization code
Vivek, On 8/11/2017 12:42 PM, Vivek Gautam wrote: On Fri, Aug 11, 2017 at 5:33 AM, Martin K. Petersen wrote: Vivek, Can you kindly review this patch series (for UFS controller changes) and consider giving your Ack so that Kishon can pull in the series through phy tree. SCSI piece looks OK. Thank you Martin for your review. Would still like Subhash to review the rest. Subhash is on vacation with limited access to emails. I will ask his team to take a look. Looks good to me. regards Vivek -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Asutosh Das (asd) Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH] soc: imx: gpcv2: fix regulator deferred probe
Hi Shawn, On 2017-08-04 20:13, Shawn Guo wrote: > On Wed, Aug 02, 2017 at 12:51:29PM -0700, Stefan Agner wrote: >> If a regulator requests a deferred probe, the power domain gets >> initialized twice. This leads to a list double add (without >> list debugging the kernel hangs due to the double add later): >> >> WARNING: CPU: 0 PID: 19 at lib/list_debug.c:31 __list_add_valid+0xbc/0xc4 >> list_add double add: new=c1229754, prev=c12383b4, next=c1229754. >> >> Initialize the power domain after we get the regulator. Also do >> not print an error in case the regulator defers probing. >> >> Cc: Fabio Estevam>> Cc: Andrey Smirnov >> Cc: linux-arm-ker...@lists.infradead.org >> Cc: linux-kernel@vger.kernel.org >> Fixes: 03aa12629fc4 ("soc: imx: Add GPCv2 power gating driver") >> Signed-off-by: Stefan Agner > > Applied, thanks. As of 4.13-rc5 this did not make it upstream yet, I guess still on its way? -- Stefan
Re: [PATCH] soc: imx: gpcv2: fix regulator deferred probe
Hi Shawn, On 2017-08-04 20:13, Shawn Guo wrote: > On Wed, Aug 02, 2017 at 12:51:29PM -0700, Stefan Agner wrote: >> If a regulator requests a deferred probe, the power domain gets >> initialized twice. This leads to a list double add (without >> list debugging the kernel hangs due to the double add later): >> >> WARNING: CPU: 0 PID: 19 at lib/list_debug.c:31 __list_add_valid+0xbc/0xc4 >> list_add double add: new=c1229754, prev=c12383b4, next=c1229754. >> >> Initialize the power domain after we get the regulator. Also do >> not print an error in case the regulator defers probing. >> >> Cc: Fabio Estevam >> Cc: Andrey Smirnov >> Cc: linux-arm-ker...@lists.infradead.org >> Cc: linux-kernel@vger.kernel.org >> Fixes: 03aa12629fc4 ("soc: imx: Add GPCv2 power gating driver") >> Signed-off-by: Stefan Agner > > Applied, thanks. As of 4.13-rc5 this did not make it upstream yet, I guess still on its way? -- Stefan
Re: linux-next: manual merge of the akpm-current tree with the tip tree
Minchan Kimwrote: > On Sun, Aug 13, 2017 at 02:50:19PM +0200, Peter Zijlstra wrote: >> On Sun, Aug 13, 2017 at 06:06:32AM +, Nadav Amit wrote: however mm_tlb_flush_nested() is a mystery, it appears to care about anything inside the range. For now rely on it doing at least _a_ PTL lock instead of taking _the_ PTL lock. >>> >>> It does not care about “anything” inside the range, but only on situations >>> in which there is at least one (same) PT that was modified by one core and >>> then read by the other. So, yes, it will always be _the_ same PTL, and not >>> _a_ PTL - in the cases that flush is really needed. >>> >>> The issue that might require additional barriers is that >>> inc_tlb_flush_pending() and mm_tlb_flush_nested() are called when the PTL is >>> not held. IIUC, since the release-acquire might not behave as a full memory >>> barrier, this requires an explicit memory barrier. >> >> So I'm not entirely clear about this yet. >> >> How about: >> >> >> CPU0CPU1 >> >> tlb_gather_mmu() >> >> lock PTLn >> no mod >> unlock PTLn >> >> tlb_gather_mmu() >> >> lock PTLm >> mod >> include in tlb range >> unlock PTLm >> >> lock PTLn >> mod >> unlock PTLn >> >> tlb_finish_mmu() >>force = mm_tlb_flush_nested(tlb->mm); >>arch_tlb_finish_mmu(force); >> >> >> ... more ... >> >> tlb_finish_mmu() >> >> >> >> In this case you also want CPU1's mm_tlb_flush_nested() call to return >> true, right? > > No, because CPU 1 mofified pte and added it into tlb range > so regardless of nested, it will flush TLB so there is no stale > TLB problem. > >> But even with an smp_mb__after_atomic() at CPU0's tlg_bather_mmu() >> you're not guaranteed CPU1 sees the increment. The only way to do that >> is to make the PTL locks RCsc and that is a much more expensive >> proposition. >> >> >> What about: >> >> >> CPU0CPU1 >> >> tlb_gather_mmu() >> >> lock PTLn >> no mod >> unlock PTLn >> >> >> lock PTLm >> mod >> include in tlb range >> unlock PTLm >> >> tlb_gather_mmu() >> >> lock PTLn >> mod >> unlock PTLn >> >> tlb_finish_mmu() >>force = mm_tlb_flush_nested(tlb->mm); >>arch_tlb_finish_mmu(force); >> >> >> ... more ... >> >> tlb_finish_mmu() >> >> Do we want CPU1 to see it here? If so, where does it end? > > Ditto. Since CPU 1 has added range, it will flush TLB regardless > of nested condition. > >> CPU0 CPU1 >> >> tlb_gather_mmu() >> >> lock PTLn >> no mod >> unlock PTLn >> >> >> lock PTLm >> mod >> include in tlb range >> unlock PTLm >> >> tlb_finish_mmu() >>force = mm_tlb_flush_nested(tlb->mm); >> >> tlb_gather_mmu() >> >> lock PTLn >> mod >> unlock PTLn >> >>arch_tlb_finish_mmu(force); >> >> >> ... more ... >> >> tlb_finish_mmu() >> >> >> This? >> >> >> Could you clarify under what exact condition mm_tlb_flush_nested() must >> return true? > > mm_tlb_flush_nested aims for the CPU side where there is no pte update > but need TLB flush. > As I wrote > https://urldefense.proofpoint.com/v2/url?u=https-3A__marc.info_-3Fl-3Dlinux-2Dmm-26m-3D150267398226529-26w-3D2=DwIDaQ=uilaK90D4TOVoH58JNXRgQ=x9zhXCtCLvTDtvE65-BGSA=v2Z7eDi7z1H9zdngcjZvlNeBudWzA9KvcXFNpU2A77s=amaSu_gurmBHHPcl3Pxfdl0Tk_uTnmf60tMQAsNDHVU= > , > it has stable TLB problem if we don't flush TLB although there is no > pte modification. To clarify: the main problem that these patches address is when the first CPU updates the PTE, and second CPU sees the updated value and thinks: “the PTE is already what I wanted - no flush is needed”. For some reason (I would assume intentional), all the
Re: linux-next: manual merge of the akpm-current tree with the tip tree
Minchan Kim wrote: > On Sun, Aug 13, 2017 at 02:50:19PM +0200, Peter Zijlstra wrote: >> On Sun, Aug 13, 2017 at 06:06:32AM +, Nadav Amit wrote: however mm_tlb_flush_nested() is a mystery, it appears to care about anything inside the range. For now rely on it doing at least _a_ PTL lock instead of taking _the_ PTL lock. >>> >>> It does not care about “anything” inside the range, but only on situations >>> in which there is at least one (same) PT that was modified by one core and >>> then read by the other. So, yes, it will always be _the_ same PTL, and not >>> _a_ PTL - in the cases that flush is really needed. >>> >>> The issue that might require additional barriers is that >>> inc_tlb_flush_pending() and mm_tlb_flush_nested() are called when the PTL is >>> not held. IIUC, since the release-acquire might not behave as a full memory >>> barrier, this requires an explicit memory barrier. >> >> So I'm not entirely clear about this yet. >> >> How about: >> >> >> CPU0CPU1 >> >> tlb_gather_mmu() >> >> lock PTLn >> no mod >> unlock PTLn >> >> tlb_gather_mmu() >> >> lock PTLm >> mod >> include in tlb range >> unlock PTLm >> >> lock PTLn >> mod >> unlock PTLn >> >> tlb_finish_mmu() >>force = mm_tlb_flush_nested(tlb->mm); >>arch_tlb_finish_mmu(force); >> >> >> ... more ... >> >> tlb_finish_mmu() >> >> >> >> In this case you also want CPU1's mm_tlb_flush_nested() call to return >> true, right? > > No, because CPU 1 mofified pte and added it into tlb range > so regardless of nested, it will flush TLB so there is no stale > TLB problem. > >> But even with an smp_mb__after_atomic() at CPU0's tlg_bather_mmu() >> you're not guaranteed CPU1 sees the increment. The only way to do that >> is to make the PTL locks RCsc and that is a much more expensive >> proposition. >> >> >> What about: >> >> >> CPU0CPU1 >> >> tlb_gather_mmu() >> >> lock PTLn >> no mod >> unlock PTLn >> >> >> lock PTLm >> mod >> include in tlb range >> unlock PTLm >> >> tlb_gather_mmu() >> >> lock PTLn >> mod >> unlock PTLn >> >> tlb_finish_mmu() >>force = mm_tlb_flush_nested(tlb->mm); >>arch_tlb_finish_mmu(force); >> >> >> ... more ... >> >> tlb_finish_mmu() >> >> Do we want CPU1 to see it here? If so, where does it end? > > Ditto. Since CPU 1 has added range, it will flush TLB regardless > of nested condition. > >> CPU0 CPU1 >> >> tlb_gather_mmu() >> >> lock PTLn >> no mod >> unlock PTLn >> >> >> lock PTLm >> mod >> include in tlb range >> unlock PTLm >> >> tlb_finish_mmu() >>force = mm_tlb_flush_nested(tlb->mm); >> >> tlb_gather_mmu() >> >> lock PTLn >> mod >> unlock PTLn >> >>arch_tlb_finish_mmu(force); >> >> >> ... more ... >> >> tlb_finish_mmu() >> >> >> This? >> >> >> Could you clarify under what exact condition mm_tlb_flush_nested() must >> return true? > > mm_tlb_flush_nested aims for the CPU side where there is no pte update > but need TLB flush. > As I wrote > https://urldefense.proofpoint.com/v2/url?u=https-3A__marc.info_-3Fl-3Dlinux-2Dmm-26m-3D150267398226529-26w-3D2=DwIDaQ=uilaK90D4TOVoH58JNXRgQ=x9zhXCtCLvTDtvE65-BGSA=v2Z7eDi7z1H9zdngcjZvlNeBudWzA9KvcXFNpU2A77s=amaSu_gurmBHHPcl3Pxfdl0Tk_uTnmf60tMQAsNDHVU= > , > it has stable TLB problem if we don't flush TLB although there is no > pte modification. To clarify: the main problem that these patches address is when the first CPU updates the PTE, and second CPU sees the updated value and thinks: “the PTE is already what I wanted - no flush is needed”. For some reason (I would assume intentional), all the examples here first “do
Re: [PATCH] ARM: dts: BCM5301X: Add support for Linksys EA9500
Florian, On Thu, Jun 29, 2017 at 5:04 PM, Vivek Ununewrote: > Florian, > >>> I have managed to use DSA driver and was able detect both internal and >>> external switches. However, I only get packets flowing only through the >>> internal switch. I have used the ip & bridge commands to setup the vlan >>> 101 & 102 for lan and wan respectively. >>> >>> VLAN101 = lan1 lan2 lan3 lan4 lan5 lan6 lan7 lan8 eth0.101 >> >> That looks reasonable although keep in mind that the IMP/CPU interfaces >> of the switch are configured with VLAN tags (see commit [1]), so you may >> need to make sure that port 0 of the internal switch is not accidentally >> configured back to untagged since that would cause problem when >> terminating the VLAN tag on the SW side. >> >> So here are a few things that you want to check: >> >> - read the MIB counters from the "extswitch" interface and see if >> packets flow through in both directions with no errors > > lan4 is on internal switch, lan1 on external. I cannot ping router from lan1 > > Inter-| Receive| Transmit > face |bytespackets errs drop|bytespackets errs drop > br-lan: 168590172600 190542 75300 > extswitch:0 000 101012122100 > lan1: 0 000 5382 11100 > lan4: 0 000 1306680 1390900 > eth0: 3276924553900 1106135508400 > eth0.101:169338173200 190256 75000 > eth0.102: 2959522327400 587248109400 > lo: 39390 5020039390 50200 > br-wan: 2956822325400 587248109400 > Please ignore above mib counters. I noticed that I see lot of RxFCSErrors and RxSymbolErrors for extswitch. The count for both counters always remain same. root@LEDE:/# ethtool -S extswitch NIC statistics: tx_packets: 212 tx_bytes: 19179 rx_packets: 0 rx_bytes: 0 TxOctets: 14403 TxDropPkts: 0 TxBroadcastPkts: 24 TxMulticastPkts: 122 TxUnicastPkts: 0 TxCollisions: 0 TxSingleCollision: 0 TxMultipleCollision: 0 TxDeferredTransmit: 0 TxLateCollision: 0 TxExcessiveCollision: 0 TxPausePkts: 0 RxOctets: 3593 RxUndersizePkts: 0 RxPausePkts: 0 Pkts64Octets: 0 Pkts65to127Octets: 36 Pkts128to255Octets: 1 Pkts256to511Octets: 0 Pkts512to1023Octets: 0 Pkts1024to1522Octets: 0 RxOversizePkts: 0 RxJabbers: 0 RxAlignmentErrors: 0 RxFCSErrors: 37 RxGoodOctets: 0 RxDropPkts: 0 RxUnicastPkts: 0 RxMulticastPkts: 0 RxBroadcastPkts: 0 RxSAChanges: 0 RxFragments: 0 RxJumboPkts: 0 RxSymbolErrors: 37 RxDiscarded: 0 p08_TxOctets: 47537 p08_TxDropPkts: 0 p08_TxBroadcastPkts: 163 p08_TxMulticastPkts: 319 p08_TxUnicastPkts: 0 p08_TxCollisions: 0 p08_TxSingleCollision: 0 p08_TxMultipleCollision: 0 p08_TxDeferredTransmit: 0 p08_TxLateCollision: 0 p08_TxExcessiveCollision: 0 p08_TxPausePkts: 0 p08_RxOctets: 14403 p08_RxUndersizePkts: 0 p08_RxPausePkts: 0 p08_Pkts64Octets: 25 p08_Pkts65to127Octets: 102 p08_Pkts128to255Octets: 17 p08_Pkts256to511Octets: 2 p08_Pkts512to1023Octets: 0 p08_Pkts1024to1522Octets: 0 p08_RxOversizePkts: 0 p08_RxJabbers: 0 p08_RxAlignmentErrors: 0 p08_RxFCSErrors: 0 p08_RxGoodOctets: 14403 p08_RxDropPkts: 0 p08_RxUnicastPkts: 0 p08_RxMulticastPkts: 122 p08_RxBroadcastPkts: 24 p08_RxSAChanges: 40 p08_RxFragments: 0 p08_RxJumboPkts: 0 p08_RxSymbolErrors: 0 p08_RxDiscarded: 146 >> >> - check the "extswitch" VLAN configuration on both the internal switch >> side (port 0) and on the external switch side ("cpu", port 8, not visible) > > #bridge vlan show > portvlan ids > extswitch None > extswitch > lan7 101 PVID Egress Untagged > lan7 101 PVID Egress Untagged > lan4 101 PVID Egress Untagged > lan4 101 PVID Egress Untagged > lan8 101 PVID Egress Untagged > lan8 101 PVID Egress Untagged > wan 102 PVID Egress Untagged > wan 102 PVID Egress Untagged > lan1 101 PVID Egress Untagged > lan1 101 PVID Egress Untagged > lan5 101 PVID Egress Untagged > lan5 101 PVID Egress Untagged > lan2 101 PVID Egress Untagged > lan2 101 PVID Egress Untagged > lan6 101 PVID Egress Untagged > lan6 101 PVID Egress Untagged > lan3 101 PVID Egress Untagged > lan3 101 PVID Egress Untagged > br-lan None > eth0.101 101 PVID Egress Untagged > eth0.101 > br-wan None > eth0.102 102 PVID Egress Untagged > eth0.102 > >> >> - see if you can get traffic end-to-end from eth0 all the way through >> one of the external switch port. If that's the case, that
Re: [PATCH] ARM: dts: BCM5301X: Add support for Linksys EA9500
Florian, On Thu, Jun 29, 2017 at 5:04 PM, Vivek Unune wrote: > Florian, > >>> I have managed to use DSA driver and was able detect both internal and >>> external switches. However, I only get packets flowing only through the >>> internal switch. I have used the ip & bridge commands to setup the vlan >>> 101 & 102 for lan and wan respectively. >>> >>> VLAN101 = lan1 lan2 lan3 lan4 lan5 lan6 lan7 lan8 eth0.101 >> >> That looks reasonable although keep in mind that the IMP/CPU interfaces >> of the switch are configured with VLAN tags (see commit [1]), so you may >> need to make sure that port 0 of the internal switch is not accidentally >> configured back to untagged since that would cause problem when >> terminating the VLAN tag on the SW side. >> >> So here are a few things that you want to check: >> >> - read the MIB counters from the "extswitch" interface and see if >> packets flow through in both directions with no errors > > lan4 is on internal switch, lan1 on external. I cannot ping router from lan1 > > Inter-| Receive| Transmit > face |bytespackets errs drop|bytespackets errs drop > br-lan: 168590172600 190542 75300 > extswitch:0 000 101012122100 > lan1: 0 000 5382 11100 > lan4: 0 000 1306680 1390900 > eth0: 3276924553900 1106135508400 > eth0.101:169338173200 190256 75000 > eth0.102: 2959522327400 587248109400 > lo: 39390 5020039390 50200 > br-wan: 2956822325400 587248109400 > Please ignore above mib counters. I noticed that I see lot of RxFCSErrors and RxSymbolErrors for extswitch. The count for both counters always remain same. root@LEDE:/# ethtool -S extswitch NIC statistics: tx_packets: 212 tx_bytes: 19179 rx_packets: 0 rx_bytes: 0 TxOctets: 14403 TxDropPkts: 0 TxBroadcastPkts: 24 TxMulticastPkts: 122 TxUnicastPkts: 0 TxCollisions: 0 TxSingleCollision: 0 TxMultipleCollision: 0 TxDeferredTransmit: 0 TxLateCollision: 0 TxExcessiveCollision: 0 TxPausePkts: 0 RxOctets: 3593 RxUndersizePkts: 0 RxPausePkts: 0 Pkts64Octets: 0 Pkts65to127Octets: 36 Pkts128to255Octets: 1 Pkts256to511Octets: 0 Pkts512to1023Octets: 0 Pkts1024to1522Octets: 0 RxOversizePkts: 0 RxJabbers: 0 RxAlignmentErrors: 0 RxFCSErrors: 37 RxGoodOctets: 0 RxDropPkts: 0 RxUnicastPkts: 0 RxMulticastPkts: 0 RxBroadcastPkts: 0 RxSAChanges: 0 RxFragments: 0 RxJumboPkts: 0 RxSymbolErrors: 37 RxDiscarded: 0 p08_TxOctets: 47537 p08_TxDropPkts: 0 p08_TxBroadcastPkts: 163 p08_TxMulticastPkts: 319 p08_TxUnicastPkts: 0 p08_TxCollisions: 0 p08_TxSingleCollision: 0 p08_TxMultipleCollision: 0 p08_TxDeferredTransmit: 0 p08_TxLateCollision: 0 p08_TxExcessiveCollision: 0 p08_TxPausePkts: 0 p08_RxOctets: 14403 p08_RxUndersizePkts: 0 p08_RxPausePkts: 0 p08_Pkts64Octets: 25 p08_Pkts65to127Octets: 102 p08_Pkts128to255Octets: 17 p08_Pkts256to511Octets: 2 p08_Pkts512to1023Octets: 0 p08_Pkts1024to1522Octets: 0 p08_RxOversizePkts: 0 p08_RxJabbers: 0 p08_RxAlignmentErrors: 0 p08_RxFCSErrors: 0 p08_RxGoodOctets: 14403 p08_RxDropPkts: 0 p08_RxUnicastPkts: 0 p08_RxMulticastPkts: 122 p08_RxBroadcastPkts: 24 p08_RxSAChanges: 40 p08_RxFragments: 0 p08_RxJumboPkts: 0 p08_RxSymbolErrors: 0 p08_RxDiscarded: 146 >> >> - check the "extswitch" VLAN configuration on both the internal switch >> side (port 0) and on the external switch side ("cpu", port 8, not visible) > > #bridge vlan show > portvlan ids > extswitch None > extswitch > lan7 101 PVID Egress Untagged > lan7 101 PVID Egress Untagged > lan4 101 PVID Egress Untagged > lan4 101 PVID Egress Untagged > lan8 101 PVID Egress Untagged > lan8 101 PVID Egress Untagged > wan 102 PVID Egress Untagged > wan 102 PVID Egress Untagged > lan1 101 PVID Egress Untagged > lan1 101 PVID Egress Untagged > lan5 101 PVID Egress Untagged > lan5 101 PVID Egress Untagged > lan2 101 PVID Egress Untagged > lan2 101 PVID Egress Untagged > lan6 101 PVID Egress Untagged > lan6 101 PVID Egress Untagged > lan3 101 PVID Egress Untagged > lan3 101 PVID Egress Untagged > br-lan None > eth0.101 101 PVID Egress Untagged > eth0.101 > br-wan None > eth0.102 102 PVID Egress Untagged > eth0.102 > >> >> - see if you can get traffic end-to-end from eth0 all the way through >> one of the external switch port. If that's the case, that means that the >>
Re: [v3 1/2] sched/clock: interface to allow timestamps early in boot
Hi Pavel, At 08/12/2017 02:50 AM, Pavel Tatashin wrote: In Linux printk() can output timestamps next to every line. This is very useful for tracking regressions, and finding places that can be optimized. However, the timestamps are available only later in boot. On smaller machines it is insignificant amount of time, but on larger it can be many seconds or even minutes into the boot process. This patch adds an interface for platforms with unstable sched clock to show timestamps early in boot. In order to get this functionality a platform must do: - Implement u64 sched_clock_early() Clock that returns monotonic time - Call sched_clock_early_init() Tells sched clock that the early clock can be used - Call sched_clock_early_fini() Tells sched clock that the early clock is finished, and sched clock should hand over the operation to permanent clock. - Use weak sched_clock_early() interface to determine time from boot in arch specific read_boot_clock64() Signed-off-by: Pavel Tatashin--- arch/x86/kernel/time.c | 22 include/linux/sched/clock.h | 4 +++ kernel/sched/clock.c| 61 - 3 files changed, 86 insertions(+), 1 deletion(-) diff --git a/arch/x86/kernel/time.c b/arch/x86/kernel/time.c index e0754cdbad37..6ede0da7041a 100644 --- a/arch/x86/kernel/time.c +++ b/arch/x86/kernel/time.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -85,6 +86,7 @@ static __init void x86_late_time_init(void) { x86_init.timers.timer_init(); tsc_init(); + tsc_early_fini(); tsc_early_fini() is defined in patch 2, I guess you may miss it when you split your patches. } /* @@ -95,3 +97,23 @@ void __init time_init(void) { late_time_init = x86_late_time_init; } + +/* + * Called once during to boot to initialize boot time. + */ +void read_boot_clock64(struct timespec64 *ts) +{ + u64 ns_boot = sched_clock_early(); /* nsec from boot */ + struct timespec64 ts_now; + bool valid_clock; + + /* Time from epoch */ + read_persistent_clock64(_now); + valid_clock = ns_boot && timespec64_valid_strict(_now) && + (ts_now.tv_sec || ts_now.tv_nsec); + + if (!valid_clock) + *ts = (struct timespec64){0, 0}; + else + *ts = ns_to_timespec64(timespec64_to_ns(_now) - ns_boot); +} diff --git a/include/linux/sched/clock.h b/include/linux/sched/clock.h index a55600ffdf4b..f8291fa28c0c 100644 --- a/include/linux/sched/clock.h +++ b/include/linux/sched/clock.h @@ -63,6 +63,10 @@ extern void sched_clock_tick_stable(void); extern void sched_clock_idle_sleep_event(void); extern void sched_clock_idle_wakeup_event(void); +void sched_clock_early_init(void); +void sched_clock_early_fini(void); +u64 sched_clock_early(void); + /* * As outlined in clock.c, provides a fast, high resolution, nanosecond * time source that is monotonic per cpu argument and has bounded drift diff --git a/kernel/sched/clock.c b/kernel/sched/clock.c index ca0f8fc945c6..be5b60af4ca9 100644 --- a/kernel/sched/clock.c +++ b/kernel/sched/clock.c @@ -80,9 +80,24 @@ EXPORT_SYMBOL_GPL(sched_clock); __read_mostly int sched_clock_running; +/* + * We start with sched clock early static branch enabled, and global status + * disabled. Early in boot it is decided whether to enable the global + * status as well (set sched_clock_early_running to true), and later, when + * early clock is no longer needed, the static branch is disabled. + */ +static DEFINE_STATIC_KEY_TRUE(__use_sched_clock_early); +static bool __read_mostly sched_clock_early_running; + In my opinion, these two parameters are repetitive, I suggest remove one. eg. remove sched_clock_early_running like below First, static DEFINE_STATIC_KEY_FALSE(__use_sched_clock_early); void sched_clock_init(void) we can make sched_clock_init __init { - sched_clock_running = 1; + /* +* We start clock only once early clock is finished, or if early clock +* was not running. +*/ + if (!sched_clock_early_running) s/ !sched_clock_early_running/ !static_branch_unlikely(&__use_sched_clock_early)/ + sched_clock_running = 1; + } #ifdef CONFIG_HAVE_UNSTABLE_SCHED_CLOCK @@ -362,6 +377,11 @@ u64 sched_clock_cpu(int cpu) if (sched_clock_stable()) return sched_clock() + __sched_clock_offset; + if (static_branch_unlikely(&__use_sched_clock_early)) { + if (sched_clock_early_running) s/if (sched_clock_early_running)// + return sched_clock_early(); + } + if (unlikely(!sched_clock_running)) return 0ull; @@ -444,6 +464,45 @@ void sched_clock_idle_wakeup_event(void) } EXPORT_SYMBOL_GPL(sched_clock_idle_wakeup_event); +u64 __weak sched_clock_early(void) +{ + return 0; +} + +/* + * Is called when
Re: [v3 1/2] sched/clock: interface to allow timestamps early in boot
Hi Pavel, At 08/12/2017 02:50 AM, Pavel Tatashin wrote: In Linux printk() can output timestamps next to every line. This is very useful for tracking regressions, and finding places that can be optimized. However, the timestamps are available only later in boot. On smaller machines it is insignificant amount of time, but on larger it can be many seconds or even minutes into the boot process. This patch adds an interface for platforms with unstable sched clock to show timestamps early in boot. In order to get this functionality a platform must do: - Implement u64 sched_clock_early() Clock that returns monotonic time - Call sched_clock_early_init() Tells sched clock that the early clock can be used - Call sched_clock_early_fini() Tells sched clock that the early clock is finished, and sched clock should hand over the operation to permanent clock. - Use weak sched_clock_early() interface to determine time from boot in arch specific read_boot_clock64() Signed-off-by: Pavel Tatashin --- arch/x86/kernel/time.c | 22 include/linux/sched/clock.h | 4 +++ kernel/sched/clock.c| 61 - 3 files changed, 86 insertions(+), 1 deletion(-) diff --git a/arch/x86/kernel/time.c b/arch/x86/kernel/time.c index e0754cdbad37..6ede0da7041a 100644 --- a/arch/x86/kernel/time.c +++ b/arch/x86/kernel/time.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -85,6 +86,7 @@ static __init void x86_late_time_init(void) { x86_init.timers.timer_init(); tsc_init(); + tsc_early_fini(); tsc_early_fini() is defined in patch 2, I guess you may miss it when you split your patches. } /* @@ -95,3 +97,23 @@ void __init time_init(void) { late_time_init = x86_late_time_init; } + +/* + * Called once during to boot to initialize boot time. + */ +void read_boot_clock64(struct timespec64 *ts) +{ + u64 ns_boot = sched_clock_early(); /* nsec from boot */ + struct timespec64 ts_now; + bool valid_clock; + + /* Time from epoch */ + read_persistent_clock64(_now); + valid_clock = ns_boot && timespec64_valid_strict(_now) && + (ts_now.tv_sec || ts_now.tv_nsec); + + if (!valid_clock) + *ts = (struct timespec64){0, 0}; + else + *ts = ns_to_timespec64(timespec64_to_ns(_now) - ns_boot); +} diff --git a/include/linux/sched/clock.h b/include/linux/sched/clock.h index a55600ffdf4b..f8291fa28c0c 100644 --- a/include/linux/sched/clock.h +++ b/include/linux/sched/clock.h @@ -63,6 +63,10 @@ extern void sched_clock_tick_stable(void); extern void sched_clock_idle_sleep_event(void); extern void sched_clock_idle_wakeup_event(void); +void sched_clock_early_init(void); +void sched_clock_early_fini(void); +u64 sched_clock_early(void); + /* * As outlined in clock.c, provides a fast, high resolution, nanosecond * time source that is monotonic per cpu argument and has bounded drift diff --git a/kernel/sched/clock.c b/kernel/sched/clock.c index ca0f8fc945c6..be5b60af4ca9 100644 --- a/kernel/sched/clock.c +++ b/kernel/sched/clock.c @@ -80,9 +80,24 @@ EXPORT_SYMBOL_GPL(sched_clock); __read_mostly int sched_clock_running; +/* + * We start with sched clock early static branch enabled, and global status + * disabled. Early in boot it is decided whether to enable the global + * status as well (set sched_clock_early_running to true), and later, when + * early clock is no longer needed, the static branch is disabled. + */ +static DEFINE_STATIC_KEY_TRUE(__use_sched_clock_early); +static bool __read_mostly sched_clock_early_running; + In my opinion, these two parameters are repetitive, I suggest remove one. eg. remove sched_clock_early_running like below First, static DEFINE_STATIC_KEY_FALSE(__use_sched_clock_early); void sched_clock_init(void) we can make sched_clock_init __init { - sched_clock_running = 1; + /* +* We start clock only once early clock is finished, or if early clock +* was not running. +*/ + if (!sched_clock_early_running) s/ !sched_clock_early_running/ !static_branch_unlikely(&__use_sched_clock_early)/ + sched_clock_running = 1; + } #ifdef CONFIG_HAVE_UNSTABLE_SCHED_CLOCK @@ -362,6 +377,11 @@ u64 sched_clock_cpu(int cpu) if (sched_clock_stable()) return sched_clock() + __sched_clock_offset; + if (static_branch_unlikely(&__use_sched_clock_early)) { + if (sched_clock_early_running) s/if (sched_clock_early_running)// + return sched_clock_early(); + } + if (unlikely(!sched_clock_running)) return 0ull; @@ -444,6 +464,45 @@ void sched_clock_idle_wakeup_event(void) } EXPORT_SYMBOL_GPL(sched_clock_idle_wakeup_event); +u64 __weak sched_clock_early(void) +{ + return 0; +} + +/* + * Is called when sched_clock_early() is about to
Re: [PATCH ALT4 V2 1/2] audit: show fstype:pathname for entries with anonymous parents
On 2017-08-11 02:36, Richard Guy Briggs wrote: > On 2017-06-28 15:03, Paul Moore wrote: > > On Tue, Jun 27, 2017 at 5:11 PM, Richard Guy Briggswrote: > > > On 2017-05-30 17:21, Paul Moore wrote: > > >> On Tue, Apr 4, 2017 at 5:21 AM, Richard Guy Briggs > > >> wrote: > > > > ... > > > > >> > diff --git a/kernel/audit.c b/kernel/audit.c > > >> > index 25dd70a..7d83c5a 100644 > > >> > --- a/kernel/audit.c > > >> > +++ b/kernel/audit.c > > >> > @@ -66,6 +66,7 @@ > > >> > #include > > >> > #include > > >> > #include > > >> > +#include > > >> > > > >> > #include "audit.h" > > >> > > > >> > @@ -1884,6 +1885,10 @@ void audit_copy_inode(struct audit_names *name, > > >> > const struct dentry *dentry, > > >> > name->gid = inode->i_gid; > > >> > name->rdev = inode->i_rdev; > > >> > security_inode_getsecid(inode, >osid); > > >> > + if (name->dentry) { > > >> > + dput(name->dentry); > > >> > + name->dentry = NULL; > > >> > + } > > >> > > >> Out of curiosity, what terrible things happen if we take a reference > > >> to a non-NULL dentry passed to audit_copy_inode() and store it in > > >> name->dentry? Does performance tank? > > > > > > Interesting idea. Right now it is optimized to only take a reference to > > > the dentry's parent dentry in the case we're handed an anonymous entry. > > > Most of the time it will never be used even though we invest in the > > > overhead of taking a reference count. Besides, __audit_inode_child() > > > hands in a NULL for the dentry parameter to audit_copy_inode(). > > > > [NOTE: audit_copy_inode() hands a NULL dentry only in the anonymous parent > > case] > > > > I believe I was just thinking of less conditional handling, especially > > when reference counts are concerned. I'm just trying to limit future > > headaches, but I suspect the perf cost would be problematic, and as > > you point out, there is no *need* for the majority of cases. > > > > Looking at this again today, why would we want to clear name->dentry > > in audit_copy_inode() if it is already set? Does that ever happen? > > I'm not sure it does ... > > Ok, I just tried re-writing that part to dput that dentry only for the > child of an anonymous parent rather than whenever audit_copy_inode() was > called and I ended up with a: > > BUG: Dentry 8800338f1dc0{i=369a,n=stderr} still in use (1) > [unmount of tmpfs tmpfs] > WARNING: CPU: 0 PID: 387 at fs/dcache.c:1445 umount_check+0x99/0xc0 > > This was after rebasing on a recent audit-next based on 4.11 rather than > the previous based on a 4.8 audit-next. Something else changed in the > kernel or kernel config between these two points. I was getting some > "INFO: suspicious RCU usage." warnings in idmap for NFS on the earlier > kernel and that is no longer happenning, and I'm no longer getting any > tracefs audit PATH entries in the more recent kernel which suggests that > whatever was causing the anonymous parent PATH records from tracefs has > been changed/fixed/configured-out. This tmpfs Dentry issue happens on > both 4.8 and 4.11 kernels. Minor correction here: I'm still getting tracefs PATH records on 4.11, so whatever happenned in that test appears to be a test procedure error. > So the tmpfs is being unmounted within the time of a task that has taken > an anonymous parent audit_name and it appears that the dput in > audit_copy_inode() called from elsewhere had reset it to avoid > needlessly extending the life of this dentry. > > I see two obvious ways to solve this: > - Return to freeing this dentry from audit_copy_inode() > - Add tmpfs to the list of filesystems to not create PATH records > > I'm really not crazy aobut this second one unless I know why tmpfs is > generating calls with anonymous parents. > > For reference, the rest of the call trace is: > dump_stack+0x85/0xc9 > __warn+0xd1/0xf0 > ? d_genocide_kill+0x40/0x40 > warn_slowpath_null+0x1d/0x20 > umount_check+0x99/0xc0 > d_walk+0x10b/0x580 > ? do_one_tree+0x26/0x40 > do_one_tree+0x26/0x40 > shrink_dcache_for_umount+0x5d/0xd0 > generic_shutdown_super+0x1f/0xf0 > kill_litter_super+0x29/0x40 > deactivate_locked_super+0x43/0x70 > deactivate_super+0x88/0xa0 > cleanup_mnt+0x8e/0xe0 > __cleanup_mnt+0x12/0x20 > task_work_run+0x83/0xc0 > do_exit+0x45c/0xfe0 > ? syscall_trace_enter+0x2e4/0x400 > do_group_exit+0x68/0xe0 > SyS_exit_group+0x14/0x20 > do_syscall_64+0x82/0x270 > entry_SYSCALL64_slow_path+0x25/0x25 > > > > I'm assuming you are hinting at also using that dentry to compare the > > > audit_names entry, which I think it a bad idea since there could be > > > multiple paths to access a dentry. I did orignially have another patch > > > that would have tried to use that as well, which didn't seem to hurt, > > > but I didn't
Re: [PATCH ALT4 V2 1/2] audit: show fstype:pathname for entries with anonymous parents
On 2017-08-11 02:36, Richard Guy Briggs wrote: > On 2017-06-28 15:03, Paul Moore wrote: > > On Tue, Jun 27, 2017 at 5:11 PM, Richard Guy Briggs wrote: > > > On 2017-05-30 17:21, Paul Moore wrote: > > >> On Tue, Apr 4, 2017 at 5:21 AM, Richard Guy Briggs > > >> wrote: > > > > ... > > > > >> > diff --git a/kernel/audit.c b/kernel/audit.c > > >> > index 25dd70a..7d83c5a 100644 > > >> > --- a/kernel/audit.c > > >> > +++ b/kernel/audit.c > > >> > @@ -66,6 +66,7 @@ > > >> > #include > > >> > #include > > >> > #include > > >> > +#include > > >> > > > >> > #include "audit.h" > > >> > > > >> > @@ -1884,6 +1885,10 @@ void audit_copy_inode(struct audit_names *name, > > >> > const struct dentry *dentry, > > >> > name->gid = inode->i_gid; > > >> > name->rdev = inode->i_rdev; > > >> > security_inode_getsecid(inode, >osid); > > >> > + if (name->dentry) { > > >> > + dput(name->dentry); > > >> > + name->dentry = NULL; > > >> > + } > > >> > > >> Out of curiosity, what terrible things happen if we take a reference > > >> to a non-NULL dentry passed to audit_copy_inode() and store it in > > >> name->dentry? Does performance tank? > > > > > > Interesting idea. Right now it is optimized to only take a reference to > > > the dentry's parent dentry in the case we're handed an anonymous entry. > > > Most of the time it will never be used even though we invest in the > > > overhead of taking a reference count. Besides, __audit_inode_child() > > > hands in a NULL for the dentry parameter to audit_copy_inode(). > > > > [NOTE: audit_copy_inode() hands a NULL dentry only in the anonymous parent > > case] > > > > I believe I was just thinking of less conditional handling, especially > > when reference counts are concerned. I'm just trying to limit future > > headaches, but I suspect the perf cost would be problematic, and as > > you point out, there is no *need* for the majority of cases. > > > > Looking at this again today, why would we want to clear name->dentry > > in audit_copy_inode() if it is already set? Does that ever happen? > > I'm not sure it does ... > > Ok, I just tried re-writing that part to dput that dentry only for the > child of an anonymous parent rather than whenever audit_copy_inode() was > called and I ended up with a: > > BUG: Dentry 8800338f1dc0{i=369a,n=stderr} still in use (1) > [unmount of tmpfs tmpfs] > WARNING: CPU: 0 PID: 387 at fs/dcache.c:1445 umount_check+0x99/0xc0 > > This was after rebasing on a recent audit-next based on 4.11 rather than > the previous based on a 4.8 audit-next. Something else changed in the > kernel or kernel config between these two points. I was getting some > "INFO: suspicious RCU usage." warnings in idmap for NFS on the earlier > kernel and that is no longer happenning, and I'm no longer getting any > tracefs audit PATH entries in the more recent kernel which suggests that > whatever was causing the anonymous parent PATH records from tracefs has > been changed/fixed/configured-out. This tmpfs Dentry issue happens on > both 4.8 and 4.11 kernels. Minor correction here: I'm still getting tracefs PATH records on 4.11, so whatever happenned in that test appears to be a test procedure error. > So the tmpfs is being unmounted within the time of a task that has taken > an anonymous parent audit_name and it appears that the dput in > audit_copy_inode() called from elsewhere had reset it to avoid > needlessly extending the life of this dentry. > > I see two obvious ways to solve this: > - Return to freeing this dentry from audit_copy_inode() > - Add tmpfs to the list of filesystems to not create PATH records > > I'm really not crazy aobut this second one unless I know why tmpfs is > generating calls with anonymous parents. > > For reference, the rest of the call trace is: > dump_stack+0x85/0xc9 > __warn+0xd1/0xf0 > ? d_genocide_kill+0x40/0x40 > warn_slowpath_null+0x1d/0x20 > umount_check+0x99/0xc0 > d_walk+0x10b/0x580 > ? do_one_tree+0x26/0x40 > do_one_tree+0x26/0x40 > shrink_dcache_for_umount+0x5d/0xd0 > generic_shutdown_super+0x1f/0xf0 > kill_litter_super+0x29/0x40 > deactivate_locked_super+0x43/0x70 > deactivate_super+0x88/0xa0 > cleanup_mnt+0x8e/0xe0 > __cleanup_mnt+0x12/0x20 > task_work_run+0x83/0xc0 > do_exit+0x45c/0xfe0 > ? syscall_trace_enter+0x2e4/0x400 > do_group_exit+0x68/0xe0 > SyS_exit_group+0x14/0x20 > do_syscall_64+0x82/0x270 > entry_SYSCALL64_slow_path+0x25/0x25 > > > > I'm assuming you are hinting at also using that dentry to compare the > > > audit_names entry, which I think it a bad idea since there could be > > > multiple paths to access a dentry. I did orignially have another patch > > > that would have tried to use that as well, which didn't seem to hurt, > > > but I didn't think was worth upstreaming. > >
Re: [PATCH net-next v2 2/3] net/ncsi: Configure VLAN tag filter
On Mon, 2017-08-14 at 11:39 +0930, Joel Stanley wrote: > On Mon, Aug 14, 2017 at 10:59 AM, Samuel Mendoza-Jonas >wrote: > > Make use of the ndo_vlan_rx_{add,kill}_vid callbacks to have the NCSI > > stack process new VLAN tags and configure the channel VLAN filter > > appropriately. > > Several VLAN tags can be set and a "Set VLAN Filter" packet must be sent > > for each one, meaning the ncsi_dev_state_config_svf state must be > > repeated. An internal list of VLAN tags is maintained, and compared > > against the current channel's ncsi_channel_filter in order to keep track > > within the state. VLAN filters are removed in a similar manner, with the > > introduction of the ncsi_dev_state_config_clear_vids state. The maximum > > number of VLAN tag filters is determined by the "Get Capabilities" > > response from the channel. > > > > Signed-off-by: Samuel Mendoza-Jonas > > I've given this some testing, but there are a few things I saw below > that we should sort out. > > > --- a/net/ncsi/ncsi-manage.c > > +++ b/net/ncsi/ncsi-manage.c > > @@ -38,6 +38,22 @@ static inline int ncsi_filter_size(int table) > > return sizes[table]; > > } > > > > +u32 *ncsi_get_filter(struct ncsi_channel *nc, int table, int index) > > +{ > > + struct ncsi_channel_filter *ncf; > > + int size; > > + > > + ncf = nc->filters[table]; > > + if (!ncf) > > + return NULL; > > + > > + size = ncsi_filter_size(table); > > + if (size < 0) > > + return NULL; > > + > > + return ncf->data + size * index; > > +} > > + > > int ncsi_find_filter(struct ncsi_channel *nc, int table, void *data) > > { > > struct ncsi_channel_filter *ncf; > > @@ -58,7 +74,7 @@ int ncsi_find_filter(struct ncsi_channel *nc, int table, > > void *data) > > index = -1; > > while ((index = find_next_bit(bitmap, ncf->total, index + 1)) > >< ncf->total) { > > - if (!memcmp(ncf->data + size * index, data, size)) { > > + if (!data || !memcmp(ncf->data + size * index, data, size)) > > { > > Not clear why this check is required? Right, this could use a comment. This is a small workaround to having a way to finding an arbitrary active filter, below in clear_one_vid(). We pass NULL as a way of saying "return any enabled filter". > > > spin_unlock_irqrestore(>lock, flags); > > return index; > > } > > @@ -639,6 +655,83 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv > > *ndp) > > nd->state = ncsi_dev_state_functional; > > } > > > > +/* Check the VLAN filter bitmap for a set filter, and construct a > > + * "Set VLAN Filter - Disable" packet if found. > > + */ > > +static int clear_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel > > *nc, > > +struct ncsi_cmd_arg *nca) > > +{ > > + int index; > > + u16 vid; > > + > > + index = ncsi_find_filter(nc, NCSI_FILTER_VLAN, NULL); > > + if (index < 0) { > > + /* Filter table empty */ > > + return -1; > > + } > > + > > + vid = *(u16 *)ncsi_get_filter(nc, NCSI_FILTER_VLAN, index); > > You just added this function that returns a pointer to a u32. It's > strange to see the only call site then throw half of it away. The problem here is that the single ncsi_channel_filter struct is used to represent several different filters, and the filter data is stored in a u32 data[] type. We cast to u16 in clear_one_vid because we know it's a VLAN tag (NCSI_FILTER_VLAN), although we probably should call ncsi_filter_size() to be sure. > > Also, ncsi_get_filter can return NULL. That is indeed a problem though! > > > + netdev_printk(KERN_DEBUG, ndp->ndev.dev, > > + "ncsi: removed vlan tag %u at index %d\n", > > + vid, index + 1); > > + ncsi_remove_filter(nc, NCSI_FILTER_VLAN, index); > > + > > + nca->type = NCSI_PKT_CMD_SVF; > > + nca->words[1] = vid; > > + /* HW filter index starts at 1 */ > > + nca->bytes[6] = index + 1; > > + nca->bytes[7] = 0x00; > > + return 0; > > +} > > @@ -751,6 +876,26 @@ static void ncsi_configure_channel(struct > > ncsi_dev_priv *ndp) > > break; > > case ncsi_dev_state_config_done: > > spin_lock_irqsave(>lock, flags); > > + if (nc->reconfigure_needed) { > > + /* This channel's configuration has been updated > > +* part-way during the config state - start the > > +* channel configuration over > > +*/ > > + nc->reconfigure_needed = false; > > + nc->state = NCSI_CHANNEL_INVISIBLE; > > + spin_unlock_irqrestore(>lock, flags); > > + > > +
Re: [PATCH net-next v2 2/3] net/ncsi: Configure VLAN tag filter
On Mon, 2017-08-14 at 11:39 +0930, Joel Stanley wrote: > On Mon, Aug 14, 2017 at 10:59 AM, Samuel Mendoza-Jonas > wrote: > > Make use of the ndo_vlan_rx_{add,kill}_vid callbacks to have the NCSI > > stack process new VLAN tags and configure the channel VLAN filter > > appropriately. > > Several VLAN tags can be set and a "Set VLAN Filter" packet must be sent > > for each one, meaning the ncsi_dev_state_config_svf state must be > > repeated. An internal list of VLAN tags is maintained, and compared > > against the current channel's ncsi_channel_filter in order to keep track > > within the state. VLAN filters are removed in a similar manner, with the > > introduction of the ncsi_dev_state_config_clear_vids state. The maximum > > number of VLAN tag filters is determined by the "Get Capabilities" > > response from the channel. > > > > Signed-off-by: Samuel Mendoza-Jonas > > I've given this some testing, but there are a few things I saw below > that we should sort out. > > > --- a/net/ncsi/ncsi-manage.c > > +++ b/net/ncsi/ncsi-manage.c > > @@ -38,6 +38,22 @@ static inline int ncsi_filter_size(int table) > > return sizes[table]; > > } > > > > +u32 *ncsi_get_filter(struct ncsi_channel *nc, int table, int index) > > +{ > > + struct ncsi_channel_filter *ncf; > > + int size; > > + > > + ncf = nc->filters[table]; > > + if (!ncf) > > + return NULL; > > + > > + size = ncsi_filter_size(table); > > + if (size < 0) > > + return NULL; > > + > > + return ncf->data + size * index; > > +} > > + > > int ncsi_find_filter(struct ncsi_channel *nc, int table, void *data) > > { > > struct ncsi_channel_filter *ncf; > > @@ -58,7 +74,7 @@ int ncsi_find_filter(struct ncsi_channel *nc, int table, > > void *data) > > index = -1; > > while ((index = find_next_bit(bitmap, ncf->total, index + 1)) > >< ncf->total) { > > - if (!memcmp(ncf->data + size * index, data, size)) { > > + if (!data || !memcmp(ncf->data + size * index, data, size)) > > { > > Not clear why this check is required? Right, this could use a comment. This is a small workaround to having a way to finding an arbitrary active filter, below in clear_one_vid(). We pass NULL as a way of saying "return any enabled filter". > > > spin_unlock_irqrestore(>lock, flags); > > return index; > > } > > @@ -639,6 +655,83 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv > > *ndp) > > nd->state = ncsi_dev_state_functional; > > } > > > > +/* Check the VLAN filter bitmap for a set filter, and construct a > > + * "Set VLAN Filter - Disable" packet if found. > > + */ > > +static int clear_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel > > *nc, > > +struct ncsi_cmd_arg *nca) > > +{ > > + int index; > > + u16 vid; > > + > > + index = ncsi_find_filter(nc, NCSI_FILTER_VLAN, NULL); > > + if (index < 0) { > > + /* Filter table empty */ > > + return -1; > > + } > > + > > + vid = *(u16 *)ncsi_get_filter(nc, NCSI_FILTER_VLAN, index); > > You just added this function that returns a pointer to a u32. It's > strange to see the only call site then throw half of it away. The problem here is that the single ncsi_channel_filter struct is used to represent several different filters, and the filter data is stored in a u32 data[] type. We cast to u16 in clear_one_vid because we know it's a VLAN tag (NCSI_FILTER_VLAN), although we probably should call ncsi_filter_size() to be sure. > > Also, ncsi_get_filter can return NULL. That is indeed a problem though! > > > + netdev_printk(KERN_DEBUG, ndp->ndev.dev, > > + "ncsi: removed vlan tag %u at index %d\n", > > + vid, index + 1); > > + ncsi_remove_filter(nc, NCSI_FILTER_VLAN, index); > > + > > + nca->type = NCSI_PKT_CMD_SVF; > > + nca->words[1] = vid; > > + /* HW filter index starts at 1 */ > > + nca->bytes[6] = index + 1; > > + nca->bytes[7] = 0x00; > > + return 0; > > +} > > @@ -751,6 +876,26 @@ static void ncsi_configure_channel(struct > > ncsi_dev_priv *ndp) > > break; > > case ncsi_dev_state_config_done: > > spin_lock_irqsave(>lock, flags); > > + if (nc->reconfigure_needed) { > > + /* This channel's configuration has been updated > > +* part-way during the config state - start the > > +* channel configuration over > > +*/ > > + nc->reconfigure_needed = false; > > + nc->state = NCSI_CHANNEL_INVISIBLE; > > + spin_unlock_irqrestore(>lock, flags); > > + > > + spin_lock_irqsave(>lock, flags); > > +
Re: [PATCH 1/3] dt-bindings: display: Add Document for Rockchip Soc LVDS
在 2017/8/11 23:38, Philipp Zabel 写道: On Wed, 2017-08-09 at 18:00 +0800, Sandy Huang wrote: This patch add Document for Rockchip Soc RK3288 LVDS, This based on the patches from Mark yao and Heiko Stuebner. Signed-off-by: Sandy HuangSigned-off-by: Mark yao Signed-off-by: Heiko Stuebner --- .../bindings/display/rockchip/rockchip-lvds.txt| 104 + 1 file changed, 104 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt b/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt new file mode 100644 index 000..bf934ba --- /dev/null +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt @@ -0,0 +1,104 @@ +Rockchip RK3288 LVDS interface + + +Required properties: +- compatible: matching the soc type, one of + - "rockchip,rk3288-lvds"; + +- reg: physical base address of the controller and length + of memory mapped region. +- clocks: must include clock specifiers corresponding to entries in the + clock-names property. +- clock-names: must contain "pclk_lvds" + +- avdd1v0-supply: regulator phandle for 1.0V analog power +- avdd1v8-supply: regulator phandle for 1.8V analog power +- avdd3v3-supply: regulator phandle for 3.3V analog power + +- rockchip,grf: phandle to the general register files syscon + +Optional properties +- pinctrl-names: must contain a "lcdc" entry. +- pinctrl-0: pin control group to be used for this controller. + +Required nodes: + +The lvds has two video ports as described by + Documentation/devicetree/bindings/media/video-interfaces.txt. +Their connections are modeled using the OF graph bindings specified in + Documentation/devicetree/bindings/graph.txt. + +- video port 0 for the VOP inputs +- video port 1 for either a panel or subsequent encoder + +the lvds panel described by + Documentation/devicetree/bindings/display/panel/simple-panel.txt + +- rockchip,data-mapping: should be "vesa" or "jeida", + This describes how the color bits are laid out in the + serialized LVDS signal. +- rockchip,data-width : should be <18> or <24>; This can already be described by the panel itself, via the bus_format property in the panel_desc for panel-simple, or via the existing panel device tree property "data-mapping" in panel-lvds, which can be set to "jeida-18", "jeida-24", or "vesa-24". The LVDS driver can then read the panel bus information from the panel's connector->display_info.bus_formats. Because the lvds maybe connected to panel or some cover chip just like rgb2cvbs etc. , in this case, we no need to enable panel, so we have to get this proterty at here. So if these properties are necessary at all, they at least should be optional (overrides). ok, i will set data-mapping to option. regards Philipp ___ Linux-rockchip mailing list linux-rockc...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip
Re: [PATCH 1/3] dt-bindings: display: Add Document for Rockchip Soc LVDS
在 2017/8/11 23:38, Philipp Zabel 写道: On Wed, 2017-08-09 at 18:00 +0800, Sandy Huang wrote: This patch add Document for Rockchip Soc RK3288 LVDS, This based on the patches from Mark yao and Heiko Stuebner. Signed-off-by: Sandy Huang Signed-off-by: Mark yao Signed-off-by: Heiko Stuebner --- .../bindings/display/rockchip/rockchip-lvds.txt| 104 + 1 file changed, 104 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt b/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt new file mode 100644 index 000..bf934ba --- /dev/null +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-lvds.txt @@ -0,0 +1,104 @@ +Rockchip RK3288 LVDS interface + + +Required properties: +- compatible: matching the soc type, one of + - "rockchip,rk3288-lvds"; + +- reg: physical base address of the controller and length + of memory mapped region. +- clocks: must include clock specifiers corresponding to entries in the + clock-names property. +- clock-names: must contain "pclk_lvds" + +- avdd1v0-supply: regulator phandle for 1.0V analog power +- avdd1v8-supply: regulator phandle for 1.8V analog power +- avdd3v3-supply: regulator phandle for 3.3V analog power + +- rockchip,grf: phandle to the general register files syscon + +Optional properties +- pinctrl-names: must contain a "lcdc" entry. +- pinctrl-0: pin control group to be used for this controller. + +Required nodes: + +The lvds has two video ports as described by + Documentation/devicetree/bindings/media/video-interfaces.txt. +Their connections are modeled using the OF graph bindings specified in + Documentation/devicetree/bindings/graph.txt. + +- video port 0 for the VOP inputs +- video port 1 for either a panel or subsequent encoder + +the lvds panel described by + Documentation/devicetree/bindings/display/panel/simple-panel.txt + +- rockchip,data-mapping: should be "vesa" or "jeida", + This describes how the color bits are laid out in the + serialized LVDS signal. +- rockchip,data-width : should be <18> or <24>; This can already be described by the panel itself, via the bus_format property in the panel_desc for panel-simple, or via the existing panel device tree property "data-mapping" in panel-lvds, which can be set to "jeida-18", "jeida-24", or "vesa-24". The LVDS driver can then read the panel bus information from the panel's connector->display_info.bus_formats. Because the lvds maybe connected to panel or some cover chip just like rgb2cvbs etc. , in this case, we no need to enable panel, so we have to get this proterty at here. So if these properties are necessary at all, they at least should be optional (overrides). ok, i will set data-mapping to option. regards Philipp ___ Linux-rockchip mailing list linux-rockc...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip
Re: [PATCH 3/3] drm/rockchip: Add support for Rockchip Soc LVDS
在 2017/8/11 22:44, Sean Paul 写道: On Fri, Aug 11, 2017 at 10:15:16AM +0800, Sandy Huang wrote: 在 2017/8/11 2:05, Sean Paul 写道: On Thu, Aug 10, 2017 at 05:35:52PM +0800, Sandy Huang wrote: Hi Sean Paul, Thanks for your review. 在 2017/8/10 3:58, Sean Paul 写道: On Wed, Aug 09, 2017 at 06:00:59PM +0800, Sandy Huang wrote: This adds support for Rockchip soc lvds found on rk3288 Based on the patches from Mark yao and Heiko Stuebner Signed-off-by: Sandy HuangSigned-off-by: Mark yao Signed-off-by: Heiko Stuebner --- drivers/gpu/drm/rockchip/Kconfig | 9 + drivers/gpu/drm/rockchip/Makefile| 1 + drivers/gpu/drm/rockchip/rockchip_lvds.c | 734 +++ drivers/gpu/drm/rockchip/rockchip_lvds.h | 112 + 4 files changed, 856 insertions(+) create mode 100644 drivers/gpu/drm/rockchip/rockchip_lvds.c create mode 100644 drivers/gpu/drm/rockchip/rockchip_lvds.h diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c new file mode 100644 index 000..a4ad3f0 --- /dev/null +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c + lvds->drm_dev = drm_dev; + port = of_graph_get_port_by_id(dev->of_node, 1); + if (!port) { + dev_err(dev, "can't found port point, please init lvds panel port!\n"); + return -EINVAL; + } + + for_each_child_of_node(port, endpoint) { + remote = of_graph_get_remote_port_parent(endpoint); + if (!remote) { + dev_err(dev, "can't found panel node, please init!\n"); + ret = -EINVAL; + goto err_put_port; + } + if (!of_device_is_available(remote)) { + of_node_put(remote); + remote = NULL; + continue; + } + break; + } + if (!remote) { + dev_err(dev, "can't found remote node, please init!\n"); + ret = -EINVAL; + goto err_put_port; + } + + lvds->panel = of_drm_find_panel(remote); + if (!lvds->panel) + lvds->bridge = of_drm_find_bridge(remote); drm_of_find_panel_or_bridge() because the lvds ports maybe connect to lvds-panel or connect to rk1000(which is convert RGB to CVBS output), so i have to get the remote port parent and check the status, and final get the active remote point. lvds_panel: lvds-panel { status = "disabled"; ports { panel_in_lvds: endpoint { remote-endpoint = <_out_panel>; }; }; }; rk1000: rk1000@0xff00 { status = "okay"; ports { rk1000_in_lvds: endpoint { remote-endpoint = <_out_panel>; }; }; }; { status = "okay"; ports { lvds_out: port@1 { reg = <1>; lvds_out_panel: endpoint@0 { reg = <0>; remote-endpoint = <_in_lvds>; }; lvds_out_rk1000: endpoint@1 { reg = <1>; remote-endpoint = <_in_lvds>; }; }; }; }; Hi Sandy, Forgive me, this is probably a stupid question. I don't see how this usecase is unique from the other users of drm_of_find_panel_or_bridge. Couldn't you change your devicetree bindings to conform to something drm_of_find_panel_or_bridge() can work with? Thank you, Hi sean, Maybe i can use the following method to use drm_of_find_panel_or_bridge() and no need to change my DT, but there is another question: The LVDS output format(rockchip,output、rockchip,data-mapping etc.) depend on different panel, so it should be put under remote panel point. If use drm_of_find_panel_or_bridge(), this just return panel or bridge, so i have to back to get remote panel point and get the output format. This should be easy since you can grab dev->of_node from panel or bridge once it's found. ok, thanks. ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, >panel, >bridge); if (ret) ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 1, >panel, >bridge); Would be easier to read in a for loop. Sean ok, thanks. if (ret) { DRM_DEV_ERROR(dev, "failed to find panel and bridge node\n"); ret = -EPROBE_DEFER; goto err_put_remote; }
Re: [PATCH 3/3] drm/rockchip: Add support for Rockchip Soc LVDS
在 2017/8/11 22:44, Sean Paul 写道: On Fri, Aug 11, 2017 at 10:15:16AM +0800, Sandy Huang wrote: 在 2017/8/11 2:05, Sean Paul 写道: On Thu, Aug 10, 2017 at 05:35:52PM +0800, Sandy Huang wrote: Hi Sean Paul, Thanks for your review. 在 2017/8/10 3:58, Sean Paul 写道: On Wed, Aug 09, 2017 at 06:00:59PM +0800, Sandy Huang wrote: This adds support for Rockchip soc lvds found on rk3288 Based on the patches from Mark yao and Heiko Stuebner Signed-off-by: Sandy Huang Signed-off-by: Mark yao Signed-off-by: Heiko Stuebner --- drivers/gpu/drm/rockchip/Kconfig | 9 + drivers/gpu/drm/rockchip/Makefile| 1 + drivers/gpu/drm/rockchip/rockchip_lvds.c | 734 +++ drivers/gpu/drm/rockchip/rockchip_lvds.h | 112 + 4 files changed, 856 insertions(+) create mode 100644 drivers/gpu/drm/rockchip/rockchip_lvds.c create mode 100644 drivers/gpu/drm/rockchip/rockchip_lvds.h diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c new file mode 100644 index 000..a4ad3f0 --- /dev/null +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c + lvds->drm_dev = drm_dev; + port = of_graph_get_port_by_id(dev->of_node, 1); + if (!port) { + dev_err(dev, "can't found port point, please init lvds panel port!\n"); + return -EINVAL; + } + + for_each_child_of_node(port, endpoint) { + remote = of_graph_get_remote_port_parent(endpoint); + if (!remote) { + dev_err(dev, "can't found panel node, please init!\n"); + ret = -EINVAL; + goto err_put_port; + } + if (!of_device_is_available(remote)) { + of_node_put(remote); + remote = NULL; + continue; + } + break; + } + if (!remote) { + dev_err(dev, "can't found remote node, please init!\n"); + ret = -EINVAL; + goto err_put_port; + } + + lvds->panel = of_drm_find_panel(remote); + if (!lvds->panel) + lvds->bridge = of_drm_find_bridge(remote); drm_of_find_panel_or_bridge() because the lvds ports maybe connect to lvds-panel or connect to rk1000(which is convert RGB to CVBS output), so i have to get the remote port parent and check the status, and final get the active remote point. lvds_panel: lvds-panel { status = "disabled"; ports { panel_in_lvds: endpoint { remote-endpoint = <_out_panel>; }; }; }; rk1000: rk1000@0xff00 { status = "okay"; ports { rk1000_in_lvds: endpoint { remote-endpoint = <_out_panel>; }; }; }; { status = "okay"; ports { lvds_out: port@1 { reg = <1>; lvds_out_panel: endpoint@0 { reg = <0>; remote-endpoint = <_in_lvds>; }; lvds_out_rk1000: endpoint@1 { reg = <1>; remote-endpoint = <_in_lvds>; }; }; }; }; Hi Sandy, Forgive me, this is probably a stupid question. I don't see how this usecase is unique from the other users of drm_of_find_panel_or_bridge. Couldn't you change your devicetree bindings to conform to something drm_of_find_panel_or_bridge() can work with? Thank you, Hi sean, Maybe i can use the following method to use drm_of_find_panel_or_bridge() and no need to change my DT, but there is another question: The LVDS output format(rockchip,output、rockchip,data-mapping etc.) depend on different panel, so it should be put under remote panel point. If use drm_of_find_panel_or_bridge(), this just return panel or bridge, so i have to back to get remote panel point and get the output format. This should be easy since you can grab dev->of_node from panel or bridge once it's found. ok, thanks. ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 0, >panel, >bridge); if (ret) ret = drm_of_find_panel_or_bridge(dev->of_node, 1, 1, >panel, >bridge); Would be easier to read in a for loop. Sean ok, thanks. if (ret) { DRM_DEV_ERROR(dev, "failed to find panel and bridge node\n"); ret = -EPROBE_DEFER; goto err_put_remote; }
Re: [PATCH v2 1/1] i2c: aspeed: add proper support fo 24xx clock params
On Sat, Aug 12, 2017 at 4:30 AM, Wolfram Sangwrote: > >> That being said, I could implement this as a custom clock subclass, which >> would probably be cleaner that what I have done. > > Shall I wait for that one or do you want this patch to be included? > I don't mind, your call here... > Let's go ahead with this patch. I do not have too much experience with the clock stuff, so I imagine that will probably take some back and forth. Thanks!
Re: [PATCH v2 1/1] i2c: aspeed: add proper support fo 24xx clock params
On Sat, Aug 12, 2017 at 4:30 AM, Wolfram Sang wrote: > >> That being said, I could implement this as a custom clock subclass, which >> would probably be cleaner that what I have done. > > Shall I wait for that one or do you want this patch to be included? > I don't mind, your call here... > Let's go ahead with this patch. I do not have too much experience with the clock stuff, so I imagine that will probably take some back and forth. Thanks!
Re: [PATCH v1] i2c: aspeed: fixed potential null pointer dereference
On Sat, Aug 12, 2017 at 9:02 PM, Wolfram Sangwrote: > On Fri, Jul 28, 2017 at 06:00:12PM -0700, Brendan Higgins wrote: >> Before I skipped null checks when the master is in the STOP state; this >> fixes that. >> >> Signed-off-by: Brendan Higgins > > Is there a suitable "Fixes:" tag for this? > The driver was introduced in 4.13, so we could add: Fixes: f327c686d3ba ("i2c: aspeed: added driver for Aspeed I2C") Cheers, Joel
Re: [PATCH v1] i2c: aspeed: fixed potential null pointer dereference
On Sat, Aug 12, 2017 at 9:02 PM, Wolfram Sang wrote: > On Fri, Jul 28, 2017 at 06:00:12PM -0700, Brendan Higgins wrote: >> Before I skipped null checks when the master is in the STOP state; this >> fixes that. >> >> Signed-off-by: Brendan Higgins > > Is there a suitable "Fixes:" tag for this? > The driver was introduced in 4.13, so we could add: Fixes: f327c686d3ba ("i2c: aspeed: added driver for Aspeed I2C") Cheers, Joel
Re: linux-next: manual merge of the akpm-current tree with the tip tree
On Sun, Aug 13, 2017 at 02:50:19PM +0200, Peter Zijlstra wrote: > On Sun, Aug 13, 2017 at 06:06:32AM +, Nadav Amit wrote: > > > however mm_tlb_flush_nested() is a mystery, it appears to care about > > > anything inside the range. For now rely on it doing at least _a_ PTL > > > lock instead of taking _the_ PTL lock. > > > > It does not care about “anything” inside the range, but only on situations > > in which there is at least one (same) PT that was modified by one core and > > then read by the other. So, yes, it will always be _the_ same PTL, and not > > _a_ PTL - in the cases that flush is really needed. > > > > The issue that might require additional barriers is that > > inc_tlb_flush_pending() and mm_tlb_flush_nested() are called when the PTL is > > not held. IIUC, since the release-acquire might not behave as a full memory > > barrier, this requires an explicit memory barrier. > > So I'm not entirely clear about this yet. > > How about: > > > CPU0CPU1 > > tlb_gather_mmu() > > lock PTLn > no mod > unlock PTLn > > tlb_gather_mmu() > > lock PTLm > mod > include in tlb range > unlock PTLm > > lock PTLn > mod > unlock PTLn > > tlb_finish_mmu() > force = mm_tlb_flush_nested(tlb->mm); > arch_tlb_finish_mmu(force); > > > ... more ... > > tlb_finish_mmu() > > > > In this case you also want CPU1's mm_tlb_flush_nested() call to return > true, right? No, because CPU 1 mofified pte and added it into tlb range so regardless of nested, it will flush TLB so there is no stale TLB problem. > > But even with an smp_mb__after_atomic() at CPU0's tlg_bather_mmu() > you're not guaranteed CPU1 sees the increment. The only way to do that > is to make the PTL locks RCsc and that is a much more expensive > proposition. > > > What about: > > > CPU0CPU1 > > tlb_gather_mmu() > > lock PTLn > no mod > unlock PTLn > > > lock PTLm > mod > include in tlb range > unlock PTLm > > tlb_gather_mmu() > > lock PTLn > mod > unlock PTLn > > tlb_finish_mmu() > force = mm_tlb_flush_nested(tlb->mm); > arch_tlb_finish_mmu(force); > > > ... more ... > > tlb_finish_mmu() > > Do we want CPU1 to see it here? If so, where does it end? Ditto. Since CPU 1 has added range, it will flush TLB regardless of nested condition. > > CPU0CPU1 > > tlb_gather_mmu() > > lock PTLn > no mod > unlock PTLn > > > lock PTLm > mod > include in tlb range > unlock PTLm > > tlb_finish_mmu() > force = mm_tlb_flush_nested(tlb->mm); > > tlb_gather_mmu() > > lock PTLn > mod > unlock PTLn > > arch_tlb_finish_mmu(force); > > > ... more ... > > tlb_finish_mmu() > > > This? > > > Could you clarify under what exact condition mm_tlb_flush_nested() must > return true? mm_tlb_flush_nested aims for the CPU side where there is no pte update but need TLB flush. As I wrote https://marc.info/?l=linux-mm=150267398226529=2, it has stable TLB problem if we don't flush TLB although there is no pte modification.
Re: linux-next: manual merge of the akpm-current tree with the tip tree
On Sun, Aug 13, 2017 at 02:50:19PM +0200, Peter Zijlstra wrote: > On Sun, Aug 13, 2017 at 06:06:32AM +, Nadav Amit wrote: > > > however mm_tlb_flush_nested() is a mystery, it appears to care about > > > anything inside the range. For now rely on it doing at least _a_ PTL > > > lock instead of taking _the_ PTL lock. > > > > It does not care about “anything” inside the range, but only on situations > > in which there is at least one (same) PT that was modified by one core and > > then read by the other. So, yes, it will always be _the_ same PTL, and not > > _a_ PTL - in the cases that flush is really needed. > > > > The issue that might require additional barriers is that > > inc_tlb_flush_pending() and mm_tlb_flush_nested() are called when the PTL is > > not held. IIUC, since the release-acquire might not behave as a full memory > > barrier, this requires an explicit memory barrier. > > So I'm not entirely clear about this yet. > > How about: > > > CPU0CPU1 > > tlb_gather_mmu() > > lock PTLn > no mod > unlock PTLn > > tlb_gather_mmu() > > lock PTLm > mod > include in tlb range > unlock PTLm > > lock PTLn > mod > unlock PTLn > > tlb_finish_mmu() > force = mm_tlb_flush_nested(tlb->mm); > arch_tlb_finish_mmu(force); > > > ... more ... > > tlb_finish_mmu() > > > > In this case you also want CPU1's mm_tlb_flush_nested() call to return > true, right? No, because CPU 1 mofified pte and added it into tlb range so regardless of nested, it will flush TLB so there is no stale TLB problem. > > But even with an smp_mb__after_atomic() at CPU0's tlg_bather_mmu() > you're not guaranteed CPU1 sees the increment. The only way to do that > is to make the PTL locks RCsc and that is a much more expensive > proposition. > > > What about: > > > CPU0CPU1 > > tlb_gather_mmu() > > lock PTLn > no mod > unlock PTLn > > > lock PTLm > mod > include in tlb range > unlock PTLm > > tlb_gather_mmu() > > lock PTLn > mod > unlock PTLn > > tlb_finish_mmu() > force = mm_tlb_flush_nested(tlb->mm); > arch_tlb_finish_mmu(force); > > > ... more ... > > tlb_finish_mmu() > > Do we want CPU1 to see it here? If so, where does it end? Ditto. Since CPU 1 has added range, it will flush TLB regardless of nested condition. > > CPU0CPU1 > > tlb_gather_mmu() > > lock PTLn > no mod > unlock PTLn > > > lock PTLm > mod > include in tlb range > unlock PTLm > > tlb_finish_mmu() > force = mm_tlb_flush_nested(tlb->mm); > > tlb_gather_mmu() > > lock PTLn > mod > unlock PTLn > > arch_tlb_finish_mmu(force); > > > ... more ... > > tlb_finish_mmu() > > > This? > > > Could you clarify under what exact condition mm_tlb_flush_nested() must > return true? mm_tlb_flush_nested aims for the CPU side where there is no pte update but need TLB flush. As I wrote https://marc.info/?l=linux-mm=150267398226529=2, it has stable TLB problem if we don't flush TLB although there is no pte modification.
Re: [PATCH 04/11] serial: sunsu: constify uart_ops structures
From: Julia LawallDate: Sun, 13 Aug 2017 08:21:43 +0200 > These uart_ops structures are only stored in the ops field of a > uart_port structure and this fields is const, so the uart_ops > structures can also be const. > > Done with the help of Coccinelle. > > Signed-off-by: Julia Lawall Acked-by: David S. Miller
Re: [PATCH 04/11] serial: sunsu: constify uart_ops structures
From: Julia Lawall Date: Sun, 13 Aug 2017 08:21:43 +0200 > These uart_ops structures are only stored in the ops field of a > uart_port structure and this fields is const, so the uart_ops > structures can also be const. > > Done with the help of Coccinelle. > > Signed-off-by: Julia Lawall Acked-by: David S. Miller
Re: [PATCH 03/11] serial: sunsab: constify uart_ops structures
From: Julia LawallDate: Sun, 13 Aug 2017 08:21:42 +0200 > These uart_ops structures are only stored in the ops field of a > uart_port structure and this fields is const, so the uart_ops > structures can also be const. > > Done with the help of Coccinelle. > > Signed-off-by: Julia Lawall Acked-by: David S. Miller
Re: [PATCH 0/4] constify net platform_device_id
From: Arvind YadavDate: Sun, 13 Aug 2017 16:41:44 +0530 > platform_device_id are not supposed to change at runtime. All functions > working with platform_device_id provided by > work with const platform_device_id. So mark the non-const structs as const. Series applied, thanks.
Re: [PATCH 03/11] serial: sunsab: constify uart_ops structures
From: Julia Lawall Date: Sun, 13 Aug 2017 08:21:42 +0200 > These uart_ops structures are only stored in the ops field of a > uart_port structure and this fields is const, so the uart_ops > structures can also be const. > > Done with the help of Coccinelle. > > Signed-off-by: Julia Lawall Acked-by: David S. Miller
Re: [PATCH 0/4] constify net platform_device_id
From: Arvind Yadav Date: Sun, 13 Aug 2017 16:41:44 +0530 > platform_device_id are not supposed to change at runtime. All functions > working with platform_device_id provided by > work with const platform_device_id. So mark the non-const structs as const. Series applied, thanks.
Re: [PATCH] tap: make struct tap_fops static
From: Colin KingDate: Sat, 12 Aug 2017 22:52:31 +0100 > From: Colin Ian King > > The structure tap_fops is local to the source and does not need to > be in global scope, so make it static. > > Cleans up sparse warning: > symbol 'tap_fops' was not declared. Should it be static? > > Signed-off-by: Colin Ian King Applied.
Re: [PATCH] tap: make struct tap_fops static
From: Colin King Date: Sat, 12 Aug 2017 22:52:31 +0100 > From: Colin Ian King > > The structure tap_fops is local to the source and does not need to > be in global scope, so make it static. > > Cleans up sparse warning: > symbol 'tap_fops' was not declared. Should it be static? > > Signed-off-by: Colin Ian King Applied.
Re: [PATCH] virtio-net: make array guest_offloads static
From: Colin KingDate: Sat, 12 Aug 2017 22:45:53 +0100 > From: Colin Ian King > > The array guest_offloads is local to the source and does not need to > be in global scope, so make it static. Also tweak formatting. > > Cleans up sparse warnings: > symbol 'guest_offloads' was not declared. Should it be static? > > Signed-off-by: Colin Ian King Applied.
Re: linux-next: manual merge of the akpm-current tree with the tip tree
Hi Peter, On Fri, Aug 11, 2017 at 04:04:50PM +0200, Peter Zijlstra wrote: > > Ok, so I have the below to still go on-top. > > Ideally someone would clarify the situation around > mm_tlb_flush_nested(), because ideally we'd remove the > smp_mb__after_atomic() and go back to relying on PTL alone. > > This also removes the pointless smp_mb__before_atomic() I'm not an expert of barrier stuff but IIUC, mm_tlb_flush_nested's side full memory barrier can go with removing smp_mb__after_atomic in inc_tlb_flush_pending side? diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h index 490af494c2da..5ad0e66df363 100644 --- a/include/linux/mm_types.h +++ b/include/linux/mm_types.h @@ -544,7 +544,12 @@ static inline bool mm_tlb_flush_pending(struct mm_struct *mm) */ static inline bool mm_tlb_flush_nested(struct mm_struct *mm) { - return atomic_read(>tlb_flush_pending) > 1; + /* +* atomic_dec_and_test's full memory barrier guarantees +* to see uptodate tlb_flush_pending count in other CPU +* without relying on page table lock. +*/ + return !atomic_dec_and_test(>tlb_flush_pending); } static inline void init_tlb_flush_pending(struct mm_struct *mm) diff --git a/mm/memory.c b/mm/memory.c index f571b0eb9816..e90b57bc65fb 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -407,6 +407,10 @@ void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm, unsigned long start, unsigned long end) { arch_tlb_gather_mmu(tlb, mm, start, end); + /* +* couterpart is mm_tlb_flush_nested in tlb_finish_mmu +* which decreases pending count. +*/ inc_tlb_flush_pending(tlb->mm); } @@ -446,9 +450,7 @@ void tlb_finish_mmu(struct mmu_gather *tlb, * */ bool force = mm_tlb_flush_nested(tlb->mm); - arch_tlb_finish_mmu(tlb, start, end, force); - dec_tlb_flush_pending(tlb->mm); } /*
Re: [PATCH] virtio-net: make array guest_offloads static
From: Colin King Date: Sat, 12 Aug 2017 22:45:53 +0100 > From: Colin Ian King > > The array guest_offloads is local to the source and does not need to > be in global scope, so make it static. Also tweak formatting. > > Cleans up sparse warnings: > symbol 'guest_offloads' was not declared. Should it be static? > > Signed-off-by: Colin Ian King Applied.
Re: linux-next: manual merge of the akpm-current tree with the tip tree
Hi Peter, On Fri, Aug 11, 2017 at 04:04:50PM +0200, Peter Zijlstra wrote: > > Ok, so I have the below to still go on-top. > > Ideally someone would clarify the situation around > mm_tlb_flush_nested(), because ideally we'd remove the > smp_mb__after_atomic() and go back to relying on PTL alone. > > This also removes the pointless smp_mb__before_atomic() I'm not an expert of barrier stuff but IIUC, mm_tlb_flush_nested's side full memory barrier can go with removing smp_mb__after_atomic in inc_tlb_flush_pending side? diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h index 490af494c2da..5ad0e66df363 100644 --- a/include/linux/mm_types.h +++ b/include/linux/mm_types.h @@ -544,7 +544,12 @@ static inline bool mm_tlb_flush_pending(struct mm_struct *mm) */ static inline bool mm_tlb_flush_nested(struct mm_struct *mm) { - return atomic_read(>tlb_flush_pending) > 1; + /* +* atomic_dec_and_test's full memory barrier guarantees +* to see uptodate tlb_flush_pending count in other CPU +* without relying on page table lock. +*/ + return !atomic_dec_and_test(>tlb_flush_pending); } static inline void init_tlb_flush_pending(struct mm_struct *mm) diff --git a/mm/memory.c b/mm/memory.c index f571b0eb9816..e90b57bc65fb 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -407,6 +407,10 @@ void tlb_gather_mmu(struct mmu_gather *tlb, struct mm_struct *mm, unsigned long start, unsigned long end) { arch_tlb_gather_mmu(tlb, mm, start, end); + /* +* couterpart is mm_tlb_flush_nested in tlb_finish_mmu +* which decreases pending count. +*/ inc_tlb_flush_pending(tlb->mm); } @@ -446,9 +450,7 @@ void tlb_finish_mmu(struct mmu_gather *tlb, * */ bool force = mm_tlb_flush_nested(tlb->mm); - arch_tlb_finish_mmu(tlb, start, end, force); - dec_tlb_flush_pending(tlb->mm); } /*
Re: [PATCH net-next V2 0/3] XDP support for tap
From: Jason WangDate: Fri, 11 Aug 2017 19:41:15 +0800 > Hi all: > > This series tries to implement XDP support for tap. Two path were > implemented: > > - fast path: small & non-gso packet, For performance reason we do it > at page level and use build_skb() to create skb if necessary. > - slow path: big or gso packet, we don't want to lose the capability > compared to generic XDP, so we export some generic xdp helpers and > do it after skb was created. > > xdp1 shows about 41% improvement, xdp_redirect shows about 60% > improvement. > > Changes from V1: > - fix the race between xdp set and free > - don't hold extra refcount > - add XDP_REDIRECT support > > Please review. Series applied, thanks Jason.
Re: [PATCH net-next V2 0/3] XDP support for tap
From: Jason Wang Date: Fri, 11 Aug 2017 19:41:15 +0800 > Hi all: > > This series tries to implement XDP support for tap. Two path were > implemented: > > - fast path: small & non-gso packet, For performance reason we do it > at page level and use build_skb() to create skb if necessary. > - slow path: big or gso packet, we don't want to lose the capability > compared to generic XDP, so we export some generic xdp helpers and > do it after skb was created. > > xdp1 shows about 41% improvement, xdp_redirect shows about 60% > improvement. > > Changes from V1: > - fix the race between xdp set and free > - don't hold extra refcount > - add XDP_REDIRECT support > > Please review. Series applied, thanks Jason.
Re: [PATCH v3 0/7] Add rk3328 pwm support
Hi Boris && thierry, Do you have any other suggestion for the patches? 在 2017/8/8 23:38, David Wu 写道: There are two features of rk3328 pwm module. - PWM APB and function clocks are different. - Add pwm atomic hardware update David Wu (7): pwm: rockchip: Add APB and function both clocks support pwm: rockchip: Remove the judge from return value of pwm_config pwm: rockchip: Use pwm_apply instead of the pwm_enable pwm: rockchip: Move the configuration of polarity from rockchip_pwm_set_enable() to rockchip_pwm_config() pwm: rockchip: Use same pwm ops for each IP pwm: rockchip: Add rk3328 pwm support arm64: dts: rockchip: Add pwm nodes for rk3328 .../devicetree/bindings/pwm/pwm-rockchip.txt | 8 +- arch/arm64/boot/dts/rockchip/rk3328.dtsi | 45 drivers/pwm/pwm-rockchip.c | 281 +++-- 3 files changed, 199 insertions(+), 135 deletions(-)
Re: [PATCH v3 0/7] Add rk3328 pwm support
Hi Boris && thierry, Do you have any other suggestion for the patches? 在 2017/8/8 23:38, David Wu 写道: There are two features of rk3328 pwm module. - PWM APB and function clocks are different. - Add pwm atomic hardware update David Wu (7): pwm: rockchip: Add APB and function both clocks support pwm: rockchip: Remove the judge from return value of pwm_config pwm: rockchip: Use pwm_apply instead of the pwm_enable pwm: rockchip: Move the configuration of polarity from rockchip_pwm_set_enable() to rockchip_pwm_config() pwm: rockchip: Use same pwm ops for each IP pwm: rockchip: Add rk3328 pwm support arm64: dts: rockchip: Add pwm nodes for rk3328 .../devicetree/bindings/pwm/pwm-rockchip.txt | 8 +- arch/arm64/boot/dts/rockchip/rk3328.dtsi | 45 drivers/pwm/pwm-rockchip.c | 281 +++-- 3 files changed, 199 insertions(+), 135 deletions(-)
Re: [PATCH v2] x86/xen/64: Rearrange the SYSCALL entries
On Mon, Aug 7, 2017 at 11:59 PM, Andy Lutomirskiwrote: > Xen's raw SYSCALL entries are much less weird than native. Rather > than fudging them to look like native entries, use the Xen-provided > stack frame directly. > > This lets us eliminate entry_SYSCALL_64_after_swapgs and two uses of > the SWAPGS_UNSAFE_STACK paravirt hook. The SYSENTER code would > benefit from similar treatment. > > This makes one change to the native code path: the compat > instruction that clears the high 32 bits of %rax is moved slightly > later. I'd be surprised if this affects performance at all. > > Signed-off-by: Andy Lutomirski > --- > > Changes from v1 (which I never actually emailed): > - Fix zero-extension in the compat case. > > arch/x86/entry/entry_64.S| 9 ++--- > arch/x86/entry/entry_64_compat.S | 7 +++ > arch/x86/xen/xen-asm_64.S| 23 +-- > 3 files changed, 14 insertions(+), 25 deletions(-) > > diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S > index aa58155187c5..7cee92cf807f 100644 > --- a/arch/x86/entry/entry_64.S > +++ b/arch/x86/entry/entry_64.S > @@ -142,14 +142,8 @@ ENTRY(entry_SYSCALL_64) > * We do not frame this tiny irq-off block with TRACE_IRQS_OFF/ON, > * it is too small to ever cause noticeable irq latency. > */ > - SWAPGS_UNSAFE_STACK > - /* > -* A hypervisor implementation might want to use a label > -* after the swapgs, so that it can do the swapgs > -* for the guest and jump here on syscall. > -*/ > -GLOBAL(entry_SYSCALL_64_after_swapgs) > > + swapgs > movq%rsp, PER_CPU_VAR(rsp_scratch) > movqPER_CPU_VAR(cpu_current_top_of_stack), %rsp > > @@ -161,6 +155,7 @@ GLOBAL(entry_SYSCALL_64_after_swapgs) > pushq %r11/* pt_regs->flags */ > pushq $__USER_CS /* pt_regs->cs */ > pushq %rcx/* pt_regs->ip */ > +GLOBAL(entry_SYSCALL_64_after_hwframe) > pushq %rax/* pt_regs->orig_ax */ > pushq %rdi/* pt_regs->di */ > pushq %rsi/* pt_regs->si */ > diff --git a/arch/x86/entry/entry_64_compat.S > b/arch/x86/entry/entry_64_compat.S > index e1721dafbcb1..5314d7b8e5ad 100644 > --- a/arch/x86/entry/entry_64_compat.S > +++ b/arch/x86/entry/entry_64_compat.S > @@ -183,21 +183,20 @@ ENDPROC(entry_SYSENTER_compat) > */ > ENTRY(entry_SYSCALL_compat) > /* Interrupts are off on entry. */ > - SWAPGS_UNSAFE_STACK > + swapgs > > /* Stash user ESP and switch to the kernel stack. */ > movl%esp, %r8d > movqPER_CPU_VAR(cpu_current_top_of_stack), %rsp > > - /* Zero-extending 32-bit regs, do not remove */ > - movl%eax, %eax > - > /* Construct struct pt_regs on stack */ > pushq $__USER32_DS/* pt_regs->ss */ > pushq %r8 /* pt_regs->sp */ > pushq %r11/* pt_regs->flags */ > pushq $__USER32_CS/* pt_regs->cs */ > pushq %rcx/* pt_regs->ip */ > +GLOBAL(entry_SYSCALL_compat_after_hwframe) > + movl%eax, %eax /* discard orig_ax high bits */ > pushq %rax/* pt_regs->orig_ax */ > pushq %rdi/* pt_regs->di */ > pushq %rsi/* pt_regs->si */ > diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S > index c3df43141e70..a8a4f4c460a6 100644 > --- a/arch/x86/xen/xen-asm_64.S > +++ b/arch/x86/xen/xen-asm_64.S > @@ -82,34 +82,29 @@ RELOC(xen_sysret64, 1b+1) > * rip > * r11 > * rsp->rcx > - * > - * In all the entrypoints, we undo all that to make it look like a > - * CPU-generated syscall/sysenter and jump to the normal entrypoint. > */ > > -.macro undo_xen_syscall > - mov 0*8(%rsp), %rcx > - mov 1*8(%rsp), %r11 > - mov 5*8(%rsp), %rsp > -.endm > - > /* Normal 64-bit system call target */ > ENTRY(xen_syscall_target) > - undo_xen_syscall > - jmp entry_SYSCALL_64_after_swapgs > + popq %rcx > + popq %r11 > + jmp entry_SYSCALL_64_after_hwframe > ENDPROC(xen_syscall_target) > > #ifdef CONFIG_IA32_EMULATION > > /* 32-bit compat syscall target */ > ENTRY(xen_syscall32_target) > - undo_xen_syscall > - jmp entry_SYSCALL_compat > + popq %rcx > + popq %r11 > + jmp entry_SYSCALL_compat_after_hwframe > ENDPROC(xen_syscall32_target) > > /* 32-bit compat sysenter target */ > ENTRY(xen_sysenter_target) > - undo_xen_syscall > + mov 0*8(%rsp), %rcx > + mov 1*8(%rsp), %r11 > + mov 5*8(%rsp), %rsp > jmp entry_SYSENTER_compat > ENDPROC(xen_sysenter_target) This patch causes the iopl_32 and
Re: [PATCH v2] x86/xen/64: Rearrange the SYSCALL entries
On Mon, Aug 7, 2017 at 11:59 PM, Andy Lutomirski wrote: > Xen's raw SYSCALL entries are much less weird than native. Rather > than fudging them to look like native entries, use the Xen-provided > stack frame directly. > > This lets us eliminate entry_SYSCALL_64_after_swapgs and two uses of > the SWAPGS_UNSAFE_STACK paravirt hook. The SYSENTER code would > benefit from similar treatment. > > This makes one change to the native code path: the compat > instruction that clears the high 32 bits of %rax is moved slightly > later. I'd be surprised if this affects performance at all. > > Signed-off-by: Andy Lutomirski > --- > > Changes from v1 (which I never actually emailed): > - Fix zero-extension in the compat case. > > arch/x86/entry/entry_64.S| 9 ++--- > arch/x86/entry/entry_64_compat.S | 7 +++ > arch/x86/xen/xen-asm_64.S| 23 +-- > 3 files changed, 14 insertions(+), 25 deletions(-) > > diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S > index aa58155187c5..7cee92cf807f 100644 > --- a/arch/x86/entry/entry_64.S > +++ b/arch/x86/entry/entry_64.S > @@ -142,14 +142,8 @@ ENTRY(entry_SYSCALL_64) > * We do not frame this tiny irq-off block with TRACE_IRQS_OFF/ON, > * it is too small to ever cause noticeable irq latency. > */ > - SWAPGS_UNSAFE_STACK > - /* > -* A hypervisor implementation might want to use a label > -* after the swapgs, so that it can do the swapgs > -* for the guest and jump here on syscall. > -*/ > -GLOBAL(entry_SYSCALL_64_after_swapgs) > > + swapgs > movq%rsp, PER_CPU_VAR(rsp_scratch) > movqPER_CPU_VAR(cpu_current_top_of_stack), %rsp > > @@ -161,6 +155,7 @@ GLOBAL(entry_SYSCALL_64_after_swapgs) > pushq %r11/* pt_regs->flags */ > pushq $__USER_CS /* pt_regs->cs */ > pushq %rcx/* pt_regs->ip */ > +GLOBAL(entry_SYSCALL_64_after_hwframe) > pushq %rax/* pt_regs->orig_ax */ > pushq %rdi/* pt_regs->di */ > pushq %rsi/* pt_regs->si */ > diff --git a/arch/x86/entry/entry_64_compat.S > b/arch/x86/entry/entry_64_compat.S > index e1721dafbcb1..5314d7b8e5ad 100644 > --- a/arch/x86/entry/entry_64_compat.S > +++ b/arch/x86/entry/entry_64_compat.S > @@ -183,21 +183,20 @@ ENDPROC(entry_SYSENTER_compat) > */ > ENTRY(entry_SYSCALL_compat) > /* Interrupts are off on entry. */ > - SWAPGS_UNSAFE_STACK > + swapgs > > /* Stash user ESP and switch to the kernel stack. */ > movl%esp, %r8d > movqPER_CPU_VAR(cpu_current_top_of_stack), %rsp > > - /* Zero-extending 32-bit regs, do not remove */ > - movl%eax, %eax > - > /* Construct struct pt_regs on stack */ > pushq $__USER32_DS/* pt_regs->ss */ > pushq %r8 /* pt_regs->sp */ > pushq %r11/* pt_regs->flags */ > pushq $__USER32_CS/* pt_regs->cs */ > pushq %rcx/* pt_regs->ip */ > +GLOBAL(entry_SYSCALL_compat_after_hwframe) > + movl%eax, %eax /* discard orig_ax high bits */ > pushq %rax/* pt_regs->orig_ax */ > pushq %rdi/* pt_regs->di */ > pushq %rsi/* pt_regs->si */ > diff --git a/arch/x86/xen/xen-asm_64.S b/arch/x86/xen/xen-asm_64.S > index c3df43141e70..a8a4f4c460a6 100644 > --- a/arch/x86/xen/xen-asm_64.S > +++ b/arch/x86/xen/xen-asm_64.S > @@ -82,34 +82,29 @@ RELOC(xen_sysret64, 1b+1) > * rip > * r11 > * rsp->rcx > - * > - * In all the entrypoints, we undo all that to make it look like a > - * CPU-generated syscall/sysenter and jump to the normal entrypoint. > */ > > -.macro undo_xen_syscall > - mov 0*8(%rsp), %rcx > - mov 1*8(%rsp), %r11 > - mov 5*8(%rsp), %rsp > -.endm > - > /* Normal 64-bit system call target */ > ENTRY(xen_syscall_target) > - undo_xen_syscall > - jmp entry_SYSCALL_64_after_swapgs > + popq %rcx > + popq %r11 > + jmp entry_SYSCALL_64_after_hwframe > ENDPROC(xen_syscall_target) > > #ifdef CONFIG_IA32_EMULATION > > /* 32-bit compat syscall target */ > ENTRY(xen_syscall32_target) > - undo_xen_syscall > - jmp entry_SYSCALL_compat > + popq %rcx > + popq %r11 > + jmp entry_SYSCALL_compat_after_hwframe > ENDPROC(xen_syscall32_target) > > /* 32-bit compat sysenter target */ > ENTRY(xen_sysenter_target) > - undo_xen_syscall > + mov 0*8(%rsp), %rcx > + mov 1*8(%rsp), %r11 > + mov 5*8(%rsp), %rsp > jmp entry_SYSENTER_compat > ENDPROC(xen_sysenter_target) This patch causes the iopl_32 and ioperm_32 self-tests to fail on a
Re: [PATCH 1/5] cramfs: direct memory access support
On Sat, 12 Aug 2017, Christoph Hellwig wrote: > Direct physical memory access in a file system is never safe. > Please make sure this goes through struct dax_operations. Well, after having a closer look, I don't think dax might be relevant here for a couple reasons: - cramfs is read-only. No concurrent writes to worry about. Please elaborate if you have other safety concerns in mind. - This series targets small no-MMU systems. There is no paging involved given the lack of a MMU. The whole of the filesystem is always directly accessible in the address space from ROM alongside with the actual kernel code being executed. I don't see how dax would ever be pertinent here. - Even with MMU systems, the maximum size of a cramfs image can't exceed 272MB. In practice it is likely to be much much less. Given this targets small memory systems, there is always plenty of vmalloc space left to map it all and even a 272MB memremap() wouldn't be a problem. If it is a problem then maybe your system has large resources to manage already and you're pretty unlikely to be using cramfs in the first place, otherwise accessing it through a block device would be just fine, at which point you'd better consider squashfs instead of this. All this to say that I think that dax is _way_ overkill and inappropriate for the intended cramfs use case this series is addressing. Nicolas
Re: [PATCH 1/5] cramfs: direct memory access support
On Sat, 12 Aug 2017, Christoph Hellwig wrote: > Direct physical memory access in a file system is never safe. > Please make sure this goes through struct dax_operations. Well, after having a closer look, I don't think dax might be relevant here for a couple reasons: - cramfs is read-only. No concurrent writes to worry about. Please elaborate if you have other safety concerns in mind. - This series targets small no-MMU systems. There is no paging involved given the lack of a MMU. The whole of the filesystem is always directly accessible in the address space from ROM alongside with the actual kernel code being executed. I don't see how dax would ever be pertinent here. - Even with MMU systems, the maximum size of a cramfs image can't exceed 272MB. In practice it is likely to be much much less. Given this targets small memory systems, there is always plenty of vmalloc space left to map it all and even a 272MB memremap() wouldn't be a problem. If it is a problem then maybe your system has large resources to manage already and you're pretty unlikely to be using cramfs in the first place, otherwise accessing it through a block device would be just fine, at which point you'd better consider squashfs instead of this. All this to say that I think that dax is _way_ overkill and inappropriate for the intended cramfs use case this series is addressing. Nicolas
Re: [PATCHv6 3/3] ARM:drm ivip Intel FPGA Video and Image Processing Suite
On Fri, 2017-08-11 at 08:21 -0700, Randy Dunlap wrote: > On 08/10/2017 11:49 PM, Hean-Loong, Ong wrote: > > > > diff --git a/drivers/gpu/drm/ivip/Kconfig > > b/drivers/gpu/drm/ivip/Kconfig > > new file mode 100644 > > index 000..398c9ab > > --- /dev/null > > +++ b/drivers/gpu/drm/ivip/Kconfig > > @@ -0,0 +1,14 @@ > > +config DRM_IVIP > > +tristate "Intel FGPA Video and Image Processing" > > +depends on DRM && OF > > +select DRM_GEM_CMA_HELPER > > +select DRM_KMS_HELPER > > +select DRM_KMS_FB_HELPER > > +select DRM_KMS_CMA_HELPER > > +help > > +Choose this option if you have a Intel FPGA Arria 10 > > system > an > > > > > +and above with an Intel Display Port IP. This does not > > support > > +legacy Intel FPGA Cyclone V display port. Currently > > only single > > +frame buffer is supported. Note that ACPI and X_86 > > architecture > > +is not supported for Arria10.If M is selected the > > module will be > Arria10. If M is > > > > > +called ivip. > All of the help text should be indented with one tab + 2 spaces > according to coding-style.rst. > > Noted.
Re: [PATCHv6 3/3] ARM:drm ivip Intel FPGA Video and Image Processing Suite
On Fri, 2017-08-11 at 08:21 -0700, Randy Dunlap wrote: > On 08/10/2017 11:49 PM, Hean-Loong, Ong wrote: > > > > diff --git a/drivers/gpu/drm/ivip/Kconfig > > b/drivers/gpu/drm/ivip/Kconfig > > new file mode 100644 > > index 000..398c9ab > > --- /dev/null > > +++ b/drivers/gpu/drm/ivip/Kconfig > > @@ -0,0 +1,14 @@ > > +config DRM_IVIP > > +tristate "Intel FGPA Video and Image Processing" > > +depends on DRM && OF > > +select DRM_GEM_CMA_HELPER > > +select DRM_KMS_HELPER > > +select DRM_KMS_FB_HELPER > > +select DRM_KMS_CMA_HELPER > > +help > > +Choose this option if you have a Intel FPGA Arria 10 > > system > an > > > > > +and above with an Intel Display Port IP. This does not > > support > > +legacy Intel FPGA Cyclone V display port. Currently > > only single > > +frame buffer is supported. Note that ACPI and X_86 > > architecture > > +is not supported for Arria10.If M is selected the > > module will be > Arria10. If M is > > > > > +called ivip. > All of the help text should be indented with one tab + 2 spaces > according to coding-style.rst. > > Noted.
Re: [PATCH v4 4/5] squashfs: Add zstd support
On Sun, Aug 13, 2017 at 9:31 AM, Geert Uytterhoevenwrote: > On Fri, Aug 4, 2017 at 10:19 PM, Nick Terrell wrote: >> --- a/fs/squashfs/decompressor.c >> +++ b/fs/squashfs/decompressor.c >> @@ -65,6 +65,12 @@ static const struct squashfs_decompressor >> squashfs_zlib_comp_ops = { >> }; >> #endif >> >> +#ifndef CONFIG_SQUASHFS_ZSTD >> +static const struct squashfs_decompressor squashfs_zstd_comp_ops = { >> + NULL, NULL, NULL, NULL, ZSTD_COMPRESSION, "zstd", 0 > > Can you please use designated initializers? I prefer it as it is. It matches the coding style that I used in the rest of that file to declare the "unsupported" compressor entries (if this patch uses a different style it would look odd).There's no pointers to functions being assigned here, and it makes it a short and concise one-line. Phillip > >> +}; >> +#endif >> + >> static const struct squashfs_decompressor squashfs_unknown_comp_ops = { >> NULL, NULL, NULL, NULL, 0, "unknown", 0 >> ; > >> +const struct squashfs_decompressor squashfs_zstd_comp_ops = { >> + .init = zstd_init, >> + .free = zstd_free, >> + .decompress = zstd_uncompress, >> + .id = ZSTD_COMPRESSION, >> + .name = "zstd", >> + .supported = 1 >> +}; > > ... like you did here. > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > ge...@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like > that. > -- Linus Torvalds
Re: [PATCH v4 4/5] squashfs: Add zstd support
On Sun, Aug 13, 2017 at 9:31 AM, Geert Uytterhoeven wrote: > On Fri, Aug 4, 2017 at 10:19 PM, Nick Terrell wrote: >> --- a/fs/squashfs/decompressor.c >> +++ b/fs/squashfs/decompressor.c >> @@ -65,6 +65,12 @@ static const struct squashfs_decompressor >> squashfs_zlib_comp_ops = { >> }; >> #endif >> >> +#ifndef CONFIG_SQUASHFS_ZSTD >> +static const struct squashfs_decompressor squashfs_zstd_comp_ops = { >> + NULL, NULL, NULL, NULL, ZSTD_COMPRESSION, "zstd", 0 > > Can you please use designated initializers? I prefer it as it is. It matches the coding style that I used in the rest of that file to declare the "unsupported" compressor entries (if this patch uses a different style it would look odd).There's no pointers to functions being assigned here, and it makes it a short and concise one-line. Phillip > >> +}; >> +#endif >> + >> static const struct squashfs_decompressor squashfs_unknown_comp_ops = { >> NULL, NULL, NULL, NULL, 0, "unknown", 0 >> ; > >> +const struct squashfs_decompressor squashfs_zstd_comp_ops = { >> + .init = zstd_init, >> + .free = zstd_free, >> + .decompress = zstd_uncompress, >> + .id = ZSTD_COMPRESSION, >> + .name = "zstd", >> + .supported = 1 >> +}; > > ... like you did here. > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > ge...@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like > that. > -- Linus Torvalds
Re: [PATCH net-next v2 3/3] ftgmac100: Support NCSI VLAN filtering when available
On Mon, Aug 14, 2017 at 10:59 AM, Samuel Mendoza-Jonaswrote: > Register the ndo_vlan_rx_{add,kill}_vid callbacks and set the > NETIF_F_HW_VLAN_CTAG_FILTER if NCSI is available. > This allows the VLAN core to notify the NCSI driver when changes occur > so that the remote NCSI channel can be properly configured to filter on > the set VLAN tags. > > Signed-off-by: Samuel Mendoza-Jonas I'm not a vlan expert, so I asked why this was only being set when doing NCSI. The answer was: > There is no point setting it when not doing ncsi. The HW doesn't have a > filter in that case (we do have HW vlan tag extraction and injection, > which my driver supports, but that's different flags). Reviewed-by: Joel Stanley > --- > v2: Moved ftgmac100 change into same patch and reordered > > drivers/net/ethernet/faraday/ftgmac100.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/drivers/net/ethernet/faraday/ftgmac100.c > b/drivers/net/ethernet/faraday/ftgmac100.c > index 34dae51effd4..05fe7123d5ae 100644 > --- a/drivers/net/ethernet/faraday/ftgmac100.c > +++ b/drivers/net/ethernet/faraday/ftgmac100.c > @@ -1623,6 +1623,8 @@ static const struct net_device_ops ftgmac100_netdev_ops > = { > #ifdef CONFIG_NET_POLL_CONTROLLER > .ndo_poll_controller= ftgmac100_poll_controller, > #endif > + .ndo_vlan_rx_add_vid= ncsi_vlan_rx_add_vid, > + .ndo_vlan_rx_kill_vid = ncsi_vlan_rx_kill_vid, > }; > > static int ftgmac100_setup_mdio(struct net_device *netdev) > @@ -1837,6 +1839,9 @@ static int ftgmac100_probe(struct platform_device *pdev) > NETIF_F_GRO | NETIF_F_SG | NETIF_F_HW_VLAN_CTAG_RX | > NETIF_F_HW_VLAN_CTAG_TX; > > + if (priv->use_ncsi) > + netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER; > + > /* AST2400 doesn't have working HW checksum generation */ > if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac"))) > netdev->hw_features &= ~NETIF_F_HW_CSUM; > -- > 2.14.0 >
Re: [PATCH net-next v2 3/3] ftgmac100: Support NCSI VLAN filtering when available
On Mon, Aug 14, 2017 at 10:59 AM, Samuel Mendoza-Jonas wrote: > Register the ndo_vlan_rx_{add,kill}_vid callbacks and set the > NETIF_F_HW_VLAN_CTAG_FILTER if NCSI is available. > This allows the VLAN core to notify the NCSI driver when changes occur > so that the remote NCSI channel can be properly configured to filter on > the set VLAN tags. > > Signed-off-by: Samuel Mendoza-Jonas I'm not a vlan expert, so I asked why this was only being set when doing NCSI. The answer was: > There is no point setting it when not doing ncsi. The HW doesn't have a > filter in that case (we do have HW vlan tag extraction and injection, > which my driver supports, but that's different flags). Reviewed-by: Joel Stanley > --- > v2: Moved ftgmac100 change into same patch and reordered > > drivers/net/ethernet/faraday/ftgmac100.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/drivers/net/ethernet/faraday/ftgmac100.c > b/drivers/net/ethernet/faraday/ftgmac100.c > index 34dae51effd4..05fe7123d5ae 100644 > --- a/drivers/net/ethernet/faraday/ftgmac100.c > +++ b/drivers/net/ethernet/faraday/ftgmac100.c > @@ -1623,6 +1623,8 @@ static const struct net_device_ops ftgmac100_netdev_ops > = { > #ifdef CONFIG_NET_POLL_CONTROLLER > .ndo_poll_controller= ftgmac100_poll_controller, > #endif > + .ndo_vlan_rx_add_vid= ncsi_vlan_rx_add_vid, > + .ndo_vlan_rx_kill_vid = ncsi_vlan_rx_kill_vid, > }; > > static int ftgmac100_setup_mdio(struct net_device *netdev) > @@ -1837,6 +1839,9 @@ static int ftgmac100_probe(struct platform_device *pdev) > NETIF_F_GRO | NETIF_F_SG | NETIF_F_HW_VLAN_CTAG_RX | > NETIF_F_HW_VLAN_CTAG_TX; > > + if (priv->use_ncsi) > + netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER; > + > /* AST2400 doesn't have working HW checksum generation */ > if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac"))) > netdev->hw_features &= ~NETIF_F_HW_CSUM; > -- > 2.14.0 >
Re: [PATCH net-next v2 1/3] net/ncsi: Fix several packet definitions
On Mon, Aug 14, 2017 at 10:59 AM, Samuel Mendoza-Jonaswrote: I asked Sam if these should be backported to stable and he said: > These are straight up bugs except... without my changes we never call > this code. As Ben says as time provides a lot of the current definitions > need to be gone over, there's a few command/response code paths that are > never triggered and could be broken in similar ways. So we're okay here. > Signed-off-by: Samuel Mendoza-Jonas Reviewed-by: Joel Stanley Cheers, Joel > --- > v2: Rebased on latest net-next > > net/ncsi/ncsi-cmd.c | 10 +- > net/ncsi/ncsi-pkt.h | 2 +- > net/ncsi/ncsi-rsp.c | 3 ++- > 3 files changed, 8 insertions(+), 7 deletions(-) > > diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c > index 5e03ed190e18..7567ca63aae2 100644 > --- a/net/ncsi/ncsi-cmd.c > +++ b/net/ncsi/ncsi-cmd.c > @@ -139,9 +139,9 @@ static int ncsi_cmd_handler_svf(struct sk_buff *skb, > struct ncsi_cmd_svf_pkt *cmd; > > cmd = skb_put_zero(skb, sizeof(*cmd)); > - cmd->vlan = htons(nca->words[0]); > - cmd->index = nca->bytes[2]; > - cmd->enable = nca->bytes[3]; > + cmd->vlan = htons(nca->words[1]); > + cmd->index = nca->bytes[6]; > + cmd->enable = nca->bytes[7]; > ncsi_cmd_build_header(>cmd.common, nca); > > return 0; > @@ -153,7 +153,7 @@ static int ncsi_cmd_handler_ev(struct sk_buff *skb, > struct ncsi_cmd_ev_pkt *cmd; > > cmd = skb_put_zero(skb, sizeof(*cmd)); > - cmd->mode = nca->bytes[0]; > + cmd->mode = nca->bytes[3]; > ncsi_cmd_build_header(>cmd.common, nca); > > return 0; > @@ -228,7 +228,7 @@ static struct ncsi_cmd_handler { > { NCSI_PKT_CMD_AE, 8, ncsi_cmd_handler_ae }, > { NCSI_PKT_CMD_SL, 8, ncsi_cmd_handler_sl }, > { NCSI_PKT_CMD_GLS,0, ncsi_cmd_handler_default }, > - { NCSI_PKT_CMD_SVF,4, ncsi_cmd_handler_svf }, > + { NCSI_PKT_CMD_SVF,8, ncsi_cmd_handler_svf }, > { NCSI_PKT_CMD_EV, 4, ncsi_cmd_handler_ev }, > { NCSI_PKT_CMD_DV, 0, ncsi_cmd_handler_default }, > { NCSI_PKT_CMD_SMA,8, ncsi_cmd_handler_sma }, > diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h > index 3ea49ed0a935..91b4b66438df 100644 > --- a/net/ncsi/ncsi-pkt.h > +++ b/net/ncsi/ncsi-pkt.h > @@ -104,7 +104,7 @@ struct ncsi_cmd_svf_pkt { > unsigned char index; /* VLAN table index */ > unsigned char enable;/* Enable or disable */ > __be32 checksum; /* Checksum */ > - unsigned char pad[14]; > + unsigned char pad[18]; > }; > > /* Enable VLAN */ > diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c > index 087db775b3dc..c1a191d790e2 100644 > --- a/net/ncsi/ncsi-rsp.c > +++ b/net/ncsi/ncsi-rsp.c > @@ -354,7 +354,8 @@ static int ncsi_rsp_handler_svf(struct ncsi_request *nr) > > /* Add or remove the VLAN filter */ > if (!(cmd->enable & 0x1)) { > - ret = ncsi_remove_filter(nc, NCSI_FILTER_VLAN, cmd->index); > + /* HW indexes from 1 */ > + ret = ncsi_remove_filter(nc, NCSI_FILTER_VLAN, cmd->index - > 1); > } else { > vlan = ntohs(cmd->vlan); > ret = ncsi_add_filter(nc, NCSI_FILTER_VLAN, ); > -- > 2.14.0 >
Re: [PATCH net-next v2 1/3] net/ncsi: Fix several packet definitions
On Mon, Aug 14, 2017 at 10:59 AM, Samuel Mendoza-Jonas wrote: I asked Sam if these should be backported to stable and he said: > These are straight up bugs except... without my changes we never call > this code. As Ben says as time provides a lot of the current definitions > need to be gone over, there's a few command/response code paths that are > never triggered and could be broken in similar ways. So we're okay here. > Signed-off-by: Samuel Mendoza-Jonas Reviewed-by: Joel Stanley Cheers, Joel > --- > v2: Rebased on latest net-next > > net/ncsi/ncsi-cmd.c | 10 +- > net/ncsi/ncsi-pkt.h | 2 +- > net/ncsi/ncsi-rsp.c | 3 ++- > 3 files changed, 8 insertions(+), 7 deletions(-) > > diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c > index 5e03ed190e18..7567ca63aae2 100644 > --- a/net/ncsi/ncsi-cmd.c > +++ b/net/ncsi/ncsi-cmd.c > @@ -139,9 +139,9 @@ static int ncsi_cmd_handler_svf(struct sk_buff *skb, > struct ncsi_cmd_svf_pkt *cmd; > > cmd = skb_put_zero(skb, sizeof(*cmd)); > - cmd->vlan = htons(nca->words[0]); > - cmd->index = nca->bytes[2]; > - cmd->enable = nca->bytes[3]; > + cmd->vlan = htons(nca->words[1]); > + cmd->index = nca->bytes[6]; > + cmd->enable = nca->bytes[7]; > ncsi_cmd_build_header(>cmd.common, nca); > > return 0; > @@ -153,7 +153,7 @@ static int ncsi_cmd_handler_ev(struct sk_buff *skb, > struct ncsi_cmd_ev_pkt *cmd; > > cmd = skb_put_zero(skb, sizeof(*cmd)); > - cmd->mode = nca->bytes[0]; > + cmd->mode = nca->bytes[3]; > ncsi_cmd_build_header(>cmd.common, nca); > > return 0; > @@ -228,7 +228,7 @@ static struct ncsi_cmd_handler { > { NCSI_PKT_CMD_AE, 8, ncsi_cmd_handler_ae }, > { NCSI_PKT_CMD_SL, 8, ncsi_cmd_handler_sl }, > { NCSI_PKT_CMD_GLS,0, ncsi_cmd_handler_default }, > - { NCSI_PKT_CMD_SVF,4, ncsi_cmd_handler_svf }, > + { NCSI_PKT_CMD_SVF,8, ncsi_cmd_handler_svf }, > { NCSI_PKT_CMD_EV, 4, ncsi_cmd_handler_ev }, > { NCSI_PKT_CMD_DV, 0, ncsi_cmd_handler_default }, > { NCSI_PKT_CMD_SMA,8, ncsi_cmd_handler_sma }, > diff --git a/net/ncsi/ncsi-pkt.h b/net/ncsi/ncsi-pkt.h > index 3ea49ed0a935..91b4b66438df 100644 > --- a/net/ncsi/ncsi-pkt.h > +++ b/net/ncsi/ncsi-pkt.h > @@ -104,7 +104,7 @@ struct ncsi_cmd_svf_pkt { > unsigned char index; /* VLAN table index */ > unsigned char enable;/* Enable or disable */ > __be32 checksum; /* Checksum */ > - unsigned char pad[14]; > + unsigned char pad[18]; > }; > > /* Enable VLAN */ > diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c > index 087db775b3dc..c1a191d790e2 100644 > --- a/net/ncsi/ncsi-rsp.c > +++ b/net/ncsi/ncsi-rsp.c > @@ -354,7 +354,8 @@ static int ncsi_rsp_handler_svf(struct ncsi_request *nr) > > /* Add or remove the VLAN filter */ > if (!(cmd->enable & 0x1)) { > - ret = ncsi_remove_filter(nc, NCSI_FILTER_VLAN, cmd->index); > + /* HW indexes from 1 */ > + ret = ncsi_remove_filter(nc, NCSI_FILTER_VLAN, cmd->index - > 1); > } else { > vlan = ntohs(cmd->vlan); > ret = ncsi_add_filter(nc, NCSI_FILTER_VLAN, ); > -- > 2.14.0 >
Re: [PATCH net-next v2 2/3] net/ncsi: Configure VLAN tag filter
On Mon, Aug 14, 2017 at 10:59 AM, Samuel Mendoza-Jonaswrote: > Make use of the ndo_vlan_rx_{add,kill}_vid callbacks to have the NCSI > stack process new VLAN tags and configure the channel VLAN filter > appropriately. > Several VLAN tags can be set and a "Set VLAN Filter" packet must be sent > for each one, meaning the ncsi_dev_state_config_svf state must be > repeated. An internal list of VLAN tags is maintained, and compared > against the current channel's ncsi_channel_filter in order to keep track > within the state. VLAN filters are removed in a similar manner, with the > introduction of the ncsi_dev_state_config_clear_vids state. The maximum > number of VLAN tag filters is determined by the "Get Capabilities" > response from the channel. > > Signed-off-by: Samuel Mendoza-Jonas I've given this some testing, but there are a few things I saw below that we should sort out. > --- a/net/ncsi/ncsi-manage.c > +++ b/net/ncsi/ncsi-manage.c > @@ -38,6 +38,22 @@ static inline int ncsi_filter_size(int table) > return sizes[table]; > } > > +u32 *ncsi_get_filter(struct ncsi_channel *nc, int table, int index) > +{ > + struct ncsi_channel_filter *ncf; > + int size; > + > + ncf = nc->filters[table]; > + if (!ncf) > + return NULL; > + > + size = ncsi_filter_size(table); > + if (size < 0) > + return NULL; > + > + return ncf->data + size * index; > +} > + > int ncsi_find_filter(struct ncsi_channel *nc, int table, void *data) > { > struct ncsi_channel_filter *ncf; > @@ -58,7 +74,7 @@ int ncsi_find_filter(struct ncsi_channel *nc, int table, > void *data) > index = -1; > while ((index = find_next_bit(bitmap, ncf->total, index + 1)) >< ncf->total) { > - if (!memcmp(ncf->data + size * index, data, size)) { > + if (!data || !memcmp(ncf->data + size * index, data, size)) { Not clear why this check is required? > spin_unlock_irqrestore(>lock, flags); > return index; > } > @@ -639,6 +655,83 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv > *ndp) > nd->state = ncsi_dev_state_functional; > } > > +/* Check the VLAN filter bitmap for a set filter, and construct a > + * "Set VLAN Filter - Disable" packet if found. > + */ > +static int clear_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc, > +struct ncsi_cmd_arg *nca) > +{ > + int index; > + u16 vid; > + > + index = ncsi_find_filter(nc, NCSI_FILTER_VLAN, NULL); > + if (index < 0) { > + /* Filter table empty */ > + return -1; > + } > + > + vid = *(u16 *)ncsi_get_filter(nc, NCSI_FILTER_VLAN, index); You just added this function that returns a pointer to a u32. It's strange to see the only call site then throw half of it away. Also, ncsi_get_filter can return NULL. > + netdev_printk(KERN_DEBUG, ndp->ndev.dev, > + "ncsi: removed vlan tag %u at index %d\n", > + vid, index + 1); > + ncsi_remove_filter(nc, NCSI_FILTER_VLAN, index); > + > + nca->type = NCSI_PKT_CMD_SVF; > + nca->words[1] = vid; > + /* HW filter index starts at 1 */ > + nca->bytes[6] = index + 1; > + nca->bytes[7] = 0x00; > + return 0; > +} > @@ -751,6 +876,26 @@ static void ncsi_configure_channel(struct ncsi_dev_priv > *ndp) > break; > case ncsi_dev_state_config_done: > spin_lock_irqsave(>lock, flags); > + if (nc->reconfigure_needed) { > + /* This channel's configuration has been updated > +* part-way during the config state - start the > +* channel configuration over > +*/ > + nc->reconfigure_needed = false; > + nc->state = NCSI_CHANNEL_INVISIBLE; > + spin_unlock_irqrestore(>lock, flags); > + > + spin_lock_irqsave(>lock, flags); > + nc->state = NCSI_CHANNEL_INACTIVE; This looks strange. What's nc->state up to? Does setting it to NCSI_CHANNEL_INVISIBLE have any affect? The locking is confusing too. > + list_add_tail_rcu(>link, >channel_queue); > + spin_unlock_irqrestore(>lock, flags); > + > + netdev_printk(KERN_DEBUG, dev, > + "Dirty NCSI channel state reset\n"); > + ncsi_process_next_channel(ndp); > + break; > + } > + > if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1) { > hot_nc = nc; > nc->state = NCSI_CHANNEL_ACTIVE; > @@ -1191,6 +1336,149 @@ static struct notifier_block
Re: [PATCH net-next v2 2/3] net/ncsi: Configure VLAN tag filter
On Mon, Aug 14, 2017 at 10:59 AM, Samuel Mendoza-Jonas wrote: > Make use of the ndo_vlan_rx_{add,kill}_vid callbacks to have the NCSI > stack process new VLAN tags and configure the channel VLAN filter > appropriately. > Several VLAN tags can be set and a "Set VLAN Filter" packet must be sent > for each one, meaning the ncsi_dev_state_config_svf state must be > repeated. An internal list of VLAN tags is maintained, and compared > against the current channel's ncsi_channel_filter in order to keep track > within the state. VLAN filters are removed in a similar manner, with the > introduction of the ncsi_dev_state_config_clear_vids state. The maximum > number of VLAN tag filters is determined by the "Get Capabilities" > response from the channel. > > Signed-off-by: Samuel Mendoza-Jonas I've given this some testing, but there are a few things I saw below that we should sort out. > --- a/net/ncsi/ncsi-manage.c > +++ b/net/ncsi/ncsi-manage.c > @@ -38,6 +38,22 @@ static inline int ncsi_filter_size(int table) > return sizes[table]; > } > > +u32 *ncsi_get_filter(struct ncsi_channel *nc, int table, int index) > +{ > + struct ncsi_channel_filter *ncf; > + int size; > + > + ncf = nc->filters[table]; > + if (!ncf) > + return NULL; > + > + size = ncsi_filter_size(table); > + if (size < 0) > + return NULL; > + > + return ncf->data + size * index; > +} > + > int ncsi_find_filter(struct ncsi_channel *nc, int table, void *data) > { > struct ncsi_channel_filter *ncf; > @@ -58,7 +74,7 @@ int ncsi_find_filter(struct ncsi_channel *nc, int table, > void *data) > index = -1; > while ((index = find_next_bit(bitmap, ncf->total, index + 1)) >< ncf->total) { > - if (!memcmp(ncf->data + size * index, data, size)) { > + if (!data || !memcmp(ncf->data + size * index, data, size)) { Not clear why this check is required? > spin_unlock_irqrestore(>lock, flags); > return index; > } > @@ -639,6 +655,83 @@ static void ncsi_suspend_channel(struct ncsi_dev_priv > *ndp) > nd->state = ncsi_dev_state_functional; > } > > +/* Check the VLAN filter bitmap for a set filter, and construct a > + * "Set VLAN Filter - Disable" packet if found. > + */ > +static int clear_one_vid(struct ncsi_dev_priv *ndp, struct ncsi_channel *nc, > +struct ncsi_cmd_arg *nca) > +{ > + int index; > + u16 vid; > + > + index = ncsi_find_filter(nc, NCSI_FILTER_VLAN, NULL); > + if (index < 0) { > + /* Filter table empty */ > + return -1; > + } > + > + vid = *(u16 *)ncsi_get_filter(nc, NCSI_FILTER_VLAN, index); You just added this function that returns a pointer to a u32. It's strange to see the only call site then throw half of it away. Also, ncsi_get_filter can return NULL. > + netdev_printk(KERN_DEBUG, ndp->ndev.dev, > + "ncsi: removed vlan tag %u at index %d\n", > + vid, index + 1); > + ncsi_remove_filter(nc, NCSI_FILTER_VLAN, index); > + > + nca->type = NCSI_PKT_CMD_SVF; > + nca->words[1] = vid; > + /* HW filter index starts at 1 */ > + nca->bytes[6] = index + 1; > + nca->bytes[7] = 0x00; > + return 0; > +} > @@ -751,6 +876,26 @@ static void ncsi_configure_channel(struct ncsi_dev_priv > *ndp) > break; > case ncsi_dev_state_config_done: > spin_lock_irqsave(>lock, flags); > + if (nc->reconfigure_needed) { > + /* This channel's configuration has been updated > +* part-way during the config state - start the > +* channel configuration over > +*/ > + nc->reconfigure_needed = false; > + nc->state = NCSI_CHANNEL_INVISIBLE; > + spin_unlock_irqrestore(>lock, flags); > + > + spin_lock_irqsave(>lock, flags); > + nc->state = NCSI_CHANNEL_INACTIVE; This looks strange. What's nc->state up to? Does setting it to NCSI_CHANNEL_INVISIBLE have any affect? The locking is confusing too. > + list_add_tail_rcu(>link, >channel_queue); > + spin_unlock_irqrestore(>lock, flags); > + > + netdev_printk(KERN_DEBUG, dev, > + "Dirty NCSI channel state reset\n"); > + ncsi_process_next_channel(ndp); > + break; > + } > + > if (nc->modes[NCSI_MODE_LINK].data[2] & 0x1) { > hot_nc = nc; > nc->state = NCSI_CHANNEL_ACTIVE; > @@ -1191,6 +1336,149 @@ static struct notifier_block ncsi_inet6addr_notifier > = { > }; > #endif /*
linux-next: no release today
Hi all, There will be no linux-next release today (I am a bit under the weather). -- Cheers, Stephen Rothwell
linux-next: no release today
Hi all, There will be no linux-next release today (I am a bit under the weather). -- Cheers, Stephen Rothwell
Re: [PATCH v2 1/4] dt-bindings: hwmon: pmbus: Add Maxim MAX31785 documentation
On Thu, 2017-08-10 at 11:13 -0500, Rob Herring wrote: > On Wed, Aug 02, 2017 at 04:45:38PM +0930, Andrew Jeffery wrote: > > > > Signed-off-by: Andrew Jeffery> > --- > > .../devicetree/bindings/hwmon/pmbus/max31785.txt | 126 > > + > > 1 file changed, 126 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt > > > > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt > > b/Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt > > new file mode 100644 > > index ..8ddc4ea1958d > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt > > @@ -0,0 +1,126 @@ > > +Bindings for the Maxim MAX31785 Intelligent Fan Controller > > +== > > This is the second fan controller binding I've seen recently. The other > being hwmon/aspeed-pwm-tacho.txt. I think some of this needs to be > common. There's only so many types of fans, right? Heh, you'd think so, I'll take a look. However much of this is driven by the PMBus specification and the Aspeed PWM/Tacho isn't a PMBus-based device. I was hesitant to start a generic PMBus bindings document without having more PMBus devices with bindings, but maybe that's the way I should go? It would make clear what's from the fan-control parts of the PMBus specification and what's here for the purpose of supporting the MAX31785. > > And we have the thermal binding which you don't appear to tie into. I'll look into that also. > > > + > > +Reference: > > +[1] https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf > > + > > +Required properties: > > +- compatible : One of "maxim,max31785" or "maxim,max31785a" > > +- reg: I2C address, one of 0x52, 0x53, 0x54, 0x55. > > +- #address-cells : Must be 1 > > +- #size-cells: Must be 0 > > + > > +Optional properties: > > +- use-stored-presence: Do not treat the devicetree description as > > canon for > > Is this maxim specific? If so, needs a vendor prefix. PMBus specifies two 8-bit registers of the same structure: FAN_CONFIG_1_2 and FAN_CONFIG_3_4. It's not intended to be manufacturer-specific. A bit in each nibble of FAN_CONFIG_* is specified as marking whether or not the fan is installed. The intent of this property is to define whether the consumer should consider the devicetree as canonical, or the device. In the absence of the property consumer of the node should mark fans described in the devicetree as installed (set the bit, and clear the bit for any fan pages that are not described in the devicetree). Alternatively, the device may be pre-programmed with a default register value that is suitable for the system the controller resides in, in which case the devicetree consumer should itself consume the installed bit from the device rather than set/clear it. The two remaining fields in FAN_CONFIG_*, fan mode (RPM/PWM) and tach-pulses-per-revolution (1-4), are covered by optional properties below. > > > + fan presence (the 'installed' bit of > > FAN_CONFIG_*). > > + Instead, rely on the on the default value store > > of > > + the device to populate it. > > + > > +Capabilities are configured through subnodes of the controller's node. > > + > > +Fans > > + > > + > > +Only fans with subnodes present will be considered as installed. If > > +use-stored-presence is present in the parent node, then only fans that are > > both > > +defined in the devicetree and have their installed bit set are considered > > +installed. > > + > > +Required subnode properties: > > +- compatible : Must be "pmbus-fan" > > What defines a pmbus-fan? Sounds generic, but then you have all these > maxim specific properties. It's driven by the two optional properties, fan-mode and tach-pulses, which make up the remaining fields of the FAN_CONFIG_* registers from the PMBus specification. PMBus reserves command ranges for manufacturer-specific use, and Maxim chose to use one of these reserved commands as MFR_FAN_CONFIG (Manufacturer-specific Fan Configuration). The vendor-prefixed properties deal with the properties described in the 16-bit MFR_FAN_CONFIG register. > > > +- reg: The PMBus page the properties apply to. > > +- maxim,fan-rotor-input : The type of rotor measurement provided to the > > + controller. Must be either "tach" for tachometer > > + pulses or "lock" for a locked-rotor signal. > > +- maxim,fan-lock-polarity: Required iff maxim,fan-rotor-input is "lock". > > Valid > > + values are "low" for active low, "high" for > > active > > + high. > > + > > +Optional subnode properties: > > +- fan-mode : "rpm" or "pwm". Default value is "pwm". > > +- tach-pulses: Tachometer
Re: [PATCH v2 1/4] dt-bindings: hwmon: pmbus: Add Maxim MAX31785 documentation
On Thu, 2017-08-10 at 11:13 -0500, Rob Herring wrote: > On Wed, Aug 02, 2017 at 04:45:38PM +0930, Andrew Jeffery wrote: > > > > Signed-off-by: Andrew Jeffery > > --- > > .../devicetree/bindings/hwmon/pmbus/max31785.txt | 126 > > + > > 1 file changed, 126 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt > > > > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt > > b/Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt > > new file mode 100644 > > index ..8ddc4ea1958d > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt > > @@ -0,0 +1,126 @@ > > +Bindings for the Maxim MAX31785 Intelligent Fan Controller > > +== > > This is the second fan controller binding I've seen recently. The other > being hwmon/aspeed-pwm-tacho.txt. I think some of this needs to be > common. There's only so many types of fans, right? Heh, you'd think so, I'll take a look. However much of this is driven by the PMBus specification and the Aspeed PWM/Tacho isn't a PMBus-based device. I was hesitant to start a generic PMBus bindings document without having more PMBus devices with bindings, but maybe that's the way I should go? It would make clear what's from the fan-control parts of the PMBus specification and what's here for the purpose of supporting the MAX31785. > > And we have the thermal binding which you don't appear to tie into. I'll look into that also. > > > + > > +Reference: > > +[1] https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf > > + > > +Required properties: > > +- compatible : One of "maxim,max31785" or "maxim,max31785a" > > +- reg: I2C address, one of 0x52, 0x53, 0x54, 0x55. > > +- #address-cells : Must be 1 > > +- #size-cells: Must be 0 > > + > > +Optional properties: > > +- use-stored-presence: Do not treat the devicetree description as > > canon for > > Is this maxim specific? If so, needs a vendor prefix. PMBus specifies two 8-bit registers of the same structure: FAN_CONFIG_1_2 and FAN_CONFIG_3_4. It's not intended to be manufacturer-specific. A bit in each nibble of FAN_CONFIG_* is specified as marking whether or not the fan is installed. The intent of this property is to define whether the consumer should consider the devicetree as canonical, or the device. In the absence of the property consumer of the node should mark fans described in the devicetree as installed (set the bit, and clear the bit for any fan pages that are not described in the devicetree). Alternatively, the device may be pre-programmed with a default register value that is suitable for the system the controller resides in, in which case the devicetree consumer should itself consume the installed bit from the device rather than set/clear it. The two remaining fields in FAN_CONFIG_*, fan mode (RPM/PWM) and tach-pulses-per-revolution (1-4), are covered by optional properties below. > > > + fan presence (the 'installed' bit of > > FAN_CONFIG_*). > > + Instead, rely on the on the default value store > > of > > + the device to populate it. > > + > > +Capabilities are configured through subnodes of the controller's node. > > + > > +Fans > > + > > + > > +Only fans with subnodes present will be considered as installed. If > > +use-stored-presence is present in the parent node, then only fans that are > > both > > +defined in the devicetree and have their installed bit set are considered > > +installed. > > + > > +Required subnode properties: > > +- compatible : Must be "pmbus-fan" > > What defines a pmbus-fan? Sounds generic, but then you have all these > maxim specific properties. It's driven by the two optional properties, fan-mode and tach-pulses, which make up the remaining fields of the FAN_CONFIG_* registers from the PMBus specification. PMBus reserves command ranges for manufacturer-specific use, and Maxim chose to use one of these reserved commands as MFR_FAN_CONFIG (Manufacturer-specific Fan Configuration). The vendor-prefixed properties deal with the properties described in the 16-bit MFR_FAN_CONFIG register. > > > +- reg: The PMBus page the properties apply to. > > +- maxim,fan-rotor-input : The type of rotor measurement provided to the > > + controller. Must be either "tach" for tachometer > > + pulses or "lock" for a locked-rotor signal. > > +- maxim,fan-lock-polarity: Required iff maxim,fan-rotor-input is "lock". > > Valid > > + values are "low" for active low, "high" for > > active > > + high. > > + > > +Optional subnode properties: > > +- fan-mode : "rpm" or "pwm". Default value is "pwm". > > +- tach-pulses: Tachometer pulses per
Re: [PATCH v3 10/13] ARM: dts: rockchip: add saradc support for rv1108
Hi Heiko: On 2017年08月13日 20:13, Heiko Stuebner wrote: Hi Andy, Am Freitag, 11. August 2017, 11:46:51 CEST schrieb Andy Yan: Add saradc device tree node for rv1108 soc Signed-off-by: Andy Yan+ adc: adc@1038c000 { + compatible = "rockchip,rv1108-saradc", "rockchip,rk3399-saradc"; + reg = <0x1038c000 0x100>; + interrupts = ; + #io-channel-cells = <1>; + io-channel-ranges; What do you need the io-channel-ranges for? I.e. according to the documenation it is meant for when the adc is also a "bus node" and has children that should inherit channels from it. In all saradc uses so far we only have separate nodes referencing the adc controller (like adc-keys etc), so this does not look like it is needed? Sorry, I just copied it from the downstream dts. After checked with David, he suggested that we could remove it. Heiko
Re: [PATCH v3 10/13] ARM: dts: rockchip: add saradc support for rv1108
Hi Heiko: On 2017年08月13日 20:13, Heiko Stuebner wrote: Hi Andy, Am Freitag, 11. August 2017, 11:46:51 CEST schrieb Andy Yan: Add saradc device tree node for rv1108 soc Signed-off-by: Andy Yan + adc: adc@1038c000 { + compatible = "rockchip,rv1108-saradc", "rockchip,rk3399-saradc"; + reg = <0x1038c000 0x100>; + interrupts = ; + #io-channel-cells = <1>; + io-channel-ranges; What do you need the io-channel-ranges for? I.e. according to the documenation it is meant for when the adc is also a "bus node" and has children that should inherit channels from it. In all saradc uses so far we only have separate nodes referencing the adc controller (like adc-keys etc), so this does not look like it is needed? Sorry, I just copied it from the downstream dts. After checked with David, he suggested that we could remove it. Heiko
Re: [PATCH v2 1/7] zram: set BDI_CAP_STABLE_WRITES once
On (08/11/17 14:17), Minchan Kim wrote: > [1] fixed weird thing(i.e., reset BDI_CAP_STABLE_WRITES flag > unconditionally whenever revalidat_disk is called) so zram doesn't > need to reset the flag any more whenever revalidating the bdev. > Instead, set the flag just once when the zram device is created. > > It shouldn't change any behavior. > > [1] 19b7ccf8651d, block: get rid of blk_integrity_revalidate() > Cc: Senozhatsky> Cc: Ilya Dryomov > Signed-off-by: Minchan Kim Reviewed-by: Sergey Senozhatsky -ss
Re: [PATCH v2 1/7] zram: set BDI_CAP_STABLE_WRITES once
On (08/11/17 14:17), Minchan Kim wrote: > [1] fixed weird thing(i.e., reset BDI_CAP_STABLE_WRITES flag > unconditionally whenever revalidat_disk is called) so zram doesn't > need to reset the flag any more whenever revalidating the bdev. > Instead, set the flag just once when the zram device is created. > > It shouldn't change any behavior. > > [1] 19b7ccf8651d, block: get rid of blk_integrity_revalidate() > Cc: Senozhatsky > Cc: Ilya Dryomov > Signed-off-by: Minchan Kim Reviewed-by: Sergey Senozhatsky -ss
Re: [PATCH v2 2/2] i2c: mediatek: Add i2c compatible for MediaTek MT7622
On Sat, 2017-08-12 at 16:44 +0200, Wolfram Sang wrote: > > +static const struct i2c_adapter_quirks mt7622_i2c_quirks = { > > + .max_num_msgs = 255, > > + .max_write_len = 65535, > > + .max_read_len = 65535, > > + .max_comb_1st_msg_len = 65535, > > + .max_comb_2nd_msg_len = 65535, > > +}; > > That looks like no quirks? Then just leave the quirks pointer below > empty. > Compare to MT8173 i2c controller, MT7622 limits message numbers to 255. Jun
Re: [PATCH v2 2/2] i2c: mediatek: Add i2c compatible for MediaTek MT7622
On Sat, 2017-08-12 at 16:44 +0200, Wolfram Sang wrote: > > +static const struct i2c_adapter_quirks mt7622_i2c_quirks = { > > + .max_num_msgs = 255, > > + .max_write_len = 65535, > > + .max_read_len = 65535, > > + .max_comb_1st_msg_len = 65535, > > + .max_comb_2nd_msg_len = 65535, > > +}; > > That looks like no quirks? Then just leave the quirks pointer below > empty. > Compare to MT8173 i2c controller, MT7622 limits message numbers to 255. Jun