On Thu, Apr 23, 2020 at 6:48 AM Jan Kara <jack@xxxxxxx> wrote: > > On Thu 23-04-20 08:24:23, Amir Goldstein wrote: > > On Thu, Apr 23, 2020 at 7:45 AM Joel Fernandes <joel@xxxxxxxxxxxxxxxxx> 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. > > > > > > > > spin_lock_init(&conn->lock); > > > > INIT_HLIST_HEAD(&conn->list); > > > > > > > > [1] https://pdfs.semanticscholar.org/1acc/3a14da69dd240f2fbc11d00e09610263bdbd.pdf > > > > > > > > > > The commit message could be better. Just to clarify, doing it this way is > > > more efficient because the object will only have its spinlock init and hlist > > > init happen during object construction, not object allocation. > > > > > > > This change may be correct, but completely unjustified IMO. > > conn objects are very rarely allocated, from user syscall path only. > > I see no reason to micro optimize this. > > > > Perhaps there is another justification to do this, but not efficiency. > > Thanks for the suggestion Joel but I agree with Amir here. In principle > using constructor is correct however it puts initialization of object in > two places which makes the code harder to follow and the allocation of > connector does not happen frequently enough for optimizing out these two > stores to matter in any tangible way. Thanks a lot Jan and Amir for your comments on the RFC patch. I am glad I got learn about this concept and appreciate the discussion very much. I agree with your analysis about the lack of constructor benefit with infrequent allocations, the other ones being: splitting object initialization into 2 code paths and also dirtying the entire page and the L1 cache that Matthew mentioned. - Joel
