Messages in this thread | | | From | Ben Hutchings <> | Date | Sat, 20 Dec 2008 23:22:39 +0000 | Subject | Re: [PATCH 7/8] cpumask: convert misc driver functions |
| |
On Fri, 2008-12-19 at 08:01 -0800, Mike Travis wrote: [...] > --- linux-2.6-for-ingo.orig/drivers/net/sfc/efx.c > +++ linux-2.6-for-ingo/drivers/net/sfc/efx.c > @@ -809,19 +809,18 @@ static void efx_fini_io(struct efx_nic * > /* Get number of RX queues wanted. Return number of online CPU > * packages in the expectation that an IRQ balancer will spread > * interrupts across them. */ > -static int efx_wanted_rx_queues(void) > +static int efx_wanted_rx_queues(struct cpumask *scratch) > { > - cpumask_t core_mask; > int count; > int cpu; > > - cpus_clear(core_mask); > + cpumask_clear(scratch); > count = 0; > for_each_online_cpu(cpu) { > - if (!cpu_isset(cpu, core_mask)) { > + if (!cpumask_test_cpu(cpu, scratch)) { > ++count; > - cpus_or(core_mask, core_mask, > - topology_core_siblings(cpu)); > + cpumask_or(scratch, scratch, > + topology_core_cpumask(cpu)); > } > } > > @@ -831,7 +830,7 @@ static int efx_wanted_rx_queues(void) > /* Probe the number and type of interrupts we are able to obtain, and > * the resulting numbers of channels and RX queues. > */ > -static void efx_probe_interrupts(struct efx_nic *efx) > +static void efx_probe_interrupts(struct efx_nic *efx, struct cpumask *scratch) > { > int max_channels = > min_t(int, efx->type->phys_addr_channels, EFX_MAX_CHANNELS); > @@ -845,7 +844,8 @@ static void efx_probe_interrupts(struct > * (or as specified by the rss_cpus module parameter). > * We will need one channel per interrupt. > */ > - wanted_ints = rss_cpus ? rss_cpus : efx_wanted_rx_queues(); > + wanted_ints = rss_cpus ? rss_cpus : > + efx_wanted_rx_queues(scratch); > efx->n_rx_queues = min(wanted_ints, max_channels); > > for (i = 0; i < efx->n_rx_queues; i++) > @@ -923,24 +923,29 @@ static void efx_set_channels(struct efx_ > static int efx_probe_nic(struct efx_nic *efx) > { > int rc; > + cpumask_var_t scratch; > > EFX_LOG(efx, "creating NIC\n"); > > + if (!alloc_cpumask_var(&scratch, GFP_KERNEL)) > + return -ENOMEM; > + > /* Carry out hardware-type specific initialisation */ > rc = falcon_probe_nic(efx); > if (rc) > - return rc; > + goto out; > > /* Determine the number of channels and RX queues by trying to hook > * in MSI-X interrupts. */ > - efx_probe_interrupts(efx); > + efx_probe_interrupts(efx, scratch); > > efx_set_channels(efx); > > /* Initialise the interrupt moderation settings */ > efx_init_irq_moderation(efx, tx_irq_mod_usec, rx_irq_mod_usec); > - > - return 0; > +out: > + free_cpumask_var(scratch); > + return rc; > } > > static void efx_remove_nic(struct efx_nic *efx) [...]
I assume you're moving the allocation up to efx_probe_nic() because efx_wanted_rx_queues() and efx_probe_interrupts() cannot return failure. It really isn't worth exposing that detail up the call chain though. I think it's acceptable for efx_wanted_rx_queues() to log an error message and return 1 in the exceedingly unlikely case that the allocation fails.
Ben.
-- Ben Hutchings, Senior Software Engineer, Solarflare Communications Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked.
| |