[PATCH v1] powerpc/powernv/pci: fix PE in re-used pci_dn for pnv_pci_enable_device_hook
after hot remove a pcie deivce with pci_dn having pnp_php driver attached, pci rescan with echo 1 > /sys/bus/pci/rescan could fail with error message like: pci 0020:0e:00.0: BAR 0: assigned [mem 0x3fe80182-0x3fe80182 64bit] nvme nvme1: pci function 0020:0e:00.0 nvme 0020:0e:00.0 pci_enable_device() blocked, no PE assigned. It appears that the pci_dn object is reused with only pe_number clobbered in the case. And a simple call to pnv_ioda_setup_dev_PE should get PE number back and solve the problem. Signed-off-by: Luming Yu --- v0 -> v1: -clean up garbage leaked in git format patch that stems from git clone and checkout -conflicts of files in local windows filesystem with weird cases and names quriks. --- arch/powerpc/platforms/powernv/pci-ioda.c | 11 +- 1 files changed, 9 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c index 28fac4770073..9d7add79ee3d 100644 --- a/arch/powerpc/platforms/powernv/pci-ioda.c +++ b/arch/powerpc/platforms/powernv/pci-ioda.c @@ -2325,11 +2325,18 @@ static resource_size_t pnv_pci_default_alignment(void) static bool pnv_pci_enable_device_hook(struct pci_dev *dev) { struct pci_dn *pdn; + struct pnv_ioda_pe *pe; pdn = pci_get_pdn(dev); - if (!pdn || pdn->pe_number == IODA_INVALID_PE) { - pci_err(dev, "pci_enable_device() blocked, no PE assigned.\n"); + if (!pdn) return false; + + if (pdn->pe_number == IODA_INVALID_PE) { + pe = pnv_ioda_setup_dev_PE(dev); + if (!pe) { + pci_err(dev, "pci_enable_device() blocked, no PE assigned.\n"); + return false; + } } return true;
linux-next: build warning after merge of the mm tree
Hi all, After merging the mm tree, today's linux-next build (powerpc allyesconfig) produced this warning: arch/powerpc/kernel/swsusp_64.c:14:6: warning: no previous prototype for 'do_after_copyback' [-Wmissing-prototypes] 14 | void do_after_copyback(void) | ^ Exposed by commit c6345dfa6e3e ("Makefile.extrawarn: turn on missing-prototypes globally") -- Cheers, Stephen Rothwell pgpaJJeShMjb3.pgp Description: OpenPGP digital signature
Re: [PATCH v2] powerpc: Don't clobber fr0/vs0 during fp|altivec register save
On Tue Nov 28, 2023 at 10:59 AM AEST, Michael Ellerman wrote: > Christophe Leroy writes: > > Le 27/11/2023 à 19:39, Timothy Pearson a écrit : > >> Just wanted to check back and see if this patch was going to be > >> queued up soon? We're still having to work around / advertise the > >> data destruction issues the underlying bug is causing on e.g. Debian > >> Stable. > > > > Has any agreement been reach on the final solution ? Seeing the many > > discussion on patch v2 I had the feeling that it was not the final solution. > > The actual patch is fine I think. > > The discussion was about improving the explanation of exactly what's > happening in the change log, and whether there is a larger bug causing > FP corruption unrelated to io-uring. One thing I said is maybe we should remove the "register save" concept entirely and always giveup. But that would not be suitable for a minimal fix. I didn't mean it as an alternate fix. Even if we did always giveup in future, this patch still gives a nicer API. There would have to be a noticable performance advantage to not restoring fr0/vs0 after saving before we'd think about changing it back to clobber, since that encourages foot shooting. Thanks, Nick
Re: [PATCH v2] powerpc: Don't clobber fr0/vs0 during fp|altivec register save
Christophe Leroy writes: > Le 27/11/2023 à 19:39, Timothy Pearson a écrit : >> Just wanted to check back and see if this patch was going to be >> queued up soon? We're still having to work around / advertise the >> data destruction issues the underlying bug is causing on e.g. Debian >> Stable. > > Has any agreement been reach on the final solution ? Seeing the many > discussion on patch v2 I had the feeling that it was not the final solution. The actual patch is fine I think. The discussion was about improving the explanation of exactly what's happening in the change log, and whether there is a larger bug causing FP corruption unrelated to io-uring. I'm now reasonably confident there's no detectable corruption of fr0 happening except via the io-uring -> clone path. It's still a bad bug for us to corrupt fr0 across sys_clone(), but in practice it doesn't affect userspace because fr0 is volatile across function calls. cheers
[PATCH] powerpc/rtas_pci: rename and properly expose config access APIs
it a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c index def184da51cf..b1ae0c0d1187 100644 --- a/arch/powerpc/platforms/pseries/eeh_pseries.c +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c @@ -252,7 +252,7 @@ static int pseries_eeh_cap_start(struct pci_dn *pdn) if (!pdn) return 0; - rtas_read_config(pdn, PCI_STATUS, 2, &status); + rtas_pci_dn_read_config(pdn, PCI_STATUS, 2, &status); if (!(status & PCI_STATUS_CAP_LIST)) return 0; @@ -270,11 +270,11 @@ static int pseries_eeh_find_cap(struct pci_dn *pdn, int cap) return 0; while (cnt--) { - rtas_read_config(pdn, pos, 1, &pos); + rtas_pci_dn_read_config(pdn, pos, 1, &pos); if (pos < 0x40) break; pos &= ~3; - rtas_read_config(pdn, pos + PCI_CAP_LIST_ID, 1, &id); + rtas_pci_dn_read_config(pdn, pos + PCI_CAP_LIST_ID, 1, &id); if (id == 0xff) break; if (id == cap) @@ -294,7 +294,7 @@ static int pseries_eeh_find_ecap(struct pci_dn *pdn, int cap) if (!edev || !edev->pcie_cap) return 0; - if (rtas_read_config(pdn, pos, 4, &header) != PCIBIOS_SUCCESSFUL) + if (rtas_pci_dn_read_config(pdn, pos, 4, &header) != PCIBIOS_SUCCESSFUL) return 0; else if (!header) return 0; @@ -307,7 +307,7 @@ static int pseries_eeh_find_ecap(struct pci_dn *pdn, int cap) if (pos < 256) break; - if (rtas_read_config(pdn, pos, 4, &header) != PCIBIOS_SUCCESSFUL) + if (rtas_pci_dn_read_config(pdn, pos, 4, &header) != PCIBIOS_SUCCESSFUL) break; } @@ -412,8 +412,8 @@ static void pseries_eeh_init_edev(struct pci_dn *pdn) if ((pdn->class_code >> 8) == PCI_CLASS_BRIDGE_PCI) { edev->mode |= EEH_DEV_BRIDGE; if (edev->pcie_cap) { - rtas_read_config(pdn, edev->pcie_cap + PCI_EXP_FLAGS, -2, &pcie_flags); + rtas_pci_dn_read_config(pdn, edev->pcie_cap + PCI_EXP_FLAGS, + 2, &pcie_flags); pcie_flags = (pcie_flags & PCI_EXP_FLAGS_TYPE) >> 4; if (pcie_flags == PCI_EXP_TYPE_ROOT_PORT) edev->mode |= EEH_DEV_ROOT_PORT; @@ -676,7 +676,7 @@ static int pseries_eeh_read_config(struct eeh_dev *edev, int where, int size, u3 { struct pci_dn *pdn = eeh_dev_to_pdn(edev); - return rtas_read_config(pdn, where, size, val); + return rtas_pci_dn_read_config(pdn, where, size, val); } /** @@ -692,7 +692,7 @@ static int pseries_eeh_write_config(struct eeh_dev *edev, int where, int size, u { struct pci_dn *pdn = eeh_dev_to_pdn(edev); - return rtas_write_config(pdn, where, size, val); + return rtas_pci_dn_write_config(pdn, where, size, val); } #ifdef CONFIG_PCI_IOV --- base-commit: 0d555b57ee660d8a871781c0eebf006e855e918d change-id: 20231127-rtas-pci-rw-config-20a5610f2d35 Best regards, -- Nathan Lynch
Re: [PATCH v2] powerpc: Don't clobber fr0/vs0 during fp|altivec register save
Timothy Pearson writes: > Just wanted to check back and see if this patch was going to be queued > up soon? We're still having to work around / advertise the data > destruction issues the underlying bug is causing on e.g. Debian > Stable. Yeah I'll apply it this week, so it will be in rc4. I wanted to be more certain the corruption only happens in practice with io-uring before applying it. cheers
Re: [PATCH v2 2/4] eventfd: simplify eventfd_signal()
On Wed, 2023-11-22 at 13:48 +0100, Christian Brauner wrote: > Ever since the evenfd type was introduced back in 2007 in commit s/evenfd/eventfd/ > e1ad7468c77d ("signal/timer/event: eventfd core") the > eventfd_signal() > function only ever passed 1 as a value for @n. There's no point in > keeping that additional argument. > > Signed-off-by: Christian Brauner > --- > arch/x86/kvm/hyperv.c | 2 +- > arch/x86/kvm/xen.c | 2 +- > drivers/accel/habanalabs/common/device.c | 2 +- > drivers/fpga/dfl.c | 2 +- > drivers/gpu/drm/drm_syncobj.c | 6 +++--- > drivers/gpu/drm/i915/gvt/interrupt.c | 2 +- > drivers/infiniband/hw/mlx5/devx.c | 2 +- > drivers/misc/ocxl/file.c | 2 +- > drivers/s390/cio/vfio_ccw_chp.c | 2 +- > drivers/s390/cio/vfio_ccw_drv.c | 4 ++-- > drivers/s390/cio/vfio_ccw_ops.c | 6 +++--- > drivers/s390/crypto/vfio_ap_ops.c | 2 +- Acked-by: Eric Farman # s390
[PATCH] powerpc/powernv: Add error handling to opal_prd_range_is_valid
In the opal_prd_range_is_valid function within opal-prd.c, error handling was missing for the of_get_address call. This patch adds necessary error checking, ensuring that the function gracefully handles scenarios where of_get_address fails. Signed-off-by: Haoran Liu --- arch/powerpc/platforms/powernv/opal-prd.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/platforms/powernv/opal-prd.c b/arch/powerpc/platforms/powernv/opal-prd.c index 327e2f76905d..b66b06efcef1 100644 --- a/arch/powerpc/platforms/powernv/opal-prd.c +++ b/arch/powerpc/platforms/powernv/opal-prd.c @@ -66,6 +66,8 @@ static bool opal_prd_range_is_valid(uint64_t addr, uint64_t size) const char *label; addrp = of_get_address(node, 0, &range_size, NULL); + if (!addrp) + continue; range_addr = of_read_number(addrp, 2); range_end = range_addr + range_size; -- 2.17.1
Re: Powerpc: maple_defconfig: kernel/rtas_pci.c:46:5: error: no previous prototype for function 'rtas_read_config' [-Werror,-Wmissing-prototypes]
Naresh Kamboju writes: > Following Powerpc maple_defconfig and other builds failed with gcc-13 / 8 > and clang toolchains on Linux next-20231127 tag. > > build: > * gcc-8-cell_defconfig > * gcc-8-maple_defconfig > * gcc-8-tinyconfig > * gcc-13-tinyconfig > * gcc-13-cell_defconfig > * gcc-13-maple_defconfig > * clang-17-cell_defconfig > * clang-17-tinyconfig > * clang-17-maple_defconfig > * clang-nightly-cell_defconfig > * clang-nightly-maple_defconfig > * clang-nightly-tinyconfig > > Reported-by: Linux Kernel Functional Testing > > Build logs: > --- > arch/powerpc/kernel/rtas_pci.c:46:5: error: no previous prototype for > function 'rtas_read_config' [-Werror,-Wmissing-prototypes] >46 | int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 > *val) > | ^ > arch/powerpc/kernel/rtas_pci.c:46:1: note: declare 'static' if the > function is not intended to be used outside of this translation unit >46 | int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 > *val) > | ^ > | static > arch/powerpc/kernel/rtas_pci.c:98:5: error: no previous prototype for > function 'rtas_write_config' [-Werror,-Wmissing-prototypes] >98 | int rtas_write_config(struct pci_dn *pdn, int where, int size, u32 > val) > | ^ > arch/powerpc/kernel/rtas_pci.c:98:1: note: declare 'static' if the > function is not intended to be used outside of this translation unit >98 | int rtas_write_config(struct pci_dn *pdn, int where, int size, u32 > val) > | ^ > | static > 2 errors generated. > make[5]: *** [scripts/Makefile.build:243: > arch/powerpc/kernel/rtas_pci.o] Error 1 This appears to be a latent issue in this code... the prototypes for rtas_read_config() and rtas_write_config() in asm/ppc-pci.h are guarded by #ifdef CONFIG_EEH for some reason. So I would expect this to happen whenever it is built with CONFIG_EEH disabled and -Wmissing-prototypes. So I guess it's fallout from commit c6345dfa6e3e ("Makefile.extrawarn: turn on missing-prototypes globally"). Unfortunately the resolution isn't as simple as moving the prototypes out of the CONFIG_EEH-guarded region, but I think I'll have a fix for this later today.
Re: [PATCH v2] powerpc: Don't clobber fr0/vs0 during fp|altivec register save
Hi, Le 27/11/2023 à 19:39, Timothy Pearson a écrit : > Just wanted to check back and see if this patch was going to be queued up > soon? We're still having to work around / advertise the data destruction > issues the underlying bug is causing on e.g. Debian Stable. > Has any agreement been reach on the final solution ? Seeing the many discussion on patch v2 I had the feeling that it was not the final solution. Christophe
Re: [PATCH v2 0/5] MODVERSIONS + RUST Redux
> > > > > > > With regards to future directions that likely won't work for loosening > > > > it: > > > > Unfortunately, the .rmeta format itself is not stable, so I wouldn't > > > > want to > > > > teach genksyms to open it up and split out the pieces for specific > > > > functions. > > > > Extending genksyms to parse Rust would also not solve the situation - > > > > layouts are allowed to differ across compiler versions or even (in rare > > > > cases) seemingly unrelated code changes. > > > > > > What do you mean by "layout" here? Yes, the crcs can be different > > > across compiler versions and seemingly unrelated code changes (genksyms > > > is VERY fragile) but that's ok, that's not what you are checking here. > > > You want to know if the rust function signature changes or not from the > > > last time you built the code, with the same compiler and options, that's > > > all you are verifying. What I mean by layout here is that if you write in Rust: struct Foo { x: i32, y: i32, } it is not guaranteed to have the same layout across different compilations, even within the same compiler. See https://doc.rust-lang.org/reference/type-layout.html#the-rust-representation Specifically, the compiler is allowed to arbitrarily insert padding, reorder fields, etc. on the same code as long as the overall alignment of the struct and individual alignment of the fields remains correct and non-overlapping. This means the compiler is *explicitly* allowed to, for example, permute x and y as an optimization. In the above example this is unlikely, but if you instead consider struct Bar { x: i8, y: i64, z: i8, } It's easy to see why the compiler might decide to structure this as y,x,z to reduce the size of the struct. Those optimization decisions may be affected by any other part of the code, PGO, etc. > > > > > > > Future directions that might work for loosening it: > > > > * Generating crcs from debuginfo + compiler + flags > > > > * Adding a feature to the rust compiler to dump this information. This > > > > is likely to > > > > get pushback because Rust's current stance is that there is no > > > > ability to load > > > > object code built against a different library. > > > > > > Why not parse the function signature like we do for C? Because the function signature is insufficient to check the ABI, see above. > > > > > > > Would setting up Rust symbols so that they have a crc built out of > > > > .rmeta be > > > > sufficient for you to consider this useful? If not, can you help me > > > > understand > > > > what level of precision would be required? > > > > > > What exactly does .rmeta have to do with the function signature? That's > > > all you care about here. The .rmeta file contains the decisions the compiler made about layout in the crate you're interfacing with. For example, the choice to encode Bar with a yxz field order would be written into the .rmeta file. > > > > > > > > > > rmeta is generated per crate. > > > > CRC is computed per symbol. > > > > They have different granularity. > > It is weird to refuse a module for incompatibility > > of a symbol that it is not using at all. > > I agree, this should be on a per-symbol basis, so the Rust > infrastructure in the kernel needs to be fixed up to support this > properly, not just ignored like this patchset does. I agree there is a divergence here, I tried to point it out so that it wouldn't be a surprise later. The .rmeta file itself (which is the only way we could know that the ABI actually matches, because layout decisions are in there) is an unstable format, which is why I would be reluctant to try to parse it and find only the relevant portions to hash. This isn't just a "technically unstable" format, but one in which the compiler essentially just serializes out relevant internal data structures, so any parser for it will involve significant alterations on compiler updates, which doesn't seem like a good plan. > > thanks, > > greg k-h Given the above additional information, would you be interested in a patchset which either: A. Computes the CRC off the Rust type signature, knowing the compiler is allowed to change the ABI based on information not contained in the CRC. B. Uses the CRC of the .rmeta file, knowing, as was pointed out, that this effectively contains the ABI of every symbol in the compilation unit, as well as inline functions and polymorphic functions. If neither of these works, we likely can't turn on MODVERSIONS+RUST until further work is done upstream in the compiler to export some of this data in an at least semi-stable fashion.
Re: [PATCH v2] powerpc: Don't clobber fr0/vs0 during fp|altivec register save
Just wanted to check back and see if this patch was going to be queued up soon? We're still having to work around / advertise the data destruction issues the underlying bug is causing on e.g. Debian Stable. Thanks!
Re: [PATCH] dt-bindings: fsl,dpaa2-console: drop unneeded quotes
On Wed, 22 Nov 2023 15:44:19 -0700, Rob Herring wrote: > Drop unneeded quotes over simple string values to fix a soon to be > enabled yamllint warning: > > [error] string value is redundantly quoted with any quotes (quoted-strings) > > Signed-off-by: Rob Herring > --- > Documentation/devicetree/bindings/misc/fsl,dpaa2-console.yaml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Applied, thanks!
Re: [linus:master] [file] 0ede61d858: will-it-scale.per_thread_ops -2.9% regression
On Mon, 27 Nov 2023 at 02:27, Christian Brauner wrote: > > So I've picked up your patch (vfs.misc). It's clever alright so thanks > for the comments in there otherwise I would've stared at this for far > too long. Note that I should probably have commented on one other thing: that whole "just load from fd[0] is always safe, because the fd[] array always exists". IOW, that whole "load and mask" thing only works when you know the array exists at all. Doing that "just mask the index" wouldn't be valid if "size = 0" is an option and might mean that we don't have an array at all (ie if "->fd" itself could be NULL. But we never have a completely empty file descriptor array, and fdp->fd is never NULL. At a minimum 'max_fds' is NR_OPEN_DEFAULT. (The whole 'tsk->files' could be NULL, but only for kernel threads or when exiting, so fget_task() will check for *that*, but it's a separate thing) So that's why it's safe to *entirely* remove the whole if (unlikely(fd >= fdt->max_fds)) test, and do it *all* with just "mask the index, and mask the resulting load". Because we can *always* do that load at "fdt->fd[0]", and we want to check the result for NULL anyway, so the "mask at the end and check for NULL" is both natural and generates very good code. Anyway, not a big deal, bit it might be worth noting before somebody tries the same trick on some other array that *could* be zero-sized and with a NULL base pointer, and where that 'array[0]' access isn't necessarily guaranteed to be ok. > It's a little unpleasant because of the cast-orama going on before we > check the file pointer but I don't see that it's in any way wrong. In my cleanup phase - which was a bit messy - I did wonder if I should have some helper for it, since it shows up in both __fget_files_rcu() and in files_lookup_fd_raw(). So I *could* have tried to add something like a "masked_rcu_dereference()" that took the base pointer, the index, and the mask, and did that whole dance. Or I could have had just a "mask_pointer()" function, which we do occasionally do in other places too (ie we hide data in low bits, and then we mask them away when the pointer is used as a pointer). But with only two users, it seemed to add more conceptual complexity than it's worth, and I was not convinced that we'd want to expose that pattern and have others use it. So having a helper might clarify things, but it might also encourage wrong users. I dunno. I suspect the only real use for this ends up being this very special "access the fdt->fd[] array using a file descriptor". Anyway, that's why I largely just did it with comments, and commented both places - and just kept the cast there in the open. Linus
Re: [PATCH] scsi: ipr: Remove obsolete check for old CPUs
Acked-by: Brian King -- Brian King Power Linux I/O IBM Linux Technology Center
Re: [PATCH V4] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf
Em Mon, Nov 27, 2023 at 11:12:57AM +, James Clark escreveu: > On 23/11/2023 16:02, Athira Rajeev wrote: > > Add rule in new Makefile "tests/Makefile.tests" for running > > shellcheck on shell test scripts. This automates below shellcheck > > into the build. > Seems to work really well. I also tested it on Ubuntu, and checked > NO_SHELLCHECK, cleaning and with and without shellcheck installed etc. > Reviewed-by: James Clark Tested on Fedora 38, works as advertised, applied. - Arnaldo
Re: linux-next: duplicate patch in the tty tree
On Mon, Nov 27, 2023 at 11:57:18AM +1100, Stephen Rothwell wrote: > Hi all, > > The following commit is also in the powerpc tree as a different commit > (but the same patch): > > aa46b225ebbf ("tty: hvc: hvc_opal: Convert to platform remove callback > returning void") > > This is commit > > f99c0e0d0859 ("tty: hvc: hvc_opal: Convert to platform remove callback > returning void") > > in the powerpc tree. Should be fine, thanks for the notice. greg k-h
Re: [PATCH 1/2] powerpc/rtas: Create rtas_busy_sleep function
Haren Myneni writes: > Move the RTAS delay sleep code to new rtas_busy_sleep(). It can > be called from HCALL delay code that needs to support both usleep() > or msleep() depends on delay value. While there may be some future utility in factoring out the code that handles extended delay statuses, your patch 2/2 isn't an appropriate use case: VAS makes hcalls and does not interact with RTAS. Create a separate helper function in the vas code or in the generic hcall support code please. I think it's OK if it somewhat duplicates logic from rtas_busy_delay().
[PATCH v2 19/32] fbdev/ps3fb: Initialize fb_ops with fbdev macros
Initialize the instance of struct fb_ops with fbdev initializer macros for framebuffers in virtual address space. Set the read/write, draw and mmap callbacks to the correct implementation and avoid implicit defaults. Also select the necessary helpers in Kconfig. Fbdev drivers sometimes rely on the callbacks being NULL for a default I/O-memory-based implementation to be invoked; hence requiring the I/O helpers to be built in any case. Setting all callbacks in all drivers explicitly will allow to make the I/O helpers optional. This benefits systems that do not use these functions. Signed-off-by: Thomas Zimmermann Cc: Michael Ellerman Cc: Nicholas Piggin Cc: Christophe Leroy Cc: linuxppc-dev@lists.ozlabs.org Reviewed-by: Javier Martinez Canillas --- drivers/video/fbdev/Kconfig | 5 + drivers/video/fbdev/ps3fb.c | 7 ++- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig index 4c82890887ba1..f9dab4c900332 100644 --- a/drivers/video/fbdev/Kconfig +++ b/drivers/video/fbdev/Kconfig @@ -1715,10 +1715,7 @@ config FB_IBM_GXT4500 config FB_PS3 tristate "PS3 GPU framebuffer driver" depends on FB && PS3_PS3AV - select FB_SYS_FILLRECT - select FB_SYS_COPYAREA - select FB_SYS_IMAGEBLIT - select FB_SYS_FOPS + select FB_SYSMEM_HELPERS help Include support for the virtual frame buffer in the PS3 platform. diff --git a/drivers/video/fbdev/ps3fb.c b/drivers/video/fbdev/ps3fb.c index de81ad3a5d1ed..de8d78bf070a0 100644 --- a/drivers/video/fbdev/ps3fb.c +++ b/drivers/video/fbdev/ps3fb.c @@ -939,15 +939,12 @@ static const struct fb_ops ps3fb_ops = { .owner = THIS_MODULE, .fb_open= ps3fb_open, .fb_release = ps3fb_release, - .fb_read= fb_sys_read, - .fb_write = fb_sys_write, + __FB_DEFAULT_SYSMEM_OPS_RDWR, .fb_check_var = ps3fb_check_var, .fb_set_par = ps3fb_set_par, .fb_setcolreg = ps3fb_setcolreg, .fb_pan_display = ps3fb_pan_display, - .fb_fillrect= sys_fillrect, - .fb_copyarea= sys_copyarea, - .fb_imageblit = sys_imageblit, + __FB_DEFAULT_SYSMEM_OPS_DRAW, .fb_mmap= ps3fb_mmap, .fb_blank = ps3fb_blank, .fb_ioctl = ps3fb_ioctl, -- 2.43.0
[PATCH v2 18/32] fbdev/ps3fb: Set FBINFO_VIRTFB flag
The ps3fb driver operates on system memory. Mark the framebuffer accordingly. Helpers operating on the framebuffer memory will test for the presence of this flag. Signed-off-by: Thomas Zimmermann Cc: Michael Ellerman Cc: Nicholas Piggin Cc: Christophe Leroy Cc: linuxppc-dev@lists.ozlabs.org Reviewed-by: Javier Martinez Canillas --- drivers/video/fbdev/ps3fb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/video/fbdev/ps3fb.c b/drivers/video/fbdev/ps3fb.c index 64d291d6b1532..de81ad3a5d1ed 100644 --- a/drivers/video/fbdev/ps3fb.c +++ b/drivers/video/fbdev/ps3fb.c @@ -1145,7 +1145,7 @@ static int ps3fb_probe(struct ps3_system_bus_device *dev) info->fix.smem_len = ps3fb_videomemory.size - GPU_FB_START; info->pseudo_palette = par->pseudo_palette; - info->flags = FBINFO_READS_FAST | + info->flags = FBINFO_VIRTFB | FBINFO_READS_FAST | FBINFO_HWACCEL_XPAN | FBINFO_HWACCEL_YPAN; retval = fb_alloc_cmap(&info->cmap, 256, 0); -- 2.43.0
Re: [PATCH 0/8] devm_led_classdev_register() usage problem
On Sat, Nov 25, 2023 at 03:47:41AM +0300, George Stark wrote: > On 11/24/23 18:28, Andy Shevchenko wrote: > > On Wed, Oct 25, 2023 at 04:07:29PM +0300, George Stark wrote: > > > Lots of drivers use devm_led_classdev_register() to register their led > > > objects > > > and let the kernel free those leds at the driver's remove stage. > > > It can lead to a problem due to led_classdev_unregister() > > > implementation calls led_set_brightness() to turn off the led. > > > led_set_brightness() may call one of the module's brightness_set > > > callbacks. > > > If that callback uses module's resources allocated without using devm > > > funcs() > > > then those resources will be already freed at module's remove() callback > > > and > > > we may have use-after-free situation. > > > > > > Here is an example: > > > > > > module_probe() > > > { > > > devm_led_classdev_register(module_brightness_set_cb); > > > mutex_init(&mutex); > > > } > > > > > > module_brightness_set_cb() > > > { > > > mutex_lock(&mutex); > > > do_set_brightness(); > > > mutex_unlock(&mutex); > > > } > > > > > > module_remove() > > > { > > > mutex_destroy(&mutex); > > > } > > > > > > at rmmod: > > > module_remove() > > > ->mutex_destroy(&mutex); > > > devres_release_all() > > > ->led_classdev_unregister(); > > > ->led_set_brightness(); > > > ->module_brightness_set_cb(); > > > ->mutex_lock(&mutex); /* use-after-free */ > > > > > > I think it's an architectural issue and should be discussed thoroughly. > > > Some thoughts about fixing it as a start: > > > 1) drivers can use devm_led_classdev_unregister() to explicitly free leds > > > before > > > dependend resources are freed. devm_led_classdev_register() remains being > > > useful > > > to simplify probe implementation. > > > As a proof of concept I examined all drivers from drivers/leds and > > > prepared > > > patches where it's needed. Sometimes it was not as clean as just calling > > > devm_led_classdev_unregister() because several drivers do not track > > > their leds object at all - they can call devm_led_classdev_register() and > > > drop the > > > returned pointer. In that case I used devres group API. > > > > > > Drivers outside drivers/leds should be checked too after discussion. > > > > > > 2) remove led_set_brightness from led_classdev_unregister() and force the > > > drivers > > > to turn leds off at shutdown. May be add check that led's brightness is 0 > > > at led_classdev_unregister() and put a warning to dmesg if it's not. > > > Actually in many cases it doesn't really need to turn off the leds > > > manually one-by-one > > > if driver shutdowns whole led controller. For the last case to disable > > > the warning > > > new flag can be brought in e.g LED_AUTO_OFF_AT_SHUTDOWN (similar to > > > LED_RETAIN_AT_SHUTDOWN). > > > > NAK. > > > > Just fix the drivers by wrapping mutex_destroy() into devm, There are many > > doing so. You may be brave enough to introduce devm_mutex_init() somewhere > > in include/linux/device* > > Just one thing about mutex_destroy(). It seems like there's no single > opinion on should it be called in 100% cases e.g. in remove() paths. > For example in iio subsystem Jonathan suggests it can be dropped in simple > cases: https://www.spinics.net/lists/linux-iio/msg73423.html > > So the question is can we just drop mutex_destroy() in module's remove() > callback here if that mutex is needed for devm subsequent callbacks? mutex_destroy() makes sense when debugging mutexes. It's harmless to drop, but will make life harder to one who is trying to debug something there... -- With Best Regards, Andy Shevchenko
Re: [PATCH] perf vendor events: Update datasource event name to fix duplicate events
On 23/11/23 9:31 pm, Athira Rajeev wrote: Running "perf list" on powerpc fails with segfault as below: ./perf list Segmentation fault (core dumped) This happens because of duplicate events in the json list. The powerpc Json event list contains some event with same event name, but different event code. They are: - PM_INST_FROM_L3MISS (Present in datasource and frontend) - PM_MRK_DATA_FROM_L2MISS (Present in datasource and marked) - PM_MRK_INST_FROM_L3MISS (Present in datasource and marked) - PM_MRK_DATA_FROM_L3MISS (Present in datasource and marked) pmu_events_table__num_events uses the value from table_pmu->num_entries which includes duplicate events as well. This causes issue during "perf list" and results in segmentation fault. Since both event codes are valid, append _DSRC to the Data Source events (datasource.json), so that they would have a unique name. Also add PM_DATA_FROM_L2MISS_DSRC and PM_DATA_FROM_L3MISS_DSRC events. With the fix, perf list works as expected. Fixes: fc1435807533 ("perf vendor events power10: Update JSON/events") Signed-off-by: Athira Rajeev I have tested the patch on Power10 machine. Perf list works correctly without any segfault now. # ./perf list List of pre-defined events (to be used in -e or -M): branch-instructions OR branches[Hardware event] branch-misses [Hardware event] Tested-by: Disha Goel --- .../arch/powerpc/power10/datasource.json | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/tools/perf/pmu-events/arch/powerpc/power10/datasource.json b/tools/perf/pmu-events/arch/powerpc/power10/datasource.json index 6b0356f2d301..0eeaaf1a95b8 100644 --- a/tools/perf/pmu-events/arch/powerpc/power10/datasource.json +++ b/tools/perf/pmu-events/arch/powerpc/power10/datasource.json @@ -99,6 +99,11 @@ "EventName": "PM_INST_FROM_L2MISS", "BriefDescription": "The processor's instruction cache was reloaded from a source beyond the local core's L2 due to a demand miss." }, + { +"EventCode": "0x0003C000C040", +"EventName": "PM_DATA_FROM_L2MISS_DSRC", +"BriefDescription": "The processor's L1 data cache was reloaded from a source beyond the local core's L2 due to a demand miss." + }, { "EventCode": "0x00038010C040", "EventName": "PM_INST_FROM_L2MISS_ALL", @@ -161,9 +166,14 @@ }, { "EventCode": "0x00078000C040", -"EventName": "PM_INST_FROM_L3MISS", +"EventName": "PM_INST_FROM_L3MISS_DSRC", "BriefDescription": "The processor's instruction cache was reloaded from beyond the local core's L3 due to a demand miss." }, + { +"EventCode": "0x0007C000C040", +"EventName": "PM_DATA_FROM_L3MISS_DSRC", +"BriefDescription": "The processor's L1 data cache was reloaded from beyond the local core's L3 due to a demand miss." + }, { "EventCode": "0x00078010C040", "EventName": "PM_INST_FROM_L3MISS_ALL", @@ -981,7 +991,7 @@ }, { "EventCode": "0x0003C000C142", -"EventName": "PM_MRK_DATA_FROM_L2MISS", +"EventName": "PM_MRK_DATA_FROM_L2MISS_DSRC", "BriefDescription": "The processor's L1 data cache was reloaded from a source beyond the local core's L2 due to a demand miss for a marked instruction." }, { @@ -1046,12 +1056,12 @@ }, { "EventCode": "0x00078000C142", -"EventName": "PM_MRK_INST_FROM_L3MISS", +"EventName": "PM_MRK_INST_FROM_L3MISS_DSRC", "BriefDescription": "The processor's instruction cache was reloaded from beyond the local core's L3 due to a demand miss for a marked instruction." }, { "EventCode": "0x0007C000C142", -"EventName": "PM_MRK_DATA_FROM_L3MISS", +"EventName": "PM_MRK_DATA_FROM_L3MISS_DSRC", "BriefDescription": "The processor's L1 data cache was reloaded from beyond the local core's L3 due to a demand miss for a marked instruction." }, {
Re: linux-next: manual merge of the tty tree with the powerpc tree
Hello, On Mon, Nov 27, 2023 at 10:00:58PM +1100, Michael Ellerman wrote: > Stephen Rothwell writes: > > Hi all, > > > > Today's linux-next merge of the tty tree got a conflict in: > > > > drivers/tty/hvc/hvc_console.h > > > > between commit: > > > > c9e38dc90e1c ("tty: hvc: Make hvc_remove() return no value") > > > > from the powerpc tree and commit: > > > > 7f30c19caf94 ("tty: hvc: Make hvc_remove() return no value") > > > > from the tty tree. > > > > These are slightly different versions of the same patch. > > I'll drop it from my tree. FTR: Regarding the slightly difference: The variant in the tty tree is the better one. So nothing to do for Greg (or me) here. Thanks Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
[PATCH] scsi: ipr: Remove obsolete check for old CPUs
The IPR driver has a routine to check whether it's running on certain CPU versions and if so whether the adapter is supported on that CPU. But none of the CPUs it checks for are supported by Linux anymore. The most recent CPU it checks for is Power4+ which was removed in commit 471d7ff8b51b ("powerpc/64s: Remove POWER4 support"). So drop the check. That makes the "testmode" module paramter unused, so remove it as well. Signed-off-by: Michael Ellerman --- drivers/scsi/ipr.c | 55 -- 1 file changed, 55 deletions(-) diff --git a/drivers/scsi/ipr.c b/drivers/scsi/ipr.c index 81e3d464d1f6..3819f7c42788 100644 --- a/drivers/scsi/ipr.c +++ b/drivers/scsi/ipr.c @@ -77,7 +77,6 @@ static LIST_HEAD(ipr_ioa_head); static unsigned int ipr_log_level = IPR_DEFAULT_LOG_LEVEL; static unsigned int ipr_max_speed = 1; -static int ipr_testmode = 0; static unsigned int ipr_fastfail = 0; static unsigned int ipr_transop_timeout = 0; static unsigned int ipr_debug = 0; @@ -193,8 +192,6 @@ module_param_named(max_speed, ipr_max_speed, uint, 0); MODULE_PARM_DESC(max_speed, "Maximum bus speed (0-2). Default: 1=U160. Speeds: 0=80 MB/s, 1=U160, 2=U320"); module_param_named(log_level, ipr_log_level, uint, 0); MODULE_PARM_DESC(log_level, "Set to 0 - 4 for increasing verbosity of device driver"); -module_param_named(testmode, ipr_testmode, int, 0); -MODULE_PARM_DESC(testmode, "DANGEROUS!!! Allows unsupported configurations"); module_param_named(fastfail, ipr_fastfail, int, S_IRUGO | S_IWUSR); MODULE_PARM_DESC(fastfail, "Reduce timeouts and retries"); module_param_named(transop_timeout, ipr_transop_timeout, int, 0); @@ -6416,45 +6413,6 @@ static const struct scsi_host_template driver_template = { .proc_name = IPR_NAME, }; -#ifdef CONFIG_PPC_PSERIES -static const u16 ipr_blocked_processors[] = { - PVR_NORTHSTAR, - PVR_PULSAR, - PVR_POWER4, - PVR_ICESTAR, - PVR_SSTAR, - PVR_POWER4p, - PVR_630, - PVR_630p -}; - -/** - * ipr_invalid_adapter - Determine if this adapter is supported on this hardware - * @ioa_cfg: ioa cfg struct - * - * Adapters that use Gemstone revision < 3.1 do not work reliably on - * certain pSeries hardware. This function determines if the given - * adapter is in one of these confgurations or not. - * - * Return value: - * 1 if adapter is not supported / 0 if adapter is supported - **/ -static int ipr_invalid_adapter(struct ipr_ioa_cfg *ioa_cfg) -{ - int i; - - if ((ioa_cfg->type == 0x5702) && (ioa_cfg->pdev->revision < 4)) { - for (i = 0; i < ARRAY_SIZE(ipr_blocked_processors); i++) { - if (pvr_version_is(ipr_blocked_processors[i])) - return 1; - } - } - return 0; -} -#else -#define ipr_invalid_adapter(ioa_cfg) 0 -#endif - /** * ipr_ioa_bringdown_done - IOA bring down completion. * @ipr_cmd: ipr command struct @@ -7385,19 +7343,6 @@ static int ipr_ioafp_page0_inquiry(struct ipr_cmnd *ipr_cmd) type[4] = '\0'; ioa_cfg->type = simple_strtoul((char *)type, NULL, 16); - if (ipr_invalid_adapter(ioa_cfg)) { - dev_err(&ioa_cfg->pdev->dev, - "Adapter not supported in this hardware configuration.\n"); - - if (!ipr_testmode) { - ioa_cfg->reset_retries += IPR_NUM_RESET_RELOAD_RETRIES; - ipr_initiate_ioa_reset(ioa_cfg, IPR_SHUTDOWN_NONE); - list_add_tail(&ipr_cmd->queue, - &ioa_cfg->hrrq->hrrq_free_q); - return IPR_RC_JOB_RETURN; - } - } - ipr_cmd->job_step = ipr_ioafp_page3_inquiry; ipr_ioafp_inquiry(ipr_cmd, 1, 0, -- 2.41.0
Re: [PATCH v13 17/35] KVM: Add transparent hugepage support for dedicated guest memory
On 11/2/23 16:46, Paolo Bonzini wrote: > On Thu, Nov 2, 2023 at 4:38 PM Sean Christopherson wrote: >> Actually, looking that this again, there's not actually a hard dependency on >> THP. >> A THP-enabled kernel _probably_ gives a higher probability of using >> hugepages, >> but mostly because THP selects COMPACTION, and I suppose because using THP >> for >> other allocations reduces overall fragmentation. > > Yes, that's why I didn't even bother enabling it unless THP is > enabled, but it makes even more sense to just try. > >> So rather than honor KVM_GUEST_MEMFD_ALLOW_HUGEPAGE iff THP is enabled, I >> think >> we should do the below (I verified KVM can create hugepages with THP=n). >> We'll >> need another capability, but (a) we probably should have that anyways and >> (b) it >> provides a cleaner path to adding PUD-sized hugepage support in the future. > > I wonder if we need KVM_CAP_GUEST_MEMFD_HUGEPAGE_PMD_SIZE though. This > should be a generic kernel API and in fact the sizes are available in > a not-so-friendly format in /sys/kernel/mm/hugepages. > > We should just add /sys/kernel/mm/hugepages/sizes that contains > "2097152 1073741824" on x86 (only the former if 1G pages are not > supported). > > Plus: is this the best API if we need something else for 1G pages? > > Let's drop *this* patch and proceed incrementally. (Again, this is > what I want to do with this final review: identify places that are > stil sticky, and don't let them block the rest). > > Coincidentially we have an open spot next week at plumbers. Let's > extend Fuad's section to cover more guestmem work. Hi, was there any outcome wrt this one? Based on my experience with THP's it would be best if userspace didn't have to opt-in, nor care about the supported size. If the given size is unaligned, provide a mix of large pages up to an aligned size, and for the rest fallback to base pages, which should be better than -EINVAL on creation (is it possible with the current implementation? I'd hope so so?). A way to opt-out from huge pages could be useful although there's always the risk of some initial troubles resulting in various online sources cargo-cult recommending to opt-out forever. Vlastimil
Re: [PATCH V4] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf
On 23/11/2023 16:02, Athira Rajeev wrote: > Add rule in new Makefile "tests/Makefile.tests" for running > shellcheck on shell test scripts. This automates below shellcheck > into the build. > Seems to work really well. I also tested it on Ubuntu, and checked NO_SHELLCHECK, cleaning and with and without shellcheck installed etc. Reviewed-by: James Clark > $ for F in $(find tests/shell/ -perm -o=x -name '*.sh'); do shellcheck > -S warning $F; done > > Condition for shellcheck is added in Makefile.perf to avoid build > breakage in the absence of shellcheck binary. Update Makefile.perf > to contain new rule for "SHELLCHECK_TEST" which is for making > shellcheck test as a dependency on perf binary. > > Added "tests/Makefile.tests" to run shellcheck on shellscripts in > tests/shell. The make rule "SHLLCHECK_RUN" ensures that, every > time during make, shellcheck will be run only on modified files > during subsequent invocations. By this, if any newly added shell > scripts or fixes in existing scripts breaks coding/formatting > style, it will get captured during the perf build. > > Example build failure by modifying probe_vfs_getname.sh in tests/shell: > > In tests/shell/probe_vfs_getname.sh line 8: > . $(dirname $0)/lib/probe.sh > ^---^ SC2046 (warning): Quote this to prevent word splitting. > > For more information: > https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word > splitt... > make[3]: *** > [/root/athira/perf-tools-next/tools/perf/tests/Makefile.tests:18: > tests/shell/.probe_vfs_getname.sh.shellcheck_log] Error 1 > make[2]: *** [Makefile.perf:686: SHELLCHECK_TEST] Error 2 > make[2]: *** Waiting for unfinished jobs > make[1]: *** [Makefile.perf:244: sub-make] Error 2 > make: *** [Makefile:70: all] Error 2 > > Here, like other files which gets created during > compilation (ex: .builtin-bench.o.cmd or .perf.o.cmd ), > create .shellcheck_log also as a hidden file. > Example: tests/shell/.probe_vfs_getname.sh.shellcheck_log > shellcheck is re-run if any of the script gets modified > based on its dependency of this log file. > > After this, for testing, changed "tests/shell/trace+probe_vfs_getname.sh" to > break shellcheck format. In the next make run, it is > also captured: > > In tests/shell/probe_vfs_getname.sh line 8: > . $(dirname $0)/lib/probe.sh > ^---^ SC2046 (warning): Quote this to prevent word splitting. > > For more information: > https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word > splitt... > make[3]: *** > [/root/athira/perf-tools-next/tools/perf/tests/Makefile.tests:18: > tests/shell/.probe_vfs_getname.sh.shellcheck_log] Error 1 > make[3]: *** Waiting for unfinished jobs > > In tests/shell/trace+probe_vfs_getname.sh line 14: > . $(dirname $0)/lib/probe.sh > ^---^ SC2046 (warning): Quote this to prevent word splitting. > > For more information: > https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word > splitt... > make[3]: *** > [/root/athira/perf-tools-next/tools/perf/tests/Makefile.tests:18: > tests/shell/.trace+probe_vfs_getname.sh.shellcheck_log] Error 1 > make[2]: *** [Makefile.perf:686: SHELLCHECK_TEST] Error 2 > make[2]: *** Waiting for unfinished jobs > make[1]: *** [Makefile.perf:244: sub-make] Error 2 > make: *** [Makefile:70: all] Error 2 > > Failure log can be found in the stdout of make itself. > > This is reported at build time. To be able to go ahead with > the build or disable shellcheck even though it is known that > some test is broken, add a "NO_SHELLCHECK" option. Example: > > make NO_SHELLCHECK=1 > > INSTALL libsubcmd_headers > INSTALL libsymbol_headers > INSTALL libapi_headers > INSTALL libperf_headers > INSTALL libbpf_headers > LINKperf > > Note: > This is tested on RHEL and also SLES. Use below check: > "$(shell which shellcheck 2> /dev/null)" to look for presence > of shellcheck binary. The approach "shell command -v" is not > used here. In some of the distros(RHEL), command is available > as executable file (/usr/bin/command). But in some distros(SLES), > it is a shell builtin and not available as executable file. > > Signed-off-by: Athira Rajeev > --- > changelog: > v3 -> v4: > Addressed review comments from James Clark. > - Made the shellcheck errors to be reported in make output > itself during make like any other build error. > - Removed creating .dep files. Instead use the log file > to determine whether shellcheck has to be re-run when > there is a change in source file. > - Change log file to have suffix as shellcheck_log so > as to differentiate it from test execution log. > - Also like other files which gets created during > compilation, example, .builtin-bench.o.cmd or .perf.o.cmd, > create .shellcheck_log as
Re: linux-next: manual merge of the tty tree with the powerpc tree
Stephen Rothwell writes: > Hi all, > > Today's linux-next merge of the tty tree got a conflict in: > > drivers/tty/hvc/hvc_console.h > > between commit: > > c9e38dc90e1c ("tty: hvc: Make hvc_remove() return no value") > > from the powerpc tree and commit: > > 7f30c19caf94 ("tty: hvc: Make hvc_remove() return no value") > > from the tty tree. > > These are slightly different versions of the same patch. I'll drop it from my tree. cheers
Re: [linus:master] [file] 0ede61d858: will-it-scale.per_thread_ops -2.9% regression
> So that nobody else would waste any time on this, attached is a new > attempt. This time actually tested *after* the changes. So I've picked up your patch (vfs.misc). It's clever alright so thanks for the comments in there otherwise I would've stared at this for far too long. It's a little unpleasant because of the cast-orama going on before we check the file pointer but I don't see that it's in any way wrong. And given how focussed people are with __fget_* performance I think it might even be the right thing to do. But the cleverness means we have the same logic slightly differently twice. Not too bad ofc but not too nice either especially because that rcu lookup is pretty complicated already. A few days ago I did just write a long explanatory off-list email to someone who had questions about this and who is fairly experienced so we're not making it easy on people. But performance or simplicity; one can't necessarily always have both.
Powerpc: maple_defconfig: kernel/rtas_pci.c:46:5: error: no previous prototype for function 'rtas_read_config' [-Werror,-Wmissing-prototypes]
Following Powerpc maple_defconfig and other builds failed with gcc-13 / 8 and clang toolchains on Linux next-20231127 tag. build: * gcc-8-cell_defconfig * gcc-8-maple_defconfig * gcc-8-tinyconfig * gcc-13-tinyconfig * gcc-13-cell_defconfig * gcc-13-maple_defconfig * clang-17-cell_defconfig * clang-17-tinyconfig * clang-17-maple_defconfig * clang-nightly-cell_defconfig * clang-nightly-maple_defconfig * clang-nightly-tinyconfig Reported-by: Linux Kernel Functional Testing Build logs: --- arch/powerpc/kernel/rtas_pci.c:46:5: error: no previous prototype for function 'rtas_read_config' [-Werror,-Wmissing-prototypes] 46 | int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val) | ^ arch/powerpc/kernel/rtas_pci.c:46:1: note: declare 'static' if the function is not intended to be used outside of this translation unit 46 | int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val) | ^ | static arch/powerpc/kernel/rtas_pci.c:98:5: error: no previous prototype for function 'rtas_write_config' [-Werror,-Wmissing-prototypes] 98 | int rtas_write_config(struct pci_dn *pdn, int where, int size, u32 val) | ^ arch/powerpc/kernel/rtas_pci.c:98:1: note: declare 'static' if the function is not intended to be used outside of this translation unit 98 | int rtas_write_config(struct pci_dn *pdn, int where, int size, u32 val) | ^ | static 2 errors generated. make[5]: *** [scripts/Makefile.build:243: arch/powerpc/kernel/rtas_pci.o] Error 1 steps to reproduce: # tuxmake --runtime podman --target-arch powerpc --toolchain clang-17 --kconfig maple_defconfig LLVM=1 LLVM_IAS=0 LD=powerpc64le-linux-gnu-ld Links: - https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20231127/testrun/21324129/suite/build/test/clang-17-maple_defconfig/log - https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20231127/testrun/21324129/suite/build/test/clang-17-maple_defconfig/history/ - https://storage.tuxsuite.com/public/linaro/lkft/builds/2Yk9XaK95NuGJL9barjaXrOWxib/ -- Linaro LKFT https://lkft.linaro.org
Re: [linus:master] [file] 0ede61d858: will-it-scale.per_thread_ops -2.9% regression
> I took a look at the code generation, and honestly, I think we're > better off just making __fget_files_rcu() have special logic for this > all, and not use __get_file_rcu(). My initial massaging of the patch did that btw. Then I sat there wondering whether it would matter if we just made it possible to reuse that code and I went through a bunch of iterations. Oh well, it seems to matter. > Comments? I also looked at that odd OPTIMIZER_HIDE_VAR() that Concept looks sane to me. > __get_file_rcu() does, and I don't get it. Both things come from > volatile accesses, I don't see the point of those games, but I also > didn't care, since it's no longer in a critical code path. > > Christian? Puts his completely imagined "I understand RCU head on". SLAB_TYPESAFE_BY_RCU makes the RCU consume memory ordering that the compiler doesn't officialy support (afaik) a bit wonky. So the thinking was that we could have code patterns where you could free the object and reallocate it while legitimatly passing the pointer recheck. In that case there is no memory ordering between the allocation and the pointer recheck because the last (re)allocation could have been after the rcu_dereference(). To combat that all future loads were made to have a dependency on the first load using the hidevar trick. I guess that might only be theoretically possible but not in practice? But then I liked that we explicitly commented on it as a reminder.
Re: [PATCH 00/17] tty: small cleanups and fixes
On 23. 11. 23, 21:19, Greg KH wrote: On Tue, Nov 21, 2023 at 10:22:41AM +0100, Jiri Slaby (SUSE) wrote: This is a series to fix/clean up some obvious issues I revealed during u8+size_t conversions (to be posted later). I applied most of these except the last few, as I think you were going to reorder them, right? Yes, great. I will rebase and see/resend what is missing. thanks, -- js suse labs
[PATCH 2/2] powerpc/pseries/vas: Call rtas_busy_sleep() to support HCALL delay
VAS allocate, modify and deallocate HCALLs returns H_LONG_BUSY_ORDER_1_MSEC or H_LONG_BUSY_ORDER_10_MSEC for busy delay and expects OS to reissue HCALL after that delay. But using msleep() will often sleep at least 20 msecs even though the hypervisor expects to reissue these HCALLs after 1 or 10msecs. It might cause these HCALLs takes longer when multiple threads issue open or close VAS windows simultaneously. So instead of using msleep(), call rtas_busy_sleep() which uses usleep_range() if the delay is <= 20msecs. Signed-off-by: Haren Myneni Suggested-by: Nathan Lynch --- arch/powerpc/platforms/pseries/vas.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/platforms/pseries/vas.c b/arch/powerpc/platforms/pseries/vas.c index 71d52a670d95..c0ffdfc51f96 100644 --- a/arch/powerpc/platforms/pseries/vas.c +++ b/arch/powerpc/platforms/pseries/vas.c @@ -18,6 +18,7 @@ #include #include #include +#include #include #include "vas.h" @@ -38,7 +39,13 @@ static long hcall_return_busy_check(long rc) { /* Check if we are stalled for some time */ if (H_IS_LONG_BUSY(rc)) { - msleep(get_longbusy_msecs(rc)); + /* +* Allocate, Modify and Deallocate HCALLs can return +* H_LONG_BUSY_ORDER_1_MSEC or H_LONG_BUSY_ORDER_10_MSEC +* and expects OS to reissue HCALL after 1msec or +* 10msecs. +*/ + rtas_busy_sleep(rc); rc = H_BUSY; } else if (rc == H_BUSY) { cond_resched(); -- 2.26.3
[PATCH 1/2] powerpc/rtas: Create rtas_busy_sleep function
Move the RTAS delay sleep code to new rtas_busy_sleep(). It can be called from HCALL delay code that needs to support both usleep() or msleep() depends on delay value. Signed-off-by: Haren Myneni --- arch/powerpc/include/asm/rtas.h | 1 + arch/powerpc/kernel/rtas.c | 56 ++--- 2 files changed, 32 insertions(+), 25 deletions(-) diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h index c697c3c74694..b389351a0045 100644 --- a/arch/powerpc/include/asm/rtas.h +++ b/arch/powerpc/include/asm/rtas.h @@ -435,6 +435,7 @@ extern void rtas_get_rtc_time(struct rtc_time *rtc_time); extern int rtas_set_rtc_time(struct rtc_time *rtc_time); extern unsigned int rtas_busy_delay_time(int status); +extern void rtas_busy_sleep(int value); bool rtas_busy_delay(int status); extern int early_init_dt_scan_rtas(unsigned long node, diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c index eddc031c4b95..aa0bd7c4dcf1 100644 --- a/arch/powerpc/kernel/rtas.c +++ b/arch/powerpc/kernel/rtas.c @@ -1250,6 +1250,36 @@ static bool __init rtas_busy_delay_early(int status) return retry; } +void rtas_busy_sleep(int value) +{ + unsigned int ms; + + ms = rtas_busy_delay_time(value); + /* +* The extended delay hint can be as high as 100 seconds. +* Surely any function returning such a status is either +* buggy or isn't going to be significantly slowed by us +* polling at 1HZ. Clamp the sleep time to one second. +*/ + ms = clamp(ms, 1U, 1000U); + /* +* The delay hint is an order-of-magnitude suggestion, not +* a minimum. It is fine, possibly even advantageous, for +* us to pause for less time than hinted. For small values, +* use usleep_range() to ensure we don't sleep much longer +* than actually needed. +* +* See Documentation/timers/timers-howto.rst for +* explanation of the threshold used here. In effect we use +* usleep_range() for 9900 and 9901, msleep() for +* 9902-9905. +*/ + if (ms <= 20) + usleep_range(ms * 100, ms * 1000); + else + msleep(ms); +} + /** * rtas_busy_delay() - helper for RTAS busy and extended delay statuses * @@ -1270,7 +1300,6 @@ static bool __init rtas_busy_delay_early(int status) */ bool __ref rtas_busy_delay(int status) { - unsigned int ms; bool ret; /* @@ -1282,30 +1311,7 @@ bool __ref rtas_busy_delay(int status) switch (status) { case RTAS_EXTENDED_DELAY_MIN...RTAS_EXTENDED_DELAY_MAX: ret = true; - ms = rtas_busy_delay_time(status); - /* -* The extended delay hint can be as high as 100 seconds. -* Surely any function returning such a status is either -* buggy or isn't going to be significantly slowed by us -* polling at 1HZ. Clamp the sleep time to one second. -*/ - ms = clamp(ms, 1U, 1000U); - /* -* The delay hint is an order-of-magnitude suggestion, not -* a minimum. It is fine, possibly even advantageous, for -* us to pause for less time than hinted. For small values, -* use usleep_range() to ensure we don't sleep much longer -* than actually needed. -* -* See Documentation/timers/timers-howto.rst for -* explanation of the threshold used here. In effect we use -* usleep_range() for 9900 and 9901, msleep() for -* 9902-9905. -*/ - if (ms <= 20) - usleep_range(ms * 100, ms * 1000); - else - msleep(ms); + rtas_busy_sleep(status); break; case RTAS_BUSY: ret = true; -- 2.26.3