[PATCH 00/10] LP#922906: oops in ticket_spin_lock

[Brad Figg]

On 04/16/2012 11:02 AM, Andy Whitcroft wrote:
> We have been seeing a large number of reports of oopses in
> ticket_spin_lock, these are mostly occuring in inotify related calls.
> There seems to be several races in this code and there is a fairly
> substantial patch set upstream to reorder the locking and sort this out.
> I have pulled these patches back to precise from the notify upstream tree,
> the patches themselves are slated for the 3.5 merge window.
> 
> Patches follow, pull request below for those who prefer to look at them
> locally.
> 
> I am proposing them (somewhat reluctantly due to their size) for Precise.
> 
> Comments?
> 
> -apw
> 
> The following changes since commit 48853edab3b414cc73c3e1c037f477524053cf15:
> 
>   fsnotify: change locking order (2012-04-16 18:33:10 +0100)
> 
> are available in the git repository at:
> 
>   git://kernel.ubuntu.com/apw/ubuntu-precise.git lp922906
> 
> for you to fetch changes up to 48853edab3b414cc73c3e1c037f477524053cf15:
> 
>   fsnotify: change locking order (2012-04-16 18:33:10 +0100)
> 
> ----------------------------------------------------------------
> 
> Lino Sanfilippo (10):
>   inotify, fanotify: replace fsnotify_put_group() with
>     fsnotify_destroy_group()
>   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/dnotify/dnotify.c          |    4 +-
>  fs/notify/fanotify/fanotify_user.c   |   33 +++++++-----
>  fs/notify/group.c                    |   40 +++++++--------
>  fs/notify/inode_mark.c               |   14 ++++--
>  fs/notify/inotify/inotify_fsnotify.c |    4 +-
>  fs/notify/inotify/inotify_user.c     |   11 ++--
>  fs/notify/mark.c                     |   91 +++++++++++++++++++---------------
>  fs/notify/vfsmount_mark.c            |   14 ++++--
>  include/linux/fsnotify_backend.h     |   26 ++++++----
>  kernel/audit_tree.c                  |   10 ++--
>  kernel/audit_watch.c                 |    4 +-
>  11 files changed, 147 insertions(+), 104 deletions(-)
> 

I've looked these over and I don't see any obvious problems.

Are we proposing to take these before they are in either Linus' or the upstream
stable trees?

Problems I have taking them now:
1. Obviously they have not have the kind of upstream review that we require for
   most patches.
2. The patches won't have received a great deal of testing from upstream. However,
   people should recognise that most of these patches don't get any kind of real
   testing until they are picked up by a distro.
3. The number of changes and the type of changes (locking and reference counting).

I'm supposed to be the one that guards the stability of the kernels (especially
LTS kernels) and be more conservative that most. However, I'm inclined to take
these patches, get them out and tested as best we can and as early in the support
period that we can so that if there are issues, they are found and fixed as
quickly as possible.

Brad
-- 
Brad Figg brad.figg at canonical.com http://www.canonical.com