* Re: DMA allocations from CMA and fatal_signal_pending check
From: Michal Nazarewicz @ 2014-11-03 16:45 UTC (permalink / raw)
To: Florian Fainelli, Joonsoo Kim
Cc: linux-arm-kernel, Brian Norris, Gregory Fong, linux-kernel,
linux-mm, lauraa, gioh.kim, aneesh.kumar, m.szyprowski, akpm,
netdev@vger.kernel.org
In-Reply-To: <5453F80C.4090006@gmail.com>
On Fri, Oct 31 2014, Florian Fainelli wrote:
> I agree that the CMA allocation should not be allowed to succeed, but
> the dma_alloc_coherent() allocation should succeed. If we look at the
> sysport driver, there are kmalloc() calls to initialize private
> structures, those will succeed (except under high memory pressure), so
> by the same token, a driver expects DMA allocations to succeed (unless
> we are under high memory pressure)
>
> What are we trying to solve exactly with the fatal_signal_pending()
> check here? Are we just optimizing for the case where a process has
> allocated from a CMA region to allow this region to be returned to the
> pool of free pages when it gets killed? Could there be another mechanism
> used to reclaim those pages if we know the process is getting killed
> anyway?
We're guarding against situations where process may hang around
arbitrarily long time after receiving SIGKILL. If user does “kill -9
$pid” the usual expectation is that the $pid process will die within
seconds and anything longer is perceived by user as a bug.
What problem are *you* trying to solve? If user sent SIGKILL to
a process that imitated device initialisation, what is the point of
continuing initialising the device? Just recover and return -EINTR.
> Well, not really. This driver is not an isolated case, there are tons of
> other networking drivers that do exactly the same thing, and we do
> expect these dma_alloc_* calls to succeed.
Again, why do you expect them to succeed? The code must handle failures
correctly anyway so why do you wish to ignore fatal signal?
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michał “mina86” Nazarewicz (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 7/7] can: m_can: workaround for transmit data less than 4 bytes
From: Marc Kleine-Budde @ 2014-11-03 16:48 UTC (permalink / raw)
To: Dong Aisheng, linux-can
Cc: wg, varkabhadram, netdev, socketcan, linux-arm-kernel
In-Reply-To: <1414579527-31100-7-git-send-email-b29396@freescale.com>
[-- Attachment #1: Type: text/plain, Size: 2685 bytes --]
On 10/29/2014 11:45 AM, Dong Aisheng wrote:
> We meet an IC issue that we have to write the full 8 bytes (whatever
> value for the second word) in Message RAM to avoid bit error for transmit
> data less than 4 bytes.
Is this a SoC or a m_can problem? Are all versions of the SoC/m_can
affected? Is there a m_can version register somewhere?
> Without the workaround, we can easily see the following errors:
> root@imx6qdlsolo:~# ip link set can0 up type can bitrate 1000000
> [ 66.882520] IPv6: ADDRCONF(NETDEV_CHANGE): can0: link becomes ready
> root@imx6qdlsolo:~# cansend can0 123#112233
> [ 66.935640] m_can 20e8000.can can0: Bit Error Uncorrected
>
> Signed-off-by: Dong Aisheng <b29396@freescale.com>
> ---
> drivers/net/can/m_can/m_can.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 219e0e3..f2d9ebe 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -1058,10 +1058,19 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,
> m_can_fifo_write(priv, 0, M_CAN_FIFO_ID, id);
> m_can_fifo_write(priv, 0, M_CAN_FIFO_DLC, can_len2dlc(cf->len) << 16);
>
> - for (i = 0; i < cf->len; i += 4)
> + for (i = 0; i < cf->len; i += 4) {
> m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(i / 4),
> *(u32 *)(cf->data + i));
>
> + /* FIXME: we meet an IC issue that we have to write the full 8
FIXME usually indicates that the driver needs some work here. Just
describe your hardware bug, you might add a reference to an errata if
available, though.
> + * bytes (whatever value for the second word) in Message RAM to
> + * avoid bit error for transmit data less than 4 bytes
> + */
> + if (cf->len <= 4)
> + m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(i / 4 + 1),
> + 0x0);
This workaround doesn't handle the dlc == 0 case, your error description
isn't completely if this is a problem, too.
It should be possible to change the for loop to go always to 8, or
simply unroll the loop:
/* errata description goes here */
m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(0), *(u32 *)(cf->data + 0));
m_can_fifo_write(priv, 0, M_CAN_FIFO_DATA(1), *(u32 *)(cf->data + 4));
> + }
> +
> can_put_echo_skb(skb, dev, 0);
>
> if (priv->can.ctrlmode & CAN_CTRLMODE_FD) {
>
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 3/7] can: m_can: add .ndo_change_mtu function
From: Marc Kleine-Budde @ 2014-11-03 16:49 UTC (permalink / raw)
To: Dong Aisheng, linux-can
Cc: wg, varkabhadram, netdev, socketcan, linux-arm-kernel
In-Reply-To: <1414579527-31100-3-git-send-email-b29396@freescale.com>
[-- Attachment #1: Type: text/plain, Size: 462 bytes --]
On 10/29/2014 11:45 AM, Dong Aisheng wrote:
> Use common can_change_mtu function.
>
> Signed-off-by: Dong Aisheng <b29396@freescale.com>
Applied to can/master.
Thanks,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 4/7] can: m_can: add a bit delay after setting CCCR_INIT bit
From: Marc Kleine-Budde @ 2014-11-03 16:52 UTC (permalink / raw)
To: Dong Aisheng, linux-can
Cc: wg, varkabhadram, netdev, socketcan, linux-arm-kernel
In-Reply-To: <1414579527-31100-4-git-send-email-b29396@freescale.com>
[-- Attachment #1: Type: text/plain, Size: 825 bytes --]
On 10/29/2014 11:45 AM, Dong Aisheng wrote:
> The spec mentions there may be a delay until the value written to
> INIT can be read back due to the synchronization mechanism between the
> two clock domains. But it does not indicate the exact clock cycles needed.
> The 5us delay is a test value and seems ok.
>
> Without the delay, CCCR.CCE bit may fail to be set and then the
> initialization fail sometimes when do repeatly up and down.
>
> Signed-off-by: Dong Aisheng <b29396@freescale.com>
Applied to can/master.
Thanks,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 1/7] can: m_can: fix possible sleep in napi poll
From: Marc Kleine-Budde @ 2014-11-03 17:02 UTC (permalink / raw)
To: Dong Aisheng, linux-can
Cc: wg, varkabhadram, netdev, socketcan, linux-arm-kernel
In-Reply-To: <1414579527-31100-1-git-send-email-b29396@freescale.com>
[-- Attachment #1: Type: text/plain, Size: 529 bytes --]
Hey Dong Aisheng,
the state of the patches are:
1: can/master + stable
2: can/mster
3: can/mster
4: can/master + stable
5: waiting for Oliver's opinion due to the user space change
6: waiting for your input
7: waiting for your input
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2] PPC: bpf_jit_comp: add SKF_AD_PKTTYPE instruction
From: David Miller @ 2014-11-03 17:06 UTC (permalink / raw)
To: kda; +Cc: netdev, linuxppc-dev, alexei.starovoitov, mpe, matt
In-Reply-To: <1414649535-3956-1-git-send-email-kda@linux-powerpc.org>
From: Denis Kirjanov <kda@linux-powerpc.org>
Date: Thu, 30 Oct 2014 09:12:15 +0300
> Add BPF extension SKF_AD_PKTTYPE to ppc JIT to load
> skb->pkt_type field.
>
> Before:
> [ 88.262622] test_bpf: #11 LD_IND_NET 86 97 99 PASS
> [ 88.265740] test_bpf: #12 LD_PKTTYPE 109 107 PASS
>
> After:
> [ 80.605964] test_bpf: #11 LD_IND_NET 44 40 39 PASS
> [ 80.607370] test_bpf: #12 LD_PKTTYPE 9 9 PASS
>
> CC: Alexei Starovoitov<alexei.starovoitov@gmail.com>
> CC: Michael Ellerman<mpe@ellerman.id.au>
> Cc: Matt Evans <matt@ozlabs.org>
> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
>
> v2: Added test rusults
So, can I apply this now?
^ permalink raw reply
* Re: [PATCH net-next 3/7] gue: Add infrastructure for flags and options
From: David Miller @ 2014-11-03 17:18 UTC (permalink / raw)
To: therbert; +Cc: netdev
In-Reply-To: <1414882683-25484-4-git-send-email-therbert@google.com>
From: Tom Herbert <therbert@google.com>
Date: Sat, 1 Nov 2014 15:57:59 -0700
> @@ -20,7 +20,16 @@ static size_t fou_encap_hlen(struct ip_tunnel_encap *e)
>
> static size_t gue_encap_hlen(struct ip_tunnel_encap *e)
> {
> - return sizeof(struct udphdr) + sizeof(struct guehdr);
> + size_t len;
> + bool need_priv = false;
> +
> + len = sizeof(struct udphdr) + sizeof(struct guehdr);
> +
> + /* Add in lengths flags */
> +
> + len += need_priv ? GUE_LEN_PRIV : 0;
Add this need_priv logic in patch #6, not here.
^ permalink raw reply
* Re: [PATCH bluetooth-next] netdevice: add ieee802154_ptr to net_device
From: David Miller @ 2014-11-03 17:20 UTC (permalink / raw)
To: alex.aring-Re5JQEeQqe8AvxtiuMwx3w
Cc: linux-wpan-u79uwXL29TY76Z2rM5mHXA, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1414907094-9623-1-git-send-email-alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
From: Alexander Aring <alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Sun, 2 Nov 2014 06:44:54 +0100
> This patch adds an ieee802154_ptr to the net_device structure.
> Furthermore the 802.15.4 subsystem will introduce a nl802154 framework
> which is similar like the nl80211 framework and a wpan_dev structure.
> The wpan_dev structure will hold additional net_device attributes like
> address options which are 802.15.4 specific. In the upcoming nl802154
> implementation we will introduce a NL802154_FLAG_NEED_WPAN_DEV like
> NL80211_FLAG_NEED_WDEV. For this flag an ieee802154_ptr in net_device is
> needed. Additional we can access the wpan_dev attributes in upper layers
> like IEEE 802.15.4 6LoWPAN easily. Current solution is a complicated
> callback interface and getting these values over subif data structure
> in mac802154.
>
> Signed-off-by: Alexander Aring <alex.aring-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
No objections, and you can merge this via the bluetooth tree:
Acked-by: David S. Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] PPC: bpf_jit_comp: add SKF_AD_PKTTYPE instruction
From: Alexei Starovoitov @ 2014-11-03 17:21 UTC (permalink / raw)
To: David Miller, felix
Cc: Denis Kirjanov, netdev@vger.kernel.org, linuxppc-dev,
Michael Ellerman, Matt Evans
In-Reply-To: <20141103.120633.183870310578884991.davem@davemloft.net>
On Mon, Nov 3, 2014 at 9:06 AM, David Miller <davem@davemloft.net> wrote:
> From: Denis Kirjanov <kda@linux-powerpc.org>
> Date: Thu, 30 Oct 2014 09:12:15 +0300
>
>> Add BPF extension SKF_AD_PKTTYPE to ppc JIT to load
>> skb->pkt_type field.
>>
>> Before:
>> [ 88.262622] test_bpf: #11 LD_IND_NET 86 97 99 PASS
>> [ 88.265740] test_bpf: #12 LD_PKTTYPE 109 107 PASS
>>
>> After:
>> [ 80.605964] test_bpf: #11 LD_IND_NET 44 40 39 PASS
>> [ 80.607370] test_bpf: #12 LD_PKTTYPE 9 9 PASS
>>
>> CC: Alexei Starovoitov<alexei.starovoitov@gmail.com>
>> CC: Michael Ellerman<mpe@ellerman.id.au>
>> Cc: Matt Evans <matt@ozlabs.org>
>> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
>>
>> v2: Added test rusults
>
> So, can I apply this now?
I think this question is more towards ppc folks,
since both Daniel and myself said before that it looks ok.
Philippe just tested the previous version of this patch on ppc64le...
I'm guessing that Matt (original author of bpf jit for ppc) is not replying,
because he has no objections.
Either way the addition is tiny and contained, so can go in now.
^ permalink raw reply
* Re: [stable request <= 3.11] net/mlx4_en: Fix BlueFlame race
From: Cong Wang @ 2014-11-03 17:22 UTC (permalink / raw)
To: David Miller
Cc: Ben Hutchings, Vinson Lee, Amir Vadai, Or Gerlitz,
Jack Morgenstein, Eugenia Emantayev, Matan Barak, netdev
In-Reply-To: <20141101.134142.888127846627939710.davem@davemloft.net>
On Sat, Nov 1, 2014 at 10:41 AM, David Miller <davem@davemloft.net> wrote:
>
> There is no documented way nor do I wish to state anything so strictly.
> I want maximum flexibility for such a time consuming task.
>
> I tend to go back 3 or 4 releases at most, and it really depends upon
> the difficulty of the backports and my own time constraints.
You should really offload to developers, otherwise too much work for you. :)
Thanks!
^ permalink raw reply
* [PATCHv1 net-next] xen-netback: remove unconditional pull_skb_tail in guest Tx path
From: David Vrabel @ 2014-11-03 17:23 UTC (permalink / raw)
To: netdev; +Cc: David Vrabel, xen-devel, Ian Campbell, Wei Liu, Malcolm Crossley
From: Malcolm Crossley <malcolm.crossley@citrix.com>
Unconditionally pulling 128 bytes into the linear buffer is not
required. Netback has already grant copied up-to 128 bytes from the
first slot of a packet into the linear buffer. The first slot normally
contain all the IPv4/IPv6 and TCP/UDP headers.
The unconditional pull would often copy frag data unnecessarily. This
is a performance problem when running on a version of Xen where grant
unmap avoids TLB flushes for pages which are not accessed. TLB
flushes can now be avoided for > 99% of unmaps (it was 0% before).
Grant unmap TLB flush avoidance will be available in a future version
of Xen (probably 4.6).
Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com>
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
drivers/net/xen-netback/netback.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 730252c..14e18bb 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -82,6 +82,16 @@ MODULE_PARM_DESC(max_queues,
static unsigned int fatal_skb_slots = FATAL_SKB_SLOTS_DEFAULT;
module_param(fatal_skb_slots, uint, 0444);
+/* The amount to copy out of the first guest Tx slot into the skb's
+ * linear area. If the first slot has more data, it will be mapped
+ * and put into the first frag.
+ *
+ * This is sized to avoid pulling headers from the frags for most
+ * TCP/IP packets.
+ */
+#define XEN_NETBACK_TX_COPY_LEN 128
+
+
static void xenvif_idx_release(struct xenvif_queue *queue, u16 pending_idx,
u8 status);
@@ -125,13 +135,6 @@ static inline struct xenvif_queue *ubuf_to_queue(const struct ubuf_info *ubuf)
pending_tx_info[0]);
}
-/* This is a miniumum size for the linear area to avoid lots of
- * calls to __pskb_pull_tail() as we set up checksum offsets. The
- * value 128 was chosen as it covers all IPv4 and most likely
- * IPv6 headers.
- */
-#define PKT_PROT_LEN 128
-
static u16 frag_get_pending_idx(skb_frag_t *frag)
{
return (u16)frag->page_offset;
@@ -1446,9 +1449,9 @@ static void xenvif_tx_build_gops(struct xenvif_queue *queue,
index = pending_index(queue->pending_cons);
pending_idx = queue->pending_ring[index];
- data_len = (txreq.size > PKT_PROT_LEN &&
+ data_len = (txreq.size > XEN_NETBACK_TX_COPY_LEN &&
ret < XEN_NETBK_LEGACY_SLOTS_MAX) ?
- PKT_PROT_LEN : txreq.size;
+ XEN_NETBACK_TX_COPY_LEN : txreq.size;
skb = xenvif_alloc_skb(data_len);
if (unlikely(skb == NULL)) {
@@ -1653,11 +1656,6 @@ static int xenvif_tx_submit(struct xenvif_queue *queue)
}
}
- if (skb_is_nonlinear(skb) && skb_headlen(skb) < PKT_PROT_LEN) {
- int target = min_t(int, skb->len, PKT_PROT_LEN);
- __pskb_pull_tail(skb, target - skb_headlen(skb));
- }
-
skb->dev = queue->vif->dev;
skb->protocol = eth_type_trans(skb, skb->dev);
skb_reset_network_header(skb);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] net: shrink struct softnet_data
From: David Miller @ 2014-11-03 17:25 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1414936812.31792.31.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 02 Nov 2014 06:00:12 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> flow_limit in struct softnet_data is only read from local cpu
> and can be moved to fill a hole, reducing softnet_data size by
> 64 bytes on x86_64
>
> While we are at it, move output_queue, output_queue_tailp and
> completion_queue, so that rx / tx paths touch a single cache line.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: less interrupt masking in NAPI
From: David Miller @ 2014-11-03 17:25 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, willemb
In-Reply-To: <1414937973.31792.37.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 02 Nov 2014 06:19:33 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> net_rx_action() can mask irqs a single time to transfert sd->poll_list
> into a private list, for a very short duration.
>
> Then, napi_complete() can avoid masking irqs again,
> and net_rx_action() only needs to mask irq again in slow path.
>
> This patch removes 2 couples of irq mask/unmask per typical NAPI run,
> more if multiple napi were triggered.
>
> Note this also allows to give control back to caller (do_softirq())
> more often, so that other softirq handlers can be called a bit earlier,
> or ksoftirqd can be wakeup earlier under pressure.
>
> This was developed while testing an alternative to RX interrupt
> mitigation to reduce latencies while keeping or improving GRO
> aggregation on fast NIC.
>
> Idea is to test napi->gro_list at the end of a napi->poll() and
> reschedule one NAPI poll, but after servicing a full round of
> softirqs (timers, TX, rcu, ...). This will be allowed only if softirq
> is currently serviced by idle task or ksoftirqd, and resched not needed.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Also applied, thanks Eric.
^ permalink raw reply
* Re: [PATCH V1 net-next 0/5] Mellanox ethernet driver update Oct-30-2014
From: David Miller @ 2014-11-03 17:28 UTC (permalink / raw)
To: ogerlitz; +Cc: netdev, matanb, amirv, saeedm, shanim, idos
In-Reply-To: <1414938377-421-1-git-send-email-ogerlitz@mellanox.com>
From: Or Gerlitz <ogerlitz@mellanox.com>
Date: Sun, 2 Nov 2014 16:26:12 +0200
> The 1st patch from Saeed fixes a bug in the last net-next batch where
> a VF could get access to set port configuration, the next patch from Amir
> fixes a race in the port VPI logic. Next are two performance patches from Ido.
>
> The patch to add checksum complete status on GRE and such packets was
> preceded with a patch that converted the driver to only use napi_gro_receive
> vs. the current code which goes through napi_gro_frags on it's usual track.
> Eric D. has some thoughts and suggestions on that change for which we
> want to take the time and consider, so for the time being dropped that
> patch and the ones that depend on it.
...
> Changes from V0:
> - have the caller to provide the __GFP_COLD hint to the service function
> - dropped the patch that changes the GRO logic and the subsequent dependent
> patches.
Series applied, thanks.
^ permalink raw reply
* [PATCH] stmmac: remove custom implementation of print_hex_dump()
From: Andy Shevchenko @ 2014-11-03 17:28 UTC (permalink / raw)
To: Giuseppe Cavallaro, netdev, Kweh Hock Leong, David S . Miller,
Vince Bridgers
Cc: Andy Shevchenko
In-Reply-To: <1415035734-24163-1-git-send-email-andriy.shevchenko@linux.intel.com>
There is a kernel helper to dump buffers in a hexdecimal format. This patch
substitutes the open coded function by calling that helper.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6f77a46..5fb87f5 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -191,14 +191,8 @@ static void stmmac_clk_csr_set(struct stmmac_priv *priv)
static void print_pkt(unsigned char *buf, int len)
{
- int j;
- pr_debug("len = %d byte, buf addr: 0x%p", len, buf);
- for (j = 0; j < len; j++) {
- if ((j % 16) == 0)
- pr_debug("\n %03x:", j);
- pr_debug(" %02x", buf[j]);
- }
- pr_debug("\n");
+ pr_debug("len = %d byte, buf addr: 0x%p\n", len, buf);
+ print_hex_dump(KERN_DEBUG, " ", DUMP_PREFIX_OFFSET, 16, 1, buf, len, 0);
}
/* minimum number of free TX descriptors required to wake up TX process */
--
2.1.1
^ permalink raw reply related
* Re: Connection with Smart Z hangs
From: David Miller @ 2014-11-03 17:32 UTC (permalink / raw)
To: torvalds; +Cc: mlg.hessigheim, devel, netdev
In-Reply-To: <CA+55aFzB+d8+pjvuPo-n5G7X9MK9vh9ER5zYLCXZBzjMD4bseA@mail.gmail.com>
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Sun, 2 Nov 2014 10:20:22 -0800
> David, I'm just going to remove that whole bogus disconnect call. It
> won't make things *work* for Martin (because this is all in the
> "connect failed" path), but that call as-is is just wrong, wrong,
> wrong. And apparently nobody cares about irda any more.
I totally agree, just kill it. For the record this got added in:
commit 5b40964eadea40509d353318d2c82e8b7bf5e8a5
Author: Samuel Ortiz <samuel@sortiz.org>
Date: Mon Oct 11 00:46:51 2010 +0200
irda: Remove BKL instances from af_irda.c
which claims to be just a locking change, but obviously added
this bogus ->disconnect() call as well.
> If anybody is at all interested in helping maintain irda code, holler
> to David and to the netdev mailing list. The position is up for grabs.
Seriously... :-)
^ permalink raw reply
* [PATCH] stmmac: fix sparse warnings
From: Andy Shevchenko @ 2014-11-03 17:28 UTC (permalink / raw)
To: Giuseppe Cavallaro, netdev, Kweh Hock Leong, David S . Miller,
Vince Bridgers
Cc: Andy Shevchenko
This patch fixes the following sparse warnings.
drivers/net/ethernet/stmicro/stmmac/enh_desc.c:381:30: warning: symbol 'enh_desc_ops' was not declared. Should it be static?
drivers/net/ethernet/stmicro/stmmac/norm_desc.c:253:30: warning: symbol 'ndesc_ops' was not declared. Should it be static?
drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c:141:33: warning: symbol 'stmmac_ptp' was not declared. Should it be static?
There is no functional change.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/net/ethernet/stmicro/stmmac/enh_desc.c | 2 ++
drivers/net/ethernet/stmicro/stmmac/norm_desc.c | 2 ++
drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c | 2 ++
3 files changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
index 1e2bcf5..ddd4272 100644
--- a/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/enh_desc.c
@@ -23,8 +23,10 @@
*******************************************************************************/
#include <linux/stmmac.h>
+
#include "common.h"
#include "descs_com.h"
+#include "stmmac.h"
static int enh_desc_get_tx_status(void *data, struct stmmac_extra_stats *x,
struct dma_desc *p, void __iomem *ioaddr)
diff --git a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
index 35ad4f4..46b882c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
+++ b/drivers/net/ethernet/stmicro/stmmac/norm_desc.c
@@ -23,8 +23,10 @@
*******************************************************************************/
#include <linux/stmmac.h>
+
#include "common.h"
#include "descs_com.h"
+#include "stmmac.h"
static int ndesc_get_tx_status(void *data, struct stmmac_extra_stats *x,
struct dma_desc *p, void __iomem *ioaddr)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
index 76ad214..88e0da3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_hwtstamp.c
@@ -25,8 +25,10 @@
#include <linux/io.h>
#include <linux/delay.h>
+
#include "common.h"
#include "stmmac_ptp.h"
+#include "stmmac.h"
static void stmmac_config_hw_tstamping(void __iomem *ioaddr, u32 data)
{
--
2.1.1
^ permalink raw reply related
* Re: [PATCH net] uapi: add missing network related headers to kbuild
From: David Miller @ 2014-11-03 17:33 UTC (permalink / raw)
To: stephen; +Cc: netdev
In-Reply-To: <20141102113141.7aa1065c@urahara>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Sun, 2 Nov 2014 11:31:41 -0800
> The makefile for sanitizing kernel headers uses the kbuild file
> to determine which files to do. Several networking related headers
> were missing. Without these headers iproute2 build would break.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Applied, thanks Stephen.
^ permalink raw reply
* Re: [PATCH] stmmac: remove custom implementation of print_hex_dump()
From: Joe Perches @ 2014-11-03 17:39 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Giuseppe Cavallaro, netdev, Kweh Hock Leong, David S . Miller,
Vince Bridgers
In-Reply-To: <1415035734-24163-2-git-send-email-andriy.shevchenko@linux.intel.com>
On Mon, 2014-11-03 at 19:28 +0200, Andy Shevchenko wrote:
> There is a kernel helper to dump buffers in a hexdecimal format. This patch
> substitutes the open coded function by calling that helper.
[]
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
[]
> @@ -191,14 +191,8 @@ static void stmmac_clk_csr_set(struct stmmac_priv *priv)
>
> static void print_pkt(unsigned char *buf, int len)
> {
> - int j;
> - pr_debug("len = %d byte, buf addr: 0x%p", len, buf);
> - for (j = 0; j < len; j++) {
> - if ((j % 16) == 0)
> - pr_debug("\n %03x:", j);
> - pr_debug(" %02x", buf[j]);
> - }
> - pr_debug("\n");
> + pr_debug("len = %d byte, buf addr: 0x%p\n", len, buf);
> + print_hex_dump(KERN_DEBUG, " ", DUMP_PREFIX_OFFSET, 16, 1, buf, len, 0);
Prefix should be ""
Please use true/false for the last argument
Another (better?) option would be to use:
print_hex_dump_bytes("", DUMP_PREFIX_OFFSET, buf, len);
so that it can be dynamically controlled like
the pr_debug statements.
^ permalink raw reply
* Re: [PATCHv1 net-next] xen-netback: remove unconditional pull_skb_tail in guest Tx path
From: David Vrabel @ 2014-11-03 17:46 UTC (permalink / raw)
To: Ian Campbell; +Cc: netdev, xen-devel, Wei Liu, Malcolm Crossley
In-Reply-To: <1415036346.1411.3.camel@citrix.com>
On 03/11/14 17:39, Ian Campbell wrote:
> On Mon, 2014-11-03 at 17:23 +0000, David Vrabel wrote:
>> From: Malcolm Crossley <malcolm.crossley@citrix.com>
>>
>> Unconditionally pulling 128 bytes into the linear buffer is not
>> required. Netback has already grant copied up-to 128 bytes from the
>> first slot of a packet into the linear buffer. The first slot normally
>> contain all the IPv4/IPv6 and TCP/UDP headers.
>
> What about when it doesn't? It sounds as if we now won't pull up, which
> would be bad.
The network stack will always pull any headers it needs to inspect (the
frag may be a userspace page which has the same security issues as a
frag with a foreign page).
e.g., see skb_checksum_setup() called slightly later on in netback.
> To avoid the pull up the code would need to grant copy up-to 128 bytes
> from as many slots as needed, not only the first.
>
> Also, if the grant copy has already placed 128 bytes in the linear area,
> why is the pull up touching anything in the first place? Shouldn't it be
> a nop in that case?
The grant copy only copies from the first frag which may be less than
128 bytes in length.
David
^ permalink raw reply
* Re: [PATCH] stmmac: remove custom implementation of print_hex_dump()
From: Andy Shevchenko @ 2014-11-03 17:53 UTC (permalink / raw)
To: Joe Perches
Cc: Giuseppe Cavallaro, netdev, Kweh Hock Leong, David S . Miller,
Vince Bridgers
In-Reply-To: <1415036355.17743.22.camel@perches.com>
On Mon, 2014-11-03 at 09:39 -0800, Joe Perches wrote:
> On Mon, 2014-11-03 at 19:28 +0200, Andy Shevchenko wrote:
> > There is a kernel helper to dump buffers in a hexdecimal format. This patch
> > substitutes the open coded function by calling that helper.
> []
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> []
> > @@ -191,14 +191,8 @@ static void stmmac_clk_csr_set(struct stmmac_priv *priv)
> >
> > static void print_pkt(unsigned char *buf, int len)
> > {
> > - int j;
> > - pr_debug("len = %d byte, buf addr: 0x%p", len, buf);
> > - for (j = 0; j < len; j++) {
> > - if ((j % 16) == 0)
> > - pr_debug("\n %03x:", j);
> > - pr_debug(" %02x", buf[j]);
> > - }
> > - pr_debug("\n");
> > + pr_debug("len = %d byte, buf addr: 0x%p\n", len, buf);
> > + print_hex_dump(KERN_DEBUG, " ", DUMP_PREFIX_OFFSET, 16, 1, buf, len, 0);
>
> Prefix should be ""
Oh, initially there is an indentation in one space.
Maybe Giuseppe would comment on this.
> Please use true/false for the last argument
Okay.
>
> Another (better?) option would be to use:
>
> print_hex_dump_bytes("", DUMP_PREFIX_OFFSET, buf, len);
> so that it can be dynamically controlled like
> the pr_debug statements.
Ah, yes, but unfortunately it will print ASCII part always. Giuseppe,
what do you think?
P.S. [off topic] Joe, just would like to know if you have you seen my
last version of the patch series against hexdump.c which adds
seq_hex_dump() call [1]. If so, could you comment on it?
[1] https://lkml.org/lkml/2014/9/4/309
--
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCHv1 net-next] xen-netback: remove unconditional pull_skb_tail in guest Tx path
From: Ian Campbell @ 2014-11-03 17:39 UTC (permalink / raw)
To: David Vrabel; +Cc: netdev, xen-devel, Wei Liu, Malcolm Crossley
In-Reply-To: <1415035431-27485-1-git-send-email-david.vrabel@citrix.com>
On Mon, 2014-11-03 at 17:23 +0000, David Vrabel wrote:
> From: Malcolm Crossley <malcolm.crossley@citrix.com>
>
> Unconditionally pulling 128 bytes into the linear buffer is not
> required. Netback has already grant copied up-to 128 bytes from the
> first slot of a packet into the linear buffer. The first slot normally
> contain all the IPv4/IPv6 and TCP/UDP headers.
What about when it doesn't? It sounds as if we now won't pull up, which
would be bad.
To avoid the pull up the code would need to grant copy up-to 128 bytes
from as many slots as needed, not only the first.
Also, if the grant copy has already placed 128 bytes in the linear area,
why is the pull up touching anything in the first place? Shouldn't it be
a nop in that case?
> The unconditional pull would often copy frag data unnecessarily. This
> is a performance problem when running on a version of Xen where grant
> unmap avoids TLB flushes for pages which are not accessed. TLB
> flushes can now be avoided for > 99% of unmaps (it was 0% before).
>
> Grant unmap TLB flush avoidance will be available in a future version
> of Xen (probably 4.6).
>
> Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com>
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
> drivers/net/xen-netback/netback.c | 26 ++++++++++++--------------
> 1 file changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index 730252c..14e18bb 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -82,6 +82,16 @@ MODULE_PARM_DESC(max_queues,
> static unsigned int fatal_skb_slots = FATAL_SKB_SLOTS_DEFAULT;
> module_param(fatal_skb_slots, uint, 0444);
>
> +/* The amount to copy out of the first guest Tx slot into the skb's
> + * linear area. If the first slot has more data, it will be mapped
> + * and put into the first frag.
> + *
> + * This is sized to avoid pulling headers from the frags for most
> + * TCP/IP packets.
> + */
> +#define XEN_NETBACK_TX_COPY_LEN 128
Was it strictly necessary to both move and rename this? I can see why
the comment needed to change.
> static void xenvif_idx_release(struct xenvif_queue *queue, u16 pending_idx,
> u8 status);
>
> @@ -125,13 +135,6 @@ static inline struct xenvif_queue *ubuf_to_queue(const struct ubuf_info *ubuf)
> pending_tx_info[0]);
> }
>
> -/* This is a miniumum size for the linear area to avoid lots of
> - * calls to __pskb_pull_tail() as we set up checksum offsets. The
> - * value 128 was chosen as it covers all IPv4 and most likely
> - * IPv6 headers.
> - */
> -#define PKT_PROT_LEN 128
> -
> static u16 frag_get_pending_idx(skb_frag_t *frag)
> {
> return (u16)frag->page_offset;
> @@ -1446,9 +1449,9 @@ static void xenvif_tx_build_gops(struct xenvif_queue *queue,
> index = pending_index(queue->pending_cons);
> pending_idx = queue->pending_ring[index];
>
> - data_len = (txreq.size > PKT_PROT_LEN &&
> + data_len = (txreq.size > XEN_NETBACK_TX_COPY_LEN &&
> ret < XEN_NETBK_LEGACY_SLOTS_MAX) ?
> - PKT_PROT_LEN : txreq.size;
> + XEN_NETBACK_TX_COPY_LEN : txreq.size;
>
> skb = xenvif_alloc_skb(data_len);
> if (unlikely(skb == NULL)) {
> @@ -1653,11 +1656,6 @@ static int xenvif_tx_submit(struct xenvif_queue *queue)
> }
> }
>
> - if (skb_is_nonlinear(skb) && skb_headlen(skb) < PKT_PROT_LEN) {
> - int target = min_t(int, skb->len, PKT_PROT_LEN);
> - __pskb_pull_tail(skb, target - skb_headlen(skb));
> - }
> -
> skb->dev = queue->vif->dev;
> skb->protocol = eth_type_trans(skb, skb->dev);
> skb_reset_network_header(skb);
^ permalink raw reply
* Re: [PATCH] PPC: bpf_jit_comp: add SKF_AD_PKTTYPE instruction
From: Denis Kirjanov @ 2014-11-03 18:04 UTC (permalink / raw)
To: Philippe Bergheaud; +Cc: linuxppc-dev, netdev, Matt Evans, mpe
In-Reply-To: <5457A306.4040302@linux.vnet.ibm.com>
On 11/3/14, Philippe Bergheaud <felix@linux.vnet.ibm.com> wrote:
> Denis Kirjanov wrote:
>> Any feedback from PPC folks?
>
> I have reviewed the patch and it looks fine to me.
> I have tested successfuly on ppc64le.
> I could not test it on ppc64.
Nice, I've tested it on PPC64be
>
> Philippe
>
>> On 10/26/14, Denis Kirjanov <kda@linux-powerpc.org> wrote:
>>
>>>Cc: Matt Evans <matt@ozlabs.org>
>>>Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
>>>---
>>> arch/powerpc/include/asm/ppc-opcode.h | 1 +
>>> arch/powerpc/net/bpf_jit.h | 7 +++++++
>>> arch/powerpc/net/bpf_jit_comp.c | 5 +++++
>>> 3 files changed, 13 insertions(+)
>>>
>>>diff --git a/arch/powerpc/include/asm/ppc-opcode.h
>>>b/arch/powerpc/include/asm/ppc-opcode.h
>>>index 6f85362..1a52877 100644
>>>--- a/arch/powerpc/include/asm/ppc-opcode.h
>>>+++ b/arch/powerpc/include/asm/ppc-opcode.h
>>>@@ -204,6 +204,7 @@
>>> #define PPC_INST_ERATSX_DOT 0x7c000127
>>>
>>> /* Misc instructions for BPF compiler */
>>>+#define PPC_INST_LBZ 0x88000000
>>> #define PPC_INST_LD 0xe8000000
>>> #define PPC_INST_LHZ 0xa0000000
>>> #define PPC_INST_LHBRX 0x7c00062c
>>>diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
>>>index 9aee27c..c406aa9 100644
>>>--- a/arch/powerpc/net/bpf_jit.h
>>>+++ b/arch/powerpc/net/bpf_jit.h
>>>@@ -87,6 +87,9 @@ DECLARE_LOAD_FUNC(sk_load_byte_msh);
>>> #define PPC_STD(r, base, i) EMIT(PPC_INST_STD | ___PPC_RS(r) | \
>>> ___PPC_RA(base) | ((i) & 0xfffc))
>>>
>>>+
>>>+#define PPC_LBZ(r, base, i) EMIT(PPC_INST_LBZ | ___PPC_RT(r) | \
>>>+ ___PPC_RA(base) | IMM_L(i))
>>> #define PPC_LD(r, base, i) EMIT(PPC_INST_LD | ___PPC_RT(r) | \
>>> ___PPC_RA(base) | IMM_L(i))
>>> #define PPC_LWZ(r, base, i) EMIT(PPC_INST_LWZ | ___PPC_RT(r) | \
>>>@@ -96,6 +99,10 @@ DECLARE_LOAD_FUNC(sk_load_byte_msh);
>>> #define PPC_LHBRX(r, base, b) EMIT(PPC_INST_LHBRX | ___PPC_RT(r) |
>>> \
>>> ___PPC_RA(base) | ___PPC_RB(b))
>>> /* Convenience helpers for the above with 'far' offsets: */
>>>+#define PPC_LBZ_OFFS(r, base, i) do { if ((i) < 32768) PPC_LBZ(r, base,
>>> i);
>>> \
>>>+ else { PPC_ADDIS(r, base, IMM_HA(i)); \
>>>+ PPC_LBZ(r, r, IMM_L(i)); } } while(0)
>>>+
>>> #define PPC_LD_OFFS(r, base, i) do { if ((i) < 32768) PPC_LD(r, base,
>>> i);
>>> \
>>> else { PPC_ADDIS(r, base, IMM_HA(i)); \
>>> PPC_LD(r, r, IMM_L(i)); } } while(0)
>>>diff --git a/arch/powerpc/net/bpf_jit_comp.c
>>>b/arch/powerpc/net/bpf_jit_comp.c
>>>index cbae2df..d110e28 100644
>>>--- a/arch/powerpc/net/bpf_jit_comp.c
>>>+++ b/arch/powerpc/net/bpf_jit_comp.c
>>>@@ -407,6 +407,11 @@ static int bpf_jit_build_body(struct bpf_prog *fp,
>>> u32
>>>*image,
>>> PPC_LHZ_OFFS(r_A, r_skb, offsetof(struct sk_buff,
>>> queue_mapping));
>>> break;
>>>+ case BPF_ANC | SKF_AD_PKTTYPE:
>>>+ PPC_LBZ_OFFS(r_A, r_skb, PKT_TYPE_OFFSET());
>>>+ PPC_ANDI(r_A, r_A, PKT_TYPE_MAX);
>>>+ PPC_SRWI(r_A, r_A, 5);
>>>+ break;
>>> case BPF_ANC | SKF_AD_CPU:
>>> #ifdef CONFIG_SMP
>>> /*
>>>--
>>>2.1.0
>>>
>>>
>>
>> _______________________________________________
>> Linuxppc-dev mailing list
>> Linuxppc-dev@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
>
^ permalink raw reply
* Re: [PATCHv1 net-next] xen-netback: remove unconditional pull_skb_tail in guest Tx path
From: Ian Campbell @ 2014-11-03 17:55 UTC (permalink / raw)
To: David Vrabel; +Cc: netdev, xen-devel, Wei Liu, Malcolm Crossley
In-Reply-To: <5457BF80.2000205@citrix.com>
On Mon, 2014-11-03 at 17:46 +0000, David Vrabel wrote:
> On 03/11/14 17:39, Ian Campbell wrote:
> > On Mon, 2014-11-03 at 17:23 +0000, David Vrabel wrote:
> >> From: Malcolm Crossley <malcolm.crossley@citrix.com>
> >>
> >> Unconditionally pulling 128 bytes into the linear buffer is not
> >> required. Netback has already grant copied up-to 128 bytes from the
> >> first slot of a packet into the linear buffer. The first slot normally
> >> contain all the IPv4/IPv6 and TCP/UDP headers.
> >
> > What about when it doesn't? It sounds as if we now won't pull up, which
> > would be bad.
>
> The network stack will always pull any headers it needs to inspect (the
> frag may be a userspace page which has the same security issues as a
> frag with a foreign page).
I don't believe it will, unless something changed since I last looked.
The kernel assumes that it has been sensible enough to put the headers
in the linear area, since it is the one which generates them in most
cases. In other cases its up to the relevant driver to make sure this is
true.
> e.g., see skb_checksum_setup() called slightly later on in netback.
This however is what will make things safe for us (note that this is
only used by xen-net* in practice), it is this which should be mentioned
in the commit message I think.
> > To avoid the pull up the code would need to grant copy up-to 128 bytes
> > from as many slots as needed, not only the first.
> >
> > Also, if the grant copy has already placed 128 bytes in the linear area,
> > why is the pull up touching anything in the first place? Shouldn't it be
> > a nop in that case?
>
> The grant copy only copies from the first frag which may be less than
> 128 bytes in length.
>
> David
^ permalink raw reply
* Re: [PATCH] stmmac: remove custom implementation of print_hex_dump()
From: Joe Perches @ 2014-11-03 18:11 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Giuseppe Cavallaro, netdev, Kweh Hock Leong, David S . Miller,
Vince Bridgers
In-Reply-To: <1415037237.472.1.camel@linux.intel.com>
On Mon, 2014-11-03 at 19:53 +0200, Andy Shevchenko wrote:
> On Mon, 2014-11-03 at 09:39 -0800, Joe Perches wrote:
> > On Mon, 2014-11-03 at 19:28 +0200, Andy Shevchenko wrote:
> > > There is a kernel helper to dump buffers in a hexdecimal format. This patch
> > > substitutes the open coded function by calling that helper.
> > []
> > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> > []
> > > @@ -191,14 +191,8 @@ static void stmmac_clk_csr_set(struct stmmac_priv *priv)
> > >
> > > static void print_pkt(unsigned char *buf, int len)
> > > {
> > > - int j;
> > > - pr_debug("len = %d byte, buf addr: 0x%p", len, buf);
> > > - for (j = 0; j < len; j++) {
> > > - if ((j % 16) == 0)
> > > - pr_debug("\n %03x:", j);
> > > - pr_debug(" %02x", buf[j]);
> > > - }
> > > - pr_debug("\n");
> > > + pr_debug("len = %d byte, buf addr: 0x%p\n", len, buf);
> > > + print_hex_dump(KERN_DEBUG, " ", DUMP_PREFIX_OFFSET, 16, 1, buf, len, 0);
> >
> > Prefix should be ""
>
> Oh, initially there is an indentation in one space.
> Maybe Giuseppe would comment on this.
I think that's not particularly important.
This changes the output anyway as the the printed offset
is now 8 chars wide not 3.
> > Another (better?) option would be to use:
> >
> > print_hex_dump_bytes("", DUMP_PREFIX_OFFSET, buf, len);
>
> > so that it can be dynamically controlled like
> > the pr_debug statements.
>
> Ah, yes, but unfortunately it will print ASCII part always. Giuseppe,
> what do you think?
I'm not bothered by logging message output changes.
It's not a guaranteed stable output.
> P.S. [off topic] Joe, just would like to know if you have you seen my
> last version of the patch series against hexdump.c which adds
> seq_hex_dump() call [1]. If so, could you comment on it?
> [1] https://lkml.org/lkml/2014/9/4/309
seq_<foo> output however is guaranteed.
So any conversion to the seq_hex_dump function
would need to be exactly the same.
New code could use seq_hex_dump.
Other than that, it seems sensible enough.
^ 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