bnx2i kthread madness

bnx2i kthread madness

[Peter Zijlstra]

Hi,

I accidentally looked at all kthreads in existence on my system and
found I had:

[iscsi_eh]
[bnx2i_thread/0]
[bnx2i_thread/1]
[bnx2i_thread/2]
[bnx2i_thread/3]
[bnx2i_thread/4]
[bnx2i_thread/5]
[bnx2i_thread/6]
[bnx2i_thread/7]
[bnx2i_thread/8]
[bnx2i_thread/9]
[bnx2i_thread/10]
[bnx2i_thread/11]
[bnx2i_thread/12]
[bnx2i_thread/13]
[bnx2i_thread/14]
[bnx2i_thread/15]
[bnx2i_thread/16]
[bnx2i_thread/17]
[bnx2i_thread/18]
[bnx2i_thread/19]
[bnx2i_thread/20]
[bnx2i_thread/21]
[bnx2i_thread/22]
[bnx2i_thread/23]

This left me wondering why, because I most certainly am not using iSCSI.
I don't even know why its enabled in my .config (and it won't be long). 

Please fix this muck to not create useless threads.

Re: bnx2i kthread madness

[Eddie Wai]

On Wed, 2011-09-28 at 08:05 -0700, Peter Zijlstra wrote:
> Hi,
> 
> I accidentally looked at all kthreads in existence on my system and
> found I had:
> 
> [iscsi_eh]
> [bnx2i_thread/0]
> [bnx2i_thread/1]
> [bnx2i_thread/2]
> [bnx2i_thread/3]
> [bnx2i_thread/4]
> [bnx2i_thread/5]
> [bnx2i_thread/6]
> [bnx2i_thread/7]
> [bnx2i_thread/8]
> [bnx2i_thread/9]
> [bnx2i_thread/10]
> [bnx2i_thread/11]
> [bnx2i_thread/12]
> [bnx2i_thread/13]
> [bnx2i_thread/14]
> [bnx2i_thread/15]
> [bnx2i_thread/16]
> [bnx2i_thread/17]
> [bnx2i_thread/18]
> [bnx2i_thread/19]
> [bnx2i_thread/20]
> [bnx2i_thread/21]
> [bnx2i_thread/22]
> [bnx2i_thread/23]
> 
> This left me wondering why, because I most certainly am not using iSCSI.
> I don't even know why its enabled in my .config (and it won't be long). 
> 
> Please fix this muck to not create useless threads.

Hello Peter,

Point noted.  In the current bnx2i driver, one kthread is created per
cpu core upon module init (and destroyed upon module exit).  The
kthreads are meant only to improve I/O performance when iSCSI is
employed.  Otherwise, I agree that they should not exist.

Eddie
> 
> 
> 

Re: bnx2i kthread madness

[Bart Van Assche]

On Wed, Sep 28, 2011 at 7:29 PM, Eddie Wai <eddie.wai@broadcom.com> wrote:
> On Wed, 2011-09-28 at 08:05 -0700, Peter Zijlstra wrote:
> > I accidentally looked at all kthreads in existence on my system and
> > found I had:
> >
> > [iscsi_eh]
> > [bnx2i_thread/0]
> > [bnx2i_thread/1]
> > [bnx2i_thread/2]
> > [bnx2i_thread/3]
> > [bnx2i_thread/4]
> > [bnx2i_thread/5]
> > [bnx2i_thread/6]
> > [bnx2i_thread/7]
> > [bnx2i_thread/8]
> > [bnx2i_thread/9]
> > [bnx2i_thread/10]
> > [bnx2i_thread/11]
> > [bnx2i_thread/12]
> > [bnx2i_thread/13]
> > [bnx2i_thread/14]
> > [bnx2i_thread/15]
> > [bnx2i_thread/16]
> > [bnx2i_thread/17]
> > [bnx2i_thread/18]
> > [bnx2i_thread/19]
> > [bnx2i_thread/20]
> > [bnx2i_thread/21]
> > [bnx2i_thread/22]
> > [bnx2i_thread/23]
> >
> > This left me wondering why, because I most certainly am not using iSCSI.
> > I don't even know why its enabled in my .config (and it won't be long).
> >
> > Please fix this muck to not create useless threads.
>
> Point noted.  In the current bnx2i driver, one kthread is created per
> cpu core upon module init (and destroyed upon module exit).  The
> kthreads are meant only to improve I/O performance when iSCSI is
> employed.  Otherwise, I agree that they should not exist.

