All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marcel Holtmann <marcel@holtmann.org>
To: Mat Martineau <mathewm@codeaurora.org>
Cc: David Herrmann <dh.herrmann@googlemail.com>,
	Andre Guedes <andre.guedes@openbossa.org>,
	padovan@profusion.mobi, linux-bluetooth@vger.kernel.org
Subject: Re: Does it make sense to have the hdev workqueue serialized?
Date: Fri, 16 Dec 2011 17:04:55 -0800	[thread overview]
Message-ID: <1324083895.1965.87.camel@aeonflux> (raw)
In-Reply-To: <alpine.DEB.2.02.1112161021100.24351@mathewm-linux>

Hi Mat,

> The benefit would be in having no need to keep track of which context 
> functions are executing in.  It's been a big headache with the ERTM 
> and AMP changes, and there is a bunch of code that could work better 
> in process context if it didn't have to also handle calls from 
> tasklets.
> 
> That said, after learning more about how workqueues are implemented 
> now, it may be worthwhile to change the "use one single-threaded 
> workqueue for everything" assumption.  alloc_workqueue() has a 
> max_active parameter, and it is possible to have many work items 
> running concurrently.  Some of those threads could be suspended, like 
> Andre does in his patch.  Workqueues created with the old 
> create_workqueue() or create_singlethread_workqueue() have max_active 
> == 1, which enforces serialization on each processor.
> 
> 
> So there are two big questions:  Do we want to keep pushing everything 
> on the hdev workqueue, since workqueues are not as heavyweight as they 
> used to be?  And does it make sense to keep our workqueues serialized?
> 
> 
> Advantages of serialization:
>   * An HCI device is serialized by the transport anyway, so it makes 
> sense to match that model.
>   * Ordering is maintained.  The order of incoming HCI events may queue 
> work in a particular order and need to assume the work will be 
> executed in that order.
>   * Simplicity.
>   * No lock contention between multiple workers.
> 
> Advantages of not serializing:
>   * Takes advantage of SMP
>   * Workers can block without affecting the rest of the queue, enabling 
> workers to be long-lived and use state on the thread stack instead of 
> complicated lists of pending operations and dynamic allocation.
>   * We need to have proper locking to deal with user processes anyway, 
> so why not allow more concurrency internally.
>   * Some work can proceed while waiting for locks in other workers.
>   * Can use WQ_HIGHPRI to keep tx/rx data moving even when workers are 
> waiting for locks
> 
> 
> I think what's called for is a hybrid approach that serializes where 
> necessary, but uses multiple workqueues.  How about this:
> 
>   * Use the serialized hdev workqueue for rx/tx only.  This could use 
> WQ_HIGHPRI to help performance.

I think this is what we have to do to ensure that our event, cmd and ACL
and also SCO processing is not affected by anything else.

>   * Have a serialized workqueue for each L2CAP channel to handle 
> per-channel timeouts.

Why per channel? Wouldn't be one per ACL connection by enough?

>   * Have a global, non-serialized workqueue for stuff like sysfs and 
> mgmt to use.

And yes, one global workqueue for random tasks we have to do seems like
a good idea as well.

Regards

Marcel



  parent reply	other threads:[~2011-12-17  1:04 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-14 16:25 [PATCH 0/7] MGMT Start Discovery command LE-Only Support Andre Guedes
2011-12-14 16:25 ` [PATCH 1/7] Bluetooth: Add 'eir_len' param to mgmt_device_found() Andre Guedes
2011-12-14 16:25 ` [PATCH 2/7] Bluetooth: Report LE devices Andre Guedes
2011-12-14 16:25 ` [PATCH 3/7] Bluetooth: LE scan should send MGMT Discovering events Andre Guedes
2011-12-14 16:25 ` [PATCH 4/7] Bluetooth: Add helper functions to send LE scan commands Andre Guedes
2011-12-14 16:25 ` [PATCH 5/7] Bluetooth: LE scan infra-structure Andre Guedes
2011-12-14 19:36   ` Marcel Holtmann
2011-12-15 19:25     ` Mat Martineau
2011-12-15 20:00       ` Andre Guedes
2011-12-16 18:21         ` Mat Martineau
2011-12-15 20:05       ` David Herrmann
2011-12-16 19:20         ` Does it make sense to have the hdev workqueue serialized? Mat Martineau
2011-12-16 19:26           ` Changes to workqueues (was: Does it make sense to have the hdev workqueue serialized?) Mat Martineau
2011-12-16 20:05           ` Does it make sense to have the hdev workqueue serialized? Gustavo Padovan
2011-12-16 23:35             ` Mat Martineau
2011-12-17  1:04           ` Marcel Holtmann [this message]
2011-12-16 19:13       ` [PATCH 5/7] Bluetooth: LE scan infra-structure Gustavo Padovan
2011-12-14 16:25 ` [PATCH 6/7] Bluetooth: Add hci_do_le_scan() to hci_core Andre Guedes
2011-12-14 16:25 ` [PATCH 7/7] Bluetooth: MGMT start discovery LE-Only support Andre Guedes

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=1324083895.1965.87.camel@aeonflux \
    --to=marcel@holtmann.org \
    --cc=andre.guedes@openbossa.org \
    --cc=dh.herrmann@googlemail.com \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=mathewm@codeaurora.org \
    --cc=padovan@profusion.mobi \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.