lkml.org 
[lkml]   [2009]   [May]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [BUG] 2.6.30-rc4 hid bluetooth not working
    From
    Date
    Hi Justin,

    > > > >> >>> > Subject: bluetooth: Fix serialization when adding/deleting connections in hci_sysfs
    > > > >> >>> >
    > > > >> >>> > add_conn and del_conn should be serialized, but flush_workqueue can't be used
    > > > >> >>> > by the worker thread on it's own queue, so use flush_work to serialize add_conn
    > > > >> >>> > and del_conn against each other.
    > > > >> >>> >
    > > > >> >>> > Signed-off-by: Marc Pignat <marc.pignat@hevs.ch>
    > > > >> >>>
    > > > >> >>> Acked-by: Jiri Kosina <jkosina@suse.cz>
    > > > >> >>>
    > > > >> >>> FWIW.
    > > > >> >>
    > > > >> >>nak from my side since I think it is the wrong fix. We really wanna wait
    > > > >> >>for all works to finish here. This includes work from other connection
    > > > >> >>attempts or terminations.
    > > > >> >
    > > > >> > IMHO, there is no need to wait for work currently running, since this is a
    > > > >> > singlethread workqueue.
    > > > >>
    > > > >> Yes, sounds right.
    > > > >>
    > > > >> >
    > > > >> > But it is perhaps simpler to use a lock (mutex or watherver locking primitive).
    > > > >>
    > > > >> I'm here a little bit late. Marcel, I'm quite busy recently, I just
    > > > >> see the commit and then this thread.
    > > > >>
    > > > >> Let me explain why I add two workqueue originally, because workqueue
    > > > >> will be defered, so we must guarantee "connection deletion" finished
    > > > >> before "connection adding with same bt addr", or the "connection
    > > > >> adding" will fail.
    > > > >>
    > > > >> On the other hand flush "adding" workqueue in "connection deletion"
    > > > >> function is not necessary.
    > > > >>
    > > > >> To fix this bug, I think we can just use the two work struct for
    > > > >> add/del, at the same time keeping the original two workqueue.
    > > > >>
    > > > >> Please see following patch for this, (building-test only, I have no
    > > > >> bluetooth device at hand, I can test this the day after tommorrow)
    > > > >
    > > > > so I spent the whole day figuring out what is going on here and we keep
    > > > > making the wrong assumptions over and over again.
    > > > >
    > > > > First of all, we only add the sysfs device when we have a successful
    > > > > connection. And we identify it with the handle. This means that we can
    > > > > NOT have any name clashes anymore since the controller has to make sure
    > > > > a handle is only assigned once. Previously we did this on the BD_ADDR
    > > > > value and that lead to it. That is no longer the case.
    > > > >
    > > > > Second of all the two work queues introduces way too much complexity for
    > > > > a really simple task of adding and removing a sysfs device entry.
    > > > >
    > > > > The real problem we have right now are that we are not initializing the
    > > > > sysfs device when creating the hci_conn. This is just wrong and can lead
    > > > > to all kinds of weird invalid data access. And as a result the adding of
    > > > > the sysfs device should only set the name and add it.
    > > > >
    > > > > We also check device_registered before making sure that device_add has
    > > > > been run. And instead of adding more locking or crazy work queue
    > > > > dependencies, we should use the single thread work queue to ensure the
    > > > > correct order of things.
    > > > >
    > > > > The attached patch introduces a hci_conn_init_sysfs step to make sure we
    > > > > setup the sysfs device correctly. I left the flush_work calls, but I
    > > > > think they are not needed since a del_conn before add_conn is no longer
    > > > > possible now.
    > > >
    > > > well it seems your not the only one
    > > > with a broken bluetooth(latest git pull
    > > > this morning)
    > >
    > > can you try the patch that I attached to the previous email. It should
    > > fix exactly this.
    >
    > nice patch.
    > works like a charm
    > bluetooth is alive again.

    thanks for testing.

    Marc, Roger, can you test this and confirm that it works for you and
    doesn't have any other side effects. Then I prepare it work upstream
    inclusion.

    Regards

    Marcel




    \
     
     \ /
      Last update: 2009-05-02 23:17    [W:0.036 / U:1.008 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site