Re: [PREEMPT-RT] [REPOST PATCH 1/5] scsi: bnx2i: convert to kworker

2016-11-07 Thread Johannes Thumshirn
On Mon, Nov 07, 2016 at 08:10:25PM +0100, Christoph Hellwig wrote:
> On Mon, Nov 07, 2016 at 06:04:21PM +0100, Sebastian Andrzej Siewior wrote:
> > So we keep things as they are right now or are we getting also rid of
> > the internal list? This was tested by Johannes and Chad (claimed to do
> > testing)
> 
> IFF the patches actually are tested as-is let's get them in.  I don't
> think they are how we want the code to like in the long run, though.

I'm definitively sure I did test the bnx2fc patches. I'm not 100% sure I did
test the bnx2i patches as well (althoug I wrote "for the whole series").
Testing was IIRC booting from the FCoE LUN and the doing a series of fio tests
for about an hour.

Byte,
Johannes

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PREEMPT-RT] [REPOST PATCH 1/5] scsi: bnx2i: convert to kworker

2016-11-07 Thread Christoph Hellwig
On Mon, Nov 07, 2016 at 06:04:21PM +0100, Sebastian Andrzej Siewior wrote:
> So we keep things as they are right now or are we getting also rid of
> the internal list? This was tested by Johannes and Chad (claimed to do
> testing)

IFF the patches actually are tested as-is let's get them in.  I don't
think they are how we want the code to like in the long run, though.

> 
> Not sure what the last point is about.

It's about bnx2i and bnx2fc being clients of the main cnix driver,
which does the actual interrupt handling.  It seems we should actually
offloading things to a workqueue or threaded interrupts in that core
module instead of duplicating it in the clients.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PREEMPT-RT] [REPOST PATCH 1/5] scsi: bnx2i: convert to kworker

2016-11-07 Thread Sebastian Andrzej Siewior
On 2016-11-07 17:48:46 [+0100], Christoph Hellwig wrote:
> On Mon, Nov 07, 2016 at 05:46:29PM +0100, Sebastian Andrzej Siewior wrote:
> > sorry for the confusion in the subject. If I remember correctly you said
> > that we may not have enough room for a larger / work_struct struct and I
> > should keep the list for now.
> 
> But that was for libfc where it only has a tiny bit of private data
> i nthe skb - this is for cnix which has plenty of space, and in fact

Right. now I remember.

> probably should do the workqueue offload in the core before invoking
> the protocol drivers.

So we keep things as they are right now or are we getting also rid of
the internal list? This was tested by Johannes and Chad (claimed to do
testing) and the removal the internal list afterwards shouldn't be that
big deal. Not sure what the last point is about.

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PREEMPT-RT] [REPOST PATCH 1/5] scsi: bnx2i: convert to kworker

2016-11-07 Thread Christoph Hellwig
On Mon, Nov 07, 2016 at 05:46:29PM +0100, Sebastian Andrzej Siewior wrote:
> sorry for the confusion in the subject. If I remember correctly you said
> that we may not have enough room for a larger / work_struct struct and I
> should keep the list for now.

But that was for libfc where it only has a tiny bit of private data
i nthe skb - this is for cnix which has plenty of space, and in fact
probably should do the workqueue offload in the core before invoking
the protocol drivers.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PREEMPT-RT] [REPOST PATCH 1/5] scsi: bnx2i: convert to kworker

2016-11-07 Thread Sebastian Andrzej Siewior
On 2016-11-07 17:38:48 [+0100], Christoph Hellwig wrote:
> You're right it does - between the incorrect subject and the fact
> that it still keeps the linked list of items arounds instead of fully
> using the workqueue infrastructure I was a bit confused before my
> first coffee this morning.  Same applies to patch 2.

sorry for the confusion in the subject. If I remember correctly you said
that we may not have enough room for a larger / work_struct struct and I
should keep the list for now.

Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PREEMPT-RT] [REPOST PATCH 1/5] scsi: bnx2i: convert to kworker

2016-11-07 Thread Christoph Hellwig
On Mon, Nov 07, 2016 at 05:31:03PM +0100, Thomas Gleixner wrote:
> On Mon, 7 Nov 2016, Christoph Hellwig wrote:
> 
> > It seems like the whole damn driver should just use threaded interrupts.
> > Of course it's a giant beast and not just the iSCSI one.  But even
> > if we don't go all the way I'd much prefer workqueues.  kthread work
> > is simply the worst API ever and I'd prefer to not have it proliferate.
> 
> That's what the patch is doing. It uses INIT_WORK() and
> schedule_work[_on](). I can't find any reference to kthread work.

You're right it does - between the incorrect subject and the fact
that it still keeps the linked list of items arounds instead of fully
using the workqueue infrastructure I was a bit confused before my
first coffee this morning.  Same applies to patch 2.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PREEMPT-RT] [REPOST PATCH 1/5] scsi: bnx2i: convert to kworker

2016-11-07 Thread Thomas Gleixner
On Mon, 7 Nov 2016, Christoph Hellwig wrote:

> It seems like the whole damn driver should just use threaded interrupts.
> Of course it's a giant beast and not just the iSCSI one.  But even
> if we don't go all the way I'd much prefer workqueues.  kthread work
> is simply the worst API ever and I'd prefer to not have it proliferate.

That's what the patch is doing. It uses INIT_WORK() and
schedule_work[_on](). I can't find any reference to kthread work.

Thanks,

tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html