* [net-next 5/9] ixgbe: Make pull tail function separate from rest of cleanup_headers
From: Peter P Waskiewicz Jr @ 2012-08-16 22:48 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, sassmann, Peter P Waskiewicz Jr
In-Reply-To: <1345157318-23731-1-git-send-email-peter.p.waskiewicz.jr@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
This change creates a separate function for functionality similar to
pskb_pull_tail. The main motivation for moving it to a separate function
is so that later I can just skip this function in the case where we have
already copied the buffer into skb->head.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 94 ++++++++++++++++-----------
1 file changed, 57 insertions(+), 37 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index b0020fc..d926973 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1458,6 +1458,61 @@ static bool ixgbe_is_non_eop(struct ixgbe_ring *rx_ring,
}
/**
+ * ixgbe_pull_tail - ixgbe specific version of skb_pull_tail
+ * @rx_ring: rx descriptor ring packet is being transacted on
+ * @skb: pointer to current skb being adjusted
+ *
+ * This function is an ixgbe specific version of __pskb_pull_tail. The
+ * main difference between this version and the original function is that
+ * this function can make several assumptions about the state of things
+ * that allow for significant optimizations versus the standard function.
+ * As a result we can do things like drop a frag and maintain an accurate
+ * truesize for the skb.
+ */
+static void ixgbe_pull_tail(struct ixgbe_ring *rx_ring,
+ struct sk_buff *skb)
+{
+ struct skb_frag_struct *frag = &skb_shinfo(skb)->frags[0];
+ unsigned char *va;
+ unsigned int pull_len;
+
+ /*
+ * it is valid to use page_address instead of kmap since we are
+ * working with pages allocated out of the lomem pool per
+ * alloc_page(GFP_ATOMIC)
+ */
+ va = skb_frag_address(frag);
+
+ /*
+ * we need the header to contain the greater of either ETH_HLEN or
+ * 60 bytes if the skb->len is less than 60 for skb_pad.
+ */
+ pull_len = skb_frag_size(frag);
+ if (pull_len > IXGBE_RX_HDR_SIZE)
+ pull_len = ixgbe_get_headlen(va, IXGBE_RX_HDR_SIZE);
+
+ /* align pull length to size of long to optimize memcpy performance */
+ skb_copy_to_linear_data(skb, va, ALIGN(pull_len, sizeof(long)));
+
+ /* update all of the pointers */
+ skb_frag_size_sub(frag, pull_len);
+ frag->page_offset += pull_len;
+ skb->data_len -= pull_len;
+ skb->tail += pull_len;
+
+ /*
+ * if we sucked the frag empty then we should free it,
+ * if there are other frags here something is screwed up in hardware
+ */
+ if (skb_frag_size(frag) == 0) {
+ BUG_ON(skb_shinfo(skb)->nr_frags != 1);
+ skb_shinfo(skb)->nr_frags = 0;
+ __skb_frag_unref(frag);
+ skb->truesize -= ixgbe_rx_bufsz(rx_ring);
+ }
+}
+
+/**
* ixgbe_dma_sync_frag - perform DMA sync for first frag of SKB
* @rx_ring: rx descriptor ring packet is being transacted on
* @skb: pointer to current skb being updated
@@ -1509,10 +1564,7 @@ static bool ixgbe_cleanup_headers(struct ixgbe_ring *rx_ring,
union ixgbe_adv_rx_desc *rx_desc,
struct sk_buff *skb)
{
- struct skb_frag_struct *frag = &skb_shinfo(skb)->frags[0];
struct net_device *netdev = rx_ring->netdev;
- unsigned char *va;
- unsigned int pull_len;
/* verify that the packet does not have any known errors */
if (unlikely(ixgbe_test_staterr(rx_desc,
@@ -1522,40 +1574,8 @@ static bool ixgbe_cleanup_headers(struct ixgbe_ring *rx_ring,
return true;
}
- /*
- * it is valid to use page_address instead of kmap since we are
- * working with pages allocated out of the lomem pool per
- * alloc_page(GFP_ATOMIC)
- */
- va = skb_frag_address(frag);
-
- /*
- * we need the header to contain the greater of either ETH_HLEN or
- * 60 bytes if the skb->len is less than 60 for skb_pad.
- */
- pull_len = skb_frag_size(frag);
- if (pull_len > IXGBE_RX_HDR_SIZE)
- pull_len = ixgbe_get_headlen(va, IXGBE_RX_HDR_SIZE);
-
- /* align pull length to size of long to optimize memcpy performance */
- skb_copy_to_linear_data(skb, va, ALIGN(pull_len, sizeof(long)));
-
- /* update all of the pointers */
- skb_frag_size_sub(frag, pull_len);
- frag->page_offset += pull_len;
- skb->data_len -= pull_len;
- skb->tail += pull_len;
-
- /*
- * if we sucked the frag empty then we should free it,
- * if there are other frags here something is screwed up in hardware
- */
- if (skb_frag_size(frag) == 0) {
- BUG_ON(skb_shinfo(skb)->nr_frags != 1);
- skb_shinfo(skb)->nr_frags = 0;
- __skb_frag_unref(frag);
- skb->truesize -= ixgbe_rx_bufsz(rx_ring);
- }
+ /* place header in linear portion of buffer */
+ ixgbe_pull_tail(rx_ring, skb);
#ifdef IXGBE_FCOE
/* do not attempt to pad FCoE Frames as this will disrupt DDP */
--
1.7.11.2
^ permalink raw reply related
* [net-next 7/9] ixgbe: Make allocating skb and placing data in it a separate function
From: Peter P Waskiewicz Jr @ 2012-08-16 22:48 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, sassmann, Peter P Waskiewicz Jr
In-Reply-To: <1345157318-23731-1-git-send-email-peter.p.waskiewicz.jr@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
This patch creates a function named ixgbe_fetch_rx_buffer. The sole
purpose of this function is to retrieve a single buffer off of the ring and
to place it in an skb.
The advantage to doing this is that it helps improve the readability since
I can decrease the indentation and for the code in this section.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 166 ++++++++++++++------------
1 file changed, 89 insertions(+), 77 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index d11fac5..9e72ae6 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1693,6 +1693,89 @@ static bool ixgbe_add_rx_frag(struct ixgbe_ring *rx_ring,
return true;
}
+static struct sk_buff *ixgbe_fetch_rx_buffer(struct ixgbe_ring *rx_ring,
+ union ixgbe_adv_rx_desc *rx_desc)
+{
+ struct ixgbe_rx_buffer *rx_buffer;
+ struct sk_buff *skb;
+ struct page *page;
+
+ rx_buffer = &rx_ring->rx_buffer_info[rx_ring->next_to_clean];
+ page = rx_buffer->page;
+ prefetchw(page);
+
+ skb = rx_buffer->skb;
+
+ if (likely(!skb)) {
+ void *page_addr = page_address(page) +
+ rx_buffer->page_offset;
+
+ /* prefetch first cache line of first page */
+ prefetch(page_addr);
+#if L1_CACHE_BYTES < 128
+ prefetch(page_addr + L1_CACHE_BYTES);
+#endif
+
+ /* allocate a skb to store the frags */
+ skb = netdev_alloc_skb_ip_align(rx_ring->netdev,
+ IXGBE_RX_HDR_SIZE);
+ if (unlikely(!skb)) {
+ rx_ring->rx_stats.alloc_rx_buff_failed++;
+ return NULL;
+ }
+
+ /*
+ * we will be copying header into skb->data in
+ * pskb_may_pull so it is in our interest to prefetch
+ * it now to avoid a possible cache miss
+ */
+ prefetchw(skb->data);
+
+ /*
+ * Delay unmapping of the first packet. It carries the
+ * header information, HW may still access the header
+ * after the writeback. Only unmap it when EOP is
+ * reached
+ */
+ if (likely(ixgbe_test_staterr(rx_desc, IXGBE_RXD_STAT_EOP)))
+ goto dma_sync;
+
+ IXGBE_CB(skb)->dma = rx_buffer->dma;
+ } else {
+ if (ixgbe_test_staterr(rx_desc, IXGBE_RXD_STAT_EOP))
+ ixgbe_dma_sync_frag(rx_ring, skb);
+
+dma_sync:
+ /* we are reusing so sync this buffer for CPU use */
+ dma_sync_single_range_for_cpu(rx_ring->dev,
+ rx_buffer->dma,
+ rx_buffer->page_offset,
+ ixgbe_rx_bufsz(rx_ring),
+ DMA_FROM_DEVICE);
+ }
+
+ /* pull page into skb */
+ if (ixgbe_add_rx_frag(rx_ring, rx_buffer, rx_desc, skb)) {
+ /* hand second half of page back to the ring */
+ ixgbe_reuse_rx_page(rx_ring, rx_buffer);
+ } else if (IXGBE_CB(skb)->dma == rx_buffer->dma) {
+ /* the page has been released from the ring */
+ IXGBE_CB(skb)->page_released = true;
+ } else {
+ /* we are not reusing the buffer so unmap it */
+ dma_unmap_page(rx_ring->dev, rx_buffer->dma,
+ ixgbe_rx_pg_size(rx_ring),
+ DMA_FROM_DEVICE);
+ }
+
+ /* clear contents of buffer_info */
+ rx_buffer->skb = NULL;
+ rx_buffer->dma = 0;
+ rx_buffer->page = NULL;
+
+ return skb;
+}
+
/**
* ixgbe_clean_rx_irq - Clean completed descriptors from Rx ring - bounce buf
* @q_vector: structure containing interrupt and ring information
@@ -1718,11 +1801,8 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
u16 cleaned_count = ixgbe_desc_unused(rx_ring);
do {
- struct ixgbe_rx_buffer *rx_buffer;
union ixgbe_adv_rx_desc *rx_desc;
struct sk_buff *skb;
- struct page *page;
- u16 ntc;
/* return some buffers to hardware, one at a time is too slow */
if (cleaned_count >= IXGBE_RX_BUFFER_WRITE) {
@@ -1730,9 +1810,7 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
cleaned_count = 0;
}
- ntc = rx_ring->next_to_clean;
- rx_desc = IXGBE_RX_DESC(rx_ring, ntc);
- rx_buffer = &rx_ring->rx_buffer_info[ntc];
+ rx_desc = IXGBE_RX_DESC(rx_ring, rx_ring->next_to_clean);
if (!ixgbe_test_staterr(rx_desc, IXGBE_RXD_STAT_DD))
break;
@@ -1744,78 +1822,12 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
*/
rmb();
- page = rx_buffer->page;
- prefetchw(page);
-
- skb = rx_buffer->skb;
-
- if (likely(!skb)) {
- void *page_addr = page_address(page) +
- rx_buffer->page_offset;
-
- /* prefetch first cache line of first page */
- prefetch(page_addr);
-#if L1_CACHE_BYTES < 128
- prefetch(page_addr + L1_CACHE_BYTES);
-#endif
-
- /* allocate a skb to store the frags */
- skb = netdev_alloc_skb_ip_align(rx_ring->netdev,
- IXGBE_RX_HDR_SIZE);
- if (unlikely(!skb)) {
- rx_ring->rx_stats.alloc_rx_buff_failed++;
- break;
- }
-
- /*
- * we will be copying header into skb->data in
- * pskb_may_pull so it is in our interest to prefetch
- * it now to avoid a possible cache miss
- */
- prefetchw(skb->data);
-
- /*
- * Delay unmapping of the first packet. It carries the
- * header information, HW may still access the header
- * after the writeback. Only unmap it when EOP is
- * reached
- */
- if (likely(ixgbe_test_staterr(rx_desc,
- IXGBE_RXD_STAT_EOP)))
- goto dma_sync;
+ /* retrieve a buffer from the ring */
+ skb = ixgbe_fetch_rx_buffer(rx_ring, rx_desc);
- IXGBE_CB(skb)->dma = rx_buffer->dma;
- } else {
- if (ixgbe_test_staterr(rx_desc, IXGBE_RXD_STAT_EOP))
- ixgbe_dma_sync_frag(rx_ring, skb);
-
-dma_sync:
- /* we are reusing so sync this buffer for CPU use */
- dma_sync_single_range_for_cpu(rx_ring->dev,
- rx_buffer->dma,
- rx_buffer->page_offset,
- ixgbe_rx_bufsz(rx_ring),
- DMA_FROM_DEVICE);
- }
-
- /* pull page into skb */
- if (ixgbe_add_rx_frag(rx_ring, rx_buffer, rx_desc, skb)) {
- /* hand second half of page back to the ring */
- ixgbe_reuse_rx_page(rx_ring, rx_buffer);
- } else if (IXGBE_CB(skb)->dma == rx_buffer->dma) {
- /* the page has been released from the ring */
- IXGBE_CB(skb)->page_released = true;
- } else {
- /* we are not reusing the buffer so unmap it */
- dma_unmap_page(rx_ring->dev, rx_buffer->dma,
- ixgbe_rx_pg_size(rx_ring),
- DMA_FROM_DEVICE);
- }
-
- /* clear contents of buffer_info */
- rx_buffer->skb = NULL;
- rx_buffer->dma = 0;
- rx_buffer->page = NULL;
+ /* exit if we failed to retrieve a buffer */
+ if (!skb)
+ break;
ixgbe_get_rsc_cnt(rx_ring, rx_desc, skb);
--
1.7.11.2
^ permalink raw reply related
* [net-next 8/9] ixgbe: Roll RSC code into non-EOP code
From: Peter P Waskiewicz Jr @ 2012-08-16 22:48 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, sassmann, Peter P Waskiewicz Jr
In-Reply-To: <1345157318-23731-1-git-send-email-peter.p.waskiewicz.jr@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
This change moves the RSC code into the non-EOP descriptor handling
function. The main motivation behind this change is to help reduce the
overhead in the non-RSC case. Previously the non-RSC path code would
always be checking for append count even if RSC had been disabled. Now
this code is completely skipped in a single conditional check instead of
having to make two separate checks.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 51 ++++++++++-----------------
1 file changed, 19 insertions(+), 32 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 9e72ae6..aa37b84 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1320,29 +1320,6 @@ static unsigned int ixgbe_get_headlen(unsigned char *data,
return max_len;
}
-static void ixgbe_get_rsc_cnt(struct ixgbe_ring *rx_ring,
- union ixgbe_adv_rx_desc *rx_desc,
- struct sk_buff *skb)
-{
- __le32 rsc_enabled;
- u32 rsc_cnt;
-
- if (!ring_is_rsc_enabled(rx_ring))
- return;
-
- rsc_enabled = rx_desc->wb.lower.lo_dword.data &
- cpu_to_le32(IXGBE_RXDADV_RSCCNT_MASK);
-
- /* If this is an RSC frame rsc_cnt should be non-zero */
- if (!rsc_enabled)
- return;
-
- rsc_cnt = le32_to_cpu(rsc_enabled);
- rsc_cnt >>= IXGBE_RXDADV_RSCCNT_SHIFT;
-
- IXGBE_CB(skb)->append_cnt += rsc_cnt - 1;
-}
-
static void ixgbe_set_rsc_gso_size(struct ixgbe_ring *ring,
struct sk_buff *skb)
{
@@ -1440,16 +1417,28 @@ static bool ixgbe_is_non_eop(struct ixgbe_ring *rx_ring,
prefetch(IXGBE_RX_DESC(rx_ring, ntc));
- if (likely(ixgbe_test_staterr(rx_desc, IXGBE_RXD_STAT_EOP)))
- return false;
+ /* update RSC append count if present */
+ if (ring_is_rsc_enabled(rx_ring)) {
+ __le32 rsc_enabled = rx_desc->wb.lower.lo_dword.data &
+ cpu_to_le32(IXGBE_RXDADV_RSCCNT_MASK);
+
+ if (unlikely(rsc_enabled)) {
+ u32 rsc_cnt = le32_to_cpu(rsc_enabled);
+
+ rsc_cnt >>= IXGBE_RXDADV_RSCCNT_SHIFT;
+ IXGBE_CB(skb)->append_cnt += rsc_cnt - 1;
- /* append_cnt indicates packet is RSC, if so fetch nextp */
- if (IXGBE_CB(skb)->append_cnt) {
- ntc = le32_to_cpu(rx_desc->wb.upper.status_error);
- ntc &= IXGBE_RXDADV_NEXTP_MASK;
- ntc >>= IXGBE_RXDADV_NEXTP_SHIFT;
+ /* update ntc based on RSC value */
+ ntc = le32_to_cpu(rx_desc->wb.upper.status_error);
+ ntc &= IXGBE_RXDADV_NEXTP_MASK;
+ ntc >>= IXGBE_RXDADV_NEXTP_SHIFT;
+ }
}
+ /* if we are the last buffer then there is nothing else to do */
+ if (likely(ixgbe_test_staterr(rx_desc, IXGBE_RXD_STAT_EOP)))
+ return false;
+
/* place skb in next buffer to be received */
rx_ring->rx_buffer_info[ntc].skb = skb;
rx_ring->rx_stats.non_eop_descs++;
@@ -1829,8 +1818,6 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
if (!skb)
break;
- ixgbe_get_rsc_cnt(rx_ring, rx_desc, skb);
-
cleaned_count++;
/* place incomplete frames back on ring for completion */
--
1.7.11.2
^ permalink raw reply related
* [net-next 9/9] ixgbe: Rewrite code related to configuring IFCS bit in Tx descriptor
From: Peter P Waskiewicz Jr @ 2012-08-16 22:48 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, gospo, sassmann, Peter P Waskiewicz Jr
In-Reply-To: <1345157318-23731-1-git-send-email-peter.p.waskiewicz.jr@intel.com>
From: Alexander Duyck <alexander.h.duyck@intel.com>
This change updates the code related to configuring the transmit frame
checksum. Specifically I have updated the code so that we can only skip
inserting the checksum in the case that we are not performing some other
offload that will modify the frame data.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 +
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 16 ++++++++++------
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index fd2bc69..bffcf1f 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -107,6 +107,7 @@
#define IXGBE_TX_FLAGS_FSO (u32)(1 << 6)
#define IXGBE_TX_FLAGS_TXSW (u32)(1 << 7)
#define IXGBE_TX_FLAGS_TSTAMP (u32)(1 << 8)
+#define IXGBE_TX_FLAGS_NO_IFCS (u32)(1 << 9)
#define IXGBE_TX_FLAGS_VLAN_MASK 0xffff0000
#define IXGBE_TX_FLAGS_VLAN_PRIO_MASK 0xe0000000
#define IXGBE_TX_FLAGS_VLAN_PRIO_SHIFT 29
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index aa37b84..fa0d6e1 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -5903,9 +5903,12 @@ static void ixgbe_tx_csum(struct ixgbe_ring *tx_ring,
u32 type_tucmd = 0;
if (skb->ip_summed != CHECKSUM_PARTIAL) {
- if (!(first->tx_flags & IXGBE_TX_FLAGS_HW_VLAN) &&
- !(first->tx_flags & IXGBE_TX_FLAGS_TXSW))
- return;
+ if (!(first->tx_flags & IXGBE_TX_FLAGS_HW_VLAN)) {
+ if (unlikely(skb->no_fcs))
+ first->tx_flags |= IXGBE_TX_FLAGS_NO_IFCS;
+ if (!(first->tx_flags & IXGBE_TX_FLAGS_TXSW))
+ return;
+ }
} else {
u8 l4_hdr = 0;
switch (first->protocol) {
@@ -5967,7 +5970,6 @@ static __le32 ixgbe_tx_cmd_type(u32 tx_flags)
{
/* set type for advanced descriptor with frame checksum insertion */
__le32 cmd_type = cpu_to_le32(IXGBE_ADVTXD_DTYP_DATA |
- IXGBE_ADVTXD_DCMD_IFCS |
IXGBE_ADVTXD_DCMD_DEXT);
/* set HW vlan bit if vlan is present */
@@ -5987,6 +5989,10 @@ static __le32 ixgbe_tx_cmd_type(u32 tx_flags)
#endif
cmd_type |= cpu_to_le32(IXGBE_ADVTXD_DCMD_TSE);
+ /* insert frame checksum */
+ if (!(tx_flags & IXGBE_TX_FLAGS_NO_IFCS))
+ cmd_type |= cpu_to_le32(IXGBE_ADVTXD_DCMD_IFCS);
+
return cmd_type;
}
@@ -6092,8 +6098,6 @@ static void ixgbe_tx_map(struct ixgbe_ring *tx_ring,
if (likely(!data_len))
break;
- if (unlikely(skb->no_fcs))
- cmd_type &= ~(cpu_to_le32(IXGBE_ADVTXD_DCMD_IFCS));
tx_desc->read.cmd_type_len = cmd_type | cpu_to_le32(size);
i++;
--
1.7.11.2
^ permalink raw reply related
* Re: [patch] ipv6: move dereference after check in fl_free()
From: Eric W. Biederman @ 2012-08-16 23:11 UTC (permalink / raw)
To: Dan Carpenter
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, kernel-janitors
In-Reply-To: <20120816131502.GB23188@elgon.mountain>
Dan Carpenter <dan.carpenter@oracle.com> writes:
> There is a dereference before checking for NULL bug here. Generally
> free() functions should accept NULL pointers. For example, fl_create()
> can pass a NULL pointer to fl_free() on the error path.
Thanks.
Applied to user-namespace.git
Eric
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Only needed on linux-next.
>
> diff --git a/net/ipv6/ip6_flowlabel.c b/net/ipv6/ip6_flowlabel.c
> index c836a6a..90bbefb 100644
> --- a/net/ipv6/ip6_flowlabel.c
> +++ b/net/ipv6/ip6_flowlabel.c
> @@ -91,12 +91,9 @@ static struct ip6_flowlabel *fl_lookup(struct net *net, __be32 label)
>
> static void fl_free(struct ip6_flowlabel *fl)
> {
> - switch (fl->share) {
> - case IPV6_FL_S_PROCESS:
> - put_pid(fl->owner.pid);
> - break;
> - }
> if (fl) {
> + if (fl->share == IPV6_FL_S_PROCESS)
> + put_pid(fl->owner.pid);
> release_net(fl->fl_net);
> kfree(fl->opt);
> }
^ permalink raw reply
* Re: [PATCH v0 5/5] cgroup: Assign subsystem IDs during compile time
From: Tejun Heo @ 2012-08-16 23:20 UTC (permalink / raw)
To: Daniel Wagner
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, cgroups-u79uwXL29TY76Z2rM5mHXA,
Daniel Wagner, David S. Miller, Andrew Morton, Eric Dumazet,
Gao feng, Glauber Costa, Jamal Hadi Salim, John Fastabend,
Kamezawa Hiroyuki, Li Zefan, Neil Horman
In-Reply-To: <1345126336-20755-6-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
On Thu, Aug 16, 2012 at 04:12:16PM +0200, Daniel Wagner wrote:
> From: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>
>
> We are able to safe some space when we assign the subsystem
> IDs at compile time. Instead of allocating per cgroup
> cgroup->subsys[CGROUP_SUBSYS_COUNT] where CGROUP_SUBSYS_COUNT is
> always 64, we allocate 12 + 1 at max (at this point there are 12
> subsystem).
So, IIUC, this is effectively removing the capability to implement
modularized controller which isn't known at kernel compile time. Am I
right?
I don't think that's a bad idea but if we're doing that, can't we make
things even simpler? Do we need to distinguish in-kernel and module
at all?
Li, what do you think about this?
Thanks.
--
tejun
^ permalink raw reply
* Re: suspicious RCU usage in xfrm_net_init()
From: Fan Du @ 2012-08-17 1:07 UTC (permalink / raw)
To: David Miller; +Cc: Fengguang Wu, Priyanka Jain, netdev, LKML
In-Reply-To: <20120816151949.GA18681@localhost>
[-- Attachment #1: Type: text/plain, Size: 4159 bytes --]
On 2012年08月16日 23:19, Fengguang Wu wrote:
> Hi Fan,
>
> On Thu, Aug 16, 2012 at 05:36:35PM +0800, Fan Du wrote:
>>
>> Hi, Fengguang
>>
>> Could you please try the below patch, see if spewing still there?
>> thanks
>
> Yes, it worked, thank you very much!
>
Hi, Dave
Could you please pick up this patch?
thanks
> btw, your email client wraps long lines..
>
Oh, I will definitely fix this.
thanks feng guang for the testing :)
> Thanks,
> Fengguang
>
>> From a3f86ecc3ee16ff81d49416bbf791780422988b3 Mon Sep 17 00:00:00 2001
>> From: Fan Du<fan.du@windriver.com>
>> Date: Thu, 16 Aug 2012 17:31:25 +0800
>> Subject: [PATCH] Use rcu_dereference_bh to deference pointer
>> protected by rcu_read_lock_bh
>>
>> Signed-off-by: Fan Du<fan.du@windriver.com>
>> ---
>> net/xfrm/xfrm_policy.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
>> index 5ad4d2c..75a9d6a 100644
>> --- a/net/xfrm/xfrm_policy.c
>> +++ b/net/xfrm/xfrm_policy.c
>> @@ -2501,7 +2501,7 @@ static void __net_init
>> xfrm_dst_ops_init(struct net *net)
>> struct xfrm_policy_afinfo *afinfo;
>>
>> rcu_read_lock_bh();
>> - afinfo = rcu_dereference(xfrm_policy_afinfo[AF_INET]);
>> + afinfo = rcu_dereference_bh(xfrm_policy_afinfo[AF_INET]);
>> if (afinfo)
>> net->xfrm.xfrm4_dst_ops = *afinfo->dst_ops;
>> #if IS_ENABLED(CONFIG_IPV6)
>> --
>> 1.7.1
>>
>>
>>
>>
>> On 2012年08月16日 15:37, Fengguang Wu wrote:
>>> Hi Priyanka,
>>>
>>> The below warning shows up, probably related to this commit:
>>>
>>> 418a99ac6ad487dc9c42e6b0e85f941af56330f2 Replace rwlock on xfrm_policy_afinfo with rcu
>>>
>>> [ 0.921216]
>>> [ 0.921645] ===============================
>>> [ 0.922766] [ INFO: suspicious RCU usage. ]
>>> [ 0.923887] 3.5.0-01540-g1669891 #64 Not tainted
>>> [ 0.925123] -------------------------------
>>> [ 0.932860] /c/kernel-tests/src/tip/net/xfrm/xfrm_policy.c:2504 suspicious rcu_dereference_check() usage!
>>> [ 0.935361]
>>> [ 0.935361] other info that might help us debug this:
>>> [ 0.935361]
>>> [ 0.937472]
>>> [ 0.937472] rcu_scheduler_active = 1, debug_locks = 0
>>> [ 0.939182] 2 locks held by swapper/1:
>>> [ 0.940171] #0: (net_mutex){+.+.+.}, at: [<ffffffff814e1ad0>] register_pernet_subsys+0x21/0x57
>>> [ 0.942705] #1: (rcu_read_lock_bh){......}, at: [<ffffffff822c7329>] xfrm_net_init+0x1e4/0x437
>>> [ 0.951507]
>>> [ 0.951507] stack backtrace:
>>> [ 0.952660] Pid: 1, comm: swapper Not tainted 3.5.0-01540-g1669891 #64
>>> [ 0.954364] Call Trace:
>>> [ 0.955074] [<ffffffff8108b375>] lockdep_rcu_suspicious+0x174/0x187
>>> [ 0.956736] [<ffffffff822c7453>] xfrm_net_init+0x30e/0x437
>>> [ 0.958205] [<ffffffff822c7329>] ? xfrm_net_init+0x1e4/0x437
>>> [ 0.959712] [<ffffffff814e134a>] ops_init+0x1bb/0x1ff
>>> [ 0.961067] [<ffffffff810861f9>] ? trace_hardirqs_on+0x1b/0x24
>>> [ 0.962644] [<ffffffff814e17cd>] register_pernet_operations.isra.5+0x9d/0xfe
>>> [ 0.971376] [<ffffffff814e1adf>] register_pernet_subsys+0x30/0x57
>>> [ 0.972992] [<ffffffff822c7130>] xfrm_init+0x17/0x2c
>>> [ 0.974316] [<ffffffff822c2f8c>] ip_rt_init+0x82/0xe7
>>> [ 0.975668] [<ffffffff822c31dc>] ip_init+0x10/0x25
>>> [ 0.976952] [<ffffffff822c3f77>] inet_init+0x235/0x360
>>> [ 0.978352] [<ffffffff822c3d42>] ? devinet_init+0xf2/0xf2
>>> [ 0.979808] [<ffffffff82283252>] do_one_initcall+0xb4/0x203
>>> [ 0.981313] [<ffffffff8228354a>] kernel_init+0x1a9/0x29a
>>> [ 0.982732] [<ffffffff822826d9>] ? loglevel+0x46/0x46
>>> [ 0.990889] [<ffffffff816d3d84>] kernel_thread_helper+0x4/0x10
>>> [ 0.992472] [<ffffffff816d262c>] ? retint_restore_args+0x13/0x13
>>> [ 0.994076] [<ffffffff822833a1>] ? do_one_initcall+0x203/0x203
>>> [ 0.995636] [<ffffffff816d3d80>] ? gs_change+0x13/0x13
>>> [ 0.997197] TCP established hash table entries: 8192 (order: 5, 131072 bytes)
>>> [ 1.000074] TCP bind hash table entries: 8192 (order: 7, 655360 bytes)
>>>
>>> Thanks,
>>> Fengguang
>>
>> --
>>
>> Love each day!
>> --fan
>
--
Love each day!
--fan
[-- Attachment #2: 0001-Use-rcu_dereference_bh-to-deference-pointer-protecte.patch --]
[-- Type: text/x-diff, Size: 1072 bytes --]
>From f3b4d84f7ca2ed235c8c8ae5186f45e20b9b80db Mon Sep 17 00:00:00 2001
From: Fan Du <fan.du@windriver.com>
Date: Thu, 16 Aug 2012 17:51:25 +0800
Subject: [PATCH] Use rcu_dereference_bh to deference pointer protected by rcu_read_lock_bh
Signed-off-by: Fan Du <fan.du@windriver.com>
---
net/xfrm/xfrm_policy.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 5ad4d2c..6405764 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2501,11 +2501,11 @@ static void __net_init xfrm_dst_ops_init(struct net *net)
struct xfrm_policy_afinfo *afinfo;
rcu_read_lock_bh();
- afinfo = rcu_dereference(xfrm_policy_afinfo[AF_INET]);
+ afinfo = rcu_dereference_bh(xfrm_policy_afinfo[AF_INET]);
if (afinfo)
net->xfrm.xfrm4_dst_ops = *afinfo->dst_ops;
#if IS_ENABLED(CONFIG_IPV6)
- afinfo = rcu_dereference(xfrm_policy_afinfo[AF_INET6]);
+ afinfo = rcu_dereference_bh(xfrm_policy_afinfo[AF_INET6]);
if (afinfo)
net->xfrm.xfrm6_dst_ops = *afinfo->dst_ops;
#endif
--
1.7.1
^ permalink raw reply related
* Re: suspicious RCU usage in xfrm_net_init()
From: Paul Gortmaker @ 2012-08-17 1:34 UTC (permalink / raw)
To: Fan Du; +Cc: David Miller, Fengguang Wu, Priyanka Jain, netdev, LKML
In-Reply-To: <502D9938.2010908@windriver.com>
On Thu, Aug 16, 2012 at 9:07 PM, Fan Du <fan.du@windriver.com> wrote:
>
>
> On 2012年08月16日 23:19, Fengguang Wu wrote:
>>
>> Hi Fan,
>>
>> On Thu, Aug 16, 2012 at 05:36:35PM +0800, Fan Du wrote:
>>>
>>>
>>> Hi, Fengguang
>>>
>>> Could you please try the below patch, see if spewing still there?
>>> thanks
>>
>>
>> Yes, it worked, thank you very much!
>>
>
> Hi, Dave
>
> Could you please pick up this patch?
Please do not make extra work for maintainers by sending attachments,
or requests for status/merge etc. Your 1st patch had to be manually
set to an RFC, and now you add another patch less than 24h later.
Please see:
http://patchwork.ozlabs.org/patch/177934/
http://patchwork.ozlabs.org/patch/178132/
Also, a patch should describe the problem it solves (i.e. the symptom
the end user sees), and how the problem originated, and why the fix
in the patch is the _right_ fix. The worst description a commit log
can have is one that just describes the C change in words, since
most people can read C on their own.
Here you add "_bh" in the code and then repeat exactly that in
the commit log. Your commit log does not tell me when it broke,
or why it broke, or who had their use case broken. Can you see
why this is not acceptable?
Please take the time to look at the traffic in netdev, and read
the feedback given by maintainers on other patches, so that the
common errors are understood by you, and not repeated. It
will be time well spent!
Thanks,
Paul.
---
> thanks
>
>
>
>
>> btw, your email client wraps long lines..
>>
> Oh, I will definitely fix this.
> thanks feng guang for the testing :)
>
>
>
>> Thanks,
>> Fengguang
>>
>>> From a3f86ecc3ee16ff81d49416bbf791780422988b3 Mon Sep 17 00:00:00 2001
>>> From: Fan Du<fan.du@windriver.com>
>>> Date: Thu, 16 Aug 2012 17:31:25 +0800
>>> Subject: [PATCH] Use rcu_dereference_bh to deference pointer
>>> protected by rcu_read_lock_bh
>>>
>>> Signed-off-by: Fan Du<fan.du@windriver.com>
>>> ---
>>> net/xfrm/xfrm_policy.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
>>> index 5ad4d2c..75a9d6a 100644
>>> --- a/net/xfrm/xfrm_policy.c
>>> +++ b/net/xfrm/xfrm_policy.c
>>> @@ -2501,7 +2501,7 @@ static void __net_init
>>> xfrm_dst_ops_init(struct net *net)
>>> struct xfrm_policy_afinfo *afinfo;
>>>
>>> rcu_read_lock_bh();
>>> - afinfo = rcu_dereference(xfrm_policy_afinfo[AF_INET]);
>>> + afinfo = rcu_dereference_bh(xfrm_policy_afinfo[AF_INET]);
>>> if (afinfo)
>>> net->xfrm.xfrm4_dst_ops = *afinfo->dst_ops;
>>> #if IS_ENABLED(CONFIG_IPV6)
>>> --
>>> 1.7.1
>>>
>>>
>>>
>>>
>>> On 2012年08月16日 15:37, Fengguang Wu wrote:
>>>>
>>>> Hi Priyanka,
>>>>
>>>> The below warning shows up, probably related to this commit:
>>>>
>>>> 418a99ac6ad487dc9c42e6b0e85f941af56330f2 Replace rwlock on
>>>> xfrm_policy_afinfo with rcu
>>>>
>>>> [ 0.921216]
>>>> [ 0.921645] ===============================
>>>> [ 0.922766] [ INFO: suspicious RCU usage. ]
>>>> [ 0.923887] 3.5.0-01540-g1669891 #64 Not tainted
>>>> [ 0.925123] -------------------------------
>>>> [ 0.932860] /c/kernel-tests/src/tip/net/xfrm/xfrm_policy.c:2504
>>>> suspicious rcu_dereference_check() usage!
>>>> [ 0.935361]
>>>> [ 0.935361] other info that might help us debug this:
>>>> [ 0.935361]
>>>> [ 0.937472]
>>>> [ 0.937472] rcu_scheduler_active = 1, debug_locks = 0
>>>> [ 0.939182] 2 locks held by swapper/1:
>>>> [ 0.940171] #0: (net_mutex){+.+.+.}, at: [<ffffffff814e1ad0>]
>>>> register_pernet_subsys+0x21/0x57
>>>> [ 0.942705] #1: (rcu_read_lock_bh){......}, at:
>>>> [<ffffffff822c7329>] xfrm_net_init+0x1e4/0x437
>>>> [ 0.951507]
>>>> [ 0.951507] stack backtrace:
>>>> [ 0.952660] Pid: 1, comm: swapper Not tainted 3.5.0-01540-g1669891
>>>> #64
>>>> [ 0.954364] Call Trace:
>>>> [ 0.955074] [<ffffffff8108b375>] lockdep_rcu_suspicious+0x174/0x187
>>>> [ 0.956736] [<ffffffff822c7453>] xfrm_net_init+0x30e/0x437
>>>> [ 0.958205] [<ffffffff822c7329>] ? xfrm_net_init+0x1e4/0x437
>>>> [ 0.959712] [<ffffffff814e134a>] ops_init+0x1bb/0x1ff
>>>> [ 0.961067] [<ffffffff810861f9>] ? trace_hardirqs_on+0x1b/0x24
>>>> [ 0.962644] [<ffffffff814e17cd>]
>>>> register_pernet_operations.isra.5+0x9d/0xfe
>>>> [ 0.971376] [<ffffffff814e1adf>] register_pernet_subsys+0x30/0x57
>>>> [ 0.972992] [<ffffffff822c7130>] xfrm_init+0x17/0x2c
>>>> [ 0.974316] [<ffffffff822c2f8c>] ip_rt_init+0x82/0xe7
>>>> [ 0.975668] [<ffffffff822c31dc>] ip_init+0x10/0x25
>>>> [ 0.976952] [<ffffffff822c3f77>] inet_init+0x235/0x360
>>>> [ 0.978352] [<ffffffff822c3d42>] ? devinet_init+0xf2/0xf2
>>>> [ 0.979808] [<ffffffff82283252>] do_one_initcall+0xb4/0x203
>>>> [ 0.981313] [<ffffffff8228354a>] kernel_init+0x1a9/0x29a
>>>> [ 0.982732] [<ffffffff822826d9>] ? loglevel+0x46/0x46
>>>> [ 0.990889] [<ffffffff816d3d84>] kernel_thread_helper+0x4/0x10
>>>> [ 0.992472] [<ffffffff816d262c>] ? retint_restore_args+0x13/0x13
>>>> [ 0.994076] [<ffffffff822833a1>] ? do_one_initcall+0x203/0x203
>>>> [ 0.995636] [<ffffffff816d3d80>] ? gs_change+0x13/0x13
>>>> [ 0.997197] TCP established hash table entries: 8192 (order: 5,
>>>> 131072 bytes)
>>>> [ 1.000074] TCP bind hash table entries: 8192 (order: 7, 655360
>>>> bytes)
>>>>
>>>> Thanks,
>>>> Fengguang
>>>
>>>
>>> --
>>>
>>> Love each day!
>>> --fan
>>
>>
>
> --
>
> Love each day!
> --fan
^ permalink raw reply
* RE: [PATCH] net: add new QCA alx ethernet driver
From: Ren, Cloud @ 2012-08-17 1:49 UTC (permalink / raw)
To: Joe Perches
Cc: davem@davemloft.net, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, qca-linux-team, nic-devel,
Huang, Xiong, Rodriguez, Luis
In-Reply-To: <1345122983.26882.9.camel@joe2Laptop>
From: Joe Perches [mailto:joe@perches.com]
Sent: Thursday, August 16, 2012 9:16 PM
>Hi Cloud. Please convert this to
>
> netdev_printk(level, hw->adpt->netdev, "%pV", &vaf);
>
>I will submit a patch soon to make the __netdev_printk variant static.
Ok, thanks.
^ permalink raw reply
* Re: suspicious RCU usage in xfrm_net_init()
From: Fan Du @ 2012-08-17 2:03 UTC (permalink / raw)
To: Paul Gortmaker; +Cc: David Miller, Fengguang Wu, Priyanka Jain, netdev, LKML
In-Reply-To: <CAP=VYLqHW+NOS-gPgUkUpZh-Vim2jkmMT7NK+7uRbg730d=xHw@mail.gmail.com>
On 2012年08月17日 09:34, Paul Gortmaker wrote:
> On Thu, Aug 16, 2012 at 9:07 PM, Fan Du<fan.du@windriver.com> wrote:
>>
>>
>> On 2012年08月16日 23:19, Fengguang Wu wrote:
>>>
>>> Hi Fan,
>>>
>>> On Thu, Aug 16, 2012 at 05:36:35PM +0800, Fan Du wrote:
>>>>
>>>>
>>>> Hi, Fengguang
>>>>
>>>> Could you please try the below patch, see if spewing still there?
>>>> thanks
>>>
>>>
>>> Yes, it worked, thank you very much!
>>>
>>
>> Hi, Dave
>>
>> Could you please pick up this patch?
>
> Please do not make extra work for maintainers by sending attachments,
> or requests for status/merge etc. Your 1st patch had to be manually
> set to an RFC, and now you add another patch less than 24h later.
>
> Please see:
>
> http://patchwork.ozlabs.org/patch/177934/
> http://patchwork.ozlabs.org/patch/178132/
>
> Also, a patch should describe the problem it solves (i.e. the symptom
> the end user sees), and how the problem originated, and why the fix
> in the patch is the _right_ fix. The worst description a commit log
> can have is one that just describes the C change in words, since
> most people can read C on their own.
>
> Here you add "_bh" in the code and then repeat exactly that in
> the commit log. Your commit log does not tell me when it broke,
> or why it broke, or who had their use case broken. Can you see
> why this is not acceptable?
>
> Please take the time to look at the traffic in netdev, and read
> the feedback given by maintainers on other patches, so that the
> common errors are understood by you, and not repeated. It
> will be time well spent!
>
Rick Jones has already well informed me the etiquettes off the list,
which I just broken.
Anyway, thanks for your time writing those suggestions.
> Thanks,
> Paul.
> ---
>
>> thanks
>>
>>
>>
>>
>>> btw, your email client wraps long lines..
>>>
>> Oh, I will definitely fix this.
>> thanks feng guang for the testing :)
>>
>>
>>
>>> Thanks,
>>> Fengguang
>>>
>>>> From a3f86ecc3ee16ff81d49416bbf791780422988b3 Mon Sep 17 00:00:00 2001
>>>> From: Fan Du<fan.du@windriver.com>
>>>> Date: Thu, 16 Aug 2012 17:31:25 +0800
>>>> Subject: [PATCH] Use rcu_dereference_bh to deference pointer
>>>> protected by rcu_read_lock_bh
>>>>
>>>> Signed-off-by: Fan Du<fan.du@windriver.com>
>>>> ---
>>>> net/xfrm/xfrm_policy.c | 2 +-
>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
>>>> index 5ad4d2c..75a9d6a 100644
>>>> --- a/net/xfrm/xfrm_policy.c
>>>> +++ b/net/xfrm/xfrm_policy.c
>>>> @@ -2501,7 +2501,7 @@ static void __net_init
>>>> xfrm_dst_ops_init(struct net *net)
>>>> struct xfrm_policy_afinfo *afinfo;
>>>>
>>>> rcu_read_lock_bh();
>>>> - afinfo = rcu_dereference(xfrm_policy_afinfo[AF_INET]);
>>>> + afinfo = rcu_dereference_bh(xfrm_policy_afinfo[AF_INET]);
>>>> if (afinfo)
>>>> net->xfrm.xfrm4_dst_ops = *afinfo->dst_ops;
>>>> #if IS_ENABLED(CONFIG_IPV6)
>>>> --
>>>> 1.7.1
>>>>
>>>>
>>>>
>>>>
>>>> On 2012年08月16日 15:37, Fengguang Wu wrote:
>>>>>
>>>>> Hi Priyanka,
>>>>>
>>>>> The below warning shows up, probably related to this commit:
>>>>>
>>>>> 418a99ac6ad487dc9c42e6b0e85f941af56330f2 Replace rwlock on
>>>>> xfrm_policy_afinfo with rcu
>>>>>
>>>>> [ 0.921216]
>>>>> [ 0.921645] ===============================
>>>>> [ 0.922766] [ INFO: suspicious RCU usage. ]
>>>>> [ 0.923887] 3.5.0-01540-g1669891 #64 Not tainted
>>>>> [ 0.925123] -------------------------------
>>>>> [ 0.932860] /c/kernel-tests/src/tip/net/xfrm/xfrm_policy.c:2504
>>>>> suspicious rcu_dereference_check() usage!
>>>>> [ 0.935361]
>>>>> [ 0.935361] other info that might help us debug this:
>>>>> [ 0.935361]
>>>>> [ 0.937472]
>>>>> [ 0.937472] rcu_scheduler_active = 1, debug_locks = 0
>>>>> [ 0.939182] 2 locks held by swapper/1:
>>>>> [ 0.940171] #0: (net_mutex){+.+.+.}, at: [<ffffffff814e1ad0>]
>>>>> register_pernet_subsys+0x21/0x57
>>>>> [ 0.942705] #1: (rcu_read_lock_bh){......}, at:
>>>>> [<ffffffff822c7329>] xfrm_net_init+0x1e4/0x437
>>>>> [ 0.951507]
>>>>> [ 0.951507] stack backtrace:
>>>>> [ 0.952660] Pid: 1, comm: swapper Not tainted 3.5.0-01540-g1669891
>>>>> #64
>>>>> [ 0.954364] Call Trace:
>>>>> [ 0.955074] [<ffffffff8108b375>] lockdep_rcu_suspicious+0x174/0x187
>>>>> [ 0.956736] [<ffffffff822c7453>] xfrm_net_init+0x30e/0x437
>>>>> [ 0.958205] [<ffffffff822c7329>] ? xfrm_net_init+0x1e4/0x437
>>>>> [ 0.959712] [<ffffffff814e134a>] ops_init+0x1bb/0x1ff
>>>>> [ 0.961067] [<ffffffff810861f9>] ? trace_hardirqs_on+0x1b/0x24
>>>>> [ 0.962644] [<ffffffff814e17cd>]
>>>>> register_pernet_operations.isra.5+0x9d/0xfe
>>>>> [ 0.971376] [<ffffffff814e1adf>] register_pernet_subsys+0x30/0x57
>>>>> [ 0.972992] [<ffffffff822c7130>] xfrm_init+0x17/0x2c
>>>>> [ 0.974316] [<ffffffff822c2f8c>] ip_rt_init+0x82/0xe7
>>>>> [ 0.975668] [<ffffffff822c31dc>] ip_init+0x10/0x25
>>>>> [ 0.976952] [<ffffffff822c3f77>] inet_init+0x235/0x360
>>>>> [ 0.978352] [<ffffffff822c3d42>] ? devinet_init+0xf2/0xf2
>>>>> [ 0.979808] [<ffffffff82283252>] do_one_initcall+0xb4/0x203
>>>>> [ 0.981313] [<ffffffff8228354a>] kernel_init+0x1a9/0x29a
>>>>> [ 0.982732] [<ffffffff822826d9>] ? loglevel+0x46/0x46
>>>>> [ 0.990889] [<ffffffff816d3d84>] kernel_thread_helper+0x4/0x10
>>>>> [ 0.992472] [<ffffffff816d262c>] ? retint_restore_args+0x13/0x13
>>>>> [ 0.994076] [<ffffffff822833a1>] ? do_one_initcall+0x203/0x203
>>>>> [ 0.995636] [<ffffffff816d3d80>] ? gs_change+0x13/0x13
>>>>> [ 0.997197] TCP established hash table entries: 8192 (order: 5,
>>>>> 131072 bytes)
>>>>> [ 1.000074] TCP bind hash table entries: 8192 (order: 7, 655360
>>>>> bytes)
>>>>>
>>>>> Thanks,
>>>>> Fengguang
>>>>
>>>>
>>>> --
>>>>
>>>> Love each day!
>>>> --fan
>>>
>>>
>>
>> --
>>
>> Love each day!
>> --fan
>
--
Love each day!
--fan
^ permalink raw reply
* Re: suspicious RCU usage in xfrm_net_init()
From: David Miller @ 2012-08-17 2:39 UTC (permalink / raw)
To: fan.du; +Cc: fengguang.wu, Priyanka.Jain, netdev, linux-kernel
In-Reply-To: <502D9938.2010908@windriver.com>
From: Fan Du <fan.du@windriver.com>
Date: Fri, 17 Aug 2012 09:07:04 +0800
> Could you please pick up this patch?
Done, but could you please put proper prefixes in your commit
message subject lines? I had to prepend "xfrm: " this time.
^ permalink raw reply
* Re: suspicious RCU usage in xfrm_net_init()
From: David Miller @ 2012-08-17 2:41 UTC (permalink / raw)
To: paul.gortmaker; +Cc: fan.du, fengguang.wu, Priyanka.Jain, netdev, linux-kernel
In-Reply-To: <CAP=VYLqHW+NOS-gPgUkUpZh-Vim2jkmMT7NK+7uRbg730d=xHw@mail.gmail.com>
From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Thu, 16 Aug 2012 21:34:25 -0400
> Also, a patch should describe the problem it solves (i.e. the symptom
> the end user sees), and how the problem originated, and why the fix
> in the patch is the _right_ fix. The worst description a commit log
> can have is one that just describes the C change in words, since
> most people can read C on their own.
I've frankly given up on Fan Du submitting sophisticated patches
that are easy to review and are properly documented.
Just getting simple things like a WORKING EMAIL ADDRESS was beyond a
struggle.
So when I get a patch that applies, from a properly working email
address, it's an accomplishment.
^ permalink raw reply
* [PATCH 0/3] raid, kmemleak, netfilter: replace list_for_each_continue_rcu with new interface
From: Michael Wang @ 2012-08-17 4:33 UTC (permalink / raw)
To: LKML, linux-raid, linux-mm, netdev@vger.kernel.org, netfilter,
coreteam, netfilter-devel
Cc: neilb, catalin.marinas, David Miller, kaber, pablo,
paulmck@linux.vnet.ibm.com
In-Reply-To: <502CB91E.4050304@linux.vnet.ibm.com>
From: Michael Wang <wangyun@linux.vnet.ibm.com>
This patch set will replace the list_for_each_continue_rcu with the new
interface list_for_each_entry_continue_rcu, so we could remove the old
one later.
Changed:
raid: in "next_active_rdev"
kmemleak: in "kmemleak_seq_next"
netfilter: in "nf_iterate"
Tested:
raid:
mdadm command with an internal bitmap.
kmemleak:
enable kmemleak and check the info it captured.
netfilter:
add rule to iptables and check result by ping.
nfqnl_test which is a test utility of libnetfilter_queue.
All testing are using printk to make sure the code we want test
was invoked.
Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
---
drivers/md/bitmap.c | 9 +++------
mm/kmemleak.c | 6 ++----
net/netfilter/core.c | 11 +++++++----
3 files changed, 12 insertions(+), 14 deletions(-)
--
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
* [PATCH 3/3] netfilter: replace list_for_each_continue_rcu with new interface
From: Michael Wang @ 2012-08-17 4:33 UTC (permalink / raw)
To: LKML, netdev@vger.kernel.org, netfilter, coreteam,
netfilter-devel
Cc: David Miller, kaber, pablo, paulmck@linux.vnet.ibm.com
In-Reply-To: <502CB939.3050008@linux.vnet.ibm.com>
From: Michael Wang <wangyun@linux.vnet.ibm.com>
This patch replaces list_for_each_continue_rcu() with
list_for_each_entry_continue_rcu() to allow removing
list_for_each_continue_rcu().
Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
---
net/netfilter/core.c | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index e19f365..50225bd 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -131,14 +131,14 @@ unsigned int nf_iterate(struct list_head *head,
int hook_thresh)
{
unsigned int verdict;
+ struct nf_hook_ops *elem = list_entry_rcu(*i,
+ struct nf_hook_ops, list);
/*
* The caller must not block between calls to this
* function because of risk of continuing from deleted element.
*/
- list_for_each_continue_rcu(*i, head) {
- struct nf_hook_ops *elem = (struct nf_hook_ops *)*i;
-
+ list_for_each_entry_continue_rcu(elem, head, list) {
if (hook_thresh > elem->priority)
continue;
@@ -155,11 +155,14 @@ repeat:
continue;
}
#endif
- if (verdict != NF_REPEAT)
+ if (verdict != NF_REPEAT) {
+ *i = &elem->list;
return verdict;
+ }
goto repeat;
}
}
+ *i = &elem->list;
return NF_ACCEPT;
}
--
1.7.4.1
^ permalink raw reply related
* [PATCH] xfrm:Use rcu_dereference_bh to deference pointer protected by rcu_read_lock_bh
From: Fan Du @ 2012-08-17 6:19 UTC (permalink / raw)
To: davem; +Cc: fengguang.wu, netdev
418a99ac6ad487dc9c42e6b0e85f941af56330f2 "Replace rwlock on xfrm_policy_afinfo with rcu"
triggers below warnings, which is caused by abusing rcu_dereference_bh with rcu_read_lock.
RCU rules must be honored:
- rcu_dereference_bh paired with rcu_read_lock_bh/rcu_read_unlock_bh
- rcu_dereference paired with rcu_read_lock/rcu_read_unlock
[ 0.921216]
[ 0.921645] ===============================
[ 0.922766] [ INFO: suspicious RCU usage. ]
[ 0.923887] 3.5.0-01540-g1669891 #64 Not tainted
[ 0.925123] -------------------------------
[ 0.932860] /c/kernel-tests/src/tip/net/xfrm/xfrm_policy.c:2504 suspicious rcu_dereference_check() usage!
[ 0.935361]
[ 0.935361] other info that might help us debug this:
[ 0.935361]
[ 0.937472]
[ 0.937472] rcu_scheduler_active = 1, debug_locks = 0
[ 0.939182] 2 locks held by swapper/1:
[ 0.940171] #0: (net_mutex){+.+.+.}, at: [<ffffffff814e1ad0>] register_pernet_subsys+0x21/0x57
[ 0.942705] #1: (rcu_read_lock_bh){......}, at: [<ffffffff822c7329>] xfrm_net_init+0x1e4/0x437
[ 0.951507]
[ 0.951507] stack backtrace:
[ 0.952660] Pid: 1, comm: swapper Not tainted 3.5.0-01540-g1669891 #64
[ 0.954364] Call Trace:
[ 0.955074] [<ffffffff8108b375>] lockdep_rcu_suspicious+0x174/0x187
[ 0.956736] [<ffffffff822c7453>] xfrm_net_init+0x30e/0x437
[ 0.958205] [<ffffffff822c7329>] ? xfrm_net_init+0x1e4/0x437
[ 0.959712] [<ffffffff814e134a>] ops_init+0x1bb/0x1ff
[ 0.961067] [<ffffffff810861f9>] ? trace_hardirqs_on+0x1b/0x24
[ 0.962644] [<ffffffff814e17cd>] register_pernet_operations.isra.5+0x9d/0xfe
[ 0.971376] [<ffffffff814e1adf>] register_pernet_subsys+0x30/0x57
[ 0.972992] [<ffffffff822c7130>] xfrm_init+0x17/0x2c
[ 0.974316] [<ffffffff822c2f8c>] ip_rt_init+0x82/0xe7
[ 0.975668] [<ffffffff822c31dc>] ip_init+0x10/0x25
[ 0.976952] [<ffffffff822c3f77>] inet_init+0x235/0x360
[ 0.978352] [<ffffffff822c3d42>] ? devinet_init+0xf2/0xf2
[ 0.979808] [<ffffffff82283252>] do_one_initcall+0xb4/0x203
[ 0.981313] [<ffffffff8228354a>] kernel_init+0x1a9/0x29a
[ 0.982732] [<ffffffff822826d9>] ? loglevel+0x46/0x46
[ 0.990889] [<ffffffff816d3d84>] kernel_thread_helper+0x4/0x10
[ 0.992472] [<ffffffff816d262c>] ? retint_restore_args+0x13/0x13
[ 0.994076] [<ffffffff822833a1>] ? do_one_initcall+0x203/0x203
[ 0.995636] [<ffffffff816d3d80>] ? gs_change+0x13/0x13
[ 0.997197] TCP established hash table entries: 8192 (order: 5, 131072 bytes)
[ 1.000074] TCP bind hash table entries: 8192 (order: 7, 655360 bytes)
Reported-by: Wu Fengguang <fengguang.wu@intel.com>
Tested-by: Wu Fengguang <fengguang.wu@intel.com>
Signed-off-by: Fan Du <fan.du@windriver.com>
---
net/xfrm/xfrm_policy.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 5ad4d2c..6405764 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2501,11 +2501,11 @@ static void __net_init xfrm_dst_ops_init(struct net *net)
struct xfrm_policy_afinfo *afinfo;
rcu_read_lock_bh();
- afinfo = rcu_dereference(xfrm_policy_afinfo[AF_INET]);
+ afinfo = rcu_dereference_bh(xfrm_policy_afinfo[AF_INET]);
if (afinfo)
net->xfrm.xfrm4_dst_ops = *afinfo->dst_ops;
#if IS_ENABLED(CONFIG_IPV6)
- afinfo = rcu_dereference(xfrm_policy_afinfo[AF_INET6]);
+ afinfo = rcu_dereference_bh(xfrm_policy_afinfo[AF_INET6]);
if (afinfo)
net->xfrm.xfrm6_dst_ops = *afinfo->dst_ops;
#endif
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] xfrm:Use rcu_dereference_bh to deference pointer protected by rcu_read_lock_bh
From: David Miller @ 2012-08-17 6:24 UTC (permalink / raw)
To: fan.du; +Cc: fengguang.wu, netdev
In-Reply-To: <1345184349-5849-1-git-send-email-fan.du@windriver.com>
I already applied your patch, as I told you here:
http://marc.info/?l=linux-netdev&m=134517122805719&w=2
This means you are submitting a patch which doesn't not even apply
to the net-next tree.
Instead of continuing to dig yourself deeper and deeper, take a
break, take a deep breath, and work slowly and carefully.
^ permalink raw reply
* Re: [PATCH] xfrm:Use rcu_dereference_bh to deference pointer protected by rcu_read_lock_bh
From: Fan Du @ 2012-08-17 6:36 UTC (permalink / raw)
To: David Miller; +Cc: fengguang.wu, netdev
In-Reply-To: <20120816.232414.545877018277576779.davem@davemloft.net>
On 2012年08月17日 14:24, David Miller wrote:
>
> I already applied your patch, as I told you here:
>
> http://marc.info/?l=linux-netdev&m=134517122805719&w=2
>
> This means you are submitting a patch which doesn't not even apply
> to the net-next tree.
>
> Instead of continuing to dig yourself deeper and deeper, take a
> break, take a deep breath, and work slowly and carefully.
>
OK, thanks for your kind guidance :)
--
Love each day!
--fan
^ permalink raw reply
* [PATCH 1/2] ipv6: do not hold route table lock when send ndisc probe
From: Cong Wang @ 2012-08-17 7:11 UTC (permalink / raw)
To: netdev
Cc: Cong Wang, Banerjee, Debabrata, David S. Miller,
Hideaki YOSHIFUJI, Patrick McHardy
In rt6_probe(), we call ndisc_send_ns() with root->rwlock,
but this is not necessary, so we can drop it before calling
ndisc_send_ns().
This could probably fix the deadlock reported by Debabrata:
https://lkml.org/lkml/2012/8/16/432
Reported-by: "Banerjee, Debabrata" <dbanerje@akamai.com>
Cc: "Banerjee, Debabrata" <dbanerje@akamai.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Patrick McHardy <kaber@trash.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
net/ipv6/route.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 0ddf2d1..7a36df2 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -460,13 +460,18 @@ static void rt6_probe(struct rt6_info *rt)
time_after(jiffies, neigh->updated + rt->rt6i_idev->cnf.rtr_probe_interval)) {
struct in6_addr mcaddr;
struct in6_addr *target;
+ struct net_device *dev = rt->dst.dev;
+ struct fib6_table *table = rt->rt6i_table;
neigh->updated = jiffies;
read_unlock_bh(&neigh->lock);
+ read_unlock_bh(&table->tb6_lock);
target = (struct in6_addr *)&neigh->primary_key;
addrconf_addr_solict_mult(target, &mcaddr);
- ndisc_send_ns(rt->dst.dev, NULL, target, &mcaddr, NULL);
+ ndisc_send_ns(dev, NULL, target, &mcaddr, NULL);
+
+ read_lock_bh(&table->tb6_lock);
} else {
read_unlock_bh(&neigh->lock);
}
--
1.7.7.6
^ permalink raw reply related
* [PATCH 2/2] ipv6: remove some useless RCU read lock
From: Cong Wang @ 2012-08-17 7:11 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, David S. Miller
In-Reply-To: <1345187499-16929-1-git-send-email-amwang@redhat.com>
After this commit:
commit 97cac0821af4474ec4ba3a9e7a36b98ed9b6db88
Author: David S. Miller <davem@davemloft.net>
Date: Mon Jul 2 22:43:47 2012 -0700
ipv6: Store route neighbour in rt6_info struct.
we no longer use RCU to protect route neighbour.
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
net/ipv6/ip6_output.c | 13 ++-----------
net/ipv6/route.c | 15 ++-------------
2 files changed, 4 insertions(+), 24 deletions(-)
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 5b2d63e..9f67746 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -123,16 +123,11 @@ static int ip6_finish_output2(struct sk_buff *skb)
skb->len);
}
- rcu_read_lock();
rt = (struct rt6_info *) dst;
neigh = rt->n;
- if (neigh) {
- int res = dst_neigh_output(dst, neigh, skb);
+ if (neigh)
+ return dst_neigh_output(dst, neigh, skb);
- rcu_read_unlock();
- return res;
- }
- rcu_read_unlock();
IP6_INC_STATS_BH(dev_net(dst->dev),
ip6_dst_idev(dst), IPSTATS_MIB_OUTNOROUTES);
kfree_skb(skb);
@@ -980,7 +975,6 @@ static int ip6_dst_lookup_tail(struct sock *sk,
* dst entry and replace it instead with the
* dst entry of the nexthop router
*/
- rcu_read_lock();
rt = (struct rt6_info *) *dst;
n = rt->n;
if (n && !(n->nud_state & NUD_VALID)) {
@@ -988,7 +982,6 @@ static int ip6_dst_lookup_tail(struct sock *sk,
struct flowi6 fl_gw6;
int redirect;
- rcu_read_unlock();
ifp = ipv6_get_ifaddr(net, &fl6->saddr,
(*dst)->dev, 1);
@@ -1008,8 +1001,6 @@ static int ip6_dst_lookup_tail(struct sock *sk,
if ((err = (*dst)->error))
goto out_err_release;
}
- } else {
- rcu_read_unlock();
}
#endif
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 7a36df2..0aeeb98 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -451,10 +451,9 @@ static void rt6_probe(struct rt6_info *rt)
* Router Reachability Probe MUST be rate-limited
* to no more than one per minute.
*/
- rcu_read_lock();
neigh = rt ? rt->n : NULL;
if (!neigh || (neigh->nud_state & NUD_VALID))
- goto out;
+ return;
read_lock_bh(&neigh->lock);
if (!(neigh->nud_state & NUD_VALID) &&
time_after(jiffies, neigh->updated + rt->rt6i_idev->cnf.rtr_probe_interval)) {
@@ -475,8 +474,6 @@ static void rt6_probe(struct rt6_info *rt)
} else {
read_unlock_bh(&neigh->lock);
}
-out:
- rcu_read_unlock();
}
#else
static inline void rt6_probe(struct rt6_info *rt)
@@ -503,7 +500,6 @@ static inline int rt6_check_neigh(struct rt6_info *rt)
struct neighbour *neigh;
int m;
- rcu_read_lock();
neigh = rt->n;
if (rt->rt6i_flags & RTF_NONEXTHOP ||
!(rt->rt6i_flags & RTF_GATEWAY))
@@ -521,7 +517,6 @@ static inline int rt6_check_neigh(struct rt6_info *rt)
read_unlock_bh(&neigh->lock);
} else
m = 0;
- rcu_read_unlock();
return m;
}
@@ -2470,15 +2465,11 @@ static int rt6_fill_node(struct net *net,
if (rtnetlink_put_metrics(skb, dst_metrics_ptr(&rt->dst)) < 0)
goto nla_put_failure;
- rcu_read_lock();
n = rt->n;
if (n) {
- if (nla_put(skb, RTA_GATEWAY, 16, &n->primary_key) < 0) {
- rcu_read_unlock();
+ if (nla_put(skb, RTA_GATEWAY, 16, &n->primary_key) < 0)
goto nla_put_failure;
- }
}
- rcu_read_unlock();
if (rt->dst.dev &&
nla_put_u32(skb, RTA_OIF, rt->dst.dev->ifindex))
@@ -2680,14 +2671,12 @@ static int rt6_info_route(struct rt6_info *rt, void *p_arg)
#else
seq_puts(m, "00000000000000000000000000000000 00 ");
#endif
- rcu_read_lock();
n = rt->n;
if (n) {
seq_printf(m, "%pi6", n->primary_key);
} else {
seq_puts(m, "00000000000000000000000000000000");
}
- rcu_read_unlock();
seq_printf(m, " %08x %08x %08x %08x %8s\n",
rt->rt6i_metric, atomic_read(&rt->dst.__refcnt),
rt->dst.__use, rt->rt6i_flags,
--
1.7.7.6
^ permalink raw reply related
* Re: [PATCH v0 5/5] cgroup: Assign subsystem IDs during compile time
From: Li Zefan @ 2012-08-17 7:25 UTC (permalink / raw)
To: Tejun Heo
Cc: Daniel Wagner, netdev-u79uwXL29TY76Z2rM5mHXA,
cgroups-u79uwXL29TY76Z2rM5mHXA, Daniel Wagner, David S. Miller,
Andrew Morton, Eric Dumazet, Gao feng, Glauber Costa,
Jamal Hadi Salim, John Fastabend, Kamezawa Hiroyuki, Neil Horman
In-Reply-To: <20120816232010.GJ24861-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
On 2012/8/17 7:20, Tejun Heo wrote:
> On Thu, Aug 16, 2012 at 04:12:16PM +0200, Daniel Wagner wrote:
>> From: Daniel Wagner <daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org>
>>
>> We are able to safe some space when we assign the subsystem
>> IDs at compile time. Instead of allocating per cgroup
>> cgroup->subsys[CGROUP_SUBSYS_COUNT] where CGROUP_SUBSYS_COUNT is
>> always 64, we allocate 12 + 1 at max (at this point there are 12
>> subsystem).
>
> So, IIUC, this is effectively removing the capability to implement
> modularized controller which isn't known at kernel compile time. Am I
> right?
>
I think so.
> I don't think that's a bad idea but if we're doing that, can't we make
> things even simpler? Do we need to distinguish in-kernel and module
> at all?
>
> Li, what do you think about this?
>
I'm definitely all for simplicity, but I'm not sure if we can do better in
simplifying the code for modularized cgroup subsystem. (I guess you didn't
mean to remove this feature?)
^ permalink raw reply
* Re: [PATCH v0 3/5] cgroup: Protect access to task_cls_classid() when built as module
From: Li Zefan @ 2012-08-17 7:35 UTC (permalink / raw)
To: Daniel Wagner
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, cgroups-u79uwXL29TY76Z2rM5mHXA,
Daniel Wagner, David S. Miller, Gao feng, Jamal Hadi Salim,
John Fastabend, Neil Horman, Tejun Heo
In-Reply-To: <1345126336-20755-4-git-send-email-wagi-kQCPcA+X3s7YtjvyW6yDsg@public.gmane.org>
> +#if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
> +extern struct static_key cgroup_cls_enabled;
> +#define clscg_enabled static_key_false(&cgroup_cls_enabled)
> +#endif
> +
If it's built-in, clscg_enabled is always true (after we call cgroup_subsys_init at
boot), so we don't need jump label at all.
> extern void sock_update_classid(struct sock *sk);
>
> #if IS_BUILTIN(CONFIG_NET_CLS_CGROUP)
> @@ -52,7 +58,7 @@ static inline u32 task_cls_classid(struct task_struct *p)
> int id;
> u32 classid = 0;
>
> - if (in_interrupt())
> + if (!clscg_enabled || in_interrupt())
> return 0;
>
> rcu_read_lock();
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 8f67ced..8d3a400 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -327,6 +327,11 @@ int __sk_backlog_rcv(struct sock *sk, struct sk_buff *skb)
> EXPORT_SYMBOL(__sk_backlog_rcv);
>
> #if defined(CONFIG_CGROUPS)
> +#if IS_ENABLED(CONFIG_NET_CLS_CGROUP)
> +struct static_key cgroup_cls_enabled = STATIC_KEY_INIT_FALSE;
> +EXPORT_SYMBOL_GPL(cgroup_cls_enabled);
> +#endif
> +
> #if !defined(CONFIG_NET_CLS_CGROUP)
> int net_cls_subsys_id = -1;
> EXPORT_SYMBOL_GPL(net_cls_subsys_id);
> diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
> index 7743ea8..f40086b 100644
> --- a/net/sched/cls_cgroup.c
> +++ b/net/sched/cls_cgroup.c
> @@ -44,12 +44,17 @@ static struct cgroup_subsys_state *cgrp_create(struct cgroup *cgrp)
>
> if (cgrp->parent)
> cs->classid = cgrp_cls_state(cgrp->parent)->classid;
> + else if (!clscg_enabled)
> + static_key_slow_inc(&cgroup_cls_enabled);
It's not necessary to check if !clsg_enabled.
>
> return &cs->css;
> }
>
> static void cgrp_destroy(struct cgroup *cgrp)
> {
> + if (!cgrp->parent && clscg_enabled)
> + static_key_slow_dec(&cgroup_cls_enabled);
> +
ditto.
> kfree(cgrp_cls_state(cgrp));
> }
>
>
^ permalink raw reply
* [PATCH 0/3] fix error return code
From: Julia Lawall @ 2012-08-17 7:46 UTC (permalink / raw)
To: netdev; +Cc: kernel-janitors, linux-kernel
These patches fix cases where the return code appears to be unintentially
nonnegative.
The complete semantic match that finds the problem is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@@
identifier ret,l;
expression e1,e2,e3;
statement S;
@@
if (ret < 0)
{ ... return ret; }
... when != ret = e1
when forall
(
goto l;
|
return ...;
|
if (<+... ret = e3 ...+>) S
|
*if(...)
{
... when != ret = e2
* return ret;
}
)
// </smpl>
^ permalink raw reply
* [PATCH 1/3] drivers/net/wimax/i2400m/fw.c: fix error return code
From: Julia Lawall @ 2012-08-17 7:46 UTC (permalink / raw)
To: Inaky Perez-Gonzalez
Cc: netdev, kernel-janitors, linux-kernel, wimax, linux-wimax
In-Reply-To: <1345189618-13758-1-git-send-email-Julia.Lawall@lip6.fr>
From: Julia Lawall <Julia.Lawall@lip6.fr>
Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
identifier ret;
expression e1,e2;
@@
if (ret < 0)
{ ... return ret; }
... when != ret = e1
when forall
*if(...)
{
... when != ret = e2
* return ret;
}
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
drivers/net/wimax/i2400m/fw.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wimax/i2400m/fw.c b/drivers/net/wimax/i2400m/fw.c
index 283237f..def12b3 100644
--- a/drivers/net/wimax/i2400m/fw.c
+++ b/drivers/net/wimax/i2400m/fw.c
@@ -326,8 +326,10 @@ int i2400m_barker_db_init(const char *_options)
unsigned barker;
options_orig = kstrdup(_options, GFP_KERNEL);
- if (options_orig == NULL)
+ if (options_orig == NULL) {
+ result = -ENOMEM;
goto error_parse;
+ }
options = options_orig;
while ((token = strsep(&options, ",")) != NULL) {
^ permalink raw reply related
* [PATCH 2/3] drivers/net/wan/dscc4.c: fix error return code
From: Julia Lawall @ 2012-08-17 7:46 UTC (permalink / raw)
To: Francois Romieu; +Cc: kernel-janitors, netdev, linux-kernel
In-Reply-To: <1345189618-13758-1-git-send-email-Julia.Lawall@lip6.fr>
From: Julia Lawall <Julia.Lawall@lip6.fr>
Move up the initialization of rc so that failure of pci_alloc_consistent
returns -ENOMEM as well.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
identifier ret;
expression e1,e2;
@@
if (ret < 0)
{ ... return ret; }
... when != ret = e1
when forall
*if(...)
{
... when != ret = e2
* return ret;
}
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
drivers/net/wan/dscc4.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wan/dscc4.c b/drivers/net/wan/dscc4.c
index 9eb6479..ef36caf 100644
--- a/drivers/net/wan/dscc4.c
+++ b/drivers/net/wan/dscc4.c
@@ -774,14 +774,15 @@ static int __devinit dscc4_init_one(struct pci_dev *pdev,
}
/* Global interrupt queue */
writel((u32)(((IRQ_RING_SIZE >> 5) - 1) << 20), ioaddr + IQLENR1);
+
+ rc = -ENOMEM;
+
priv->iqcfg = (__le32 *) pci_alloc_consistent(pdev,
IRQ_RING_SIZE*sizeof(__le32), &priv->iqcfg_dma);
if (!priv->iqcfg)
goto err_free_irq_5;
writel(priv->iqcfg_dma, ioaddr + IQCFG);
- rc = -ENOMEM;
-
/*
* SCC 0-3 private rx/tx irq structures
* IQRX/TXi needs to be set soon. Learned it the hard way...
^ permalink raw reply related
* [PATCH 3/3] drivers/net/irda: fix error return code
From: Julia Lawall @ 2012-08-17 7:46 UTC (permalink / raw)
To: Samuel Ortiz; +Cc: kernel-janitors, netdev, linux-kernel
In-Reply-To: <1345189618-13758-1-git-send-email-Julia.Lawall@lip6.fr>
From: Julia Lawall <Julia.Lawall@lip6.fr>
Convert a nonnegative error return code to a negative one, as returned
elsewhere in the function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
identifier ret;
expression e1,e2;
@@
if (ret < 0)
{ ... return ret; }
... when != ret = e1
when forall
*if(...)
{
... when != ret = e2
* return ret;
}
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
drivers/net/irda/ks959-sir.c | 1 +
drivers/net/irda/ksdazzle-sir.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/net/irda/ks959-sir.c b/drivers/net/irda/ks959-sir.c
index 824e2a9..5f3aeac 100644
--- a/drivers/net/irda/ks959-sir.c
+++ b/drivers/net/irda/ks959-sir.c
@@ -542,6 +542,7 @@ static int ks959_net_open(struct net_device *netdev)
sprintf(hwname, "usb#%d", kingsun->usbdev->devnum);
kingsun->irlap = irlap_open(netdev, &kingsun->qos, hwname);
if (!kingsun->irlap) {
+ err = -ENOMEM;
dev_err(&kingsun->usbdev->dev, "irlap_open failed\n");
goto free_mem;
}
diff --git a/drivers/net/irda/ksdazzle-sir.c b/drivers/net/irda/ksdazzle-sir.c
index 5a278ab..2d4b6a1 100644
--- a/drivers/net/irda/ksdazzle-sir.c
+++ b/drivers/net/irda/ksdazzle-sir.c
@@ -436,6 +436,7 @@ static int ksdazzle_net_open(struct net_device *netdev)
sprintf(hwname, "usb#%d", kingsun->usbdev->devnum);
kingsun->irlap = irlap_open(netdev, &kingsun->qos, hwname);
if (!kingsun->irlap) {
+ err = -ENOMEM;
dev_err(&kingsun->usbdev->dev, "irlap_open failed\n");
goto free_mem;
}
^ permalink raw reply related
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