linux-kernel - Re: [RFC] fs: Use slab constructor to initialize conn objects in fsnotify

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200423114008.GB13910@bombadil.infradead.org>
Date:   Thu, 23 Apr 2020 04:40:08 -0700
From:   Matthew Wilcox <willy@...radead.org>
To:     "Joel Fernandes (Google)" <joel@...lfernandes.org>
Cc:     linux-kernel@...r.kernel.org, Amir Goldstein <amir73il@...il.com>,
        Jan Kara <jack@...e.cz>, linux-fsdevel@...r.kernel.org
Subject: Re: [RFC] fs: Use slab constructor to initialize conn objects in
 fsnotify

On Thu, Apr 23, 2020 at 12:40:50AM -0400, Joel Fernandes (Google) wrote:
> While reading the famous slab paper [1], I noticed that the conn->lock
> spinlock and conn->list hlist in fsnotify code is being initialized
> during every object allocation. This seems a good fit for the
> constructor within the slab to take advantage of the slab design. Move
> the initializtion to that.

The slab paper was written a number of years ago when CPU caches were
not as they are today.  With this patch, every time you allocate a
new page, we dirty the entire page, and then the dirty cachelines will
gradually fall out of cache as the other objects on the page are not used
immediately.  Then, when we actually use one of the objects on the page,
we bring those cachelines back in and dirty them again by initialising
'type' and 'obj'.  The two stores to initialise lock and list are almost
free when done in fsnotify_attach_connector_to_object(), but are costly
when done in a slab constructor.

There are very few places where a slab constructor is justified with a
modern CPU.  We've considered removing the functionality before.

> @@ -479,8 +479,6 @@ static int fsnotify_attach_connector_to_object(fsnotify_connp_t *connp,
>  	conn = kmem_cache_alloc(fsnotify_mark_connector_cachep, GFP_KERNEL);
>  	if (!conn)
>  		return -ENOMEM;
> -	spin_lock_init(&conn->lock);
> -	INIT_HLIST_HEAD(&conn->list);
>  	conn->type = type;
>  	conn->obj = connp;
>  	/* Cache fsid of filesystem containing the object */
> -- 
> 2.26.1.301.g55bc3eb7cb9-goog