* Re: [PATCH 11/11] netlink: add documentation for memory mapped I/O
From: Patrick McHardy @ 2012-08-22 22:19 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: Florian.Westphal, netdev, netfilter-devel
In-Reply-To: <alpine.LNX.2.01.1208201224430.8114@frira.zrqbmnf.qr>
On Mon, 20 Aug 2012, Jan Engelhardt wrote:
> On Monday 2012-08-20 08:18, Patrick McHardy wrote:
>> +
>> +RX and TX rings
>> +----------------
>> +
>> +Each ring contains a number of continous memory blocks, containing frames of
>> +fixed size dependant on the parameters used for ring setup.
>
> dependent
> [...]
All fixed, thanks.
^ permalink raw reply
* Re: [PATCH 09/18] netfilter: ipv6: add IPv6 NAT support
From: Patrick McHardy @ 2012-08-22 22:15 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: netfilter-devel, netdev
In-Reply-To: <alpine.LNX.2.01.1208201100340.6101@frira.zrqbmnf.qr>
On Mon, 20 Aug 2012, Jan Engelhardt wrote:
> On Monday 2012-08-20 05:39, Patrick McHardy wrote:
>> +static struct nf_hook_ops nf_nat_ipv6_ops[] __read_mostly = {
>> + /* Before packet filtering, change destination */
>> + {
>> + .hook = nf_nat_ipv6_in,
>> + .owner = THIS_MODULE,
>> + .pf = NFPROTO_IPV6,
>> + .hooknum = NF_INET_PRE_ROUTING,
>> + .priority = NF_IP_PRI_NAT_DST,
>
> NF_IP6_PRI_NAT_DST
>
>> + .hook = nf_nat_ipv6_out,
>> + .owner = THIS_MODULE,
>> + .pf = NFPROTO_IPV6,
>> + .hooknum = NF_INET_POST_ROUTING,
>> + .priority = NF_IP_PRI_NAT_SRC,
>
> IP6 too... (2 more occurrences)
Fixed, thanks.
>> +static void nf_nat_ipv6_csum_recalc(struct sk_buff *skb,
>> + u8 proto, void *data, __sum16 *check,
>> + int datalen, int oldlen)
>> +{
>> + const struct ipv6hdr *ipv6h = ipv6_hdr(skb);
>> + struct rt6_info *rt = (struct rt6_info *)skb_dst(skb);
>> +
>> + if (skb->ip_summed != CHECKSUM_PARTIAL) {
>
> Maybe invert to == CHECKSUM_PARTIAL like in p06/18.
I didn't change the other patch.
^ permalink raw reply
* Re: [PATCH 06/18] netfilter: add protocol independant NAT core
From: Patrick McHardy @ 2012-08-22 22:13 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: netfilter-devel, netdev
In-Reply-To: <alpine.LNX.2.01.1208201011480.6101@frira.zrqbmnf.qr>
On Mon, 20 Aug 2012, Jan Engelhardt wrote:
> On Monday 2012-08-20 05:39, Patrick McHardy wrote:
>>
>> enum ctattr_nat {
>> CTA_NAT_UNSPEC,
>> - CTA_NAT_MINIP,
>> - CTA_NAT_MAXIP,
>> + CTA_NAT_V4_MINIP,
>> +#define CTA_NAT_MINIP CTA_NAT_V4_MINIP
>> + CTA_NAT_V4_MAXIP,
>> +#define CTA_NAT_MAXIP CTA_NAT_V4_MAXIP
>> CTA_NAT_PROTO,
>> __CTA_NAT_MAX
>> };
>
> One could also
>
> enum ctattr_nat {
> ...
> __CTA_NAT_MAX,
>
> CTA_NAT_MINIP = CTA_NAT_V4_MINIP,
> CTA_NAT_MAXIP = CTA_NAT_V4_MAXIP,
> };
>
> to provide the old names.
Sure. Doesn't really matter since defines are not used consistently
(in which case you could use them for #ifdefs).
>> diff --git a/net/ipv4/netfilter/Kconfig b/net/ipv4/netfilter/Kconfig
>> index fcc543c..33372a1 100644
>> --- a/net/ipv4/netfilter/Kconfig
>> +++ b/net/ipv4/netfilter/Kconfig
> [...]
>>
>> -config NF_NAT_NEEDED
>> - bool
>> - depends on NF_NAT
>> - default y
>> -
>
> Could add "if NF_NAT_IPV4".."endif" block as appropriate around here,
> to save on all the extra "depend on NF_NAT_IPV4" clauses.
Agreed.
>> +static int nf_nat_ipv4_in_range(const struct nf_conntrack_tuple *t,
>> + const struct nf_nat_range *range)
>> +{
>> + return ntohl(t->src.u3.ip) >= ntohl(range->min_addr.ip) &&
>> + ntohl(t->src.u3.ip) <= ntohl(range->max_addr.ip);
>> +}
>
> static bool ..
Changed.
>> +static bool nf_nat_ipv4_manip_pkt(struct sk_buff *skb,
>> + unsigned int iphdroff,
>> + const struct nf_nat_l4proto *l4proto,
>> + const struct nf_conntrack_tuple *target,
>> + enum nf_nat_manip_type maniptype)
>> +{
>> + struct iphdr *iph;
>> + unsigned int hdroff;
>> +
>> + if (!skb_make_writable(skb, iphdroff + sizeof(*iph)))
>> + return false;
>> +
>> + iph = (void *)skb->data + iphdroff;
>
> Is iph = ip_hdr(skb), hdroff = iphdroff+skb_iphdrlen(iph) not usable here?
No, we're also translating the inner packets in ICMP error messages.
>> + hdroff = iphdroff + iph->ihl * 4;
>> +
>> + if (!l4proto->manip_pkt(skb, &nf_nat_l3proto_ipv4, iphdroff, hdroff,
>> + target, maniptype))
>> + return false;
>> + iph = (void *)skb->data + iphdroff;
>
> Is trying to avoid some GNU extensions a worthwhile goal? If so,
> iph = (struct iphdr *)(skb->data + iphdroff) should be used, like in:
I don't get your point.
>> +static void nf_nat_ipv4_csum_update(struct sk_buff *skb,
>> + unsigned int iphdroff, __sum16 *check,
>> + const struct nf_conntrack_tuple *t,
>> + enum nf_nat_manip_type maniptype)
>> +{
>> + struct iphdr *iph = (struct iphdr *)(skb->data + iphdroff);
>> [...]
>> +}
>
>
>
>> +static void nf_nat_ipv4_csum_recalc(struct sk_buff *skb,
>> + u8 proto, void *data, __sum16 *check,
>> + int datalen, int oldlen)
>> +{
>> + const struct iphdr *iph = ip_hdr(skb);
>> + struct rtable *rt = skb_rtable(skb);
>> +
>> + if (skb->ip_summed != CHECKSUM_PARTIAL) {
>> + if (!(rt->rt_flags & RTCF_LOCAL) &&
>> + (!skb->dev || skb->dev->features & NETIF_F_V4_CSUM)) {
>> + skb->ip_summed = CHECKSUM_PARTIAL;
>> + skb->csum_start = skb_headroom(skb) +
>> + skb_network_offset(skb) +
>> + ip_hdrlen(skb);
>> + skb->csum_offset = (void *)check - data;
>> + *check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
>> + datalen, proto, 0);
>> + } else {
>> + *check = 0;
>> + *check = csum_tcpudp_magic(iph->saddr, iph->daddr,
>> + datalen, proto,
>> + csum_partial(data, datalen,
>> + 0));
>> + if (proto == IPPROTO_UDP && !*check)
>> + *check = CSUM_MANGLED_0;
>> + }
>> + } else
>> + inet_proto_csum_replace2(check, skb,
>> + htons(oldlen), htons(datalen), 1);
>> +}
>
> Here is a style factory trick: invert the condition such that the
> simple case is first, and the big one becomes an else if
> with a reduced indent:
This is existing code, I don't want to bloat the diff by unnecessarily
rearranging it.
>> +static void __exit nf_nat_l3proto_ipv4_exit(void)
>> +{
>> + nf_nat_l3proto_unregister(&nf_nat_l3proto_ipv4);
>> + nf_nat_l4proto_unregister(NFPROTO_IPV4, &nf_nat_l4proto_icmp);
>> +}
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_ALIAS("nf-nat-" __stringify(AF_INET));
>
> Technically, this would have to be NFPROTO_IPV4, though GNU C has yet
> to gain an extension to stringify enum constants..
I'm aware of that.
>> + /* 1) If this srcip/proto/src-proto-part is currently mapped,
>> + * and that same mapping gives a unique tuple within the given
>> + * range, use that.
>> + *
>> + * This is only required for source (ie. NAT/masq) mappings.
>> + * So far, we don't do local source mappings, so multiple
>> + * manips not an issue.
> manips are not an issue.
>
>
>> - /* nf_conntrack_alter_reply might re-allocate extension area */
>> + /* nf_conntrack_alter_reply might re-allocate exntension aera */
>
> extension was correct :)
Fixed, thanks.
>> + .target = xt_snat_target_v1,
>> + .targetsize = sizeof(struct nf_nat_range),
>> + .table = "nat",
>> + .hooks = (1 << NF_INET_POST_ROUTING) |
>> + (1 << NF_INET_LOCAL_OUT),
>> + .me = THIS_MODULE,
>
> .family = NFPROTO_UNSPEC,
>
> Just for completeness.
This is obvious.
^ permalink raw reply
* [PATCH] netdev/phy: add MDIO bus multiplexer driven by a memory-mapped FPGA
From: Timur Tabi @ 2012-08-22 21:45 UTC (permalink / raw)
To: Andy Fleming, David Miller, netdev, david.daney
An FPGA controls which sub-bus is connected to the master MDIO bus. The
FPGA must be memory-mapped and contain only 8-bit registers (which keeps
things simple).
Tested on a Freescale P5020DS board which uses the "PIXIS" FPGA attached
to the localbus.
Signed-off-by: Timur Tabi <timur@freescale.com>
---
.../devicetree/bindings/net/mdio-mux-fpga.txt | 74 ++++++++
drivers/net/phy/Kconfig | 13 ++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/mdio-mux-fpga.c | 186 ++++++++++++++++++++
4 files changed, 274 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/mdio-mux-fpga.txt
create mode 100644 drivers/net/phy/mdio-mux-fpga.c
diff --git a/Documentation/devicetree/bindings/net/mdio-mux-fpga.txt b/Documentation/devicetree/bindings/net/mdio-mux-fpga.txt
new file mode 100644
index 0000000..ef567c6
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/mdio-mux-fpga.txt
@@ -0,0 +1,74 @@
+Properties for an MDIO bus multiplexer/switch controlled by an FPGA register.
+
+This is a special case of a MDIO bus multiplexer. An FPGA register is used
+to control which child bus is connected.
+
+Required properties in addition to the generic multiplexer properties:
+
+- compatible : string, must contain "mdio-mux-fpga"
+
+- mdio-mux-device : phandle, points to the FPGA (or similar) node. This
+ must be a memory-mapped device with 8-bit registers.
+
+- mdio-mux-register : integer, contains the offset of the register that
+ controls the bus multiplexer.
+
+- mdio-mux-mask : integer, contains an 8-bit mask that specifies which
+ bits in the register control the actual bus multiplexer. The
+ 'reg' property of each child mdio-mux node must be constrained by
+ this mask.
+
+Example:
+
+The FPGA node defines a memory-mapped FPGA with a register space of 0x30 bytes.
+For the "EMI2" MDIO bus, register 9 (BRDCFG1) controls the mux on that bus.
+A bitmask of 0x6 means that bits 1 and 2 (bit 0 is lsb) are the bits on
+BRDCFG1 that control the actual mux.
+
+ /* The FPGA node */
+ fpga: board-control@3,0 {
+ compatible = "fsl,p5020ds-fpga", "fsl,fpga-ngpixis";
+ reg = <3 0 0x30>;
+ };
+
+ /* The parent MDIO bus. */
+ xmdio0: mdio@f1000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "fsl,fman-xmdio";
+ reg = <0xf1000 0x1000>;
+ interrupts = <100 1 0 0>;
+ };
+
+ mdio-mux-emi2 {
+ compatible = "mdio-mux-fpga", "mdio-mux";
+ mdio-parent-bus = <&xmdio0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ mdio-mux-device = <&fpga>;
+ mdio-mux-register = <9>; // BRDCFG1
+ mdio-mux-mask = <0x6>; // EMI2
+
+ emi2_slot1: mdio@0 { // Slot 1 XAUI (FM2)
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phy_xgmii_slot1: ethernet-phy@0 {
+ compatible = "ethernet-phy-ieee802.3-c45";
+ reg = <4>;
+ };
+ };
+
+ emi2_slot2: mdio@2 { // Slot 2 XAUI (FM1)
+ reg = <2>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ phy_xgmii_slot2: ethernet-phy@4 {
+ compatible = "ethernet-phy-ieee802.3-c45";
+ reg = <0>;
+ };
+ };
+ };
+
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 3090dc6..c3fc957 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -159,6 +159,19 @@ config MDIO_BUS_MUX_GPIO
several child MDIO busses to a parent bus. Child bus
selection is under the control of GPIO lines.
+config MDIO_BUS_MUX_FPGA
+ tristate "Support for FPGA-controlled MDIO bus multiplexers"
+ depends on OF_MDIO
+ select MDIO_BUS_MUX
+ help
+ This module provides a driver for MDIO bus multiplexers that
+ are controlled via a simple memory-mapped FPGA device. The
+ multiplexer connects one of several child MDIO busses to a parent
+ bus. Child bus selection is under the control of one of the
+ FPGA's registers.
+
+ Currently, only 8-bit registers are supported.
+
endif # PHYLIB
config MICREL_KS8995MA
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 6d2dc6c..3bf4d7a 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -28,3 +28,4 @@ obj-$(CONFIG_MICREL_KS8995MA) += spi_ks8995.o
obj-$(CONFIG_AMD_PHY) += amd.o
obj-$(CONFIG_MDIO_BUS_MUX) += mdio-mux.o
obj-$(CONFIG_MDIO_BUS_MUX_GPIO) += mdio-mux-gpio.o
+obj-$(CONFIG_MDIO_BUS_MUX_FPGA) += mdio-mux-fpga.o
diff --git a/drivers/net/phy/mdio-mux-fpga.c b/drivers/net/phy/mdio-mux-fpga.c
new file mode 100644
index 0000000..7b4e69c
--- /dev/null
+++ b/drivers/net/phy/mdio-mux-fpga.c
@@ -0,0 +1,186 @@
+/*
+ * FPGA MDIO MUX driver
+ *
+ * This driver supports
+ * Author: Timur Tabi <timur@freescale.com>
+ *
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2. This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#include <linux/platform_device.h>
+#include <linux/device.h>
+#include <linux/of_mdio.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/phy.h>
+#include <linux/mdio-mux.h>
+
+struct mdio_mux_fpga_state {
+ void *mux_handle;
+ phys_addr_t phys;
+ unsigned int offset;
+ uint8_t mask;
+};
+
+/*
+ * MDIO multiplexing switch function
+ *
+ * This function is called by the mdio-mux layer when it thinks the mdio bus
+ * multiplexer needs to switch.
+ *
+ * 'current_child' is the current value of the mux register (masked via
+ * s->mask).
+ *
+ * 'desired_child' is the value of the 'reg' property of the target child MDIO
+ * node.
+ *
+ * The first time this function is called, current_child == -1.
+ *
+ * If current_child == desired_child, then the mux is already set to the
+ * correct bus.
+ */
+static int mdio_mux_fpga_switch_fn(int current_child, int desired_child,
+ void *data)
+{
+ struct mdio_mux_fpga_state *s = data;
+
+ if (current_child ^ desired_child) {
+ void *p = ioremap(s->phys + s->offset, 1);
+ uint8_t x;
+
+ if (!p)
+ return -ENOMEM;
+
+ x = ioread8(p);
+ iowrite8((x & ~s->mask) | desired_child, p);
+
+ iounmap(p);
+ }
+
+ return 0;
+}
+
+static int __devinit mdio_mux_fpga_probe(struct platform_device *pdev)
+{
+ struct device_node *np2, *np = pdev->dev.of_node;
+ struct mdio_mux_fpga_state *s;
+ struct resource res;
+ const __be32 *iprop;
+ int len, ret;
+
+ dev_dbg(&pdev->dev, "probing node %s\n", np->full_name);
+
+ s = devm_kzalloc(&pdev->dev, sizeof(*s), GFP_KERNEL);
+ if (!s)
+ return -ENOMEM;
+
+ iprop = of_get_property(np, "mdio-mux-device", &len);
+ if (!iprop || len != sizeof(phandle)) {
+ dev_err(&pdev->dev, "missing mdio-mux-device property\n");
+ return -ENODEV;
+ }
+ np2 = of_find_node_by_phandle(be32_to_cpup(iprop));
+ if (!np2) {
+ dev_err(&pdev->dev, "mdio-mux-device points to invalid node\n");
+ return -ENODEV;
+ }
+
+ ret = of_address_to_resource(np2, 0, &res);
+ if (ret) {
+ dev_err(&pdev->dev, "cannot obtain memory map for node %s\n",
+ np2->full_name);
+ return ret;
+ }
+ s->phys = res.start;
+
+ iprop = of_get_property(np, "mdio-mux-register", &len);
+ if (!iprop || len != sizeof(uint32_t)) {
+ dev_err(&pdev->dev, "missing mdio-mux-register property\n");
+ return -EINVAL;
+ }
+ s->offset = be32_to_cpup(iprop);
+ if (s->offset >= resource_size(&res)) {
+ dev_err(&pdev->dev, "mdio-mux-register value %u is too large\n",
+ s->offset);
+ return -EINVAL;
+ }
+
+ iprop = of_get_property(np, "mdio-mux-mask", &len);
+ if (!iprop || len != sizeof(uint32_t)) {
+ dev_err(&pdev->dev, "missing mdio-mux-mask property\n");
+ return -ENODEV;
+ }
+ if (be32_to_cpup(iprop) > 255) {
+ dev_err(&pdev->dev, "only 8-bit registers are supported\n");
+ return -EINVAL;
+ }
+ s->mask = be32_to_cpup(iprop);
+
+ /*
+ * Verify that the 'reg' property of each child MDIO bus does not
+ * set any bits outside of the 'mask'.
+ */
+ for_each_available_child_of_node(np, np2) {
+ iprop = of_get_property(np2, "reg", &len);
+ if (!iprop || len != sizeof(uint32_t)) {
+ dev_err(&pdev->dev, "mdio-mux child node %s is "
+ "missing a 'reg' property\n", np2->full_name);
+ return -ENODEV;
+ }
+ if (be32_to_cpup(iprop) & ~s->mask) {
+ dev_err(&pdev->dev, "mdio-mux child node %s has "
+ "a 'reg' value with unmasked bits\n",
+ np2->full_name);
+ return -ENODEV;
+ }
+ }
+
+ ret = mdio_mux_init(&pdev->dev, mdio_mux_fpga_switch_fn,
+ &s->mux_handle, s);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to register mdio-mux bus %s\n",
+ np->full_name);
+ return ret;
+ }
+
+ pdev->dev.platform_data = s;
+
+ return 0;
+}
+
+static int __devexit mdio_mux_fpga_remove(struct platform_device *pdev)
+{
+ struct mdio_mux_fpga_state *s = dev_get_platdata(&pdev->dev);
+
+ mdio_mux_uninit(s->mux_handle);
+
+ return 0;
+}
+
+static struct of_device_id mdio_mux_fpga_match[] = {
+ {
+ .compatible = "mdio-mux-fpga",
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mdio_mux_fpga_match);
+
+static struct platform_driver mdio_mux_fpga_driver = {
+ .driver = {
+ .name = "mdio-mux-fpga",
+ .owner = THIS_MODULE,
+ .of_match_table = mdio_mux_fpga_match,
+ },
+ .probe = mdio_mux_fpga_probe,
+ .remove = __devexit_p(mdio_mux_fpga_remove),
+};
+
+module_platform_driver(mdio_mux_fpga_driver);
+
+MODULE_AUTHOR("Timur Tabi <timur@freescale.com>");
+MODULE_DESCRIPTION("FPGA MDIO MUX driver");
+MODULE_LICENSE("GPL v2");
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH 00/18] netfilter: IPv6 NAT
From: David Miller @ 2012-08-22 21:42 UTC (permalink / raw)
To: kaber; +Cc: netfilter-devel, netdev
In-Reply-To: <Pine.GSO.4.63.1208222322060.25423@stinky-local.trash.net>
From: Patrick McHardy <kaber@trash.net>
Date: Wed, 22 Aug 2012 23:23:53 +0200 (MEST)
> The NAT patches depend on a couple of fixes in Pablo's latest
> submission, if you could merge net.git into net-next.git, I can push
> them to Pablo without creating conflicts. Thanks!
I just did that right now, should show up on kernel.org shortly.
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: David Miller @ 2012-08-22 21:38 UTC (permalink / raw)
To: torvalds; +Cc: hpa, bhutchings, tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <CA+55aFxxSFMiohu80BZCObA0APPe08h1-7eAZ_BLoAZDWqqv0Q@mail.gmail.com>
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Wed, 22 Aug 2012 14:28:50 -0700
> On Wed, Aug 22, 2012 at 2:14 PM, David Miller <davem@davemloft.net> wrote:
>>
>> BTW, just to clarify, I'm not saying that we should save the FPU on
>> every trap where we find the FPU enabled or anything stupid like that.
>>
>> Definitely keep the kern_fpu_begin()/kern_fpu_end() type markers
>> around FPU usage, but allow some kind of nesting facility.
>
> So nesting shouldn't be horrible, but the thing that really screws
> with people like the crypto use is not nesting, but the fact that
> sometimes you can't save at all, and the whole "kernel_fpu_possible()"
> or whatever we call the checking function.
>
> IOW, in [soft]irq context, to avoid races with the irq happening as
> the process is going to do something with the FPU state, we don't
> allow saving and changing state, because that would mean that the
> normal FP state paths would have to be irq-safe, and they aren't.
>
> And once you have to have that fpu possible check, if it happens to
> also disallow nested use, I doubt that's going to really affect
> anybody. The code has to take the case of "I'm not allowed to change
> FPU state" case into account regardless.
I don't think you really have to do anything special to handle
interrupts properly.
Let's assume that we use some variable length save area at the end of
thread_info to do this nested saving.
When you are asked for FPU usage, you first figure out how much you're
going to save.
Then you advance the allocation pointer in the thread_info, and save
into the space you allocated.
If an interrupt wants to use the FPU, that should be fine as well.
Whether the interrupt FPU save does it's save after you did, or
before, it should work out fine.
I suppose you might have some issues in determining whether we need to
do the full fxsave stuff or not. There could be a state bit for that,
or similar.
Another idea, instead of doing this in thread_info, is to do it on the
local stack. That way if we're in an interrupt, we'll use that
interrupt type's kernel stack.
You might be able to get away with always doing the full FPU
save/restore in that situation.
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Linus Torvalds @ 2012-08-22 21:28 UTC (permalink / raw)
To: David Miller; +Cc: hpa, bhutchings, tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <20120822.141433.730254311852927123.davem@davemloft.net>
On Wed, Aug 22, 2012 at 2:14 PM, David Miller <davem@davemloft.net> wrote:
>
> BTW, just to clarify, I'm not saying that we should save the FPU on
> every trap where we find the FPU enabled or anything stupid like that.
>
> Definitely keep the kern_fpu_begin()/kern_fpu_end() type markers
> around FPU usage, but allow some kind of nesting facility.
So nesting shouldn't be horrible, but the thing that really screws
with people like the crypto use is not nesting, but the fact that
sometimes you can't save at all, and the whole "kernel_fpu_possible()"
or whatever we call the checking function.
IOW, in [soft]irq context, to avoid races with the irq happening as
the process is going to do something with the FPU state, we don't
allow saving and changing state, because that would mean that the
normal FP state paths would have to be irq-safe, and they aren't.
And once you have to have that fpu possible check, if it happens to
also disallow nested use, I doubt that's going to really affect
anybody. The code has to take the case of "I'm not allowed to change
FPU state" case into account regardless.
Linus
^ permalink raw reply
* Re: [net-next 0/6][pull request] Intel Wired LAN Driver Updates
From: David Miller @ 2012-08-22 21:24 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, sassmann
In-Reply-To: <1345538275-1690-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 21 Aug 2012 01:37:49 -0700
> This series contains updates to ethtool.h, e1000, e1000e, and igb to
> implement MDI/MDIx control.
>
> The following are changes since commit 1d76efe1577b4323609b1bcbfafa8b731eda071a:
> team: add support for non-ethernet devices
> and are available in the git repository at:
> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master
Pulled, thanks Jeff.
^ permalink raw reply
* Re: [PATCH 00/18] netfilter: IPv6 NAT
From: Patrick McHardy @ 2012-08-22 21:23 UTC (permalink / raw)
To: David Miller; +Cc: netfilter-devel, netdev
In-Reply-To: <20120822.022801.740821969974372752.davem@davemloft.net>
On Wed, 22 Aug 2012, David Miller wrote:
> From: Patrick McHardy <kaber@trash.net>
> Date: Mon, 20 Aug 2012 05:39:48 +0200
>
>> Following is the latest IPv6 NAT patchset, based on -rc2.
>
> Feel free to push patch #1 via the nf tree when give the
> final version of this to Pablo, and you can add my ACK to
> it as well if you like:
>
> Acked-by: David S. Miller <davem@davemloft.net>
Thanks, will do. The NAT patches depend on a couple of fixes in Pablo's
latest submission, if you could merge net.git into net-next.git, I can
push them to Pablo without creating conflicts. Thanks!
^ permalink raw reply
* Re: bonding: why zero out bond_dev->dev_addr when last slave removed?
From: Jay Vosburgh @ 2012-08-22 21:20 UTC (permalink / raw)
To: Chris Friesen; +Cc: Andy Gospodarek, netdev
In-Reply-To: <5035458C.9000804@genband.com>
Chris Friesen <chris.friesen@genband.com> wrote:
[...]
>I was wondering about the rationale for zeroing out bond_dev->dev_addr
>when the last slave is removed from the bond.
>
>Assuming bond0 is currently using the mac address of eth0 then doing
>
>ifenslave -d bond0 eth0
>ifenslave -d bond0 eth1
>ifenslave bond0 eth1
>ifenslave bond0 eth0
>
>ends up changing the MAC address of the bond link. Given that the bond
>itself stays up during this time, why don't we let the bond device keep
>it's previous MAC address (at least if fail_over_mac is zero)?
>
>Is this to account for the case where we move the bond to totally
>different devices such that the old MAC no longer belongs to one of the
>slaves but instead belongs to a NIC outside the bond?
More or less. When the last slave is removed from the bond,
several properties of the bond are reset to an initial state, one of
which is that the bond's MAC is reset to all zeroes.
If fail_over_mac is set to "active," then the bond must use the
MAC of one of its slaves, as the implication is that the slaves cannot
or should not change their MAC addresses. In this case, I believe the
bond's MAC will be set to the first active slave's MAC, so holding over
the bond's MAC makes no practical difference.
For fail_over_mac=follow, if the bond kept its MAC, then it
would be possible for both slaves to end up with the same MAC (because
the follow logic will set the first slave to the bond's MAC when it is
made active, and the second slave will keep its MAC after enslavement,
and those could be the same MAC if the second slave is the one the bond
originally got its MAC from).
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: David Miller @ 2012-08-22 21:14 UTC (permalink / raw)
To: hpa; +Cc: torvalds, bhutchings, tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <20120821.211427.1832042852041589162.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Tue, 21 Aug 2012 21:14:27 -0700 (PDT)
> From: "H. Peter Anvin" <hpa@zytor.com>
> Date: Tue, 21 Aug 2012 20:59:26 -0700
>
>> kernel_fpu_end() would still have to re-enable preemption (and
>> preemption would have to check the work flag), but that should be cheap.
>>
>> We could allow the FPU in the kernel to have preemption, if we allocated
>> space for two xstates per thread instead of one. That is, however, a
>> fair hunk of memory.
>
> Once you have done the first FPU save for the sake of the kernel, you
> can minimize what you save for any deeper nesting because the kernel
> only cares about a very limited part of that FPU state not the whole
> 1K thing.
>
> Those bits you can save by hand with a bunch of explicit stores of the
> XMM registers, or something like that.
BTW, just to clarify, I'm not saying that we should save the FPU on
every trap where we find the FPU enabled or anything stupid like that.
Definitely keep the kern_fpu_begin()/kern_fpu_end() type markers
around FPU usage, but allow some kind of nesting facility.
Here's one idea. Anyone using the existing kern_fpu_*() markers get
the existing behavior. Only one level of kernel FPU usage is allowed.
But a new interface allows specification of a state-save mask. And it
is only users of this interface for which we allow nesting past the
first FPU user.
If this is the first kernel FPU user, we always do the full fxsave or
whatever to push out the full state. For any level of kernel FPU
nesting we save only what is in the save-mask, by hand.
^ permalink raw reply
* bonding: why zero out bond_dev->dev_addr when last slave removed?
From: Chris Friesen @ 2012-08-22 20:48 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, netdev
Hi all,
I've got a couple questions about the bonding driver.
I was wondering about the rationale for zeroing out bond_dev->dev_addr
when the last slave is removed from the bond.
Assuming bond0 is currently using the mac address of eth0 then doing
ifenslave -d bond0 eth0
ifenslave -d bond0 eth1
ifenslave bond0 eth1
ifenslave bond0 eth0
ends up changing the MAC address of the bond link. Given that the bond
itself stays up during this time, why don't we let the bond device keep
it's previous MAC address (at least if fail_over_mac is zero)?
Is this to account for the case where we move the bond to totally
different devices such that the old MAC no longer belongs to one of the
slaves but instead belongs to a NIC outside the bond?
Chris
--
Chris Friesen
Software Designer
3500 Carling Avenue
Ottawa, Ontario K2H 8E9
www.genband.com
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Ben Hutchings @ 2012-08-22 19:01 UTC (permalink / raw)
To: Linus Torvalds
Cc: H. Peter Anvin, David Laight, Benjamin LaHaise, David Miller,
tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <CA+55aFyBVRHk6ZcWxEPoUPRmFYyW6yihbi26vS-SAKey0nv9yA@mail.gmail.com>
On Wed, 2012-08-22 at 11:28 -0700, Linus Torvalds wrote:
> On Wed, Aug 22, 2012 at 11:11 AM, Ben Hutchings
> <bhutchings@solarflare.com> wrote:
> >
> > Right, I think it's been made pretty clear that it's going to be
> > dependent on more than just architecture.
>
> Well, it's entirely possible that the 128-bit case will work correctly
> on all x86-64 hardware out there on PCIe.
>
> I just can't guarantee it, because we certainly have had issues with
> hw doing odd things before. But maybe PCIe really is well-specified
> enough, and maybe nobody has done a odd PCIe bridges, and maybe every
> time some 128-bit access is split, the bus in question still always
> remembers the original 128-bit size in the transaction. It's not at
> all impossible. I just wouldn't *guarantee* it.
Even then, everything works out OK if the particular MMIO write I'm
concerned about *is* split - just as long as the resulting operations
are in ascending address order. This is not true on all systems if we
enable write-combining (without a fence in the middle, which defeats the
purpose), and I think hpa was saying that it may not be the case with
SSE writes either.
> And to some degree, for high-end server-only hardware in particular,
> it really *is* acceptable to say "If you have odd hardware, odd things
> will happen". So for this particular driver, maybe the right approach
> is simply to say "we require that your fabric works right". And see if
> anybody ever complains.
Maybe. At the moment reordering tends to cause the hardware to complain
that we sent an invalid sequence of DMA descriptors, but that's only
because we're not being as smart as we could about using TX push. I
don't want to run the risk of sending out corrupted packets (with
offloaded checksums, so they're not that obviously invalid) on the wire.
Ben.
> The 100ns may be worth those kinds of "you'd better not have old/crap
> hardware" decisions. It's not acceptable for some drivers (a driver
> for some consumer ATA chip might not want to make that kind of choice,
> and say "whatever, we'll be really conservative), but "Quod licet
> Jovi, non licet bovi".
>
> The fact that something might not be *guaranteed* to always work
> doesn't necessarily mean that it is always the wrong thing to do..
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: bonding: time limits too tight in bond_ab_arp_inspect
From: Chris Friesen @ 2012-08-22 18:58 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: Jiri Bohac, Andy Gospodarek, netdev, Petr Tesarik
In-Reply-To: <24655.1345660922@death.nxdomain>
On 08/22/2012 12:42 PM, Jay Vosburgh wrote:
> Chris Friesen<chris.friesen@genband.com> wrote:
>
>> On 08/22/2012 11:45 AM, Jiri Bohac wrote:
>>
>>> This code is run from bond_activebackup_arp_mon() about
>>> delta_in_ticks jiffies after the previous ARP probe has been
>>> sent. If the delayed work gets executed exactly in delta_in_ticks
>>> jiffies, there is a chance the slave will be brought up. If the
>>> delayed work runs one jiffy later, the slave will stay down.
>
> Presumably the ARP reply is coming back in less than one jiffy,
> then, so the slave_last_rx() value is the same jiffy as when the
> _inspect was previously called?
>
>> <snip>
>>
>>> Should they perhaps all be increased by, say, delta_in_ticks/2, to make this
>>> less dependent on the current scheduling latencies?
>>
>> We have been using a patch that tracks the arpmon requested sleep time vs
>> the actual sleep time and adds any scheduling latency to the allowed
>> delta. That way if we sleep too long due to scheduling latency it doesn't
>> affect the calculation.
>
> How much scheduling latency do you see?
>
> Is that really better than just permitting a bit more slack in
> the timing window?
We hit enough latency that it triggered arpmon to falsely mark multiple
links as lost. This triggered our system maintenance code to go into a
"oh no we can't talk to the outside world" secenario, which does fairly
intrusive things to try and bring connectivity back up. Basically a bad
thing to happen just because of a random scheduler latency spike.
I should note that we added this some time back and are still running
older kernels so I have no idea what latency on modern kernels is like.
Chris
^ permalink raw reply
* Re: bonding: time limits too tight in bond_ab_arp_inspect
From: Jay Vosburgh @ 2012-08-22 18:42 UTC (permalink / raw)
To: Chris Friesen; +Cc: Jiri Bohac, Andy Gospodarek, netdev, Petr Tesarik
In-Reply-To: <50351CC5.3030109@genband.com>
Chris Friesen <chris.friesen@genband.com> wrote:
>On 08/22/2012 11:45 AM, Jiri Bohac wrote:
>
>> This code is run from bond_activebackup_arp_mon() about
>> delta_in_ticks jiffies after the previous ARP probe has been
>> sent. If the delayed work gets executed exactly in delta_in_ticks
>> jiffies, there is a chance the slave will be brought up. If the
>> delayed work runs one jiffy later, the slave will stay down.
Presumably the ARP reply is coming back in less than one jiffy,
then, so the slave_last_rx() value is the same jiffy as when the
_inspect was previously called?
><snip>
>
>> Should they perhaps all be increased by, say, delta_in_ticks/2, to make this
>> less dependent on the current scheduling latencies?
>
>We have been using a patch that tracks the arpmon requested sleep time vs
>the actual sleep time and adds any scheduling latency to the allowed
>delta. That way if we sleep too long due to scheduling latency it doesn't
>affect the calculation.
How much scheduling latency do you see?
Is that really better than just permitting a bit more slack in
the timing window?
As to the 2 * delta and 3 * delta calculations, these values
predate my involvement with bonding, so I'm not entirely sure why those
specific values were chosen (there are no log messages from that era
that I'm aware of). My presumption has been that this part:
/*
* Active slave is down if:
* - more than 2*delta since transmitting OR
* - (more than 2*delta since receive AND
* the bond has an IP address)
*/
trans_start = dev_trans_start(slave->dev);
if (bond_is_active_slave(slave) &&
(!time_in_range(jiffies,
trans_start - delta_in_ticks,
trans_start + 2 * delta_in_ticks) ||
!time_in_range(jiffies,
slave_last_rx(bond, slave) - delta_in_ticks,
slave_last_rx(bond, slave) + 2 * delta_in_ticks))) {
slave->new_link = BOND_LINK_DOWN;
commit++;
}
was structured this way (allowing 2 * delta) to permit the loss
of a single ARP on an otherwise idle interface without triggering a link
down.
My guess, though, is that until relatively recently the timing
window was not too tight, and there was effectively some slack in the
calculation, because the slave_last_rx() would be set to some small
number of jiffies after the last exection of the monitor, and so the
"slave_last_rx() + delta_in_ticks" wasn't as narrow a window as it
appears to be now.
So, without having tested this myself, based on the above, I
don't see that adding some slack would be a problem.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Linus Torvalds @ 2012-08-22 18:28 UTC (permalink / raw)
To: Ben Hutchings
Cc: H. Peter Anvin, David Laight, Benjamin LaHaise, David Miller,
tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <1345659074.2709.80.camel@bwh-desktop.uk.solarflarecom.com>
On Wed, Aug 22, 2012 at 11:11 AM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
>
> Right, I think it's been made pretty clear that it's going to be
> dependent on more than just architecture.
Well, it's entirely possible that the 128-bit case will work correctly
on all x86-64 hardware out there on PCIe.
I just can't guarantee it, because we certainly have had issues with
hw doing odd things before. But maybe PCIe really is well-specified
enough, and maybe nobody has done a odd PCIe bridges, and maybe every
time some 128-bit access is split, the bus in question still always
remembers the original 128-bit size in the transaction. It's not at
all impossible. I just wouldn't *guarantee* it.
And to some degree, for high-end server-only hardware in particular,
it really *is* acceptable to say "If you have odd hardware, odd things
will happen". So for this particular driver, maybe the right approach
is simply to say "we require that your fabric works right". And see if
anybody ever complains.
The 100ns may be worth those kinds of "you'd better not have old/crap
hardware" decisions. It's not acceptable for some drivers (a driver
for some consumer ATA chip might not want to make that kind of choice,
and say "whatever, we'll be really conservative), but "Quod licet
Jovi, non licet bovi".
The fact that something might not be *guaranteed* to always work
doesn't necessarily mean that it is always the wrong thing to do..
Linus
^ permalink raw reply
* Unusual page allocation failure (network softirq related)
From: David @ 2012-08-22 18:24 UTC (permalink / raw)
To: linux-kernel, netdev
[-- Attachment #1: Type: text/plain, Size: 5690 bytes --]
Hi
Just for information, my home server produced the following allocation
failure this morning. First time I've seen it on this box. All seems
fine though, so no symptoms.
config & dmesg attached.
Cheers
David
Aug 22 08:52:29 server kernel: [302505.069788] swapper/0: page
allocation failure: order:0, mode:0x120
Aug 22 08:52:29 server kernel: [302505.069794] Pid: 0, comm: swapper/0
Tainted: G O 3.5.2 #2
Aug 22 08:52:29 server kernel: [302505.069796] Call Trace:
Aug 22 08:52:29 server kernel: [302505.069798] <IRQ>
[<ffffffff810e15f1>] warn_alloc_failed+0xf1/0x140
Aug 22 08:52:29 server kernel: [302505.069809] [<ffffffff814c82b0>] ?
ip_rcv+0x2c0/0x2c0
Aug 22 08:52:29 server kernel: [302505.069813] [<ffffffff810e28a1>]
__alloc_pages_nodemask+0x591/0x840
Aug 22 08:52:29 server kernel: [302505.069817] [<ffffffff814e7e25>] ?
tcp4_gro_receive+0x55/0xc0
Aug 22 08:52:29 server kernel: [302505.069821] [<ffffffff8148ca1d>]
netdev_alloc_frag+0xbd/0xf0
Aug 22 08:52:29 server kernel: [302505.069823] [<ffffffff8148cad0>]
__netdev_alloc_skb+0x80/0xc0
Aug 22 08:52:29 server kernel: [302505.069842] [<ffffffffa01a9c7d>]
rtl8169_poll+0x3fd/0x670 [r8169]
Aug 22 08:52:29 server kernel: [302505.069845] [<ffffffff81496d3f>]
net_rx_action+0xef/0x230
Aug 22 08:52:29 server kernel: [302505.069849] [<ffffffff8103997f>]
__do_softirq+0xaf/0x1e0
Aug 22 08:52:29 server kernel: [302505.069854] [<ffffffff815b4c0c>]
call_softirq+0x1c/0x30
Aug 22 08:52:29 server kernel: [302505.069858] [<ffffffff81003fad>]
do_softirq+0x4d/0x80
Aug 22 08:52:29 server kernel: [302505.069860] [<ffffffff810397a5>]
irq_exit+0xb5/0xd0
Aug 22 08:52:29 server kernel: [302505.069863] [<ffffffff81003911>]
do_IRQ+0x61/0xe0
Aug 22 08:52:29 server kernel: [302505.069867] [<ffffffff815ac4a7>]
common_interrupt+0x67/0x67
Aug 22 08:52:29 server kernel: [302505.069868] <EOI>
[<ffffffff8100aa75>] ? default_idle+0x55/0x190
Aug 22 08:52:29 server kernel: [302505.069875] [<ffffffff8100ac33>]
amd_e400_idle+0x83/0x100
Aug 22 08:52:29 server kernel: [302505.069877] [<ffffffff8100a1e6>]
cpu_idle+0x86/0xd0
Aug 22 08:52:29 server kernel: [302505.069881] [<ffffffff81595338>]
rest_init+0x68/0x70
Aug 22 08:52:29 server kernel: [302505.069885] [<ffffffff81ab2cc3>]
start_kernel+0x344/0x351
Aug 22 08:52:29 server kernel: [302505.069888] [<ffffffff81ab279f>] ?
kernel_init+0x1ca/0x1ca
Aug 22 08:52:29 server kernel: [302505.069891] [<ffffffff81ab232d>]
x86_64_start_reservations+0x131/0x136
Aug 22 08:52:29 server kernel: [302505.069894] [<ffffffff81ab241f>]
x86_64_start_kernel+0xed/0xf4
Aug 22 08:52:29 server kernel: [302505.069895] Mem-Info:
Aug 22 08:52:29 server kernel: [302505.069897] DMA per-cpu:
Aug 22 08:52:29 server kernel: [302505.069899] CPU 0: hi: 0,
btch: 1 usd: 0
Aug 22 08:52:29 server kernel: [302505.069900] CPU 1: hi: 0,
btch: 1 usd: 0
Aug 22 08:52:29 server kernel: [302505.069902] DMA32 per-cpu:
Aug 22 08:52:29 server kernel: [302505.069903] CPU 0: hi: 186,
btch: 31 usd: 175
Aug 22 08:52:29 server kernel: [302505.069905] CPU 1: hi: 186,
btch: 31 usd: 31
Aug 22 08:52:29 server kernel: [302505.069909] active_anon:50844
inactive_anon:54976 isolated_anon:0
Aug 22 08:52:29 server kernel: [302505.069909] active_file:57409
inactive_file:235120 isolated_file:0
Aug 22 08:52:29 server kernel: [302505.069909] unevictable:0
dirty:74711 writeback:8193 unstable:0
Aug 22 08:52:29 server kernel: [302505.069909] free:2248
slab_reclaimable:16276 slab_unreclaimable:7333
Aug 22 08:52:29 server kernel: [302505.069909] mapped:10831 shmem:8286
pagetables:3710 bounce:0
Aug 22 08:52:29 server kernel: [302505.069917] DMA free:7004kB min:44kB
low:52kB high:64kB active_anon:0kB inactive_anon:44kB active_file:36kB
inactive_file:2216kB unevictable:0kB isolated(anon):0kB
isolated(file):0kB present:15632kB mlocked:0kB dirty:248kB writeback:0kB
mapped:4kB shmem:4kB slab_reclaimable:6528kB slab_unreclaimable:16kB
kernel_stack:8kB pagetables:0kB unstable:0kB bounce:0kB
writeback_tmp:0kB pages_scanned:0 all_unreclaimable? no
Aug 22 08:52:29 server kernel: [302505.069919] lowmem_reserve[]: 0 1747
1747 1747
Aug 22 08:52:29 server kernel: [302505.069926] DMA32 free:1988kB
min:5324kB low:6652kB high:7984kB active_anon:203376kB
inactive_anon:219860kB active_file:229600kB inactive_file:938264kB
unevictable:0kB isolated(anon):0kB isolated(file):0kB present:1789764kB
mlocked:0kB dirty:298596kB writeback:32772kB mapped:43320kB
shmem:33140kB slab_reclaimable:58576kB slab_unreclaimable:29316kB
kernel_stack:3240kB pagetables:14840kB unstable:0kB bounce:0kB
writeback_tmp:0kB pages_scanned:0 all_unreclaimable? no
Aug 22 08:52:29 server kernel: [302505.069928] lowmem_reserve[]: 0 0 0 0
Aug 22 08:52:29 server kernel: [302505.069931] DMA: 8*4kB 6*8kB 9*16kB
10*32kB 7*64kB 9*128kB 5*256kB 3*512kB 0*1024kB 1*2048kB 0*4096kB = 7008kB
Aug 22 08:52:29 server kernel: [302505.069938] DMA32: 14*4kB 11*8kB
10*16kB 11*32kB 2*64kB 1*128kB 0*256kB 2*512kB 0*1024kB 0*2048kB
0*4096kB = 1936kB
Aug 22 08:52:29 server kernel: [302505.069944] 312175 total pagecache pages
Aug 22 08:52:29 server kernel: [302505.069946] 11352 pages in swap cache
Aug 22 08:52:29 server kernel: [302505.069947] Swap cache stats: add
92803, delete 81451, find 4745898/4749655
Aug 22 08:52:29 server kernel: [302505.069949] Free swap = 3652688kB
Aug 22 08:52:29 server kernel: [302505.069950] Total swap = 3903676kB
Aug 22 08:52:29 server kernel: [302505.077625] 458624 pages RAM
Aug 22 08:52:29 server kernel: [302505.077626] 10730 pages reserved
Aug 22 08:52:29 server kernel: [302505.077627] 241182 pages shared
Aug 22 08:52:29 server kernel: [302505.077628] 233322 pages non-shared
[-- Attachment #2: config.bz2 --]
[-- Type: application/x-bzip, Size: 29749 bytes --]
[-- Attachment #3: dmesg.bz2 --]
[-- Type: application/x-bzip, Size: 16819 bytes --]
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: H. Peter Anvin @ 2012-08-22 18:18 UTC (permalink / raw)
To: Ben Hutchings
Cc: Linus Torvalds, David Laight, Benjamin LaHaise, David Miller,
tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <1345659074.2709.80.camel@bwh-desktop.uk.solarflarecom.com>
On 08/22/2012 11:11 AM, Ben Hutchings wrote:
>
> Well the whole point of having the two 32-bit generic implementations is
> that hardware may care about the order! How can it be right that a
> 64-bit implementation assumes it doesn't?
>
On x86 platforms it is pretty much universal (as in: I have never seen
an exception) that transactions that are broken up are broken up in
littleendian order. That was the original readq/writeq implementation
on x86-32, but it screwed up some other architectures.
The other reason to not have readq/writeq by default where we *know* it
can't be supported is that some drivers (e.g. the CNIC/OPA-2 driver I
mentioned) can do better, performance-wise, if it knows that.
-hpa
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Ben Hutchings @ 2012-08-22 18:11 UTC (permalink / raw)
To: Linus Torvalds
Cc: H. Peter Anvin, David Laight, Benjamin LaHaise, David Miller,
tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <CA+55aFzcqhHU_+uR33=q+49jK97e=EofFF7g7Z0K1RU_PGwMTw@mail.gmail.com>
On Wed, 2012-08-22 at 10:54 -0700, Linus Torvalds wrote:
> On Wed, Aug 22, 2012 at 10:27 AM, Ben Hutchings
> <bhutchings@solarflare.com> wrote:
> >
> > If this is right, how can it be safe to use readq/writeq at all?
>
> Pray.
>
> Or don't care about ordering: use hardware that is well-designed and
> doesn't have crap interfaces that are fragile.
Well the whole point of having the two 32-bit generic implementations is
that hardware may care about the order! How can it be right that a
64-bit implementation assumes it doesn't?
> If you care about ordering, you need to do them as two separate
> accesses, and have a fence in between. Which, quite frankly, sounds
> like the right model for you *anyway*, since then you could use
> write-combining memory and you might even go faster, despite an
> explicit fence and thus a minimum of 2 transactions.
Yes, which unfortunately is no better than we have at the moment.
> Seriously. If you care that deeply about the ordering of the bytes you
> write out, MAKE THAT ORDERING VERY EXPLICIT IN THE SOURCE CODE. Don't
> say "oh, with this hack, I win 100ns". You need to ask yourself: what
> do you care about more? Going really fast on some machine that you can
> test, or being safe?
I have to care quite a lot about both. :-) But yes, safety first.
> With PCIe, it's *probably* fine to just say "we expect 64-bit accesses
> to make it through unmolested".
I have to hope so.
> The 128-bit case I really don't know about. It probably works too. But
> while I'd call the 64-bit case almost certain (in the absence of truly
> crap hardware), the 128-bit case I have a hard time judging how
> certain it is going to be.
Right, I think it's been made pretty clear that it's going to be
dependent on more than just architecture.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH v3 04/17] workqueue: use new hashtable implementation
From: Tejun Heo @ 2012-08-22 18:05 UTC (permalink / raw)
To: Sasha Levin
Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
josh-iaAMLnmF4UmaiuxdJuQwMA, rostedt-nx8X9YLhiw1AfugRpC6u6w,
mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w,
axboe-tSWWG44O7X1aa/9Udqfwiw, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ejt-H+wXaHxf7aLQT0dZR+AlfA,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, lw-BthXqXjhjHXQFUHtdCDX3A,
teigland-H+wXaHxf7aLQT0dZR+AlfA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1345602432-27673-5-git-send-email-levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Wed, Aug 22, 2012 at 04:26:59AM +0200, Sasha Levin wrote:
> Switch workqueues to use the new hashtable implementation. This reduces the amount of
> generic unrelated code in the workqueues.
>
> Signed-off-by: Sasha Levin <levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Acked-by: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v3 01/17] hashtable: introduce a small and naive hashtable
From: Tejun Heo @ 2012-08-22 18:01 UTC (permalink / raw)
To: Sasha Levin
Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
bfields-uC3wQj2KruNg9hUCZPvPmw,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
josh-iaAMLnmF4UmaiuxdJuQwMA, rostedt-nx8X9YLhiw1AfugRpC6u6w,
mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w,
axboe-tSWWG44O7X1aa/9Udqfwiw, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ejt-H+wXaHxf7aLQT0dZR+AlfA,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, lw-BthXqXjhjHXQFUHtdCDX3A,
teigland-H+wXaHxf7aLQT0dZR+AlfA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1345602432-27673-2-git-send-email-levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Hello, Sasha.
On Wed, Aug 22, 2012 at 04:26:56AM +0200, Sasha Levin wrote:
> +#define DEFINE_HASHTABLE(name, bits) \
> + struct hlist_head name[HASH_SIZE(bits)];
Shouldn't this be something like the following?
#define DEFINE_HASHTABLE(name, bits) \
struct hlist_head name[HASH_SIZE(bits)] = \
{ [0 ... HASH_SIZE(bits) - 1] = HLIST_HEAD_INIT };
Also, given that the declaration isn't non-trivial, you'll probably
want a matching DECLARE_HASHTABLE() macro too.
> +/* Use hash_32 when possible to allow for fast 32bit hashing in 64bit kernels. */
> +#define hash_min(val, bits) ((sizeof(val)==4) ? hash_32((val), (bits)) : hash_long((val), (bits)))
Why is the branching condition sizeof(val) == 4 instead of <= 4?
Also, no biggie but why isn't this macro in caps?
> +/**
> + * hash_add_size - add an object to a hashtable
> + * @hashtable: hashtable to add to
> + * @bits: bit count used for hashing
> + * @node: the &struct hlist_node of the object to be added
> + * @key: the key of the object to be added
> + */
> +#define hash_add_size(hashtable, bits, node, key) \
> + hlist_add_head(node, &hashtable[hash_min(key, bits)]);
> +
> +/**
> + * hash_add - add an object to a hashtable
> + * @hashtable: hashtable to add to
> + * @node: the &struct hlist_node of the object to be added
> + * @key: the key of the object to be added
> + */
> +#define hash_add(hashtable, node, key) \
> + hash_add_size(hashtable, HASH_BITS(hashtable), node, key)
It would be nice if the comments actually say something which can
differentiate the two. Ditto for rcu variants.
> +/**
> + * hash_add_rcu_size - add an object to a rcu enabled hashtable
> + * @hashtable: hashtable to add to
> + * @bits: bit count used for hashing
> + * @node: the &struct hlist_node of the object to be added
> + * @key: the key of the object to be added
> + */
> +#define hash_add_rcu_size(hashtable, bits, node, key) \
> + hlist_add_head_rcu(node, &hashtable[hash_min(key, bits)]);
> +
> +/**
> + * hash_add_rcu - add an object to a rcu enabled hashtable
> + * @hashtable: hashtable to add to
> + * @node: the &struct hlist_node of the object to be added
> + * @key: the key of the object to be added
> + */
> +#define hash_add_rcu(hashtable, node, key) \
> + hash_add_rcu_size(hashtable, HASH_BITS(hashtable), node, key)
Or maybe we're better off with hash_head_size() and hash_head()? I'll
expand on it later. Please bear with me.
> +/**
> + * hash_hashed - check whether an object is in any hashtable
> + * @node: the &struct hlist_node of the object to be checked
> + */
> +#define hash_hashed(node) (!hlist_unhashed(node))
As the 'h' in hlist* stand for hash anyway and I think this type of
thin wrappers tend to obfuscate more than anything else.
> +/**
> + * hash_del - remove an object from a hashtable
> + * @node: &struct hlist_node of the object to remove
> + */
> +static inline void hash_del(struct hlist_node *node)
> +{
> + hlist_del_init(node);
> +}
> +
> +/**
> + * hash_del_rcu - remove an object from a rcu enabled hashtable
> + * @node: &struct hlist_node of the object to remove
> + */
> +static inline void hash_del_rcu(struct hlist_node *node)
> +{
> + hlist_del_init_rcu(node);
> +}
If we do that, we can remove all these thin wrappers.
> +#define hash_for_each_size(name, bits, bkt, node, obj, member) \
> + for (bkt = 0; bkt < HASH_SIZE(bits); bkt++) \
> + hlist_for_each_entry(obj, node, &name[bkt], member)
..
> +#define hash_for_each(name, bkt, node, obj, member) \
> + hash_for_each_size(name, HASH_BITS(name), bkt, node, obj, member)
...
> +#define hash_for_each_rcu_size(name, bits, bkt, node, obj, member) \
> + for (bkt = 0; bkt < HASH_SIZE(bits); bkt++) \
> + hlist_for_each_entry_rcu(obj, node, &name[bkt], member)
...
> +#define hash_for_each_rcu(name, bkt, node, obj, member) \
> + hash_for_each_rcu_size(name, HASH_BITS(name), bkt, node, obj, member)
...
> +#define hash_for_each_safe_size(name, bits, bkt, node, tmp, obj, member) \
> + for (bkt = 0; bkt < HASH_SIZE(bits); bkt++) \
> + hlist_for_each_entry_safe(obj, node, tmp, &name[bkt], member)
...
> +#define hash_for_each_safe(name, bkt, node, tmp, obj, member) \
> + hash_for_each_safe_size(name, HASH_BITS(name), bkt, node, \
> + tmp, obj, member)
...
> +#define hash_for_each_possible_size(name, obj, bits, node, member, key) \
> + hlist_for_each_entry(obj, node, &name[hash_min(key, bits)], member)
...
> +#define hash_for_each_possible(name, obj, node, member, key) \
> + hash_for_each_possible_size(name, obj, HASH_BITS(name), node, member, key)
...
> +#define hash_for_each_possible_rcu_size(name, obj, bits, node, member, key) \
> + hlist_for_each_entry_rcu(obj, node, &name[hash_min(key, bits)], member)
...
> +#define hash_for_each_possible_rcu(name, obj, node, member, key) \
> + hash_for_each_possible_rcu_size(name, obj, HASH_BITS(name), \
...
> +#define hash_for_each_possible_safe_size(name, obj, bits, node, tmp, member, key)\
> + hlist_for_each_entry_safe(obj, node, tmp, \
> + &name[hash_min(key, bits)], member)
...
> +#define hash_for_each_possible_safe(name, obj, node, tmp, member, key) \
> + hash_for_each_possible_safe_size(name, obj, HASH_BITS(name), \
And also all these. We'd only need hash_for_each_head() and
hash_head(). hash_for_each_possible*() could be nice for convenience,
I suppose.
I think the almost trivial nature of hlist hashtables makes this a bit
tricky and I'm not very sure but having this combinatory explosion is
a bit dazzling when the same functionality can be achieved by simply
combining operations which are already defined and named considering
hashtable. I'm not feeling too strong about this tho. What do others
think?
Also, can you please audit the comments on top of each macro? They
have wrong names and don't differentiate the different variants very
well.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH 2/3] x86_64: Define 128-bit memory-mapped I/O operations
From: Linus Torvalds @ 2012-08-22 17:54 UTC (permalink / raw)
To: Ben Hutchings
Cc: H. Peter Anvin, David Laight, Benjamin LaHaise, David Miller,
tglx, mingo, netdev, linux-net-drivers, x86
In-Reply-To: <1345656446.2709.65.camel@bwh-desktop.uk.solarflarecom.com>
On Wed, Aug 22, 2012 at 10:27 AM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
>
> If this is right, how can it be safe to use readq/writeq at all?
Pray.
Or don't care about ordering: use hardware that is well-designed and
doesn't have crap interfaces that are fragile.
If you care about ordering, you need to do them as two separate
accesses, and have a fence in between. Which, quite frankly, sounds
like the right model for you *anyway*, since then you could use
write-combining memory and you might even go faster, despite an
explicit fence and thus a minimum of 2 transactions.
Seriously. If you care that deeply about the ordering of the bytes you
write out, MAKE THAT ORDERING VERY EXPLICIT IN THE SOURCE CODE. Don't
say "oh, with this hack, I win 100ns". You need to ask yourself: what
do you care about more? Going really fast on some machine that you can
test, or being safe?
With PCIe, it's *probably* fine to just say "we expect 64-bit accesses
to make it through unmolested".
The 128-bit case I really don't know about. It probably works too. But
while I'd call the 64-bit case almost certain (in the absence of truly
crap hardware), the 128-bit case I have a hard time judging how
certain it is going to be.
Linus
^ permalink raw reply
* Re: bonding: time limits too tight in bond_ab_arp_inspect
From: Chris Friesen @ 2012-08-22 17:54 UTC (permalink / raw)
To: Jiri Bohac; +Cc: Jay Vosburgh, Andy Gospodarek, netdev, Petr Tesarik
In-Reply-To: <20120822174534.GA20260@midget.suse.cz>
On 08/22/2012 11:45 AM, Jiri Bohac wrote:
> This code is run from bond_activebackup_arp_mon() about
> delta_in_ticks jiffies after the previous ARP probe has been
> sent. If the delayed work gets executed exactly in delta_in_ticks
> jiffies, there is a chance the slave will be brought up. If the
> delayed work runs one jiffy later, the slave will stay down.
<snip>
> Should they perhaps all be increased by, say, delta_in_ticks/2, to make this
> less dependent on the current scheduling latencies?
We have been using a patch that tracks the arpmon requested sleep time
vs the actual sleep time and adds any scheduling latency to the allowed
delta. That way if we sleep too long due to scheduling latency it
doesn't affect the calculation.
Chris
^ permalink raw reply
* bonding: time limits too tight in bond_ab_arp_inspect
From: Jiri Bohac @ 2012-08-22 17:45 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, netdev; +Cc: Petr Tesarik
Hi,
a customer reported that a bonding slave did not come back up
after setting their link down and then up again. ARP monitoring +
arp_validate were used.
Petr has tracked the problem down to the time comaprisons in
bond_ab_arp_inspect().
if (slave->link != BOND_LINK_UP) {
if (time_in_range(jiffies,
slave_last_rx(bond, slave) - delta_in_ticks,
slave_last_rx(bond, slave) + delta_in_ticks)) {
slave->new_link = BOND_LINK_UP;
commit++;
}
continue;
}
This code is run from bond_activebackup_arp_mon() about
delta_in_ticks jiffies after the previous ARP probe has been
sent. If the delayed work gets executed exactly in delta_in_ticks
jiffies, there is a chance the slave will be brought up. If the
delayed work runs one jiffy later, the slave will stay down.
With arp_validate this is more noticeable, since traffic other than the
bonding-generated ARP probes does not update the slave_last_rx timestamp.
A simple patch will fix this case.
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3001,7 +3001,7 @@ static int bond_ab_arp_inspect(struct bo
if (slave->link != BOND_LINK_UP) {
if (time_in_range(jiffies,
slave_last_rx(bond, slave) - delta_in_ticks,
- slave_last_rx(bond, slave) + delta_in_ticks)) {
+ slave_last_rx(bond, slave) + 2 * delta_in_ticks)) {
slave->new_link = BOND_LINK_UP;
commit++;
The remaining time comparisons inside bond_ab_arp_inspect() have larger
tolerances (3*delta_in_ticks or 2*delta_in_ticks), but it still seems strange
that the precision of delayed work scheduling should steal a full
arp_interval from the time limits.
What is the intention of e.g. the "3*delta since last receive" limit?
Was this really meant to be "as little as 2*delta + 1 jiffy"?
Should they perhaps all be increased by, say, delta_in_ticks/2, to make this
less dependent on the current scheduling latencies?
Thoughts?
--
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, SUSE CZ
^ permalink raw reply
* Re: [PATCH v3 13/17] lockd: use new hashtable implementation
From: Sasha Levin @ 2012-08-22 17:32 UTC (permalink / raw)
To: Mathieu Desnoyers
Cc: snitzer-H+wXaHxf7aLQT0dZR+AlfA, neilb-l3A5Bk7waGM,
fweisbec-Re5JQEeQqe8AvxtiuMwx3w,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA, J. Bruce Fields,
paul.gortmaker-CWA4WttNNZF54TAoqtyWWQ,
dm-devel-H+wXaHxf7aLQT0dZR+AlfA, agk-H+wXaHxf7aLQT0dZR+AlfA,
aarcange-H+wXaHxf7aLQT0dZR+AlfA, rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
venkat.x.venkatsubra-QHcLZuEGTsvQT0dZR+AlfA,
ccaulfie-H+wXaHxf7aLQT0dZR+AlfA, mingo-X9Un+BFzKDI,
dev-yBygre7rU0TnMu66kgdUjQ, ericvh-Re5JQEeQqe8AvxtiuMwx3w,
josh-iaAMLnmF4UmaiuxdJuQwMA, rostedt-nx8X9YLhiw1AfugRpC6u6w,
lw-BthXqXjhjHXQFUHtdCDX3A, teigland-H+wXaHxf7aLQT0dZR+AlfA,
axboe-tSWWG44O7X1aa/9Udqfwiw, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
edumazet-hpIqsD4AKlfQT0dZR+AlfA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, ejt-H+wXaHxf7aLQT0dZR+AlfA,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, tj-DgEjT+Ai2ygdnm+yROfE0A,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20120822132243.GA2844@Krystal>
On 08/22/2012 03:22 PM, Mathieu Desnoyers wrote:
> * Sasha Levin (levinsasha928-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote:
>> On 08/22/2012 01:47 PM, J. Bruce Fields wrote:
>>> On Wed, Aug 22, 2012 at 04:27:08AM +0200, Sasha Levin wrote:
>>>> +static int __init nlm_init(void)
>>>> +{
>>>> + hash_init(nlm_files);
>>>> + return 0;
>>>> +}
>>>> +
>>>> +module_init(nlm_init);
>>>
>>> That's giving me:
>>>
>>> fs/lockd/svcsubs.o: In function `nlm_init':
>>> /home/bfields/linux-2.6/fs/lockd/svcsubs.c:454: multiple definition of `init_module'
>>> fs/lockd/svc.o:/home/bfields/linux-2.6/fs/lockd/svc.c:606: first defined here
>>> make[2]: *** [fs/lockd/lockd.o] Error 1
>>> make[1]: *** [fs/lockd] Error 2
>>> make[1]: *** Waiting for unfinished jobs....
>>
>> I tested this entire patch set both with linux-next and Linus' latest master,
>> and it worked fine in both places.
>>
>> Is it possible that lockd has a -next tree which isn't pulled into linux-next?
>> (there's nothing listed in MAINTAINERS that I could see).
>
> fs/lockd/Makefile:
>
> obj-$(CONFIG_LOCKD) += lockd.o
>
> lockd-objs-y := clntlock.o clntproc.o clntxdr.o host.o svc.o svclock.o \
> svcshare.o svcproc.o svcsubs.o mon.o xdr.o grace.o
>
> your patch adds a module_init to svcsubs.c.
> However, there is already one in svc.c, pulled into the same module.
>
> in your test build, is CONFIG_LOCKD defined as "m" or "y" ? You should
> always test both.
>
> One solution here is to create a "local" init function in svcsubs.c and
> expose it to svc.c, so the latter can call it from its module init
> function.
Ah yes, it was on =y and I didn't notice :/
I'll fix that.
> Thanks,
>
> Mathieu
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox