10 Mar, 2012

1 commit


09 Feb, 2012

1 commit

  • This fixes a memory-corrupting bug: not only does it cause the warning,
    but as a result of dropping the refcount to zero, it causes the
    pcmcia_socket0 device structure to be freed while it still has
    references, causing slab caches corruption. A fatal oops quickly
    follows this warning - often even just a 'dmesg' following the warning
    causes the kernel to oops.

    While testing suspend/resume on an ARM device with PCMCIA support, and a
    CF card inserted, I found that after five suspend and resumes, the
    kernel would complain, and shortly die after with slab corruption.

    WARNING: at include/linux/kref.h:41 kobject_get+0x28/0x50()

    As the message doesn't give a clue about which kobject, and the built-in
    debugging in drivers/base/power/main.c happens too late, this was added
    right before each get_device():

    printk("%s: %p [%s] %u\n", __func__, dev, kobject_name(&dev->kobj), atomic_read(&dev->kobj.kref.refcount));

    and on the 3rd s2ram cycle, the following behaviour observed:

    On the 3rd suspend/resume cycle:

    dpm_prepare: c1a0d998 [pcmcia_socket0] 3
    dpm_suspend: c1a0d998 [pcmcia_socket0] 3
    dpm_suspend_noirq: c1a0d998 [pcmcia_socket0] 3
    dpm_resume_noirq: c1a0d998 [pcmcia_socket0] 3
    dpm_resume: c1a0d998 [pcmcia_socket0] 3
    dpm_complete: c1a0d998 [pcmcia_socket0] 2

    4th:

    dpm_prepare: c1a0d998 [pcmcia_socket0] 2
    dpm_suspend: c1a0d998 [pcmcia_socket0] 2
    dpm_suspend_noirq: c1a0d998 [pcmcia_socket0] 2
    dpm_resume_noirq: c1a0d998 [pcmcia_socket0] 2
    dpm_resume: c1a0d998 [pcmcia_socket0] 2
    dpm_complete: c1a0d998 [pcmcia_socket0] 1

    5th:

    dpm_prepare: c1a0d998 [pcmcia_socket0] 1
    dpm_suspend: c1a0d998 [pcmcia_socket0] 1
    dpm_suspend_noirq: c1a0d998 [pcmcia_socket0] 1
    dpm_resume_noirq: c1a0d998 [pcmcia_socket0] 1
    dpm_resume: c1a0d998 [pcmcia_socket0] 1
    dpm_complete: c1a0d998 [pcmcia_socket0] 0
    ------------[ cut here ]------------
    WARNING: at include/linux/kref.h:41 kobject_get+0x28/0x50()
    Modules linked in: ucb1x00_core
    Backtrace:
    [] (dump_backtrace+0x0/0x110) from [] (dump_stack+0x18/0x1c)
    [] (dump_stack+0x0/0x1c) from [] (warn_slowpath_common+0x50/0x68)
    [] (warn_slowpath_common+0x0/0x68) from [] (warn_slowpath_null+0x24/0x28)
    [] (warn_slowpath_null+0x0/0x28) from [] (kobject_get+0x28/0x50)
    [] (kobject_get+0x0/0x50) from [] (get_device+0x1c/0x24)
    [] (dpm_complete+0x0/0x1a0) from [] (dpm_resume_end+0x1c/0x20)
    ...

    Looking at commit 7b24e7988263 ("pcmcia: split up central event handler"),
    the following change was made to cs.c:

    return 0;
    }
    #endif
    -
    - send_event(skt, CS_EVENT_PM_RESUME, CS_EVENT_PRI_LOW);
    + if (!(skt->state & SOCKET_CARDBUS) && (skt->callback))
    + skt->callback->early_resume(skt);
    return 0;
    }

    And the corresponding change in ds.c is from:

    -static int ds_event(struct pcmcia_socket *skt, event_t event, int priority)
    -{
    - struct pcmcia_socket *s = pcmcia_get_socket(skt);
    ...
    - switch (event) {
    ...
    - case CS_EVENT_PM_RESUME:
    - if (verify_cis_cache(skt) != 0) {
    - dev_dbg(&skt->dev, "cis mismatch - different card\n");
    - /* first, remove the card */
    - ds_event(skt, CS_EVENT_CARD_REMOVAL, CS_EVENT_PRI_HIGH);
    - mutex_lock(&s->ops_mutex);
    - destroy_cis_cache(skt);
    - kfree(skt->fake_cis);
    - skt->fake_cis = NULL;
    - s->functions = 0;
    - mutex_unlock(&s->ops_mutex);
    - /* now, add the new card */
    - ds_event(skt, CS_EVENT_CARD_INSERTION,
    - CS_EVENT_PRI_LOW);
    - }
    - break;
    ...
    - }

    - pcmcia_put_socket(s);

    - return 0;
    -} /* ds_event */

    to:

    +static int pcmcia_bus_early_resume(struct pcmcia_socket *skt)
    +{
    + if (!verify_cis_cache(skt)) {
    + pcmcia_put_socket(skt);
    + return 0;
    + }

    + dev_dbg(&skt->dev, "cis mismatch - different card\n");

    + /* first, remove the card */
    + pcmcia_bus_remove(skt);
    + mutex_lock(&skt->ops_mutex);
    + destroy_cis_cache(skt);
    + kfree(skt->fake_cis);
    + skt->fake_cis = NULL;
    + skt->functions = 0;
    + mutex_unlock(&skt->ops_mutex);

    + /* now, add the new card */
    + pcmcia_bus_add(skt);
    + return 0;
    +}

    As can be seen, the original function called pcmcia_get_socket() and
    pcmcia_put_socket() around the guts, whereas the replacement code
    calls pcmcia_put_socket() only in one path. This creates an imbalance
    in the refcounting.

    Testing with pcmcia_put_socket() put removed shows that the bug is gone:

    dpm_suspend: c1a10998 [pcmcia_socket0] 5
    dpm_suspend_noirq: c1a10998 [pcmcia_socket0] 5
    dpm_resume_noirq: c1a10998 [pcmcia_socket0] 5
    dpm_resume: c1a10998 [pcmcia_socket0] 5
    dpm_complete: c1a10998 [pcmcia_socket0] 5

    Tested-by: Russell King
    Signed-off-by: Russell King
    Cc: Dominik Brodowski
    Cc:
    Signed-off-by: Andrew Morton
    Signed-off-by: Linus Torvalds

    Russell King
     

27 Jan, 2012

1 commit

  • This patch (as1514) cleans up some places where new_id and remove_id
    sysfs attributes are created and deleted. Handling both attributes in
    a single routine rather than a pair of routines makes the code
    smaller. It also prevents certain kinds of errors, like one we
    currently have in the USB subsystem: The removeid attribute is often
    created even when newid isn't (because the driver's no_dynamid_id flag
    is set).

    In the case of the PCMCIA subsystem, the newid attribute is created
    but never explicitly deleted. The patch adds a deletion routine.

    Signed-off-by: Alan Stern
    Acked-by: Jesse Barnes
    Acked-by: Dominik Brodowski
    Signed-off-by: Greg Kroah-Hartman

    Alan Stern
     

25 Jan, 2012

1 commit

  • As part of the removal of get_driver()/put_driver(), this patch
    (as1511) changes all the places that add dynamic IDs for drivers.
    Since these additions are done by writing to the drivers' sysfs
    attribute files, and the attributes are removed when the drivers are
    unregistered, there is no reason to take an extra reference to the
    drivers.

    The one exception is the pci-stub driver, which calls pci_add_dynid()
    as part of its registration. But again, there's no reason to take an
    extra reference here, because the driver can't be unloaded while it is
    being registered.

    Signed-off-by: Alan Stern
    CC: Dmitry Torokhov
    CC: Jiri Kosina
    CC: Jesse Barnes
    CC: Dominik Brodowski
    Signed-off-by: Greg Kroah-Hartman

    Alan Stern
     

06 May, 2011

1 commit


29 Sep, 2010

