Linux-NVME Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: hch@infradead.org (Christoph Hellwig)
Subject: [PATCH v3 2/3] nvmet_fc: Reduce work_q count
Date: Wed, 24 May 2017 10:35:00 -0700	[thread overview]
Message-ID: <20170524173500.GA13151@infradead.org> (raw)
In-Reply-To: <79da00e0-8abb-8694-2075-6461d6855623@gmail.com>

On Tue, May 23, 2017@12:31:07PM -0700, James Smart wrote:
> > Why?  The whole point of workqueues is to avoid this sort of open coded
> > work lists in drivers.  To me it seems like you should simply make
> > the existing workqueue global, and maybe mark it as cpu itensive based
> > on profiling, but that's about it.
> 
> Why: to have parallelism and cpu affinity and its benefits for all the
> interim work the transport does for moving data/responses.

Which is exactly the reason for using a workqueue - they do have
per cpu helpers and keep the affinity, and they keep the logic
for this in one place.

Documentation/core-api/workqueue.rst wil explain quite a bit of the
details.

> So I'm not sure how this differs from rdma. The bottom ib cq handler, which
> can be a workqueue element or a soft_irq instance, sits in a loop and
> processes the cq elements, calling the rdma transport done routine for each
> one, which does equivalent work. So both fc and rdma can be a workqueue
> element, both pull variable numbers of work items with caps on items per
> call, and the work per item is similar. So the only difference is rdma is
> pulling from a memory ring vs nvme-fc pulling from a linked list.

The difference is that by the time they are called we don't even know
how many elements we'll process.

> I can certainly remove the work list and go back to a work queue element
> work item. But I have to believe a work queue element for every completion
> is not as efficient as a simple linked list of the completions.


It should be very similar. The whole point of workqueues is to factor
this pattern into common helpers.

  reply	other threads:[~2017-05-24 17:35 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-22 22:28 [PATCH v3 0/3] outstanding nvme_fc/nvmet_fc fixes James Smart
2017-05-22 22:28 ` [PATCH v3 1/3] nvme_fc: replace ioabort msleep loop with completion James Smart
2017-05-23  7:12   ` Christoph Hellwig
2017-05-22 22:28 ` [PATCH v3 2/3] nvmet_fc: Reduce work_q count James Smart
2017-05-23  7:15   ` Christoph Hellwig
2017-05-23 19:31     ` James Smart
2017-05-24 17:35       ` Christoph Hellwig [this message]
2017-05-22 22:28 ` [PATCH v3 3/3] nvme_fcloop: fix port deletes and callbacks James Smart
2017-05-23  7:16   ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170524173500.GA13151@infradead.org \
    --to=hch@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox