On Thu, Apr 23, 2020 at 9:20 AM Joel Fernandes <joel@xxxxxxxxxxxxxxxxx> wrote: > > On Thu, Apr 23, 2020 at 7:40 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > > > 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. > > Thanks a lot for this reasoning. Basically, you're saying when a slab > allocates a page, it would construct all objects which end up dirtying > the entire page before the object is even allocated. That makes sense. > > There's one improvement (although probably verys small) that the paper mentions: > Also according to the paper you referenced, the instruction cache is Correcting myself, the paper wasn't referenced by you but by a colleague :) Apologies for mistyping :) Thanks, - Joel > what would also benefit. Those spinlock and hlist initialization > instructions wouldn't cost L1 I-cache footprint for every allocation. > > > There are very few places where a slab constructor is justified with a > > modern CPU. We've considered removing the functionality before. > > I see, thanks again for the insights. > > - Joel > > > > > > @@ -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