4 commits

  • Tested-by: Wolfram Sang
    Signed-off-by: Dominik Brodowski

    Dominik Brodowski
     
  • Remove this unnecessary message -- this info is either available
    in sysfs or by enabling dynamic debug from the PCMCIA core.

    CC: netdev@vger.kernel.org
    CC: linux-wireless@vger.kernel.org
    CC: linux-scsi@vger.kernel.org
    CC: linux-usb@vger.kernel.org
    Tested-by: Wolfram Sang
    Signed-off-by: Dominik Brodowski

    Dominik Brodowski
     
  • pcmcia_enable_device() now replaces pcmcia_request_configuration().
    Instead of config_req_t, all necessary flags are either passed as
    a parameter to pcmcia_enable_device(), or (in rare circumstances)
    set in struct pcmcia_device -> flags.

    With the last remaining user of include/pcmcia/cs.h gone, remove
    all references.

    CC: netdev@vger.kernel.org
    CC: linux-wireless@vger.kernel.org
    CC: linux-ide@vger.kernel.org
    CC: linux-usb@vger.kernel.org
    CC: laforge@gnumonks.org
    CC: linux-mtd@lists.infradead.org
    CC: alsa-devel@alsa-project.org
    CC: linux-serial@vger.kernel.org
    CC: Jiri Kosina
    CC: linux-scsi@vger.kernel.org
    Acked-by: Gustavo F. Padovan (for drivers/bluetooth)
    Tested-by: Wolfram Sang
    Signed-off-by: Dominik Brodowski

    Dominik Brodowski
     
  • Several drivers prefer to explicitly set config_{base,index,regs},
    formerly known as ConfigBase, ConfigIndex and Present. Instead of
    passing these values inside config_req_t, store it in struct
    pcmcia_device.

    CC: netdev@vger.kernel.org
    CC: linux-wireless@vger.kernel.org
    CC: linux-ide@vger.kernel.org
    CC: linux-usb@vger.kernel.org
    CC: laforge@gnumonks.org
    CC: linux-mtd@lists.infradead.org
    CC: alsa-devel@alsa-project.org
    CC: linux-serial@vger.kernel.org
    CC: Jiri Kosina
    CC: linux-scsi@vger.kernel.org
    Acked-by: Gustavo F. Padovan (for drivers/bluetooth)
    Tested-by: Wolfram Sang
    Signed-off-by: Dominik Brodowski

    Dominik Brodowski
     

03 Aug, 2010

4 commits


31 Jul, 2010

5 commits

  • Remove cs_types.h which is no longer needed: Most definitions aren't
    used at all, a few can be made away with, and two remaining definitions
    (typedefs, unfortunatley) may be moved to more specific places.

    CC: linux-ide@vger.kernel.org
    CC: linux-usb@vger.kernel.org
    CC: laforge@gnumonks.org
    CC: linux-mtd@lists.infradead.org
    CC: alsa-devel@alsa-project.org
    CC: linux-serial@vger.kernel.org
    Acked-by: Marcel Holtmann (for drivers/bluetooth/)
    Acked-by: David S. Miller
    Signed-off-by: Dominik Brodowski

    Dominik Brodowski
     
  • As we only provide one way to set up resources now, we can remove
    the resource-setup-related bitfield (except resource_setup_done).
    In addition, pcmcia_state only consisted of one entry, so remove
    this bitfield as well.

    Suggested-by: Komuro
    Signed-off-by: Dominik Brodowski

    Dominik Brodowski
     
  • Split up the central event handler for 16bit cards into three individual
    functions.

    Signed-off-by: Dominik Brodowski

    Dominik Brodowski
     
  • The event callback for handling 16bit PCMCIA cards only needs to be
    informed about a few events. Furthermore, send_event may already
    only be called with skt->skt_mutex held, which also protects against
    the module being removed behind the callback's back.

    Signed-off-by: Dominik Brodowski

    Dominik Brodowski
     
  • Signed-off-by: Dominik Brodowski

    Dominik Brodowski
     

05 Jul, 2010

1 commit


08 Jun, 2010

1 commit

  • Commit a8408c17 introduced a new check to pccard_validate_cis(),
    which avoids any "late" calls to this function. This broke the
    insertion of cards which require a CIS override which changes
    the number of card functions. Fix this by asserting that this
    is _not_ a late call, but a proper call early during the card
    insertion process.

    Fixes https://bugzilla.kernel.org/show_bug.cgi?id=16138

    Reported-by: Mikulas Patocka
    CC:
    Signed-off-by: Dominik Brodowski

    Dominik Brodowski
     