Why doesn't bnx2i use one of the system work queues defined in
<linux/workqueue.h> ?

$ git grep system include/linux/workqueue.h|cat
include/linux/workqueue.h: * system_wq is the one used by
schedule[_delayed]_work[_on]().
include/linux/workqueue.h: * system_long_wq is similar to system_wq
but may host long running
include/linux/workqueue.h: * system_nrt_wq is non-reentrant and
guarantees that any given work
include/linux/workqueue.h: * system_unbound_wq is unbound workqueue.
Workers are not bound to
include/linux/workqueue.h: * system_freezable_wq is equivalent to
system_wq except that it's
include/linux/workqueue.h:extern struct workqueue_struct *system_wq;
include/linux/workqueue.h:extern struct workqueue_struct *system_long_wq;
include/linux/workqueue.h:extern struct workqueue_struct *system_nrt_wq;
include/linux/workqueue.h:extern struct workqueue_struct *system_unbound_wq;
include/linux/workqueue.h:extern struct workqueue_struct *system_freezable_wq;

Bart.
--
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

Re: bnx2i kthread madness

[James Bottomley]

On Wed, 2011-09-28 at 10:29 -0700, Eddie Wai wrote:
> On Wed, 2011-09-28 at 08:05 -0700, Peter Zijlstra wrote:
> > Hi,
> > 
> > I accidentally looked at all kthreads in existence on my system and
> > found I had:
> > 
> > [iscsi_eh]
> > [bnx2i_thread/0]
> > [bnx2i_thread/1]
> > [bnx2i_thread/2]
> > [bnx2i_thread/3]
> > [bnx2i_thread/4]
> > [bnx2i_thread/5]
> > [bnx2i_thread/6]
> > [bnx2i_thread/7]
> > [bnx2i_thread/8]
> > [bnx2i_thread/9]
> > [bnx2i_thread/10]
> > [bnx2i_thread/11]
> > [bnx2i_thread/12]
> > [bnx2i_thread/13]
> > [bnx2i_thread/14]
> > [bnx2i_thread/15]
> > [bnx2i_thread/16]
> > [bnx2i_thread/17]
> > [bnx2i_thread/18]
> > [bnx2i_thread/19]
> > [bnx2i_thread/20]
> > [bnx2i_thread/21]
> > [bnx2i_thread/22]
> > [bnx2i_thread/23]
> > 
> > This left me wondering why, because I most certainly am not using iSCSI.
> > I don't even know why its enabled in my .config (and it won't be long). 
> > 
> > Please fix this muck to not create useless threads.
> 
> Hello Peter,
> 
> Point noted.  In the current bnx2i driver, one kthread is created per
> cpu core upon module init (and destroyed upon module exit).  The
> kthreads are meant only to improve I/O performance when iSCSI is
> employed.  Otherwise, I agree that they should not exist.

So if I look at the code, you seem to be trying to direct outbound
commands via the CPU specified in the request?  I assume this is because
we need to access the data to generate things like the checksum, in
which case the command data needs to be CPU hot in the outbound path?
(although I thought the bnx2 had a hardware engine that can do this).

If we genuinely need to do something like this, the interface
work_on_cpu() was defined for something like this purpose.

James

Re: bnx2i kthread madness

[Eddie Wai]

