| Message ID | 20100107044741.28605.31414.stgit@localhost.localdomain |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
Jeff Kirsher <jeffrey.t.kirsher@intel.com> writes: > enum ixbge_state_t { > diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c > index 2ad754c..6895de7 100644 > --- a/drivers/net/ixgbe/ixgbe_main.c > +++ b/drivers/net/ixgbe/ixgbe_main.c > @@ -3741,7 +3741,8 @@ static int ixgbe_alloc_q_vectors(struct ixgbe_adapter *adapter) > } > > for (q_idx = 0; q_idx < num_q_vectors; q_idx++) { > - q_vector = kzalloc(sizeof(struct ixgbe_q_vector), GFP_KERNEL); > + q_vector = kzalloc_node(sizeof(struct ixgbe_q_vector), > + GFP_KERNEL, adapter->node); The problem of doing this is that the node might be full or have no memory and k*alloc_node will fail then. So you would need a fallback to be reliable (we probably should have a generic utility function for this somewhere, but we don't currently) -Andi
>-----Original Message----- >From: Andi Kleen [mailto:andi@firstfloor.org] >Sent: Thursday, January 14, 2010 3:02 AM >To: Kirsher, Jeffrey T >Cc: davem@davemloft.net; netdev@vger.kernel.org; gospo@redhat.com; >Waskiewicz Jr, Peter P >Subject: Re: [net-next-2.6 PATCH 1/5] ixgbe: Allocate driver resources >per NUMA node > >Jeff Kirsher <jeffrey.t.kirsher@intel.com> writes: >> enum ixbge_state_t { >> diff --git a/drivers/net/ixgbe/ixgbe_main.c >b/drivers/net/ixgbe/ixgbe_main.c >> index 2ad754c..6895de7 100644 >> --- a/drivers/net/ixgbe/ixgbe_main.c >> +++ b/drivers/net/ixgbe/ixgbe_main.c >> @@ -3741,7 +3741,8 @@ static int ixgbe_alloc_q_vectors(struct >ixgbe_adapter *adapter) >> } >> >> for (q_idx = 0; q_idx < num_q_vectors; q_idx++) { >> - q_vector = kzalloc(sizeof(struct ixgbe_q_vector), >GFP_KERNEL); >> + q_vector = kzalloc_node(sizeof(struct ixgbe_q_vector), >> + GFP_KERNEL, adapter->node); > > >The problem of doing this is that the node might be full or have >no memory and k*alloc_node will fail then. > >So you would need a fallback to be reliable (we probably should have a >generic utility function for this somewhere, but we don't currently) So you'd rather see us call kzalloc() if kzalloc_node() fails, instead of immediately failing out to err_out? Thanks, -PJ -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> >The problem of doing this is that the node might be full or have > >no memory and k*alloc_node will fail then. > > > >So you would need a fallback to be reliable (we probably should have a > >generic utility function for this somewhere, but we don't currently) > > So you'd rather see us call kzalloc() if kzalloc_node() fails, instead of immediately failing out to err_out? Yes. After all it's only an optimization to place memory, it's not a functional requirement. -Andi
>-----Original Message----- >From: Andi Kleen [mailto:andi@firstfloor.org] >Sent: Tuesday, January 19, 2010 11:34 AM >To: Waskiewicz Jr, Peter P >Cc: Andi Kleen; Kirsher, Jeffrey T; davem@davemloft.net; >netdev@vger.kernel.org; gospo@redhat.com >Subject: Re: [net-next-2.6 PATCH 1/5] ixgbe: Allocate driver resources >per NUMA node > >> >The problem of doing this is that the node might be full or have >> >no memory and k*alloc_node will fail then. >> > >> >So you would need a fallback to be reliable (we probably should have >a >> >generic utility function for this somewhere, but we don't currently) >> >> So you'd rather see us call kzalloc() if kzalloc_node() fails, instead >of immediately failing out to err_out? > >Yes. > >After all it's only an optimization to place memory, it's not >a functional requirement. > >-Andi > Makes sense. I have the change in my local tree and will push new patches through Jeff shortly. Thanks Andi, -PJ -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ixgbe/ixgbe.h b/drivers/net/ixgbe/ixgbe.h index 8da8eb5..998b8d9 100644 --- a/drivers/net/ixgbe/ixgbe.h +++ b/drivers/net/ixgbe/ixgbe.h @@ -379,6 +379,8 @@ struct ixgbe_adapter { u64 rsc_total_flush; u32 wol; u16 eeprom_version; + + int node; }; enum ixbge_state_t { diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c index 2ad754c..6895de7 100644 --- a/drivers/net/ixgbe/ixgbe_main.c +++ b/drivers/net/ixgbe/ixgbe_main.c @@ -3741,7 +3741,8 @@ static int ixgbe_alloc_q_vectors(struct ixgbe_adapter *adapter) } for (q_idx = 0; q_idx < num_q_vectors; q_idx++) { - q_vector = kzalloc(sizeof(struct ixgbe_q_vector), GFP_KERNEL); + q_vector = kzalloc_node(sizeof(struct ixgbe_q_vector), + GFP_KERNEL, adapter->node); if (!q_vector) goto err_out; q_vector->adapter = adapter; @@ -4041,6 +4042,9 @@ static int __devinit ixgbe_sw_init(struct ixgbe_adapter *adapter) /* enable rx csum by default */ adapter->flags |= IXGBE_FLAG_RX_CSUM_ENABLED; + /* get assigned NUMA node */ + adapter->node = dev_to_node(&pdev->dev); + set_bit(__IXGBE_DOWN, &adapter->state); return 0; @@ -4060,7 +4064,7 @@ int ixgbe_setup_tx_resources(struct ixgbe_adapter *adapter, int size; size = sizeof(struct ixgbe_tx_buffer) * tx_ring->count; - tx_ring->tx_buffer_info = vmalloc(size); + tx_ring->tx_buffer_info = vmalloc_node(size, adapter->node); if (!tx_ring->tx_buffer_info) goto err; memset(tx_ring->tx_buffer_info, 0, size); @@ -4100,8 +4104,15 @@ err: static int ixgbe_setup_all_tx_resources(struct ixgbe_adapter *adapter) { int i, err = 0; + int orig_node = adapter->node; for (i = 0; i < adapter->num_tx_queues; i++) { + if (orig_node == -1) { + int cur_node = next_online_node(adapter->node); + if (cur_node == MAX_NUMNODES) + cur_node = first_online_node; + adapter->node = cur_node; + } err = ixgbe_setup_tx_resources(adapter, &adapter->tx_ring[i]); if (!err) continue; @@ -4109,6 +4120,9 @@ static int ixgbe_setup_all_tx_resources(struct ixgbe_adapter *adapter) break; } + /* reset the node back to its starting value */ + adapter->node = orig_node; + return err; } @@ -4126,7 +4140,7 @@ int ixgbe_setup_rx_resources(struct ixgbe_adapter *adapter, int size; size = sizeof(struct ixgbe_rx_buffer) * rx_ring->count; - rx_ring->rx_buffer_info = vmalloc(size); + rx_ring->rx_buffer_info = vmalloc_node(size, adapter->node); if (!rx_ring->rx_buffer_info) { DPRINTK(PROBE, ERR, "vmalloc allocation failed for the rx desc ring\n"); @@ -4170,8 +4184,15 @@ alloc_failed: static int ixgbe_setup_all_rx_resources(struct ixgbe_adapter *adapter) { int i, err = 0; + int orig_node = adapter->node; for (i = 0; i < adapter->num_rx_queues; i++) { + if (orig_node == -1) { + int cur_node = next_online_node(adapter->node); + if (cur_node == MAX_NUMNODES) + cur_node = first_online_node; + adapter->node = cur_node; + } err = ixgbe_setup_rx_resources(adapter, &adapter->rx_ring[i]); if (!err) continue; @@ -4179,6 +4200,9 @@ static int ixgbe_setup_all_rx_resources(struct ixgbe_adapter *adapter) break; } + /* reset the node back to its starting value */ + adapter->node = orig_node; + return err; }