10 May, 2010

4 commits

  • As all cards to pcmcia_device_add() are already locked by skt_mutex, and
    the critical sections inside this function are further protected by
    ops_mutex, there's no need to keep a third lock around. Therfore, remove
    pcmcia_add_device_lock.

    Signed-off-by: Dominik Brodowski

    Dominik Brodowski
     
  • Remove the dev_node declaration. We now only pass the device name
    to the deprecated userspace tools.

    Signed-off-by: Dominik Brodowski

    Dominik Brodowski
     
  • Instead of the old pcmcia_request_irq() interface, drivers may now
    choose between:

    - calling request_irq/free_irq directly. Use the IRQ from *p_dev->irq.

    - use pcmcia_request_irq(p_dev, handler_t); the PCMCIA core will
    clean up automatically on calls to pcmcia_disable_device() or
    device ejection.

    - drivers still not capable of IRQF_SHARED (or not telling us so) may
    use the deprecated pcmcia_request_exclusive_irq() for the time
    being; they might receive a shared IRQ nonetheless.

    CC: linux-bluetooth@vger.kernel.org
    CC: netdev@vger.kernel.org
    CC: linux-wireless@vger.kernel.org
    CC: linux-serial@vger.kernel.org
    CC: alsa-devel@alsa-project.org
    CC: linux-usb@vger.kernel.org
    CC: linux-ide@vger.kernel.org
    Signed-off-by: Dominik Brodowski

    Dominik Brodowski
     
  • Setup the IRQ to be used by PCMCIA drivers already during the device
    registration stage, making use of a new function pcmcia_setup_irq().
    This will allow us to get rid of quite a lot of indirection in the
    future.

    Signed-off-by: Dominik Brodowski

    Dominik Brodowski
     

03 May, 2010

1 commit


27 Apr, 2010

1 commit

  • Prevent PCMCIA_DEV_ID_MATCH_FUNC_ID from grabbing PFC-cards:
    I changed the code, so that the first matching struct
    pcmcia_device_id _PFC_ entry will mark the card has_pfc,
    preventing PCMCIA_DEV_ID_MATCH_FUNC_ID to match.

    [linux-pcmcia@lists.infradead.org: re-order commit message]
    Signed-off-by: Alexander Kurz
    Signed-off-by: Dominik Brodowski

    Alexander Kurz
     

21 Apr, 2010

1 commit

  • pcmcia_dev_present is in and by itself buggy. Add a note specifying
    why it is broken, and replace the broken locking -- taking a mutex
    is a bad idea in IRQ context, from which this function is rarely
    called -- by an atomic_t.

    Signed-off-by: Dominik Brodowski

    Dominik Brodowski
     

11 Apr, 2010

1 commit


30 Mar, 2010

1 commit

  • …it slab.h inclusion from percpu.h

    percpu.h is included by sched.h and module.h and thus ends up being
    included when building most .c files. percpu.h includes slab.h which
    in turn includes gfp.h making everything defined by the two files
    universally available and complicating inclusion dependencies.

    percpu.h -> slab.h dependency is about to be removed. Prepare for
    this change by updating users of gfp and slab facilities include those
    headers directly instead of assuming availability. As this conversion
    needs to touch large number of source files, the following script is
    used as the basis of conversion.

    http://userweb.kernel.org/~tj/misc/slabh-sweep.py

    The script does the followings.

    * Scan files for gfp and slab usages and update includes such that
    only the necessary includes are there. ie. if only gfp is used,
    gfp.h, if slab is used, slab.h.

    * When the script inserts a new include, it looks at the include
    blocks and try to put the new include such that its order conforms
    to its surrounding. It's put in the include block which contains
    core kernel includes, in the same order that the rest are ordered -
    alphabetical, Christmas tree, rev-Xmas-tree or at the end if there
    doesn't seem to be any matching order.

    * If the script can't find a place to put a new include (mostly
    because the file doesn't have fitting include block), it prints out
    an error message indicating which .h file needs to be added to the
    file.

    The conversion was done in the following steps.

    1. The initial automatic conversion of all .c files updated slightly
    over 4000 files, deleting around 700 includes and adding ~480 gfp.h
    and ~3000 slab.h inclusions. The script emitted errors for ~400
    files.

    2. Each error was manually checked. Some didn't need the inclusion,
    some needed manual addition while adding it to implementation .h or
    embedding .c file was more appropriate for others. This step added
    inclusions to around 150 files.

    3. The script was run again and the output was compared to the edits
    from #2 to make sure no file was left behind.

    4. Several build tests were done and a couple of problems were fixed.
    e.g. lib/decompress_*.c used malloc/free() wrappers around slab
    APIs requiring slab.h to be added manually.

    5. The script was run on all .h files but without automatically
    editing them as sprinkling gfp.h and slab.h inclusions around .h
    files could easily lead to inclusion dependency hell. Most gfp.h
    inclusion directives were ignored as stuff from gfp.h was usually
    wildly available and often used in preprocessor macros. Each
    slab.h inclusion directive was examined and added manually as
    necessary.

    6. percpu.h was updated not to include slab.h.

    7. Build test were done on the following configurations and failures
    were fixed. CONFIG_GCOV_KERNEL was turned off for all tests (as my
    distributed build env didn't work with gcov compiles) and a few
    more options had to be turned off depending on archs to make things
    build (like ipr on powerpc/64 which failed due to missing writeq).

    * x86 and x86_64 UP and SMP allmodconfig and a custom test config.
    * powerpc and powerpc64 SMP allmodconfig
    * sparc and sparc64 SMP allmodconfig
    * ia64 SMP allmodconfig
    * s390 SMP allmodconfig
    * alpha SMP allmodconfig
    * um on x86_64 SMP allmodconfig

    8. percpu.h modifications were reverted so that it could be applied as
    a separate patch and serve as bisection point.

    Given the fact that I had only a couple of failures from tests on step
    6, I'm fairly confident about the coverage of this conversion patch.
    If there is a breakage, it's likely to be something in one of the arch
    headers which should be easily discoverable easily on most builds of
    the specific arch.

    Signed-off-by: Tejun Heo <tj@kernel.org>
    Guess-its-ok-by: Christoph Lameter <cl@linux-foundation.org>
    Cc: Ingo Molnar <mingo@redhat.com>
    Cc: Lee Schermerhorn <Lee.Schermerhorn@hp.com>

    Tejun Heo
     

24 Mar, 2010

1 commit


08 Mar, 2010

1 commit

  • In the future, we are going to be changing the lock type for struct
    device (once we get the lockdep infrastructure properly worked out) To
    make that changeover easier, and to possibly burry the lock in a
    different part of struct device, let's create some functions to lock and
    unlock a device so that no out-of-core code needs to be changed in the
    future.

    This patch creates the device_lock/unlock/trylock() functions, and
    converts all in-tree users to them.

    Cc: Thomas Gleixner
    Cc: Jean Delvare
    Cc: Dave Young
    Cc: Ming Lei
    Cc: Jiri Kosina
    Cc: Phil Carmody
    Cc: Arjan van de Ven
    Cc: Cornelia Huck
    Cc: Rafael J. Wysocki
    Cc: Pavel Machek
    Cc: Len Brown
    Cc: Magnus Damm
    Cc: Alan Stern
    Cc: Randy Dunlap
    Cc: Stefan Richter
    Cc: David Brownell
    Cc: Vegard Nossum
    Cc: Jesse Barnes
    Cc: Alex Chiang
    Cc: Kenji Kaneshige
    Cc: Andrew Morton
    Cc: Andrew Patterson
    Cc: Yu Zhao
    Cc: Dominik Brodowski
    Cc: Samuel Ortiz
    Cc: Wolfram Sang
    Cc: CHENG Renquan
    Cc: Oliver Neukum
    Cc: Frans Pop
    Cc: David Vrabel
    Cc: Kay Sievers
    Cc: Sarah Sharp
    Signed-off-by: Greg Kroah-Hartman

    Greg Kroah-Hartman
     

18 Feb, 2010

9 commits