Re: [PATCH v2 09/18] iio: afe: iio-rescale: Simplify with dev_err_probe()
On 2020-08-28 08:24, Krzysztof Kozlowski wrote: > On Thu, Aug 27, 2020 at 11:46:40PM +0200, Peter Rosin wrote: >> On 2020-08-27 21:26, Krzysztof Kozlowski wrote: >>> Common pattern of handling deferred probe can be simplified with >>> dev_err_probe(). Less code and also it prints the error value. >>> >>> Signed-off-by: Krzysztof Kozlowski >>> >>> --- >>> >>> Changes since v1: >>> 1. Wrap dev_err_probe() lines at 100 character >>> --- >>> drivers/iio/afe/iio-rescale.c | 7 ++- >>> 1 file changed, 2 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c >>> index 69c0f277ada0..8cd9645c50e8 100644 >>> --- a/drivers/iio/afe/iio-rescale.c >>> +++ b/drivers/iio/afe/iio-rescale.c >>> @@ -276,11 +276,8 @@ static int rescale_probe(struct platform_device *pdev) >>> int ret; >>> >>> source = devm_iio_channel_get(dev, NULL); >>> - if (IS_ERR(source)) { >>> - if (PTR_ERR(source) != -EPROBE_DEFER) >>> - dev_err(dev, "failed to get source channel\n"); >>> - return PTR_ERR(source); >>> - } >>> + if (IS_ERR(source)) >>> + return dev_err_probe(dev, PTR_ERR(source), "failed to get >>> source channel\n"); >> >> Hi! >> >> Sorry to be a pain...but... >> >> I'm not a huge fan of adding *one* odd line breaking the 80 column >> recommendation to any file. I like to be able to fit multiple >> windows side by side in a meaningful way. Also, I don't like having >> a shitload of emptiness on my screen, which is what happens when some >> lines are longer and you want to see it all. I strongly believe that >> the 80 column rule/recommendation is still as valid as it ever was. >> It's just hard to read longish lines; there's a reason newspapers >> columns are quite narrow... >> >> Same comment for the envelope-detector (3/18). >> >> You will probably never look at these files again, but *I* might have >> to revisit them for one reason or another, and these long lines will >> annoy me when that happens. > > Initially I posted it with 80-characters wrap. Then I received a comment > - better to stick to the new 100, as checkpatch accepts it. > > Now you write, better to go back to 80. > > Maybe then someone else will write to me, better to go to 100. > > And another person will reply, no, coding style still mentions 80, so > keep it at 80. > > Sure guys, please first decide which one you prefer, then I will wrap it > accordingly. :) > > Otherwise I will just jump from one to another depending on one person's > personal preference. > > If there is no consensus among discussing people, I find this 100 line > more readable, already got review, checkpatch accepts it so if subsystem > maintainer likes it, I prefer to leave it like this. I'm not impressed by that argument. For the files I have mentioned, it does not matter very much to me if you and some random person think that 100 columns might *slightly* improve readability. Quoting coding-style Statements longer than 80 columns should be broken into sensible chunks, unless exceeding 80 columns significantly increases readability and does not hide information. Notice that word? *significantly* Why do I even have to speak up about this? WTF? For the patches that touch files that I originally wrote [1], my preference should be clear by now. Cheers, Peter [1] drivers/iio/adc/envelope-detector.c drivers/iio/afe/iio-rescale.c drivers/iio/dac/dpot-dac.c drivers/iio/multiplexer/iio-mux.c >> You did wrap the lines for dpot-dac (12/18) - which is excellent - but >> that makes me curious as to what the difference is? > > Didn't fit into limit of 100.
Re: [PATCH] Remove unneeded variable t1
Xu Wang wrote: > Remove unneeded variable t1 seed_encrypt() and > seed_decrypt(). > > Signed-off-by: Xu Wang > --- > crypto/seed.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/crypto/seed.c b/crypto/seed.c > index 5e3bef3a617d..69b3058d6a32 100644 > --- a/crypto/seed.c > +++ b/crypto/seed.c > @@ -366,7 +366,7 @@ static void seed_encrypt(struct crypto_tfm *tfm, u8 *out, > const u8 *in) >const struct seed_ctx *ctx = crypto_tfm_ctx(tfm); >const __be32 *src = (const __be32 *)in; >__be32 *dst = (__be32 *)out; > - u32 x1, x2, x3, x4, t0, t1; > + u32 x1, x2, x3, x4, t0; >const u32 *ks = ctx->keysched; > >x1 = be32_to_cpu(src[0]); > @@ -404,7 +404,7 @@ static void seed_decrypt(struct crypto_tfm *tfm, u8 *out, > const u8 *in) >const struct seed_ctx *ctx = crypto_tfm_ctx(tfm); >const __be32 *src = (const __be32 *)in; >__be32 *dst = (__be32 *)out; > - u32 x1, x2, x3, x4, t0, t1; > + u32 x1, x2, x3, x4, t0; >const u32 *ks = ctx->keysched; > >x1 = be32_to_cpu(src[0]); This doesn't even compile! -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v3] mm: Fix kthread_use_mm() vs TLB invalidate
Excerpts from Nicholas Piggin's message of August 28, 2020 1:26 pm: > Excerpts from pet...@infradead.org's message of August 21, 2020 11:04 pm: >> On Fri, Aug 21, 2020 at 11:09:51AM +0530, Aneesh Kumar K.V wrote: >>> Peter Zijlstra writes: >>> >>> > For SMP systems using IPI based TLB invalidation, looking at >>> > current->active_mm is entirely reasonable. This then presents the >>> > following race condition: >>> > >>> > >>> > CPU0CPU1 >>> > >>> > flush_tlb_mm(mm)use_mm(mm) >>> > >>> > tsk->active_mm = mm; >>> > >>> > if (tsk->active_mm == mm) >>> > // flush TLBs >>> > >>> > switch_mm(old_mm,mm,tsk); >>> > >>> > >>> > Where it is possible the IPI flushed the TLBs for @old_mm, not @mm, >>> > because the IPI lands before we actually switched. >>> > >>> > Avoid this by disabling IRQs across changing ->active_mm and >>> > switch_mm(). >>> > >>> > [ There are all sorts of reasons this might be harmless for various >>> > architecture specific reasons, but best not leave the door open at >>> > all. ] >>> >>> >>> Do we have similar race with exec_mmap()? I am looking at exec_mmap() >>> runnning parallel to do_exit_flush_lazy_tlb(). We can get >>> >>> if (current->active_mm == mm) { >>> >>> true and if we don't disable irq around updating tsk->mm/active_mm we >>> can end up doing mmdrop on wrong mm? >> >> exec_mmap() is called after de_thread(), there should not be any mm >> specific invalidations around I think. >> >> Then again, CLONE_VM without CLONE_THREAD might still be possible, so >> yeah, we probably want IRQs disabled there too, just for consistency and >> general paranoia if nothing else. > > The problem is probably not this TLB flushing race, but I think there > is a lazy tlb race. Hmm, is it possible for something to be holding the mm_users when we exec? That could actually make it a problem for TLB flushing too. Thanks, Nick
[PATCH] clk: imx: fix i.MX7D peripheral clk mux flags
From: Peng Fan According to RM, Page 574, Chapter 5.2.6.4.3 Peripheral clock slice, "IP clock slices must be stopped to change the clock source.". So we must have CLK_SET_PARENT_GATE flag to avoid glitch. Signed-off-by: Peng Fan --- drivers/clk/imx/clk-imx7d.c | 131 ++-- 1 file changed, 66 insertions(+), 65 deletions(-) diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c index 6197710ee8c5..22d24a6a05e7 100644 --- a/drivers/clk/imx/clk-imx7d.c +++ b/drivers/clk/imx/clk-imx7d.c @@ -506,72 +506,73 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node) hws[IMX7D_ARM_M4_ROOT_SRC] = imx_clk_hw_mux2("arm_m4_src", base + 0x8080, 24, 3, arm_m4_sel, ARRAY_SIZE(arm_m4_sel)); hws[IMX7D_MAIN_AXI_ROOT_SRC] = imx_clk_hw_mux2("axi_src", base + 0x8800, 24, 3, axi_sel, ARRAY_SIZE(axi_sel)); hws[IMX7D_DISP_AXI_ROOT_SRC] = imx_clk_hw_mux2("disp_axi_src", base + 0x8880, 24, 3, disp_axi_sel, ARRAY_SIZE(disp_axi_sel)); - hws[IMX7D_ENET_AXI_ROOT_SRC] = imx_clk_hw_mux2("enet_axi_src", base + 0x8900, 24, 3, enet_axi_sel, ARRAY_SIZE(enet_axi_sel)); - hws[IMX7D_NAND_USDHC_BUS_ROOT_SRC] = imx_clk_hw_mux2("nand_usdhc_src", base + 0x8980, 24, 3, nand_usdhc_bus_sel, ARRAY_SIZE(nand_usdhc_bus_sel)); hws[IMX7D_AHB_CHANNEL_ROOT_SRC] = imx_clk_hw_mux2("ahb_src", base + 0x9000, 24, 3, ahb_channel_sel, ARRAY_SIZE(ahb_channel_sel)); - hws[IMX7D_DRAM_PHYM_ROOT_SRC] = imx_clk_hw_mux2("dram_phym_src", base + 0x9800, 24, 1, dram_phym_sel, ARRAY_SIZE(dram_phym_sel)); - hws[IMX7D_DRAM_ROOT_SRC] = imx_clk_hw_mux2("dram_src", base + 0x9880, 24, 1, dram_sel, ARRAY_SIZE(dram_sel)); - hws[IMX7D_DRAM_PHYM_ALT_ROOT_SRC] = imx_clk_hw_mux2("dram_phym_alt_src", base + 0xa000, 24, 3, dram_phym_alt_sel, ARRAY_SIZE(dram_phym_alt_sel)); - hws[IMX7D_DRAM_ALT_ROOT_SRC] = imx_clk_hw_mux2("dram_alt_src", base + 0xa080, 24, 3, dram_alt_sel, ARRAY_SIZE(dram_alt_sel)); - hws[IMX7D_USB_HSIC_ROOT_SRC] = imx_clk_hw_mux2("usb_hsic_src", base + 0xa100, 24, 3, usb_hsic_sel, ARRAY_SIZE(usb_hsic_sel)); - hws[IMX7D_PCIE_CTRL_ROOT_SRC] = imx_clk_hw_mux2("pcie_ctrl_src", base + 0xa180, 24, 3, pcie_ctrl_sel, ARRAY_SIZE(pcie_ctrl_sel)); - hws[IMX7D_PCIE_PHY_ROOT_SRC] = imx_clk_hw_mux2("pcie_phy_src", base + 0xa200, 24, 3, pcie_phy_sel, ARRAY_SIZE(pcie_phy_sel)); - hws[IMX7D_EPDC_PIXEL_ROOT_SRC] = imx_clk_hw_mux2("epdc_pixel_src", base + 0xa280, 24, 3, epdc_pixel_sel, ARRAY_SIZE(epdc_pixel_sel)); - hws[IMX7D_LCDIF_PIXEL_ROOT_SRC] = imx_clk_hw_mux2("lcdif_pixel_src", base + 0xa300, 24, 3, lcdif_pixel_sel, ARRAY_SIZE(lcdif_pixel_sel)); - hws[IMX7D_MIPI_DSI_ROOT_SRC] = imx_clk_hw_mux2("mipi_dsi_src", base + 0xa380, 24, 3, mipi_dsi_sel, ARRAY_SIZE(mipi_dsi_sel)); - hws[IMX7D_MIPI_CSI_ROOT_SRC] = imx_clk_hw_mux2("mipi_csi_src", base + 0xa400, 24, 3, mipi_csi_sel, ARRAY_SIZE(mipi_csi_sel)); - hws[IMX7D_MIPI_DPHY_ROOT_SRC] = imx_clk_hw_mux2("mipi_dphy_src", base + 0xa480, 24, 3, mipi_dphy_sel, ARRAY_SIZE(mipi_dphy_sel)); - hws[IMX7D_SAI1_ROOT_SRC] = imx_clk_hw_mux2("sai1_src", base + 0xa500, 24, 3, sai1_sel, ARRAY_SIZE(sai1_sel)); - hws[IMX7D_SAI2_ROOT_SRC] = imx_clk_hw_mux2("sai2_src", base + 0xa580, 24, 3, sai2_sel, ARRAY_SIZE(sai2_sel)); - hws[IMX7D_SAI3_ROOT_SRC] = imx_clk_hw_mux2("sai3_src", base + 0xa600, 24, 3, sai3_sel, ARRAY_SIZE(sai3_sel)); - hws[IMX7D_SPDIF_ROOT_SRC] = imx_clk_hw_mux2("spdif_src", base + 0xa680, 24, 3, spdif_sel, ARRAY_SIZE(spdif_sel)); - hws[IMX7D_ENET1_REF_ROOT_SRC] = imx_clk_hw_mux2("enet1_ref_src", base + 0xa700, 24, 3, enet1_ref_sel, ARRAY_SIZE(enet1_ref_sel)); - hws[IMX7D_ENET1_TIME_ROOT_SRC] = imx_clk_hw_mux2("enet1_time_src", base + 0xa780, 24, 3, enet1_time_sel, ARRAY_SIZE(enet1_time_sel)); - hws[IMX7D_ENET2_REF_ROOT_SRC] = imx_clk_hw_mux2("enet2_ref_src", base + 0xa800, 24, 3, enet2_ref_sel, ARRAY_SIZE(enet2_ref_sel)); - hws[IMX7D_ENET2_TIME_ROOT_SRC] = imx_clk_hw_mux2("enet2_time_src", base + 0xa880, 24, 3, enet2_time_sel, ARRAY_SIZE(enet2_time_sel)); - hws[IMX7D_ENET_PHY_REF_ROOT_SRC] = imx_clk_hw_mux2("enet_phy_ref_src", base + 0xa900, 24, 3, enet_phy_ref_sel, ARRAY_SIZE(enet_phy_ref_sel)); - hws[IMX7D_EIM_ROOT_SRC] = imx_clk_hw_mux2("eim_src", base + 0xa980, 24, 3, eim_sel, ARRAY_SIZE(eim_sel)); - hws[IMX7D_NAND_ROOT_SRC] = imx_clk_hw_mux2("nand_src", base + 0xaa00, 24, 3, nand_sel, ARRAY_SIZE(nand_sel)); - hws[IMX7D_QSPI_ROOT_SRC] = imx_clk_hw_mux2("qspi_src", base + 0xaa80, 24, 3, qspi_sel, ARRAY_SIZE(qspi_sel)); - hws[IMX7D_USDHC1_ROOT_SRC] = imx_clk_hw_mux2("usdhc1_src", base + 0xab00, 24, 3, usdhc1_sel, ARRAY_SIZE(usdhc1_sel)); - hws[IMX7D_USDHC2_ROOT_SRC] = imx_clk_hw_mux2("usdhc2_src", base + 0xab80, 24, 3, usdhc2_sel, ARRAY_SIZE(usdhc2_sel)); - hws[IMX7D_USDHC3_ROOT_SRC] = imx_clk_hw_mux2("usdhc3_src", base + 0xac00, 24, 3, usdhc3
Re: [PATCH 1/7] soundwire: bus: use property to set interrupt masks
Hi Mark, On 18-08-20, 22:06, Bard Liao wrote: > From: Pierre-Louis Bossart > > Add a slave-level property and program the SCP_INT1_MASK as desired by > the codec driver. Since there is no DisCo property this has to be an > implementation-specific firmware property or hard-coded in the driver. > > The only functionality change is that implementation-defined > interrupts are no longer set for amplifiers - those interrupts are > typically for jack detection or acoustic event detection/hotwording. > > Tested-by: Srinivas Kandagatla > Signed-off-by: Pierre-Louis Bossart > Reviewed-by: Kai Vehmanen > Reviewed-by: Guennadi Liakhovetski > Signed-off-by: Bard Liao > --- > drivers/soundwire/bus.c | 12 ++-- > include/linux/soundwire/sdw.h | 2 ++ > sound/soc/codecs/max98373-sdw.c | 3 +++ > sound/soc/codecs/rt1308-sdw.c | 2 ++ > sound/soc/codecs/rt5682-sdw.c | 4 > sound/soc/codecs/rt700-sdw.c| 4 > sound/soc/codecs/rt711-sdw.c| 4 > sound/soc/codecs/rt715-sdw.c| 4 > sound/soc/codecs/wsa881x.c | 1 + This touches codecs, can you Ack it please Ideally this should have been split up to header, the codec updates and finally the bus change! > 9 files changed, 30 insertions(+), 6 deletions(-) > > diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c > index e6e0fb9a81b4..3b6a87bc254e 100644 > --- a/drivers/soundwire/bus.c > +++ b/drivers/soundwire/bus.c > @@ -1184,13 +1184,13 @@ static int sdw_initialize_slave(struct sdw_slave > *slave) > return ret; > > /* > - * Set bus clash, parity and SCP implementation > - * defined interrupt mask > - * TODO: Read implementation defined interrupt mask > - * from Slave property > + * Set SCP_INT1_MASK register, typically bus clash and > + * implementation-defined interrupt mask. The Parity detection > + * may not always be correct on startup so its use is > + * device-dependent, it might e.g. only be enabled in > + * steady-state after a couple of frames. >*/ > - val = SDW_SCP_INT1_IMPL_DEF | SDW_SCP_INT1_BUS_CLASH | > - SDW_SCP_INT1_PARITY; > + val = slave->prop.scp_int1_mask; > > /* Enable SCP interrupts */ > ret = sdw_update(slave, SDW_SCP_INTMASK1, val, val); > diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h > index 76052f12c9f7..6d91f2ca20b2 100644 > --- a/include/linux/soundwire/sdw.h > +++ b/include/linux/soundwire/sdw.h > @@ -355,6 +355,7 @@ struct sdw_dpn_prop { > * @dp0_prop: Data Port 0 properties > * @src_dpn_prop: Source Data Port N properties > * @sink_dpn_prop: Sink Data Port N properties > + * @scp_int1_mask: SCP_INT1_MASK desired settings > */ > struct sdw_slave_prop { > u32 mipi_revision; > @@ -376,6 +377,7 @@ struct sdw_slave_prop { > struct sdw_dp0_prop *dp0_prop; > struct sdw_dpn_prop *src_dpn_prop; > struct sdw_dpn_prop *sink_dpn_prop; > + u8 scp_int1_mask; > }; > > /** > diff --git a/sound/soc/codecs/max98373-sdw.c b/sound/soc/codecs/max98373-sdw.c > index 5fe724728e84..17fd1989e873 100644 > --- a/sound/soc/codecs/max98373-sdw.c > +++ b/sound/soc/codecs/max98373-sdw.c > @@ -15,6 +15,7 @@ > #include > #include > #include > +#include > #include "max98373.h" > #include "max98373-sdw.h" > > @@ -287,6 +288,8 @@ static int max98373_read_prop(struct sdw_slave *slave) > unsigned long addr; > struct sdw_dpn_prop *dpn; > > + prop->scp_int1_mask = SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY; > + > /* BITMAP: 1000 Dataport 3 is active */ > prop->source_ports = BIT(3); > /* BITMAP: 0010 Dataport 1 is active */ > diff --git a/sound/soc/codecs/rt1308-sdw.c b/sound/soc/codecs/rt1308-sdw.c > index b0ba0d2acbdd..5cf10fd447eb 100644 > --- a/sound/soc/codecs/rt1308-sdw.c > +++ b/sound/soc/codecs/rt1308-sdw.c > @@ -123,6 +123,8 @@ static int rt1308_read_prop(struct sdw_slave *slave) > unsigned long addr; > struct sdw_dpn_prop *dpn; > > + prop->scp_int1_mask = SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY; > + > prop->paging_support = true; > > /* first we need to allocate memory for set bits in port lists */ > diff --git a/sound/soc/codecs/rt5682-sdw.c b/sound/soc/codecs/rt5682-sdw.c > index 94bf6bee78e6..544073975020 100644 > --- a/sound/soc/codecs/rt5682-sdw.c > +++ b/sound/soc/codecs/rt5682-sdw.c > @@ -19,6 +19,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -542,6 +543,9 @@ static int rt5682_read_prop(struct sdw_slave *slave) > unsigned long addr; > struct sdw_dpn_prop *dpn; > > + prop->scp_int1_mask = SDW_SCP_INT1_IMPL_DEF | SDW_SCP_INT1_BUS_CLASH | > + SDW_SCP_INT1_PARITY; > + > prop->paging_support = false; > > /* first we need to allocate memory for set bits in port lists */ > diff --git a/sound/soc/c
Re: [PATCH v5 02/18] crypto: sun8i-ss: Add support for the PRNG
On Fri, Aug 21, 2020 at 01:43:19PM +, Corentin Labbe wrote: > > + err = pm_runtime_get_sync(ss->dev); > + if (err < 0) > + goto err_pm; > + err = 0; The error case needs to do this: https://patchwork.kernel.org/patch/11728595/ Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH 2/2] ASoC: wm8994: Ensure the device is resumed in wm89xx_mic_detect functions
On Thu, Aug 27, 2020 at 07:33:57PM +0200, Sylwester Nawrocki wrote: > When the wm8958_mic_detect, wm8994_mic_detect functions get called from > the machine driver, e.g. from the card's late_probe() callback, the CODEC > device may be PM runtime suspended and any regmap writes have no effect. > Add PM runtime calls to these functions to ensure the device registers > are updated as expected. > This suppresses an error during boot > "wm8994-codec: ASoC: error at snd_soc_component_update_bits on wm8994-codec" > caused by the regmap access error due to the cache_only flag being set. > > Signed-off-by: Sylwester Nawrocki > --- > sound/soc/codecs/wm8994.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c > index b3ba053..fc9ea19 100644 > --- a/sound/soc/codecs/wm8994.c > +++ b/sound/soc/codecs/wm8994.c > @@ -3514,6 +3514,8 @@ int wm8994_mic_detect(struct snd_soc_component > *component, struct snd_soc_jack * > return -EINVAL; > } > > + pm_runtime_get_sync(component->dev); The driver enables PM runtime unconditionally so you should probably handle the error code here. I know that driver does not do it in other cases but it should not be a reason to multiple this pattern... unless it really does not matter as there are no runtime PM ops? Best regards, Krzysztof
Re: [PATCH] sched/numa: use runnable_avg to classify node
On Thu, 27 Aug 2020 at 20:22, Mel Gorman wrote: > > On Thu, Aug 27, 2020 at 05:43:11PM +0200, Vincent Guittot wrote: > > > The testing was a mixed bag of wins and losses but wins more than it > > > loses. Biggest loss was a 9.04% regression on nas-SP using openmp for > > > parallelisation on Zen1. Biggest win was around 8% gain running > > > specjbb2005 on Zen2 (with some major gains of up to 55% for some thread > > > counts). Most workloads were stable across multiple Intel and AMD > > > machines. > > > > > > There were some oddities in changes in NUMA scanning rate but that is > > > likely a side-effect because the locality over time for the same loads > > > did not look obviously worse. There was no negative result I could point > > > at that was not offset by a positive result elsewhere. Given it's not > > > a univeral win or loss, matching numa and lb balancing as closely as > > > possible is best so > > > > > > Reviewed-by: Mel Gorman > > > > Thanks. > > > > I will try to reproduce the nas-SP test on my setup to see what is going one > > > > You can try but you might be chasing ghosts. Please note that this nas-SP > observation was only on zen1 and only for C-class and OMP. The other > machines tested for the same class and OMP were fine (including zen2). Even > D-class on the same machine with OMP was fine as was MPI in both cases. The > bad result indicated that NUMA scanning and faulting was higher but that > is more likely to be a problem with NUMA balancing than your patch. > > In the five iterations, two iterations showed a large spike in scan rate > towards the end of an iteration but not the other three. The scan rate > was also not consistently high so there is a degree of luck involved with > SP specifically and there is not a consistently penalty as a result of > your patch. > > The only thing to be aware of is that this patch might show up in > bisections once it's merged for both performance gains and losses. Thanks for the detailed explanation. I will save my time and continue on the fairness problem in this case. Vincent > > -- > Mel Gorman > SUSE Labs
Re: [PATCH v5 03/18] crypto: sun8i-ss: support hash algorithms
On Fri, Aug 21, 2020 at 01:43:20PM +, Corentin Labbe wrote: > > + err = pm_runtime_get_sync(op->ss->dev); > + if (err < 0) > + goto error_pm; > + return 0; You need to handle the error case like this: https://patchwork.kernel.org/patch/11728595/ Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH 1/2] ASoC: wm8994: Skip setting of the WM8994_MICBIAS register for WM1811
On Thu, Aug 27, 2020 at 07:33:56PM +0200, Sylwester Nawrocki wrote: > The WM8994_MICBIAS register is not available in the WM1811 CODEC so skip > initialization of that register for that device. > This suppresses an error during boot: > "wm8994-codec: ASoC: error at snd_soc_component_update_bits on wm8994-codec" > > Signed-off-by: Sylwester Nawrocki > --- > sound/soc/codecs/wm8994.c | 2 ++ > sound/soc/codecs/wm_hubs.c | 3 +++ > sound/soc/codecs/wm_hubs.h | 1 + > 3 files changed, 6 insertions(+) Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH 3/3] clk: samsung: Use cached clk_hws instead of __clk_lookup() calls
On Wed, Aug 26, 2020 at 07:15:29PM +0200, Sylwester Nawrocki wrote: > For the CPU clock registration two parent clocks are required, these > are now being passed as struct clk_hw pointers, rather than by the > global scope names. That allows us to avoid __clk_lookup() calls > and simplifies a bit the CPU clock registration function. > While at it drop unneeded extern keyword in the function declaration. > > Signed-off-by: Sylwester Nawrocki > --- > drivers/clk/samsung/clk-cpu.c| 37 > +++- > drivers/clk/samsung/clk-cpu.h| 6 +++--- > drivers/clk/samsung/clk-exynos3250.c | 6 -- > drivers/clk/samsung/clk-exynos4.c| 7 +-- > drivers/clk/samsung/clk-exynos5250.c | 4 +++- > drivers/clk/samsung/clk-exynos5420.c | 6 +++--- > drivers/clk/samsung/clk-exynos5433.c | 10 -- > 7 files changed, 41 insertions(+), 35 deletions(-) Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v2] x86: Use xorl %0,%0 in __get_user_asm
On Thu, Aug 27, 2020 at 10:14 PM Al Viro wrote: > > On Thu, Aug 27, 2020 at 08:09:04PM +0200, Uros Bizjak wrote: > > xorl %0,%0 is equivalent to xorq %0,%0 as both will zero the > > entire register. Use xorl %0,%0 for all operand sizes to avoid > > REX prefix byte when legacy registers are used and to avoid size > > prefix byte when 16bit registers are used. > > > > Zeroing the full register is OK in this use case. xorl %0,%0 also > > breaks register dependency chains, avoiding potential partial > > register stalls with 8 and 16bit operands. > > No objections, but talking about stalls is more than slightly > ridiculous - we'd just taken a #PF, failed there, flipped > pt_regs %rip to fixup section, returned from fault and are > about to fail whatever syscall that had been; a stall here > is really not an issue... Should I submit a v3 with the offending sentence removed, or could I just ask a committer to remove it on the fly? Uros.
[PATCH] In the device tree file openbmc-flash-layout.dtsi, rofs partition offset is defined as 0x4c0000, while its node name is "rofs@c0000" which is a typo. It should be "rofs@4c0000".
ARM: dts: openbmc-flash-layout: Fix a typo of rofs offset Signed-off-by: Kun Zhao --- arch/arm/boot/dts/openbmc-flash-layout.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/openbmc-flash-layout.dtsi b/arch/arm/boot/dts/openbmc-flash-layout.dtsi index 6c26524e93e1..b47e14063c38 100644 --- a/arch/arm/boot/dts/openbmc-flash-layout.dtsi +++ b/arch/arm/boot/dts/openbmc-flash-layout.dtsi @@ -20,7 +20,7 @@ kernel@8 { label = "kernel"; }; - rofs@c { + rofs@4c { reg = <0x4c 0x174>; label = "rofs"; }; -- 2.28.0
Re: [PATCH v2 3/5] fpga manager: xilinx-spi: rework write_complete loop implementation
Hi Tom, On 27/08/20 21:34, Tom Rix wrote: > > On 8/27/20 12:26 PM, Luca Ceresoli wrote: >> Hi Tom, >> >> thanks for the prompt feedback! >> >> On 27/08/20 20:59, Tom Rix wrote: >>> On 8/27/20 7:32 AM, Luca Ceresoli wrote: In preparation to add error checking for gpiod_get_value(), rework the loop to avoid the duplication of these lines: if (gpiod_get_value(conf->done)) return xilinx_spi_apply_cclk_cycles(conf); There is little advantage in this rework with current code. However error checking will expand these two lines to five, making code duplication more annoying. Signed-off-by: Luca Ceresoli --- This patch is new in v2 --- drivers/fpga/xilinx-spi.c | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/fpga/xilinx-spi.c b/drivers/fpga/xilinx-spi.c index 01f494172379..cfc933d70f52 100644 --- a/drivers/fpga/xilinx-spi.c +++ b/drivers/fpga/xilinx-spi.c @@ -151,22 +151,19 @@ static int xilinx_spi_write_complete(struct fpga_manager *mgr, struct fpga_image_info *info) { struct xilinx_spi_conf *conf = mgr->priv; - unsigned long timeout; + unsigned long timeout = jiffies + usecs_to_jiffies(info->config_complete_timeout_us); int ret; - if (gpiod_get_value(conf->done)) - return xilinx_spi_apply_cclk_cycles(conf); - - timeout = jiffies + usecs_to_jiffies(info->config_complete_timeout_us); + while (true) { + if (gpiod_get_value(conf->done)) + return xilinx_spi_apply_cclk_cycles(conf); - while (time_before(jiffies, timeout)) { + if (time_after(jiffies, timeout)) + break; ret = xilinx_spi_apply_cclk_cycles(conf); if (ret) return ret; - - if (gpiod_get_value(conf->done)) - return xilinx_spi_apply_cclk_cycles(conf); } >>> Do you need another >>> >>> if (gpiod_get_value(conf->done)) >>> return xilinx_spi_apply_cclk_cycles(conf); >>> >>> here to cover the chance of sleeping in the loop ? >> If I got your question correctly: if we get here it's because of a >> timeout, thus programming has failed (DONE didn't come up after some >> time), and checking it one more here seems pointless. > > It may not be pointless, if this routine sleeps because it was scheduled out, > when it wakes up a lot of time happened. You will see this as a timeout but > the state may be good. Another, final check at the end will cover this case. Oh, now I got your point! Yes, there is this risk, and it exists in current code as well but with a smaller risk window. Unrolling the current and new loop code they behave the same except for the position of the timeout computation (after vs before the first 'if (done) return' group). I think this reimplementation is sleep-safe, check for GPIO errors and also avoid code duplication: static int xilinx_spi_write_complete(struct fpga_manager *mgr, struct fpga_image_info *info) { struct xilinx_spi_conf *conf = mgr->priv; unsigned long timeout = jiffies + usecs_to_jiffies(info->config_complete_timeout_us); bool expired; int done; int ret; while (!expired) { expired = time_after(jiffies, timeout); done = get_done_gpio(mgr); if (done < 0) return done; ret = xilinx_spi_apply_cclk_cycles(conf); if (ret) return ret; if (done) return 0; } dev_err(&mgr->dev, "Timeout after config data transfer\n"); return -ETIMEDOUT; } A key point is to assess all the status (expired and done variables) before taking any action based on it. Then we can unconditionally apply 8 cclk cycles before even checking the actual DONE value, so that we always do that after DONE has been seen asserted. Does it look good? -- Luca
[PATCH 2/2] pci: fix memleak when calling pci_iomap/unmap()
config GENERIC_IOMAP is disabled on some archs(e.g. arm64), so pci_iounmap() does nothing, when we using pci_iomap/pci_iounmap(), it will lead to memory leak. Move pci_iounmap() to lib/pci_map.c to fix this. Signed-off-by: Yang Yingliang --- include/asm-generic/pci_iomap.h | 2 ++ lib/iomap.c | 10 -- lib/pci_iomap.c | 8 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/include/asm-generic/pci_iomap.h b/include/asm-generic/pci_iomap.h index d4f16dcc2ed79..d6a04d2462238 100644 --- a/include/asm-generic/pci_iomap.h +++ b/include/asm-generic/pci_iomap.h @@ -18,6 +18,8 @@ extern void __iomem *pci_iomap_range(struct pci_dev *dev, int bar, extern void __iomem *pci_iomap_wc_range(struct pci_dev *dev, int bar, unsigned long offset, unsigned long maxlen); +#define pci_iounmap pci_iounmap +extern void pci_iounmap(struct pci_dev *dev, void __iomem * addr); /* Create a virtual mapping cookie for a port on a given PCI device. * Do not call this directly, it exists to make it easier for architectures * to override */ diff --git a/lib/iomap.c b/lib/iomap.c index d40bc6f662540..df0b3c5fa2065 100644 --- a/lib/iomap.c +++ b/lib/iomap.c @@ -337,13 +337,3 @@ void ioport_unmap(void __iomem *addr) EXPORT_SYMBOL(ioport_map); EXPORT_SYMBOL(ioport_unmap); #endif /* CONFIG_HAS_IOPORT_MAP */ - -#ifdef CONFIG_PCI -/* Hide the details if this is a MMIO or PIO address space and just do what - * you expect in the correct way. */ -void pci_iounmap(struct pci_dev *dev, void __iomem * addr) -{ - IO_COND(addr, /* nothing */, iounmap(addr)); -} -EXPORT_SYMBOL(pci_iounmap); -#endif /* CONFIG_PCI */ diff --git a/lib/pci_iomap.c b/lib/pci_iomap.c index 2d3eb1cb73b8f..833b702771ecd 100644 --- a/lib/pci_iomap.c +++ b/lib/pci_iomap.c @@ -134,4 +134,12 @@ void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long maxlen) return pci_iomap_wc_range(dev, bar, 0, maxlen); } EXPORT_SYMBOL_GPL(pci_iomap_wc); + +/* Hide the details if this is a MMIO or PIO address space and just do what + * you expect in the correct way. */ +void pci_iounmap(struct pci_dev *dev, void __iomem * addr) +{ + IO_COND(addr, /* nothing */, iounmap(addr)); +} +EXPORT_SYMBOL(pci_iounmap); #endif /* CONFIG_PCI */ -- 2.25.1
[PATCH 0/2] fix memleak when using pci_iounmap()
config GENERIC_IOMAP is not selected on some arch, pci_iounmap() don't implement, when we using pci_iomap/pci_iounmap, it will lead to memory leak. This patch set moves the implemention of pci_iounmap() to lib/pci_iomap.c to fix this. Yang Yingliang (2): iomap: move some definitions to include/linux/io.h pci: fix memleak when calling pci_iomap/unmap() include/asm-generic/pci_iomap.h | 2 ++ include/linux/io.h | 36 ++ lib/iomap.c | 46 - lib/pci_iomap.c | 8 ++ 4 files changed, 46 insertions(+), 46 deletions(-) -- 2.25.1
[PATCH 1/2] iomap: move some definitions to include/linux/io.h
Move some IO macros and bad_io_access() to include/linux/io.h This prepares for moving pci_iounmap() to lib/pci_iomap.c. Signed-off-by: Yang Yingliang --- include/linux/io.h | 36 lib/iomap.c| 36 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/include/linux/io.h b/include/linux/io.h index 8394c56babc26..fa040f8114eaa 100644 --- a/include/linux/io.h +++ b/include/linux/io.h @@ -13,6 +13,42 @@ #include #include +#ifndef HAVE_ARCH_PIO_SIZE +/* + * We encode the physical PIO addresses (0-0x) into the + * pointer by offsetting them with a constant (0x1) and + * assuming that all the low addresses are always PIO. That means + * we can do some sanity checks on the low bits, and don't + * need to just take things for granted. + */ +#define PIO_OFFSET 0x1UL +#define PIO_MASK 0x0UL +#define PIO_RESERVED 0x4UL +#endif + +static inline void bad_io_access(unsigned long port, const char *access) +{ + static int count = 10; + if (count) { + count--; + WARN(1, KERN_ERR "Bad IO access at port %#lx (%s)\n", port, access); + } +} + +/* + * Ugly macros are a way of life. + */ +#define IO_COND(addr, is_pio, is_mmio) do {\ + unsigned long port = (unsigned long __force)addr; \ + if (port >= PIO_RESERVED) { \ + is_mmio;\ + } else if (port > PIO_OFFSET) { \ + port &= PIO_MASK; \ + is_pio; \ + } else \ + bad_io_access(port, #is_pio); \ +} while (0) + struct device; struct resource; diff --git a/lib/iomap.c b/lib/iomap.c index fbaa3e8f19d6c..d40bc6f662540 100644 --- a/lib/iomap.c +++ b/lib/iomap.c @@ -23,42 +23,6 @@ * implementation and should do their own copy. */ -#ifndef HAVE_ARCH_PIO_SIZE -/* - * We encode the physical PIO addresses (0-0x) into the - * pointer by offsetting them with a constant (0x1) and - * assuming that all the low addresses are always PIO. That means - * we can do some sanity checks on the low bits, and don't - * need to just take things for granted. - */ -#define PIO_OFFSET 0x1UL -#define PIO_MASK 0x0UL -#define PIO_RESERVED 0x4UL -#endif - -static void bad_io_access(unsigned long port, const char *access) -{ - static int count = 10; - if (count) { - count--; - WARN(1, KERN_ERR "Bad IO access at port %#lx (%s)\n", port, access); - } -} - -/* - * Ugly macros are a way of life. - */ -#define IO_COND(addr, is_pio, is_mmio) do {\ - unsigned long port = (unsigned long __force)addr; \ - if (port >= PIO_RESERVED) { \ - is_mmio;\ - } else if (port > PIO_OFFSET) { \ - port &= PIO_MASK; \ - is_pio; \ - } else \ - bad_io_access(port, #is_pio ); \ -} while (0) - #ifndef pio_read16be #define pio_read16be(port) swab16(inw(port)) #define pio_read32be(port) swab32(inl(port)) -- 2.25.1
Re: [PATCH v2] opp: Power on (virtual) power domains managed by the OPP core
On 27-08-20, 13:44, Stephan Gerhold wrote: > Hmm. Actually I was using this parameter for initial testing, and forced > on the power domains from the qcom-cpufreq-nvmem driver. For my v1 patch > I wanted to enable the power domains in dev_pm_opp_set_rate(), so there > using the virt_devs parameter was not possible. Right, as we really do not want to enable it there and leave it for the real consumers to handle. > On the other hand, creating the device links would be possible from the > platform driver by using the parameter. Right. > > And so I think again if this patch should be picked instead of letting > > the platform handle this ? > > It seems like originally the motivation for the parameter was that > cpufreq consumers do *not* need to power on the power domains: > > Commit 17a8f868ae3e ("opp: Return genpd virtual devices from > dev_pm_opp_attach_genpd()"): > "The cpufreq drivers don't need to do runtime PM operations on > the virtual devices returned by dev_pm_domain_attach_by_name() and so > the virtual devices weren't shared with the callers of > dev_pm_opp_attach_genpd() earlier. > > But the IO device drivers would want to do that. This patch updates > the prototype of dev_pm_opp_attach_genpd() to accept another argument > to return the pointer to the array of genpd virtual devices." Not just that I believe. There were also arguments that only the real consumer knows how to handle multiple power domains. For example for a USB or Camera module which can work in multiple modes, we may want to enable only one power domain in say slow mode and another power domain in fast mode. And so these kind of complex behavior/choices better be left for the end consumer and not try to handle this generically in the OPP core. > But the reason why I made this patch is that actually something *should* > enable the power domains even for the cpufreq case. Ulf, what do you think about this ? IIRC from our previous discussions someone asked me not do so. > If every user of dev_pm_opp_attach_genpd() ends up creating these device > links we might as well manage those directly from the OPP core. Sure, I am all in for reducing code duplication, but ... > I cannot think of any use case where you would not want to manage those > power domains using device links. And if there is such a use case, > chances are good that this use case is so special that using the OPP API > to set the performance states would not work either. In either case, > this seems like something that should be discussed once there is such a > use case. The example I gave earlier shows a common case where we need to handle this at the end users which still want to use the OPP API. > At the moment, there are only two users of dev_pm_opp_attach_genpd(): > > - cpufreq (qcom-cpufreq-nvmem) > - I/O (venus, recently added in linux-next [1]) > > [1]: > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=9a538b83612c8b5848bf840c2ddcd86dda1c8c76 > > In fact, venus adds the device link exactly the same way as in my patch. > So we could move that to the OPP core, simplify the venus code and > remove the virt_devs parameter. That would be my suggestion. > > I can submit a v3 with that if you agree (or we take this patch as-is > and remove the parameter separately - I just checked and creating a > device link twice does not seem to cause any problems...) I normally tend to agree with the logic that lets only focus on what's upstream and not think of virtual cases which may never happen. But I was told that this is too common of a scenario and so it made sense to do it this way. Maybe Ulf can again throw some light here :) -- viresh
Re: [PATCH 2/3] clk: samsung: exynos5420/5250: Add IDs to the CPU parent clk definitions
On Wed, Aug 26, 2020 at 07:15:28PM +0200, Sylwester Nawrocki wrote: > Use non-zero clock IDs in definitions of the CPU parent clocks > for exynos5420, exynos5250 SoCs. This will allow us to reference > the parent clocks directly in the driver by cached struct clk_hw > pointers, rather than doing clk lookup by name. > > Signed-off-by: Sylwester Nawrocki > --- > drivers/clk/samsung/clk-exynos5250.c | 4 ++-- > drivers/clk/samsung/clk-exynos5420.c | 11 ++- > 2 files changed, 8 insertions(+), 7 deletions(-) Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH 1/3] clk: samsung: Add clk ID definitions for the CPU parent clocks
On Wed, Aug 26, 2020 at 07:15:27PM +0200, Sylwester Nawrocki wrote: > Add clock ID definitions for the CPU parent clocks for SoCs > which don't have such definitions yet. This will allow us to > reference the parent clocks directly by cached struct clk_hw > pointers in the clock provider, rather than doing clk lookup > by name. > > Signed-off-by: Sylwester Nawrocki > --- > include/dt-bindings/clock/exynos5250.h | 4 +++- > include/dt-bindings/clock/exynos5420.h | 5 + > 2 files changed, 8 insertions(+), 1 deletion(-) Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH 3/3] drm/vc4: hdmi: Add pixel bvb clock control
On 8/27/20 6:49 PM, Stefan Wahren wrote: > Am 27.08.20 um 06:35 schrieb Hoegeun Kwon: >> Hi Stefan, >> >> Thank you for your review. >> >> >> On 8/26/20 7:04 PM, Stefan Wahren wrote: >>> Hi Hoeguen, >>> >>> Am 21.08.20 um 09:10 schrieb Hoegeun Kwon: There is a problem that the output does not work at a resolution exceeding FHD. To solve this, we need to adjust the bvb clock at a resolution exceeding FHD. >>> this patch introduces a mandatory clock, please update >>> brcm,bcm2835-hdmi.yaml first. >>> >>> Is this clock physically available on BCM283x or only on BCM2711? >> As far as I know, BCM2711 raspberry pi 4 supports 4k, >> >> don't supported on pi 3 and pi 3+. >> >> Since 4k is not supported in versions prior to Raspberry Pi 4, >> >> I don't think we need to modify the bvb clock. >> >> >> So I think it is better to update 'brcm,bcm2711-hdmi.yaml' >> >> instead of 'brcm,bcm2835-hdmi.yaml'. > You are correct please update only brcm,bcm2711-hdmi.yaml. > > My concern was that the function vc4_hdmi_encoder_pre_crtc_configure() > is called on a non-bcm2711 platform or on a Raspberry Pi 4 with an older > DTB. So making the BVB clock optional might be better? You are right, if use old dtb, we have a problem with the hdmi driver. So how about modifying it like this? @@ -1614,8 +1614,8 @@ static int vc5_hdmi_init_resources(struct vc4_hdmi *vc4_hdmi) vc4_hdmi->pixel_bvb_clock = devm_clk_get(dev, "bvb"); if (IS_ERR(vc4_hdmi->pixel_bvb_clock)) { - DRM_ERROR("Failed to get pixel bvb clock\n"); - return PTR_ERR(vc4_hdmi->pixel_bvb_clock); + DRM_WARN("Failed to get pixel bvb clock\n"); + vc4_hdmi->pixel_bvb_clock = NULL; } vc4_hdmi->reset = devm_reset_control_get(dev, NULL); If we modify like this, if there is no bvb clock in dtb, then pixel_bvb_clock will be null and it will not affect the clk control function below. - clk_set_rate() - clk_prepare_enable() - clk_disable_unprepare() Best regards Hoegeun > >> Please comment, what do you think? >> >>> I'm a little bit afraid, this change could break with older firmware >>> versions on BCM283x. >> Tested it several times with libdrm modetest. >> >> I expect there will be no problem. >> >> >> Best regards, >> >> Hoegeun >> >>> Best regards >>> Stefan >>> Signed-off-by: Hoegeun Kwon --- drivers/gpu/drm/vc4/vc4_hdmi.c | 25 + drivers/gpu/drm/vc4/vc4_hdmi.h | 1 + 2 files changed, 26 insertions(+) diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index 95ec5eedea39..eb3192d1fd86 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -80,6 +80,7 @@ # define VC4_HD_M_ENABLEBIT(0) #define CEC_CLOCK_FREQ 4 +#define VC4_HSM_MID_CLOCK 149985000 static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused) { @@ -380,6 +381,7 @@ static void vc4_hdmi_encoder_post_crtc_powerdown(struct drm_encoder *encoder) HDMI_WRITE(HDMI_VID_CTL, HDMI_READ(HDMI_VID_CTL) & ~VC4_HD_VID_CTL_ENABLE); + clk_disable_unprepare(vc4_hdmi->pixel_bvb_clock); clk_disable_unprepare(vc4_hdmi->hsm_clock); clk_disable_unprepare(vc4_hdmi->pixel_clock); @@ -638,6 +640,23 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct drm_encoder *encoder) return; } + ret = clk_set_rate(vc4_hdmi->pixel_bvb_clock, + (hsm_rate > VC4_HSM_MID_CLOCK ? 15000 : 7500)); + if (ret) { + DRM_ERROR("Failed to set pixel bvb clock rate: %d\n", ret); + clk_disable_unprepare(vc4_hdmi->hsm_clock); + clk_disable_unprepare(vc4_hdmi->pixel_clock); + return; + } + + ret = clk_prepare_enable(vc4_hdmi->pixel_bvb_clock); + if (ret) { + DRM_ERROR("Failed to turn on pixel bvb clock: %d\n", ret); + clk_disable_unprepare(vc4_hdmi->hsm_clock); + clk_disable_unprepare(vc4_hdmi->pixel_clock); + return; + } + if (vc4_hdmi->variant->reset) vc4_hdmi->variant->reset(vc4_hdmi); @@ -1593,6 +1612,12 @@ static int vc5_hdmi_init_resources(struct vc4_hdmi *vc4_hdmi) return PTR_ERR(vc4_hdmi->audio_clock); } + vc4_hdmi->pixel_bvb_clock = devm_clk_get(dev, "bvb"); + if (IS_ERR(vc4_hdmi->pixel_bvb_clock)) { + DRM_ERROR("Failed to get pixel bvb clock\n"); + return PTR_ERR(vc4_hdmi->pixel_bvb_clock); + } + vc4_hdmi->reset = devm_reset_control_get(dev, NULL); if (IS_ERR(
Re: [PATCH v2 09/18] iio: afe: iio-rescale: Simplify with dev_err_probe()
On Thu, Aug 27, 2020 at 11:46:40PM +0200, Peter Rosin wrote: > On 2020-08-27 21:26, Krzysztof Kozlowski wrote: > > Common pattern of handling deferred probe can be simplified with > > dev_err_probe(). Less code and also it prints the error value. > > > > Signed-off-by: Krzysztof Kozlowski > > > > --- > > > > Changes since v1: > > 1. Wrap dev_err_probe() lines at 100 character > > --- > > drivers/iio/afe/iio-rescale.c | 7 ++- > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c > > index 69c0f277ada0..8cd9645c50e8 100644 > > --- a/drivers/iio/afe/iio-rescale.c > > +++ b/drivers/iio/afe/iio-rescale.c > > @@ -276,11 +276,8 @@ static int rescale_probe(struct platform_device *pdev) > > int ret; > > > > source = devm_iio_channel_get(dev, NULL); > > - if (IS_ERR(source)) { > > - if (PTR_ERR(source) != -EPROBE_DEFER) > > - dev_err(dev, "failed to get source channel\n"); > > - return PTR_ERR(source); > > - } > > + if (IS_ERR(source)) > > + return dev_err_probe(dev, PTR_ERR(source), "failed to get > > source channel\n"); > > Hi! > > Sorry to be a pain...but... > > I'm not a huge fan of adding *one* odd line breaking the 80 column > recommendation to any file. I like to be able to fit multiple > windows side by side in a meaningful way. Also, I don't like having > a shitload of emptiness on my screen, which is what happens when some > lines are longer and you want to see it all. I strongly believe that > the 80 column rule/recommendation is still as valid as it ever was. > It's just hard to read longish lines; there's a reason newspapers > columns are quite narrow... > > Same comment for the envelope-detector (3/18). > > You will probably never look at these files again, but *I* might have > to revisit them for one reason or another, and these long lines will > annoy me when that happens. Initially I posted it with 80-characters wrap. Then I received a comment - better to stick to the new 100, as checkpatch accepts it. Now you write, better to go back to 80. Maybe then someone else will write to me, better to go to 100. And another person will reply, no, coding style still mentions 80, so keep it at 80. Sure guys, please first decide which one you prefer, then I will wrap it accordingly. :) Otherwise I will just jump from one to another depending on one person's personal preference. If there is no consensus among discussing people, I find this 100 line more readable, already got review, checkpatch accepts it so if subsystem maintainer likes it, I prefer to leave it like this. > You did wrap the lines for dpot-dac (12/18) - which is excellent - but > that makes me curious as to what the difference is? Didn't fit into limit of 100. Best regards, Krzysztof
Re: [PATCH v11 25/25] x86/cet/shstk: Add arch_prctl functions for shadow stack
* H. J. Lu: > Can you think of ANY issues of passing more arguments to arch_prctl? On x32, the glibc arch_prctl system call wrapper only passes two arguments to the kernel, and applications have no way of detecting that. musl only passes two arguments on all architectures. It happens to work anyway with default compiler flags, but that's an accident. Thanks, Florian
Re: Some questions about the patching process
On Thu, Aug 27, 2020 at 02:17:20PM -0500, Qiushi Wu wrote: > Hi Greg, > Thanks for your response! You responded in html format which got rejected by the public list, please resend in text-only and I will be glad to reply. thanks, greg k-h
Re: [PATCH 1/4] drivers: hv: dxgkrnl: core code
On Thu, Aug 27, 2020 at 05:25:23PM -0700, Iouri Tarassov wrote: > > > +bool dxghwqueue_acquire_reference(struct dxghwqueue *hwqueue) > > > +{ > > > + return refcount_inc_not_zero(&hwqueue->refcount); > > > +} > > > > Midlayers are evil. > I strongly agree in general, but think that in our case the layers are very > small. It allows to quickly find all places where a reference/dereference on > an object is done and addition of debug tracing to catch errors. Again, no, please remove all layers like this. They just make it impossible for others to review and understand the code over time. Also, in this specific case, it would have allowed me to easily realize that you are doing this type of call incorrectly and should be using a different data structure :) thanks, greg k-h
Re: [PATCH 1/4] drivers: hv: dxgkrnl: core code
On Thu, Aug 27, 2020 at 05:25:23PM -0700, Iouri Tarassov wrote: > > > +{ > > > + struct dxgprocess_adapter *adapter_info = dxgmem_alloc(process, > > > + > > > DXGMEM_PROCESS_ADAPTER, > > > +sizeof > > > +(*adapter_info)); > > > > We normally use kernel functions in kernel code. > Using a custom memory allocation function allows us to track memory > allocations per DXGPROCESS and catch memory leaks when a DXGPROCESS is > destroyed or when the driver is unloaded. It also allows to easily change > the memory allocation implementation if needed. There is only one "memory allocation implementation" in the kernel, please use that and not any wrapper functions. You wouldn't want to see 1000's of different memory allocation functions, each driver having a unique one, right? Remember, your code is becoming part of the larger kernel, so follow the guidelines and rules of it. There is nothing different from your code and a serial port driver when it comes to these expectations. thanks, greg k-h
[PATCH] aio: make aio wait path to account iowait time
As the normal aio wait path(read_events() -> wait_event_interruptible_hrtimeout()) doesn't account iowait time, so use this patch to make it to account iowait time, which can truely reflect the system io situation when using a tool like 'top'. The test result as below. Test environment: Architecture: x86_64 CPU op-mode(s):32-bit, 64-bit Byte Order:Little Endian CPU(s):32 On-line CPU(s) list: 0-31 Thread(s) per core:2 Core(s) per socket:8 Socket(s): 2 NUMA node(s): 2 Vendor ID: GenuineIntel CPU family:6 Model: 85 Model name:Intel(R) Xeon(R) Silver 4108 CPU @ 1.80GHz Stepping: 4 CPU MHz: 801.660 AIO test command: fio -ioengine=libaio -bs=8k -direct=1 -numjobs 32 -rw=read -size=10G -filename=/dev/sda3 -name="Max throughput" -iodepth=128 -runtime=60 Before test, set nr_requests to 512(default is 256), aim to to make the backend device busy to handle io request, and make io_getevents() have more chances to wait for io completion: echo 512 > /sys/block/sda/queue/nr_requests Fio test result with the AIO iowait time accounting patch showed as below, almost all fio threads are in 'D' state to waiting for io completion, and the iowait time is accounted as 48.0%: top - 19:19:23 up 29 min, 2 users, load average: 14.60, 9.45, 9.45 Tasks: 456 total, 4 running, 247 sleeping, 0 stopped, 0 zombie %Cpu(s): 0.4 us, 1.0 sy, 0.0 ni, 50.6 id, 48.0 wa, 0.0 hi, 0.0 si, 0.0 st KiB Mem : 19668915+total, 19515264+free, 866476 used, 670028 buff/cache KiB Swap: 4194300 total, 4194300 free,0 used. 19449948+avail Mem PID USER PR NIVIRTRESSHR S %CPU %MEM TIME+ COMMAND 16135 root 20 0 294092 63724 63060 S 1.7 0.0 0:03.31 fio 16173 root 20 0 272352 3540 1792 D 1.7 0.0 0:03.85 fio 16175 root 20 0 272360 3544 1796 D 1.7 0.0 0:03.85 fio 16185 root 20 0 272400 3556 1808 D 1.7 0.0 0:03.84 fio 16187 root 20 0 272408 3552 1804 D 1.7 0.0 0:03.82 fio 16190 root 20 0 272420 3500 1804 R 1.7 0.0 0:03.88 fio 16169 root 20 0 272336 3444 1740 D 1.3 0.0 0:03.75 fio 16170 root 20 0 272340 3504 1804 R 1.3 0.0 0:03.80 fio 16172 root 20 0 272348 3500 1800 D 1.3 0.0 0:03.86 fio 16174 root 20 0 272356 3544 1796 D 1.3 0.0 0:03.77 fio 16179 root 20 0 272376 3528 1780 D 1.3 0.0 0:03.79 fio 16180 root 20 0 272380 3500 1800 D 1.3 0.0 0:03.85 fio 16181 root 20 0 272384 3552 1804 D 1.3 0.0 0:03.87 fio 16182 root 20 0 272388 3520 1772 D 1.3 0.0 0:03.80 fio 16183 root 20 0 272392 3552 1804 D 1.3 0.0 0:03.77 fio 16186 root 20 0 272404 3500 1804 D 1.3 0.0 0:03.88 fio 16188 root 20 0 272412 3500 1800 D 1.3 0.0 0:03.89 fio 16191 root 20 0 272424 3500 1800 D 1.3 0.0 0:03.92 fio 16192 root 20 0 272428 3500 1800 D 1.3 0.0 0:03.87 fio 16194 root 20 0 272436 3500 1804 D 1.3 0.0 0:03.82 fio 16195 root 20 0 272440 3500 1800 R 1.3 0.0 0:03.82 fio 16196 root 20 0 272444 3552 1804 D 1.3 0.0 0:03.84 fio 16198 root 20 0 272452 3500 1804 D 1.3 0.0 0:03.89 fio 16199 root 20 0 272456 3504 1800 D 1.3 0.0 0:03.84 fio 16200 root 20 0 272460 3552 1804 D 1.3 0.0 0:03.85 fio 16171 root 20 0 272344 3504 1800 D 1.0 0.0 0:03.84 fio 16176 root 20 0 272364 3520 1772 D 1.0 0.0 0:03.76 fio 16177 root 20 0 272368 3556 1808 D 1.0 0.0 0:03.74 fio 16178 root 20 0 272372 3500 1804 D 1.0 0.0 0:03.90 fio 16184 root 20 0 272396 3500 1800 D 1.0 0.0 0:03.83 fio 16189 root 20 0 272416 3500 1804 D 1.0 0.0 0:03.86 fio 16193 root 20 0 272432 3500 1804 D 1.0 0.0 0:03.85 fio 16197 root 20 0 272448 3556 1808 D 1.0 0.0 0:03.75 fio Fio test result without the AIO iowait time accounting patch showed as below, almost all fio threads are in 'S' state to waiting for io completion, and the iowait time is not accounted, iowait is 0.0%. top - 19:20:44 up 31 min, 2 users, load average: 12.50, 10.15, 9.72 Tasks: 458 total, 2 running, 249 sleeping, 0 stopped, 0 zombie %Cpu(s): 0.4 us,
Re: [PATCH 1/4] drivers: hv: dxgkrnl: core code
On Thu, Aug 27, 2020 at 04:45:55PM -0700, Iouri Tarassov wrote: > > On 8/14/2020 5:57 AM, Greg KH wrote: > > On Fri, Aug 14, 2020 at 08:38:53AM -0400, Sasha Levin wrote: > > > Add support for a Hyper-V based vGPU implementation that exposes the > > > DirectX API to Linux userspace. > > > > Api questions: > > > > > +struct d3dkmthandle { > > > + union { > > > + struct { > > > + u32 instance: 6; > > > + u32 index : 24; > > > + u32 unique : 2; > > > > What is the endian of this? > > > > > + }; > > > + u32 v; > > > + }; > > > +}; > > > + > > > +extern const struct d3dkmthandle zerohandle; > > > + > The definition is the same as on the Windows side. The driver communicates > with a Windows host, so I do not expect any issues with endiannes. Windows > currently runs only on the little endian platforms. As I mentioned before, you need to document that somewhere (like maybe preventing your code from being built on big endian systems?) > User mode applications see this as an opaque 32 bit value (D3DKMTHANDLE). I > prefer not to use the u32 definition to avoid mistakes when another integer > or a 64-bit handle is assigned to the handle or the handle is assigned to a > 64 or 32 bit integer variable. There are many handles in the driver model > (shared NT handle, d3dkmthandle, etc). Using a specific type allows to avoid > assigning one handle to another. Specific types are great, that is fine. > > > +struct ntstatus { > > > + union { > > > + struct { > > > + int code: 16; > > > + int facility: 13; > > > + int customer: 1; > > > + int severity: 2; > > > > Same here. > > > > Are these things that cross the user/kernel boundry? > > > > And why int on one and u32 on the other? > > > > > + }; > > > + int v; > > > + }; > > > +}; > > > + > "struct ntstatus" follows the definition for NTSTATUS on the Windows side. > NTSTATUS is an integer where the negative values indicate errors. It is > success otherwise. NTSTATUS is returned by the VM bus messages from host. > IOCTLs from the driver return Linux negative error code or NTSTATUS positive > success codes. DxCore applications expect certain positive success codes. > DxCore is a shared library, which translates the D3DKMT* Windows interface > to Linux ioctls. Applications link with DxCore to use a paravirtualized GPU. > D3DKMTHANDLE is a 32-bit unsigned value (bitfield), not an integer. Ok, again, please document this, and as these fields are crossing the kernel/user boundry, use the correct types (hint, 'int' is never that type). > > > +struct winluid { > > > + uint a; > > > + uint b; > > > > And now uint? Come on, be consistent please :) > Sorry about this. This came from the Windows size where we use UINT a lot. > All uints will be replaced by u32 in the next patch set. thank you. greg k-h
Re: [PATCH 1/4] drivers: hv: dxgkrnl: core code
On Thu, Aug 27, 2020 at 05:05:44PM -0700, Iouri Tarassov wrote: > > On 8/14/2020 6:04 AM, Greg KH wrote: > > On Fri, Aug 14, 2020 at 08:38:53AM -0400, Sasha Levin wrote: > > > Add support for a Hyper-V based vGPU implementation that exposes the > > > DirectX API to Linux userspace. > > > > Signed-off-by: Sasha Levin > > > --- > > > drivers/hv/dxgkrnl/Kconfig | 10 + > > > drivers/hv/dxgkrnl/Makefile | 12 + > > > drivers/hv/dxgkrnl/d3dkmthk.h | 1636 ++ > > > drivers/hv/dxgkrnl/dxgadapter.c | 1406 > > > drivers/hv/dxgkrnl/dxgkrnl.h| 927 ++ > > > drivers/hv/dxgkrnl/dxgmodule.c | 656 > > > drivers/hv/dxgkrnl/dxgprocess.c | 357 ++ > > > drivers/hv/dxgkrnl/dxgvmbus.c | 3084 ++ > > > drivers/hv/dxgkrnl/dxgvmbus.h | 873 + > > > drivers/hv/dxgkrnl/hmgr.c | 604 > > > drivers/hv/dxgkrnl/hmgr.h | 112 + > > > drivers/hv/dxgkrnl/ioctl.c | 5413 +++ > > > drivers/hv/dxgkrnl/misc.c | 279 ++ > > > drivers/hv/dxgkrnl/misc.h | 309 ++ > > > 14 files changed, 15678 insertions(+) > > > > It's almost impossible to review 15k lines at once, please break this up > > into reviewable chunks next time. > > Sorry about this, but we had to replace a lot of typedefs, which are not > allowed by the coding style. Ok, nice work, but that has nothing to do with how you submit a patch to us for review. > We expect one more big patch, which cannot be split in my opinion. I disagree with that opinion, and so do thousands of other Linux kernel developers who have done this successfully in the past :) Remember, it is your job to make this as simple and as easy as possible for me to review your code, such that it is trivial for me to understand and accept it. That takes more work on your side to do this, as we have thousands of developers, but very few reviewers. We know we waste engineering time on this type of thing, but the end result makes for better reviews and consequentially, better reviews. So don't ignore this advice, remember, you are wanting me to do something for you, for free. Make it easy for me to do so. > The VM > vbus message format was changed to include additional header. As the result, > every function in dxgvmbus.c needs to be changed to handle the new header. I > do not see how this can be split to multiple patches so each patch produces > a working driver. It doesn't have to "work" fully, see many many examples of how to do this every week submitted to us. It's not an impossible task at all. > > > +++ b/drivers/hv/dxgkrnl/Kconfig > > > @@ -0,0 +1,10 @@ > > > +# > > > +# dxgkrnl configuration > > > +# > > > + > > > +config DXGKRNL > > > + tristate "Microsoft virtual GPU support" > > > + depends on HYPERV > > > + help > > > + This driver supports Microsoft virtual GPU. > > > + > > > > You need more text here, this isn't a staging driver submission :) > Is the the proposed description good enough? What proposed description? > "This driver handles paravirtualized GPU devices exposed by Microsoft > Hyper-V when Linux is running inside of a virtual machine hosted > by Windows." That's better, but really, when a tiny serial port driver has more text than this huge thing, you might want to consider expanding on exactly what you want people to understand... > > > +struct d3dkmt_closeadapter { > > > + struct d3dkmthandle adapter_handle; > > > +}; > > > > A "handle"? And that has to be one of the most difficult structure > > names ever :) > > > > Why not just use the "handle" for the structure as obviously that's all > > that is needed here. > The structure definition matches the Windows D3DKMT interface. Some input > structures to the interface functions have only one member. But there is > possibility that new member could be added in the future. We prefer to have > matching names between Windows and Linux to avoid confusion. Don't write code because "it might change in the future". Write code for what you have today. If it does change in the future, wonderful, go and change the code. You have the full ability to do so then, no need to hurt all of us today for that potential. As for "matching names", why does that matter? Who sees both names at the same time? > > > + > > > +struct d3dkmt_openadapterfromluid { > > > + struct winluid adapter_luid; > > > + struct d3dkmthandle adapter_handle; > > > +}; > > > + > > > +struct d3dddi_allocationlist { > > > + struct d3dkmthandle allocation; > > > + union { > > > + struct { > > > + uintwrite_operation :1; > > > + uintdo_not_retire_instance :1; > > > + uintoffer_priority :3; > > > + uintreserved:27; > > > > endian issues? > > > > If not, why are these bit fields? > This matches the definition on the Windo
[PATCH V2 5/8] spi: spi-geni-qcom: Unconditionally call dev_pm_opp_of_remove_table()
dev_pm_opp_of_remove_table() doesn't report any errors when it fails to find the OPP table with error -ENODEV (i.e. OPP table not present for the device). And we can call dev_pm_opp_of_remove_table() unconditionally here. While at it, create a new label and put clkname on errors. Signed-off-by: Viresh Kumar --- V2: - Compare with -ENODEV only for failures. - Create new label to put clkname. --- drivers/spi/spi-geni-qcom.c | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c index 80cea5cd3612..0dc3f4c55b0b 100644 --- a/drivers/spi/spi-geni-qcom.c +++ b/drivers/spi/spi-geni-qcom.c @@ -613,11 +613,9 @@ static int spi_geni_probe(struct platform_device *pdev) return PTR_ERR(mas->se.opp_table); /* OPP table is optional */ ret = dev_pm_opp_of_add_table(&pdev->dev); - if (!ret) { - mas->se.has_opp_table = true; - } else if (ret != -ENODEV) { + if (ret && ret != -ENODEV) { dev_err(&pdev->dev, "invalid OPP table in device tree\n"); - return ret; + goto put_clkname; } spi->bus_num = -1; @@ -669,8 +667,8 @@ static int spi_geni_probe(struct platform_device *pdev) spi_geni_probe_runtime_disable: pm_runtime_disable(dev); spi_master_put(spi); - if (mas->se.has_opp_table) - dev_pm_opp_of_remove_table(&pdev->dev); + dev_pm_opp_of_remove_table(&pdev->dev); +put_clkname: dev_pm_opp_put_clkname(mas->se.opp_table); return ret; } @@ -685,8 +683,7 @@ static int spi_geni_remove(struct platform_device *pdev) free_irq(mas->irq, spi); pm_runtime_disable(&pdev->dev); - if (mas->se.has_opp_table) - dev_pm_opp_of_remove_table(&pdev->dev); + dev_pm_opp_of_remove_table(&pdev->dev); dev_pm_opp_put_clkname(mas->se.opp_table); return 0; } -- 2.25.0.rc1.19.g042ed3e048af
[PATCH V2 1/8] cpufreq: imx6q: Unconditionally call dev_pm_opp_of_remove_table()
dev_pm_opp_of_remove_table() doesn't report any errors when it fails to find the OPP table with error -ENODEV (i.e. OPP table not present for the device). And we can call dev_pm_opp_of_remove_table() unconditionally here. Signed-off-by: Viresh Kumar --- V2: No changes. --- drivers/cpufreq/imx6q-cpufreq.c | 10 ++ 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/drivers/cpufreq/imx6q-cpufreq.c b/drivers/cpufreq/imx6q-cpufreq.c index ef7b34c1fd2b..5bf5fc759881 100644 --- a/drivers/cpufreq/imx6q-cpufreq.c +++ b/drivers/cpufreq/imx6q-cpufreq.c @@ -48,7 +48,6 @@ static struct clk_bulk_data clks[] = { }; static struct device *cpu_dev; -static bool free_opp; static struct cpufreq_frequency_table *freq_table; static unsigned int max_freq; static unsigned int transition_latency; @@ -390,9 +389,6 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev) goto put_reg; } - /* Because we have added the OPPs here, we must free them */ - free_opp = true; - if (of_machine_is_compatible("fsl,imx6ul") || of_machine_is_compatible("fsl,imx6ull")) { ret = imx6ul_opp_check_speed_grading(cpu_dev); @@ -507,8 +503,7 @@ static int imx6q_cpufreq_probe(struct platform_device *pdev) free_freq_table: dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table); out_free_opp: - if (free_opp) - dev_pm_opp_of_remove_table(cpu_dev); + dev_pm_opp_of_remove_table(cpu_dev); put_reg: if (!IS_ERR(arm_reg)) regulator_put(arm_reg); @@ -528,8 +523,7 @@ static int imx6q_cpufreq_remove(struct platform_device *pdev) { cpufreq_unregister_driver(&imx6q_cpufreq_driver); dev_pm_opp_free_cpufreq_table(cpu_dev, &freq_table); - if (free_opp) - dev_pm_opp_of_remove_table(cpu_dev); + dev_pm_opp_of_remove_table(cpu_dev); regulator_put(arm_reg); if (!IS_ERR(pu_reg)) regulator_put(pu_reg); -- 2.25.0.rc1.19.g042ed3e048af
[PATCH V2 7/8] tty: serial: qcom_geni_serial: Unconditionally call dev_pm_opp_of_remove_table()
dev_pm_opp_of_remove_table() doesn't report any errors when it fails to find the OPP table with error -ENODEV (i.e. OPP table not present for the device). And we can call dev_pm_opp_of_remove_table() unconditionally here. While at it, create a new label to put clkname on errors. Signed-off-by: Viresh Kumar --- V2: - Compare with -ENODEV only for failures. - Create new label to put clkname. --- drivers/tty/serial/qcom_geni_serial.c | 13 + 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c index 3aa29d201f54..33f1af6c61d1 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -1433,11 +1433,9 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) return PTR_ERR(port->se.opp_table); /* OPP table is optional */ ret = dev_pm_opp_of_add_table(&pdev->dev); - if (!ret) { - port->se.has_opp_table = true; - } else if (ret != -ENODEV) { + if (ret && ret != -ENODEV) { dev_err(&pdev->dev, "invalid OPP table in device tree\n"); - return ret; + goto put_clkname; } port->private_data.drv = drv; @@ -1478,8 +1476,8 @@ static int qcom_geni_serial_probe(struct platform_device *pdev) return 0; err: - if (port->se.has_opp_table) - dev_pm_opp_of_remove_table(&pdev->dev); + dev_pm_opp_of_remove_table(&pdev->dev); +put_clkname: dev_pm_opp_put_clkname(port->se.opp_table); return ret; } @@ -1489,8 +1487,7 @@ static int qcom_geni_serial_remove(struct platform_device *pdev) struct qcom_geni_serial_port *port = platform_get_drvdata(pdev); struct uart_driver *drv = port->private_data.drv; - if (port->se.has_opp_table) - dev_pm_opp_of_remove_table(&pdev->dev); + dev_pm_opp_of_remove_table(&pdev->dev); dev_pm_opp_put_clkname(port->se.opp_table); dev_pm_clear_wake_irq(&pdev->dev); device_init_wakeup(&pdev->dev, false); -- 2.25.0.rc1.19.g042ed3e048af
[PATCH V2 8/8] qcom-geni-se: remove has_opp_table
has_opp_table isn't used anymore, remove it. Signed-off-by: Viresh Kumar --- V2: No changes. --- include/linux/qcom-geni-se.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/linux/qcom-geni-se.h b/include/linux/qcom-geni-se.h index 8f385fbe5a0e..02d1417c8ecf 100644 --- a/include/linux/qcom-geni-se.h +++ b/include/linux/qcom-geni-se.h @@ -48,7 +48,6 @@ struct geni_icc_path { * @clk_perf_tbl: Table of clock frequency input to serial engine clock * @icc_paths: Array of ICC paths for SE * @opp_table: Pointer to the OPP table - * @has_opp_table: Specifies if the SE has an OPP table */ struct geni_se { void __iomem *base; @@ -59,7 +58,6 @@ struct geni_se { unsigned long *clk_perf_tbl; struct geni_icc_path icc_paths[3]; struct opp_table *opp_table; - bool has_opp_table; }; /* Common SE registers */ -- 2.25.0.rc1.19.g042ed3e048af
[PATCH V2 2/8] drm/lima: Unconditionally call dev_pm_opp_of_remove_table()
dev_pm_opp_of_remove_table() doesn't report any errors when it fails to find the OPP table with error -ENODEV (i.e. OPP table not present for the device). And we can call dev_pm_opp_of_remove_table() unconditionally here. Reviewed-by: Qiang Yu Signed-off-by: Viresh Kumar --- V2: Applied Reviewed by tag. --- drivers/gpu/drm/lima/lima_devfreq.c | 6 +- drivers/gpu/drm/lima/lima_devfreq.h | 1 - 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/gpu/drm/lima/lima_devfreq.c b/drivers/gpu/drm/lima/lima_devfreq.c index bbe02817721b..cd290d866a04 100644 --- a/drivers/gpu/drm/lima/lima_devfreq.c +++ b/drivers/gpu/drm/lima/lima_devfreq.c @@ -105,10 +105,7 @@ void lima_devfreq_fini(struct lima_device *ldev) devfreq->devfreq = NULL; } - if (devfreq->opp_of_table_added) { - dev_pm_opp_of_remove_table(ldev->dev); - devfreq->opp_of_table_added = false; - } + dev_pm_opp_of_remove_table(ldev->dev); if (devfreq->regulators_opp_table) { dev_pm_opp_put_regulators(devfreq->regulators_opp_table); @@ -162,7 +159,6 @@ int lima_devfreq_init(struct lima_device *ldev) ret = dev_pm_opp_of_add_table(dev); if (ret) goto err_fini; - ldevfreq->opp_of_table_added = true; lima_devfreq_reset(ldevfreq); diff --git a/drivers/gpu/drm/lima/lima_devfreq.h b/drivers/gpu/drm/lima/lima_devfreq.h index 5eed2975a375..2d9b3008ce77 100644 --- a/drivers/gpu/drm/lima/lima_devfreq.h +++ b/drivers/gpu/drm/lima/lima_devfreq.h @@ -18,7 +18,6 @@ struct lima_devfreq { struct opp_table *clkname_opp_table; struct opp_table *regulators_opp_table; struct thermal_cooling_device *cooling; - bool opp_of_table_added; ktime_t busy_time; ktime_t idle_time; -- 2.25.0.rc1.19.g042ed3e048af
[PATCH V2 4/8] mmc: sdhci-msm: Unconditionally call dev_pm_opp_of_remove_table()
dev_pm_opp_of_remove_table() doesn't report any errors when it fails to find the OPP table with error -ENODEV (i.e. OPP table not present for the device). And we can call dev_pm_opp_of_remove_table() unconditionally here. Signed-off-by: Viresh Kumar --- V2: - Compare with -ENODEV only for failures. - Create new label to put clkname. --- drivers/mmc/host/sdhci-msm.c | 14 +- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c index 5a33389037cd..f7beaec6412e 100644 --- a/drivers/mmc/host/sdhci-msm.c +++ b/drivers/mmc/host/sdhci-msm.c @@ -263,7 +263,6 @@ struct sdhci_msm_host { unsigned long clk_rate; struct mmc_host *mmc; struct opp_table *opp_table; - bool has_opp_table; bool use_14lpp_dll_reset; bool tuning_done; bool calibration_done; @@ -2285,11 +2284,9 @@ static int sdhci_msm_probe(struct platform_device *pdev) /* OPP table is optional */ ret = dev_pm_opp_of_add_table(&pdev->dev); - if (!ret) { - msm_host->has_opp_table = true; - } else if (ret != -ENODEV) { + if (ret && ret != -ENODEV) { dev_err(&pdev->dev, "Invalid OPP table in Device tree\n"); - goto opp_cleanup; + goto opp_put_clkname; } /* Vote for maximum clock rate for maximum performance */ @@ -2453,8 +2450,8 @@ static int sdhci_msm_probe(struct platform_device *pdev) clk_bulk_disable_unprepare(ARRAY_SIZE(msm_host->bulk_clks), msm_host->bulk_clks); opp_cleanup: - if (msm_host->has_opp_table) - dev_pm_opp_of_remove_table(&pdev->dev); + dev_pm_opp_of_remove_table(&pdev->dev); +opp_put_clkname: dev_pm_opp_put_clkname(msm_host->opp_table); bus_clk_disable: if (!IS_ERR(msm_host->bus_clk)) @@ -2474,8 +2471,7 @@ static int sdhci_msm_remove(struct platform_device *pdev) sdhci_remove_host(host, dead); - if (msm_host->has_opp_table) - dev_pm_opp_of_remove_table(&pdev->dev); + dev_pm_opp_of_remove_table(&pdev->dev); dev_pm_opp_put_clkname(msm_host->opp_table); pm_runtime_get_sync(&pdev->dev); pm_runtime_disable(&pdev->dev); -- 2.25.0.rc1.19.g042ed3e048af
[PATCH V2 6/8] spi: spi-qcom-qspi: Unconditionally call dev_pm_opp_of_remove_table()
dev_pm_opp_of_remove_table() doesn't report any errors when it fails to find the OPP table with error -ENODEV (i.e. OPP table not present for the device). And we can call dev_pm_opp_of_remove_table() unconditionally here. While at it, create a new label and put clkname on errors. Signed-off-by: Viresh Kumar --- V2: - Compare with -ENODEV only for failures. - Create new label to put clkname. --- drivers/spi/spi-qcom-qspi.c | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/drivers/spi/spi-qcom-qspi.c b/drivers/spi/spi-qcom-qspi.c index b8857a97f40a..e5829c56650b 100644 --- a/drivers/spi/spi-qcom-qspi.c +++ b/drivers/spi/spi-qcom-qspi.c @@ -143,7 +143,6 @@ struct qcom_qspi { struct qspi_xfer xfer; struct icc_path *icc_path_cpu_to_qspi; struct opp_table *opp_table; - bool has_opp_table; unsigned long last_speed; /* Lock to protect data accessed by IRQs */ spinlock_t lock; @@ -546,11 +545,9 @@ static int qcom_qspi_probe(struct platform_device *pdev) } /* OPP table is optional */ ret = dev_pm_opp_of_add_table(&pdev->dev); - if (!ret) { - ctrl->has_opp_table = true; - } else if (ret != -ENODEV) { + if (ret && ret != -ENODEV) { dev_err(&pdev->dev, "invalid OPP table in device tree\n"); - goto exit_probe_master_put; + goto exit_probe_put_clkname; } pm_runtime_use_autosuspend(dev); @@ -562,8 +559,9 @@ static int qcom_qspi_probe(struct platform_device *pdev) return 0; pm_runtime_disable(dev); - if (ctrl->has_opp_table) - dev_pm_opp_of_remove_table(&pdev->dev); + dev_pm_opp_of_remove_table(&pdev->dev); + +exit_probe_put_clkname: dev_pm_opp_put_clkname(ctrl->opp_table); exit_probe_master_put: @@ -581,8 +579,7 @@ static int qcom_qspi_remove(struct platform_device *pdev) spi_unregister_master(master); pm_runtime_disable(&pdev->dev); - if (ctrl->has_opp_table) - dev_pm_opp_of_remove_table(&pdev->dev); + dev_pm_opp_of_remove_table(&pdev->dev); dev_pm_opp_put_clkname(ctrl->opp_table); return 0; -- 2.25.0.rc1.19.g042ed3e048af
[PATCH V2 3/8] drm/msm: Unconditionally call dev_pm_opp_of_remove_table()
dev_pm_opp_of_remove_table() doesn't report any errors when it fails to find the OPP table with error -ENODEV (i.e. OPP table not present for the device). And we can call dev_pm_opp_of_remove_table() unconditionally here. While at it, also create a label to put clkname. Signed-off-by: Viresh Kumar --- V2: - Compare with -ENODEV only for failures. - Create new label to put clkname. --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 14 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 - drivers/gpu/drm/msm/dsi/dsi_host.c | 8 ++-- 3 files changed, 7 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c index c0a4d4e16d82..c8287191951f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c @@ -1010,12 +1010,9 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) return PTR_ERR(dpu_kms->opp_table); /* OPP table is optional */ ret = dev_pm_opp_of_add_table(dev); - if (!ret) { - dpu_kms->has_opp_table = true; - } else if (ret != -ENODEV) { + if (ret && ret != -ENODEV) { dev_err(dev, "invalid OPP table in device tree\n"); - dev_pm_opp_put_clkname(dpu_kms->opp_table); - return ret; + goto put_clkname; } mp = &dpu_kms->mp; @@ -1037,8 +1034,8 @@ static int dpu_bind(struct device *dev, struct device *master, void *data) priv->kms = &dpu_kms->base; return ret; err: - if (dpu_kms->has_opp_table) - dev_pm_opp_of_remove_table(dev); + dev_pm_opp_of_remove_table(dev); +put_clkname: dev_pm_opp_put_clkname(dpu_kms->opp_table); return ret; } @@ -1056,8 +1053,7 @@ static void dpu_unbind(struct device *dev, struct device *master, void *data) if (dpu_kms->rpm_enabled) pm_runtime_disable(&pdev->dev); - if (dpu_kms->has_opp_table) - dev_pm_opp_of_remove_table(dev); + dev_pm_opp_of_remove_table(dev); dev_pm_opp_put_clkname(dpu_kms->opp_table); } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h index e140cd633071..8295979a7165 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h @@ -129,7 +129,6 @@ struct dpu_kms { bool rpm_enabled; struct opp_table *opp_table; - bool has_opp_table; struct dss_module_power mp; diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index b17ac6c27554..4335fe33250c 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -113,7 +113,6 @@ struct msm_dsi_host { struct clk *byte_intf_clk; struct opp_table *opp_table; - bool has_opp_table; u32 byte_clk_rate; u32 pixel_clk_rate; @@ -1891,9 +1890,7 @@ int msm_dsi_host_init(struct msm_dsi *msm_dsi) return PTR_ERR(msm_host->opp_table); /* OPP table is optional */ ret = dev_pm_opp_of_add_table(&pdev->dev); - if (!ret) { - msm_host->has_opp_table = true; - } else if (ret != -ENODEV) { + if (ret && ret != -ENODEV) { dev_err(&pdev->dev, "invalid OPP table in device tree\n"); dev_pm_opp_put_clkname(msm_host->opp_table); return ret; @@ -1934,8 +1931,7 @@ void msm_dsi_host_destroy(struct mipi_dsi_host *host) mutex_destroy(&msm_host->cmd_mutex); mutex_destroy(&msm_host->dev_mutex); - if (msm_host->has_opp_table) - dev_pm_opp_of_remove_table(&msm_host->pdev->dev); + dev_pm_opp_of_remove_table(&msm_host->pdev->dev); dev_pm_opp_put_clkname(msm_host->opp_table); pm_runtime_disable(&msm_host->pdev->dev); } -- 2.25.0.rc1.19.g042ed3e048af
[PATCH V2 0/8] opp: Unconditionally call dev_pm_opp_of_remove_table()
Hello, This cleans up some of the user code around calls to dev_pm_opp_of_remove_table(). All the patches can be picked by respective maintainers directly except for the last patch, which needs the previous two to get merged first. These are based for 5.9-rc1. Rajendra, Since most of these changes are related to qcom stuff, it would be great if you can give them a try. I wasn't able to test them due to lack of hardware. Ulf, I had to revise the sdhci patch, sorry about that. Please pick this one. Diff between V1 and V2 is mentioned in each of the patches separately. Viresh Kumar (8): cpufreq: imx6q: Unconditionally call dev_pm_opp_of_remove_table() drm/lima: Unconditionally call dev_pm_opp_of_remove_table() drm/msm: Unconditionally call dev_pm_opp_of_remove_table() mmc: sdhci-msm: Unconditionally call dev_pm_opp_of_remove_table() spi: spi-geni-qcom: Unconditionally call dev_pm_opp_of_remove_table() spi: spi-qcom-qspi: Unconditionally call dev_pm_opp_of_remove_table() tty: serial: qcom_geni_serial: Unconditionally call dev_pm_opp_of_remove_table() qcom-geni-se: remove has_opp_table drivers/cpufreq/imx6q-cpufreq.c | 10 ++ drivers/gpu/drm/lima/lima_devfreq.c | 6 +- drivers/gpu/drm/lima/lima_devfreq.h | 1 - drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 14 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 1 - drivers/gpu/drm/msm/dsi/dsi_host.c | 8 ++-- drivers/mmc/host/sdhci-msm.c| 14 +- drivers/spi/spi-geni-qcom.c | 13 + drivers/spi/spi-qcom-qspi.c | 15 ++- drivers/tty/serial/qcom_geni_serial.c | 13 + include/linux/qcom-geni-se.h| 2 -- 11 files changed, 31 insertions(+), 66 deletions(-) base-commit: 9123e3a74ec7b934a4a099e98af6a61c2f80bbf5 -- 2.25.0.rc1.19.g042ed3e048af
Re: [PATCH 06/23] rxrpc: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
Hi Chunguang, Thank you for the patch! Yet something to improve: [auto build test ERROR on mkp-scsi/for-next] [also build test ERROR on scsi/for-next block/for-next linus/master asm-generic/master v5.9-rc2 next-20200827] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Chunguang-Xu/clean-up-the-code-related-to-ASSERT/20200827-182148 base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next config: x86_64-randconfig-a015-20200827 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 71f3169e1baeff262583b35ef88f8fb6df7be85e) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): >> net/rxrpc/af_rxrpc.c:226:3: error: use of undeclared identifier 'x' ASSERT(rx->local != NULL); ^ net/rxrpc/ar-internal.h:1184:31: note: expanded from macro 'ASSERT' #define ASSERT(X) ASSERT_FAIL(x) ^ >> net/rxrpc/af_rxrpc.c:226:3: error: use of undeclared identifier 'x' net/rxrpc/ar-internal.h:1184:31: note: expanded from macro 'ASSERT' #define ASSERT(X) ASSERT_FAIL(x) ^ 2 errors generated. -- >> net/rxrpc/call_accept.c:479:2: error: use of undeclared identifier 'x' ASSERT(!irqs_disabled()); ^ net/rxrpc/ar-internal.h:1184:31: note: expanded from macro 'ASSERT' #define ASSERT(X) ASSERT_FAIL(x) ^ >> net/rxrpc/call_accept.c:479:2: error: use of undeclared identifier 'x' net/rxrpc/ar-internal.h:1184:31: note: expanded from macro 'ASSERT' #define ASSERT(X) ASSERT_FAIL(x) ^ net/rxrpc/call_accept.c:602:2: error: use of undeclared identifier 'x' ASSERT(!irqs_disabled()); ^ net/rxrpc/ar-internal.h:1184:31: note: expanded from macro 'ASSERT' #define ASSERT(X) ASSERT_FAIL(x) ^ net/rxrpc/call_accept.c:602:2: error: use of undeclared identifier 'x' net/rxrpc/ar-internal.h:1184:31: note: expanded from macro 'ASSERT' #define ASSERT(X) ASSERT_FAIL(x) ^ 4 errors generated. -- >> net/rxrpc/call_event.c:177:2: error: use of undeclared identifier 'x' ASSERT(before_eq(cursor, top)); ^ net/rxrpc/ar-internal.h:1184:31: note: expanded from macro 'ASSERT' #define ASSERT(X) ASSERT_FAIL(x) ^ >> net/rxrpc/call_event.c:177:2: error: use of undeclared identifier 'x' net/rxrpc/ar-internal.h:1184:31: note: expanded from macro 'ASSERT' #define ASSERT(X) ASSERT_FAIL(x) ^ 2 errors generated. -- >> net/rxrpc/call_object.c:555:2: error: use of undeclared identifier 'x' ASSERT(call != NULL); ^ net/rxrpc/ar-internal.h:1184:31: note: expanded from macro 'ASSERT' #define ASSERT(X) ASSERT_FAIL(x) ^ >> net/rxrpc/call_object.c:555:2: error: use of undeclared identifier 'x' net/rxrpc/ar-internal.h:1184:31: note: expanded from macro 'ASSERT' #define ASSERT(X) ASSERT_FAIL(x) ^ net/rxrpc/call_object.c:619:2: error: use of undeclared identifier 'x' ASSERT(test_bit(RXRPC_CALL_RELEASED, &call->flags)); ^ net/rxrpc/ar-internal.h:1184:31: note: expanded from macro 'ASSERT' #define ASSERT(X) ASSERT_FAIL(x) ^ net/rxrpc/call_object.c:619:2: error: use of undeclared identifier 'x' net/rxrpc/ar-internal.h:1184:31: note: expanded from macro 'ASSERT' #define ASSERT(X) ASSERT_FAIL(x) ^ 4 errors generated. -- >> net/rxrpc/conn_client.c:811:3: error: use of undeclared identifier 'x' ASSERT(!test_bit(RXRPC_CALL_EXPOSED, &call->fl
Re: drivers/net/wireless/realtek/rtw88/pci.c:1477:5: warning: no previous prototype for 'rtw_pci_probe'
Tony Chuang writes: >> + linux-wireless >> >> kernel test robot writes: >> >> > Hi Zong-Zhe, >> > >> > FYI, the error/warning still remains. >> > >> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git >> master >> > head: 23ee3e4e5bd27bdbc0f1785eef7209ce872794c7 >> > commit: 72f256c2b948622cc45ff8bc0456dd6039d8fe36 rtw88: extract: >> > export symbols about pci interface >> > date: 10 weeks ago >> > config: arc-randconfig-r026-20200725 (attached as .config) >> > compiler: arc-elf-gcc (GCC) 9.3.0 >> > reproduce (this is a W=1 build): >> > wget >> > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross >> > -O ~/bin/make.cross >> > chmod +x ~/bin/make.cross >> > git checkout 72f256c2b948622cc45ff8bc0456dd6039d8fe36 >> > # save the attached .config to linux build tree >> > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 >> make.cross ARCH=arc >> > >> > If you fix the issue, kindly add following tag as appropriate >> > Reported-by: kernel test robot >> > >> > All warnings (new ones prefixed by >>): >> > >> >>> drivers/net/wireless/realtek/rtw88/pci.c:1477:5: warning: no >> >>> previous prototype for 'rtw_pci_probe' [-Wmissing-prototypes] >> > 1477 | int rtw_pci_probe(struct pci_dev *pdev, >> > | ^ >> >>> drivers/net/wireless/realtek/rtw88/pci.c:1557:6: warning: no >> >>> previous prototype for 'rtw_pci_remove' [-Wmissing-prototypes] >> > 1557 | void rtw_pci_remove(struct pci_dev *pdev) >> > | ^~ >> >>> drivers/net/wireless/realtek/rtw88/pci.c:1579:6: warning: no >> >>> previous prototype for 'rtw_pci_shutdown' [-Wmissing-prototypes] >> > 1579 | void rtw_pci_shutdown(struct pci_dev *pdev) >> > | ^~~~ >> >> Tony, these are older warnings but please also check these. >> > > I think this warning can be ignored, as the commit was going to export > pci symbols for the follow-up patches to use, such as: > > f56f08636dda rtw88: extract: make 8723d an individual kernel module > 416e87fcc780 rtw88: extract: make 8822b an individual kernel module > ba0fbe236fb8 rtw88: extract: make 8822c an individual kernel module > > And these patches were submitted and applied together. Good, thanks for checking. -- https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH 1/4] drivers: hv: dxgkrnl: core code
On Thu, Aug 27, 2020 at 04:29:59PM -0700, Iouri Tarassov wrote: > On 8/14/2020 5:55 AM, Greg KH wrote: > > On Fri, Aug 14, 2020 at 08:38:53AM -0400, Sasha Levin wrote: > > > Add support for a Hyper-V based vGPU implementation that exposes the > > > DirectX API to Linux userspace. > > > > Signed-off-by: Sasha Levin > > > > Better, but what is this mess: > > > > > +#define ISERROR(ret) (ret < 0) > > The VM bus messages return the NTSTATUS error code from the host. NTSTATUS > is integer and the positive values indicate success. > The success error code needs to be returned by IOCTLs to the DxCore > applications. The ISERROR() macro is used in places where the NTSTATUS > success code could be returned from a function. "if (ret)" cannot be used. I > will add a comment to the macro in the next patch. No, please just "open code" this, there is no need for a macro that is actually more characters than the original test. > > > +#define DXGKDEBUG 1 > > > +/* #define USEPRINTK 1 */ > > > + > > > +#ifndef DXGKDEBUG > > > +#define TRACE_DEBUG(...) > > > +#define TRACE_DEFINE(...) > > > +#define TRACE_FUNC_ENTER(...) > > > +#define TRACE_FUNC_EXIT(...) > > > > No, please do not to custom "trace" printk messages, that is what ftrace > > is for, no individual driver should ever need to do that. > > > > Just use the normal dev_*() calls for error messages and the like, do > > not try to come up with a custom tracing framework for one tiny > > individual driver. If every driver in kernel did that, we would have a > > nightmare... > I understand the concern. This will be fixed later when we learn and pick > the final tracing technology for the driver. When is "later"? We don't want to review something that you do not think is ready to be merged, do it now so we don't trip over things that you know you want to fix up. > The current implementation > allows the hardware vendors to quickly identify issues without learning > about ftrace. They just need to enable the WSL debug console and mount > debugfs. Hardware vendors who know Linux already know about ftrace, don't make people have to read up and learn about custom ways to debug just-your-tiny-individual-driver. Instead follow the customs and ways the _WHOLE_ kernel works, that is just polite, don't you think? thanks, greg k-h
Re: [PATCH v4 1/4] fpga: dfl: change data type of feature id to u16
On Wed, Aug 19, 2020 at 09:14:31PM -0700, Moritz Fischer wrote: > On Thu, Aug 13, 2020 at 05:04:09PM +0800, Xu Yilun wrote: > > On Thu, Aug 13, 2020 at 08:28:05AM +, David Laight wrote: > > > From: Xu Yilun > > > > Sent: 13 August 2020 08:59 > > > > On Wed, Aug 12, 2020 at 08:52:39AM +, David Laight wrote: > > > > > From: Moritz Fischer > > > > > > Sent: 12 August 2020 04:56 > > > > > > > > > > > > On Mon, Aug 10, 2020 at 10:41:10AM +0800, Xu Yilun wrote: > > > > > > > The feature id is stored in a 12 bit field in DFH. So a u16 > > > > > > > variable is > > > > > > > enough for feature id. > > > > > > > > > > > > > > This patch changes all feature id related places to fit u16. > > > > > > > > > > How much bigger does it make the kernel? > > > > > > > > The patch changes the definition of feature id from u64 to u16, and will > > > > make the kernel slightly smaller. > > > > > > Unlikely. > > > Most of the structures will gain a 'pad' field. > > > Using u16 for function parameters and results almost certainly > > > requires instructions to mask the value. > > > Any arithmetic on u16 will require masking instructions on > > > (probably) all architectures except x86. > > > > > > Using 'unsigned int' is probably best. > > > > > > u16 is never a good idea unless you are defining enough > > > of them in a structure (eg as an array) to reduce the > > > structure size below some threshold. > > > (Or are matching some hardware layout.) > > > > I got it. Thanks for your detailed explanation. I think we may change them > > to > > u32. Is it the same case for u8? Think we may also change the > > dfl_device_id.type. > > > > > > Hi Moritz: > > > > The patch is applied to for-next, is it possible we recall it, or we > > make another fix after it? > > > > Thanks, > > Yilun. > > Sorry for the delay, can you send a follow-up please? Hi moritz: I think I don't have to change it now. As discussed with David, these fields aren't often accessed. So it should be OK. Thanks, Yilun. > > Cheers, > Moritz
[PATCH v2 5/5] powerpc/vdso: Declare vdso_patches[] as __initdata
vdso_patches[] table is used only at init time. Mark it __initdata. Signed-off-by: Christophe Leroy --- arch/powerpc/kernel/vdso.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c index 4ad042995ccc..dfa08a7b4e7c 100644 --- a/arch/powerpc/kernel/vdso.c +++ b/arch/powerpc/kernel/vdso.c @@ -76,7 +76,7 @@ struct vdso_patch_def * Currently, we only change sync_dicache to do nothing on processors * with a coherent icache */ -static struct vdso_patch_def vdso_patches[] = { +static struct vdso_patch_def vdso_patches[] __initdata = { { CPU_FTR_COHERENT_ICACHE, CPU_FTR_COHERENT_ICACHE, "__kernel_sync_dicache", "__kernel_sync_dicache_p5" -- 2.25.0
[PATCH v2 2/5] powerpc/vdso: Don't rely on vdso_pages being 0 for failure
If vdso initialisation failed, vdso_ready is not set. Otherwise, vdso_pages is only 0 when it is a 32 bits task and CONFIG_VDSO32 is not selected. As arch_setup_additional_pages() now bails out directly in that case, we don't need to set vdso_pages to 0. Signed-off-by: Christophe Leroy --- arch/powerpc/kernel/vdso.c | 23 ++- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c index 3ef3fc546ac8..8f245e988a8a 100644 --- a/arch/powerpc/kernel/vdso.c +++ b/arch/powerpc/kernel/vdso.c @@ -176,11 +176,6 @@ int arch_setup_additional_pages(struct linux_binprm *bprm, int uses_interp) current->mm->context.vdso_base = 0; - /* vDSO has a problem and was disabled, just don't "enable" it for the -* process -*/ - if (vdso_pages == 0) - return 0; /* Add a page to the vdso size for the data page */ vdso_pages ++; @@ -710,14 +705,16 @@ static int __init vdso_init(void) * Initialize the vDSO images in memory, that is do necessary * fixups of vDSO symbols, locate trampolines, etc... */ - if (vdso_setup()) - goto setup_failed; + if (vdso_setup()) { + pr_err("vDSO setup failure, not enabled !\n"); + return 0; + } if (IS_ENABLED(CONFIG_VDSO32)) { /* Make sure pages are in the correct state */ pagelist = kcalloc(vdso32_pages + 1, sizeof(struct page *), GFP_KERNEL); if (!pagelist) - goto alloc_failed; + return 0; pagelist[0] = virt_to_page(vdso_data); @@ -730,7 +727,7 @@ static int __init vdso_init(void) if (IS_ENABLED(CONFIG_PPC64)) { pagelist = kcalloc(vdso64_pages + 1, sizeof(struct page *), GFP_KERNEL); if (!pagelist) - goto alloc_failed; + return 0; pagelist[0] = virt_to_page(vdso_data); @@ -743,14 +740,6 @@ static int __init vdso_init(void) smp_wmb(); vdso_ready = 1; - return 0; - -setup_failed: - pr_err("vDSO setup failure, not enabled !\n"); -alloc_failed: - vdso32_pages = 0; - vdso64_pages = 0; - return 0; } arch_initcall(vdso_init); -- 2.25.0
[PATCH v2 4/5] powerpc/vdso: Declare constant vars as __ro_after_init
To avoid any risk of modification of vital VDSO variables, declare them __ro_after_init. vdso32_kbase and vdso64_kbase could be made 'const', but it would have high impact on all functions using them as the compiler doesn't expect const property to be discarded. Signed-off-by: Christophe Leroy --- arch/powerpc/kernel/vdso.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c index fb393266b9cb..4ad042995ccc 100644 --- a/arch/powerpc/kernel/vdso.c +++ b/arch/powerpc/kernel/vdso.c @@ -38,19 +38,19 @@ #define VDSO_ALIGNMENT (1 << 16) extern char vdso32_start, vdso32_end; -static unsigned int vdso32_pages; -static void *vdso32_kbase = &vdso32_start; -unsigned long vdso32_sigtramp; -unsigned long vdso32_rt_sigtramp; +static unsigned int vdso32_pages __ro_after_init; +static void *vdso32_kbase __ro_after_init = &vdso32_start; +unsigned long vdso32_sigtramp __ro_after_init; +unsigned long vdso32_rt_sigtramp __ro_after_init; extern char vdso64_start, vdso64_end; -static void *vdso64_kbase = &vdso64_start; -static unsigned int vdso64_pages; +static void *vdso64_kbase __ro_after_init = &vdso64_start; +static unsigned int vdso64_pages __ro_after_init; #ifdef CONFIG_PPC64 -unsigned long vdso64_rt_sigtramp; +unsigned long vdso64_rt_sigtramp __ro_after_init; #endif /* CONFIG_PPC64 */ -static int vdso_ready; +static int vdso_ready __ro_after_init; /* * The vdso data page (aka. systemcfg for old ppc64 fans) is here. -- 2.25.0
[PATCH v2 1/5] powerpc/vdso: Remove DBG()
DBG() is defined as void when DEBUG is not defined, and DEBUG is explicitly undefined. It means there is no other way than modifying source code to get the messages printed. It was most likely useful in the first days of VDSO, but today the only 3 DBG() calls don't deserve a special handling. Just remove them. If one day someone need such messages back, use a standard pr_debug() or equivalent. Signed-off-by: Christophe Leroy --- This is a follow up series, applying on top of the series that switches powerpc VDSO to _install_special_mapping(), rebased on today's powerpc/next-test (dd419a93bd99) v2 removes the modification to arch_setup_additional_pages() to consider when is_32bit_task() returning true when CONFIG_VDSO32 not set, as this should never happen. Signed-off-by: Christophe Leroy --- arch/powerpc/kernel/vdso.c | 13 - 1 file changed, 13 deletions(-) diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c index e2568d9ecdff..3ef3fc546ac8 100644 --- a/arch/powerpc/kernel/vdso.c +++ b/arch/powerpc/kernel/vdso.c @@ -31,14 +31,6 @@ #include #include -#undef DEBUG - -#ifdef DEBUG -#define DBG(fmt...) printk(fmt) -#else -#define DBG(fmt...) -#endif - /* Max supported size for symbol names */ #define MAX_SYMNAME64 @@ -567,9 +559,6 @@ static __init int vdso_fixup_alt_funcs(struct lib32_elfinfo *v32, if (!match) continue; - DBG("replacing %s with %s...\n", patch->gen_name, - patch->fix_name ? "NONE" : patch->fix_name); - /* * Patch the 32 bits and 64 bits symbols. Note that we do not * patch the "." symbol on 64 bits. @@ -704,7 +693,6 @@ static int __init vdso_init(void) * Calculate the size of the 64 bits vDSO */ vdso64_pages = (&vdso64_end - &vdso64_start) >> PAGE_SHIFT; - DBG("vdso64_kbase: %p, 0x%x pages\n", vdso64_kbase, vdso64_pages); vdso32_kbase = &vdso32_start; @@ -712,7 +700,6 @@ static int __init vdso_init(void) * Calculate the size of the 32 bits vDSO */ vdso32_pages = (&vdso32_end - &vdso32_start) >> PAGE_SHIFT; - DBG("vdso32_kbase: %p, 0x%x pages\n", vdso32_kbase, vdso32_pages); /* * Setup the syscall map in the vDOS -- 2.25.0
[PATCH v2 3/5] powerpc/vdso: Initialise vdso32_kbase at compile time
Initialise vdso32_kbase at compile time like vdso64_kbase. Signed-off-by: Christophe Leroy --- arch/powerpc/kernel/vdso.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c index 8f245e988a8a..fb393266b9cb 100644 --- a/arch/powerpc/kernel/vdso.c +++ b/arch/powerpc/kernel/vdso.c @@ -37,13 +37,12 @@ /* The alignment of the vDSO */ #define VDSO_ALIGNMENT (1 << 16) +extern char vdso32_start, vdso32_end; static unsigned int vdso32_pages; -static void *vdso32_kbase; +static void *vdso32_kbase = &vdso32_start; unsigned long vdso32_sigtramp; unsigned long vdso32_rt_sigtramp; -extern char vdso32_start, vdso32_end; - extern char vdso64_start, vdso64_end; static void *vdso64_kbase = &vdso64_start; static unsigned int vdso64_pages; @@ -689,8 +688,6 @@ static int __init vdso_init(void) */ vdso64_pages = (&vdso64_end - &vdso64_start) >> PAGE_SHIFT; - vdso32_kbase = &vdso32_start; - /* * Calculate the size of the 32 bits vDSO */ -- 2.25.0
Re: [PATCH v3 2/3] usb typec: mt6360: Rename driver/Kconfig/Makefile from mt6360 to mt636x
Guenter Roeck 於 2020年8月28日 週五 上午12:41寫道: > > On Thu, Aug 27, 2020 at 07:18:56PM +0800, cy_huang wrote: > > From: ChiYuan Huang > > > > 1. Rename file form tcpci_mt6360.c to tcpci_mt636x.c > > 2. Rename internal function from mt6360 to mt636x, except the register > > definition. > > 3. Change Kconfig/Makefile from MT6360 to MT636X. > > > > Signed-off-by: ChiYuan Huang > > --- > > drivers/usb/typec/tcpm/Kconfig| 6 +- > > drivers/usb/typec/tcpm/Makefile | 2 +- > > drivers/usb/typec/tcpm/tcpci_mt6360.c | 212 > > -- > > drivers/usb/typec/tcpm/tcpci_mt636x.c | 212 > > ++ > > 4 files changed, 216 insertions(+), 216 deletions(-) > > delete mode 100644 drivers/usb/typec/tcpm/tcpci_mt6360.c > > create mode 100644 drivers/usb/typec/tcpm/tcpci_mt636x.c > > Maybe Heikki is ok with this change, but I am not, for the reasons > mentioned before. So I won't approve this patch. Note that, either > case, it should be merged with the first patch. Yes, I agree with you opinion. use 636x, the range is too large from 0 to 9, it may not all be compatible. Even though it's also possible that the part number don't have the same function. So I'm going to remove the rename patch. Do I need to add a patch named "revert"? Or just remove it. I'm not sure which way is better. It seems you all want the code change to be squashed into the first code. And the second one is the DT binding. Right? > > Guenter
[PATCH] f2fs: prevent compressed file from being disabled after releasing cblocks
From: Daeho Jeong After releasing cblocks, the compressed file can be accidentally disabled in compression mode, since it has zero cblocks. As we are using IMMUTABLE flag to present released cblocks state, we can add IMMUTABLE state check when considering the compressed file disabling. Signed-off-by: Daeho Jeong --- fs/f2fs/f2fs.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 02811ce15059..14d30740ba03 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -3936,6 +3936,8 @@ static inline u64 f2fs_disable_compressed_file(struct inode *inode) if (!f2fs_compressed_file(inode)) return 0; if (S_ISREG(inode->i_mode)) { + if (IS_IMMUTABLE(inode)) + return 1; if (get_dirty_pages(inode)) return 1; if (fi->i_compr_blocks) -- 2.28.0.402.g5ffc5be6b7-goog
Re: [PATCH v1 4/9] powerpc/vdso: Remove unnecessary ifdefs in vdso_pagelist initialization
Le 28/08/2020 à 07:40, Christophe Leroy a écrit : Le 27/08/2020 à 15:19, Michael Ellerman a écrit : Christophe Leroy writes: On 08/26/2020 02:58 PM, Michael Ellerman wrote: Christophe Leroy writes: diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c index daef14a284a3..bbb69832fd46 100644 --- a/arch/powerpc/kernel/vdso.c +++ b/arch/powerpc/kernel/vdso.c @@ -718,16 +710,14 @@ static int __init vdso_init(void) ... - -#ifdef CONFIG_VDSO32 vdso32_kbase = &vdso32_start; /* @@ -735,8 +725,6 @@ static int __init vdso_init(void) */ vdso32_pages = (&vdso32_end - &vdso32_start) >> PAGE_SHIFT; DBG("vdso32_kbase: %p, 0x%x pages\n", vdso32_kbase, vdso32_pages); -#endif This didn't build for ppc64le: /opt/cross/gcc-8.20_binutils-2.32/powerpc64-unknown-linux-gnu/bin/powerpc64-unknown-linux-gnu-ld: arch/powerpc/kernel/vdso.o:(.toc+0x0): undefined reference to `vdso32_end' /opt/cross/gcc-8.20_binutils-2.32/powerpc64-unknown-linux-gnu/bin/powerpc64-unknown-linux-gnu-ld: arch/powerpc/kernel/vdso.o:(.toc+0x8): undefined reference to `vdso32_start' make[1]: *** [/scratch/michael/build/maint/Makefile:1166: vmlinux] Error 1 make: *** [Makefile:185: __sub-make] Error 2 So I just put that ifdef back. The problem is because is_32bit() can still return true even when CONFIG_VDSO32 is not set. Hmm, you're right. My config had CONFIG_COMPAT enabled. But that seems like a bug, if someone enables COMPAT on ppc64le they are almost certainly going to want VDSO32 as well. So I think I'll do a lead up patch as below. Ah yes, and with that then no need to consider the case where is_32bit_task() is true and CONFIG_VDSO32 is not selected. I'll update my leading series accordingly. I meant follow up series. Christophe Christophe cheers diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype index d4fd109f177e..cf2da1e401ef 100644 --- a/arch/powerpc/platforms/Kconfig.cputype +++ b/arch/powerpc/platforms/Kconfig.cputype @@ -501,13 +501,12 @@ endmenu config VDSO32 def_bool y - depends on PPC32 || CPU_BIG_ENDIAN + depends on PPC32 || COMPAT help This symbol controls whether we build the 32-bit VDSO. We obviously want to do that if we're building a 32-bit kernel. If we're building - a 64-bit kernel then we only want a 32-bit VDSO if we're building for - big endian. That is because the only little endian configuration we - support is ppc64le which is 64-bit only. + a 64-bit kernel then we only want a 32-bit VDSO if we're also enabling + COMPAT. choice prompt "Endianness selection"
[PATCH 0/1] Remove redundant condition for MTK_TIMER
Remove the redundant condition of MTK_TIMER because the driver can work on MTK platform normally, so COMPILE_TEST is no longer needed for development purpose Freddy Hsin (1): timer: mt6873: remove COMPILE_TEST condition for MTK timer drivers/clocksource/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH v1 1/1] timer: mt6873: remove COMPILE_TEST condition for MTK timer
MTK timer driver can work on MTK platform normally, so remove the redundant condition for MTK_TIMER Signed-off-by: Freddy Hsin --- drivers/clocksource/Kconfig |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig index 9141838..1ec5d94 100644 --- a/drivers/clocksource/Kconfig +++ b/drivers/clocksource/Kconfig @@ -472,7 +472,7 @@ config SYS_SUPPORTS_SH_CMT bool config MTK_TIMER - bool "Mediatek timer driver" if COMPILE_TEST + bool "Mediatek timer driver" depends on HAS_IOMEM select TIMER_OF select CLKSRC_MMIO -- 1.7.9.5
Re: [PATCH v1 4/9] powerpc/vdso: Remove unnecessary ifdefs in vdso_pagelist initialization
Le 27/08/2020 à 15:19, Michael Ellerman a écrit : Christophe Leroy writes: On 08/26/2020 02:58 PM, Michael Ellerman wrote: Christophe Leroy writes: diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c index daef14a284a3..bbb69832fd46 100644 --- a/arch/powerpc/kernel/vdso.c +++ b/arch/powerpc/kernel/vdso.c @@ -718,16 +710,14 @@ static int __init vdso_init(void) ... - -#ifdef CONFIG_VDSO32 vdso32_kbase = &vdso32_start; /* @@ -735,8 +725,6 @@ static int __init vdso_init(void) */ vdso32_pages = (&vdso32_end - &vdso32_start) >> PAGE_SHIFT; DBG("vdso32_kbase: %p, 0x%x pages\n", vdso32_kbase, vdso32_pages); -#endif This didn't build for ppc64le: /opt/cross/gcc-8.20_binutils-2.32/powerpc64-unknown-linux-gnu/bin/powerpc64-unknown-linux-gnu-ld: arch/powerpc/kernel/vdso.o:(.toc+0x0): undefined reference to `vdso32_end' /opt/cross/gcc-8.20_binutils-2.32/powerpc64-unknown-linux-gnu/bin/powerpc64-unknown-linux-gnu-ld: arch/powerpc/kernel/vdso.o:(.toc+0x8): undefined reference to `vdso32_start' make[1]: *** [/scratch/michael/build/maint/Makefile:1166: vmlinux] Error 1 make: *** [Makefile:185: __sub-make] Error 2 So I just put that ifdef back. The problem is because is_32bit() can still return true even when CONFIG_VDSO32 is not set. Hmm, you're right. My config had CONFIG_COMPAT enabled. But that seems like a bug, if someone enables COMPAT on ppc64le they are almost certainly going to want VDSO32 as well. So I think I'll do a lead up patch as below. Ah yes, and with that then no need to consider the case where is_32bit_task() is true and CONFIG_VDSO32 is not selected. I'll update my leading series accordingly. Christophe cheers diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype index d4fd109f177e..cf2da1e401ef 100644 --- a/arch/powerpc/platforms/Kconfig.cputype +++ b/arch/powerpc/platforms/Kconfig.cputype @@ -501,13 +501,12 @@ endmenu config VDSO32 def_bool y - depends on PPC32 || CPU_BIG_ENDIAN + depends on PPC32 || COMPAT help This symbol controls whether we build the 32-bit VDSO. We obviously want to do that if we're building a 32-bit kernel. If we're building - a 64-bit kernel then we only want a 32-bit VDSO if we're building for - big endian. That is because the only little endian configuration we - support is ppc64le which is 64-bit only. + a 64-bit kernel then we only want a 32-bit VDSO if we're also enabling + COMPAT. choice prompt "Endianness selection"
Re: [PATCH 09/23] cachefiles: use ASSERT_FAIL()/ASSERT_WARN() to cleanup some code
Hi Chunguang, Thank you for the patch! Yet something to improve: [auto build test ERROR on mkp-scsi/for-next] [also build test ERROR on scsi/for-next block/for-next linus/master asm-generic/master v5.9-rc2 next-20200827] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Chunguang-Xu/clean-up-the-code-related-to-ASSERT/20200827-182148 base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next config: sh-allmodconfig (attached as .config) compiler: sh4-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sh If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All error/warnings (new ones prefixed by >>): In file included from include/linux/kernel.h:11, from include/linux/list.h:9, from include/linux/module.h:12, from fs/cachefiles/bind.c:8: fs/cachefiles/bind.c: In function 'cachefiles_daemon_bind': >> fs/cachefiles/internal.h:319:31: error: 'x' undeclared (first use in this >> function) 319 | #define ASSERT(X) ASSERT_FAIL(x) | ^ include/linux/compiler.h:78:42: note: in definition of macro 'unlikely' 78 | # define unlikely(x) __builtin_expect(!!(x), 0) | ^ fs/cachefiles/internal.h:319:19: note: in expansion of macro 'ASSERT_FAIL' 319 | #define ASSERT(X) ASSERT_FAIL(x) | ^~~ fs/cachefiles/bind.c:39:2: note: in expansion of macro 'ASSERT' 39 | ASSERT(cache->fstop_percent >= 0 && | ^~ fs/cachefiles/internal.h:319:31: note: each undeclared identifier is reported only once for each function it appears in 319 | #define ASSERT(X) ASSERT_FAIL(x) | ^ include/linux/compiler.h:78:42: note: in definition of macro 'unlikely' 78 | # define unlikely(x) __builtin_expect(!!(x), 0) | ^ fs/cachefiles/internal.h:319:19: note: in expansion of macro 'ASSERT_FAIL' 319 | #define ASSERT(X) ASSERT_FAIL(x) | ^~~ fs/cachefiles/bind.c:39:2: note: in expansion of macro 'ASSERT' 39 | ASSERT(cache->fstop_percent >= 0 && | ^~ -- In file included from include/linux/kernel.h:11, from include/linux/list.h:9, from include/linux/module.h:12, from fs/cachefiles/daemon.c:8: fs/cachefiles/daemon.c: In function 'cachefiles_daemon_release': >> fs/cachefiles/internal.h:319:31: error: 'x' undeclared (first use in this >> function) 319 | #define ASSERT(X) ASSERT_FAIL(x) | ^ include/linux/compiler.h:78:42: note: in definition of macro 'unlikely' 78 | # define unlikely(x) __builtin_expect(!!(x), 0) | ^ fs/cachefiles/internal.h:319:19: note: in expansion of macro 'ASSERT_FAIL' 319 | #define ASSERT(X) ASSERT_FAIL(x) | ^~~ fs/cachefiles/daemon.c:135:2: note: in expansion of macro 'ASSERT' 135 | ASSERT(cache); | ^~ fs/cachefiles/internal.h:319:31: note: each undeclared identifier is reported only once for each function it appears in 319 | #define ASSERT(X) ASSERT_FAIL(x) | ^ include/linux/compiler.h:78:42: note: in definition of macro 'unlikely' 78 | # define unlikely(x) __builtin_expect(!!(x), 0) | ^ fs/cachefiles/internal.h:319:19: note: in expansion of macro 'ASSERT_FAIL' 319 | #define ASSERT(X) ASSERT_FAIL(x) | ^~~ fs/cachefiles/daemon.c:135:2: note: in expansion of macro 'ASSERT' 135 | ASSERT(cache); | ^~ fs/cachefiles/daemon.c: In function 'cachefiles_daemon_write': >> fs/cachefiles/internal.h:319:31: error: 'x' undeclared (first use in this >> function) 319 | #define ASSERT(X) ASSERT_FAIL(x) | ^ include/linux/compiler.h:78:42: note: in definition of macro 'unlikely' 78 | # define unlikely(x) __builtin_expec
linux-next: Tree for Aug 28
Hi all, News: There will be no linux-next releases next Monday or Tuesday. Changes since 20200827: The net-next tree gained a conflict against the net tree. Non-merge commits (relative to Linus' tree): 3122 3668 files changed, 104159 insertions(+), 39605 deletions(-) I have created today's linux-next tree at git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (patches at http://www.kernel.org/pub/linux/kernel/next/ ). If you are tracking the linux-next tree using git, you should not use "git pull" to do so as that will try to merge the new linux-next release with the old one. You should use "git fetch" and checkout or reset to the new master. You can see which trees have been included by looking in the Next/Trees file in the source. There are also quilt-import.log and merge.log files in the Next directory. Between each merge, the tree was built with a ppc64_defconfig for powerpc, an allmodconfig for x86_64, a multi_v7_defconfig for arm and a native build of tools/perf. After the final fixups (if any), I do an x86_64 modules_install followed by builds for x86_64 allnoconfig, powerpc allnoconfig (32 and 64 bit), ppc44x_defconfig, allyesconfig and pseries_le_defconfig and i386, sparc and sparc64 defconfig and htmldocs. And finally, a simple boot test of the powerpc pseries_le_defconfig kernel in qemu (with and without kvm enabled). Below is a summary of the state of the merge. I am currently merging 328 trees (counting Linus' and 86 trees of bug fix patches pending for the current merge release). Stats about the size of the tree over time can be seen at http://neuling.org/linux-next-size.html . Status of my local build tests will be at http://kisskb.ellerman.id.au/linux-next . If maintainers want to give advice about cross compilers/configs that work, we are always open to add more builds. Thanks to Randy Dunlap for doing many randconfig builds. And to Paul Gortmaker for triage and bug fixes. -- Cheers, Stephen Rothwell $ git checkout master $ git reset --hard stable Merging origin/master (15bc20c6af4c Merge tag 'tty-5.9-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty) Merging fixes/fixes (9123e3a74ec7 Linux 5.9-rc1) Merging kbuild-current/fixes (d012a7190fc1 Linux 5.9-rc2) Merging arc-current/for-curr (89d29997f103 irqchip/eznps: Fix build error for !ARC700 builds) Merging arm-current/fixes (5c6360ee4a0e ARM: 8988/1: mmu: fix crash in EFI calls due to p4d typo in create_mapping_late()) Merging arm64-fixes/for-next/fixes (8d75785a8142 ARM64: vdso32: Install vdso32 from vdso_install) Merging arm-soc-fixes/arm/fixes (9c8b0a9c37b7 Merge tag 'imx-fixes-5.9' of git://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux into arm/fixes) Merging uniphier-fixes/fixes (48778464bb7d Linux 5.8-rc2) Merging drivers-memory-fixes/fixes (7ff3a2a626f7 memory: jz4780_nemc: Fix an error pointer vs NULL check in probe()) Merging m68k-current/for-linus (382f429bb559 m68k: defconfig: Update defconfigs for v5.8-rc3) Merging powerpc-fixes/fixes (16d83a540ca4 Revert "powerpc/powernv/idle: Replace CPU feature check with PVR check") Merging s390-fixes/fixes (bffc2f7aa963 s390/vmem: fix vmem_add_range for 4-level paging) Merging sparc/master (0a95a6d1a4cd sparc: use for_each_child_of_node() macro) Merging fscrypt-current/for-stable (2b4eae95c736 fscrypt: don't evict dirty inodes after removing key) Merging net/master (b43c75abfd08 rxrpc: Fix memory leak in rxkad_verify_response()) Merging bpf/master (7787b6fc938e bpf, sysctl: Let bpf_stats_handler take a kernel pointer buffer) Merging ipsec/master (45a36a18d019 xfrmi: drop ignore_df check before updating pmtu) Merging netfilter/master (3622adb02623 ipv6: ndisc: adjust ndisc_ifinfo_sysctl_change prototype) Merging ipvs/master (7c7ab580db49 net: Convert to use the fallthrough macro) Merging wireless-drivers/master (4afc850e2e9e mwifiex: Increase AES key storage size to 256 bits) Merging mac80211/master (2d9b55508556 cfg80211: Adjust 6 GHz frequency to channel conversion) Merging rdma-fixes/for-rc (097a9d23b725 RDMA/bnxt_re: Remove the qp from list only if the qp destroy succeeds) Merging sound-current/for-linus (858e0ad9301d ALSA: hda/hdmi: always check pin power status in i915 pin fixup) Merging sound-asoc-fixes/for-linus (d563b6c834ae Merge series "ASoC: Fix return check for devm_regmap_init_sdw()" from Vinod Koul :) Merging regmap-fixes/for-linus (d012a7190fc1 Linux 5.9-rc2) Merging regulator-fixes/for-linus (3bec5b6aae83 Merge tag 'v5.9-rc2' into regulator-5.9) Merging spi-fixes/for-linus (3812e0343b42 Merge remote-tracking branch 'spi/for-5.9' into spi-linus) Merging pci-current/for-linus (7c2308f79fc8 PCI/P2PDMA: Fix build without DMA ops) Merging driver-core.current/driver-core-linus (d012a7190fc1 Linux 5.9-rc2) Merging t
Re: [PATCH v18 00/32] per memcg lru_lock
在 2020/8/28 上午9:40, Daniel Jordan 写道: > I went back to your v1 post to see what motivated you originally, and you had > some results from aim9 but nothing about where this reared its head in the > first place. How did you discover the bottleneck? I'm just curious about how > lru_lock hurts in practice. We have gotten very high 'sys' in some buiness/machines. And found much of time spent on the lru_lock and/or zone lock. Seems per memcg lru_lock could help this, but still no idea on zone lock. Thanks Alex
Re: Aw: Re: [PATCH v5 3/7] drm/mediatek: disable tmds on mt2701
Without this patch i have flickering/horizontal distortion (looks line every second line has different x position as one above approx. 5 px) on my 1280x1024 tft. Fbcon is unreadable with this problem. Hard to describe by words only :( Am 28. August 2020 01:46:07 MESZ schrieb Chun-Kuang Hu : >Hi, Frank: > >Matthias Brugger 於 2020年8月27日 週四 下午10:28寫道: >> >> >> >> On 27/08/2020 15:41, Frank Wunderlich wrote: >> > Hi Matthias, >> > >> > any opinions about the dts-changes? >> > >> >> they look good to me. >> >> > maybe series except the tmds-Patch get merged...so i add it only to >my own repo till we find a better way? >> > currently mainline does not support hdmi at all for the board. the >tmds-patch is only a fix for specific resolutions which have a >"flickering" without this Patch. >> > >> >> Well let's see what CK's opinion. >> > >Because no one has comment on this patch, I could apply this patch but >I need you to add more experiment information so if someone meets >another bug, he could fix his bug and consider your problem. > >Regards, >Chun-Kuang. > >> Regards, >> Matthias > >___ >Linux-mediatek mailing list >linux-media...@lists.infradead.org >http://lists.infradead.org/mailman/listinfo/linux-mediatek regards Frank
Re: [PATCH 6/6] gpio: zynq: Simplify with dev_err_probe()
On 27. 08. 20 22:08, Krzysztof Kozlowski wrote: > Common pattern of handling deferred probe can be simplified with > dev_err_probe(). Less code and also it prints the error value. > > Signed-off-by: Krzysztof Kozlowski > --- > drivers/gpio/gpio-zynq.c | 8 +++- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpio/gpio-zynq.c b/drivers/gpio/gpio-zynq.c > index 53d1387592fd..0b5a17ab996f 100644 > --- a/drivers/gpio/gpio-zynq.c > +++ b/drivers/gpio/gpio-zynq.c > @@ -929,11 +929,9 @@ static int zynq_gpio_probe(struct platform_device *pdev) > > /* Retrieve GPIO clock */ > gpio->clk = devm_clk_get(&pdev->dev, NULL); > - if (IS_ERR(gpio->clk)) { > - if (PTR_ERR(gpio->clk) != -EPROBE_DEFER) > - dev_err(&pdev->dev, "input clock not found.\n"); > - return PTR_ERR(gpio->clk); > - } > + if (IS_ERR(gpio->clk)) > + return dev_err_probe(&pdev->dev, PTR_ERR(gpio->clk), "input > clock not found.\n"); > + > ret = clk_prepare_enable(gpio->clk); > if (ret) { > dev_err(&pdev->dev, "Unable to enable clock.\n"); > Reviewed-by: Michal Simek Thanks, Michal
Re: [PATCH] mmc: sdhci-msm: When dev_pm_opp_of_add_table() returns 0 it's not an error
On 27-08-20, 08:33, Douglas Anderson wrote: > The commit d05a7238fe1c ("mmc: sdhci-msm: Unconditionally call > dev_pm_opp_of_remove_table()") works fine in the case where there is > no OPP table. However, if there is an OPP table then > dev_pm_opp_of_add_table() will return 0. Since 0 != -ENODEV then the > "if (ret != -ENODEV)" will evaluate to true and we'll fall into the > error case. Oops. > > Let's fix this. > > Fixes: d05a7238fe1c ("mmc: sdhci-msm: Unconditionally call > dev_pm_opp_of_remove_table()") > Signed-off-by: Douglas Anderson > --- > > drivers/mmc/host/sdhci-msm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c > index b7e47107a31a..55101dba42bd 100644 > --- a/drivers/mmc/host/sdhci-msm.c > +++ b/drivers/mmc/host/sdhci-msm.c > @@ -2284,7 +2284,7 @@ static int sdhci_msm_probe(struct platform_device *pdev) > > /* OPP table is optional */ > ret = dev_pm_opp_of_add_table(&pdev->dev); > - if (ret != -ENODEV) { > + if (ret && ret != -ENODEV) { > dev_err(&pdev->dev, "Invalid OPP table in Device tree\n"); > goto opp_cleanup; > } Wow! How many bugs did I introduce with a simple patch :( @Ulf, since this is material for 5.10 I was planning to resend the original patch itself with all the things fixed. Will you be able to rebase your tree? Or do you want to apply fixes separately ? -- viresh
Re: Printing bitfields in the kernel (Re: [PATCH] drm: Parse Colorimetry data block from EDID)
On Thu, 2020-08-27 at 10:34 +0300, Pekka Paalanen wrote: > On Wed, 26 Aug 2020 22:23:28 +0800 > Algea Cao wrote: > > > CEA 861.3 spec adds colorimetry data block for HDMI. > > Parsing the block to get the colorimetry data from > > panel. If flags are int, I could imagine another %p extension where %*p is used like: printk("flags: %*pn", flags, bitstrings) where flags is: BIT(0) BIT(1) ... BIT(last) and char *bitstrings[] = { "bit 0 description", "bit 1 description", ... "last bit description" }; Or define YA struct with 2 entries as the struct members and use that. struct foo { unsigned long flags, char ** descriptions, }; struct foo bar = {.flags = .descriptions = bitstrings}; printk("flags: %p\n, &bar);
Re: [RFC 0/2] Add risc-v vhost-net support
On Fri, Jul 24, 2020 at 2:25 PM Yifei Jiang wrote: > > Hi, > > These two patches enable support for vhost-net on RISC-V architecture. They > are developed > based on the Linux source in this repo: https://github.com/avpatel/linux, > the branch is riscv_kvm_v13. > > The accompanying QEMU is from the repo: https://github.com/alistair23/qemu, > the branch is > hyp-ext-v0.6.next. In order for the QEMU to work with KVM, the patch found > here is necessary: > https://patchwork.kernel.org/cover/11435965/ > > Several steps to use this: > > 1. create virbr0 on riscv64 emulation > $ brctl addbr virbr0 > $ brctl stp virbr0 on > $ ifconfig virbr0 up > $ ifconfig virbr0 netmask > > 2. boot riscv64 guestOS on riscv64 emulation > $ ./qemu-system-riscv64 -M virt,accel=kvm -m 1024M -cpu host -nographic \ > -name guest=riscv-guest \ > -smp 2 \ > -kernel ./Image \ > -drive file=./guest.img,format=raw,id=hd0 \ > -device virtio-blk,drive=hd0 \ > -netdev > type=tap,vhost=on,script=./ifup.sh,downscript=./ifdown.sh,id=net0 \ > -append "root=/dev/vda rw console=ttyS0 earlycon=sbi" > > $ cat ifup.sh > #!/bin/sh > brctl addif virbr0 $1 > ifconfig $1 up > > $ cat ifdown.sh > #!/bin/sh > ifconfig $1 down > brctl delif virbr0 $1 > > This brenchmark is vhost-net compare with virtio: > > $ ./netperf -H -l 100 -t TCP_STREAM > > vhost-net: > Recv SendSend > Socket Socket Message Elapsed > Size SizeSize Time Throughput > bytes bytes bytessecs.10^6bits/sec > > 131072 16384 16384100.07457.55 > > virtio: > Recv SendSend > Socket Socket Message Elapsed > Size SizeSize Time Throughput > bytes bytes bytessecs.10^6bits/sec > > 131072 16384 16384100.07227.02 > > > The next step is to support irqfd on RISC-V architecture. > > Yifei Jiang (2): > RISC-V: KVM: enable ioeventfd capability and compile for risc-v > RISC-V: KVM: read\write kernel mmio device support > > arch/riscv/kvm/Kconfig | 2 ++ > arch/riscv/kvm/Makefile| 2 +- > arch/riscv/kvm/vcpu_exit.c | 38 -- > arch/riscv/kvm/vm.c| 1 + > 4 files changed, 36 insertions(+), 7 deletions(-) > > -- > 2.19.1 > > I will be squashing these patches into PATCH7 of v14 KVM RISC-V series. I will also add your Signed-off-by to PATCH7 of v14 KVM RISC-V to acknowledge your efforts. Thanks, Anup
possible deadlock in proc_pid_syscall (2)
Hello, syzbot found the following issue on: HEAD commit:15bc20c6 Merge tag 'tty-5.9-rc3' of git://git.kernel.org/p.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=15349f9690 kernel config: https://syzkaller.appspot.com/x/.config?x=978db74cb30aa994 dashboard link: https://syzkaller.appspot.com/bug?extid=db9cdf3dd1f64252c6ef compiler: gcc (GCC) 10.1.0-syz 20200507 Unfortunately, I don't have any reproducer for this issue yet. IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+db9cdf3dd1f64252c...@syzkaller.appspotmail.com == WARNING: possible circular locking dependency detected 5.9.0-rc2-syzkaller #0 Not tainted -- syz-executor.0/18445 is trying to acquire lock: 88809f2e0dc8 (&sig->exec_update_mutex){+.+.}-{3:3}, at: lock_trace fs/proc/base.c:408 [inline] 88809f2e0dc8 (&sig->exec_update_mutex){+.+.}-{3:3}, at: proc_pid_syscall+0xaa/0x2b0 fs/proc/base.c:646 but task is already holding lock: 88808e9a3c30 (&p->lock){+.+.}-{3:3}, at: seq_read+0x61/0x1070 fs/seq_file.c:155 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #3 (&p->lock){+.+.}-{3:3}: __mutex_lock_common kernel/locking/mutex.c:956 [inline] __mutex_lock+0x134/0x10e0 kernel/locking/mutex.c:1103 seq_read+0x61/0x1070 fs/seq_file.c:155 pde_read fs/proc/inode.c:306 [inline] proc_reg_read+0x221/0x300 fs/proc/inode.c:318 do_loop_readv_writev fs/read_write.c:734 [inline] do_loop_readv_writev fs/read_write.c:721 [inline] do_iter_read+0x48e/0x6e0 fs/read_write.c:955 vfs_readv+0xe5/0x150 fs/read_write.c:1073 kernel_readv fs/splice.c:355 [inline] default_file_splice_read.constprop.0+0x4e6/0x9e0 fs/splice.c:412 do_splice_to+0x137/0x170 fs/splice.c:871 splice_direct_to_actor+0x307/0x980 fs/splice.c:950 do_splice_direct+0x1b3/0x280 fs/splice.c:1059 do_sendfile+0x55f/0xd40 fs/read_write.c:1540 __do_sys_sendfile64 fs/read_write.c:1601 [inline] __se_sys_sendfile64 fs/read_write.c:1587 [inline] __x64_sys_sendfile64+0x1cc/0x210 fs/read_write.c:1587 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 entry_SYSCALL_64_after_hwframe+0x44/0xa9 -> #2 (sb_writers#4){.+.+}-{0:0}: percpu_down_read include/linux/percpu-rwsem.h:51 [inline] __sb_start_write+0x234/0x470 fs/super.c:1672 sb_start_write include/linux/fs.h:1643 [inline] mnt_want_write+0x3a/0xb0 fs/namespace.c:354 ovl_setattr+0x5c/0x850 fs/overlayfs/inode.c:28 notify_change+0xb60/0x10a0 fs/attr.c:336 chown_common+0x4a9/0x550 fs/open.c:674 do_fchownat+0x126/0x1e0 fs/open.c:704 __do_sys_lchown fs/open.c:729 [inline] __se_sys_lchown fs/open.c:727 [inline] __x64_sys_lchown+0x7a/0xc0 fs/open.c:727 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 entry_SYSCALL_64_after_hwframe+0x44/0xa9 -> #1 (&ovl_i_mutex_dir_key[depth]){}-{3:3}: down_read+0x96/0x420 kernel/locking/rwsem.c:1492 inode_lock_shared include/linux/fs.h:789 [inline] lookup_slow fs/namei.c:1560 [inline] walk_component+0x409/0x6a0 fs/namei.c:1860 lookup_last fs/namei.c:2309 [inline] path_lookupat+0x1ba/0x830 fs/namei.c:2333 filename_lookup+0x19f/0x560 fs/namei.c:2366 create_local_trace_uprobe+0x87/0x4e0 kernel/trace/trace_uprobe.c:1574 perf_uprobe_init+0x132/0x210 kernel/trace/trace_event_perf.c:323 perf_uprobe_event_init+0xff/0x1c0 kernel/events/core.c:9580 perf_try_init_event+0x12a/0x560 kernel/events/core.c:10899 perf_init_event kernel/events/core.c:10951 [inline] perf_event_alloc.part.0+0xdee/0x3770 kernel/events/core.c:11229 perf_event_alloc kernel/events/core.c:11608 [inline] __do_sys_perf_event_open+0x72c/0x2cb0 kernel/events/core.c:11724 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 entry_SYSCALL_64_after_hwframe+0x44/0xa9 -> #0 (&sig->exec_update_mutex){+.+.}-{3:3}: check_prev_add kernel/locking/lockdep.c:2496 [inline] check_prevs_add kernel/locking/lockdep.c:2601 [inline] validate_chain kernel/locking/lockdep.c:3218 [inline] __lock_acquire+0x2a6b/0x5640 kernel/locking/lockdep.c:4426 lock_acquire+0x1f1/0xad0 kernel/locking/lockdep.c:5005 __mutex_lock_common kernel/locking/mutex.c:956 [inline] __mutex_lock+0x134/0x10e0 kernel/locking/mutex.c:1103 lock_trace fs/proc/base.c:408 [inline] proc_pid_syscall+0xaa/0x2b0 fs/proc/base.c:646 proc_single_show+0x116/0x1e0 fs/proc/base.c:775 seq_read+0x432/0x1070 fs/seq_file.c:208 do_loop_readv_writev fs/read_write.c:734 [inline] do_loop_readv_writev fs/read_write.c:721 [inline] do_iter_read+0x48e
Re: [PATCH RFC 2/2] target/kvm: Add interfaces needed for log dirty
On Thu, Aug 27, 2020 at 1:54 PM Yifei Jiang wrote: > > Add two interfaces of log dirty for kvm_main.c, and detele the interface > kvm_vm_ioctl_get_dirty_log which is redundantly defined. > > CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT is added in defconfig. > > Signed-off-by: Yifei Jiang > Signed-off-by: Yipeng Yin > --- > arch/riscv/configs/defconfig | 1 + > arch/riscv/kvm/Kconfig | 1 + > arch/riscv/kvm/mmu.c | 43 > arch/riscv/kvm/vm.c | 6 - > 4 files changed, 45 insertions(+), 6 deletions(-) > > diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig > index d36e1000bbd3..857d799672c2 100644 > --- a/arch/riscv/configs/defconfig > +++ b/arch/riscv/configs/defconfig > @@ -19,6 +19,7 @@ CONFIG_SOC_VIRT=y > CONFIG_SMP=y > CONFIG_VIRTUALIZATION=y > CONFIG_KVM=y > +CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT=y > CONFIG_HOTPLUG_CPU=y > CONFIG_MODULES=y > CONFIG_MODULE_UNLOAD=y > diff --git a/arch/riscv/kvm/Kconfig b/arch/riscv/kvm/Kconfig > index 2356dc52ebb3..91fcffc70e5d 100644 > --- a/arch/riscv/kvm/Kconfig > +++ b/arch/riscv/kvm/Kconfig > @@ -26,6 +26,7 @@ config KVM > select KVM_MMIO > select HAVE_KVM_VCPU_ASYNC_IOCTL > select SRCU > + select KVM_GENERIC_DIRTYLOG_READ_PROTECT > help > Support hosting virtualized guest machines. > > diff --git a/arch/riscv/kvm/mmu.c b/arch/riscv/kvm/mmu.c > index 88bce80ee983..df2a470c25e4 100644 > --- a/arch/riscv/kvm/mmu.c > +++ b/arch/riscv/kvm/mmu.c > @@ -358,6 +358,43 @@ void stage2_wp_memory_region(struct kvm *kvm, int slot) > kvm_flush_remote_tlbs(kvm); > } > > +/** > + * kvm_mmu_write_protect_pt_masked() - write protect dirty pages > + * @kvm:The KVM pointer > + * @slot: The memory slot associated with mask > + * @gfn_offset: The gfn offset in memory slot > + * @mask: The mask of dirty pages at offset 'gfn_offset' in this memory > + * slot to be write protected > + * > + * Walks bits set in mask write protects the associated pte's. Caller must > + * acquire kvm_mmu_lock. > + */ > +static void kvm_mmu_write_protect_pt_masked(struct kvm *kvm, > +struct kvm_memory_slot *slot, > +gfn_t gfn_offset, unsigned long mask) > +{ > +phys_addr_t base_gfn = slot->base_gfn + gfn_offset; > +phys_addr_t start = (base_gfn + __ffs(mask)) << PAGE_SHIFT; > +phys_addr_t end = (base_gfn + __fls(mask) + 1) << PAGE_SHIFT; > + > +stage2_wp_range(kvm, start, end); > +} > + > +/* > + * kvm_arch_mmu_enable_log_dirty_pt_masked - enable dirty logging for > selected > + * dirty pages. > + * > + * It calls kvm_mmu_write_protect_pt_masked to write protect selected pages > to > + * enable dirty logging for them. > + */ > +void kvm_arch_mmu_enable_log_dirty_pt_masked(struct kvm *kvm, > +struct kvm_memory_slot *slot, > +gfn_t gfn_offset, unsigned long mask) > +{ > +kvm_mmu_write_protect_pt_masked(kvm, slot, gfn_offset, mask); > +} > + > + > int stage2_ioremap(struct kvm *kvm, gpa_t gpa, phys_addr_t hpa, >unsigned long size, bool writable) > { > @@ -433,6 +470,12 @@ void kvm_arch_sync_dirty_log(struct kvm *kvm, struct > kvm_memory_slot *memslot) > { > } > > +void kvm_arch_flush_remote_tlbs_memslot(struct kvm *kvm, > + struct kvm_memory_slot *memslot) > +{ > + kvm_flush_remote_tlbs(kvm); > +} > + > void kvm_arch_free_memslot(struct kvm *kvm, struct kvm_memory_slot *free) > { > } > diff --git a/arch/riscv/kvm/vm.c b/arch/riscv/kvm/vm.c > index 4f2498198cb5..f7405676903b 100644 > --- a/arch/riscv/kvm/vm.c > +++ b/arch/riscv/kvm/vm.c > @@ -12,12 +12,6 @@ > #include > #include > > -int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm, struct kvm_dirty_log *log) > -{ > - /* TODO: To be added later. */ > - return -ENOTSUPP; > -} > - > int kvm_arch_init_vm(struct kvm *kvm, unsigned long type) > { > int r; > -- > 2.19.1 > > I already have a similar change as part of v14 KVM RISC-V series. Let us coordinate better. Please let us know in-advance for any KVM RISC-V feature you plan to work on. Otherwise, this leads to efforts wasted at your end or at our end. Regards, Anup
Re: WARNING: at drivers/opp/core.c:678 dev_pm_opp_set_rate+0x4cc/0x5d4 - on arm x15
On 27-08-20, 21:18, Stephen Rothwell wrote: > Hi Viresh, > > On Thu, 27 Aug 2020 15:16:51 +0530 Viresh Kumar > wrote: > > > > On 27-08-20, 15:04, Naresh Kamboju wrote: > > > While boot testing arm x15 devices the Kernel warning noticed with linux > > > next > > > tag 20200825. > > > > > > BAD: next-20200825 > > > GOOD: next-20200824 > > > > > > metadata: > > > git branch: master > > > git repo: > > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git > > > git commit: 3a00d3dfd4b68b208ecd5405e676d06c8ad6bb63 > > > git describe: next-20200825 > > > make_kernelversion: 5.9.0-rc2 > > > kernel-config: > > > https://builds.tuxbuild.com/LDTu4GFMmvkJspza5LJIjQ/kernel.config > > > > > > We are working on git bisect and boot testing on x15 and get back to you. > > > > > > > Was this working earlier ? But considering that multiple things > > related to OPP broke recently, it may be a OPP core bug as well. Not > > sure though. > > > > Can you give me delta between both the next branches for drivers/opp/ > > path ? I didn't get these tags after fetching linux-next. > > Yeah, you need to explicitly fetch the tags as only the latest tag is > part of the branches in the tree. Ah, I see. Thanks. -- viresh
Loan
Hello, You can review your loan agreements immediately and you never need to visit a branch. We offer a loan with an interest rate of 2% per annul. Quick payout, apply now within 48 hours, the offer covers all types of loans and the offer is open to blacklisted people. This can elevate your business to a higher level. You can apply below by e-mail Send us an email at info.global24ho...@aol.com Whatsapp? +1 (508) 571-8073
Re: [PATCH RFC 1/2] riscv/kvm: Fix use VSIP_VALID_MASK mask HIP register
On Thu, Aug 27, 2020 at 1:53 PM Yifei Jiang wrote: > > The correct sip/sie 0x222 could mask wrong 0x000 by VSIP_VALID_MASK, > This patch fix it. > > Signed-off-by: Yifei Jiang > Signed-off-by: Yipeng Yin > --- > arch/riscv/kvm/vcpu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c > index adb0815951aa..297e921f 100644 > --- a/arch/riscv/kvm/vcpu.c > +++ b/arch/riscv/kvm/vcpu.c > @@ -419,8 +419,8 @@ static int kvm_riscv_vcpu_set_reg_csr(struct kvm_vcpu > *vcpu, > > if (reg_num == KVM_REG_RISCV_CSR_REG(sip) || > reg_num == KVM_REG_RISCV_CSR_REG(sie)) { > - reg_val = reg_val << VSIP_TO_HVIP_SHIFT; > reg_val = reg_val & VSIP_VALID_MASK; > + reg_val = reg_val << VSIP_TO_HVIP_SHIFT; Thanks for this fix. I have squashed it into PATCH5 of KVM RISC-V v14 series. Regards, Anup
Re: [PATCH v2 3/3] riscv: Add cache information in AUX vector
Hi Zong, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linus/master] [also build test WARNING on v5.9-rc2 next-20200827] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Zong-Li/Get-cache-information-from-userland/20200827-162439 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 15bc20c6af4ceee97a1f90b43c0e386643c071b4 config: riscv-randconfig-s032-20200827 (attached as .config) compiler: riscv64-linux-gcc (GCC) 9.3.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # apt-get install sparse # sparse version: v0.6.2-191-g10164920-dirty # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=riscv If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot sparse warnings: (new ones prefixed by >>) >> arch/riscv/kernel/cacheinfo.c:39:16: sparse: sparse: Using plain integer as >> NULL pointer # https://github.com/0day-ci/linux/commit/a51c248ba0626069792c3f84c8879f685f4a1ff6 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Zong-Li/Get-cache-information-from-userland/20200827-162439 git checkout a51c248ba0626069792c3f84c8879f685f4a1ff6 vim +39 arch/riscv/kernel/cacheinfo.c 26 27 static struct cacheinfo *get_cacheinfo(u32 level, enum cache_type type) 28 { 29 struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(smp_processor_id()); 30 struct cacheinfo *this_leaf; 31 int index; 32 33 for (index = 0; index < this_cpu_ci->num_leaves; index++) { 34 this_leaf = this_cpu_ci->info_list + index; 35 if (this_leaf->level == level && this_leaf->type == type) 36 return this_leaf; 37 } 38 > 39 return 0; 40 } 41 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [RFC][PATCH 3/7] kprobes: Remove kretprobe hash
On Thu, 27 Aug 2020 18:12:40 +0200 Peter Zijlstra wrote: > @@ -1313,25 +1261,28 @@ void kprobe_busy_end(void) > void kprobe_flush_task(struct task_struct *tk) > { > struct kretprobe_instance *ri; > - struct hlist_head *head, empty_rp; > + struct hlist_head empty_rp; > + struct llist_node *node; > struct hlist_node *tmp; We don't need this tmp anymore. > @@ -1935,71 +1932,45 @@ unsigned long __kretprobe_trampoline_han > unsigned long trampoline_address, > void *frame_pointer) > { > + kprobe_opcode_t *correct_ret_addr = NULL; > struct kretprobe_instance *ri = NULL; > - struct hlist_head *head, empty_rp; > + unsigned long orig_ret_address = 0; > + struct llist_node *first, *node; > + struct hlist_head empty_rp; > struct hlist_node *tmp; Here too. I'm trying to port this patch on my v4 series. I'll add my RFC patch of kretprobe_holder too. Thank you, -- Masami Hiramatsu
[PATCH v4] power: supply: sbs-battery: don't assume i2c errors as battery disconnect
Current sbs-battery considers all smbus errors as disconnection events when battery-detect pin isn't supplied, and restored to present state back when any successful transaction is made. This can lead to unwanted state changes between present and !present when there's one i2c error and other following commands were successful. This patch provides a unified way of checking presence by calling sbs_get_battery_presence_and_health() when detect pin is not used. Signed-off-by: Ikjoon Jang --- v4: rebase from merge conflict, amend commit messages v3: check return value of get_presence_and_health() v2: combine get_presence_and_health functions to reuse --- drivers/power/supply/sbs-battery.c | 25 + 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/drivers/power/supply/sbs-battery.c b/drivers/power/supply/sbs-battery.c index 6273211cd673..dacc4bc1c013 100644 --- a/drivers/power/supply/sbs-battery.c +++ b/drivers/power/supply/sbs-battery.c @@ -959,10 +959,17 @@ static int sbs_get_property(struct power_supply *psy, return -EINVAL; } - if (!chip->gpio_detect && - chip->is_present != (ret >= 0)) { - sbs_update_presence(chip, (ret >= 0)); - power_supply_changed(chip->power_supply); + if (!chip->gpio_detect && chip->is_present != (ret >= 0)) { + bool old_present = chip->is_present; + union power_supply_propval val; + + ret = sbs_get_battery_presence_and_health( + client, POWER_SUPPLY_PROP_PRESENT, &val); + + sbs_update_presence(chip, !ret && val.intval); + + if (old_present != chip->is_present) + power_supply_changed(chip->power_supply); } done: @@ -1147,11 +1154,13 @@ static int sbs_probe(struct i2c_client *client) * to the battery. */ if (!(force_load || chip->gpio_detect)) { - rc = sbs_read_word_data(client, sbs_data[REG_STATUS].addr); + union power_supply_propval val; - if (rc < 0) { - dev_err(&client->dev, "%s: Failed to get device status\n", - __func__); + rc = sbs_get_battery_presence_and_health( + client, POWER_SUPPLY_PROP_PRESENT, &val); + if (rc < 0 || !val.intval) { + dev_err(&client->dev, "Failed to get present status\n"); + rc = -ENODEV; goto exit_psupply; } } -- 2.28.0.402.g5ffc5be6b7-goog
Re: [PATCH 17/19] z2ram: reindent
On Fri, Aug 28, 2020 at 10:57:46AM +1000, Finn Thain wrote: > On Thu, 27 Aug 2020, Joe Perches wrote: > > > > > checkpatch already does this. > > > > Did you use checkpatch to generate this patch? I used scripts/Lindent.
Re: [PATCH 09/10] sh: don't allow non-coherent DMA for NOMMU
On Thu, Aug 27, 2020 at 10:11:53PM -0400, Rich Felker wrote: > > This change broke SD card support on J2 because MMC_SPI spuriously > > depends on HAS_DMA. It looks like it can be fixed just by removing > > that dependency from drivers/mmc/host/Kconfig. > > It can't. mmp_spi_probe fails with ENOMEM, probably due to trying to > do some DMA setup thing that's not going to be needed if the > underlying SPI device doesn't support/use DMA. Adding the linux-mmc and linux-spi lists, as that seems pretty odd.
RE: [PATCH 1/2] dt-bindings: pwm: renesas,pwm-rcar: Add r8a774e1 support
Hi Lad-san, > From: Lad Prabhakar, Sent: Tuesday, August 25, 2020 7:45 PM > > From: Marian-Cristian Rotariu > > Document RZ/G2H (R8A774E1) SoC bindings. > > No driver change is needed due to the fallback compatible value > "renesas,pwm-rcar". > > Signed-off-by: Marian-Cristian Rotariu > > Signed-off-by: Lad Prabhakar > --- Thank you for the patch! Reviewed-by: Yoshihiro Shimoda Best regards, Yoshihiro Shimoda > Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.yaml > b/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.yaml > index daadde9ff9c4..5407c11e92a4 100644 > --- a/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.yaml > +++ b/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.yaml > @@ -20,6 +20,7 @@ properties: >- renesas,pwm-r8a774a1 # RZ/G2M >- renesas,pwm-r8a774b1 # RZ/G2N >- renesas,pwm-r8a774c0 # RZ/G2E > + - renesas,pwm-r8a774e1 # RZ/G2H >- renesas,pwm-r8a7778 # R-Car M1A >- renesas,pwm-r8a7779 # R-Car H1 >- renesas,pwm-r8a7790 # R-Car H2 > -- > 2.17.1
Re: [Cocci] [PATCH] usb: atm: don't use snprintf() for sysfs attrs
On Thu, 2020-08-27 at 15:45 -0700, Joe Perches wrote: > On Thu, 2020-08-27 at 15:20 -0700, Kees Cook wrote: > > On Fri, Aug 28, 2020 at 12:01:34AM +0300, Denis Efremov wrote: > > > Just FYI, I've send an addition to the device_attr_show.cocci script[1] > > > to turn > > > simple cases of snprintf (e.g. "%i") to sprintf. Looks like many > > > developers would > > > like it more than changing snprintf to scnprintf. As for me, I don't like > > > the idea > > > of automated altering of the original logic from bounded snprint to > > > unbouded one > > > with sprintf. > > > > Agreed. This just makes me cringe. If the API design declares that when > > a show() callback starts, buf has been allocated with PAGE_SIZE bytes, > > then that's how the logic should proceed, and it should be using > > scnprintf... > > > > show(...) { > > size_t remaining = PAGE_SIZE; > > > > ... > > remaining -= scnprintf(buf, remaining, "fmt", var args ...); > > remaining -= scnprintf(buf, remaining, "fmt", var args ...); > > remaining -= scnprintf(buf, remaining, "fmt", var args ...); > > > > return PAGE_SIZE - remaining; > > } > > It seems likely that coccinelle could do those transform > with any of sprintf/snprintf/scnprint too. > > Though my bikeshed would use a single function and have > that function know the maximum output size Perhaps something like the below with a sample conversion that uses single and multiple sysfs_emit uses. I believe coccinelle can _mostly_ automated this. --- fs/sysfs/file.c | 30 ++ include/linux/sysfs.h | 8 kernel/power/main.c | 49 ++--- 3 files changed, 64 insertions(+), 23 deletions(-) diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index eb6897ab78e7..c0ff3ba8e373 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -707,3 +707,33 @@ int sysfs_change_owner(struct kobject *kobj, kuid_t kuid, kgid_t kgid) return 0; } EXPORT_SYMBOL_GPL(sysfs_change_owner); + +/** + * sysfs_emit - scnprintf equivalent, aware of PAGE_SIZE buffer. + * @buf: start of PAGE_SIZE buffer. + * @pos: current position in buffer + * (pos - buf) must always be < PAGE_SIZE + * @fmt: format + * @...: arguments to format + * + * + * Returns number of characters written at pos. + */ +int sysfs_emit(char *buf, char *pos, const char *fmt, ...) +{ + int len; + va_list args; + + WARN(pos < buf, "pos < buf\n"); + WARN(pos - buf >= PAGE_SIZE, "pos >= PAGE_SIZE (%tu > %lu)\n", +pos - buf, PAGE_SIZE); + if (pos < buf || pos - buf >= PAGE_SIZE) + return 0; + + va_start(args, fmt); + len = vscnprintf(pos, PAGE_SIZE - (pos - buf), fmt, args); + va_end(args); + + return len; +} +EXPORT_SYMBOL_GPL(sysfs_emit); diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h index 34e84122f635..5a21d3d30016 100644 --- a/include/linux/sysfs.h +++ b/include/linux/sysfs.h @@ -329,6 +329,8 @@ int sysfs_groups_change_owner(struct kobject *kobj, int sysfs_group_change_owner(struct kobject *kobj, const struct attribute_group *groups, kuid_t kuid, kgid_t kgid); +__printf(3, 4) +int sysfs_emit(char *buf, char *pos, const char *fmt, ...); #else /* CONFIG_SYSFS */ @@ -576,6 +578,12 @@ static inline int sysfs_group_change_owner(struct kobject *kobj, return 0; } +__printf(3, 4) +static inline int sysfs_emit(char *buf, char *pos, const char *fmt, ...) +{ + return 0; +} + #endif /* CONFIG_SYSFS */ static inline int __must_check sysfs_create_file(struct kobject *kobj, diff --git a/kernel/power/main.c b/kernel/power/main.c index 40f86ec4ab30..f3fb9f255234 100644 --- a/kernel/power/main.c +++ b/kernel/power/main.c @@ -100,7 +100,7 @@ int pm_async_enabled = 1; static ssize_t pm_async_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf) { - return sprintf(buf, "%d\n", pm_async_enabled); + return sysfs_emit(buf, buf, "%d\n", pm_async_enabled); } static ssize_t pm_async_store(struct kobject *kobj, struct kobj_attribute *attr, @@ -124,7 +124,7 @@ power_attr(pm_async); static ssize_t mem_sleep_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf) { - char *s = buf; + char *pos = buf; suspend_state_t i; for (i = PM_SUSPEND_MIN; i < PM_SUSPEND_MAX; i++) @@ -132,16 +132,16 @@ static ssize_t mem_sleep_show(struct kobject *kobj, struct kobj_attribute *attr, const char *label = mem_sleep_states[i]; if (mem_sleep_current == i) - s += sprintf(s, "[%s] ", label); + pos += sysfs_emit(buf, pos, "[%s] ", label); else - s += sprintf(s, "%s ", label); +
Re: [RFC][PATCH 6/7] freelist: Lock less freelist
On Fri, Aug 28, 2020 at 12:23 AM Peter Zijlstra wrote: > +static inline void __freelist_add(struct freelist_node *node, struct > freelist_head *list) > +{ > + /* > +* Since the refcount is zero, and nobody can increase it once it's > +* zero (except us, and we run only one copy of this method per node > at > +* a time, i.e. the single thread case), then we know we can safely > + > + /* > +* OK, the head must have changed on us, but we still need to > decrement > +* the refcount we increased. > +*/ > + refs = atomic_fetch_add(-1, &prev->refs); > + if (refs == REFS_ON_FREELIST + 1) > + __freelist_add(prev, list); I'm curious whether it is correct to just set the prev->refs to zero and return @prev? So that it can remove an unneeded "add()&get()" pair (although in an unlikely branch) and __freelist_add() can be folded into freelist_add() for tidier code. Thanks Lai.
[PATCH] f2fs: make fibmap consistent with fiemap for compression chunk
From: Daeho Jeong Currently fibmap returns zero address for compression chunk. But it is not consistent with the output of fiemap, since fiemap returns real pysical block address related to the compression chunk. Therefore I suggest fibmap returns the same output with fiemap. Signed-off-by: Daeho Jeong --- fs/f2fs/data.c | 33 - 1 file changed, 33 deletions(-) diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index c1b676be67b9..8c26c5d0c778 100644 --- a/fs/f2fs/data.c +++ b/fs/f2fs/data.c @@ -3708,36 +3708,6 @@ static int f2fs_set_data_page_dirty(struct page *page) return 0; } - -static sector_t f2fs_bmap_compress(struct inode *inode, sector_t block) -{ -#ifdef CONFIG_F2FS_FS_COMPRESSION - struct dnode_of_data dn; - sector_t start_idx, blknr = 0; - int ret; - - start_idx = round_down(block, F2FS_I(inode)->i_cluster_size); - - set_new_dnode(&dn, inode, NULL, NULL, 0); - ret = f2fs_get_dnode_of_data(&dn, start_idx, LOOKUP_NODE); - if (ret) - return 0; - - if (dn.data_blkaddr != COMPRESS_ADDR) { - dn.ofs_in_node += block - start_idx; - blknr = f2fs_data_blkaddr(&dn); - if (!__is_valid_data_blkaddr(blknr)) - blknr = 0; - } - - f2fs_put_dnode(&dn); - return blknr; -#else - return 0; -#endif -} - - static sector_t f2fs_bmap(struct address_space *mapping, sector_t block) { struct inode *inode = mapping->host; @@ -3753,9 +3723,6 @@ static sector_t f2fs_bmap(struct address_space *mapping, sector_t block) if (mapping_tagged(mapping, PAGECACHE_TAG_DIRTY)) filemap_write_and_wait(mapping); - if (f2fs_compressed_file(inode)) - blknr = f2fs_bmap_compress(inode, block); - if (!get_data_block_bmap(inode, block, &tmp, 0)) blknr = tmp.b_blocknr; out: -- 2.28.0.402.g5ffc5be6b7-goog
Re: [Patch v2 0/4] tracing: trivial cleanup
Steven, Would you like to pick this up? On Sun, Jul 12, 2020 at 09:10:32AM +0800, Wei Yang wrote: >Some trivial cleanup for tracing. > >v2: > * drop patch 1 > * merge patch 4 & 5 > * introduce a new patch change the return value of tracing_init_dentry() > >Wei Yang (4): > tracing: simplify the logic by defining next to be "lasst + 1" > tracing: save one trace_event->type by using __TRACE_LAST_TYPE > tracing: toplevel d_entry already initialized > tracing: make tracing_init_dentry() returns an integer instead of a >d_entry pointer > > kernel/trace/trace.c | 36 ++-- > kernel/trace/trace.h | 2 +- > kernel/trace/trace_dynevent.c| 8 +++ > kernel/trace/trace_events.c | 9 ++- > kernel/trace/trace_events_synth.c| 9 +++ > kernel/trace/trace_functions_graph.c | 8 +++ > kernel/trace/trace_hwlat.c | 8 +++ > kernel/trace/trace_kprobe.c | 10 > kernel/trace/trace_output.c | 14 +-- > kernel/trace/trace_printk.c | 8 +++ > kernel/trace/trace_stack.c | 12 +- > kernel/trace/trace_stat.c| 8 +++ > kernel/trace/trace_uprobe.c | 9 --- > 13 files changed, 66 insertions(+), 75 deletions(-) > >-- >2.20.1 (Apple Git-117) -- Wei Yang Help you, Help me
Re: [PATCH bpf-next v1 8/8] bpf/selftests: Test for bpf_per_cpu_ptr()
Thanks for taking a look! On Fri, Aug 21, 2020 at 8:30 PM Andrii Nakryiko wrote: > > On Wed, Aug 19, 2020 at 3:42 PM Hao Luo wrote: > > > > Test bpf_per_cpu_ptr(). Test two paths in the kernel. If the base > > pointer points to a struct, the returned reg is of type PTR_TO_BTF_ID. > > Direct pointer dereference can be applied on the returned variable. > > If the base pointer isn't a struct, the returned reg is of type > > PTR_TO_MEM, which also supports direct pointer dereference. > > > > Signed-off-by: Hao Luo > > --- > > Acked-by: Andrii Nakryiko > [...] > > > > __u64 out__runqueues = -1; > > __u64 out__bpf_prog_active = -1; > > +__u32 out__rq_cpu = -1; > > +unsigned long out__process_counts = -1; > > try to not use long for variables, it is 32-bit integer in user-space > but always 64-bit in BPF. This causes problems when using skeleton on > 32-bit architecture. > Ack. I will use another variable of type 'int' instead. > > > > -extern const struct rq runqueues __ksym; /* struct type global var. */ > > +extern const struct rq runqueues __ksym; /* struct type percpu var. */ > > extern const int bpf_prog_active __ksym; /* int type global var. */ > > +extern const unsigned long process_counts __ksym; /* int type percpu var. > > */ > > > > SEC("raw_tp/sys_enter") > > int handler(const void *ctx) > > { > > + struct rq *rq; > > + unsigned long *count; > > + > > out__runqueues = (__u64)&runqueues; > > out__bpf_prog_active = (__u64)&bpf_prog_active; > > > > + rq = (struct rq *)bpf_per_cpu_ptr(&runqueues, 1); > > + if (rq) > > + out__rq_cpu = rq->cpu; > > this is awesome! > > Are there any per-cpu variables that are arrays? Would be nice to test > those too. > > There are currently per-cpu arrays, but not common. There is a 'pmc_prev_left' in arch/x86, I can add that in this test. [...]
[PATCH net-next v2 3/3] hinic: add support to query function table
add debugfs node for querying function table, for example: cat /sys/kernel/debug/hinic/:15:00.0/func_table/valid Signed-off-by: Luo bin --- V0~V1: - remove command interfaces to the read only files - split addition of each object into a separate patch V1~V2: - remove vlan_id and vlan_mode from the func_table_fields .../net/ethernet/huawei/hinic/hinic_debugfs.c | 92 ++- .../net/ethernet/huawei/hinic/hinic_debugfs.h | 79 drivers/net/ethernet/huawei/hinic/hinic_dev.h | 3 + .../net/ethernet/huawei/hinic/hinic_hw_dev.h | 2 + .../net/ethernet/huawei/hinic/hinic_main.c| 15 +++ 5 files changed, 190 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/huawei/hinic/hinic_debugfs.c b/drivers/net/ethernet/huawei/hinic/hinic_debugfs.c index d10d0a6d9f13..19eb839177ec 100644 --- a/drivers/net/ethernet/huawei/hinic/hinic_debugfs.c +++ b/drivers/net/ethernet/huawei/hinic/hinic_debugfs.c @@ -70,6 +70,63 @@ static u64 hinic_dbg_get_rq_info(struct hinic_dev *nic_dev, struct hinic_rq *rq, return 0; } +enum func_tbl_info { + VALID, + RX_MODE, + MTU, + RQ_DEPTH, + QUEUE_NUM, +}; + +static char *func_table_fields[] = {"valid", "rx_mode", "mtu", "rq_depth", "cfg_q_num"}; + +static int hinic_dbg_get_func_table(struct hinic_dev *nic_dev, int idx) +{ + struct tag_sml_funcfg_tbl *funcfg_table_elem; + struct hinic_cmd_lt_rd *read_data; + u16 out_size = sizeof(*read_data); + int err; + + read_data = kzalloc(sizeof(*read_data), GFP_KERNEL); + if (!read_data) + return ~0; + + read_data->node = TBL_ID_FUNC_CFG_SM_NODE; + read_data->inst = TBL_ID_FUNC_CFG_SM_INST; + read_data->entry_size = HINIC_FUNCTION_CONFIGURE_TABLE_SIZE; + read_data->lt_index = HINIC_HWIF_FUNC_IDX(nic_dev->hwdev->hwif); + read_data->len = HINIC_FUNCTION_CONFIGURE_TABLE_SIZE; + + err = hinic_port_msg_cmd(nic_dev->hwdev, HINIC_PORT_CMD_RD_LINE_TBL, read_data, +sizeof(*read_data), read_data, &out_size); + if (err || out_size != sizeof(*read_data) || read_data->status) { + netif_err(nic_dev, drv, nic_dev->netdev, + "Failed to get func table, err: %d, status: 0x%x, out size: 0x%x\n", + err, read_data->status, out_size); + kfree(read_data); + return ~0; + } + + funcfg_table_elem = (struct tag_sml_funcfg_tbl *)read_data->data; + + switch (idx) { + case VALID: + return funcfg_table_elem->dw0.bs.valid; + case RX_MODE: + return funcfg_table_elem->dw0.bs.nic_rx_mode; + case MTU: + return funcfg_table_elem->dw1.bs.mtu; + case RQ_DEPTH: + return funcfg_table_elem->dw13.bs.cfg_rq_depth; + case QUEUE_NUM: + return funcfg_table_elem->dw13.bs.cfg_q_num; + } + + kfree(read_data); + + return ~0; +} + static ssize_t hinic_dbg_cmd_read(struct file *filp, char __user *buffer, size_t count, loff_t *ppos) { @@ -91,6 +148,10 @@ static ssize_t hinic_dbg_cmd_read(struct file *filp, char __user *buffer, size_t out = hinic_dbg_get_rq_info(dbg->dev, dbg->object, *desc); break; + case HINIC_DBG_FUNC_TABLE: + out = hinic_dbg_get_func_table(dbg->dev, *desc); + break; + default: netif_warn(dbg->dev, drv, dbg->dev->netdev, "Invalid hinic debug cmd: %d\n", dbg->type); @@ -136,7 +197,9 @@ static int create_dbg_files(struct hinic_dev *dev, enum hinic_dbg_type type, voi static void rem_dbg_files(struct hinic_debug_priv *dbg) { - debugfs_remove_recursive(dbg->root); + if (dbg->type != HINIC_DBG_FUNC_TABLE) + debugfs_remove_recursive(dbg->root); + kfree(dbg); } @@ -184,6 +247,21 @@ void hinic_rq_debug_rem(struct hinic_rq *rq) rem_dbg_files(rq->dbg); } +int hinic_func_table_debug_add(struct hinic_dev *dev) +{ + if (HINIC_IS_VF(dev->hwdev->hwif)) + return 0; + + return create_dbg_files(dev, HINIC_DBG_FUNC_TABLE, dev, dev->func_tbl_dbgfs, &dev->dbg, + func_table_fields, ARRAY_SIZE(func_table_fields)); +} + +void hinic_func_table_debug_rem(struct hinic_dev *dev) +{ + if (!HINIC_IS_VF(dev->hwdev->hwif) && dev->dbg) + rem_dbg_files(dev->dbg); +} + void hinic_sq_dbgfs_init(struct hinic_dev *nic_dev) { nic_dev->sq_dbgfs = debugfs_create_dir("SQs", nic_dev->dbgfs_root); @@ -204,6 +282,18 @@ void hinic_rq_dbgfs_uninit(struct hinic_dev *nic_dev) debugfs_remove_recursive(nic_dev->rq_dbgfs); } +void hinic_func_tbl_dbgfs_init(struct hinic_dev *nic_dev) +{ + if (!HINIC_IS_VF(nic_dev->hwdev->hwif)) + nic_dev->func_tbl_dbgfs = debugfs_create
[PATCH net-next v2 2/3] hinic: add support to query rq info
add debugfs node for querying rq info, for example: cat /sys/kernel/debug/hinic/:15:00.0/RQs/0x0/rq_hw_pi Signed-off-by: Luo bin --- V0~V1: - remove command interfaces to the read only files - split addition of each object into a separate patch .../net/ethernet/huawei/hinic/hinic_debugfs.c | 66 +++ .../net/ethernet/huawei/hinic/hinic_debugfs.h | 8 +++ drivers/net/ethernet/huawei/hinic/hinic_dev.h | 2 + .../net/ethernet/huawei/hinic/hinic_hw_io.c | 1 + .../net/ethernet/huawei/hinic/hinic_hw_qp.h | 3 + .../net/ethernet/huawei/hinic/hinic_main.c| 23 ++- 6 files changed, 101 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/huawei/hinic/hinic_debugfs.c b/drivers/net/ethernet/huawei/hinic/hinic_debugfs.c index 2a1050cb400e..d10d0a6d9f13 100644 --- a/drivers/net/ethernet/huawei/hinic/hinic_debugfs.c +++ b/drivers/net/ethernet/huawei/hinic/hinic_debugfs.c @@ -40,6 +40,36 @@ static u64 hinic_dbg_get_sq_info(struct hinic_dev *nic_dev, struct hinic_sq *sq, return 0; } +enum rq_dbg_info { + GLB_RQ_ID, + RQ_HW_PI, + RQ_SW_CI, + RQ_SW_PI, + RQ_MSIX_ENTRY, +}; + +static char *rq_fields[] = {"glb_rq_id", "rq_hw_pi", "rq_sw_ci", "rq_sw_pi", "rq_msix_entry"}; + +static u64 hinic_dbg_get_rq_info(struct hinic_dev *nic_dev, struct hinic_rq *rq, int idx) +{ + struct hinic_wq *wq = rq->wq; + + switch (idx) { + case GLB_RQ_ID: + return nic_dev->hwdev->func_to_io.global_qpn + rq->qid; + case RQ_HW_PI: + return be16_to_cpu(*(__be16 *)(rq->pi_virt_addr)) & wq->mask; + case RQ_SW_CI: + return atomic_read(&wq->cons_idx) & wq->mask; + case RQ_SW_PI: + return atomic_read(&wq->prod_idx) & wq->mask; + case RQ_MSIX_ENTRY: + return rq->msix_entry; + } + + return 0; +} + static ssize_t hinic_dbg_cmd_read(struct file *filp, char __user *buffer, size_t count, loff_t *ppos) { @@ -57,6 +87,10 @@ static ssize_t hinic_dbg_cmd_read(struct file *filp, char __user *buffer, size_t out = hinic_dbg_get_sq_info(dbg->dev, dbg->object, *desc); break; + case HINIC_DBG_RQ_INFO: + out = hinic_dbg_get_rq_info(dbg->dev, dbg->object, *desc); + break; + default: netif_warn(dbg->dev, drv, dbg->dev->netdev, "Invalid hinic debug cmd: %d\n", dbg->type); @@ -128,6 +162,28 @@ void hinic_sq_debug_rem(struct hinic_sq *sq) rem_dbg_files(sq->dbg); } +int hinic_rq_debug_add(struct hinic_dev *dev, u16 rq_id) +{ + struct hinic_rq *rq; + struct dentry *root; + char sub_dir[16]; + + rq = dev->rxqs[rq_id].rq; + + sprintf(sub_dir, "0x%x", rq_id); + + root = debugfs_create_dir(sub_dir, dev->rq_dbgfs); + + return create_dbg_files(dev, HINIC_DBG_RQ_INFO, rq, root, &rq->dbg, rq_fields, + ARRAY_SIZE(rq_fields)); +} + +void hinic_rq_debug_rem(struct hinic_rq *rq) +{ + if (rq->dbg) + rem_dbg_files(rq->dbg); +} + void hinic_sq_dbgfs_init(struct hinic_dev *nic_dev) { nic_dev->sq_dbgfs = debugfs_create_dir("SQs", nic_dev->dbgfs_root); @@ -138,6 +194,16 @@ void hinic_sq_dbgfs_uninit(struct hinic_dev *nic_dev) debugfs_remove_recursive(nic_dev->sq_dbgfs); } +void hinic_rq_dbgfs_init(struct hinic_dev *nic_dev) +{ + nic_dev->rq_dbgfs = debugfs_create_dir("RQs", nic_dev->dbgfs_root); +} + +void hinic_rq_dbgfs_uninit(struct hinic_dev *nic_dev) +{ + debugfs_remove_recursive(nic_dev->rq_dbgfs); +} + void hinic_dbg_init(struct hinic_dev *nic_dev) { nic_dev->dbgfs_root = debugfs_create_dir(pci_name(nic_dev->hwdev->hwif->pdev), diff --git a/drivers/net/ethernet/huawei/hinic/hinic_debugfs.h b/drivers/net/ethernet/huawei/hinic/hinic_debugfs.h index 45fb3b40f487..186ca4a26919 100644 --- a/drivers/net/ethernet/huawei/hinic/hinic_debugfs.h +++ b/drivers/net/ethernet/huawei/hinic/hinic_debugfs.h @@ -12,10 +12,18 @@ int hinic_sq_debug_add(struct hinic_dev *dev, u16 sq_id); void hinic_sq_debug_rem(struct hinic_sq *sq); +int hinic_rq_debug_add(struct hinic_dev *dev, u16 rq_id); + +void hinic_rq_debug_rem(struct hinic_rq *rq); + void hinic_sq_dbgfs_init(struct hinic_dev *nic_dev); void hinic_sq_dbgfs_uninit(struct hinic_dev *nic_dev); +void hinic_rq_dbgfs_init(struct hinic_dev *nic_dev); + +void hinic_rq_dbgfs_uninit(struct hinic_dev *nic_dev); + void hinic_dbg_init(struct hinic_dev *nic_dev); void hinic_dbg_uninit(struct hinic_dev *nic_dev); diff --git a/drivers/net/ethernet/huawei/hinic/hinic_dev.h b/drivers/net/ethernet/huawei/hinic/hinic_dev.h index 95d9548014ac..0876a699d205 100644 --- a/drivers/net/ethernet/huawei/hinic/hinic_dev.h +++ b/drivers/net/ethernet/huawei/hinic/hinic_dev.h @@ -60,6 +60,7 @@ struct hinic_intr_coal_info { enum hinic_dbg_type {
[PATCH net-next v2 1/3] hinic: add support to query sq info
add debugfs node for querying sq info, for example: cat /sys/kernel/debug/hinic/:15:00.0/SQs/0x0/sq_pi Signed-off-by: Luo bin --- V0~V1: - remove command interfaces to the read only files - split addition of each object into a separate patch drivers/net/ethernet/huawei/hinic/Makefile| 3 +- .../net/ethernet/huawei/hinic/hinic_debugfs.c | 162 ++ .../net/ethernet/huawei/hinic/hinic_debugfs.h | 27 +++ drivers/net/ethernet/huawei/hinic/hinic_dev.h | 15 ++ .../net/ethernet/huawei/hinic/hinic_hw_dev.c | 1 + .../net/ethernet/huawei/hinic/hinic_hw_io.c | 1 + .../net/ethernet/huawei/hinic/hinic_hw_io.h | 1 + .../net/ethernet/huawei/hinic/hinic_hw_qp.h | 3 + .../net/ethernet/huawei/hinic/hinic_main.c| 45 - 9 files changed, 254 insertions(+), 4 deletions(-) create mode 100644 drivers/net/ethernet/huawei/hinic/hinic_debugfs.c create mode 100644 drivers/net/ethernet/huawei/hinic/hinic_debugfs.h diff --git a/drivers/net/ethernet/huawei/hinic/Makefile b/drivers/net/ethernet/huawei/hinic/Makefile index 67b59d0ba769..2f89119c9b69 100644 --- a/drivers/net/ethernet/huawei/hinic/Makefile +++ b/drivers/net/ethernet/huawei/hinic/Makefile @@ -4,4 +4,5 @@ obj-$(CONFIG_HINIC) += hinic.o hinic-y := hinic_main.o hinic_tx.o hinic_rx.o hinic_port.o hinic_hw_dev.o \ hinic_hw_io.o hinic_hw_qp.o hinic_hw_cmdq.o hinic_hw_wq.o \ hinic_hw_mgmt.o hinic_hw_api_cmd.o hinic_hw_eqs.o hinic_hw_if.o \ - hinic_common.o hinic_ethtool.o hinic_devlink.o hinic_hw_mbox.o hinic_sriov.o + hinic_common.o hinic_ethtool.o hinic_devlink.o hinic_hw_mbox.o \ + hinic_sriov.o hinic_debugfs.o diff --git a/drivers/net/ethernet/huawei/hinic/hinic_debugfs.c b/drivers/net/ethernet/huawei/hinic/hinic_debugfs.c new file mode 100644 index ..2a1050cb400e --- /dev/null +++ b/drivers/net/ethernet/huawei/hinic/hinic_debugfs.c @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Huawei HiNIC PCI Express Linux driver + * Copyright(c) 2017 Huawei Technologies Co., Ltd + */ + +#include +#include + +#include "hinic_debugfs.h" + +static struct dentry *hinic_dbgfs_root; + +enum sq_dbg_info { + GLB_SQ_ID, + SQ_PI, + SQ_CI, + SQ_FI, + SQ_MSIX_ENTRY, +}; + +static char *sq_fields[] = {"glb_sq_id", "sq_pi", "sq_ci", "sq_fi", "sq_msix_entry"}; + +static u64 hinic_dbg_get_sq_info(struct hinic_dev *nic_dev, struct hinic_sq *sq, int idx) +{ + struct hinic_wq *wq = sq->wq; + + switch (idx) { + case GLB_SQ_ID: + return nic_dev->hwdev->func_to_io.global_qpn + sq->qid; + case SQ_PI: + return atomic_read(&wq->prod_idx) & wq->mask; + case SQ_CI: + return atomic_read(&wq->cons_idx) & wq->mask; + case SQ_FI: + return be16_to_cpu(*(__be16 *)(sq->hw_ci_addr)) & wq->mask; + case SQ_MSIX_ENTRY: + return sq->msix_entry; + } + + return 0; +} + +static ssize_t hinic_dbg_cmd_read(struct file *filp, char __user *buffer, size_t count, + loff_t *ppos) +{ + struct hinic_debug_priv *dbg; + char ret_buf[20]; + int *desc; + u64 out; + int ret; + + desc = filp->private_data; + dbg = container_of(desc, struct hinic_debug_priv, field_id[*desc]); + + switch (dbg->type) { + case HINIC_DBG_SQ_INFO: + out = hinic_dbg_get_sq_info(dbg->dev, dbg->object, *desc); + break; + + default: + netif_warn(dbg->dev, drv, dbg->dev->netdev, "Invalid hinic debug cmd: %d\n", + dbg->type); + return -EINVAL; + } + + ret = snprintf(ret_buf, sizeof(ret_buf), "0x%llx\n", out); + + return simple_read_from_buffer(buffer, count, ppos, ret_buf, ret); +} + +static const struct file_operations hinic_dbg_cmd_fops = { + .owner = THIS_MODULE, + .open = simple_open, + .read = hinic_dbg_cmd_read, +}; + +static int create_dbg_files(struct hinic_dev *dev, enum hinic_dbg_type type, void *data, + struct dentry *root, struct hinic_debug_priv **dbg, char **field, + int nfile) +{ + struct hinic_debug_priv *tmp; + int i; + + tmp = kzalloc(sizeof(*tmp), GFP_KERNEL); + if (!tmp) + return -ENOMEM; + + tmp->dev = dev; + tmp->object = data; + tmp->type = type; + tmp->root = root; + + for (i = 0; i < nfile; i++) { + tmp->field_id[i] = i; + debugfs_create_file(field[i], 0400, root, &tmp->field_id[i], &hinic_dbg_cmd_fops); + } + + *dbg = tmp; + + return 0; +} + +static void rem_dbg_files(struct hinic_debug_priv *dbg) +{ + debugfs_remove_recursive(dbg->root); + kfree(dbg); +} + +int hinic_sq_debug_add(struct hinic_dev *dev, u16 sq_id) +{ + struct hinic_sq *sq; + struct dentr
[PATCH net-next v2 0/3] hinic: add debugfs support
add debugfs node for querying sq/rq info and function table Luo bin (3): hinic: add support to query sq info hinic: add support to query rq info hinic: add support to query function table drivers/net/ethernet/huawei/hinic/Makefile| 3 +- .../net/ethernet/huawei/hinic/hinic_debugfs.c | 318 ++ .../net/ethernet/huawei/hinic/hinic_debugfs.h | 114 +++ drivers/net/ethernet/huawei/hinic/hinic_dev.h | 20 ++ .../net/ethernet/huawei/hinic/hinic_hw_dev.c | 1 + .../net/ethernet/huawei/hinic/hinic_hw_dev.h | 2 + .../net/ethernet/huawei/hinic/hinic_hw_io.c | 2 + .../net/ethernet/huawei/hinic/hinic_hw_io.h | 1 + .../net/ethernet/huawei/hinic/hinic_hw_qp.h | 6 + .../net/ethernet/huawei/hinic/hinic_main.c| 83 - 10 files changed, 544 insertions(+), 6 deletions(-) create mode 100644 drivers/net/ethernet/huawei/hinic/hinic_debugfs.c create mode 100644 drivers/net/ethernet/huawei/hinic/hinic_debugfs.h -- 2.17.1
[Patch v2 4/7] mm/hugetlb: count file_region to be added when regions_needed != NULL
There are only two cases of function add_reservation_in_range() * count file_region and return the number in regions_needed * do the real list operation without counting This means it is not necessary to have two parameters to classify these two cases. Just use regions_needed to separate them. Signed-off-by: Wei Yang Reviewed-by: Baoquan He Reviewed-by: Mike Kravetz --- mm/hugetlb.c | 33 + 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/mm/hugetlb.c b/mm/hugetlb.c index cbe67428bf99..bbccbfeb8601 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -319,16 +319,17 @@ static void coalesce_file_region(struct resv_map *resv, struct file_region *rg) } } -/* Must be called with resv->lock held. Calling this with count_only == true - * will count the number of pages to be added but will not modify the linked - * list. If regions_needed != NULL and count_only == true, then regions_needed - * will indicate the number of file_regions needed in the cache to carry out to - * add the regions for this range. +/* + * Must be called with resv->lock held. + * + * Calling this with regions_needed != NULL will count the number of pages + * to be added but will not modify the linked list. And regions_needed will + * indicate the number of file_regions needed in the cache to carry out to add + * the regions for this range. */ static long add_reservation_in_range(struct resv_map *resv, long f, long t, struct hugetlb_cgroup *h_cg, -struct hstate *h, long *regions_needed, -bool count_only) +struct hstate *h, long *regions_needed) { long add = 0; struct list_head *head = &resv->regions; @@ -364,14 +365,14 @@ static long add_reservation_in_range(struct resv_map *resv, long f, long t, */ if (rg->from > last_accounted_offset) { add += rg->from - last_accounted_offset; - if (!count_only) { + if (!regions_needed) { nrg = get_file_region_entry_from_cache( resv, last_accounted_offset, rg->from); record_hugetlb_cgroup_uncharge_info(h_cg, h, resv, nrg); list_add(&nrg->link, rg->link.prev); coalesce_file_region(resv, nrg); - } else if (regions_needed) + } else *regions_needed += 1; } @@ -383,13 +384,13 @@ static long add_reservation_in_range(struct resv_map *resv, long f, long t, */ if (last_accounted_offset < t) { add += t - last_accounted_offset; - if (!count_only) { + if (!regions_needed) { nrg = get_file_region_entry_from_cache( resv, last_accounted_offset, t); record_hugetlb_cgroup_uncharge_info(h_cg, h, resv, nrg); list_add(&nrg->link, rg->link.prev); coalesce_file_region(resv, nrg); - } else if (regions_needed) + } else *regions_needed += 1; } @@ -482,8 +483,8 @@ static long region_add(struct resv_map *resv, long f, long t, retry: /* Count how many regions are actually needed to execute this add. */ - add_reservation_in_range(resv, f, t, NULL, NULL, &actual_regions_needed, -true); + add_reservation_in_range(resv, f, t, NULL, NULL, +&actual_regions_needed); /* * Check for sufficient descriptors in the cache to accommodate @@ -511,7 +512,7 @@ static long region_add(struct resv_map *resv, long f, long t, goto retry; } - add = add_reservation_in_range(resv, f, t, h_cg, h, NULL, false); + add = add_reservation_in_range(resv, f, t, h_cg, h, NULL); resv->adds_in_progress -= in_regions_needed; @@ -547,9 +548,9 @@ static long region_chg(struct resv_map *resv, long f, long t, spin_lock(&resv->lock); - /* Count how many hugepages in this range are NOT respresented. */ + /* Count how many hugepages in this range are NOT represented. */ chg = add_reservation_in_range(resv, f, t, NULL, NULL, - out_regions_needed, true); + out_regions_needed); if (*out_regions_needed == 0) *out_regions_needed = 1; -- 2.20.1 (Apple Git-117)
[PATCH] ubifs: setflags: Don't show error message when vfs_ioc_setflags_prepare() fails
Following process will trigger ubifs_err: 1. useradd -m freg(Under root) 2. cd /home/freg && mkdir mp (Under freg) 3. mount -t ubifs /dev/ubi0_0 /home/freg/mp (Under root) 4. cd /home/freg && echo 123 > mp/a (Under root) 5. cd mp && chown freg a && chgrp freg a && chmod 777 a (Under root) 6. chattr +i a(Under freg) UBIFS error (ubi0:0 pid 1723): ubifs_ioctl [ubifs]: can't modify inode 65 attributes chattr: Operation not permitted while setting flags on a This is not an UBIFS problem, it was caused by task priviliage checking on file operations. Remove error message printing from kernel just like other filesystems (eg. ext4), since we already have enough information from userspace tools. Signed-off-by: Zhihao Cheng --- fs/ubifs/ioctl.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/ubifs/ioctl.c b/fs/ubifs/ioctl.c index 3df9be2c684c..4363d85a3fd4 100644 --- a/fs/ubifs/ioctl.c +++ b/fs/ubifs/ioctl.c @@ -134,7 +134,6 @@ static int setflags(struct inode *inode, int flags) return err; out_unlock: - ubifs_err(c, "can't modify inode %lu attributes", inode->i_ino); mutex_unlock(&ui->ui_mutex); ubifs_release_budget(c, &req); return err; -- 2.25.4
[Patch v2 5/7] mm/hugetlb: a page from buddy is not on any list
The page allocated from buddy is not on any list, so just use list_add() is enough. Signed-off-by: Wei Yang Reviewed-by: Baoquan He Reviewed-by: Mike Kravetz --- mm/hugetlb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/hugetlb.c b/mm/hugetlb.c index bbccbfeb8601..5a71cb7acf6b 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -2428,7 +2428,7 @@ struct page *alloc_huge_page(struct vm_area_struct *vma, h->resv_huge_pages--; } spin_lock(&hugetlb_lock); - list_move(&page->lru, &h->hugepage_activelist); + list_add(&page->lru, &h->hugepage_activelist); /* Fall through */ } hugetlb_cgroup_commit_charge(idx, pages_per_huge_page(h), h_cg, page); -- 2.20.1 (Apple Git-117)
[Patch v2 0/7] mm/hugetlb: code refine and simplification
Following are some cleanup for hugetlb. Simple test with tools/testing/selftests/vm/map_hugetlb pass. v2: * drop 5/6/10 since similar patches are merged or under review. * adjust 2 based on comment from Mike Kravetz Wei Yang (7): mm/hugetlb: not necessary to coalesce regions recursively mm/hugetlb: remove VM_BUG_ON(!nrg) in get_file_region_entry_from_cache() mm/hugetlb: use list_splice to merge two list at once mm/hugetlb: count file_region to be added when regions_needed != NULL mm/hugetlb: a page from buddy is not on any list mm/hugetlb: return non-isolated page in the loop instead of break and check mm/hugetlb: narrow the hugetlb_lock protection area during preparing huge page mm/hugetlb.c | 77 +++- 1 file changed, 34 insertions(+), 43 deletions(-) -- 2.20.1 (Apple Git-117)
[Patch v2 7/7] mm/hugetlb: narrow the hugetlb_lock protection area during preparing huge page
set_hugetlb_cgroup_[rsvd] just manipulate page local data, which is not necessary to be protected by hugetlb_lock. Let's take this out. Signed-off-by: Wei Yang Reviewed-by: Baoquan He Reviewed-by: Mike Kravetz --- mm/hugetlb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 6ad365dd1e96..ae840dc09197 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -1492,9 +1492,9 @@ static void prep_new_huge_page(struct hstate *h, struct page *page, int nid) { INIT_LIST_HEAD(&page->lru); set_compound_page_dtor(page, HUGETLB_PAGE_DTOR); - spin_lock(&hugetlb_lock); set_hugetlb_cgroup(page, NULL); set_hugetlb_cgroup_rsvd(page, NULL); + spin_lock(&hugetlb_lock); h->nr_huge_pages++; h->nr_huge_pages_node[nid]++; spin_unlock(&hugetlb_lock); -- 2.20.1 (Apple Git-117)
[Patch v2 3/7] mm/hugetlb: use list_splice to merge two list at once
Instead of add allocated file_region one by one to region_cache, we could use list_splice to merge two list at once. Also we know the number of entries in the list, increase the number directly. Signed-off-by: Wei Yang Reviewed-by: Baoquan He Reviewed-by: Mike Kravetz --- mm/hugetlb.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/mm/hugetlb.c b/mm/hugetlb.c index f325839be617..cbe67428bf99 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -441,11 +441,8 @@ static int allocate_file_region_entries(struct resv_map *resv, spin_lock(&resv->lock); - list_for_each_entry_safe(rg, trg, &allocated_regions, link) { - list_del(&rg->link); - list_add(&rg->link, &resv->region_cache); - resv->region_cache_count++; - } + list_splice(&allocated_regions, &resv->region_cache); + resv->region_cache_count += to_allocate; } return 0; -- 2.20.1 (Apple Git-117)
[Patch v2 6/7] mm/hugetlb: return non-isolated page in the loop instead of break and check
Function dequeue_huge_page_node_exact() iterates the free list and return the first non-isolated one. Instead of break and check the loop variant, we could return in the loop directly. This could reduce some redundant check. Signed-off-by: Wei Yang Reviewed-by: Mike Kravetz --- mm/hugetlb.c | 26 -- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 5a71cb7acf6b..6ad365dd1e96 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -1033,20 +1033,18 @@ static struct page *dequeue_huge_page_node_exact(struct hstate *h, int nid) { struct page *page; - list_for_each_entry(page, &h->hugepage_freelists[nid], lru) - if (!PageHWPoison(page)) - break; - /* -* if 'non-isolated free hugepage' not found on the list, -* the allocation fails. -*/ - if (&h->hugepage_freelists[nid] == &page->lru) - return NULL; - list_move(&page->lru, &h->hugepage_activelist); - set_page_refcounted(page); - h->free_huge_pages--; - h->free_huge_pages_node[nid]--; - return page; + list_for_each_entry(page, &h->hugepage_freelists[nid], lru) { + if (PageHWPoison(page)) + continue; + + list_move(&page->lru, &h->hugepage_activelist); + set_page_refcounted(page); + h->free_huge_pages--; + h->free_huge_pages_node[nid]--; + return page; + } + + return NULL; } static struct page *dequeue_huge_page_nodemask(struct hstate *h, gfp_t gfp_mask, int nid, -- 2.20.1 (Apple Git-117)
[Patch v2 1/7] mm/hugetlb: not necessary to coalesce regions recursively
Per my understanding, we keep the regions ordered and would always coalesce regions properly. So the task to keep this property is just to coalesce its neighbour. Let's simplify this. Signed-off-by: Wei Yang Reviewed-by: Baoquan He Reviewed-by: Mike Kravetz --- mm/hugetlb.c | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 590111ea6975..62ec74f6d03f 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -307,8 +307,7 @@ static void coalesce_file_region(struct resv_map *resv, struct file_region *rg) list_del(&rg->link); kfree(rg); - coalesce_file_region(resv, prg); - return; + rg = prg; } nrg = list_next_entry(rg, link); @@ -318,9 +317,6 @@ static void coalesce_file_region(struct resv_map *resv, struct file_region *rg) list_del(&rg->link); kfree(rg); - - coalesce_file_region(resv, nrg); - return; } } -- 2.20.1 (Apple Git-117)
[Patch v2 2/7] mm/hugetlb: remove VM_BUG_ON(!nrg) in get_file_region_entry_from_cache()
We are sure to get a valid file_region, otherwise the VM_BUG_ON(resv->region_cache_count <= 0) at the very beginning would be triggered. Let's remove the redundant one. Signed-off-by: Wei Yang --- mm/hugetlb.c | 1 - 1 file changed, 1 deletion(-) diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 62ec74f6d03f..f325839be617 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -238,7 +238,6 @@ get_file_region_entry_from_cache(struct resv_map *resv, long from, long to) resv->region_cache_count--; nrg = list_first_entry(&resv->region_cache, struct file_region, link); - VM_BUG_ON(!nrg); list_del(&nrg->link); nrg->from = from; -- 2.20.1 (Apple Git-117)
[PATCH v2] media: uvcvideo: Convey full colorspace information to V4L2
The Color Matching Descriptor has been present in USB cameras since the original version of UVC, but it has never been fully exposed in Linux. This change informs V4L2 of all of the UVC colorspace parameters: color primaries, transfer characteristics, and YCbCr encoding. videodev2.h doesn't have values for all the possible UVC color settings, so it is mapped as closely as possible. Additionally, this patch overrides the default setting for quantization for UVC MJPEG. By default, V4L2 assumes that MJPEG is full range encoded, which is not correct for UVC. JPEG itself does not specify YCbCr encoding information, this is left to some other metadata. For typical JPEG images (those that conform to JFIF, see https://www.w3.org/Graphics/JPEG/jfif3.pdf), the colorspace is specified as YCbCr CCIR 601 with full range. The use of this variant on the 601 standard in JFIF is the reason that V4L2 defaults to full range for JPEG. A JPEG image isn't a JFIF unless it contains an APP0 tag with 'JFIF', and the UVC standard is clear that APP0 is optional for its MJPEG payload. It does not mention JFIF at all. Moreover, it provides color metadata in the Color Matching Descriptor, all using limited range as of UVC 1.5. Note that web browsers such as Chrome and Firefox already ignore V4L2's quantization for USB devices and use the correct limited range, but other programs such as qv4l2 will incorrectly interpret the encoding of MJPEG from USB cameras without this change. Since there are many YUV and non-YUV formats supported by UVC cameras (but not mentioned in the official specifications), and the quantization is also not specified for these formats, I am not changing that behavior: all formats besides MJPEG will stay at V4L2_QUANTIZATION_DEFAULT as before. Signed-off-by: Adam Goode --- Changes in v2: - Apply quantization override only for MJPEG. - Provide more comments and background information about JPEG vs JFIF. - Explain the substitutions for xfer func and ycbcr encoding. drivers/media/usb/uvc/uvc_driver.c | 87 -- drivers/media/usb/uvc/uvc_v4l2.c | 6 +++ drivers/media/usb/uvc/uvcvideo.h | 6 ++- 3 files changed, 94 insertions(+), 5 deletions(-) diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 431d86e1c94b..4e530a4bf976 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -248,10 +248,10 @@ static struct uvc_format_desc *uvc_format_by_guid(const u8 guid[16]) return NULL; } -static u32 uvc_colorspace(const u8 primaries) +static enum v4l2_colorspace uvc_colorspace(const u8 primaries) { - static const u8 colorprimaries[] = { - 0, + static const enum v4l2_colorspace colorprimaries[] = { + V4L2_COLORSPACE_DEFAULT, /* Unspecified */ V4L2_COLORSPACE_SRGB, V4L2_COLORSPACE_470_SYSTEM_M, V4L2_COLORSPACE_470_SYSTEM_BG, @@ -262,7 +262,61 @@ static u32 uvc_colorspace(const u8 primaries) if (primaries < ARRAY_SIZE(colorprimaries)) return colorprimaries[primaries]; - return 0; + return V4L2_COLORSPACE_DEFAULT; /* Reserved */ +} + +static enum v4l2_xfer_func uvc_xfer_func(const u8 transfer_characteristics) +{ + /* V4L2 currently does not currently have definitions for all +* possible values of UVC transfer characteristics. If +* v4l2_xfer_func is extended with new values, the mapping +* below should be updated. +* +* Substitutions are taken from the mapping given for +* V4L2_XFER_FUNC_DEFAULT documented in videodev2.h. +*/ + static const enum v4l2_xfer_func xfer_funcs[] = { + V4L2_XFER_FUNC_DEFAULT,/* Unspecified */ + V4L2_XFER_FUNC_709, + V4L2_XFER_FUNC_709,/* Substitution for BT.470-2 M */ + V4L2_XFER_FUNC_709,/* Substitution for BT.470-2 B, G */ + V4L2_XFER_FUNC_709,/* Substitution for SMPTE 170M */ + V4L2_XFER_FUNC_SMPTE240M, + V4L2_XFER_FUNC_NONE, + V4L2_XFER_FUNC_SRGB, + }; + + if (transfer_characteristics < ARRAY_SIZE(xfer_funcs)) + return xfer_funcs[transfer_characteristics]; + + return V4L2_XFER_FUNC_DEFAULT; /* Reserved */ +} + +static enum v4l2_ycbcr_encoding uvc_ycbcr_enc(const u8 matrix_coefficients) +{ + /* V4L2 currently does not currently have definitions for all +* possible values of UVC matrix coefficients. If +* v4l2_ycbcr_encoding is extended with new values, the +* mapping below should be updated. +* +* Substitutions are taken from the mapping given for +* V4L2_YCBCR_ENC_DEFAULT documented in videodev2.h. +* +* FCC is assumed to be close enough to 601. +*/ + static const enum v4l2_ycbcr_encoding ycbcr_encs[] = { + V4L
Re: [PATCH v3] mm: Fix kthread_use_mm() vs TLB invalidate
Excerpts from pet...@infradead.org's message of August 21, 2020 11:04 pm: > On Fri, Aug 21, 2020 at 11:09:51AM +0530, Aneesh Kumar K.V wrote: >> Peter Zijlstra writes: >> >> > For SMP systems using IPI based TLB invalidation, looking at >> > current->active_mm is entirely reasonable. This then presents the >> > following race condition: >> > >> > >> > CPU0 CPU1 >> > >> > flush_tlb_mm(mm) use_mm(mm) >> > >> > tsk->active_mm = mm; >> > >> >if (tsk->active_mm == mm) >> > // flush TLBs >> > >> > switch_mm(old_mm,mm,tsk); >> > >> > >> > Where it is possible the IPI flushed the TLBs for @old_mm, not @mm, >> > because the IPI lands before we actually switched. >> > >> > Avoid this by disabling IRQs across changing ->active_mm and >> > switch_mm(). >> > >> > [ There are all sorts of reasons this might be harmless for various >> > architecture specific reasons, but best not leave the door open at >> > all. ] >> >> >> Do we have similar race with exec_mmap()? I am looking at exec_mmap() >> runnning parallel to do_exit_flush_lazy_tlb(). We can get >> >> if (current->active_mm == mm) { >> >> true and if we don't disable irq around updating tsk->mm/active_mm we >> can end up doing mmdrop on wrong mm? > > exec_mmap() is called after de_thread(), there should not be any mm > specific invalidations around I think. > > Then again, CLONE_VM without CLONE_THREAD might still be possible, so > yeah, we probably want IRQs disabled there too, just for consistency and > general paranoia if nothing else. The problem is probably not this TLB flushing race, but I think there is a lazy tlb race. call_usermodehelper() kernel_execve() old_mm = current->mm; active_mm = current->active_mm; *** preempt *** -->schedule() prev->active_mm = NULL; mmdrop(prev active mm) ... <--schedule() current->mm = mm; current->active_mm = mm; if (!old_mm) mmdrop(active_mm); /* double free! */ There's possibly other problematic interleavings. powerpc also has an issue with switching away a lazy tlb mm via IPI which is basically the same problem so I just illustrate the more general issue. I think we just make it a rule that these always get updated under local_irq_disable, to be safe. Trouble is we can't just do it, because some architectures can't do activate_mm with irqs disabled. ARM and UM, at least. UM can't even do preempt_disabled. We can probably change them to make them work, I'm not sure what the best way to go is, my first attempt is to require activate_mm to do the mm switching and the irq disable as well, but I'll need some help from the archs I'll send out rfcs in a minute. Thanks, Nick
Re: [PATCH net-next v1 3/3] hinic: add support to query function table
On 2020/8/28 3:44, Jakub Kicinski wrote: > On Thu, 27 Aug 2020 19:13:21 +0800 Luo bin wrote: >> +switch (idx) { >> +case VALID: >> +return funcfg_table_elem->dw0.bs.valid; >> +case RX_MODE: >> +return funcfg_table_elem->dw0.bs.nic_rx_mode; >> +case MTU: >> +return funcfg_table_elem->dw1.bs.mtu; >> +case VLAN_MODE: >> +return funcfg_table_elem->dw1.bs.vlan_mode; >> +case VLAN_ID: >> +return funcfg_table_elem->dw1.bs.vlan_id; >> +case RQ_DEPTH: >> +return funcfg_table_elem->dw13.bs.cfg_rq_depth; >> +case QUEUE_NUM: >> +return funcfg_table_elem->dw13.bs.cfg_q_num; > > The first two patches look fairly unobjectionable to me, but here the > information does not seem that driver-specific. What's vlan_mode, and > vlan_id in the context of PF? Why expose mtu, is it different than > netdev mtu? What's valid? rq_depth? > . > The vlan_mode and vlan_id in function table are provided for VF in QinQ scenario and they are useless for PF. Querying VF's function table is unsupported now, so there is no need to expose vlan_id and vlan mode and I'll remove them in my next patchset. The function table is saved in hw and we expose the mtu to ensure the mtu saved in hw is same with netdev mtu. The valid filed indicates whether this function is enabled or not and the hw can judge whether the RQ buffer in host is sufficient by comparing the values of rq depth, pi and ci.
[PATCH v2] stackleak: Fix a race between stack erasing sysctl handlers
There is a race between the assignment of `table->data` and write value to the pointer of `table->data` in the __do_proc_doulongvec_minmax() on the other thread. CPU0: CPU1: proc_sys_write stack_erasing_sysctlproc_sys_call_handler table->data = &state; stack_erasing_sysctl table->data = &state; proc_doulongvec_minmax do_proc_doulongvec_minmax sysctl_head_finish __do_proc_doulongvec_minmax unuse_table i = table->data; *i = val; // corrupt CPU1's stack Fix this by duplicating the `table`, and only update the duplicate of it. Fixes: 964c9dff0091 ("stackleak: Allow runtime disabling of kernel stack erasing") Signed-off-by: Muchun Song --- changelogs in v2: 1. Add more details about how the race happened to the commit message. kernel/stackleak.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/kernel/stackleak.c b/kernel/stackleak.c index a8fc9ae1d03d..fd95b87478ff 100644 --- a/kernel/stackleak.c +++ b/kernel/stackleak.c @@ -25,10 +25,15 @@ int stack_erasing_sysctl(struct ctl_table *table, int write, int ret = 0; int state = !static_branch_unlikely(&stack_erasing_bypass); int prev_state = state; + struct ctl_table dup_table = *table; - table->data = &state; - table->maxlen = sizeof(int); - ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos); + /* +* In order to avoid races with __do_proc_doulongvec_minmax(), we +* can duplicate the @table and alter the duplicate of it. +*/ + dup_table.data = &state; + dup_table.maxlen = sizeof(int); + ret = proc_dointvec_minmax(&dup_table, write, buffer, lenp, ppos); state = !!state; if (ret || !write || state == prev_state) return ret; -- 2.11.0
Re: [PATCH] coresight: cti: write regsiters directly in cti_enable_hw()
On Fri, Aug 28, 2020 at 02:12:53AM +0800, Mathieu Poirier wrote: > Hi Tingwei, > > On Tue, Aug 18, 2020 at 07:10:57PM +0800, Tingwei Zhang wrote: > > Deadlock as below is triggered by one CPU holds drvdata->spinlock > > and calls cti_enable_hw(). Smp_call_function_single() is called > > in cti_enable_hw() and tries to let another CPU write CTI registers. > > That CPU is trying to get drvdata->spinlock in cti_cpu_pm_notify() > > and doesn't response to IPI from smp_call_function_single(). > > > > [ 988.335937] CPU: 6 PID: 10258 Comm: sh Tainted: GWL > > 5.8.0-rc6-mainline-16783-gc38daa79b26b-dirty #1 > > [ 988.346364] Hardware name: Thundercomm Dragonboard 845c (DT) > > [ 988.352073] pstate: 2045 (nzCv daif +PAN -UAO BTYPE=--) > > [ 988.357689] pc : smp_call_function_single+0x158/0x1b8 > > [ 988.362782] lr : smp_call_function_single+0x124/0x1b8 > > ... > > [ 988.451638] Call trace: > > [ 988.454119] smp_call_function_single+0x158/0x1b8 > > [ 988.458866] cti_enable+0xb4/0xf8 [coresight_cti] > > [ 988.463618] coresight_control_assoc_ectdev+0x6c/0x128 [coresight] > > [ 988.469855] coresight_enable+0x1f0/0x364 [coresight] > > [ 988.474957] enable_source_store+0x5c/0x9c [coresight] > > [ 988.480140] dev_attr_store+0x14/0x28 > > [ 988.483839] sysfs_kf_write+0x38/0x4c > > [ 988.487532] kernfs_fop_write+0x1c0/0x2b0 > > [ 988.491585] vfs_write+0xfc/0x300 > > [ 988.494931] ksys_write+0x78/0xe0 > > [ 988.498283] __arm64_sys_write+0x18/0x20 > > [ 988.502240] el0_svc_common+0x98/0x160 > > [ 988.506024] do_el0_svc+0x78/0x80 > > [ 988.509377] el0_sync_handler+0xd4/0x270 > > [ 988.513337] el0_sync+0x164/0x180 > > > > Was this the full log or you did cut some of it? > I cut some CPU registers' value since it's too long and not relevant. The Call trace is full. > > This change write CTI registers directly in cti_enable_hw(). > > Config->hw_powered has been checked to be true with spinlock holded. > > CTI is powered and can be programmed until spinlock is released. > > > > From your explanation above it seems that cti_enable_hw() was called from, > say > CPUy, to enable the CTI associated to CPUx. CTIx's drvdata->spinlock was > taken > and smp_call_function_single() called right after. That woke up CPUx and > cti_cpu_pm_notify() was executed on CPUx in interrupt context, trying to > take > CTIx's drvdata->spinlock. That hung CPUx and the kernel got angry. Is my > assessment correct? > Most of them is correct. The only difference is CPUx is power on when cti_enable_hw() is called. Otherwise it will goto cti_state_unchanged: and won't call cti_enable_hw_smp_call(). cti_cpu_pm_notify() is called when CPUx tries to suspend instead of resume. > If so I don't think the fix suggested in this patch will work. The same > condition will happen whenever cti_enable_hw() is called on a CPU to > enable a > CTI that belongs to another CPU and that cti_cpu_pm_notify() is called on > latter > CPU at the same time. > I'm not sure I understand this correctly. Let me clarify it a little bit. It's a deadlock since cti_enable_hw() holds the spinlock and calls cti_enable_hw_smp_call() from CPUx to enable CTI associated to CPUy. It waits for cti_enable_hw_smp_call() to return. IPI is sent to CPUy while CPUy is in cti_cpu_pm_notify() and waits for spinlock. In this patch, I remove cti_enable_hw_smp_call() and write CTI CPU directly on CPUx. It won't wait for CPUy and release spinlock after program registers of CTI. After cti_enable_hw() releases spinlock, cti_cpu_pm_notify() will continue to run. Since spinlock is held and config->hw_powered is true, we don't need to worry about CPUy power down when we program CTI on CPUx. > I think a better solution is to grab the lock in cti_enable_hw() and check > the > value of ->ctidev.cpu. If not a global CPU, i.e >= 0, then release the > lock and > call smp_call_function_single(). In cti_enable_hw_smp_call() take the > lock > again and move forward from there. > After cti_enable_hw() releases the lock, it's possible that CPU is offline by user, cti_enable_hw_smp_call() will fail in this case. > I have applied the other two patches in this set so no need to send them > again. > Thanks, Tingwei > Thanks, > Mathieu > > > Fixes: 6a0953ce7de9 ("coresight: cti: Add CPU idle pm notifer to CTI > devices") > > Signed-off-by: Tingwei Zhang > > --- > > drivers/hwtracing/coresight/coresight-cti.c | 17 + > > 1 file changed, 1 insertion(+), 16 deletions(-) > > > > diff --git a/drivers/hwtracing/coresight/coresight-cti.c > b/drivers/hwtracing/coresight/coresight-cti.c > > index 3ccc703dc940..869569eb8c7f 100644 > > --- a/drivers/hwtracing/coresight/coresight-cti.c > > +++ b/drivers/hwtracing/coresight/coresight-cti.c > > @@ -86,13 +86,6 @@ void cti_write_all_hw_regs(struct cti_drvdata > *drvdata) > > CS_LOCK(drvdata->base); > > } > > > > -static void cti_enable_hw_smp_call(void *info) > > -{ > > - struct cti_drvda
[PATCH] arm64: fix some spelling mistakes in the comments by codespell
arch/arm64/include/asm/cpu_ops.h:24: necesary ==> necessary arch/arm64/include/asm/kvm_arm.h:69: maintainance ==> maintenance arch/arm64/include/asm/cpufeature.h:361: capabilties ==> capabilities arch/arm64/kernel/perf_regs.c:19: compatability ==> compatibility arch/arm64/kernel/smp_spin_table.c:86: endianess ==> endianness arch/arm64/kernel/smp_spin_table.c:88: endianess ==> endianness arch/arm64/kvm/vgic/vgic-mmio-v3.c:1004: targetting ==> targeting arch/arm64/kvm/vgic/vgic-mmio-v3.c:1005: targetting ==> targeting Signed-off-by: Xiaoming Ni --- arch/arm64/include/asm/cpu_ops.h| 2 +- arch/arm64/include/asm/cpufeature.h | 2 +- arch/arm64/include/asm/kvm_arm.h| 2 +- arch/arm64/kernel/perf_regs.c | 2 +- arch/arm64/kernel/smp_spin_table.c | 4 ++-- arch/arm64/kvm/vgic/vgic-mmio-v3.c | 4 ++-- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/arch/arm64/include/asm/cpu_ops.h b/arch/arm64/include/asm/cpu_ops.h index d28e8f37d3b4..e95c4df83911 100644 --- a/arch/arm64/include/asm/cpu_ops.h +++ b/arch/arm64/include/asm/cpu_ops.h @@ -21,7 +21,7 @@ * mechanism for doing so, tests whether it is possible to boot * the given CPU. * @cpu_boot: Boots a cpu into the kernel. - * @cpu_postboot: Optionally, perform any post-boot cleanup or necesary + * @cpu_postboot: Optionally, perform any post-boot cleanup or necessary * synchronisation. Called from the cpu being booted. * @cpu_can_disable: Determines whether a CPU can be disabled based on * mechanism-specific information. diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index 89b4f0142c28..3a42dc8e697c 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -358,7 +358,7 @@ static inline int cpucap_default_scope(const struct arm64_cpu_capabilities *cap) } /* - * Generic helper for handling capabilties with multiple (match,enable) pairs + * Generic helper for handling capabilities with multiple (match,enable) pairs * of call backs, sharing the same capability bit. * Iterate over each entry to see if at least one matches. */ diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h index 51c1d9918999..21f91aebc052 100644 --- a/arch/arm64/include/asm/kvm_arm.h +++ b/arch/arm64/include/asm/kvm_arm.h @@ -66,7 +66,7 @@ * TWI:Trap WFI * TIDCP: Trap L2CTLR/L2ECTLR * BSU_IS: Upgrade barriers to the inner shareable domain - * FB: Force broadcast of all maintainance operations + * FB: Force broadcast of all maintenance operations * AMO:Override CPSR.A and enable signaling with VA * IMO:Override CPSR.I and enable signaling with VI * FMO:Override CPSR.F and enable signaling with VF diff --git a/arch/arm64/kernel/perf_regs.c b/arch/arm64/kernel/perf_regs.c index 666b225aeb3a..94e8718e7229 100644 --- a/arch/arm64/kernel/perf_regs.c +++ b/arch/arm64/kernel/perf_regs.c @@ -16,7 +16,7 @@ u64 perf_reg_value(struct pt_regs *regs, int idx) /* * Our handling of compat tasks (PERF_SAMPLE_REGS_ABI_32) is weird, but -* we're stuck with it for ABI compatability reasons. +* we're stuck with it for ABI compatibility reasons. * * For a 32-bit consumer inspecting a 32-bit task, then it will look at * the first 16 registers (see arch/arm/include/uapi/asm/perf_regs.h). diff --git a/arch/arm64/kernel/smp_spin_table.c b/arch/arm64/kernel/smp_spin_table.c index c8a3fee00c11..5892e79fa429 100644 --- a/arch/arm64/kernel/smp_spin_table.c +++ b/arch/arm64/kernel/smp_spin_table.c @@ -83,9 +83,9 @@ static int smp_spin_table_cpu_prepare(unsigned int cpu) /* * We write the release address as LE regardless of the native -* endianess of the kernel. Therefore, any boot-loaders that +* endianness of the kernel. Therefore, any boot-loaders that * read this address need to convert this address to the -* boot-loader's endianess before jumping. This is mandated by +* boot-loader's endianness before jumping. This is mandated by * the boot protocol. */ writeq_relaxed(__pa_symbol(secondary_holding_pen), release_addr); diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c index 5c786b915cd3..52d6f24f65dc 100644 --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c @@ -1001,8 +1001,8 @@ void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg, bool allow_group1) raw_spin_lock_irqsave(&irq->irq_lock, flags); /* -* An access targetting Group0 SGIs can only generate -* those, while an access targetting Group1 SGIs can +* An access targeting Group0 SGIs can only generate +* those, while an access targeting Group1 SGIs can
Re: [PATCH] net: dsa: mt7530: fix advertising unsupported
On 8/27/2020 2:15 AM, Landen Chao wrote: 1000baseT_Half Looks like this part of the commit subject spilled into the commit message. Remove 1000baseT_Half to advertise correct hardware capability in phylink_validate() callback function. Fixes: 38f790a80560 ("net: dsa: mt7530: Add support for port 5") Signed-off-by: Landen Chao Reviewed-by: Florian Fainelli -- Florian
possible deadlock in proc_pid_stack (2)
Hello, syzbot found the following issue on: HEAD commit:494d311a Add linux-next specific files for 20200821 git tree: linux-next console output: https://syzkaller.appspot.com/x/log.txt?x=1644d50e90 kernel config: https://syzkaller.appspot.com/x/.config?x=a61d44f28687f508 dashboard link: https://syzkaller.appspot.com/bug?extid=a26ada9907073b2f6f97 compiler: gcc (GCC) 10.1.0-syz 20200507 Unfortunately, I don't have any reproducer for this issue yet. IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+a26ada9907073b2f6...@syzkaller.appspotmail.com == WARNING: possible circular locking dependency detected 5.9.0-rc1-next-20200821-syzkaller #0 Not tainted -- syz-executor.2/26207 is trying to acquire lock: 8880a064e688 (&sig->exec_update_mutex){+.+.}-{3:3}, at: lock_trace fs/proc/base.c:408 [inline] 8880a064e688 (&sig->exec_update_mutex){+.+.}-{3:3}, at: proc_pid_stack+0xf0/0x2a0 fs/proc/base.c:452 but task is already holding lock: 88809dc4d8f8 (&p->lock){+.+.}-{3:3}, at: seq_read+0x61/0x1070 fs/seq_file.c:155 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #3 (&p->lock){+.+.}-{3:3}: __mutex_lock_common kernel/locking/mutex.c:956 [inline] __mutex_lock+0x134/0x10e0 kernel/locking/mutex.c:1103 seq_read+0x61/0x1070 fs/seq_file.c:155 do_loop_readv_writev fs/read_write.c:734 [inline] do_loop_readv_writev fs/read_write.c:721 [inline] do_iter_read+0x48e/0x6e0 fs/read_write.c:955 vfs_readv+0xe5/0x150 fs/read_write.c:1073 kernel_readv fs/splice.c:355 [inline] default_file_splice_read.constprop.0+0x4e6/0x9e0 fs/splice.c:412 do_splice_to+0x137/0x170 fs/splice.c:871 splice_direct_to_actor+0x307/0x980 fs/splice.c:950 do_splice_direct+0x1b3/0x280 fs/splice.c:1059 do_sendfile+0x55f/0xd40 fs/read_write.c:1540 __do_sys_sendfile64 fs/read_write.c:1601 [inline] __se_sys_sendfile64 fs/read_write.c:1587 [inline] __x64_sys_sendfile64+0x1cc/0x210 fs/read_write.c:1587 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 entry_SYSCALL_64_after_hwframe+0x44/0xa9 -> #2 (sb_writers#4){.+.+}-{0:0}: percpu_down_read include/linux/percpu-rwsem.h:51 [inline] __sb_start_write+0x234/0x470 fs/super.c:1672 sb_start_write include/linux/fs.h:1643 [inline] mnt_want_write+0x3a/0xb0 fs/namespace.c:354 ovl_do_remove+0xe1/0xe00 fs/overlayfs/dir.c:889 vfs_rmdir.part.0+0x113/0x430 fs/namei.c:3712 vfs_rmdir fs/namei.c:3698 [inline] do_rmdir+0x3ae/0x440 fs/namei.c:3773 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 entry_SYSCALL_64_after_hwframe+0x44/0xa9 -> #1 (&ovl_i_mutex_dir_key[depth]#2){}-{3:3}: down_read+0x96/0x420 kernel/locking/rwsem.c:1492 inode_lock_shared include/linux/fs.h:789 [inline] lookup_slow fs/namei.c:1560 [inline] walk_component+0x409/0x6a0 fs/namei.c:1860 lookup_last fs/namei.c:2309 [inline] path_lookupat+0x1ba/0x830 fs/namei.c:2333 filename_lookup+0x19f/0x560 fs/namei.c:2366 create_local_trace_uprobe+0x87/0x4e0 kernel/trace/trace_uprobe.c:1574 perf_uprobe_init+0x132/0x210 kernel/trace/trace_event_perf.c:323 perf_uprobe_event_init+0xff/0x1c0 kernel/events/core.c:9608 perf_try_init_event+0x12a/0x560 kernel/events/core.c:10927 perf_init_event kernel/events/core.c:10979 [inline] perf_event_alloc.part.0+0xe04/0x3790 kernel/events/core.c:11257 perf_event_alloc kernel/events/core.c:11636 [inline] __do_sys_perf_event_open+0x72c/0x2cb0 kernel/events/core.c:11752 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 entry_SYSCALL_64_after_hwframe+0x44/0xa9 -> #0 (&sig->exec_update_mutex){+.+.}-{3:3}: check_prev_add kernel/locking/lockdep.c:2496 [inline] check_prevs_add kernel/locking/lockdep.c:2601 [inline] validate_chain kernel/locking/lockdep.c:3218 [inline] __lock_acquire+0x2a6b/0x5640 kernel/locking/lockdep.c:4426 lock_acquire+0x1f1/0xad0 kernel/locking/lockdep.c:5005 __mutex_lock_common kernel/locking/mutex.c:956 [inline] __mutex_lock+0x134/0x10e0 kernel/locking/mutex.c:1103 lock_trace fs/proc/base.c:408 [inline] proc_pid_stack+0xf0/0x2a0 fs/proc/base.c:452 proc_single_show+0x116/0x1e0 fs/proc/base.c:775 seq_read+0x432/0x1070 fs/seq_file.c:208 do_loop_readv_writev fs/read_write.c:734 [inline] do_loop_readv_writev fs/read_write.c:721 [inline] do_iter_read+0x48e/0x6e0 fs/read_write.c:955 vfs_readv+0xe5/0x150 fs/read_write.c:1073 do_preadv fs/read_write.c:1165 [inline] __do_sys_preadv fs/read_write.c:1215 [inline] __se_sys_preadv fs/read_write.c:1210 [inline]
Results from the 2020 Linux Foundation Technical Advisory Board election
This year's election for the Linux Foundation Technical Advisory Board had 955 authorized voters; 235 of them cast ballots. The results were: 1: Laura Abbott 2: Kees Cook 3: Dan Williams 4: Christian Brauner 5: Chris Mason 6: Olof Johansson 7: Frank Rowand The top five will serve two-year terms on the TAB. Many thanks to all who voted this year, and to all of the candidates. jon
[PATCH v2] mm/hugetlb: Fix a race between hugetlb sysctl handlers
There is a race between the assignment of `table->data` and write value to the pointer of `table->data` in the __do_proc_doulongvec_minmax() on the other thread. CPU0: CPU1: proc_sys_write hugetlb_sysctl_handler proc_sys_call_handler hugetlb_sysctl_handler_common hugetlb_sysctl_handler table->data = &tmp; hugetlb_sysctl_handler_common table->data = &tmp; proc_doulongvec_minmax do_proc_doulongvec_minmax sysctl_head_finish __do_proc_doulongvec_minmax unuse_table i = table->data; *i = val; // corrupt CPU1's stack Fix this by duplicating the `table`, and only update the duplicate of it. And introduce a helper of proc_hugetlb_doulongvec_minmax() to simplify the code. The following oops was seen: BUG: kernel NULL pointer dereference, address: #PF: supervisor instruction fetch in kernel mode #PF: error_code(0x0010) - not-present page Code: Bad RIP value. ... Call Trace: ? set_max_huge_pages+0x3da/0x4f0 ? alloc_pool_huge_page+0x150/0x150 ? proc_doulongvec_minmax+0x46/0x60 ? hugetlb_sysctl_handler_common+0x1c7/0x200 ? nr_hugepages_store+0x20/0x20 ? copy_fd_bitmaps+0x170/0x170 ? hugetlb_sysctl_handler+0x1e/0x20 ? proc_sys_call_handler+0x2f1/0x300 ? unregister_sysctl_table+0xb0/0xb0 ? __fd_install+0x78/0x100 ? proc_sys_write+0x14/0x20 ? __vfs_write+0x4d/0x90 ? vfs_write+0xef/0x240 ? ksys_write+0xc0/0x160 ? __ia32_sys_read+0x50/0x50 ? __close_fd+0x129/0x150 ? __x64_sys_write+0x43/0x50 ? do_syscall_64+0x6c/0x200 ? entry_SYSCALL_64_after_hwframe+0x44/0xa9 Fixes: e5ff215941d5 ("hugetlb: multiple hstates for multiple page sizes") Signed-off-by: Muchun Song --- chagelogs in v2: 1. Add more details about how the race happened to the commit message. 2. Remove unnecessary assignment of table->maxlen. mm/hugetlb.c | 26 -- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/mm/hugetlb.c b/mm/hugetlb.c index a301c2d672bf..4c2a2620eeed 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -3454,6 +3454,22 @@ static unsigned int allowed_mems_nr(struct hstate *h) } #ifdef CONFIG_SYSCTL +static int proc_hugetlb_doulongvec_minmax(struct ctl_table *table, int write, + void *buffer, size_t *length, + loff_t *ppos, unsigned long *out) +{ + struct ctl_table dup_table; + + /* +* In order to avoid races with __do_proc_doulongvec_minmax(), we +* can duplicate the @table and alter the duplicate of it. +*/ + dup_table = *table; + dup_table.data = out; + + return proc_doulongvec_minmax(&dup_table, write, buffer, length, ppos); +} + static int hugetlb_sysctl_handler_common(bool obey_mempolicy, struct ctl_table *table, int write, void *buffer, size_t *length, loff_t *ppos) @@ -3465,9 +3481,8 @@ static int hugetlb_sysctl_handler_common(bool obey_mempolicy, if (!hugepages_supported()) return -EOPNOTSUPP; - table->data = &tmp; - table->maxlen = sizeof(unsigned long); - ret = proc_doulongvec_minmax(table, write, buffer, length, ppos); + ret = proc_hugetlb_doulongvec_minmax(table, write, buffer, length, ppos, +&tmp); if (ret) goto out; @@ -3510,9 +3525,8 @@ int hugetlb_overcommit_handler(struct ctl_table *table, int write, if (write && hstate_is_gigantic(h)) return -EINVAL; - table->data = &tmp; - table->maxlen = sizeof(unsigned long); - ret = proc_doulongvec_minmax(table, write, buffer, length, ppos); + ret = proc_hugetlb_doulongvec_minmax(table, write, buffer, length, ppos, +&tmp); if (ret) goto out; -- 2.11.0