On Wed, 2011-09-28 at 10:47 -0700, James Bottomley wrote:
> On Wed, 2011-09-28 at 10:29 -0700, Eddie Wai wrote:
> > On Wed, 2011-09-28 at 08:05 -0700, Peter Zijlstra wrote:
> > > Hi,
> > > 
> > > I accidentally looked at all kthreads in existence on my system and
> > > found I had:
> > > 
> > > [iscsi_eh]
> > > [bnx2i_thread/0]
> > > [bnx2i_thread/1]
> > > [bnx2i_thread/2]
> > > [bnx2i_thread/3]
> > > [bnx2i_thread/4]
> > > [bnx2i_thread/5]
> > > [bnx2i_thread/6]
> > > [bnx2i_thread/7]
> > > [bnx2i_thread/8]
> > > [bnx2i_thread/9]
> > > [bnx2i_thread/10]
> > > [bnx2i_thread/11]
> > > [bnx2i_thread/12]
> > > [bnx2i_thread/13]
> > > [bnx2i_thread/14]
> > > [bnx2i_thread/15]
> > > [bnx2i_thread/16]
> > > [bnx2i_thread/17]
> > > [bnx2i_thread/18]
> > > [bnx2i_thread/19]
> > > [bnx2i_thread/20]
> > > [bnx2i_thread/21]
> > > [bnx2i_thread/22]
> > > [bnx2i_thread/23]
> > > 
> > > This left me wondering why, because I most certainly am not using iSCSI.
> > > I don't even know why its enabled in my .config (and it won't be long). 
> > > 
> > > Please fix this muck to not create useless threads.
> > 
> > Hello Peter,
> > 
> > Point noted.  In the current bnx2i driver, one kthread is created per
> > cpu core upon module init (and destroyed upon module exit).  The
> > kthreads are meant only to improve I/O performance when iSCSI is
> > employed.  Otherwise, I agree that they should not exist.
> 
> So if I look at the code, you seem to be trying to direct outbound
> commands via the CPU specified in the request?  I assume this is because
> we need to access the data to generate things like the checksum, in
> which case the command data needs to be CPU hot in the outbound path?
> (although I thought the bnx2 had a hardware engine that can do this).
The design goal was to align the cmd completions to the cmd request cpu
so we can take advantage of not only the cpu cache content but also to
complete the I/Os in parallel (instead of how we did it previously in
the bh serially).  From our test environment, we were able to see a
noticeable performance boost on IOPS across small I/Os.

> If we genuinely need to do something like this, the interface
> work_on_cpu() was defined for something like this purpose.
Not familiar with work_on_cpu(), let me see how this can be used
instead.  Thanks.
> 
> James
> 
> 
> --
> 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
> 

Re: bnx2i kthread madness

[Eddie Wai]

On Wed, 2011-09-28 at 10:33 -0700, Bart Van Assche wrote:
> On Wed, Sep 28, 2011 at 7:29 PM, Eddie Wai <eddie.wai@broadcom.com> wrote:
> > On Wed, 2011-09-28 at 08:05 -0700, Peter Zijlstra wrote:
> > > I accidentally looked at all kthreads in existence on my system and
> > > found I had:
> > >
> > > [iscsi_eh]
> > > [bnx2i_thread/0]
> > > [bnx2i_thread/1]
> > > [bnx2i_thread/2]
> > > [bnx2i_thread/3]
> > > [bnx2i_thread/4]
> > > [bnx2i_thread/5]
> > > [bnx2i_thread/6]
> > > [bnx2i_thread/7]
> > > [bnx2i_thread/8]
> > > [bnx2i_thread/9]
> > > [bnx2i_thread/10]
> > > [bnx2i_thread/11]
> > > [bnx2i_thread/12]
> > > [bnx2i_thread/13]
> > > [bnx2i_thread/14]
> > > [bnx2i_thread/15]
> > > [bnx2i_thread/16]
> > > [bnx2i_thread/17]
> > > [bnx2i_thread/18]
> > > [bnx2i_thread/19]
> > > [bnx2i_thread/20]
> > > [bnx2i_thread/21]
> > > [bnx2i_thread/22]
> > > [bnx2i_thread/23]
> > >
> > > This left me wondering why, because I most certainly am not using iSCSI.
> > > I don't even know why its enabled in my .config (and it won't be long).
> > >
> > > Please fix this muck to not create useless threads.
> >
> > Point noted.  In the current bnx2i driver, one kthread is created per
> > cpu core upon module init (and destroyed upon module exit).  The
> > kthreads are meant only to improve I/O performance when iSCSI is
> > employed.  Otherwise, I agree that they should not exist.
> 
> Why doesn't bnx2i use one of the system work queues defined in
> <linux/workqueue.h> ?
We defined our own kthreads instead is mainly because we felt that work
queues might not have the response time we like to handle I/O
completions under heavy stress environments as we migrated from
tasklets.  Although we really haven't yet quantified this assumption.
> 
> $ git grep system include/linux/workqueue.h|cat
> include/linux/workqueue.h: * system_wq is the one used by
> schedule[_delayed]_work[_on]().
> include/linux/workqueue.h: * system_long_wq is similar to system_wq
> but may host long running
> include/linux/workqueue.h: * system_nrt_wq is non-reentrant and
> guarantees that any given work
> include/linux/workqueue.h: * system_unbound_wq is unbound workqueue.
> Workers are not bound to
> include/linux/workqueue.h: * system_freezable_wq is equivalent to
> system_wq except that it's
> include/linux/workqueue.h:extern struct workqueue_struct *system_wq;
> include/linux/workqueue.h:extern struct workqueue_struct *system_long_wq;
> include/linux/workqueue.h:extern struct workqueue_struct *system_nrt_wq;
> include/linux/workqueue.h:extern struct workqueue_struct *system_unbound_wq;
> include/linux/workqueue.h:extern struct workqueue_struct *system_freezable_wq;
> 
> Bart.
> 

