* [PATCH] add cpu on/offline support in Fibre Channel exchange and sequence handling
@ 2010-10-08 14:57 Hillf Danton
2010-10-11 22:03 ` Zou, Yi
2010-10-11 22:08 ` [Open-FCoE] " Robert Love
0 siblings, 2 replies; 6+ messages in thread
From: Hillf Danton @ 2010-10-08 14:57 UTC (permalink / raw)
To: Mike Christie, Robert Love; +Cc: devel, linux-scsi
Fibre Channel exchange pool is setup in a per cpu way that
entire exchange id range is divided equally across all cpus.
When cpu goes offline, the corresponding range of exchg id
becomes unavailable until it is online again.
This work tries to fix the unavailability based on notifier_block.
Signed-off-by: Hillf Danton <dhillf@gmail.com>
---
--- o/linux-2.6.36-rc4/drivers/scsi/libfc/fc_exch.c 2010-09-13
07:07:38.000000000 +0800
+++ m/linux-2.6.36-rc4/drivers/scsi/libfc/fc_exch.c 2010-10-08
22:24:48.000000000 +0800
@@ -26,6 +26,9 @@
#include <linux/timer.h>
#include <linux/slab.h>
#include <linux/err.h>
+#include <linux/notifier.h>
+#include <linux/cpu.h>
+#include <linux/cpumask.h>
#include <scsi/fc/fc_fc2.h>
@@ -40,6 +43,8 @@ static u16 fc_cpu_order; /* 2's power to
static struct kmem_cache *fc_em_cachep; /* cache for exchanges */
struct workqueue_struct *fc_exch_workqueue;
+static cpumask_t fc_offline_cpu_mask = CPU_MASK_NONE;
+
/*
* Structure and function definitions for managing Fibre Channel Exchanges
* and Sequences.
@@ -666,6 +671,7 @@ static struct fc_exch *fc_exch_em_alloc(
unsigned int cpu;
u16 index;
struct fc_exch_pool *pool;
+ unsigned int index = -1;
/* allocate memory for exchange */
ep = mempool_alloc(mp->ep_pool, GFP_ATOMIC);
@@ -679,6 +685,7 @@ static struct fc_exch *fc_exch_em_alloc(
pool = per_cpu_ptr(mp->pool, cpu);
spin_lock_bh(&pool->lock);
put_cpu();
+again:
index = pool->next_index;
/* allocate new exch from pool */
while (fc_exch_ptr_get(pool, index)) {
@@ -719,6 +726,17 @@ out:
err:
spin_unlock_bh(&pool->lock);
atomic_inc(&mp->stats.no_free_exch_xid);
+
+ cpu = index = cpumask_next(index, &fc_offline_cpu_mask);
+ if (cpu < nr_cpu_ids) {
+ /*
+ * try to take the share of offline cpus
+ */
+ pool = per_cpu_ptr(mp->pool, cpu);
+ spin_lock_bh(&pool->lock);
+ goto again;
+ }
+
mempool_free(ep, mp->ep_pool);
return NULL;
}
@@ -2322,6 +2340,34 @@ int fc_exch_init(struct fc_lport *lport)
}
EXPORT_SYMBOL(fc_exch_init);
+/*
+ * cpu on/offline support
+ **/
+
+static int fc_cpu_callback(struct notifier_block *nb,
+ unsigned long event,
+ void *_cpu)
+{
+ int cpu = (unsigned long)_cpu;
+
+ switch (event) {
+ case CPU_ONLINE:
+ case CPU_ONLINE_FROZEN:
+ cpumask_clear_cpu(cpu, &fc_offline_cpu_mask);
+ break;
+ case CPU_DEAD:
+ case CPU_DEAD_FROZEN:
+ cpumask_set_cpu(cpu, &fc_offline_cpu_mask);
+ break;
+ }
+
+ return NOTIFY_OK;
+}
+
+static struct notifier_block fc_nb = {
+ .notifier_call = fc_cpu_callback,
+};
+
/**
* fc_setup_exch_mgr() - Setup an exchange manager
*/
@@ -2357,6 +2403,9 @@ int fc_setup_exch_mgr()
fc_exch_workqueue = create_singlethread_workqueue("fc_exch_workqueue");
if (!fc_exch_workqueue)
return -ENOMEM;
+
+ register_cpu_notifier(&fc_nb);
+
return 0;
}
@@ -2367,4 +2416,5 @@ void fc_destroy_exch_mgr()
{
destroy_workqueue(fc_exch_workqueue);
kmem_cache_destroy(fc_em_cachep);
+ unregister_cpu_notifier(&fc_nb);
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] add cpu on/offline support in Fibre Channel exchange and sequence handling
2010-10-08 14:57 [PATCH] add cpu on/offline support in Fibre Channel exchange and sequence handling Hillf Danton
@ 2010-10-11 22:03 ` Zou, Yi
2010-10-11 22:08 ` [Open-FCoE] " Robert Love
1 sibling, 0 replies; 6+ messages in thread
From: Zou, Yi @ 2010-10-11 22:03 UTC (permalink / raw)
To: Hillf Danton, Mike Christie, Love, Robert W
Cc: devel@open-fcoe.org, linux-scsi@vger.kernel.org
>
> Fibre Channel exchange pool is setup in a per cpu way that
> entire exchange id range is divided equally across all cpus.
>
> When cpu goes offline, the corresponding range of exchg id
> becomes unavailable until it is online again.
>
> This work tries to fix the unavailability based on notifier_block.
This line does not tell exactly how you've fixed the unavailability,
which I am guessing by "borrowing exchange id from the offline CPUs'
exchange id pool?"
I think you also need to point out this would break the assumed I/O
Alignment on CPU based on the per cpu xid pool, I guess it's better
than no xid at all.
Some more comments inline below.
>
> Signed-off-by: Hillf Danton <dhillf@gmail.com>
> ---
>
> --- o/linux-2.6.36-rc4/drivers/scsi/libfc/fc_exch.c 2010-09-13
> 07:07:38.000000000 +0800
> +++ m/linux-2.6.36-rc4/drivers/scsi/libfc/fc_exch.c 2010-10-08
> 22:24:48.000000000 +0800
> @@ -26,6 +26,9 @@
> #include <linux/timer.h>
> #include <linux/slab.h>
> #include <linux/err.h>
> +#include <linux/notifier.h>
> +#include <linux/cpu.h>
> +#include <linux/cpumask.h>
>
> #include <scsi/fc/fc_fc2.h>
>
> @@ -40,6 +43,8 @@ static u16 fc_cpu_order; /* 2's power to
> static struct kmem_cache *fc_em_cachep; /* cache for exchanges
> */
> struct workqueue_struct *fc_exch_workqueue;
>
> +static cpumask_t fc_offline_cpu_mask = CPU_MASK_NONE;
> +
> /*
> * Structure and function definitions for managing Fibre Channel
> Exchanges
> * and Sequences.
> @@ -666,6 +671,7 @@ static struct fc_exch *fc_exch_em_alloc(
> unsigned int cpu;
> u16 index;
> struct fc_exch_pool *pool;
> + unsigned int index = -1;
>
> /* allocate memory for exchange */
> ep = mempool_alloc(mp->ep_pool, GFP_ATOMIC);
> @@ -679,6 +685,7 @@ static struct fc_exch *fc_exch_em_alloc(
> pool = per_cpu_ptr(mp->pool, cpu);
> spin_lock_bh(&pool->lock);
> put_cpu();
> +again:
> index = pool->next_index;
> /* allocate new exch from pool */
> while (fc_exch_ptr_get(pool, index)) {
> @@ -719,6 +726,17 @@ out:
> err:
> spin_unlock_bh(&pool->lock);
> atomic_inc(&mp->stats.no_free_exch_xid);
> +
> + cpu = index = cpumask_next(index, &fc_offline_cpu_mask);
Is index needed here? Why not just:
cpu = cpumask_next(cpu, &fc_offline_cpu_mask);
Index here refers to the next available exchange id in the pool.
Exchange id.
> + if (cpu < nr_cpu_ids) {
> + /*
> + * try to take the share of offline cpus
> + */
> + pool = per_cpu_ptr(mp->pool, cpu);
> + spin_lock_bh(&pool->lock);
> + goto again;
Will we ever get stuck here if the cpu is toggled online/offline
repeatedly? Here nothing protects fc_offline_cpu_mask, so it may
be back online while you are trying to borrow xid from its pool,
which I think may be alright.
- yi
> + }
> +
> mempool_free(ep, mp->ep_pool);
> return NULL;
> }
> @@ -2322,6 +2340,34 @@ int fc_exch_init(struct fc_lport *lport)
> }
> EXPORT_SYMBOL(fc_exch_init);
>
> +/*
> + * cpu on/offline support
> + **/
> +
> +static int fc_cpu_callback(struct notifier_block *nb,
> + unsigned long event,
> + void *_cpu)
> +{
> + int cpu = (unsigned long)_cpu;
> +
> + switch (event) {
> + case CPU_ONLINE:
> + case CPU_ONLINE_FROZEN:
> + cpumask_clear_cpu(cpu, &fc_offline_cpu_mask);
> + break;
> + case CPU_DEAD:
> + case CPU_DEAD_FROZEN:
> + cpumask_set_cpu(cpu, &fc_offline_cpu_mask);
> + break;
> + }
> +
> + return NOTIFY_OK;
> +}
> +
> +static struct notifier_block fc_nb = {
> + .notifier_call = fc_cpu_callback,
> +};
> +
> /**
> * fc_setup_exch_mgr() - Setup an exchange manager
> */
> @@ -2357,6 +2403,9 @@ int fc_setup_exch_mgr()
> fc_exch_workqueue =
> create_singlethread_workqueue("fc_exch_workqueue");
> if (!fc_exch_workqueue)
> return -ENOMEM;
> +
> + register_cpu_notifier(&fc_nb);
> +
> return 0;
> }
>
> @@ -2367,4 +2416,5 @@ void fc_destroy_exch_mgr()
> {
> destroy_workqueue(fc_exch_workqueue);
> kmem_cache_destroy(fc_em_cachep);
> + unregister_cpu_notifier(&fc_nb);
> }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Open-FCoE] [PATCH] add cpu on/offline support in Fibre Channel exchange and sequence handling
2010-10-08 14:57 [PATCH] add cpu on/offline support in Fibre Channel exchange and sequence handling Hillf Danton
2010-10-11 22:03 ` Zou, Yi
@ 2010-10-11 22:08 ` Robert Love
2010-10-11 22:49 ` Nicholas A. Bellinger
2010-10-14 14:55 ` Hillf Danton
1 sibling, 2 replies; 6+ messages in thread
From: Robert Love @ 2010-10-11 22:08 UTC (permalink / raw)
To: Hillf Danton; +Cc: Mike Christie, devel, linux-scsi
On Fri, 2010-10-08 at 22:57 +0800, Hillf Danton wrote:
Hi Hillf,
Can you please prefix the libfc patches with "libfc: <title>"? Also, for
your other patches that have a "SCSI" prefix, I think James' system adds
the "[SCSI]" prefix when he applies patches so you don't need to add it.
> Fibre Channel exchange pool is setup in a per cpu way that
> entire exchange id range is divided equally across all cpus.
>
> When cpu goes offline, the corresponding range of exchg id
> becomes unavailable until it is online again.
>
> This work tries to fix the unavailability based on notifier_block.
>
> Signed-off-by: Hillf Danton <dhillf@gmail.com>
> ---
>
> --- o/linux-2.6.36-rc4/drivers/scsi/libfc/fc_exch.c 2010-09-13
> 07:07:38.000000000 +0800
> +++ m/linux-2.6.36-rc4/drivers/scsi/libfc/fc_exch.c 2010-10-08
> 22:24:48.000000000 +0800
Most patches I've seen on the various mailing lists are created by
diffing the files within the kernel directory, so that you don't have
the "/linux-2.6.36-rc/" in each hunk. It's the difference between using
'patch -p1' or 'patch -p2' when applying the patch.
I think this is preventing git-am from applying the patches. To import
them I ended up having git-am fail and then manually using the 'patch
-p2' command, so that I could get both the commit message from git-am
and then the patch itself from 'patch'. I don't know about anyone else,
but if you post patches without the kernel directory in the diff line it
would help me out. Thanks.
> @@ -26,6 +26,9 @@
> #include <linux/timer.h>
> #include <linux/slab.h>
> #include <linux/err.h>
> +#include <linux/notifier.h>
> +#include <linux/cpu.h>
> +#include <linux/cpumask.h>
>
> #include <scsi/fc/fc_fc2.h>
>
> @@ -40,6 +43,8 @@ static u16 fc_cpu_order; /* 2's power to
> static struct kmem_cache *fc_em_cachep; /* cache for exchanges */
> struct workqueue_struct *fc_exch_workqueue;
>
> +static cpumask_t fc_offline_cpu_mask = CPU_MASK_NONE;
> +
> /*
> * Structure and function definitions for managing Fibre Channel Exchanges
> * and Sequences.
> @@ -666,6 +671,7 @@ static struct fc_exch *fc_exch_em_alloc(
> unsigned int cpu;
> u16 index;
> struct fc_exch_pool *pool;
> + unsigned int index = -1;
This patch doesn't compile:
drivers/scsi/libfc/fc_exch.c: In function ‘fc_exch_em_alloc’:
drivers/scsi/libfc/fc_exch.c:674: error: conflicting types for ‘index’
drivers/scsi/libfc/fc_exch.c:672: note: previous declaration of ‘index’
was here
make[3]: *** [drivers/scsi/libfc/fc_exch.o] Error 1
In general I think the idea is fine. I haven't run into any
circumstances where the per-CPU pools are exhausted. I know others have
when using NPIV, but there were other ways to fix those issues.
>
> /* allocate memory for exchange */
> ep = mempool_alloc(mp->ep_pool, GFP_ATOMIC);
> @@ -679,6 +685,7 @@ static struct fc_exch *fc_exch_em_alloc(
> pool = per_cpu_ptr(mp->pool, cpu);
> spin_lock_bh(&pool->lock);
> put_cpu();
> +again:
> index = pool->next_index;
> /* allocate new exch from pool */
> while (fc_exch_ptr_get(pool, index)) {
> @@ -719,6 +726,17 @@ out:
> err:
> spin_unlock_bh(&pool->lock);
> atomic_inc(&mp->stats.no_free_exch_xid);
I think this should be moved below the offline pool check, when we've
really failed to allocate an exchange. It's not a per-CPU pool stat,
it's an overall resource allocation failure stat, which you're now
helping to happen less frequently. With the current placement we'll be
bumping up the counter even though we have allocated an exchange.
Thanks, //Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Open-FCoE] [PATCH] add cpu on/offline support in Fibre Channel exchange and sequence handling
2010-10-11 22:08 ` [Open-FCoE] " Robert Love
@ 2010-10-11 22:49 ` Nicholas A. Bellinger
2010-10-11 23:23 ` Robert Love
2010-10-14 14:55 ` Hillf Danton
1 sibling, 1 reply; 6+ messages in thread
From: Nicholas A. Bellinger @ 2010-10-11 22:49 UTC (permalink / raw)
To: Robert Love; +Cc: Hillf Danton, Mike Christie, devel, linux-scsi
On Mon, 2010-10-11 at 15:08 -0700, Robert Love wrote:
> On Fri, 2010-10-08 at 22:57 +0800, Hillf Danton wrote:
>
> Hi Hillf,
>
> Can you please prefix the libfc patches with "libfc: <title>"? Also, for
> your other patches that have a "SCSI" prefix, I think James' system adds
> the "[SCSI]" prefix when he applies patches so you don't need to add it.
>
> > Fibre Channel exchange pool is setup in a per cpu way that
> > entire exchange id range is divided equally across all cpus.
> >
> > When cpu goes offline, the corresponding range of exchg id
> > becomes unavailable until it is online again.
> >
> > This work tries to fix the unavailability based on notifier_block.
> >
> > Signed-off-by: Hillf Danton <dhillf@gmail.com>
> > ---
> >
> > --- o/linux-2.6.36-rc4/drivers/scsi/libfc/fc_exch.c 2010-09-13
> > 07:07:38.000000000 +0800
> > +++ m/linux-2.6.36-rc4/drivers/scsi/libfc/fc_exch.c 2010-10-08
> > 22:24:48.000000000 +0800
>
> Most patches I've seen on the various mailing lists are created by
> diffing the files within the kernel directory, so that you don't have
> the "/linux-2.6.36-rc/" in each hunk. It's the difference between using
> 'patch -p1' or 'patch -p2' when applying the patch.
>
> I think this is preventing git-am from applying the patches. To import
> them I ended up having git-am fail and then manually using the 'patch
> -p2' command, so that I could get both the commit message from git-am
> and then the patch itself from 'patch'. I don't know about anyone else,
> but if you post patches without the kernel directory in the diff line it
> would help me out. Thanks.
>
Hmmm, I am not sure about how to do this with git-am, but when I do this
between lio-core-2.6.git/lio-4.0 -> lio-core-backports.git patches, I
use:
'git-apply --index --verbose --directory=$DIR_PREFIX'
which is described in the git-apply manual here:
--directory=<root>
Prepend <root> to all filenames. If a "-p" argument was also
passed, it is applied before prepending the new root.
For example, a patch that talks about updating a/git-gui.sh to
b/git-gui.sh can be applied to the file in the working tree
modules/git-gui/git-gui.sh by running git apply
--directory=modules/git-gui.
Best,
--nab
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Open-FCoE] [PATCH] add cpu on/offline support in Fibre Channel exchange and sequence handling
2010-10-11 22:49 ` Nicholas A. Bellinger
@ 2010-10-11 23:23 ` Robert Love
0 siblings, 0 replies; 6+ messages in thread
From: Robert Love @ 2010-10-11 23:23 UTC (permalink / raw)
To: Nicholas A. Bellinger; +Cc: Hillf Danton, linux-scsi, devel
On Mon, 2010-10-11 at 15:49 -0700, Nicholas A. Bellinger wrote:
> On Mon, 2010-10-11 at 15:08 -0700, Robert Love wrote:
> > On Fri, 2010-10-08 at 22:57 +0800, Hillf Danton wrote:
> >
> > Hi Hillf,
> >
> > Can you please prefix the libfc patches with "libfc: <title>"? Also, for
> > your other patches that have a "SCSI" prefix, I think James' system adds
> > the "[SCSI]" prefix when he applies patches so you don't need to add it.
> >
> > > Fibre Channel exchange pool is setup in a per cpu way that
> > > entire exchange id range is divided equally across all cpus.
> > >
> > > When cpu goes offline, the corresponding range of exchg id
> > > becomes unavailable until it is online again.
> > >
> > > This work tries to fix the unavailability based on notifier_block.
> > >
> > > Signed-off-by: Hillf Danton <dhillf@gmail.com>
> > > ---
> > >
> > > --- o/linux-2.6.36-rc4/drivers/scsi/libfc/fc_exch.c 2010-09-13
> > > 07:07:38.000000000 +0800
> > > +++ m/linux-2.6.36-rc4/drivers/scsi/libfc/fc_exch.c 2010-10-08
> > > 22:24:48.000000000 +0800
> >
> > Most patches I've seen on the various mailing lists are created by
> > diffing the files within the kernel directory, so that you don't have
> > the "/linux-2.6.36-rc/" in each hunk. It's the difference between using
> > 'patch -p1' or 'patch -p2' when applying the patch.
> >
> > I think this is preventing git-am from applying the patches. To import
> > them I ended up having git-am fail and then manually using the 'patch
> > -p2' command, so that I could get both the commit message from git-am
> > and then the patch itself from 'patch'. I don't know about anyone else,
> > but if you post patches without the kernel directory in the diff line it
> > would help me out. Thanks.
> >
>
> Hmmm, I am not sure about how to do this with git-am, but when I do this
> between lio-core-2.6.git/lio-4.0 -> lio-core-backports.git patches, I
> use:
>
> 'git-apply --index --verbose --directory=$DIR_PREFIX'
>
> which is described in the git-apply manual here:
>
> --directory=<root>
>
> Prepend <root> to all filenames. If a "-p" argument was also
> passed, it is applied before prepending the new root.
>
> For example, a patch that talks about updating a/git-gui.sh to
> b/git-gui.sh can be applied to the file in the working tree
> modules/git-gui/git-gui.sh by running git apply
> --directory=modules/git-gui.
>
Thanks nab. While looking at '--directory' I just found that there is a
'-p' option to both git-am and git-apply. It seems like it should do the
trick. Unfortunately even with that flag it still doesn't apply.
fatal: patch fragment without header at line 7: @@ -26,6 +26,9 @@
I think the problem is with the timestamp after the path that describes
the affected file. If I remove that timestamp I can get it to apply with
'git-am -p2'.
//Rob
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Open-FCoE] [PATCH] add cpu on/offline support in Fibre Channel exchange and sequence handling
2010-10-11 22:08 ` [Open-FCoE] " Robert Love
2010-10-11 22:49 ` Nicholas A. Bellinger
@ 2010-10-14 14:55 ` Hillf Danton
1 sibling, 0 replies; 6+ messages in thread
From: Hillf Danton @ 2010-10-14 14:55 UTC (permalink / raw)
To: Robert Love; +Cc: Mike Christie, devel, linux-scsi
Hi Rob,
Sorry for replying late.
On Tue, Oct 12, 2010 at 6:08 AM, Robert Love <robert.w.love@intel.com> wrote:
> On Fri, 2010-10-08 at 22:57 +0800, Hillf Danton wrote:
>
> Hi Hillf,
>
> Can you please prefix the libfc patches with "libfc: <title>"? Also, for
Yes, I can.
> your other patches that have a "SCSI" prefix, I think James' system adds
> the "[SCSI]" prefix when he applies patches so you don't need to add it.
I will remove SCSI.
>
>> Fibre Channel exchange pool is setup in a per cpu way that
>> entire exchange id range is divided equally across all cpus.
>>
>> When cpu goes offline, the corresponding range of exchg id
>> becomes unavailable until it is online again.
>>
>> This work tries to fix the unavailability based on notifier_block.
>>
>> Signed-off-by: Hillf Danton <dhillf@gmail.com>
>> ---
>>
>> --- o/linux-2.6.36-rc4/drivers/scsi/libfc/fc_exch.c 2010-09-13
>> 07:07:38.000000000 +0800
>> +++ m/linux-2.6.36-rc4/drivers/scsi/libfc/fc_exch.c 2010-10-08
>> 22:24:48.000000000 +0800
>
> Most patches I've seen on the various mailing lists are created by
> diffing the files within the kernel directory, so that you don't have
> the "/linux-2.6.36-rc/" in each hunk. It's the difference between using
> 'patch -p1' or 'patch -p2' when applying the patch.
I want to show clearly patch reviewers the kernel version.
>
> I think this is preventing git-am from applying the patches. To import
> them I ended up having git-am fail and then manually using the 'patch
> -p2' command, so that I could get both the commit message from git-am
> and then the patch itself from 'patch'. I don't know about anyone else,
> but if you post patches without the kernel directory in the diff line it
> would help me out. Thanks.
Sorry for getting you in trouble.
>
>> @@ -26,6 +26,9 @@
>> #include <linux/timer.h>
>> #include <linux/slab.h>
>> #include <linux/err.h>
>> +#include <linux/notifier.h>
>> +#include <linux/cpu.h>
>> +#include <linux/cpumask.h>
>>
>> #include <scsi/fc/fc_fc2.h>
>>
>> @@ -40,6 +43,8 @@ static u16 fc_cpu_order; /* 2's power to
>> static struct kmem_cache *fc_em_cachep; /* cache for exchanges */
>> struct workqueue_struct *fc_exch_workqueue;
>>
>> +static cpumask_t fc_offline_cpu_mask = CPU_MASK_NONE;
>> +
>> /*
>> * Structure and function definitions for managing Fibre Channel Exchanges
>> * and Sequences.
>> @@ -666,6 +671,7 @@ static struct fc_exch *fc_exch_em_alloc(
>> unsigned int cpu;
>> u16 index;
>> struct fc_exch_pool *pool;
>> + unsigned int index = -1;
>
> This patch doesn't compile:
This is my fault when I prepare the patch based upon
for_each_cpu in cpumask.h, and sorry.
/**
* for_each_cpu - iterate over every cpu in a mask
* @cpu: the (optionally unsigned) integer iterator
* @mask: the cpumask pointer
*
* After the loop, cpu is >= nr_cpu_ids.
*/
>
> drivers/scsi/libfc/fc_exch.c: In function ‘fc_exch_em_alloc’:
> drivers/scsi/libfc/fc_exch.c:674: error: conflicting types for ‘index’
> drivers/scsi/libfc/fc_exch.c:672: note: previous declaration of ‘index’
> was here
> make[3]: *** [drivers/scsi/libfc/fc_exch.o] Error 1
>
> In general I think the idea is fine. I haven't run into any
> circumstances where the per-CPU pools are exhausted. I know others have
The design of exch pool is an outstandingly good model for applications in
the environment of multiple cpus, it will be in my work.
> when using NPIV, but there were other ways to fix those issues.
>
>>
>> /* allocate memory for exchange */
>> ep = mempool_alloc(mp->ep_pool, GFP_ATOMIC);
>> @@ -679,6 +685,7 @@ static struct fc_exch *fc_exch_em_alloc(
>> pool = per_cpu_ptr(mp->pool, cpu);
>> spin_lock_bh(&pool->lock);
>> put_cpu();
>> +again:
>> index = pool->next_index;
>> /* allocate new exch from pool */
>> while (fc_exch_ptr_get(pool, index)) {
>> @@ -719,6 +726,17 @@ out:
>> err:
>> spin_unlock_bh(&pool->lock);
>> atomic_inc(&mp->stats.no_free_exch_xid);
>
> I think this should be moved below the offline pool check, when we've
Node.
> really failed to allocate an exchange. It's not a per-CPU pool stat,
> it's an overall resource allocation failure stat, which you're now
> helping to happen less frequently. With the current placement we'll be
> bumping up the counter even though we have allocated an exchange.
>
> Thanks, //Rob
>
>
Thank you for sharing so much.
//Hillf
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-10-14 14:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-08 14:57 [PATCH] add cpu on/offline support in Fibre Channel exchange and sequence handling Hillf Danton
2010-10-11 22:03 ` Zou, Yi
2010-10-11 22:08 ` [Open-FCoE] " Robert Love
2010-10-11 22:49 ` Nicholas A. Bellinger
2010-10-11 23:23 ` Robert Love
2010-10-14 14:55 ` Hillf Danton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox