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: <CAOQ4uxgifK_XTkJO69-hQvR4xQGPgHNGKJPv6-MNgHcQat5UBQ@mail.gmail.com>
Date:   Thu, 23 Apr 2020 08:24:23 +0300
From:   Amir Goldstein <amir73il@...il.com>
To:     Joel Fernandes <joel@...lfernandes.org>
Cc:     linux-kernel <linux-kernel@...r.kernel.org>,
        Jan Kara <jack@...e.cz>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [RFC] fs: Use slab constructor to initialize conn objects in fsnotify

On Thu, Apr 23, 2020 at 7:45 AM Joel Fernandes <joel@...lfernandes.org> 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,
Amir.