Re: bnx2i kthread madness

[Mike Christie]

On 09/28/2011 01:26 PM, Eddie Wai wrote:
> 
> On Wed, 2011-09-28 at 10:33 -0700, Bart Van Assche wrote:
>> On Wed, Sep 28, 2011 at 7:29 PM, Eddie Wai <eddie.wai@broadcom.com> wrote:
>>> On Wed, 2011-09-28 at 08:05 -0700, Peter Zijlstra wrote:
>>>> I accidentally looked at all kthreads in existence on my system and
>>>> found I had:
>>>>
>>>> [iscsi_eh]
>>>> [bnx2i_thread/0]
>>>> [bnx2i_thread/1]
>>>> [bnx2i_thread/2]
>>>> [bnx2i_thread/3]
>>>> [bnx2i_thread/4]
>>>> [bnx2i_thread/5]
>>>> [bnx2i_thread/6]
>>>> [bnx2i_thread/7]
>>>> [bnx2i_thread/8]
>>>> [bnx2i_thread/9]
>>>> [bnx2i_thread/10]
>>>> [bnx2i_thread/11]
>>>> [bnx2i_thread/12]
>>>> [bnx2i_thread/13]
>>>> [bnx2i_thread/14]
>>>> [bnx2i_thread/15]
>>>> [bnx2i_thread/16]
>>>> [bnx2i_thread/17]
>>>> [bnx2i_thread/18]
>>>> [bnx2i_thread/19]
>>>> [bnx2i_thread/20]
>>>> [bnx2i_thread/21]
>>>> [bnx2i_thread/22]
>>>> [bnx2i_thread/23]
>>>>
>>>> This left me wondering why, because I most certainly am not using iSCSI.
>>>> I don't even know why its enabled in my .config (and it won't be long).
>>>>
>>>> Please fix this muck to not create useless threads.
>>>
>>> Point noted.  In the current bnx2i driver, one kthread is created per
>>> cpu core upon module init (and destroyed upon module exit).  The
>>> kthreads are meant only to improve I/O performance when iSCSI is
>>> employed.  Otherwise, I agree that they should not exist.
>>
>> Why doesn't bnx2i use one of the system work queues defined in
>> <linux/workqueue.h> ?
> We defined our own kthreads instead is mainly because we felt that work
> queues might not have the response time we like to handle I/O
> completions under heavy stress environments as we migrated from
> tasklets.  Although we really haven't yet quantified this assumption.


bnx2fc and fcoe have the same issue. qla2xxx seems to use workqueues and
is ok.

If perf is a issue then like have I said before when this patch was
submitted, and when the fcoe and bnx2fc ones were too, :) I think you
need to change blkiopoll to do what you actually want.