On Wed 20-06-18 21:29:12, Matthew Wilcox wrote:
On Thu, Jun 21, 2018 at 11:32:45AM +0800, Jia-Ju Bai wrote:
> The kernel may sleep with holding a spinlock.
> The function call paths (from bottom to top) in Linux-4.16.7 are:
>
> [FUNC] kmem_cache_alloc(GFP_KERNEL)
> fs/notify/mark.c, 439:
> kmem_cache_alloc in fsnotify_attach_connector_to_object
> fs/notify/mark.c, 520:
> fsnotify_attach_connector_to_object in fsnotify_add_mark_list
> fs/notify/mark.c, 590:
> fsnotify_add_mark_list in fsnotify_add_mark_locked
> kernel/audit_tree.c, 437:
> fsnotify_add_mark_locked in tag_chunk
> kernel/audit_tree.c, 423:
> spin_lock in tag_chunk
There are several locks here; your report would be improved by saying
which one is the problem. I'm assuming it's old_entry->lock.
spin_lock(&old_entry->lock);
...
if (fsnotify_add_inode_mark_locked(chunk_entry,
old_entry->connector->inode, 1)) {
...
return fsnotify_add_mark_locked(mark, inode, NULL, allow_dups);
...
ret = fsnotify_add_mark_list(mark, inode, mnt, allow_dups);
...
if (inode)
connp = &inode->i_fsnotify_marks;
conn = fsnotify_grab_connector(connp);
if (!conn) {
err = fsnotify_attach_connector_to_object(connp, inode, mnt);
It seems to me that this is safe because old_entry is looked up from
fsnotify_find_mark, and it can't be removed while its lock is held.
Therefore there's always a 'conn' returned from fsnotify_grab_connector(),
and so this path will never be taken.
But this code path is confusing to me, and I could be wrong. Jan, please
confirm my analysis is correct?
Yes, you are correct. The presence of another mark in the list (and the
fact we pin it there using refcount & mark_mutex) guarantees we won't need
to allocate the connector. I agree the audit code's use of fsnotify would
deserve some cleanup.
Honza
--
Jan Kara <jack(a)suse.com>
SUSE Labs, CR