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

[Stefan Bader]

On 16.04.2012 20:02, 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(-)
> 

There is as always two sides: the rules of stable sanity would rather say we
should wait till the whole series is upstream and has a certain time to settle
before considering this.
Looking through the patchset which is large and touches mainly reference counts
and locking (the natural sources of madness), there is still a certain feeling
of nervousness about 3 and 8+9. It is hard to decide those things are safe or
not without knowing the whole code by heart.
On the other hand the locking of inotify seems to be broken a lot based on the
many bug reports and duplicates. Or at least the problems are quite likely to be
hit by a lot of people based on a common workflow. So it would be good to fix
things sooner than later.
If we do that, then I agree with Brad, better early. And maybe trying to be
prepared with a revert-this emergency upload in the sleve... For some reason
people always find the serious problems _after_ moving into updates...

-Stefan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 900 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20120417/ae47273b/attachment.sig>