linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ulisses Furquim <ulisses@profusion.mobi>
To: Andrei Emeltchenko <andrei.emeltchenko.news@gmail.com>
Cc: linux-bluetooth@vger.kernel.org
Subject: Re: [RFCv5 02/16] Bluetooth: Revert to mutexes from RCU list
Date: Mon, 20 Feb 2012 09:55:35 -0200	[thread overview]
Message-ID: <CAA37ikb_7PfxQwTCBHn0GHWUh-PVg9aJ1tj3S7cCZJsNb8DMZQ@mail.gmail.com> (raw)
In-Reply-To: <CAMXE1-oiPJEF7WAc14ekR99Y6=FNPs0iSqwWA_miLit=AesP=g@mail.gmail.com>

Hi Andrei,

On Sun, Feb 19, 2012 at 5:49 PM, Andrei Emeltchenko
<andrei.emeltchenko.news@gmail.com> wrote:
> Hi Ulisses,
>
> On Fri, Feb 17, 2012 at 11:22 PM, Ulisses Furquim
> <ulisses@profusion.mobi> wrote:
>> Why don't you squash together patches 2, 3 and 4? Is there a reason?
>
> The price of squashing is zero and if there are no other comments I
> can do it right away.
>
>> Only with patch 2 the change becomes incomplete and prone to errors.
>
> However I do not think that this makes much difference to the existing code.

We're trying to improve things, right? If the current code is broken
somehow is not an excuse for us to merge patches which we know are
incomplete and will possibly break if someone is doing a bisect, for
instance.

>> With patch 3 you add some needed changes but then adds an imbalance of
>> lock/release sock locks which is only solved in patch 4. Why?
>
> How does patch 4 solves locking? If I did not miss something it shall
> not change anything.

For instance, in patch 3 you replace l2cap_get_chan_by_scid() with __
l2cap_get_chan_by_scid() in l2cap_connect_rsp(). However,  __
l2cap_get_chan_by_scid() doesn't lock_sock() as the other one did and
only in patch 4 you add an explicit lock_sock() to
l2cap_connect_rsp(). Got it? A commit should be self contained as much
as possible so we keep the code bisectable.

Regards,

-- 
Ulisses Furquim
ProFUSION embedded systems
http://profusion.mobi
Mobile: +55 19 9250 0942
Skype: ulissesffs

  reply	other threads:[~2012-02-20 11:55 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-17 13:43 [RFCv5 00/16] Bluetooth: Change socket lock to l2cap_chan lock Emeltchenko Andrei
2012-02-17 13:43 ` [RFCv5 01/16] Bluetooth: trivial: Fix long line Emeltchenko Andrei
2012-02-17 13:43 ` [RFCv5 02/16] Bluetooth: Revert to mutexes from RCU list Emeltchenko Andrei
2012-02-17 21:22   ` Ulisses Furquim
2012-02-19 19:49     ` Andrei Emeltchenko
2012-02-20 11:55       ` Ulisses Furquim [this message]
2012-02-20 12:17         ` Andrei Emeltchenko
2012-02-17 13:43 ` [RFCv5 03/16] Bluetooth: Lock chan list when deleting chan Emeltchenko Andrei
2012-02-17 13:44 ` [RFCv5 04/16] Bluetooth: Do not use sk lock in get_chan functions Emeltchenko Andrei
2012-02-17 13:44 ` [RFCv5 05/16] Bluetooth: Add l2cap_chan_lock Emeltchenko Andrei
2012-02-17 13:44 ` [RFCv5 06/16] Bluetooth: Add locked and unlocked state_change Emeltchenko Andrei
2012-02-17 13:44 ` [RFCv5 07/16] Bluetooth: Add socket error function Emeltchenko Andrei
2012-02-17 13:44 ` [RFCv5 08/16] Bluetooth: Add unlocked __l2cap_chan_add function Emeltchenko Andrei
2012-02-17 13:44 ` [RFCv5 09/16] Bluetooth: Use chan lock in timers Emeltchenko Andrei
2012-02-17 13:44 ` [RFCv5 10/16] Bluetooth: Use chan lock in L2CAP sig commands Emeltchenko Andrei
2012-02-17 13:44 ` [RFCv5 11/16] Bluetooth: Use chan lock in L2CAP conn start Emeltchenko Andrei
2012-02-17 13:44 ` [RFCv5 12/16] Bluetooth: Use chan lock in receiving data Emeltchenko Andrei
2012-02-17 13:44 ` [RFCv5 13/16] Bluetooth: Change locking logic for conn/chan ready Emeltchenko Andrei
2012-02-17 13:44 ` [RFCv5 14/16] Bluetooth: Change locking logic in security_cfm Emeltchenko Andrei
2012-02-17 13:44 ` [RFCv5 15/16] Bluetooth: Use l2cap chan lock in socket connect Emeltchenko Andrei
2012-02-17 13:44 ` [RFCv5 16/16] Bluetooth: Remove socket lock check Emeltchenko Andrei

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=CAA37ikb_7PfxQwTCBHn0GHWUh-PVg9aJ1tj3S7cCZJsNb8DMZQ@mail.gmail.com \
    --to=ulisses@profusion.mobi \
    --cc=andrei.emeltchenko.news@gmail.com \
    --cc=linux-bluetooth@vger.kernel.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;
as well as URLs for NNTP newsgroup(s).