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

login
register
mail settings
Submitter Chris J Arges
Date Dec. 10, 2012, 6:05 p.m.
Message ID <50C62454.7060700@canonical.com>
Download mbox
Permalink /patch/205003/
State New
Headers show

Pull-request

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

Comments

Chris J Arges - Dec. 10, 2012, 6:05 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 up of patches
0520bffba9685d88ad68ede4a41abd08a3e9684e..fe9b25d3ee6bdf6f9c9a9ce61d9d3e144bac13ef
found in the for-next branch in the notify.git tree solve this issue:
http://git.infradead.org/users/eparis/notify.git/shortlog/refs/heads/for-next
These have been cherry-picked and tested on precise/quantal and applied
already to raring. Only small modifications are needed for 2 of the
patches because the locations of the functions had changed other than
that the other 7 patches are clean cherry-picks.

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.

--

The following changes since commit d59169d413ab95d290a70acf6f2e42d647c5efec:

  UBUNTU: Ubuntu-3.5.0-19.30 (2012-11-13 15:52:20 +0000)

are available in the git repository at:

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

for you to fetch changes up to 763c1bb0f1f329705c16945ebf3730a9b610af2c:

  UBUNTU: SAUCE: fsnotify: dont put marks on temporary list when
clearing marks by group (2012-12-07 17:13:38 -0600)

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

 fs/notify/dnotify/dnotify.c          |    4 +-
 fs/notify/fanotify/fanotify_user.c   |   19 +++++---
 fs/notify/group.c                    |   38 ++++++++--------
 fs/notify/inode_mark.c               |   14 ++++--
 fs/notify/inotify/inotify_fsnotify.c |    4 +-
 fs/notify/inotify/inotify_user.c     |    3 +-
 fs/notify/mark.c                     |   81
+++++++++++++++++++---------------
 fs/notify/vfsmount_mark.c            |   14 ++++--
 include/linux/fsnotify_backend.h     |   16 ++++---
 kernel/audit_tree.c                  |   10 ++---
 kernel/audit_watch.c                 |    4 +-
 11 files changed, 125 insertions(+), 82 deletions(-)
Tim Gardner - Dec. 10, 2012, 10:05 p.m.
Good success with Raring. Lets get some regression testing on master-next.
Chris J Arges - Dec. 29, 2012, 7:57 p.m.
On 12/10/2012 12:05 PM, Chris J Arges wrote:
> 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 up of patches
> 0520bffba9685d88ad68ede4a41abd08a3e9684e..fe9b25d3ee6bdf6f9c9a9ce61d9d3e144bac13ef
> found in the for-next branch in the notify.git tree solve this issue:
> http://git.infradead.org/users/eparis/notify.git/shortlog/refs/heads/for-next
> These have been cherry-picked and tested on precise/quantal and applied
> already to raring. Only small modifications are needed for 2 of the
> patches because the locations of the functions had changed other than
> that the other 7 patches are clean cherry-picks.
> 

Note these patches have now been merged into mainline 3.8:
https://lkml.org/lkml/2012/12/20/439

I'm not sure if we need to reapply based on these commits or not. Let me
know if any actions are needed as we've applied this to
quantal/precise/raring already.

Happy new year,
--chris j arges


> 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.
> 
> --
> 
> The following changes since commit d59169d413ab95d290a70acf6f2e42d647c5efec:
> 
>   UBUNTU: Ubuntu-3.5.0-19.30 (2012-11-13 15:52:20 +0000)
> 
> are available in the git repository at:
> 
>   git://kernel.ubuntu.com/arges/ubuntu-quantal.git lp922906
> 
> for you to fetch changes up to 763c1bb0f1f329705c16945ebf3730a9b610af2c:
> 
>   UBUNTU: SAUCE: fsnotify: dont put marks on temporary list when
> clearing marks by group (2012-12-07 17:13:38 -0600)
> 
> ----------------------------------------------------------------
> Lino Sanfilippo (8):
>       UBUNTU: SAUCE: fsnotify: introduce fsnotify_get_group()
>       UBUNTU: SAUCE: fsnotify: use reference counting for groups
>       UBUNTU: SAUCE: fsnotify: take groups mark_lock before mark lock
>       UBUNTU: SAUCE: fanotify: add an extra flag to
> mark_remove_from_mask that indicates wheather a mark should be destroyed
>       UBUNTU: SAUCE: fsnotify: use a mutex instead of a spinlock to
> protect a groups mark list
>       UBUNTU: SAUCE: fsnotify: pass group to fsnotify_destroy_mark()
>       UBUNTU: SAUCE: fsnotify: introduce locked versions of
> fsnotify_add_mark() and fsnotify_remove_mark()
>       UBUNTU: SAUCE: fsnotify: dont put marks on temporary list when
> clearing marks by group
> 
>  fs/notify/dnotify/dnotify.c          |    4 +-
>  fs/notify/fanotify/fanotify_user.c   |   19 +++++---
>  fs/notify/group.c                    |   38 ++++++++--------
>  fs/notify/inode_mark.c               |   14 ++++--
>  fs/notify/inotify/inotify_fsnotify.c |    4 +-
>  fs/notify/inotify/inotify_user.c     |    3 +-
>  fs/notify/mark.c                     |   81
> +++++++++++++++++++---------------
>  fs/notify/vfsmount_mark.c            |   14 ++++--
>  include/linux/fsnotify_backend.h     |   16 ++++---
>  kernel/audit_tree.c                  |   10 ++---
>  kernel/audit_watch.c                 |    4 +-
>  11 files changed, 125 insertions(+), 82 deletions(-)
>
Tim Gardner - Jan. 2, 2013, 1:44 p.m.
On 12/29/2012 12:57 PM, Chris J Arges wrote:
> On 12/10/2012 12:05 PM, Chris J Arges wrote:
>> 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 up of patches
>> 0520bffba9685d88ad68ede4a41abd08a3e9684e..fe9b25d3ee6bdf6f9c9a9ce61d9d3e144bac13ef
>> found in the for-next branch in the notify.git tree solve this issue:
>> http://git.infradead.org/users/eparis/notify.git/shortlog/refs/heads/for-next
>> These have been cherry-picked and tested on precise/quantal and applied
>> already to raring. Only small modifications are needed for 2 of the
>> patches because the locations of the functions had changed other than
>> that the other 7 patches are clean cherry-picks.
>>
> Note these patches have now been merged into mainline 3.8:
> https://lkml.org/lkml/2012/12/20/439
>
> I'm not sure if we need to reapply based on these commits or not. Let me
> know if any actions are needed as we've applied this to
> quantal/precise/raring already.
>
> Happy new year,
> --chris j arges
>
>
>

It appears that there is at least one more patch 
(6960b0d909cde5bdff49e4e5c1250edd10be7ebd) in the upstream series that 
addresses fsnotify locking. Would you care to review updates since v3.7 
and suggest a patch set to replace the SAUCE patches that we're carrying 
in Quantal master-next ?

rtg
Chris J Arges - Jan. 2, 2013, 3:56 p.m.
<snip>
>>
> 
> It appears that there is at least one more patch
> (6960b0d909cde5bdff49e4e5c1250edd10be7ebd) in the upstream series that
> addresses fsnotify locking. Would you care to review updates since v3.7
> and suggest a patch set to replace the SAUCE patches that we're carrying
> in Quantal master-next ?
> 
> rtg
> 

Ok I will work on this.
--chris