lkml.org 
[lkml]   [2014]   [Sep]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] UBI: Fix possible deadlock in erase_worker()
    Am 17.09.2014 10:28, schrieb Artem Bityutskiy:
    > On Tue, 2014-09-16 at 09:48 +0200, Richard Weinberger wrote:
    >> If sync_erase() failes with EINTR, ENOMEM, EAGAIN or
    >> EBUSY erase_worker() re-schedules the failed work.
    >> This will lead to a deadlock because erase_worker() is called
    >> with work_sem held in read mode. And schedule_erase() will take
    >> this lock again.
    >
    > IIRC, the assumption was that the R/W semaphore may be taken in read
    > mode many times, so it wouldn't hurt to do:
    >
    > down_read()
    > down_read()
    > up_read()
    > up_read()

    Hmm, are you sure that this is legal?

    Quoting rwsem.h:
    /*
    * nested locking. NOTE: rwsems are not allowed to recurse
    * (which occurs if the same task tries to acquire the same
    * lock instance multiple times), but multiple locks of the
    * same lock class might be taken, if the order of the locks
    * is always the same. This ordering rule can be expressed
    * to lockdep via the _nested() APIs, but enumerating the
    * subclasses that are used. (If the nesting relationship is
    * static then another method for expressing nested locking is
    * the explicit definition of lock class keys and the use of
    * lockdep_set_class() at lock initialization time.
    * See Documentation/lockdep-design.txt for more details.)
    */

    In this case the same task is taking the same lock multiple times,
    which is not allowed according to rwsem.h.

    Thanks,
    //richard


    \
     
     \ /
      Last update: 2014-09-17 11:21    [W:3.851 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site