Re: [PATCH v5 1/2] PCI support added to ARC
On Thursday 14 January 2016 05:26:58 Vineet Gupta wrote:
> > +/*
> > + * We don't have to worry about legacy ISA devices, so nothing to do here
> > + */
> > +resource_size_t pcibios_align_resource(void *data, const struct resource
> > *res,
> > + resource_size_t size, resource_size_t align)
> > +{
> > + return res->start;
> > +}
>
> Doesn't this have to be EXPORT_SYMBOL_xxx as well given that the call
> (setup-res.c) can build as module ?
I only see a caller in drivers/pci/setup-res.c, and that is never part of a
loadable module.
> > +
> > +void pcibios_fixup_bus(struct pci_bus *bus)
> > +{
> > +}
> > +EXPORT_SYMBOL(pcibios_fixup_bus);
>
> EXPORT_SYMBOL_GPL ?
>
> As a seperate enhancement, it would be nicer if these 2 functions are defined
> weak
> in common code. That would make basic PCI support almost arch independent !
I agree, that would be ideal. An easy way to do this would be to add
them as __weak functions in drivers/pci/, similar to how we handle
a lot of the other pcibios_* functions.
A somewhat nicer method would be to have callback pointers in struct
pci_host_bridge, and call those when they are non-NULL so we can
remove the global pcibios_* functions from the API. That would also
bring us a big step closer to having PCI support itself as a loadable
module, and it would better reflect that those functions are really
host bridge specific rather than architecture specific. When you use
the same host bridge on multiple architectures, you typically have
the same requirements for hacks in there, but each architectures may
need to support multiple host bridges with different requirements.
Arnd
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v5 1/2] PCI support added to ARC
On Thursday 14 January 2016 10:51:32 Vineet Gupta wrote: > >> > >> A somewhat nicer method would be to have callback pointers in struct > >> pci_host_bridge, and call those when they are non-NULL so we can > >> remove the global pcibios_* functions from the API. That would also > >> bring us a big step closer to having PCI support itself as a loadable > >> module, and it would better reflect that those functions are really > >> host bridge specific rather than architecture specific. When you use > >> the same host bridge on multiple architectures, you typically have > >> the same requirements for hacks in there, but each architectures may > >> need to support multiple host bridges with different requirements. > > Since we will be constantly improving the driver and the core itself, I > > suggest > > that this functions be made __weak and in an update we can turn it struct > > pointers just like Arnd suggested. Is this good for you? > > There is no point in making it weak, w/o a fallback version in generic code. > For > this series, I suggest you just remove the straggler EXPORT_SYMBOL and respin. > To clarify: I don't particularly like __weak functions anywhere, but they are already common in drivers/pci, so we can add a couple more to reach the goal of removing all architecture specific code. However, there should never be a reason to add a __weak function in arch code that gets overridden by common code, that would be very confusing and not helpful. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v7 2/2] add new platform driver for PCI RC
On Monday 01 February 2016 18:07:45 Joao Pinto wrote:
> This patch adds a new driver that will be the reference platform driver
> for all PCI RC IP Protoyping Kits based on ARC SDP.
> This patch is composed by:
>
> -MAINTAINERS file was updated to include the new driver
> -Documentation/devicetree/bindings/pci was updated to include the new
> driver documentation
> -New driver called pcie-synopsys
>
> Signed-off-by: Joao Pinto
I must have completely missed all the earlier submissions and just happened
to see this one now that it was merged.
I have a couple of comments, maybe you can send a follow up patch to address
them:
> +--
> +
> +This is the reference platform driver to be used in the Synopsys PCI Root
> +Complex IP Prototyping Kit.
> +
> +Required properties:
> +- compatible: set to "snps,pcie-synopsys";
Please also include the exact version of the designware PCIe part that
is used in here, such as "snps,dw-pcie-1.234". I'm sure you can find
out the version.
"snps,pcie-synopsys" by itself is about the worst possible compatible
string because it says nothing about the specific hardware. Half the
machines we support all have a designware PCIe host that was made
by synopsys. Please use the name of the SoC for the most specific
string.
> +- reg: base address and length of the pcie controller registers.
> +- #address-cells: set to <3>
> +- #size-cells: set to <2>
> +- device_type: set to "pci"
> +- ranges: ranges for the PCI memory and I/O regions.
> +- interrupts: one interrupt source for MSI interrupts, followed by interrupt
> + source for hardware related interrupts.
> +- #interrupt-cells: set to <1>
> +- num-lanes: set to <1>;
> +
> +Example configuration:
> +
> + pcie: pcie@0xd000 {
> + compatible = "snps,pcie-synopsys";
> + reg = <0xd000 0x1000>;
> + #address-cells = <3>;
> + #size-cells = <2>;
> + device_type = "pci";
> + ranges = <0x0800 0 0xd000 0xd000 0 0x2000>,
This line looks misplaced here and does not match the documentation. It's
probably a leftover from an earlier version that had the config space
in the ranges, so you need to remove that.
> +/* This handler was created for future work */
> +static irqreturn_t synopsys_pcie_irq_handler(int irq, void *arg)
> +{
> + return IRQ_NONE;
> +}
It's not harmful, but we generally don't add code "for future work",
better remove it for now.
> +static void synopsys_pcie_establish_link(struct pcie_port *pp)
> +{
> + int retries = 0;
> +
> + /* check if the link is up or not */
> + for (retries = 0; retries < 10; retries++) {
> + if (dw_pcie_link_up(pp)) {
> + dev_info(pp->dev, "Link up\n");
> + return;
> + }
> + mdelay(100);
> + }
> +}
That mdelay() needs to be an msleep(). You should never waste
a whole second worth of CPU time in a driver.
Arnd
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v7 2/2] add new platform driver for PCI RC
On Tuesday 02 February 2016 17:14:49 Bjorn Helgaas wrote:
> On Tue, Feb 02, 2016 at 09:25:25PM +0100, Arnd Bergmann wrote:
> > On Monday 01 February 2016 18:07:45 Joao Pinto wrote:
> > > +static void synopsys_pcie_establish_link(struct pcie_port *pp)
> > > +{
> > > + int retries = 0;
> > > +
> > > + /* check if the link is up or not */
> > > + for (retries = 0; retries < 10; retries++) {
> > > + if (dw_pcie_link_up(pp)) {
> > > + dev_info(pp->dev, "Link up\n");
> > > + return;
> > > + }
> > > + mdelay(100);
> > > + }
> > > +}
> >
> > That mdelay() needs to be an msleep(). You should never waste
> > a whole second worth of CPU time in a driver.
>
> There are six other copies of this code, and two of them (exynos and
> spear) have the same problem.
Good point. Maybe move one (correct) copy into the main driver file and
have it called by the other drivers?
IIRC there were some problems in drivers that did a similar
function from the config space accessors where you cannot call
msleep(), but this driver is not one of them.
Arnd
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v7 2/2] add new platform driver for PCI RC
On Wednesday 03 February 2016 12:38:44 Bjorn Helgaas wrote: > On Wed, Feb 03, 2016 at 06:12:00PM +, Joao Pinto wrote: > > > - replace "snps,pcie-synopsys" for "snps,pcie-synopsys-ipk"? > > This is a question for Arnd. > > > - rename the driver to pcie-synopsys-ipk? > > It doesn't seem necessary to me to include both "synopsys" and "ipk" in the > filename and the driver name. Take a look at what the existing drivers do, > and do something similar. The "synopsys" can go away, it's already in the vendor field of the string. "ipk" is still a bit unspecific, I was hoping to see a specific chip and/or version of the PCIe part. Something like compatible = "snps,ipk2040-pcie", "snps,ipk-pcie", "snps,dw-pcie-1.23", "snps,dw-pcie"; which would indicate that there is a chip called "ipk2040" in a family called "ipk", and this includes the designware pcie implementation in version 1.23. > > - update the devicetree documentation referring that the ranges also > > include the > > config space > > Another one for Arnd. This one is wrong, the ranges should *not* include the config space, and if they currently do, you must change the driver. The generic dw-pcie driver still accepts the config space in the ranges for backwards compatibility with some of the earlier front-ends that mistakenly did this, but new driver should not do the same, and we should probably add some code in the common driver to prevent it for front-ends other than the ones we have to keep compatibility with. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v7 2/2] add new platform driver for PCI RC
On Thursday 04 February 2016 11:10:55 Joao Pinto wrote: > > The "synopsys" can go away, it's already in the vendor field of the > > string. "ipk" is still a bit unspecific, I was hoping to see a specific > > chip and/or version of the PCIe part. Something like > > > > compatible = "snps,ipk2040-pcie", "snps,ipk-pcie", > > "snps,dw-pcie-1.23", "snps,dw-pcie"; > > > > which would indicate that there is a chip called "ipk2040" in a family > > called "ipk", > > and this includes the designware pcie implementation in version 1.23. > > > > "snps,dw-pcie" seems a good idea! > Just to clarify: I really meant you should put all four of the strings into the compatible property. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v7 2/2] add new platform driver for PCI RC
On Thursday 04 February 2016 14:09:26 Joao Pinto wrote: > Hi Bjorn and Arnd, > > Removing the irq_handler causes the irq request to fail because in > request_threaded_irq() both handler and thread_fn are NULL. > > What's the typical procedure for this? > Don't call request_irq? ;-) Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v8 2/2] add new platform driver for PCI RC
On Friday 05 February 2016 10:44:29 Joao Pinto wrote: > Hi, > > On 2/4/2016 11:43 PM, Bjorn Helgaas wrote: > >> What do you think? > > > > I don't think the "dw" part is relevant (none of the other > > DesignWare-based drivers includes it in the driver or file name). > > > > How do people typically refer to this board? > > > > I really like "synopsys" because it fits the pattern of being > > recognizable and pronounceable like "altera", "designware", "qcom", > > "keystone", "layerscape", "tegra", etc. But I can't tell whether it's > > too generic. > > > > "ipk" or "haps" would be fine with me. I think it's OK if it doesn't > > cover 100% of the possible systems. > > I think we should follow the iproc example: pcie-iproc-platform.c > In this case we would have pcie-designware-platform.c > I think this would be the best name because the driver is a non soc specific > designware platform driver. > > Arnd and Bjorn agree on this name? Sorry, I did not realize that your submission was for the generic dw-pcie implementation rather than a particular product integrating it. I think in this case, we should do this completely differently: How about putting all the new code into drivers/pci/host/pcie-designware.c as functions that can be used by the other drivers in absence of a chip specific handler? Instead of providing a new instance of struct pcie_host_ops, maybe add it as a default implementation in dw_pcie_link_up() and dw_pcie_host_init() for drivers that don't provide their own. "hisi_pcie_host_ops" currently provides no host_init() callback function, so you will have to change the hisi frontend to a provide nop-function. For all other drivers, check if they can be changed to use your generic implementation and remove their private callbacks if possible. I think the MSI implementation should be split out into a separate file though, as not everyone uses this. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v8 2/2] add new platform driver for PCI RC
On Friday 05 February 2016 14:51:39 Joao Pinto wrote: > > It is a driver that is useful for PCIe RC prototyping and to be a reference > platform driver for DesignWare PCIe RC, I don't know if merging some of the > driver's code into pcie-designware is really necessary depends on the > usefulness > of it. I would suggest that bigger step to be done in a 2nd stage since I will > be around to maintain what's necessary. Agree? > > I made a patch that is applicable to Bjorn's host/pcie-synopsys that tries to > match your suggestions and Bjorn's. Could you please comment check it? I think it would be useful to do this as generic as possible, and you seem to be the right person to integrate it into the generic driver as you have access to the hardware documents. Normally the folks writing the drivers are just guessing based on what little information they get out of manuals or vendor-provided drivers, but I'm sure that not all of that makes sense. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v8 2/2] add new platform driver for PCI RC
On Friday 05 February 2016 17:32:48 Bjorn Helgaas wrote:
> On Fri, Feb 05, 2016 at 03:39:05PM +0100, Arnd Bergmann wrote:
> > On Friday 05 February 2016 10:44:29 Joao Pinto wrote:
> > I think in this case, we should do this completely differently:
> >
> > How about putting all the new code into drivers/pci/host/pcie-designware.c
> > as functions that can be used by the other drivers in absence of a chip
> > specific handler?
> >
> > Instead of providing a new instance of struct pcie_host_ops, maybe add
> > it as a default implementation in dw_pcie_link_up() and dw_pcie_host_init()
> > for drivers that don't provide their own. "hisi_pcie_host_ops" currently
> > provides no host_init() callback function, so you will have to change
> > the hisi frontend to a provide nop-function.
> >
> > For all other drivers, check if they can be changed to use your generic
> > implementation and remove their private callbacks if possible.
> >
> > I think the MSI implementation should be split out into a separate file
> > though, as not everyone uses this.
>
> I'm not sure I understand what you're proposing, Arnd, so let me
> ramble and you can direct me back on course.
>
> Currently drivers/pci/host/pcie-designware.c is not usable by itself;
> it doesn't register a platform_driver.
>
> There's hardly any code in Joao's patches; it looks like they add a
> minimal wrapper around the functionality in pcie-designware.c and
> register it as a platform_driver.
>
> Are you suggesting that we should just add that functionality directly
> in pcie-designware.c so that file could both be a minimal driver with
> the functionality of Joao's patches, *and* continue to provide the
> shared code used by all the existing DesignWare-based drivers? Maybe
> the platform_driver registration part could be controlled by its own
> separate Kconfig option.
Either way is fine, we just have to be a little careful about the
initialization ordering.
> For example, he could make dw_pcie_link_up() look like:
>
> int dw_pcie_link_up(struct pcie_port *pp)
> {
> u32 val;
>
> if (pp->ops->link_up)
> return pp->ops->link_up(pp);
>
> val = readl(pp->dbi_base + PCIE_PHY_DEBUG_R1);
> return val & PCIE_PHY_DEBUG_R1_LINK_UP;
> }
This is definitely good (after checking that all existing drivers
either work with the generic version, or provide their own callbacks
already).
> That seems like it would make sense to me. It would resolve the
> filename question, since there wouldn't be a new file. And if this is
> merely a driver for the generic DesignWare core without any
> extensions, I'm happy with some sort of "dw"-based driver name and
> compatibility string.
The important part I think is that the new driver should not require
and code that is seen as soc-specific: If it works with any implementation
of pci-dw rather than a specific system, the driver should know how
to do the right thing.
It may be helpful to move the actual matching on the compatible string
and calling of the generic probe function into another module, if we
are going forward with loadable PCI host drivers as posted by
Paul Gortmaker today. Otherwise we end up with a device being bound
to the generic driver when a more specific one exists and both
are loadable modules, because the generic driver is always loaded
first. As long as both drivers are built-in, it works fine because
we first look for a driver matching the most specific compatible string.
Arnd
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] arc: use little endian accesses
On Thursday 10 March 2016, Vineet Gupta wrote: > On Wednesday 09 March 2016 10:51 PM, Lada Trimasova wrote: > > Memory access primitives should use cpu_to_le16, cpu_to_le32, le16_to_cpu > > and le32_to_cpu because it is not really guaranteed that drivers handles > > any ordering themselves. > > That is the driver issue. readxx as API simply returns data in native > endianness. > We've had EZChip running big endian and so far and they didn't need this > change. Most drivers using readl() or readl_relaxed() expect those to perform byte swaps on big-endian architectures, as the registers tend to be fixed endian, so the change seems reasonable. It depends a little bit on how endian mode is implemented in the CPU: do you follow the normal model of swapping byte order in the load/store instructions of the CPU when running big-endian, or is the CPU always running little-endian but the bus addresses get mangled on byte accesses to give the illusion of a big-endian system? Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] arc: use little endian accesses
On Thursday 10 March 2016, Lada Trimasova wrote: > Driver is 8250, kernel is built for BE arc, nsim option in model > "nsim_isa_big_endian = 1". > > With current "readl" and "writel" implementation for ARC we read word from > memory without any endianess manipulation. So in case of little endian > architecture we get what we want: the first memory byte is the low byte in > the word. But in case of big endian architecture the word endianess is > swapped: the first memory byte is the high word byte. > > And for example, let's see what happens when we use "readl" in function > "serial8250_early_in" in driver/tty/serial/8250. > > Take a look to one line from memory dump: > 0xf010: 0x0b0x000x000x000x600x000x000x00 > > When kernel is built for little endian architecture, we read this data in > status register in function "serial_putc" using "readl" function in > driver/tty/serial/8250 as: > r0: 0x006 > The low byte is 0x0b, so the condition "if ((status & BOTH_EMPTY) == > BOTH_EMPTY)" is true, as BOTH_EMPTY is some mask with low bytes set. > > When kernel is built with "CPU_BIG_ENDIAN" and model nsim option is > "nsim_isa_big_endian=1", we read this data as: > r0: 0x600 > So as you can see the low byte is 0x00 and above mentioned condition never > becomes true, we can't continue initialization. > Ok, this sounds like a completely normal architecture implementation then, and your patch looks correct. If some other driver breaks because of this change, you should investigate what went wrong there, and treat it as a driver specific problem. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] ARC: build: Turn off -Wmaybe-uninitialized for ARC gcc 4.8
On Friday 18 March 2016 15:50:11 Vineet Gupta wrote: > Sure, but I prefer this to be only for gcc 4.8 as this warning seems to be > healthy in small doses At least it keeps the door open for future discussion > with gcc guys ! FWIW, testing on ARM with gcc-6.0 -O3, I also get tons of maybe-uninitialized warnings. It's unlikely that this is architecture specific or fixed in newer compiler versions. > The following nested construct actually works - does that look OK to you ? > > ARCH_CFLAGS += -O3 $(call cc-ifversion, -lt, 0408, $(call > cc-disable-warning,maybe-uninitialized,)) Yes, that seems ok. I don't really understand why -O3 is needed though, maybe it's better to assume that it won't be needed in future gcc versions and do ARCH_CFLAGS += $(call cc-ifversion, -lt, 0408, -O3 $(call cc-disable-warning,maybe-uninitialized,)) Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] ARC: build: Turn off -Wmaybe-uninitialized for ARC gcc 4.8
On Friday 18 March 2016 14:16:23 Vineet Gupta wrote: > diff --git a/arch/arc/Makefile b/arch/arc/Makefile > index fed12f39d8ce..aeb101e8e674 100644 > --- a/arch/arc/Makefile > +++ b/arch/arc/Makefile > @@ -48,9 +48,14 @@ endif > upto_gcc44:= $(call cc-ifversion, -le, 0404, y) > atleast_gcc44 := $(call cc-ifversion, -ge, 0404, y) > atleast_gcc48 := $(call cc-ifversion, -ge, 0408, y) > +is_gcc48 := $(call cc-ifversion, -eq, 0408, y) > > cflags-$(atleast_gcc44)+= -fsection-anchors > > +# gcc 4.8 spits out false positives for default -O3 > +# disable these for 4.8 and revisit when we upgrade to newer ver > +cflags-$(is_gcc48) += $(call > cc-disable-warning,maybe-uninitialized,) > + > cflags-$(CONFIG_ARC_HAS_LLSC) += -mlock > cflags-$(CONFIG_ARC_HAS_SWAPE) += -mswape Is this any better with gcc-4.9 or gcc-5? Maybe it's better to add the flag to the line that adds -O3 for consistency. We do the same thing for -Os in the global Makefile, as that triggers a similar load of warnings. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] ARC: build: Turn off -Wmaybe-uninitialized for ARC gcc 4.8
On Friday 18 March 2016 16:13:28 Vineet Gupta wrote: > On Friday 18 March 2016 03:59 PM, Arnd Bergmann wrote: > > On Friday 18 March 2016 15:50:11 Vineet Gupta wrote: > >> Sure, but I prefer this to be only for gcc 4.8 as this warning seems to be > >> healthy in small doses At least it keeps the door open for future > >> discussion > >> with gcc guys ! > > > > FWIW, testing on ARM with gcc-6.0 -O3, I also get tons of > > maybe-uninitialized > > warnings. It's unlikely that this is architecture specific or fixed in newer > > compiler versions. > > So we disable this for good just like -Os. > What a shame - seemed like a reasonable safety net for programming errors. Yes, it's an immensely useful warning, if you manage to avoid the false positives. > >> The following nested construct actually works - does that look OK to you ? > >> > >> ARCH_CFLAGS += -O3 $(call cc-ifversion, -lt, 0408, $(call > >> cc-disable-warning,maybe-uninitialized,)) > > > > Yes, that seems ok. > > There was typo actually -lt needed to be -eq > > > I don't really understand why -O3 is needed though, maybe it's better to > > assume that it won't be needed in future gcc versions and do > > Not sure what you mean, -O3 for triggering the warnings or -O3 in ARC > makefile at all. > Assuming it's latter, this is how its been forever and was added consciously > as > performance seemed better with -O3 than the default -O2. I think it's dangerous to use -O3 in one architecture when nothing else uses it. If you don't have a strong reason to use -O3, maybe just drop that use the default -O2 -Wmaybe-uninitialized like everyone else does. On a related note, I have submitted a patch that turns CONFIG_CC_OPTIMIZE_FOR_SIZE into a choice statement, so we actually get the -Wmaybe-uninitialized warnings in an allyesconfig or allmodconfig build. It would be trivial to extend that to give the choice between -Os, -O2 and -O3, and then pick -O3 in a defconfig, over the -O2 default. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] ARC: build: Turn off -Wmaybe-uninitialized for ARC gcc 4.8
On Friday 18 March 2016 18:31:53 Vineet Gupta wrote: > > > On a related note, I have submitted a patch that turns > > CONFIG_CC_OPTIMIZE_FOR_SIZE > > into a choice statement, so we actually get the -Wmaybe-uninitialized > > warnings > > in an allyesconfig or allmodconfig build. It would be trivial to extend that > > to give the choice between -Os, -O2 and -O3, and then pick -O3 in a > > defconfig, > > over the -O2 default. > > Is it posted already. I couldn't find it with quick googling. > > https://lkml.org/lkml/2016/2/12/315 Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2] asm-generic: Drop renameat syscall from default list
On Monday 02 May 2016 05:13:46 Vineet Gupta wrote: > On Saturday 30 April 2016 02:59 AM, James Hogan wrote: > > The newer renameat2 syscall provides all the functionality provided by > > the renameat syscall and adds flags, so future architectures won't need > > to include renameat. > > > > Therefore drop the renameat syscall from the generic syscall list unless > > __ARCH_WANT_RENAMEAT is defined by the architecture's unistd.h prior to > > including asm-generic/unistd.h, and adjust all architectures using the > > generic syscall list to define it so that no in-tree architectures are > > affected. > > > > Signed-off-by: James Hogan > > Acked-by: Vineet Gupta # for arch/arc Applied to the asm-generic, tree, thanks! Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 1/7] asm-generic/io.h: add io{read,write}64 accessors
On Thursday 05 May 2016 08:16:47 Vineet Gupta wrote:
> Thx for noticing this Arnd and the heads up. Does the patch below look ok to
> you ?
>
> --->
> rom b7e719831c389ab4fa338b2e2e7c0d1ff90dabb0 Mon Sep 17 00:00:00 2001
> From: Vineet Gupta
> Date: Thu, 5 May 2016 13:32:34 +0530
> Subject: [PATCH] ARC: Add missing io barriers to io{read,write}{16,32}be()
>
> While reviewing a different change to asm-generic/io.h Arnd spotted that
> ARC ioread32 and ioread32be both of which come from asm-generic versions
> are not symmetrical in terms of calling the io barriers.
>
> generic ioread32 -> ARC readl() [ has barriers]
> generic ioread32be -> __be32_to_cpu(__raw_readl()) [ lacks barriers]
>
> While generic ioread32be is being remediated to call readl(), that involves
> a swab32(), causing double swaps on ioread32be() on Big Endian systems.
>
> So provide our versions of big endian IO accessors to ensure io barrier
> calls while also keeping them optimal
>
> Suggested-by: Arnd Bergmann
> Cc: sta...@vger.kernel.org [4.2+]
> Signed-off-by: Vineet Gupta
Yes, that looks correct. We probably want this regardless of the change
I proposed for the generic file, to avoid the double swap.
Acked-by: Arnd Bergmann
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH] irqchip: nps: add 64BIT dependency
The newly added nps irqchip driver causes build warnings on ARM64. include/soc/nps/common.h: In function 'nps_host_reg_non_cl': include/soc/nps/common.h:148:9: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] As the driver is only used on ARC, we don't need to see it without COMPILE_TEST elsewhere, and we can avoid the warnings by only building on 32-bit architectures even with CONFIG_COMPILE_TEST. Signed-off-by: Arnd Bergmann --- drivers/irqchip/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index 83775f148158..37289cf6b449 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -253,6 +253,7 @@ config MVEBU_ODMI config EZNPS_GIC bool "NPS400 Global Interrupt Manager (GIM)" + depends on ARC || (COMPILE_TEST && !64BIT) select IRQ_DOMAIN help Support the EZchip NPS400 global interrupt controller -- 2.7.0 ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] irqchip: nps: add 64BIT dependency
On Friday 13 May 2016 14:05:41 Vineet Gupta wrote: > On Friday 13 May 2016 01:54 PM, Marc Zyngier wrote: > > On 12/05/16 22:03, Arnd Bergmann wrote: > ... > >> > >> config EZNPS_GIC > >> bool "NPS400 Global Interrupt Manager (GIM)" > >> +depends on ARC || (COMPILE_TEST && !64BIT) > >> select IRQ_DOMAIN > >> help > >>Support the EZchip NPS400 global interrupt controller > >> > > > > Acked-by: Marc Zyngier > > > > As I've already started collecting fixes that are aimed at -rc1 (mostly > > to avoid dependencies), I can queue that as well. > > There is a slight logistics issue here - as agreed the driver will go in > 4.7-rc1 > via ARC tree. So either I pick the fix for rc1 or you apply it post rc1 - or > towards the end of rc1 ? > I'd say the best option is to have you pick up the fix for the ARC tree, but either way works. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: Build regressions/improvements in v4.9-rc1
On Monday, October 17, 2016 9:59:24 AM CEST Vineet Gupta wrote: > +CC Arnd, Michal > > Hi Geert, Arnd > > Need some guidance here. > > On 10/17/2016 12:34 AM, Geert Uytterhoeven wrote: > >> 48 error regressions: > >> > + /home/kisskb/slave/src/arch/arc/include/asm/atomic.h: Error: bad > >> > instruction `llockd r2,[r0]': => 476 > >> > + /home/kisskb/slave/src/arch/arc/include/asm/atomic.h: Error: bad > >> > instruction `llockd r2,[r13]': => 475 > > [snip...] > > >> > + /home/kisskb/slave/src/arch/arc/include/asm/atomic.h: Error: bad > >> > instruction `scondd r4,[r8]': => 516 > >> > + /home/kisskb/slave/src/arch/arc/include/asm/atomic.h: Error: bad > >> > instruction `scondd r6,[r3]': => 478 > > arcv2/axs103_smp_defconfig > > > I'm thinking how to address this correctly. > > This is due to the older version of compiler. The fix itself is trivial - > add an > "call as-instr" construct in Makefile to get -DARC_TOOLS_SUPPORT_LLOCKD > > However the atomic64 API variant (CONFIG_GENERIC_ATOMIC64 or arch native) > which > gets included in build comes from Kconfig (ISA supports them or not). How do > we > tie the Makefile info into the Kconfig. > > We could trigger a build failure for invalid combinations of GENERIC_ATOMIC64 > and > ARC_TOOLS_SUPPORT_LLOCKD but that would be less than ideal out of box > experience. > > Or the simpler solution is that kisskb upgrades the ARC GNU compiler Some ideas, none of which are perfect: - add an #ifndef ARC_TOOLS_SUPPORT_LLOCKD clause in asm/atomic.h that uses .long with hardcoded opcodes in place of the mnemonics. - instead of setting CONFIG_GENERIC_ATOMIC64 from Kconfig, add a file in arch/arc/kernel/ that includes lib/atomic64.c if ARC_TOOLS_SUPPORT_LLOCKD is not set. - add "-DCONFIG_GENERIC_ATOMIC64" to cflags-y from arch/arc/Makefile if old binutils are found. I think someone was suggesting in the past that Kconfig could be extended to make decisions based on the gcc version, and the same thing could be done for binutils. Don't remember who that was though. I think a number of awkward hacks in the kernel could be simplified if we had this. And ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH 28/28] Kbuild: bring back -Wmaybe-uninitialized warning
Traditionally, we have always had warnings about uninitialized variables
enabled, as this is part of -Wall, and generally a good idea [1], but it
also always produced false positives, mainly because this is a variation
of the halting problem and provably impossible to get right in all cases
[2].
Various people have identified cases that are particularly bad for false
positives, and in commit e74fc973b6e5 ("Turn off -Wmaybe-uninitialized
when building with -Os"), I turned off the warning for any build that
was done with CC_OPTIMIZE_FOR_SIZE. This drastically reduced the number
of false positive warnings in the default build but unfortunately had
the side effect of turning the warning off completely in 'allmodconfig'
builds, which in turn led to a lot of warnings (both actual bugs, and
remaining false positives) to go in unnoticed.
With commit 877417e6ffb9 ("Kbuild: change CC_OPTIMIZE_FOR_SIZE
definition") enabled the warning again for allmodconfig builds in v4.7
and in v4.8-rc1, I had finally managed to address all warnings I get in
an ARM allmodconfig build and most other maybe-uninitialized warnings
for ARM randconfig builds.
However, commit 6e8d666e9253 ("Disable "maybe-uninitialized" warning
globally") was merged at the same time and disabled it completely for
all configurations, because of false-positive warnings on x86 that
I had not addressed until then. This caused a lot of actual bugs to
get merged into mainline, and I sent several dozen patches for these
during the v4.9 development cycle. Most of these are actual bugs,
some are for correct code that is safe because it is only called
under external constraints that make it impossible to run into
the case that gcc sees, and in a few cases gcc is just stupid and
finds something that can obviously never happen.
I have now done a few thousand randconfig builds on x86 and collected
all patches that I needed to address every single warning I got
(I can provide the combined patch for the other warnings if anyone
is interested), so I hope we can get the warning back and let people
catch the actual bugs earlier.
Note that the majority of the patches I created are for the third kind
of problem (stupid false-positives), for one of two reasons:
- some of them only get triggered in certain combinations of config
options, so we don't always run into them, and
- the actual bugs tend to get addressed much quicker as they also
lead to incorrect runtime behavior.
These 27 patches address the warnings that either occur in one of the more
common configurations (defconfig, allmodconfig, or something built by the
kbuild robot or kernelci.org), or they are about a real bug. It would be
good to get these all into v4.9 if we want to turn on the warning again.
I have tested these extensively with gcc-4.9 and gcc-6 and done a bit
of testing with gcc-5, and all of these should now be fine. gcc-4.8
is much worse about the false-positive warnings and is also fairly old
now, so I'm leaving the warning disabled with that version. gcc-4.7 and
older don't understand the -Wno-maybe-uninitialized option and are not
affected by this patch either way.
I have another (smaller) series of patches for warnings that are both
harmless and not as easy to trigger, and I will send them for inclusion
in v4.10.
Link: https://rusty.ozlabs.org/?p=232 [1]
Link: https://gcc.gnu.org/wiki/Better_Uninitialized_Warnings [2]
Signed-off-by: Arnd Bergmann
---
Makefile | 10 ++
arch/arc/Makefile | 4 +++-
scripts/Makefile.ubsan | 4
3 files changed, 13 insertions(+), 5 deletions(-)
Cc: x...@kernel.org
Cc: linux-me...@vger.kernel.org
Cc: Mauro Carvalho Chehab
Cc: Martin Schwidefsky
Cc: linux-s...@vger.kernel.org
Cc: Ilya Dryomov
Cc: dri-de...@lists.freedesktop.org
Cc: linux-...@lists.infradead.org
Cc: Herbert Xu
Cc: linux-cry...@vger.kernel.org
Cc: "David S. Miller"
Cc: net...@vger.kernel.org
Cc: Greg Kroah-Hartman
Cc: ceph-de...@vger.kernel.org
Cc: linux-f2fs-de...@lists.sourceforge.net
Cc: linux-e...@vger.kernel.org
Cc: netfilter-de...@vger.kernel.org
diff --git a/Makefile b/Makefile
index 512e47a..43cd3d9 100644
--- a/Makefile
+++ b/Makefile
@@ -370,7 +370,7 @@ LDFLAGS_MODULE =
CFLAGS_KERNEL =
AFLAGS_KERNEL =
LDFLAGS_vmlinux =
-CFLAGS_GCOV= -fprofile-arcs -ftest-coverage -fno-tree-loop-im
+CFLAGS_GCOV= -fprofile-arcs -ftest-coverage -fno-tree-loop-im
-Wno-maybe-uninitialized
CFLAGS_KCOV:= $(call cc-option,-fsanitize-coverage=trace-pc,)
@@ -620,7 +620,6 @@ ARCH_CFLAGS :=
include arch/$(SRCARCH)/Makefile
KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks,)
-KBUILD_CFLAGS += $(call cc-disable-warning,maybe-uninitialized,)
KBUILD_CFLAGS += $(call cc-disable-warning,frame-address,)
ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
@@ -629,15 +628,18 @@ KBUILD_CFLAGS += $(call cc-option,-fdata-sections,)
endif
ifdef CONFIG_C
Re: Build regressions/improvements in v4.9-rc1
On Thursday, October 27, 2016 11:11:18 AM CEST Thomas Petazzoni wrote: > On Thu, 27 Oct 2016 09:07:55 +, Alexey Brodkin wrote: > > > > axs101 is using a 770 core, while the toolchain is built for the HS38 > > > core. I'm somewhat surprised that a single ARC toolchain cannot produce > > > code for both 770 and HS38, but it seems to be the case. > > > > > > So you need a separate toolchain for ARC770. > > > > Indeed axs101 uses ARC770 core which is ARCv1 AKA ARCompact ISA while > > axs103 sports the same base-board but CPU daughter-card contains ARC HS38 > > core > > which has ARCv2 ISA (binary incompatible with ARCompact). > > > > Essentially both gcc and binutils will happily build for both architectures > > given > > proper options were passed on the command line. But Linux kernel gets > > linked with > > pre-built libgcc (it is a part of toolchain). And so it all boils down to a > > requirement > > to have multilibbed uClibc toolchain. Which we don't have. > > Interesting. Why is libgcc linked with the kernel on ARC? I don't think > that's the case on other architectures: the kernel is freestanding and > provides everything that it needs without relying on the compiler > runtime. A couple of other architectures do this as well: $ git grep -w LIBGCC arch/*/Makefile arch/arc/Makefile:LIBGCC:= $(shell $(CC) $(cflags-y) --print-libgcc-file-name) arch/arc/Makefile:libs-y+= arch/arc/lib/ $(LIBGCC) arch/cris/Makefile:LIBGCC = $(shell $(CC) $(KBUILD_CFLAGS) -print-file-name=libgcc.a) arch/cris/Makefile:libs-y += arch/cris/$(SARCH)/lib/ $(LIBGCC) arch/hexagon/Makefile:LIBGCC := $(shell $(CC) $(KBUILD_CFLAGS) -print-libgcc-file-name) arch/hexagon/Makefile:libs-y += $(LIBGCC) arch/m32r/Makefile:LIBGCC := $(shell $(CC) $(KBUILD_CFLAGS) -print-libgcc-file-name) arch/m32r/Makefile:libs-y += arch/m32r/lib/ $(LIBGCC) arch/nios2/Makefile:LIBGCC := $(shell $(CC) $(KBUILD_CFLAGS) $(KCFLAGS) -print-libgcc-file-name) arch/nios2/Makefile:libs-y += arch/nios2/lib/ $(LIBGCC) arch/openrisc/Makefile:LIBGCC := $(shell $(CC) $(KBUILD_CFLAGS) -print-libgcc-file-name) arch/openrisc/Makefile:libs-y += $(LIBGCC) arch/parisc/Makefile:LIBGCC = $(shell $(CC) $(KBUILD_CFLAGS) -print-libgcc-file-name) arch/parisc/Makefile:libs-y += arch/parisc/lib/ $(LIBGCC) arch/xtensa/Makefile:LIBGCC := $(shell $(CC) $(KBUILD_CFLAGS) -print-libgcc-file-name) arch/xtensa/Makefile:libs-y += arch/xtensa/lib/ $(LIBGCC) It's also not always freestanding on the architectures that don't include libgcc: $ git grep ffreestanding arch/ arch/mips/Makefile:cflags-y += -ffreestanding arch/s390/boot/compressed/Makefile:KBUILD_CFLAGS += $(call cc-option,-ffreestanding) arch/score/Makefile:-D__linux__ -ffunction-sections -ffreestanding arch/sh/Makefile:cflags-y += $(isaflags-y) -ffreestanding arch/x86/Makefile:KBUILD_CFLAGS += -ffreestanding # temporary until string.h is fixed arch/xtensa/Makefile:KBUILD_CFLAGS += -ffreestanding -D__linux__ (xtensa being the only one that apparently uses libgcc *and* passes -ffreestanding, for whatever reasons). The other architectures tend to implement the parts of libgcc that they need in the kernel. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: Build regressions/improvements in v4.9-rc1
On Thursday, October 27, 2016 10:21:16 AM CEST Vineet Gupta wrote: > > On 10/27/2016 02:39 AM, Alexey Brodkin wrote: > > > > And these are functions required by U-Boot (most probably the same is > > applied to kernel): > > 1) so-called millicode, stuff like __ld_rX_to_rY, __st_rX_to_rX > > This kicks in only at -Os and even there can be inhibited with a toggle. > I don't like it anyways, seems like a costly contraption in terms of microarch > cost of 2 extra long branches for both prologue and epilogue. > > > 2) shifts: __ashldi3, __ashrdi3, __lshrdi3, > > 3) divisions: udivmodsi4, __divsi3, __modsi3, __udivsi3, __umodsi3 > > Note that this list is not constant. I recently had to export another libgcc > symbol for modules, when a customer switched to ARC gnu 2016.03 for supposedly > building the same kernel code. > > > Indeed it is possible to have so-called private libgcc in kernel as well but > > benefit will be only for people building kernels but not user-space because > > in absence of multilibbed toolchain 2 separate toolchains will be required > > anyways. > > True, but a lot of people only care about builds (and not actually run), so > for > them having to carry only one toolchain is an improvement. > > > Still we'll have to pay an additional maintenance price to keep kernel's > > libgcc in > > sync with the one from gcc. > > True, but libgcc math emulation is likely one off thing. GNU folks will write > them > once and we use a snapshot - syncing back changes - if any around major gnu > releases. > > So I'm tending to include the libgcc code in kernel. @Arnd, @Claudiu do you > know > of any potential licensing issues ? > I'd be surprised if there were any licensing issues, as libgcc is intentionally meant to be included in everything built by gcc, and the architectures that don't link against it tend to have a direct copy. The main advantage of copying libgcc sources into the kernel instead of linking directly to it is probably that you have better control over which functions are actually used, as not everything in libgcc makes sense in kernel space. The most common example is probably the 64-bit division, which is a libgcc function on most architectures, and in the kernel we intentionally don't implement that function in order to catch drivers trying to do that (and change them to either explicit div_u64() or not do a 64-bit division). Another example of a libgcc function you don't want is anything calling abort(), which makes no sense in the kernel. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] lpfc: use %zd format string for size_t
On Friday, October 28, 2016 2:03:21 PM CEST Vineet Gupta wrote: > > I'm trying to use about to be released ARC gcc 6.x with current kernels and > see a > flood of warnings due to these legit fixes - i.e.g arc gcc 6.2 complains when > it > sees -zx formats. > > CC mm/percpu.o > ../mm/percpu.c: In function ‘pcpu_alloc’: > ../mm/percpu.c:890:14: warning: format ‘%zu’ expects argument of type > ‘size_t’, > but argument 4 has type ‘unsigned int’ [-Wformat=] >WARN(true, "illegal size (%zu) or align (%zu) for percpu allocation\n", > > I'm not sure what is going on since the data type is size_t alright - although > from posix_types.h is > > typedef unsigned int __kernel_size_t; > typedef __kernel_size_t size_t; > > And this seems to be same for ARC as well as ARM. I tried ARM gcc 6.1 @ > https://snapshots.linaro.org/components/toolchain/binaries/6.1-2016.08-rc1/arm-linux-gnueabihf/ > > which doesn't seem to be complaining. > > With V=1, I checked the respective ARM and ARC toggles in play, but nothing > related to this seems to be standing out. > > I know this is more of a question to our GNU folks, but was wondering if you > had > more insight into it - which you almost always do I've seen the problem you describe before, but I don't remember the exact details. I think what happened is that the compiler knows what type size_t is supposed to be, either unsigned int or unsigned long, regardless of what our kernel headers say it is. This is configuration specific, and something caused your compiler to be built assuming that size_t is unsigned long, while the kernel headers are assuming it should be unsigned int. You can try overriding __kernel_size_t in your asm/posix_types.h to define it as unsigned long, or try to build your compiler to match the kernel headers, but the first step would be to find out why the compiler changed in the first place, assuming that older compiler versions were matching the kernel here. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] lpfc: use %zd format string for size_t
On Friday, October 28, 2016 2:44:13 PM CEST Vineet Gupta wrote: > > Indeed if I hack include/linux/types.h > > -typedef __kernel_size_tsize_t; > +typedef unsigned long size_t; > > then the warning goes away, so gcc is indeed assuming size_t to be unsigned > long > and not unsigned int. That helps a lot. Ok, just be aware that this will introduce warnings for any compiler that is built to expect an 'unsigned int size_t' typedef. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] lpfc: use %zd format string for size_t
On Friday, October 28, 2016 2:58:33 PM CEST Vineet Gupta wrote: > On 10/28/2016 02:52 PM, Vineet Gupta wrote: > > On 10/28/2016 02:44 PM, Vineet Gupta wrote: > >> This is configuration specific, and something caused your compiler to > >>> be built assuming that size_t is unsigned long, while the kernel > >>> headers are assuming it should be unsigned int. > > > > So yes this seems to be target specific gcc thing > > > > for ARC 4.8 > > > > #define PTRDIFF_TYPE "int" > > > > ARM > > > > #ifndef PTRDIFF_TYPE > > #define PTRDIFF_TYPE (TARGET_AAPCS_BASED ? "int" : "long int") > > #endif > > > > ARC gcc 6.2 > > > > #undef PTRDIFF_TYPE > > #define PTRDIFF_TYPE "long int" > > Actually we need to adjust SIZE_TYPE (unsigned int) and PTRDIFF_TYPE (int) in > the > gcc 6.x to fix this issue. And that is exactly what ARC gcc 4.8 have. What compiler versions are most commonly used these days? You should probably stay with the version that most people have and then update either the compiler or the kernel, whichever diverges from it. I see in the gcc git log that the version that had "int" got removed at some point, and the version that had "unsigned int" was added later. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] asm-generic: Drop getrlimit and setrlimit syscalls from default list
On Saturday, October 22, 2016 3:14:04 PM CEST Yury Norov wrote: > The newer prlimit64 syscall provides all the functionality provided by > the getrlimit and setrlimit syscalls and adds the pid of target process, > so future architectures won't need to include getrlimit and setrlimit. > > Therefore drop getrlimit and setrlimit syscalls from the generic syscall > list unless __ARCH_WANT_SET_GET_RLIMIT is defined by the architecture's > unistd.h prior to including asm-generic/unistd.h, and adjust all > architectures using the generic syscall list to define it so that no > in-tree architectures are affected. The patch looks good, but shouldn't we also hide the actual syscall implementation if the symbol is not set? It's just dead code otherwise for new architectures. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] asm-generic: Drop getrlimit and setrlimit syscalls from default list
On Sunday, October 30, 2016 12:45:41 AM CEST Yury Norov wrote: > On Sat, Oct 29, 2016 at 11:02:40PM +0200, Arnd Bergmann wrote: > > On Saturday, October 22, 2016 3:14:04 PM CEST Yury Norov wrote: > > > The newer prlimit64 syscall provides all the functionality provided by > > > the getrlimit and setrlimit syscalls and adds the pid of target process, > > > so future architectures won't need to include getrlimit and setrlimit. > > > > > > Therefore drop getrlimit and setrlimit syscalls from the generic syscall > > > list unless __ARCH_WANT_SET_GET_RLIMIT is defined by the architecture's > > > unistd.h prior to including asm-generic/unistd.h, and adjust all > > > architectures using the generic syscall list to define it so that no > > > in-tree architectures are affected. > > > > The patch looks good, but shouldn't we also hide the actual syscall > > implementation if the symbol is not set? It's just dead code otherwise > > for new architectures. > > I was thinking on it. The patch of James Hogan, b0da6d4415 (asm-generic: > Drop renameat syscall from default list) doesn't do it for renameat(), so > I decided not to do it too. It's not so easy to disable syscalls because arch > may support few ABIs, and some of them may require the syscall. For example, > arm64 supports lp64, aarch32 and ilp32, and first two ABIs need renameat() > and getrlimit/setrlimit. > > At now there's no arches that doesn't need renameat() and getrlimit/setrlimit, > and there will be no such arch in nearest future. So there will be no > dead code. > > But I agree with you that we need make that implementations > conditional. If I understand it correctly, we need something like > __ARCH_WANT_SET_GET_RLIMIT in all existing Kconfigs, correct? > > I think this patch may be applied as is, and if needed I can send > another patch that disables renameat() and getrlimit/setrlimit soon. Fair enough. Actually now that I think about it, there are probably lots of other syscalls that are unused on modern architectures. It would be good to go through the full list and hide all the ones that are not referenced, but that is clearly independent of your patch. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] asm-generic: Drop getrlimit and setrlimit syscalls from default list
On Saturday, October 22, 2016 3:14:04 PM CEST Yury Norov wrote: > The newer prlimit64 syscall provides all the functionality provided by > the getrlimit and setrlimit syscalls and adds the pid of target process, > so future architectures won't need to include getrlimit and setrlimit. > > Therefore drop getrlimit and setrlimit syscalls from the generic syscall > list unless __ARCH_WANT_SET_GET_RLIMIT is defined by the architecture's > unistd.h prior to including asm-generic/unistd.h, and adjust all > architectures using the generic syscall list to define it so that no > in-tree architectures are affected. > Acked-by: Arnd Bergmann Can you include this patch in your ilp32 series? ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH v2 10/11] pcmcia: fix return value of soc_pcmcia_regulator_set
The newly introduced soc_pcmcia_regulator_set() function sometimes returns
without setting its return code, as shown by this warning:
drivers/pcmcia/soc_common.c: In function 'soc_pcmcia_regulator_set':
drivers/pcmcia/soc_common.c:112:5: error: 'ret' may be used uninitialized in
this function [-Werror=maybe-uninitialized]
This changes it to propagate the regulator_disable() result instead.
Fixes: ac61b6001a63 ("pcmcia: soc_common: add support for Vcc and Vpp
regulators")
Signed-off-by: Arnd Bergmann
---
drivers/pcmcia/soc_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pcmcia/soc_common.c b/drivers/pcmcia/soc_common.c
index 153f312..b6b316d 100644
--- a/drivers/pcmcia/soc_common.c
+++ b/drivers/pcmcia/soc_common.c
@@ -107,7 +107,7 @@ int soc_pcmcia_regulator_set(struct soc_pcmcia_socket *skt,
ret = regulator_enable(r->reg);
} else {
- regulator_disable(r->reg);
+ ret = regulator_disable(r->reg);
}
if (ret == 0)
r->on = on;
--
2.9.0
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH v2 05/11] s390: pci: don't print uninitialized data for debugging
gcc correctly warns about an incorrect use of the 'pa' variable in case we pass an empty scatterlist to __s390_dma_map_sg: arch/s390/pci/pci_dma.c: In function '__s390_dma_map_sg': arch/s390/pci/pci_dma.c:309:13: warning: 'pa' may be used uninitialized in this function [-Wmaybe-uninitialized] This adds a bogus initialization to the function to sanitize the debug output. I would have preferred a solution without the initialization, but I only got the report from the kbuild bot after turning on the warning again, and didn't manage to reproduce it myself. Signed-off-by: Arnd Bergmann Acked-by: Sebastian Ott Acked-by: Martin Schwidefsky --- arch/s390/pci/pci_dma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c index 7350c8b..6b2f72f 100644 --- a/arch/s390/pci/pci_dma.c +++ b/arch/s390/pci/pci_dma.c @@ -423,7 +423,7 @@ static int __s390_dma_map_sg(struct device *dev, struct scatterlist *sg, dma_addr_t dma_addr_base, dma_addr; int flags = ZPCI_PTE_VALID; struct scatterlist *s; - unsigned long pa; + unsigned long pa = 0; int ret; size = PAGE_ALIGN(size); -- 2.9.0 ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH v2 01/11] Kbuild: enable -Wmaybe-uninitialized warning for "make W=1"
Traditionally, we have always had warnings about uninitialized variables
enabled, as this is part of -Wall, and generally a good idea [1], but it
also always produced false positives, mainly because this is a variation
of the halting problem and provably impossible to get right in all cases
[2].
Various people have identified cases that are particularly bad for false
positives, and in commit e74fc973b6e5 ("Turn off -Wmaybe-uninitialized
when building with -Os"), I turned off the warning for any build that
was done with CC_OPTIMIZE_FOR_SIZE. This drastically reduced the number
of false positive warnings in the default build but unfortunately had
the side effect of turning the warning off completely in 'allmodconfig'
builds, which in turn led to a lot of warnings (both actual bugs, and
remaining false positives) to go in unnoticed.
With commit 877417e6ffb9 ("Kbuild: change CC_OPTIMIZE_FOR_SIZE
definition") enabled the warning again for allmodconfig builds in v4.7
and in v4.8-rc1, I had finally managed to address all warnings I get in
an ARM allmodconfig build and most other maybe-uninitialized warnings
for ARM randconfig builds.
However, commit 6e8d666e9253 ("Disable "maybe-uninitialized" warning
globally") was merged at the same time and disabled it completely for
all configurations, because of false-positive warnings on x86 that I had
not addressed until then. This caused a lot of actual bugs to get merged
into mainline, and I sent several dozen patches for these during the v4.9
development cycle. Most of these are actual bugs, some are for correct
code that is safe because it is only called under external constraints
that make it impossible to run into the case that gcc sees, and in a
few cases gcc is just stupid and finds something that can obviously
never happen.
I have now done a few thousand randconfig builds on x86 and collected all
patches that I needed to address every single warning I got (I can provide
the combined patch for the other warnings if anyone is interested),
so I hope we can get the warning back and let people catch the actual
bugs earlier.
This reverts the change to disable the warning completely and for
now brings it back at the "make W=1" level, so we can get it merged
into mainline without introducing false positives. A follow-up
patch enables it on all levels unless some configuration option
turns it off because of false-positives.
Link: https://rusty.ozlabs.org/?p=232 [1]
Link: https://gcc.gnu.org/wiki/Better_Uninitialized_Warnings [2]
Signed-off-by: Arnd Bergmann
---
Makefile | 10 ++
arch/arc/Makefile | 4 +++-
scripts/Makefile.extrawarn | 3 +++
scripts/Makefile.ubsan | 4
4 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/Makefile b/Makefile
index f97f786..06e2b73 100644
--- a/Makefile
+++ b/Makefile
@@ -370,7 +370,7 @@ LDFLAGS_MODULE =
CFLAGS_KERNEL =
AFLAGS_KERNEL =
LDFLAGS_vmlinux =
-CFLAGS_GCOV= -fprofile-arcs -ftest-coverage -fno-tree-loop-im
+CFLAGS_GCOV= -fprofile-arcs -ftest-coverage -fno-tree-loop-im
-Wno-maybe-uninitialized
CFLAGS_KCOV:= $(call cc-option,-fsanitize-coverage=trace-pc,)
@@ -620,7 +620,6 @@ ARCH_CFLAGS :=
include arch/$(SRCARCH)/Makefile
KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks,)
-KBUILD_CFLAGS += $(call cc-disable-warning,maybe-uninitialized,)
KBUILD_CFLAGS += $(call cc-disable-warning,frame-address,)
ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
@@ -629,15 +628,18 @@ KBUILD_CFLAGS += $(call cc-option,-fdata-sections,)
endif
ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
-KBUILD_CFLAGS += -Os
+KBUILD_CFLAGS += -Os $(call cc-disable-warning,maybe-uninitialized,)
else
ifdef CONFIG_PROFILE_ALL_BRANCHES
-KBUILD_CFLAGS += -O2
+KBUILD_CFLAGS += -O2 $(call cc-disable-warning,maybe-uninitialized,)
else
KBUILD_CFLAGS += -O2
endif
endif
+KBUILD_CFLAGS += $(call cc-ifversion, -lt, 0409, \
+ $(call cc-disable-warning,maybe-uninitialized,))
+
# Tell gcc to never replace conditional load with a non-conditional one
KBUILD_CFLAGS += $(call cc-option,--param=allow-store-data-races=0)
diff --git a/arch/arc/Makefile b/arch/arc/Makefile
index 864adad..25f81a1 100644
--- a/arch/arc/Makefile
+++ b/arch/arc/Makefile
@@ -68,7 +68,9 @@ cflags-$(CONFIG_ARC_DW2_UNWIND) +=
-fasynchronous-unwind-tables $(cfi)
ifndef CONFIG_CC_OPTIMIZE_FOR_SIZE
# Generic build system uses -O2, we want -O3
# Note: No need to add to cflags-y as that happens anyways
-ARCH_CFLAGS += -O3
+#
+# Disable the false maybe-uninitialized warings gcc spits out at -O3
+ARCH_CFLAGS += -O3 $(call cc-disable-warning,maybe-uninitialized,)
endif
# small data is default for elf32 tool-chain. If not usable, disable it
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 53449a6..7fc2c5a 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.
[PATCH v2 06/11] [media] dib0700: fix nec repeat handling
From: Sean Young
When receiving a nec repeat, ensure the correct scancode is repeated
rather than a random value from the stack. This removes the need
for the bogus uninitialized_var() and also fixes the warnings:
drivers/media/usb/dvb-usb/dib0700_core.c: In function
‘dib0700_rc_urb_completion’:
drivers/media/usb/dvb-usb/dib0700_core.c:679: warning: ‘protocol’ may be
used uninitialized in this function
[sean addon: So after writing the patch and submitting it, I've bought the
hardware on ebay. Without this patch you get random scancodes
on nec repeats, which the patch indeed fixes.]
Signed-off-by: Sean Young
Tested-by: Sean Young
Cc: sta...@vger.kernel.org
Signed-off-by: Arnd Bergmann
---
drivers/media/usb/dvb-usb/dib0700_core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/usb/dvb-usb/dib0700_core.c
b/drivers/media/usb/dvb-usb/dib0700_core.c
index 92d5408..47ce9d5 100644
--- a/drivers/media/usb/dvb-usb/dib0700_core.c
+++ b/drivers/media/usb/dvb-usb/dib0700_core.c
@@ -704,7 +704,7 @@ static void dib0700_rc_urb_completion(struct urb *purb)
struct dvb_usb_device *d = purb->context;
struct dib0700_rc_response *poll_reply;
enum rc_type protocol;
- u32 uninitialized_var(keycode);
+ u32 keycode;
u8 toggle;
deb_info("%s()\n", __func__);
@@ -745,7 +745,8 @@ static void dib0700_rc_urb_completion(struct urb *purb)
poll_reply->nec.data == 0x00 &&
poll_reply->nec.not_data == 0xff) {
poll_reply->data_state = 2;
- break;
+ rc_repeat(d->rc_dev);
+ goto resubmit;
}
if ((poll_reply->nec.data ^ poll_reply->nec.not_data) != 0xff) {
--
2.9.0
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH v2 07/11] [media] rc: print correct variable for z8f0811
A recent rework accidentally left a debugging printk untouched
while changing the meaning of the variables, leading to an
uninitialized variable being printed:
drivers/media/i2c/ir-kbd-i2c.c: In function 'get_key_haup_common':
drivers/media/i2c/ir-kbd-i2c.c:62:2: error: 'toggle' may be used uninitialized
in this function [-Werror=maybe-uninitialized]
This prints the correct one instead, as we did before the patch.
Fixes: 00bb820755ed ("[media] rc: Hauppauge z8f0811 can decode RC6")
Signed-off-by: Arnd Bergmann
---
drivers/media/i2c/ir-kbd-i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
I submitted this repeatedly as it is a v4.9 regression, but
I never saw a reply for it.
diff --git a/drivers/media/i2c/ir-kbd-i2c.c b/drivers/media/i2c/ir-kbd-i2c.c
index f95a6bc..cede397 100644
--- a/drivers/media/i2c/ir-kbd-i2c.c
+++ b/drivers/media/i2c/ir-kbd-i2c.c
@@ -118,7 +118,7 @@ static int get_key_haup_common(struct IR_i2c *ir, enum
rc_type *protocol,
*protocol = RC_TYPE_RC6_MCE;
dev &= 0x7f;
dprintk(1, "ir hauppauge (rc6-mce): t%d vendor=%d
dev=%d code=%d\n",
- toggle, vendor, dev, code);
+ *ptoggle, vendor, dev, code);
} else {
*ptoggle = 0;
*protocol = RC_TYPE_RC6_6A_32;
--
2.9.0
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH v2 00/11] getting back -Wmaybe-uninitialized
Hi Linus, It took a while for some patches to make it into mainline through maintainer trees, but the 28-patch series is now reduced to 10, with one tiny patch added at the end. I hope this can still make it into v4.9. Aside from patches that are no longer required, I did these changes compared to version 1: - Dropped "iio: maxim_thermocouple: detect invalid storage size in read()", which is currently in linux-next as commit 32cb7d27e65d. This is the only remaining warning I see for a couple of corner cases (kbuild bot reports it on blackfin, kernelci bot and arm-soc bot both report it on arm64) - Dropped "brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap", which is currently in net/master merge pending. - Dropped two x86 patches, "x86: math-emu: possible uninitialized variable use" and "x86: mark target address as output in 'insb' asm" as they do not seem to trigger for a default build, and I got no feedback on them. Both of these are ancient issues and seem harmless, I will send them again to the x86 maintainers once the rest is merged. - Dropped "rbd: false-postive gcc-4.9 -Wmaybe-uninitialized" based on feedback from Ilya Dryomov, who already has a different fix queued up for v4.10. The kbuild bot reports this as a warning for xtensa. - Replaced "crypto: aesni: avoid -Wmaybe-uninitialized warning" with a simpler patch, this one always triggers but my first solution would not be safe for linux-4.9 any more at this point. I'll follow up with the larger patch as a cleanup for 4.10. - Replaced "dib0700: fix nec repeat handling" with a better one, contributed by Sean Young. Please merge these directly if you are happy with the result. As the minimum, I'd hope to see the first patch get in soon, but the individual bugfixes are hopefully now all appropriate as well. If you see any regressions with the final patch, just leave that one out and let me know what problems remain. Arnd Arnd Bergmann (10): Kbuild: enable -Wmaybe-uninitialized warning for "make W=1" NFSv4.1: work around -Wmaybe-uninitialized warning x86: apm: avoid uninitialized data nios2: fix timer initcall return value s390: pci: don't print uninitialized data for debugging [media] rc: print correct variable for z8f0811 crypto: aesni: shut up -Wmaybe-uninitialized warning infiniband: shut up a maybe-uninitialized warning pcmcia: fix return value of soc_pcmcia_regulator_set Kbuild: enable -Wmaybe-uninitialized warnings by default Sean Young (1): [media] dib0700: fix nec repeat handling Makefile | 10 +++--- arch/arc/Makefile| 4 ++- arch/nios2/kernel/time.c | 1 + arch/s390/pci/pci_dma.c | 2 +- arch/x86/crypto/aesni-intel_glue.c | 4 +-- arch/x86/kernel/apm_32.c | 5 ++- drivers/infiniband/core/cma.c| 54 +--- drivers/media/i2c/ir-kbd-i2c.c | 2 +- drivers/media/usb/dvb-usb/dib0700_core.c | 5 +-- drivers/pcmcia/soc_common.c | 2 +- fs/nfs/nfs4session.c | 10 +++--- scripts/Makefile.extrawarn | 1 + scripts/Makefile.ubsan | 4 +++ 13 files changed, 61 insertions(+), 43 deletions(-) -- 2.9.0 Cc: Anna Schumaker Cc: "David S. Miller" Cc: Herbert Xu Cc: Ilya Dryomov Cc: Javier Martinez Canillas Cc: Jiri Kosina Cc: Jonathan Cameron Cc: Ley Foon Tan Cc: Luis R. Rodriguez Cc: Martin Schwidefsky Cc: Mauro Carvalho Chehab Cc: Michal Marek Cc: Russell King Cc: Sean Young Cc: Sebastian Ott Cc: Trond Myklebust Cc: x...@kernel.org Cc: linux-kbu...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: linux-snps-arc@lists.infradead.org Cc: nios2-...@lists.rocketboards.org Cc: linux-s...@vger.kernel.org Cc: linux-cry...@vger.kernel.org Cc: linux-me...@vger.kernel.org Cc: linux-...@vger.kernel.org ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH v2 02/11] NFSv4.1: work around -Wmaybe-uninitialized warning
A bugfix introduced a harmless gcc warning in nfs4_slot_seqid_in_use
if we enable -Wmaybe-uninitialized again:
fs/nfs/nfs4session.c:203:54: error: 'cur_seq' may be used uninitialized in this
function [-Werror=maybe-uninitialized]
gcc is not smart enough to conclude that the IS_ERR/PTR_ERR pair
results in a nonzero return value here. Using PTR_ERR_OR_ZERO()
instead makes this clear to the compiler.
Fixes: e09c978aae5b ("NFSv4.1: Fix Oopsable condition in server callback races")
Signed-off-by: Arnd Bergmann
---
First submitted on Aug 31, but ended up not getting applied then
as the warning was disabled in v4.8-rc
Anna Schumaker said at the kernel summit that she had applied
it and would send it for 4.9, but as of 2016-11-09 it has not
made it into linux-next.
fs/nfs/nfs4session.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/fs/nfs/nfs4session.c b/fs/nfs/nfs4session.c
index b629730..150c5a1 100644
--- a/fs/nfs/nfs4session.c
+++ b/fs/nfs/nfs4session.c
@@ -178,12 +178,14 @@ static int nfs4_slot_get_seqid(struct nfs4_slot_table
*tbl, u32 slotid,
__must_hold(&tbl->slot_tbl_lock)
{
struct nfs4_slot *slot;
+ int ret;
slot = nfs4_lookup_slot(tbl, slotid);
- if (IS_ERR(slot))
- return PTR_ERR(slot);
- *seq_nr = slot->seq_nr;
- return 0;
+ ret = PTR_ERR_OR_ZERO(slot);
+ if (!ret)
+ *seq_nr = slot->seq_nr;
+
+ return ret;
}
/*
--
2.9.0
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH v2 11/11] Kbuild: enable -Wmaybe-uninitialized warnings by default
Previously the warnings were added back at the W=1 level and above,
this now turns them on again by default, assuming that we have addressed
all warnings and again have a clean build for v4.10.
I found a number of new warnings in linux-next already and submitted
bugfixes for those. Hopefully they are caught by the 0day builder
in the future as soon as this patch is merged.
Signed-off-by: Arnd Bergmann
---
Please check if there are any remaining warnings with this
patch before applying.
The one known issue right now is commit 32cb7d27e65d ("iio:
maxim_thermocouple: detect invalid storage size in read()"), which
is currently in linux-next but not yet in mainline.
There are a couple of warnings that I get with randconfig builds,
and I have submitted patches for all of them at some point and
will follow up on them to make sure they get addressed eventually.
---
scripts/Makefile.extrawarn | 2 --
1 file changed, 2 deletions(-)
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 7fc2c5a..7c321a6 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -60,8 +60,6 @@ endif
KBUILD_CFLAGS += $(warning)
else
-KBUILD_CFLAGS += $(call cc-disable-warning, maybe-uninitialized)
-
ifeq ($(cc-name),clang)
KBUILD_CFLAGS += $(call cc-disable-warning, initializer-overrides)
KBUILD_CFLAGS += $(call cc-disable-warning, unused-value)
--
2.9.0
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH v2 09/11] [v3] infiniband: shut up a maybe-uninitialized warning
Some configurations produce this harmless warning when built with
gcc -Wmaybe-uninitialized:
infiniband/core/cma.c: In function 'cma_get_net_dev':
infiniband/core/cma.c:1242:12: warning: 'src_addr_storage.sin_addr.s_addr' may
be used uninitialized in this function [-Wmaybe-uninitialized]
I previously reported this for the powerpc64 defconfig, but have now
reproduced the same thing for x86 as well, using gcc-5 or higher.
The code looks correct to me, and this change just rearranges it
by making sure we alway initialize the entire address structure
to make the warning disappear. My first approach added an
initialization at the time of the declaration, which Doug commented
may be too costly, so I hope this version doesn't add overhead.
Link:
http://arm-soc.lixom.net/buildlogs/mainline/v4.7-rc6/buildall.powerpc.ppc64_defconfig.log.passed
Link: https://patchwork.kernel.org/patch/9212825/
Acked-by: Haggai Eran
Signed-off-by: Arnd Bergmann
---
This is marked v2 as the rest of the series but is actually version
three of the patch as I had to do some other changes already.
v3: remove accidental leftover change of the original patch
drivers/infiniband/core/cma.c | 54 ++-
1 file changed, 28 insertions(+), 26 deletions(-)
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 36bf50e..89a6b05 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -1094,47 +1094,47 @@ static void cma_save_ib_info(struct sockaddr *src_addr,
}
}
-static void cma_save_ip4_info(struct sockaddr *src_addr,
- struct sockaddr *dst_addr,
+static void cma_save_ip4_info(struct sockaddr_in *src_addr,
+ struct sockaddr_in *dst_addr,
struct cma_hdr *hdr,
__be16 local_port)
{
- struct sockaddr_in *ip4;
-
if (src_addr) {
- ip4 = (struct sockaddr_in *)src_addr;
- ip4->sin_family = AF_INET;
- ip4->sin_addr.s_addr = hdr->dst_addr.ip4.addr;
- ip4->sin_port = local_port;
+ *src_addr = (struct sockaddr_in) {
+ .sin_family = AF_INET,
+ .sin_addr.s_addr = hdr->dst_addr.ip4.addr,
+ .sin_port = local_port,
+ };
}
if (dst_addr) {
- ip4 = (struct sockaddr_in *)dst_addr;
- ip4->sin_family = AF_INET;
- ip4->sin_addr.s_addr = hdr->src_addr.ip4.addr;
- ip4->sin_port = hdr->port;
+ *dst_addr = (struct sockaddr_in) {
+ .sin_family = AF_INET,
+ .sin_addr.s_addr = hdr->src_addr.ip4.addr,
+ .sin_port = hdr->port,
+ };
}
}
-static void cma_save_ip6_info(struct sockaddr *src_addr,
- struct sockaddr *dst_addr,
+static void cma_save_ip6_info(struct sockaddr_in6 *src_addr,
+ struct sockaddr_in6 *dst_addr,
struct cma_hdr *hdr,
__be16 local_port)
{
- struct sockaddr_in6 *ip6;
-
if (src_addr) {
- ip6 = (struct sockaddr_in6 *)src_addr;
- ip6->sin6_family = AF_INET6;
- ip6->sin6_addr = hdr->dst_addr.ip6;
- ip6->sin6_port = local_port;
+ *src_addr = (struct sockaddr_in6) {
+ .sin6_family = AF_INET6,
+ .sin6_addr = hdr->dst_addr.ip6,
+ .sin6_port = local_port,
+ };
}
if (dst_addr) {
- ip6 = (struct sockaddr_in6 *)dst_addr;
- ip6->sin6_family = AF_INET6;
- ip6->sin6_addr = hdr->src_addr.ip6;
- ip6->sin6_port = hdr->port;
+ *dst_addr = (struct sockaddr_in6) {
+ .sin6_family = AF_INET6,
+ .sin6_addr = hdr->src_addr.ip6,
+ .sin6_port = hdr->port,
+ };
}
}
@@ -1159,10 +1159,12 @@ static int cma_save_ip_info(struct sockaddr *src_addr,
switch (cma_get_ip_ver(hdr)) {
case 4:
- cma_save_ip4_info(src_addr, dst_addr, hdr, port);
+ cma_save_ip4_info((struct sockaddr_in *)src_addr,
+ (struct sockaddr_in *)dst_addr, hdr, port);
break;
case 6:
- cma_save_ip6_info(src_addr, dst_addr, hdr, port);
+ cma_save_ip6_info((struct sockaddr_in6 *)src_addr,
+ (struct sockaddr_in6 *)dst_addr, hdr, port);
break;
default:
return -EAFNOSUPPORT;
--
2.9.0
[PATCH v2 04/11] nios2: fix timer initcall return value
When called more than twice, the nios2_time_init() function return an uninitialized value, as detected by gcc -Wmaybe-uninitialized arch/nios2/kernel/time.c: warning: 'ret' may be used uninitialized in this function This makes it return '0' here, matching the comment above the function. Acked-by: Ley Foon Tan Signed-off-by: Arnd Bergmann --- arch/nios2/kernel/time.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/nios2/kernel/time.c b/arch/nios2/kernel/time.c index d9563dd..746bf5c 100644 --- a/arch/nios2/kernel/time.c +++ b/arch/nios2/kernel/time.c @@ -324,6 +324,7 @@ static int __init nios2_time_init(struct device_node *timer) ret = nios2_clocksource_init(timer); break; default: + ret = 0; break; } -- 2.9.0 ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH v2 08/11] crypto: aesni: shut up -Wmaybe-uninitialized warning
The rfc4106 encrypy/decrypt helper functions cause an annoying
false-positive warning in allmodconfig if we turn on
-Wmaybe-uninitialized warnings again:
arch/x86/crypto/aesni-intel_glue.c: In function ‘helper_rfc4106_decrypt’:
include/linux/scatterlist.h:67:31: warning: ‘dst_sg_walk.sg’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
The problem seems to be that the compiler doesn't track the state of the
'one_entry_in_sg' variable across the kernel_fpu_begin/kernel_fpu_end
section.
This takes the easy way out by adding a bogus initialization, which
should be harmless enough to get the patch into v4.9 so we can turn
on this warning again by default without producing useless output.
A follow-up patch for v4.10 rearranges the code to make the warning
go away.
Signed-off-by: Arnd Bergmann
---
arch/x86/crypto/aesni-intel_glue.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/crypto/aesni-intel_glue.c
b/arch/x86/crypto/aesni-intel_glue.c
index 0ab5ee1..aa8b067 100644
--- a/arch/x86/crypto/aesni-intel_glue.c
+++ b/arch/x86/crypto/aesni-intel_glue.c
@@ -888,7 +888,7 @@ static int helper_rfc4106_encrypt(struct aead_request *req)
unsigned long auth_tag_len = crypto_aead_authsize(tfm);
u8 iv[16] __attribute__ ((__aligned__(AESNI_ALIGN)));
struct scatter_walk src_sg_walk;
- struct scatter_walk dst_sg_walk;
+ struct scatter_walk dst_sg_walk = {};
unsigned int i;
/* Assuming we are supporting rfc4106 64-bit extended */
@@ -968,7 +968,7 @@ static int helper_rfc4106_decrypt(struct aead_request *req)
u8 iv[16] __attribute__ ((__aligned__(AESNI_ALIGN)));
u8 authTag[16];
struct scatter_walk src_sg_walk;
- struct scatter_walk dst_sg_walk;
+ struct scatter_walk dst_sg_walk = {};
unsigned int i;
if (unlikely(req->assoclen != 16 && req->assoclen != 20))
--
2.9.0
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH v2 03/11] x86: apm: avoid uninitialized data
apm_bios_call() can fail, and return a status in its argument
structure. If that status however is zero during a call from
apm_get_power_status(), we end up using data that may have
never been set, as reported by "gcc -Wmaybe-uninitialized":
arch/x86/kernel/apm_32.c: In function ‘apm’:
arch/x86/kernel/apm_32.c:1729:17: error: ‘bx’ may be used uninitialized in this
function [-Werror=maybe-uninitialized]
arch/x86/kernel/apm_32.c:1835:5: error: ‘cx’ may be used uninitialized in this
function [-Werror=maybe-uninitialized]
arch/x86/kernel/apm_32.c:1730:17: note: ‘cx’ was declared here
arch/x86/kernel/apm_32.c:1842:27: error: ‘dx’ may be used uninitialized in this
function [-Werror=maybe-uninitialized]
arch/x86/kernel/apm_32.c:1731:17: note: ‘dx’ was declared here
This changes the function to return "APM_NO_ERROR" here, which
makes the code more robust to broken BIOS versions, and avoids
the warning.
Signed-off-by: Arnd Bergmann
Reviewed-by: Jiri Kosina
Reviewed-by: Luis R. Rodriguez
---
arch/x86/kernel/apm_32.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/apm_32.c b/arch/x86/kernel/apm_32.c
index c7364bd..51287cd 100644
--- a/arch/x86/kernel/apm_32.c
+++ b/arch/x86/kernel/apm_32.c
@@ -1042,8 +1042,11 @@ static int apm_get_power_status(u_short *status, u_short
*bat, u_short *life)
if (apm_info.get_power_status_broken)
return APM_32_UNSUPPORTED;
- if (apm_bios_call(&call))
+ if (apm_bios_call(&call)) {
+ if (!call.err)
+ return APM_NO_ERROR;
return call.err;
+ }
*status = call.ebx;
*bat = call.ecx;
if (apm_info.get_power_status_swabinminutes) {
--
2.9.0
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2 00/11] getting back -Wmaybe-uninitialized
On Friday, November 11, 2016 9:13:00 AM CET Linus Torvalds wrote:
> On Thu, Nov 10, 2016 at 8:44 AM, Arnd Bergmann wrote:
> >
> > Please merge these directly if you are happy with the result.
>
> I will take this.
Thanks a lot!
> I do see two warnings, but they both seem to be valid and recent,
> though, so I have no issues with the spurious cases.
Ok, both of them should have my fixes coming your way already.
> Warning #1:
>
> sound/soc/qcom/lpass-platform.c: In function ‘lpass_platform_pcmops_open’:
> sound/soc/qcom/lpass-platform.c:83:29: warning: ‘dma_ch’ may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> drvdata->substream[dma_ch] = substream;
> ~~~^~~
>
> and 'dma_ch' usage there really is crazy and wrong. Broken by
> 022d00ee0b55 ("ASoC: lpass-platform: Fix broken pcm data usage")
Right, the patches crossed here, the bugfix patch that introduced
this came into linux-next over the kernel summit, and the fix I
sent on Tuesday made it into Mark Brown's tree on Wednesday but not
before you pulled alsa tree. It should be fixed the next time you
pull from the alsa tree, the commit is
3b89e4b77ef9 ("ASoC: lpass-platform: initialize dma channel number")
> Warning #2 is not a real bug, but it's reasonable that gcc doesn't
> know that storage_bytes (chip->read_size) has to be 2/4. Again,
> introduced recently by commit 231147ee77f3 ("iio: maxim_thermocouple:
> Align 16 bit big endian value of raw reads"), so you didn't see it.
This is the one I mentioned in the commit message as one that
is fixed in linux-next and that should make it in soon.
> drivers/iio/temperature/maxim_thermocouple.c: In function
> ‘maxim_thermocouple_read_raw’:
> drivers/iio/temperature/maxim_thermocouple.c:141:5: warning: ‘ret’
> may be used uninitialized in this function [-Wmaybe-uninitialized]
> if (ret)
>^
> drivers/iio/temperature/maxim_thermocouple.c:128:6: note: ‘ret’ was
> declared here
> int ret;
> ^~~
>
> and I guess that code can just initialize 'ret' to '-EINVAL' or
> something to just make the theoretical "somehow we had a wrong
> chip->read_size" case error out cleanly.
Right, that was my conclusion too. I sent the bugfix on Oct 25
for linux-next but it didn't make it in until this Monday, after
you pulled the patch that introduced it on Oct 29.
The commit in staging-testing is
32cb7d27e65d ("iio: maxim_thermocouple: detect invalid storage size in read()")
Greg and Jonathan, I see now that this is part of the 'iio-for-4.10b'
branch, so I suspect you were not planning to send this before the
merge window. Could you make sure this ends up in v4.9 so we get
a clean build when -Wmaybe-uninitialized gets enabled again?
Arnd
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v3 2/2] DW DMAC: add multi-block property to device tree
On Friday, November 18, 2016 10:12:36 PM CET Eugeniy Paltsev wrote: > +- multi-block: Multi block transfers supported by hardware per AHB master. > + 0 (default): not supported, 1: supported. Please clarify that this is an array with one cell per master. My first thought was "why is this not a boolean property", and I'm sure others might misread it the same way. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2 1/7] arm: put types.h in uapi
On Friday, January 6, 2017 10:43:53 AM CET Nicolas Dichtel wrote: > > diff --git a/arch/arm/include/asm/types.h b/arch/arm/include/asm/types.h > index a53cdb8f068c..c48fee3d7b3b 100644 > --- a/arch/arm/include/asm/types.h > +++ b/arch/arm/include/asm/types.h > @@ -1,40 +1,6 @@ > #ifndef _ASM_TYPES_H > #define _ASM_TYPES_H > > -#include ... > -#define __UINTPTR_TYPE__ unsigned long > -#endif > +#include > > #endif /* _ASM_TYPES_H */ > Moving the file is correct as far as I can tell, but the extra #include is not necessary here, as the kernel will automatically search both arch/arm/include/ and arch/arm/include/uapi/. The same applies to patches 2 and 4. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2 0/7] uapi: export all headers under uapi directories
On Friday, January 6, 2017 10:43:52 AM CET Nicolas Dichtel wrote: > Here is the v2 of this series. The first 5 patches are just cleanup: some > exported headers were still under a non-uapi directory. Since this is meant as a cleanup, I commented on this to point out a cleaner way to do the same. > The patch 6 was spotted by code review: there is no in-tree user of this > functionality. > The last patch remove the use of header-y. Now all files under an uapi > directory are exported. Very nice! > asm is a bit special, most of architectures export asm//include/uapi/asm > only, but there is two exceptions: > - cris which exports arch/cris/include/uapi/arch-v[10|32]; This is interesting, though not your problem. Maybe someone who understands cris better can comment on this: How is the decision made about which of the arch/user.h headers gets used? I couldn't find that in the sources, but it appears to be based on kernel compile-time settings, which is wrong for user space header files that should be independent of the kernel config. > - tile which exports arch/tile/include/uapi/arch. > Because I don't know if the output of 'make headers_install_all' can be > changed, > I introduce subdir-y in Kbuild file. The headers_install_all target copies all > asm//include/uapi/asm to usr/include/asm- but > arch/cris/include/uapi/arch-v[10|32] and arch/tile/include/uapi/arch are not > prefixed (they are put asis in usr/include/). If it's acceptable to modify the > output of 'make headers_install_all' to export asm headers in > usr/include/asm-/asm, then I could remove this new subdir-y and exports > everything under arch//include/uapi/. I don't know if anyone still uses "make headers_install_all", I suspect distros these days all use "make headers_install", so it probably doesn't matter much. In case of cris, it should be easy enough to move all the contents of the uapi/arch-*/*.h headers into the respective uapi/asm/*.h headers, they only seem to be referenced from there. For tile, I suspect that would not work as the arch/*.h headers are apparently defined as interfaces for both user space and kernel. > Note also that exported files for asm are a mix of files listed by: > - include/uapi/asm-generic/Kbuild.asm; > - arch/x86/include/uapi/asm/Kbuild; > - arch/x86/include/asm/Kbuild. > This complicates a lot the processing (arch/x86/include/asm/Kbuild is also > used by scripts/Makefile.asm-generic). > > This series has been tested with a 'make headers_install' on x86 and a > 'make headers_install_all'. I've checked the result of both commands. > > This patch is built against linus tree. I don't know if it should be > made against antoher tree. The series should probably get merged through the kbuild tree, but testing it on mainline is fine here. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2 3/7] nios2: put setup.h in uapi
On Friday, January 6, 2017 10:43:55 AM CET Nicolas Dichtel wrote: > diff --git a/arch/nios2/include/uapi/asm/setup.h > b/arch/nios2/include/uapi/asm/setup.h > new file mode 100644 > index ..8d8285997ba8 > --- /dev/null > +++ b/arch/nios2/include/uapi/asm/setup.h > @@ -0,0 +1,6 @@ > +#ifndef _UAPI_ASM_NIOS2_SETUP_H > +#define _UAPI_ASM_NIOS2_SETUP_H > + > +#include > + > +#endif /* _UAPI_ASM_NIOS2_SETUP_H */ > This one is only a redirect to an asm-generic header, so it can be removed completely and replaced with a line in the arch/nios2/include/uapi/asm/ file: generic-y += setup.h Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 2/7] clocksource: Rename CLOCKSOURCE_OF_DECLARE
On Sat, May 27, 2017 at 11:58 AM, Daniel Lezcano wrote: > The CLOCKSOUCE_OF_DECLARE macro is used widely for the timers to declare the > clocksource at early stage. However, this macro is also used to initialize > the clockevent if any, or the clockevent only. > > It was originally suggested to declare another macro to initialize a > clockevent, so in order to separate the two entities even they belong to the > same IP. This was not accepted because of the impact on the DT where splitting > a clocksource/clockevent definition does not make sense as it is a Linux > concept not a hardware description. > > On the other side, the clocksource has not interrupt declared while the > clockevent has, so it is easy from the driver to know if the description is > for a clockevent or a clocksource, IOW it could be implemented at the driver > level. > > So instead of dealing with a named clocksource macro, let's use a more generic > one: TIMER_OF_DECLARE. > > The patch has not functional changes. > > Signed-off-by: Daniel Lezcano Could you either leave the old name as an alias for one release, or introduce the new name as an alias now for 4.13? I think that that would make it easier to merge new drivers. Otherwise this looks good to me, Acked-by: Arnd Bergmann ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 2/7] clocksource: Rename CLOCKSOURCE_OF_DECLARE
On Mon, May 29, 2017 at 10:48 AM, Daniel Lezcano wrote: > On Mon, May 29, 2017 at 10:41:52AM +0200, Arnd Bergmann wrote: >> On Sat, May 27, 2017 at 11:58 AM, Daniel Lezcano >> wrote: >> > The CLOCKSOUCE_OF_DECLARE macro is used widely for the timers to declare >> > the >> > clocksource at early stage. However, this macro is also used to initialize >> > the clockevent if any, or the clockevent only. >> > >> > It was originally suggested to declare another macro to initialize a >> > clockevent, so in order to separate the two entities even they belong to >> > the >> > same IP. This was not accepted because of the impact on the DT where >> > splitting >> > a clocksource/clockevent definition does not make sense as it is a Linux >> > concept not a hardware description. >> > >> > On the other side, the clocksource has not interrupt declared while the >> > clockevent has, so it is easy from the driver to know if the description is >> > for a clockevent or a clocksource, IOW it could be implemented at the >> > driver >> > level. >> > >> > So instead of dealing with a named clocksource macro, let's use a more >> > generic >> > one: TIMER_OF_DECLARE. >> > >> > The patch has not functional changes. >> > >> > Signed-off-by: Daniel Lezcano >> >> Could you either leave the old name as an alias for one release, or introduce >> the new name as an alias now for 4.13? >> >> I think that that would make it easier to merge new drivers. Otherwise this >> looks good to me, > > > New drivers should go through my tree, so I can catch them with the old macro > name and do the change. Sure, they should, and it's quite likely that you won't even need the fallback, I've just seen too many cases where a similar assumption turned out wrong, that I'd just pick the safer option just in case whenever I do an API change. Things that could go wrong include: - A platform maintainer wants to add a new platform and has a for-next branch that gets merged into linux-next, with parts of it going through different maintainers, and now they have to choose between a branch that doesn't build without the timer branch, or one that break for-next unless Stephen applies a fixup - Some architecture maintainer didn't get the memo and adds an instance of CLOCKSOUCE_OF_DECLARE in architecture specific code without asking having the patch reviewed first - A platform has a branch with complex cross-tree dependencies and it need to get merged in an unconventional way. - You make a mistake and accidentally merge one driver for an unusual architecture that escapes your test matrix. While those all are unlikely to happen in a particular merge window, they do happen occasionally and tend to cause a lot of pain. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 2/7] clocksource: Rename CLOCKSOURCE_OF_DECLARE
On Mon, May 29, 2017 at 12:55 PM, Daniel Lezcano wrote: > On Mon, May 29, 2017 at 11:57:25AM +0200, Arnd Bergmann wrote: >> On Mon, May 29, 2017 at 10:48 AM, Daniel Lezcano >> wrote: >> Things that could go wrong include: >> >> - A platform maintainer wants to add a new platform and has a for-next >> branch that gets merged into linux-next, with parts of it going through >> different maintainers, and now they have to choose between a branch >> that doesn't build without the timer branch, or one that break for-next >> unless Stephen applies a fixup >> >> - Some architecture maintainer didn't get the memo and adds an instance of >> CLOCKSOUCE_OF_DECLARE in architecture specific code without asking >> having the patch reviewed first >> >> - A platform has a branch with complex cross-tree dependencies and >> it need to get merged in an unconventional way. >> >> - You make a mistake and accidentally merge one driver for an unusual >> architecture that escapes your test matrix. >> >> While those all are unlikely to happen in a particular merge window, they do >> happen occasionally and tend to cause a lot of pain. > > Hmm, that sounds scary :) > > There is no guarantee, when removing the alias, none of the above happens, > right? No, it's just both less likely and easier to work around. > If the timer branch is in linux-next, that could be caugth before any of the > above happens, no? linux-next will find most of these problems, but it will still be more work for the people that run into build failures when testing linux-next. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 00/35] defconfig: Cleanup from old entries
On Thu, Jun 8, 2017 at 6:08 PM, Krzysztof Kozlowski wrote: > Hi, > > While cleaning Samsung ARM defconfigs with savedefconfig, I encountered > similar obsolete entries in other files. > > Except the ARM, no dependencies. > For ARM, the rest of patches depend on the first change (otherwise > it might not apply cleanly). Great work! I looked at all the ARM patches, and everything looks good to me (the changlog linewrapping may be suboptimal for readability in some cases, if I had to say anything negative ;-) ). Please add my Acked-by to the ARM patches and send a pull request. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 00/35] defconfig: Cleanup from old entries
On Sat, Jun 10, 2017 at 6:43 PM, Krzysztof Kozlowski wrote: > On Fri, Jun 09, 2017 at 09:59:48PM +0200, Arnd Bergmann wrote: >> On Thu, Jun 8, 2017 at 6:08 PM, Krzysztof Kozlowski wrote: >> > Hi, >> > >> > While cleaning Samsung ARM defconfigs with savedefconfig, I encountered >> > similar obsolete entries in other files. >> > >> > Except the ARM, no dependencies. >> > For ARM, the rest of patches depend on the first change (otherwise >> > it might not apply cleanly). >> >> Great work! >> >> I looked at all the ARM patches, and everything looks good to me (the >> changlog linewrapping may be suboptimal for readability in some cases, >> if I had to say anything negative ;-) ). >> >> Please add my Acked-by to the ARM patches and send a pull request. > > Thanks, I'll send you ARM part in pull request. Ok > As for the rest, I think respective arch/platform maintainers should > take it. I suspect most will apply the patches, but some architectures maintainers are not very active. If you are motivated, you could resend the ones that missed out to Andrew Morton, or you just leave the architectures to bitrot more. ;-) Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 03/20] asm-generic: Drop getrlimit and setrlimit syscalls from default list
On Mon, Jun 19, 2017 at 11:42 PM, James Hogan wrote: > Hi Yury, > > On Mon, Jun 19, 2017 at 06:49:46PM +0300, Yury Norov wrote: >> The newer prlimit64 syscall provides all the functionality provided by >> the getrlimit and setrlimit syscalls and adds the pid of target process, >> so future architectures won't need to include getrlimit and setrlimit. >> >> Therefore drop getrlimit and setrlimit syscalls from the generic syscall >> list unless __ARCH_WANT_SET_GET_RLIMIT is defined by the architecture's >> unistd.h prior to including asm-generic/unistd.h, and adjust all >> architectures >> using the generic syscall list to define it so that no in-tree architectures >> are affected. > > I have a similar experimental patch lying around for the stat system > calls which are superseded by statx (see below). If it looks acceptable > maybe you'd like to incorporate it (or something similar) into your > series. > > Cheers > James > > --- > From: James Hogan > Date: Fri, 2 Jun 2017 13:07:27 +0100 > Subject: [PATCH] Deprecate stat syscalls superseded by statx > > Various stat system calls can now be implemented as userland wrappers > around the new statx system call, so allow them to be removed from the > kernel by default for new architectures / ABIs. > > This involves adding __ARCH_WANT_SYSCALL_UNXSTAT to each existing > architecture, which enables the relevant stat system calls in the > generic system call list (if used). It also conditionally defines the > syscalls in fs/stat.c and struct stat / struct stat64 in > asm-generic/stat.h. > > Signed-off-by: James Hogan > Cc: David Howells > Cc: Alexander Viro > Cc: Arnd Bergmann > Cc: linux-fsde...@vger.kernel.org > Cc: linux-a...@vger.kernel.org > Cc: linux-...@vger.kernel.org > Cc: linux-ker...@vger.kernel.org Good idea: Acked-by: Arnd Bergmann > +/* statx deprecates the un-extended stat syscalls which use struct stat[64] > */ > +#ifdef __ARCH_WANT_SYSCALL_UNXSTAT I'm glad you explain what 'UNXSTAT' means here, since I would not have otherwise guessed it, but I also can't think of anything more intuitive. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 03/20] asm-generic: Drop getrlimit and setrlimit syscalls from default list
On Tue, Jun 20, 2017 at 3:37 PM, Yury Norov wrote: > On Mon, Jun 19, 2017 at 11:10:23PM +0100, James Hogan wrote: >> On Mon, Jun 19, 2017 at 11:58:41PM +0200, Arnd Bergmann wrote: >> > On Mon, Jun 19, 2017 at 11:42 PM, James Hogan >> > wrote: >> > > On Mon, Jun 19, 2017 at 06:49:46PM +0300, Yury Norov wrote: >> > > Subject: [PATCH] Deprecate stat syscalls superseded by statx >> > > >> > > Various stat system calls can now be implemented as userland wrappers >> > > around the new statx system call, so allow them to be removed from the >> > > kernel by default for new architectures / ABIs. >> > > >> > > This involves adding __ARCH_WANT_SYSCALL_UNXSTAT to each existing >> > > architecture, which enables the relevant stat system calls in the >> > > generic system call list (if used). It also conditionally defines the >> > > syscalls in fs/stat.c and struct stat / struct stat64 in >> > > asm-generic/stat.h. >> > > >> > > Signed-off-by: James Hogan >> > > Cc: David Howells >> > > Cc: Alexander Viro >> > > Cc: Arnd Bergmann >> > > Cc: linux-fsde...@vger.kernel.org >> > > Cc: linux-a...@vger.kernel.org >> > > Cc: linux-...@vger.kernel.org >> > > Cc: linux-ker...@vger.kernel.org >> > >> > Good idea: >> > >> > Acked-by: Arnd Bergmann >> > > +/* statx deprecates the un-extended stat syscalls which use struct >> > > stat[64] */ >> > > +#ifdef __ARCH_WANT_SYSCALL_UNXSTAT >> > >> > I'm glad you explain what 'UNXSTAT' means here, since I would not >> > have otherwise guessed it, but I also can't think of anything more >> > intuitive. >> >> Yeh, I renamed that several times while playing around with this :-). >> >> The stat syscalls remind me a bit of the Vicar of Dibley episode where >> the new road named "New Road" necessitates the renaming of the existing >> "New Road" to "Quite Old Road" and "Quite Old Road" to "Really Quite Old >> Road" and "Old Road" to "Very Old Road"! > > (Add Palmer Dabbelt ) > > The stat syscalls are full of hacks, and we have to pull that hacks > even to new architectures to deal with stat. So I'll be happy to drop > it in arm64/ilp32. But it means that I need some time to integrate > your patch and fix glibc accordingly. And it also means that we need > round 9 for ilp32... :( > > Arnd, once before you told that generic unistd has some duplications > and legacy syscalls, and one day we'll have to deal with it. Do you > have the list or something on it? No, I'd have to do some research for that. > I would also notice riscv people and welcome to the discussion. > > As there is more than 1 arch that goes to be added to linux soon, > maybe it's better to upstream my ans James' patches separately > from other ilp32 patches? Arnd? Do you mean upstream those two patches slightly later? That's fine with me, I don't care much whether the old new stat is part of the syscall table for arm64-ilp32 or not, I'd leave that up to you, depending on whether you want to do the rework or not. I suppose the arm64-ilp32 could benefit from not having to support the old arm32 stat structure, but doing the new syscalls based on statx could delay the glibc port some more, as there are some open questions about how that would best be integrated. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 03/20] asm-generic: Drop getrlimit and setrlimit syscalls from default list
On Tue, Jun 20, 2017 at 4:54 PM, Yury Norov wrote: > On Tue, Jun 20, 2017 at 04:20:43PM +0200, Arnd Bergmann wrote: >> On Tue, Jun 20, 2017 at 3:37 PM, Yury Norov >> wrote: >> > On Mon, Jun 19, 2017 at 11:10:23PM +0100, James Hogan wrote: >> >> On Mon, Jun 19, 2017 at 11:58:41PM +0200, Arnd Bergmann wrote: >> > I would also notice riscv people and welcome to the discussion. >> > >> > As there is more than 1 arch that goes to be added to linux soon, >> > maybe it's better to upstream my ans James' patches separately >> > from other ilp32 patches? Arnd? >> >> Do you mean upstream those two patches slightly later? That's >> fine with me, I don't care much whether the old new stat is part >> of the syscall table for arm64-ilp32 or not, I'd leave that up to >> you, depending on whether you want to do the rework or not. > > I mean that if we want to deprecate rlimit and stat syscalls for > architectures that are under development now, it's better to upstream > patches that actually deprecate it as early as possible. Makes sense. >> I suppose the arm64-ilp32 could benefit from not having to support >> the old arm32 stat structure, but doing the new syscalls based on >> statx could delay the glibc port some more, as there are some open >> questions about how that would best be integrated. > > OK. Let's leave things as is. But then I don't see any reason to > add unxstat patch to ilp32 series if ilp32 will not disable it. Right, that's what I meant: let's leave the rlimit patch in your series as it matches the work you have already done, and is the right thing to do, and let's do the unxstat patch separately so it doesn't cause you extra work. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH] bug.h: Work around GCC PR82365 in BUG()
Looking at functions with large stack frames across all architectures
led me discovering that BUG() suffers from the same problem as
fortify_panic(), which I've added a workaround for already. In short,
variables that go out of scope by calling a noreturn function or
__builtin_unreachable() keep using stack space in functions afterwards.
A workaround that was identified is to insert an empty assembler statement
just before calling the function that doesn't return. I'm adding a macro
"barrier_before_unreachable()" to document this, and insert calls to
that in all instances of BUG() that currently suffer from this problem.
The files that saw the largest change from this had these frame sizes
before, and much less with my patch:
fs/ext4/inode.c:82:1: warning: the frame size of 1672 bytes is larger than 800
bytes [-Wframe-larger-than=]
fs/ext4/namei.c:434:1: warning: the frame size of 904 bytes is larger than 800
bytes [-Wframe-larger-than=]
fs/ext4/super.c:2279:1: warning: the frame size of 1160 bytes is larger than
800 bytes [-Wframe-larger-than=]
fs/ext4/xattr.c:146:1: warning: the frame size of 1168 bytes is larger than 800
bytes [-Wframe-larger-than=]
fs/f2fs/inode.c:152:1: warning: the frame size of 1424 bytes is larger than 800
bytes [-Wframe-larger-than=]
net/netfilter/ipvs/ip_vs_core.c:1195:1: warning: the frame size of 1068 bytes
is larger than 800 bytes [-Wframe-larger-than=]
net/netfilter/ipvs/ip_vs_core.c:395:1: warning: the frame size of 1084 bytes is
larger than 800 bytes [-Wframe-larger-than=]
net/netfilter/ipvs/ip_vs_ftp.c:298:1: warning: the frame size of 928 bytes is
larger than 800 bytes [-Wframe-larger-than=]
net/netfilter/ipvs/ip_vs_ftp.c:418:1: warning: the frame size of 908 bytes is
larger than 800 bytes [-Wframe-larger-than=]
net/netfilter/ipvs/ip_vs_lblcr.c:718:1: warning: the frame size of 960 bytes is
larger than 800 bytes [-Wframe-larger-than=]
drivers/net/xen-netback/netback.c:1500:1: warning: the frame size of 1088 bytes
is larger than 800 bytes [-Wframe-larger-than=]
In case of ARC and CRIS, it turns out that the BUG() implementation
actually does return (or at least the compiler thinks it does), resulting
in lots of warnings about uninitialized variable use and leaving noreturn
functions, such as:
block/cfq-iosched.c: In function 'cfq_async_queue_prio':
block/cfq-iosched.c:3804:1: error: control reaches end of non-void function
[-Werror=return-type]
include/linux/dmaengine.h: In function 'dma_maxpq':
include/linux/dmaengine.h:1123:1: error: control reaches end of non-void
function [-Werror=return-type]
This makes them call __builtin_trap() instead, which should normally
dump the stack and kill the current process, like some of the other
architectures already do.
I tried adding barrier_before_unreachable() to panic() and fortify_panic()
as well, but that had very little effect, so I'm not submitting that
patch.
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82365
Signed-off-by: Arnd Bergmann
---
The name barrier_before_unreachable() is a bit suboptimal here,
as it fails to describe the fact that it is needed for both
__builtin_unreachable() and for calling noreturn functions. Any other
suggestions would be welcome here.
---
arch/arc/include/asm/bug.h| 3 ++-
arch/cris/include/arch-v10/arch/bug.h | 11 +--
arch/ia64/include/asm/bug.h | 6 +-
arch/m68k/include/asm/bug.h | 3 +++
arch/sparc/include/asm/bug.h | 6 +-
include/asm-generic/bug.h | 1 +
include/linux/compiler-gcc.h | 15 ++-
include/linux/compiler.h | 5 +
8 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/arch/arc/include/asm/bug.h b/arch/arc/include/asm/bug.h
index ea022d47896c..21ec82466d62 100644
--- a/arch/arc/include/asm/bug.h
+++ b/arch/arc/include/asm/bug.h
@@ -23,7 +23,8 @@ void die(const char *str, struct pt_regs *regs, unsigned long
address);
#define BUG() do {
\
pr_warn("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __func__);
\
- dump_stack();
\
+ barrier_before_unreachable();
\
+ __builtin_trap();
\
} while (0)
#define HAVE_ARCH_BUG
diff --git a/arch/cris/include/arch-v10/arch/bug.h
b/arch/cris/include/arch-v10/arch/bug.h
index 905afeacfedf..06da9d49152a 100644
--- a/arch/cris/include/arch-v10/arch/bug.h
+++ b/arch/cris/include/arch-v10/arch/bug.h
@@ -44,18 +44,25 @@ struct bug_frame {
* not be used like this with newer versions of gcc.
*/
#define BUG() \
+do { \
__asm__ __volatile__ ("clear.d ["
Re: [PATCH] bug.h: Work around GCC PR82365 in BUG()
On Tue, Dec 19, 2017 at 5:57 PM, Vineet Gupta
wrote:
> On 12/19/2017 03:41 AM, Arnd Bergmann wrote:
>> In case of ARC and CRIS, it turns out that the BUG() implementation
>> actually does return (or at least the compiler thinks it does), resulting
>> in lots of warnings about uninitialized variable use and leaving noreturn
>> functions, such as:
>>
>> block/cfq-iosched.c: In function 'cfq_async_queue_prio':
>> block/cfq-iosched.c:3804:1: error: control reaches end of non-void
>> function [-Werror=return-type]
>> include/linux/dmaengine.h: In function 'dma_maxpq':
>> include/linux/dmaengine.h:1123:1: error: control reaches end of non-void
>> function [-Werror=return-type]
>> diff --git a/arch/arc/include/asm/bug.h b/arch/arc/include/asm/bug.h
>> index ea022d47896c..21ec82466d62 100644
>> --- a/arch/arc/include/asm/bug.h
>> +++ b/arch/arc/include/asm/bug.h
>> @@ -23,7 +23,8 @@ void die(const char *str, struct pt_regs *regs, unsigned
>> long address);
>> #define BUG() do {
>> \
>> pr_warn("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__,
>> __func__); \
>> - dump_stack();
>> \
>> + barrier_before_unreachable();
>> \
>> + __builtin_trap();
>> \
>> } while (0)
>> #define HAVE_ARCH_BUG
>
>
> I suppose BUG() implies "dead end" like semantics - which ARC was lacking
> before ?
Correct. Using __builtin_trap() here avoids the 'control reaches end of non-void
function' warnings, but then makes us run into the stack size problem that
I work around with the barrier_before_unreachable().
It would be good if you could give this a quick test to see if you get sensible
output from the __builtin_trap();
Arnd
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] bug.h: Work around GCC PR82365 in BUG()
On Tue, Dec 19, 2017 at 11:38 PM, Vineet Gupta
wrote:
> On 12/19/2017 12:13 PM, Arnd Bergmann wrote:
>>
>>
>>> I suppose BUG() implies "dead end" like semantics - which ARC was lacking
>>> before ?
>>
>> Correct. Using __builtin_trap() here avoids the 'control reaches end of
>> non-void
>> function' warnings, but then makes us run into the stack size problem that
>> I work around with the barrier_before_unreachable().
>>
>> It would be good if you could give this a quick test to see if you get
>> sensible
>> output from the __builtin_trap();
>
>
> It does, added a BUG() arbit, hits an abort()
>
> ...
> ISA Extn: atomic ll64 unalign (not used)
> : mpy[opt 9] div_rem norm barrel-shift swap minmax swape
> BPU: partial match, cache:2048, Predict Table:16384
> BUG: failure at ../arch/arc/mm/tlb.c:827/arc_mmu_init()!
>
>
> Tested-by: Vineet Gupta
I meant whether it prints the right registers and stack trace, but I
assume you tested that and just did not list it above.
> FWIW newer ARC gcc actually implements the builtin so we get a trap 5
> instruction now, vs., abort() calls before.
>
> BTW I missed reading the hunk of your changelog where this addresses the
> long standing mystery with ARC builds and numerous -Wreturn-type warnings. I
> always wondered why they were not fixed upstream already, being too lazy to
> investigate myself, and turns out this was due to this BUG() thingy. phew !
Hmm, so with the new definition of abort(),
+__weak void abort(void)
+{
+ BUG();
+
+ /* if that doesn't kill us, halt */
+ panic("Oops failed to kill thread");
+}
won't that run into an endless recursion? Or do you then override abort()
for ARC?
Arnd
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] bug.h: Work around GCC PR82365 in BUG()
On Wed, Dec 20, 2017 at 7:52 PM, Vineet Gupta
wrote:
> On 12/20/2017 01:01 AM, Arnd Bergmann wrote:
>>
>> On Tue, Dec 19, 2017 at 11:38 PM, Vineet Gupta
>> wrote:
>>>
>>> On 12/19/2017 12:13 PM, Arnd Bergmann wrote:
>>>>
>>>>
>>>>
>>>>> I suppose BUG() implies "dead end" like semantics - which ARC was
>>>>> lacking
>>>>> before ?
>>>>
>>>>
>>>> Correct. Using __builtin_trap() here avoids the 'control reaches end of
>>>> non-void
>>>> function' warnings, but then makes us run into the stack size problem
>>>> that
>>>> I work around with the barrier_before_unreachable().
>>>>
>>>> It would be good if you could give this a quick test to see if you get
>>>> sensible
>>>> output from the __builtin_trap();
>>>
>>>
>>>
>>> It does, added a BUG() arbit, hits an abort()
>>>
>>> ...
>>> ISA Extn: atomic ll64 unalign (not used)
>>> : mpy[opt 9] div_rem norm barrel-shift swap minmax swape
>>> BPU: partial match, cache:2048, Predict Table:16384
>>> BUG: failure at ../arch/arc/mm/tlb.c:827/arc_mmu_init()!
>>>
>>>
>>> Tested-by: Vineet Gupta
>>
>>
>> I meant whether it prints the right registers and stack trace, but I
>> assume you tested that and just did not list it above.
>
>
> Sorry, I didn't realize we are missing the stack trace now which you removed
> from the patch - why ? Did u intend to reduce inline generated code for the
> stack dump calls - which sounds like a great idea. But it would only work
> for the synchronous abort() but not when builtin translates to actual trap
> inducing instruction.
I assumed that the trap instruction would trigger the register and
stack dump, as it does on all other architectures. The most common
way this is handled is to have one instruction that is known to trap,
and use that to trigger a BUG(), and have __builtin_trap() issue
that instruction as well.
You might also want to implement CONFIG_DEBUG_BUGVERBOSE
support to attach further data to it.
>> Hmm, so with the new definition of abort(),
>>
>> +__weak void abort(void)
>> +{
>> + BUG();
>> +
>> + /* if that doesn't kill us, halt */
>> + panic("Oops failed to kill thread");
>> +}
>>
>> won't that run into an endless recursion? Or do you then override abort()
>> for ARC?
>
>
> Indeed so. I didn't run into this in my testing as my for-curr has an ARC
> specific version (predating Sudip's generic version- because of build
> failures in our internal regression jobs etc). That version only calls
> panic.
>
> abort() is only likely to be called due to __builtin_trap() for arches where
> gcc doesn't have a target specific defn of it. And thus adding the call from
> BUG() will cause the recursion as you found out with Sudip's generic version
> and thus needs a fixup.
How about overriding abort() with the same instruction that
__builtin_trap() inserts on newer compilers then? That should
make the behavior consistent.
Arnd
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 2/2] ARC: handle gcc generated __builtin_trap for older compiler
On Wed, Dec 20, 2017 at 11:49 PM, Vineet Gupta wrote: > ARC gcc prior to GNU 2018.03 release didn't have a target specific > __builtin_trap() implementation, generating default abort() call. > > Implement the abort() call - emulating what newer gcc does for the same, > as suggested by Arnd. > > Signed-off-by: Vineet Gupta Acked-by: Arnd Bergmann ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH] arch: drop duplicate exports of abort()
We now have exports in both architecture code in in common code,
which causes a link failure when symbol versioning is eanbled, on
four architectures:
kernel/exit.o: In function `__crc_abort':
exit.c:(*ABS*+0xc0e2ec8b): multiple definition of `__crc_abort'
This removes the four architecture specific exports and only
leaves the export next to the __weak symbol.
Fixes: mmotm ("kernel/exit.c: export abort() to modules")
Signed-off-by: Arnd Bergmann
---
Andrew, can you apply this to -mm on top of the other patch?
---
arch/arc/kernel/traps.c | 1 -
arch/arm/kernel/traps.c | 1 -
arch/m32r/kernel/traps.c | 1 -
arch/unicore32/kernel/traps.c | 1 -
4 files changed, 4 deletions(-)
diff --git a/arch/arc/kernel/traps.c b/arch/arc/kernel/traps.c
index 51a55b06cb2a..133a4dae41fe 100644
--- a/arch/arc/kernel/traps.c
+++ b/arch/arc/kernel/traps.c
@@ -169,4 +169,3 @@ void abort(void)
{
__asm__ __volatile__("trap_s 5\n");
}
-EXPORT_SYMBOL(abort);
diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index e344bdd2e5ac..5e3633c24e63 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -793,7 +793,6 @@ void abort(void)
/* if that doesn't kill us, halt */
panic("Oops failed to kill thread");
}
-EXPORT_SYMBOL(abort);
void __init trap_init(void)
{
diff --git a/arch/m32r/kernel/traps.c b/arch/m32r/kernel/traps.c
index cb79fba79d43..b88a8dd14933 100644
--- a/arch/m32r/kernel/traps.c
+++ b/arch/m32r/kernel/traps.c
@@ -122,7 +122,6 @@ void abort(void)
/* if that doesn't kill us, halt */
panic("Oops failed to kill thread");
}
-EXPORT_SYMBOL(abort);
void __init trap_init(void)
{
diff --git a/arch/unicore32/kernel/traps.c b/arch/unicore32/kernel/traps.c
index 5f25b39f04d4..c4ac6043ebb0 100644
--- a/arch/unicore32/kernel/traps.c
+++ b/arch/unicore32/kernel/traps.c
@@ -298,7 +298,6 @@ void abort(void)
/* if that doesn't kill us, halt */
panic("Oops failed to kill thread");
}
-EXPORT_SYMBOL(abort);
void __init trap_init(void)
{
--
2.9.0
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH] arc: fix iounmap prototype
The missing 'volatile' keyword on the iounmap argument leads to lots of
harmless warnings in an allmodconfig build:
sound/pci/echoaudio/echoaudio.c:1879:10: warning: passing argument 1 of
'iounmap' discards 'volatile' qualifier f
pointer target type [-Wdiscarded-qualifiers]
Signed-off-by: Arnd Bergmann
---
arch/arc/include/asm/io.h | 4 ++--
arch/arc/mm/ioremap.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arc/include/asm/io.h b/arch/arc/include/asm/io.h
index c22b181e8206..2c9b98fabf82 100644
--- a/arch/arc/include/asm/io.h
+++ b/arch/arc/include/asm/io.h
@@ -30,11 +30,11 @@ static inline void __iomem *ioport_map(unsigned long port,
unsigned int nr)
return (void __iomem *)port;
}
-static inline void ioport_unmap(void __iomem *addr)
+static inline void ioport_unmap(volatile void __iomem *addr)
{
}
-extern void iounmap(const void __iomem *addr);
+extern void iounmap(const volatile void __iomem *addr);
#define ioremap_nocache(phy, sz) ioremap(phy, sz)
#define ioremap_wc(phy, sz)ioremap(phy, sz)
diff --git a/arch/arc/mm/ioremap.c b/arch/arc/mm/ioremap.c
index 9881bd740ccc..94d0116063a8 100644
--- a/arch/arc/mm/ioremap.c
+++ b/arch/arc/mm/ioremap.c
@@ -95,7 +95,7 @@ void __iomem *ioremap_prot(phys_addr_t paddr, unsigned long
size,
EXPORT_SYMBOL(ioremap_prot);
-void iounmap(const void __iomem *addr)
+void iounmap(volatile const void __iomem *addr)
{
/* weird double cast to handle phys_addr_t > 32 bits */
if (arc_uncached_addr_space((phys_addr_t)(u32)addr))
--
2.9.0
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [RFC PATCH 6/6] arch: add untagged_addr definition for other arches
On Fri, Mar 9, 2018 at 3:02 PM, Andrey Konovalov wrote: > To allow arm64 syscalls accept tagged pointers from userspace, we must > untag them when they are passed to the kernel. Since untagging is done in > generic parts of the kernel (like the mm subsystem), the untagged_addr > macro should be defined for all architectures. > > Define it as a noop for all other architectures besides arm64. > > Signed-off-by: Andrey Konovalov > --- > arch/alpha/include/asm/uaccess.h | 2 ++ > arch/arc/include/asm/uaccess.h| 1 + > arch/arm/include/asm/uaccess.h| 2 ++ > arch/blackfin/include/asm/uaccess.h | 2 ++ > arch/c6x/include/asm/uaccess.h| 2 ++ > arch/cris/include/asm/uaccess.h | 2 ++ > arch/frv/include/asm/uaccess.h| 2 ++ > arch/ia64/include/asm/uaccess.h | 2 ++ > arch/m32r/include/asm/uaccess.h | 2 ++ > arch/m68k/include/asm/uaccess.h | 2 ++ > arch/metag/include/asm/uaccess.h | 2 ++ > arch/microblaze/include/asm/uaccess.h | 2 ++ > arch/mips/include/asm/uaccess.h | 2 ++ > arch/mn10300/include/asm/uaccess.h| 2 ++ > arch/nios2/include/asm/uaccess.h | 2 ++ > arch/openrisc/include/asm/uaccess.h | 2 ++ > arch/parisc/include/asm/uaccess.h | 2 ++ > arch/powerpc/include/asm/uaccess.h| 2 ++ > arch/riscv/include/asm/uaccess.h | 2 ++ > arch/score/include/asm/uaccess.h | 2 ++ > arch/sh/include/asm/uaccess.h | 2 ++ > arch/sparc/include/asm/uaccess.h | 2 ++ > arch/tile/include/asm/uaccess.h | 2 ++ > arch/x86/include/asm/uaccess.h| 2 ++ > arch/xtensa/include/asm/uaccess.h | 2 ++ > include/asm-generic/uaccess.h | 2 ++ > 26 files changed, 51 insertions(+) I have patches to remove the blackfin, cris, frv, m32r, metag, mn10300, score, tile and unicore32 architectures from the kernel, these should be part of linux-next in the next few days. It's not a big issue, but if you keep patching them, this will cause a merge conflict. It might be easier to drop them from your patch as well. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] bug.h: Work around GCC PR82365 in BUG()
On Wed, Apr 11, 2018 at 12:48 AM, James Hogan wrote:
> Hi Arnd,
>
> On Tue, Dec 19, 2017 at 12:39:33PM +0100, Arnd Bergmann wrote:
>> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
>> index 5d595cfdb2c4..66cfdad68f7e 100644
>> --- a/include/linux/compiler-gcc.h
>> +++ b/include/linux/compiler-gcc.h
>> @@ -205,6 +205,15 @@
>> #endif
>>
>> /*
>> + * calling noreturn functions, __builtin_unreachable() and __builtin_trap()
>> + * confuse the stack allocation in gcc, leading to overly large stack
>> + * frames, see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82365
>> + *
>> + * Adding an empty inline assembly before it works around the problem
>> + */
>> +#define barrier_before_unreachable() asm volatile("")
>> +
>> +/*
>> * Mark a position in code as unreachable. This can be used to
>> * suppress control flow warnings after asm blocks that transfer
>> * control elsewhere.
>> @@ -214,7 +223,11 @@
>> * unreleased. Really, we need to have autoconf for the kernel.
>> */
>> #define unreachable() \
>> - do { annotate_unreachable(); __builtin_unreachable(); } while (0)
>> + do {\
>> + annotate_unreachable(); \
>> + barrier_before_unreachable(); \
>> + __builtin_unreachable();\
>> + } while (0)
>
> Unfortunately this breaks microMIPS builds (e.g. MIPS
> micro32r2_defconfig and micro32r2el_defconfig) on gcc 7.2, due to the
> lack of .insn in the asm volatile. Because of the
> __builtin_unreachable() there is no code following it. Without the empty
> asm the compiler will apparently put the .insn there automatically, but
> with the empty asm it doesn't. Therefore the assembler won't treat an
> immediately preceeding label as pointing at 16-bit microMIPS
> instructions which need the ISA bit set, i.e. bit 0 of the address.
> This causes assembler errors since the branch target is treated as a
> different ISA mode:
>
> arch/mips/mm/dma-default.s:3265: Error: branch to a symbol in another ISA mode
> arch/mips/mm/dma-default.s:5027: Error: branch to a symbol in another ISA mode
Ok, I see.
> Due to a compiler bug on gcc 4.9.2 -> somewhere before 7.2, Paul
> submitted these patches a while back:
> https://patchwork.linux-mips.org/patch/13360/
> https://patchwork.linux-mips.org/patch/13361/
>
> Your patch (suitably fixed for microMIPS) would I imagine fix that issue
> too (it certainly fixes the resulting link error on microMIPS builds
> with an old toolchain).
>
> Before I forward port those patches to add .insn for MIPS, is that sort
> of approach (an arch specific asm/compiler-gcc.h to allow MIPS to
> override barrier_before_unreachable()) an acceptable fix?
That sounds fine to me. However, I would suggest making that
asm/compiler.h instead of asm/compiler-gcc.h, so we can also
use the same file to include workarounds for clang if needed.
Arnd
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] bug.h: Work around GCC PR82365 in BUG()
On Wed, Apr 11, 2018 at 11:54 AM, James Hogan wrote: > On Wed, Apr 11, 2018 at 09:30:56AM +0200, Arnd Bergmann wrote: >> On Wed, Apr 11, 2018 at 12:48 AM, James Hogan wrote: >> > Before I forward port those patches to add .insn for MIPS, is that sort >> > of approach (an arch specific asm/compiler-gcc.h to allow MIPS to >> > override barrier_before_unreachable()) an acceptable fix? >> >> That sounds fine to me. However, I would suggest making that >> asm/compiler.h instead of asm/compiler-gcc.h, so we can also >> use the same file to include workarounds for clang if needed. > > Yes, though there are a few asm/compiler.h's already, and the alpha one > includes linux/compiler.h before undefining inline, so seems to have its > own specific purpose... Interesting. For the other ones, including asm/compiler.h from linux/compiler.h seems appropriate though, so the question would be what to do with the alpha case. I think we can simply remove that header file and replace it with this patch: diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig index b2022885ced8..5502404f54cd 100644 --- a/arch/alpha/Kconfig +++ b/arch/alpha/Kconfig @@ -81,6 +81,9 @@ config PGTABLE_LEVELS int default 3 +config OPTIMIZE_INLINING + def_bool y + source "init/Kconfig" source "kernel/Kconfig.freezer" which should have the same effect. ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: Misaligned Access
On Wed, Nov 21, 2018 at 8:42 PM Vineet Gupta wrote:
>
> +CC lkml, Arnd : subject matter expert
>
> On 11/21/18 10:06 AM, Vitor Soares wrote:
> > I use the follow function to get data from a RX Fifo.
> >
> >
> > static void dw_i3c_master_read_rx_fifo(struct dw_i3c_master *master,
> > u8 *bytes, int nbytes)
> > {
> > readsl(master->regs + RX_TX_DATA_PORT, bytes, nbytes / 4);
>
> So the semantics are reading the same fifo register N times, to get the N
> words,
> hence read*s*l is appropriate. That however expects the buffer to be 4 bytes
> aligned, hence your issue. You can't possibly use the reads*b* as we want the
>
> The obvious but crude hack is to use a temp array for readsl and then copy
> over
> using memcpy, but I'm sure there are better ways, @Arnd ? To summarize is
> issue is
> a driver triggering unaligned access due to the misinteraction of API (driver
> get
> an unaligned u8 *) which goes against expectations of io accessor readl
> (needed
> since the register contents are 4 bytes)
Is this again on ARC or some other architecture that cannot do unaligned
access to normal RAM? On ARMv7 or x86, you should never see a problem
because the CPU handles misaligned writes. On ARMv4/v5, the readsl()
implementation internally aligns the access to the output buffer so it
will work correctly.
Arnd
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: Misaligned Access
On Wed, Nov 21, 2018 at 9:15 PM Vineet Gupta wrote:
>
> On 11/21/18 12:12 PM, Arnd Bergmann wrote:
> > On Wed, Nov 21, 2018 at 8:42 PM Vineet Gupta
> > wrote:
> >> +CC lkml, Arnd : subject matter expert
> >>
> >> On 11/21/18 10:06 AM, Vitor Soares wrote:
> >>> I use the follow function to get data from a RX Fifo.
> >>>
> >>>
> >>> static void dw_i3c_master_read_rx_fifo(struct dw_i3c_master *master,
> >>> u8 *bytes, int nbytes)
> >>> {
> >>> readsl(master->regs + RX_TX_DATA_PORT, bytes, nbytes / 4);
> >> So the semantics are reading the same fifo register N times, to get the N
> >> words,
> >> hence read*s*l is appropriate. That however expects the buffer to be 4
> >> bytes
> >> aligned, hence your issue. You can't possibly use the reads*b* as we want
> >> the
> >>
> >> The obvious but crude hack is to use a temp array for readsl and then copy
> >> over
> >> using memcpy, but I'm sure there are better ways, @Arnd ? To summarize is
> >> issue is
> >> a driver triggering unaligned access due to the misinteraction of API
> >> (driver get
> >> an unaligned u8 *) which goes against expectations of io accessor readl
> >> (needed
> >> since the register contents are 4 bytes)
> > Is this again on ARC or some other architecture that cannot do unaligned
> > access to normal RAM? On ARMv7 or x86, you should never see a problem
> > because the CPU handles misaligned writes. On ARMv4/v5, the readsl()
> > implementation internally aligns the access to the output buffer so it
> > will work correctly.
>
> This is indeed on ARC: on ARC700 unaligned access to RAM was never supported
> and
> on HS38x cores, it is configurable, so the API probably needs to support both
> cases.
Ok, then I think you need to overwrite readsl() with a variant that
uses put_unaligned() instead the plain pointer dereference for the
output.
Arnd
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2] ARC: io.h: Implement reads{x}()/writes{x}()
On Thu, Nov 29, 2018 at 5:14 PM Jose Abreu wrote:
> --->8--
> static noinline void test_readsl(char *buf, int len)
> {
> readsl(0xdeadbeef, buf, len);
> }
> --->8---
>
> And the disassembly:
> --->8---
> 0e88 :
> e88:breq.dr1,0,eac <0xeac>/* if (count) */
> e8c:and r2,r0,3
>
> e90:mov_s lp_count,r1/* r1 = count */
> e92:brne r2,0,eb0 <0xeb0>/* if (bptr % ((t) / 8)) */
>
> e96:sub r0,r0,4
> e9a:nop_s
>
> e9c:lp eac <0xeac>/* first loop */
> ea0:ld r2,[0xdeadbeef]
> ea8:st.a r2,[r0,4]
> eac:j_s [blink]
> eae:nop_s
>
> eb0:lp ed6 <0xed6>/* second loop */
> eb4:ld r2,[0xdeadbeef]
> ebc:lsr r5,r2,8
> ec0:lsr r4,r2,16
> ec4:lsr r3,r2,24
> ec8:stb_s r2,[r0,0]
> eca:stb r5,[r0,1]
> ece:stb r4,[r0,2]
> ed2:stb_s r3,[r0,3]
> ed4:add_s r0,r0,4
> ed6:j_s [blink]
>
> --->8---
>
> See how the if condition added in this version is checked in
> and then it takes two different loops.
This looks good to me. I wonder what the result is for CPUs
that /do/ support unaligned accesses. Normally put_unaligned()
should fall back to a simple store in that case, but I'm not
sure it can fold the two stores back into one and skip the
alignment check. Probably not worth overoptimizing for that
case (the MMIO access latency should be much higher than
anything you could gain here), but I'm still curious about
how well our get/put_unaligned macros work.
Arnd
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2] ARC: io.h: Implement reads{x}()/writes{x}()
On Fri, Nov 30, 2018 at 9:57 AM Jose Abreu wrote: > On 29-11-2018 21:20, Arnd Bergmann wrote: > > On Thu, Nov 29, 2018 at 5:14 PM Jose Abreu wrote: > >> See how the if condition added in this version is checked in > >> and then it takes two different loops. > > This looks good to me. I wonder what the result is for CPUs > > that /do/ support unaligned accesses. Normally put_unaligned() > > should fall back to a simple store in that case, but I'm not > > sure it can fold the two stores back into one and skip the > > alignment check. Probably not worth overoptimizing for that > > case (the MMIO access latency should be much higher than > > anything you could gain here), but I'm still curious about > > how well our get/put_unaligned macros work. > > Here is disassembly for an ARC CPU that supports unaligned accesses: > > -->8--- > 0d48 : > d48:breq_s r1,0,28/* if (count) */ > d4a:tstr0,0x3 > d4e:bne_s 32/* if (bptr % ((t) / 8)) */ > > d50:ld r2,[0xdeadbeef]/* first loop */ > d58:sub_s r1,r1,0x1 > d5a:tst_s r1,r1 > d5c:bne.d -12 > d60:st.ab r2,[r0,4] > > d64:dmb0x1/* common exit point */ > d68:j_s[blink] > d6a:nop_s > > d6c:ld r2,[0xdeadbeef]/* second loop */ > d74:sub_s r1,r1,0x1 > d76:tst_s r1,r1 > d78:bne.d -12 > d7c:st.ab r2,[r0,4] > > d80:b_s -28/* jmp to 0xd64 */ > d82:nop_s > --->8--- > > Notice how first and second loop are exactly equal ... Ok, so it's halfway there: it managed to optimize the the unaligned case correctly, but it failed to notice that both sides are identical now. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: ARC vs. generic sigaction (was Re: [PATCH 08/21] ARC: Linux Syscall Interface)
On Thu, Dec 20, 2018 at 12:19 PM Adhemerval Zanella
wrote:
> On 19/12/2018 20:23, Vineet Gupta wrote:
> > On 12/19/18 2:00 PM, Adhemerval Zanella wrote:
> >> One possibility is to define an arch-specific __sigset_t.h with a custom
> >> _SIGSET_NWORDS value and add an optimization on Linux sigaction.c to check
> >> if both kernel_sigaction and glibc sigaction share same size and internal
> >> layout to use the struct as-is for syscall instead of copy to a temporary
> >> value (similar to what we used to have on getdents). ARC would still have
> >> arch-specific code and would be the only ABI to have a different sigset_t
> >> though.
> >
> > I don't like ARC to single out either. But as Joseph suggests could this be
> > starting point for arches of future. Assuming it does, would rather see
> > this or
> > the approach Joseph alluded to earlier [1]
> >
> > [1]
> > http://lists.infradead.org/pipermail/linux-snps-arc/2018-December/005122.html
> >
> >>
> >> However I *hardly* think sigaction is a hotspot in real world cases,
> >> usually
> >> the signal action is defined once or in a very limited number of times. I
> >> am
> >> not considering synthetic benchmarks, specially lmbench which in most cases
> >> does not measure any useful metric.
> >
> > I tend to disagree. Coming from embedded linux background, I've found it
> > immensely
> > useful to compare 2 minimal systems: especially when distos, out-of-box
> > packages,
> > fancy benchmarks don't even exist.
> >
> > At any rate, my disagreement with status quo is not so much of optimize for
> > ARC,
> > but rather pointless spending of electrons. When we know that there are 64
> > signals
> > at max, which need 64-bits, why bother shuffling around 1k bits, specially
> > when
> > one is starting afresh and there's no legacy crap getting in the way.
> >
> > The case of adding more signals in future is indeed theoretically possible
> > but
> > that will be an ABI change anyways.
>
> The only advantage of using a larger sigset_t from glibc standpoint is if
> kernel ever change it maximum number of supported signals it would not be
> a ABI change (it would be if glibc provided sigset_t need to be extended).
>
> My point was that this change would hardly help in performance or memory
> utilization due the signal set common utilization in exported and internal
> API. But at the same time the signal set hasn't been changed for a *long*
> time
> and I don't see indication that it will any time soon. So a new architecture
> might indeed assume it won't change and set its default to follow Linux user
> ABI.
I just checked again what we actually use in the kernel.
With very few exceptions, all architectures in current kernels use
#define _NSIG 64
#define _NSIG_WORDS (_NSIG / _NSIG_BPW)
typedef struct {
/* this sucks for big-endian 32-bit compat mode */
unsigned long sig[_NSIG_WORDS];
} sigset_t;
The only two exceptions I see are:
- alpha uses a scalar 'unsigned long' instead of the struct+array, but
the effect is the same layout.
- For unknown reasons, all three MIPS ABIs now use _NSIG=128.
This changed during linux-2.1.x from 65, to 32 and then the current
value...
All users of sigset_t that I could find in the kernel just check
the size argument to ensure it matches _NSIG, and return
EINVAL otherwise.
Arnd
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: perf tools build broken after v5.1-rc1
On Mon, Apr 29, 2019 at 7:17 PM Vineet Gupta wrote: > > On 4/22/19 8:31 AM, Arnaldo Carvalho de Melo wrote: > >> A quick fix for ARC will be to create our own version but I presume all > >> existing > >> arches using generic syscall abi are affected. Thoughts ? In lack of ideas > >> I'll > >> send out a patch for ARC. > >> > >> P.S. Why do we need the unistd.h duplication in tools directory, given it > >> could > >> have used the in-tree unistd headers directly ? > > I have to write down the explanation and have it in a file, but we can't > > use anything in the kernel from outside tools/ to avoid adding a burden > > to kernel developers that would then have to make sure that the changes > > that they make outside tools/ don't break things living there. > > That is a sound guiding principle in general but I don't agree here. unistd is > backbone of kernel user interface it has to work and can't possibly be broken > even > when kernel devs add a new syscall is added or condition-alize existing one. > So > adding a copy - and deferring the propagation of in-kernel unistd to usersapce > won't necessarily help with anything and it just adds the burden of keeping > them > in sync. Granted we won't necessarily need all the bleeding edge (new syscall > updates) into that header, its still more work. I think more importantly, it seems completely broken to sync a file from asm-generic but not the arch specific file that includes it. The 1a787fc5ba18ac7 commit copied over the changes for arm64, but missed all the other architectures changed in c8ce48f06503 and the related commits. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH] ARC: hide unused function unw_hdr_alloc
As kernelci.org reports, this function is not used in
vdk_hs38_defconfig:
arch/arc/kernel/unwind.c:188:14: warning: 'unw_hdr_alloc' defined but not used
[-Wunused-function]
Fixes: bc79c9a72165 ("ARC: dw2 unwind: Reinstante unwinding out of modules")
Link: https://kernelci.org/build/id/5d1cae3f59b514300340c132/logs/
Cc: sta...@vger.kernel.org
Signed-off-by: Arnd Bergmann
---
arch/arc/kernel/unwind.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/arch/arc/kernel/unwind.c b/arch/arc/kernel/unwind.c
index 182ce67dfe10..c2663fce7f6c 100644
--- a/arch/arc/kernel/unwind.c
+++ b/arch/arc/kernel/unwind.c
@@ -181,11 +181,6 @@ static void *__init unw_hdr_alloc_early(unsigned long sz)
return memblock_alloc_from(sz, sizeof(unsigned int), MAX_DMA_ADDRESS);
}
-static void *unw_hdr_alloc(unsigned long sz)
-{
- return kmalloc(sz, GFP_KERNEL);
-}
-
static void init_unwind_table(struct unwind_table *table, const char *name,
const void *core_start, unsigned long core_size,
const void *init_start, unsigned long init_size,
@@ -366,6 +361,10 @@ static void init_unwind_hdr(struct unwind_table *table,
}
#ifdef CONFIG_MODULES
+static void *unw_hdr_alloc(unsigned long sz)
+{
+ return kmalloc(sz, GFP_KERNEL);
+}
static struct unwind_table *last_table;
--
2.20.0
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] ARC: hide unused function unw_hdr_alloc
n Wed, Jul 3, 2019 at 6:13 PM Vineet Gupta wrote: > > On 7/3/19 6:39 AM, Arnd Bergmann wrote: > > As kernelci.org reports, > > Curious, how are you getting these reports ? I want to see as well. I'm subscribed to the mailing list at https://lists.linaro.org/mailman/listinfo/kernel-build-reports I think you can also ask the kernelci folks to get a different subset of the build reports. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] ARC: hide unused function unw_hdr_alloc
On Wed, Jul 3, 2019 at 6:13 PM Vineet Gupta wrote:
> On 7/3/19 6:39 AM, Arnd Bergmann wrote:
> > Fixes: bc79c9a72165 ("ARC: dw2 unwind: Reinstante unwinding out of modules")
> > Link: https://kernelci.org/build/id/5d1cae3f59b514300340c132/logs/
> > Cc: sta...@vger.kernel.org
> > Signed-off-by: Arnd Bergmann
>
> Thx, do you want me to pick this up via arc tree.
Yes, that would be great.
Thanks,
Arnd
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 10/21] asm-generic: ioremap_uc should behave the same with and without MMU
On Tue, Oct 29, 2019 at 7:49 AM Christoph Hellwig wrote:
>
> Whatever reason there is for the existence of ioremap_uc, and the fact
> that it returns NULL by default on architectures with an MMU applies
> equally to nommu architectures, so don't provide different defaults.
Makes sense.
> In practice the difference is meaningless as the only portable driver
> that uses ioremap_uc is atyfb which probably doesn't show up on nommu
> devices.
> +/*
> + * ioremap_uc is special in that we do require an explicit architecture
> + * implementation. In general you do now want to use this function in a
> + * driver and use plain ioremap, which is uncached by default. Similarly
> + * architectures should not implement it unless they have a very good
> + * reason.
> + */
> +#ifndef ioremap_uc
> +#define ioremap_uc ioremap_uc
> +static inline void __iomem *ioremap_uc(phys_addr_t offset, size_t size)
> +{
> + return NULL;
> +}
> +#endif
Maybe we could move the definition into the atyfb driver itself?
As I understand it, the difference between ioremap()/ioremap_nocache()
and ioremap_uc() only exists on pre-PAT x86-32 systems (i.e. 486, P5,
Ppro, PII, K6, VIA C3), while on more modern systems (all non-x86,
PentiumIII, Athlon, VIA C7) those three are meant to be synonyms
anyway.
Arnd
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 12/21] arch: rely on asm-generic/io.h for default ioremap_* definitions
On Tue, Oct 29, 2019 at 7:49 AM Christoph Hellwig wrote: > > Various architectures that use asm-generic/io.h still defined their > own default versions of ioremap_nocache, ioremap_wt and ioremap_wc > that point back to plain ioremap directly or indirectly. Remove these > definitions and rely on asm-generic/io.h instead. For this to work > the backup ioremap_* defintions needs to be changed to purely cpp > macros instea of inlines to cover for architectures like openrisc > that only define ioremap after including . > > Signed-off-by: Christoph Hellwig Reviewed-by: Arnd Bergmann ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 17/21] lib: provide a simple generic ioremap implementation
On Tue, Oct 29, 2019 at 7:49 AM Christoph Hellwig wrote: > > A lot of architectures reuse the same simple ioremap implementation, so > start lifting the most simple variant to lib/ioremap.c. It provides > ioremap_prot and iounmap, plus a default ioremap that uses prot_noncached, > although that can be overridden by asm/io.h. > > Signed-off-by: Christoph Hellwig Reviewed-by: Arnd Bergmann ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 10/21] asm-generic: ioremap_uc should behave the same with and without MMU
On Mon, Nov 11, 2019 at 11:15 AM Christoph Hellwig wrote:
>
> On Mon, Nov 11, 2019 at 11:09:05AM +0100, Arnd Bergmann wrote:
> > Maybe we could move the definition into the atyfb driver itself?
> >
> > As I understand it, the difference between ioremap()/ioremap_nocache()
> > and ioremap_uc() only exists on pre-PAT x86-32 systems (i.e. 486, P5,
> > Ppro, PII, K6, VIA C3), while on more modern systems (all non-x86,
> > PentiumIII, Athlon, VIA C7) those three are meant to be synonyms
> > anyway.
>
> That's not how I understood it. Based on the code and the UC-
> explanation ioremap_uc always overrides the MTRR, which can still
> be present on more modern x86 systems.
As I understand, the point is that on PAT-enabled systems, the
normal ioremap() *also* overrides the MTRR, citing from
Documentation/x86/pat.rst:
=== === = =
MTRR Non-PAT PAT Linux ioremap valueEffective memory type
=== === = =
PATNon-PAT | PAT
|PCD |
||PWT |
||||
WC000 WB _PAGE_CACHE_MODE_WB WC | WC
WC001 WC _PAGE_CACHE_MODE_WC WC* | WC
WC010 UC- _PAGE_CACHE_MODE_UC_MINUS WC* | UC
WC011 UC _PAGE_CACHE_MODE_UC UC | UC
=== === = =
> In fact I remember a patch
> floating around very recently adding another ioremap_uc caller in
> some Atom platform device driver that works around buggy MTRR
> tables. Also this series actually adds a new override and a few
> callers for ia64 platform code, which works very similar to x86
> based on the comments in the code. That being said I'm not sure
> the callers in ia64 are really required, but it was the safest thing
> to do as part of this cleanup.
Ok, fair enough. Let's just go with your version for now, if only to not
hold your series up more. I'd still suggest we change atyfb to only
use ioremap_uc() on i386 and maybe ia64. I can send a patch for that.
Arnd
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 11/21] asm-generic: don't provide ioremap for CONFIG_MMU
On Tue, Oct 29, 2019 at 7:49 AM Christoph Hellwig wrote: > > All MMU-enabled ports have a non-trivial ioremap and should thus provide > the prototype for their implementation instead of providing a generic > one unless a different symbol is not defined. Note that this only > affects sparc32 nds32 as all others do provide their own version. > > Also update the kerneldoc comments in asm-generic/io.h to explain the > situation around the default ioremap* implementations correctly. > > Signed-off-by: Christoph Hellwig Reviewed-by: Arnd Bergmann ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 11/21] asm-generic: don't provide ioremap for CONFIG_MMU
On Wed, Nov 6, 2019 at 7:16 PM Geert Uytterhoeven wrote: > > Hi Palmer, > > On Wed, Nov 6, 2019 at 7:11 PM Palmer Dabbelt wrote: > > It looks like the difference in prototype between the architectures is > > between > > > > void __iomem *ioremap(resource_size_t, size_t) > > void __iomem *ioremap(phys_addr_t, size_t) > > void __iomem *ioremap(phys_addr_t, unsigned long) > > void __iomem *ioremap(unsigned long, unsigned long) > > > > shouldn't they all just be that first one? In other words, wouldn't it be > > better to always provide the generic ioremap prototype and unify the ports > > instead? > > Agreed. But I'd go for the second one. Right, phys_addr_t is the correct type here, resource_size_t is just a generic type that is at least as long as any resource, and usually the same as phys_addr_t, which is supposed to be used for physical addresses. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 01/21] arm: remove ioremap_cached
On Tue, Oct 29, 2019 at 7:48 AM Christoph Hellwig wrote: > > No users of ioremap_cached are left, remove it. > > Signed-off-by: Christoph Hellwig Reviewed-by: Arnd Bergmann ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 03/21] ia64: rename ioremap_nocache to ioremap_uc
On Tue, Oct 29, 2019 at 7:48 AM Christoph Hellwig wrote: > > On ia64 ioremap_nocache fails if attributes don't match. Not other > architectures does this, and we plan to get rid of ioremap_nocache. > So get rid of the special semantics and define ioremap_nocache in > terms of ioremap as no portable driver could rely on the behavior > anyway. > > However x86 implements ioremap_uc in a similar way as the ia64 > version of ioremap_nocache, in that it ignores the firmware tables. > Switch ia64 to override ioremap_uc instead. > > Signed-off-by: Christoph Hellwig Good idea, Reviewed-by: Arnd Bergmann ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH 10/21] asm-generic: ioremap_uc should behave the same with and without MMU
On Mon, Nov 11, 2019 at 11:29 AM Christoph Hellwig wrote: > > On Mon, Nov 11, 2019 at 11:27:27AM +0100, Arnd Bergmann wrote: > > Ok, fair enough. Let's just go with your version for now, if only to not > > hold your series up more. I'd still suggest we change atyfb to only > > use ioremap_uc() on i386 and maybe ia64. I can send a patch for that. > > I don't think we even need it on ia64. But lets kick off a dicussion > with the atyfb, x86 and ia64 maintainers after this series is in. > Which was kinda my plan anyway. I missed your reply and already sent my patch now. I guess it doesn't hurt to discuss that in parallel. Anyway I think that this patch is the last one you want an Ack from me for (let me know if I missed one), so Reviewed-by: Arnd Bergmann ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [RFT 03/13] sh: Constify ioreadX() iomem argument (as in generic implementation)
On Tue, Jan 7, 2020 at 5:54 PM Krzysztof Kozlowski wrote: > > The ioreadX() helpers have inconsistent interface. On some architectures > void *__iomem address argument is a pointer to const, on some not. > > Implementations of ioreadX() do not modify the memory under the address > so they can be converted to a "const" version for const-safety and > consistency among architectures. > > Signed-off-by: Krzysztof Kozlowski The patch looks good, but I think this has to be done together with the powerpc version and the header that declares the function, for bisectibility. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [RFT 06/13] arc: Constify ioreadX() iomem argument (as in generic implementation)
On Tue, Jan 7, 2020 at 5:54 PM Krzysztof Kozlowski wrote: > > The ioreadX() helpers have inconsistent interface. On some architectures > void *__iomem address argument is a pointer to const, on some not. > > Implementations of ioreadX() do not modify the memory under the > address so they can be converted to a "const" version for const-safety > and consistency among architectures. > > Signed-off-by: Krzysztof Kozlowski The patch looks correct, but I would not bother here, as it has no effect on the compiler or its output. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [RFT 00/13] iomap: Constify ioreadX() iomem argument
On Wed, Jan 8, 2020 at 9:36 AM Christophe Leroy wrote:
> Le 08/01/2020 à 09:18, Krzysztof Kozlowski a écrit :
> > On Wed, 8 Jan 2020 at 09:13, Geert Uytterhoeven
> > wrote:
> > I'll add to this one also changes to ioreadX_rep() and add another
> > patch for volatile for reads and writes. I guess your review will be
> > appreciated once more because of ioreadX_rep()
> >
>
> volatile should really only be used where deemed necessary:
>
> https://www.kernel.org/doc/html/latest/process/volatile-considered-harmful.html
>
> It is said: " ... accessor functions might use volatile on
> architectures where direct I/O memory access does work. Essentially,
> each accessor call becomes a little critical section on its own and
> ensures that the access happens as expected by the programmer."
The I/O accessors are one of the few places in which 'volatile' generally
makes sense, at least for the implementations that do a plain pointer
dereference (probably none of the ones in question here).
In case of readl/writel, this is what we do in asm-generic:
static inline u32 __raw_readl(const volatile void __iomem *addr)
{
return *(const volatile u32 __force *)addr;
}
The __force-cast that removes the __iomem here also means that
the 'volatile' keyword could be dropped from the argument list,
as it has no real effect any more, but then there are a few drivers
that mark their iomem pointers as either 'volatile void __iomem*' or
(worse) 'volatile void *', so we keep it in the argument list to not
add warnings for those drivers.
It may be time to change these drivers to not use volatile for __iomem
pointers, but that seems out of scope for what Krzysztof is trying
to do. Ideally we would be consistent here though, either using volatile
all the time or never.
Arnd
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [RFT 00/13] iomap: Constify ioreadX() iomem argument
On Wed, Jan 8, 2020 at 10:15 AM Krzysztof Kozlowski wrote: > > The __force-cast that removes the __iomem here also means that > > the 'volatile' keyword could be dropped from the argument list, > > as it has no real effect any more, but then there are a few drivers > > that mark their iomem pointers as either 'volatile void __iomem*' or > > (worse) 'volatile void *', so we keep it in the argument list to not > > add warnings for those drivers. > > > > It may be time to change these drivers to not use volatile for __iomem > > pointers, but that seems out of scope for what Krzysztof is trying > > to do. Ideally we would be consistent here though, either using volatile > > all the time or never. > > Indeed. I guess there are no objections around const so let me send v2 > for const only. Ok, sounds good. Maybe mention in the changelog then that the 'volatile' in the interface is intentionally left out, and that only users of readl/writel still have it to deal with existing drivers. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2 1/9] iomap: Constify ioreadX() iomem argument (as in generic implementation)
On Wed, Jan 8, 2020 at 9:05 PM Krzysztof Kozlowski wrote: > > The ioreadX() and ioreadX_rep() helpers have inconsistent interface. On > some architectures void *__iomem address argument is a pointer to const, > on some not. > > Implementations of ioreadX() do not modify the memory under the address > so they can be converted to a "const" version for const-safety and > consistency among architectures. > > Suggested-by: Geert Uytterhoeven > Signed-off-by: Krzysztof Kozlowski > Reviewed-by: Geert Uytterhoeven Thanks for getting this done! Reviewed-by: Arnd Bergmann > Changes since v1: > 1. Constify also ioreadX_rep() and mmio_insX(), > 2. Squash lib+alpha+powerpc+parisc+sh into one patch for bisectability, The alpha and parisc versions should be independent, I was thinking you leave them as separate patches, but this work for me too. I have no real opinion on the other 8 patches, I would have left them out completely, but they don't hurt either. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [RFC 4/4] ARC: uaccess: use optimized generic __strnlen_user/__strncpy_from_user
On Tue, Jan 14, 2020 at 9:08 PM Vineet Gupta wrote:
> diff --git a/arch/arc/include/asm/word-at-a-time.h
> b/arch/arc/include/asm/word-at-a-time.h
> new file mode 100644
> index ..00e92be70987
> --- /dev/null
> +++ b/arch/arc/include/asm/word-at-a-time.h
> @@ -0,0 +1,49 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2020 Synopsys Inc.
> + */
> +#ifndef __ASM_ARC_WORD_AT_A_TIME_H
> +#define __ASM_ARC_WORD_AT_A_TIME_H
> +
> +#ifdef __LITTLE_ENDIAN__
> +
> +#include
> +
> +struct word_at_a_time {
> + const unsigned long one_bits, high_bits;
> +};
What's wrong with the generic version on little-endian? Any
chance you can find a way to make it work as well for you as
this copy?
> +static inline unsigned long find_zero(unsigned long mask)
> +{
> +#ifdef CONFIG_64BIT
> + return fls64(mask) >> 3;
> +#else
> + return fls(mask) >> 3;
> +#endif
The CONFIG_64BIT check not be needed, unless you are adding
support for 64-bit ARC really soon.
Arnd
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [RFC 1/4] asm-generic/uaccess: don't define inline functions if noinline lib/* in use
On Tue, Jan 14, 2020 at 9:08 PM Vineet Gupta wrote: > > There are 2 generic varaints of strncpy_from_user() / strnlen_user() > (1). inline version in asm-generic/uaccess.h > (2). optimized word-at-a-time version in lib/* > > This patch disables #1 if #2 selected. This allows arches to continue > reusing asm-generic/uaccess.h for rest of code > > This came up when switching ARC to generic word-at-a-time interface > > Signed-off-by: Vineet Gupta This looks like a useful change, but I think we can do even better: It seems that there are no callers of __strnlen_user or __strncpy_from_user in the kernel today, so these should not be defined either when the Kconfig symbols are set. Also, I would suggest moving the 'extern' declaration for the two functions into the #else branch of the conditional so it does not need to be duplicated. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [RFC 1/4] asm-generic/uaccess: don't define inline functions if noinline lib/* in use
On Tue, Jan 14, 2020 at 10:33 PM Linus Torvalds wrote: > > On Tue, Jan 14, 2020 at 12:09 PM Vineet Gupta > wrote: > > > > There are 2 generic varaints of strncpy_from_user() / strnlen_user() > > (1). inline version in asm-generic/uaccess.h > > I think we should get rid of this entirely. It's just a buggy garbage > implementation that nobody should ever actually use. > > It does just about everything wrong that you *can* do, wrong, > including doing the NUL-filling termination of standard strncpy() that > "strncpy_from_user()" doesn't actually do. > > So: > > - the asm-generic/uaccess.h __strncpy_from_user() function is just > horribly wrong I checked who is actually using it, and the only ones I found are c6x and rv32-nommu. It shouldn't be hard to move them over to the generic version. > - the generic/uaccess.h version of strncpy_from_user() shouldn't be > an inline function either, since the only thing it can do inline is > the bogus one-byte access check that _barely_ makes security work (you > also need to have a guard page to _actually_ make it work, and I'm not > atr all convinced that people do). That would be arc, hexagon, unicore32, and um. Hexagon already has the same bug in strncpy_from_user and should be converted to the generic version as you say. For unicore32 the existing asm imlpementation may be fine, but it's clearly easier to use the generic code than moving the range check in there. I don't know what the arch/um implementation needs, but since it's in C, moving the access_ok() in there is easy enough. > I would suggest that anybody who uses asm-generic/uaccess.h needs to > simply use the generic library version. Or possibly just everybody altogether: the remaining architectures that have a custom implementation don't seem to be doing any better either. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [RFC 1/4] asm-generic/uaccess: don't define inline functions if noinline lib/* in use
On Wed, Jan 15, 2020 at 3:12 PM Al Viro wrote: > > On Wed, Jan 15, 2020 at 10:08:31AM +0100, Arnd Bergmann wrote: > > > > I would suggest that anybody who uses asm-generic/uaccess.h needs to > > > simply use the generic library version. > > > > Or possibly just everybody altogether: the remaining architectures that > > have a custom implementation don't seem to be doing any better either. > > No go for s390. There you really want to access userland memory in > larger chunks - it's oriented for block transfers. IIRC, the insn > they are using has a costly setup phase, independent of the amount > to copy, followed by reasonably fast transfer more or less linear > by the size. Right, I missed that one while looking through the remaining implementations. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [RFC 1/4] asm-generic/uaccess: don't define inline functions if noinline lib/* in use
On Thu, Jan 16, 2020 at 12:02 AM Vineet Gupta wrote: > On 1/14/20 12:57 PM, Arnd Bergmann wrote: > > On Tue, Jan 14, 2020 at 9:08 PM Vineet Gupta > > wrote: > >> There are 2 generic varaints of strncpy_from_user() / strnlen_user() > >> (1). inline version in asm-generic/uaccess.h > >> (2). optimized word-at-a-time version in lib/* > >> > >> This patch disables #1 if #2 selected. This allows arches to continue > >> reusing asm-generic/uaccess.h for rest of code > >> > >> This came up when switching ARC to generic word-at-a-time interface > >> > >> Signed-off-by: Vineet Gupta > > This looks like a useful change, but I think we can do even better: It > > seems that > > there are no callers of __strnlen_user or __strncpy_from_user in the > > kernel today, so these should not be defined either when the Kconfig symbols > > are set. Also, I would suggest moving the 'extern' declaration for the two > > functions into the #else branch of the conditional so it does not need to be > > duplicated. > > Given where things seem to be heading, do u still want an updated patch for > this > or do u plan to ditch the inline version in short term anyways. I'm trying to come up with a cleanup series now that I'll send you. You can base on top of that if you like. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [RFC v6 07/23] RISC-V: Use 64-bit time_t and off_t for RV32 and RV64
On Wed, Feb 12, 2020 at 2:42 AM Vineet Gupta wrote: > On 2/11/20 4:14 PM, Alistair Francis wrote: > > On Tue, Feb 11, 2020 at 4:14 PM Vineet Gupta wrote: > > >>> +/* Same for ino_t and ino64_t. */ > >>> +# define __INO_T_MATCHES_INO64_T 1 > > I'm surprised that ARC port doesn't define this in glibc, yet we use the > asm-generic syscall interface where this is true. I need to investigate more. All 32-bit kernels supported by glibc today define the 64-bit file offset types (__off64_t, __ino64_t, ...) and a lot of them never had the old 'long' types (__off_t, __ino_t, ...), but applications can still pick between the two ABIs when compiling against glibc, see /usr/include/fcntl.h: #ifndef __off_t_defined # ifndef __USE_FILE_OFFSET64 typedef __off_t off_t; # else typedef __off64_t off_t; # endif # define __off_t_defined #endif If you use the old types, glibc will do the conversion in the syscall wrappers on architectures that only have the 64-bit interfaces. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [RFC v6 07/23] RISC-V: Use 64-bit time_t and off_t for RV32 and RV64
On Thu, Feb 20, 2020 at 1:46 AM Joseph Myers wrote: > > On Thu, 20 Feb 2020, Vineet Gupta wrote: > > > The first 4 will need more work as sym aliasing like > > strong_alias (__xstat64, __xstat) > > > > will be needed in those ARM files (which in turn use i386). > > The situation for Arm is fundamentally different from that for ARC. > > For ARC, you only need a single public stat structure (using 64-bit times > and offsets). > > For Arm, a third public stat structure will need to be added alongside the > existing two, initially used internally in 64-bit-time stat functions that > aren't exported from glibc, eventually to be used with _TIME_BITS=64 with > the 64-bit-time stat interfaces exported once all the _TIME_BITS=64 > interfaces are ready. But surely that structure layout would be the same on ARM and ARC as well as all other 32-bit architectures with _TIME_BITS=64, right? What's wrong with having a single implementation for the most recent set of stat syscalls, with the older variants being only compiled for architectures that need them to support _TIME_BITS=32 and/or _FILE_OFFSET_BITS=32? Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: switching ARC to 64-bit time_t (Re: [RFC v6 07/23] RISC-V: Use 64-bit time_t and off_t for RV32 and RV64)
On Thu, Feb 20, 2020 at 12:11 AM Lukasz Majewski wrote: > > On 2/14/20 2:39 PM, Alistair Francis wrote: > > > On Tue, Feb 11, 2020 at 5:30 PM Joseph Myers > > An the reason this all works on RISCV is that your kernel doesn't > > define __ARCH_WANT_STAT64 -> lacks __NR_statat64 and instead uses the > > statx call which does itemized copy and would work fine when copying > > from 32-bits time (in kernel) to 64-bits container in glibc. Is this > > is right understanding or am I missing something here. > > > > How do I build a latest RISCV 32-bit kernel + userland - do you have > > a buildroot branch somewhere that I can build / test with qemu ? > > Maybe a bit off topic - there is such QEMU and Yocto/OE based test > sandbox for ARM32: > > https://github.com/lmajewski/meta-y2038 > > (the README provides steps for setup). (continuing off-topic, with debian-arm and Helmut on Cc) Would it be possible to take a snapshot of your glibc tree and start testing this out with debian-rebootstrap [1]? Are there any glibc issues that prevent it from working correctly, aside from the exact ABI not being final yet? Arnd [1] https://wiki.debian.org/HelmutGrohne/rebootstrap ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: switching ARC to 64-bit time_t (Re: [RFC v6 07/23] RISC-V: Use 64-bit time_t and off_t for RV32 and RV64)
On Thu, Feb 20, 2020 at 10:37 AM Lukasz Majewski wrote: > > On Thu, Feb 20, 2020 at 12:11 AM Lukasz Majewski > > > > Would it be possible to take a snapshot of your glibc tree > > The description of the status of Y2038 supporting glibc on ARM 32 can > be found here [1]. > > The most recent patches for Y2038 supporting glibc can be always found > in the 'y2038_edge' branch [2]. Ok. > > and start testing this out with debian-rebootstrap [1]? > > I've been using OE/Yocto for testing as it allows building glibc > sources for x86_64, x86, x86-x32, arm32 (probably also for ppc32 and > mips - but not tested). >... > However, I did not yet tried debian-rebootstrap. I will look if this > can be reused as well. The reason I'm asking about debian-rebootstrap is less about testing glibc itself than about testing the rest of user space to figure out better what needs to be done when rebuilding with _TIME_BITS=64, and to start fixing more upstream packages, with the hope of having enough of it done in time for the Debian 11 release. > > Are there any glibc issues that prevent it from working correctly, > > I think that the glibc wrappers for most important syscalls are now > converted. > > What is missing: > > - NTPL (threads) > - stat Do you mean that code using these will fail to work correctly with -D_TIME_BITS=64 at the moment, or that the interfaces are there but they are not y2038 safe? Without pthreads or stat, we probably wouldn't get too far in rebootstrap, but if the interfaces are there and mostly work, then we don't need to rely on them being y2038-safe just yet. An obvious next step would be to run the resulting code with the RTC set 20 years ahead, and that requires it all to work. > - In-glibc test coverage when -D_TIME_BITS=64 is used. I do have > some basic tests [4], but this may be not enough. This is probably something where debian-rebootstrap could help, as building and testing more user space packages will excercise additional code paths in glibc as well. There is also some work in Linaro to ensure that LTP tests the low-level syscall interfaces in both the time32 and time64 variants. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: switching ARC to 64-bit time_t (Re: [RFC v6 07/23] RISC-V: Use 64-bit time_t and off_t for RV32 and RV64)
On Thu, Feb 20, 2020 at 2:15 PM Lukasz Majewski wrote: > > On Thu, Feb 20, 2020 at 10:37 AM Lukasz Majewski > > wrote: > > > > On Thu, Feb 20, 2020 at 12:11 AM Lukasz Majewski > > > > Are there any glibc issues that prevent it from working > > > > correctly, > > > > > > I think that the glibc wrappers for most important syscalls are now > > > converted. > > > > > > What is missing: > > > > > > - NTPL (threads) > > > - stat > > > > Do you mean that code using these will fail to work correctly with > > -D_TIME_BITS=64 at the moment, or that the interfaces are there > > but they are not y2038 safe? > > For ARM32 (branch [2]): > > - Without -D_TIME_BITS=64 defined during compilation (as we do have > now) the glibc is fully functional, but when you set date after > 03:14:08 UTC on 19.01.2038 you will see the date reset (to 1901) in > the user space programs (after calling e.g. 'date'). I'd actually consider intentionally breaking this for a Debian bootstrap, at least initially, so that any application that accidentally gets built without -D_TIME_BITS=64 runs into a build or link failure. > - With -D_TIME_BITS=64 set during compilation - and using branch [2] - > syscalls listed in [1] will provide correct date after Y2038 32 bit > overflow. Other (i.e. not converted ones) will use overflow date (1901 > year). The glibc will also be fully functional up till Y2038 overflow. Ok. > > > - In-glibc test coverage when -D_TIME_BITS=64 is used. I do have > > > some basic tests [4], but this may be not enough. > > > > This is probably something where debian-rebootstrap could help, > > as building and testing more user space packages will excercise > > additional code paths in glibc as well. > > Yes this _could_ help. Do you have any tutorial/howto similar to one > from [4]? Not sure, maybe Helmut has some references. > > There is also some work > > in Linaro to ensure that LTP tests the low-level syscall interfaces > > in both the time32 and time64 variants. > > Interesting. Is this work now publicly available? I think this is currently in the planning stage, but once patches are available, they would be posted to the ltp mailing list. Viresh should have more details on this. Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: switching ARC to 64-bit time_t (Re: [RFC v6 07/23] RISC-V: Use 64-bit time_t and off_t for RV32 and RV64)
On Thu, Feb 20, 2020 at 4:42 PM Lukasz Majewski wrote: > > On Thu, Feb 20, 2020 at 2:15 PM Lukasz Majewski wrote: > > I do see two approaches here: > > 1. In glibc: > > When -D_TIME_BITS=64 is set - redirections are enabled for syscall > wrappers; for example __clock_settime64 is used instead of > __clock_settime (e.g. sysdeps/unix/sysv/linux/clock_settime). > > The latter is guarded by #ifdef __TIMESIZE != 64 so we could change > mechanically that __clock_settime returns -1 and sets errno to -ENOTSUPP What I meant is to remove the __clock_settime function from the library entirely to cause a link failure. I suppose replacing most "__TIMESIZE != 64" with '0' would do that. Ideally I'd just set __TIMESIZE to 64, but doing that would make the ABI incompatible with mainline glibc. > 2. In kernel - return -ENOTSUPP when clock_settime syscall instead of > clock_settime64 is invoked. We already have that with CONFIG_COMPAT_32BIT_TIME, but at the moment I think that still breaks glibc's usage of __NR_futex, and a compile-time error is always better than a runtime error, as it's easier to catch them reliably Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: switching ARC to 64-bit time_t (Re: [RFC v6 07/23] RISC-V: Use 64-bit time_t and off_t for RV32 and RV64)
On Thu, Feb 20, 2020 at 10:37 AM Lukasz Majewski wrote: > [2] - https://github.com/lmajewski/y2038_glibc/commits/y2038_edge I tried packaging this into a .deb package for use with rebootstrap, but ran into a couple of test failures from glibc itself, when testing the i386 version while running on an older x86_64 kernel (4.15): +-+ | Encountered regressions that don't match expected failures. | +-+ FAIL: elf/check-localplt FAIL: nptl/tst-cancel19 FAIL: rt/tst-mqueue2 make: *** [debian/rules.d/build.mk:116: /home/arnd/glibc-2.31/stamp-dir/check_i386] Error 1 elf/check-localplt complains about the newly added symbols Extra PLT reference: libc.so: __lutimes64 Extra PLT reference: libc.so: __wait4_time64 Extra PLT reference: libc.so: __setitimer64 Extra PLT reference: libc.so: __utime64 Extra PLT reference: libc.so: __timerfd_gettime64 Extra PLT reference: libc.so: __clock_settime64 Extra PLT reference: libc.so: __utimes64 Extra PLT reference: libc.so: __gettimeofday64 Extra PLT reference: libc.so: __clock_gettime64 Extra PLT reference: libc.so: __futimesat64 Extra PLT reference: libc.so: __clock_getres64 Extra PLT reference: libc.so: __futimes64 Extra PLT reference: libc.so: __futimens64 Extra PLT reference: libc.so: __utimensat64 Extra PLT reference: libc.so: __getrusage64 Extra PLT reference: libc.so: __timespec_get64 Extra PLT reference: libc.so: __getitimer64 Extra PLT reference: libc.so: __ppoll64 Extra PLT reference: libc.so: __timerfd_settime64 Extra PLT reference: libc.so: __clock_nanosleep_time64 Extra PLT reference: libc.so: __sched_rr_get_interval64 Extra PLT reference: libc.so: __settimeofday64 Extra PLT reference: librt.so: __timer_gettime64 Extra PLT reference: librt.so: __mq_timedreceive_time64 Extra PLT reference: librt.so: __mq_timedsend_time64 Extra PLT reference: librt.so: __timer_settime64 which seems a bug in the test suite. The other two get a segfault that I have not debugged, but I guess this is likely a problem in your patches. Have you seen the same thing? Arnd ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
