Patchwork [precise,sru] pull-request: fsnotify: simplify locking

login
register
mail settings
Submitter Chris J Arges
Date Jan. 3, 2013, 8:16 p.m.
Message ID <50E5E737.9060702@canonical.com>
Download mbox
Permalink /patch/209305/
State New
Headers show

Pull-request

git://kernel.ubuntu.com/arges/ubuntu-precise.git lp922906

Comments

Chris J Arges - Jan. 3, 2013, 8:16 p.m.
BugLink: http://bugs.launchpad.net/bugs/922906

SRU Justification:

Impact:
When plugging and unplugging a USB drive occasionally a race condition
in the notify subsystem causes a kernel oops. A more in depth explaining
can be found here: https://lkml.org/lkml/2011/1/19/213.

Fix:
A set of patches from upstream address this issue, this should already
be present in raring.

Testcase:
Comment #8 and #9 in the upstream bug:
https://bugzilla.kernel.org/show_bug.cgi?id=22602 has a test case that
easily reproduces this issue within 15-30 minutes. I have applied the
above fixes and was able to run this test case overnight in all cases.
In addition I've tested using the LTP tests for inotfy and these run
properly with the fix applied.

Note:
The SAUCE patches will need to be reverted, and it seems I missed a
patch when I did the fix before.

--

The following changes since commit 517fd8561b857c856d4a8663ca2f4594d7c40288:

  UBUNTU: Ubuntu-3.2.0-36.56 (2013-01-02 12:38:18 -0800)

are available in the git repository at:

  git://kernel.ubuntu.com/arges/ubuntu-precise.git lp922906

for you to fetch changes up to 82149d163c4e957b8542422c58678960c2340b51:

  fsnotify: change locking order (2013-01-03 13:38:04 -0600)

----------------------------------------------------------------
Chris J Arges (8):
      Revert "UBUNTU: SAUCE: fsnotify: dont put marks on temporary list
when clearing marks by group"
      Revert "UBUNTU: SAUCE: fsnotify: introduce locked versions of
fsnotify_add_mark() and fsnotify_remove_mark()"
      Revert "UBUNTU: SAUCE: fsnotify: pass group to
fsnotify_destroy_mark()"
      Revert "UBUNTU: SAUCE: fsnotify: use a mutex instead of a spinlock
to protect a groups mark list"
      Revert "UBUNTU: SAUCE: fanotify: add an extra flag to
mark_remove_from_mask that indicates wheather a mark should be destroyed"
      Revert "UBUNTU: SAUCE: fsnotify: take groups mark_lock before mark
lock"
      Revert "UBUNTU: SAUCE: fsnotify: use reference counting for groups"
      Revert "UBUNTU: SAUCE: fsnotify: introduce fsnotify_get_group()"

Lino Sanfilippo (9):
      fsnotify: introduce fsnotify_get_group()
      fsnotify: use reference counting for groups
      fsnotify: take groups mark_lock before mark lock
      fanotify: add an extra flag to mark_remove_from_mask that
indicates wheather a mark should be destroyed
      fsnotify: use a mutex instead of a spinlock to protect a groups
mark list
      fsnotify: pass group to fsnotify_destroy_mark()
      fsnotify: introduce locked versions of fsnotify_add_mark() and
fsnotify_remove_mark()
      fsnotify: dont put marks on temporary list when clearing marks by
group
      fsnotify: change locking order

 fs/notify/mark.c                 |   20 ++++++++++----------
 include/linux/fsnotify_backend.h |    7 ++++---
 kernel/audit_tree.c              |    4 ++--
 3 files changed, 16 insertions(+), 15 deletions(-)
Tim Gardner - Jan. 4, 2013, 12:24 p.m.
Chris - I hate to be a giant pain in your ass, but I think this patch 
set requires a new bug report. We missed the packaging window so I can't 
rebase the original series out of existence, plus there is one extra 
patch in this new series that again messes with locking order which 
deserves regression testing.

rtg
Chris J Arges - Jan. 4, 2013, 2:03 p.m.
On 01/04/2013 06:24 AM, Tim Gardner wrote:
> Chris - I hate to be a giant pain in your ass, but I think this patch
> set requires a new bug report. We missed the packaging window so I can't
> rebase the original series out of existence, plus there is one extra
> patch in this new series that again messes with locking order which
> deserves regression testing.
> 
> rtg

Ok I'll create a new bug report and new SRU.

Overall is this pull-request OK with the reverts + new patches? I'll
change the branch name before resubmitting, but want to be sure I have
all my i's crossed and t's dotted.

I've run the quantal patches for most of the day a few days ago, but I
can run a few VM's over the weekend as well with the test running after
rebuilding.

--chris
Tim Gardner - Jan. 4, 2013, 2:32 p.m.
On 01/04/2013 07:03 AM, Chris J Arges wrote:
> On 01/04/2013 06:24 AM, Tim Gardner wrote:
>> Chris - I hate to be a giant pain in your ass, but I think this patch
>> set requires a new bug report. We missed the packaging window so I can't
>> rebase the original series out of existence, plus there is one extra
>> patch in this new series that again messes with locking order which
>> deserves regression testing.
>>
>> rtg
> 
> Ok I'll create a new bug report and new SRU.
> 
> Overall is this pull-request OK with the reverts + new patches? I'll
> change the branch name before resubmitting, but want to be sure I have
> all my i's crossed and t's dotted.
> 
> I've run the quantal patches for most of the day a few days ago, but I
> can run a few VM's over the weekend as well with the test running after
> rebuilding.
> 
> --chris
> 

The pull request was fine with the reverts (which are now required).

rtg