Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220530013958.577941-1-dlunev@chromium.org> <20220530113953.v3.1.I0e579520b03aa244906b8fe2ef1ec63f2ab7eecf@changeid>
 <YpXCt14eL2edq6IB@infradead.org>
In-Reply-To: <YpXCt14eL2edq6IB@infradead.org>
From:   Daniil Lunev <dlunev@chromium.org>
Date:   Wed, 1 Jun 2022 11:11:55 +1000
Message-ID: <CAONX=-dQuiG26gCp7FVfZk83au=RXXcnTWJO8+Ygep0iVqvnxA@mail.gmail.com>
Subject: Re: [PATCH v3 1/2] fs/super: function to prevent super re-use
To:     Christoph Hellwig <hch@infradead.org>
Cc:     linux-fsdevel@vger.kernel.org, miklos@szeredi.hu,
        viro@zeniv.linux.org.uk, tytso@mit.edu,
        fuse-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Thank you for your comments. Uploaded v4 with requested changes.
--Daniil

On Tue, May 31, 2022 at 5:24 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Mon, May 30, 2022 at 11:39:57AM +1000, Daniil Lunev wrote:
> > +void retire_super(struct super_block *sb)
> > +{
> > +     down_write(&sb->s_umount);
> > +     if (sb->s_bdi != &noop_backing_dev_info) {
> > +             if (sb->s_iflags & SB_I_PERSB_BDI && !(sb->s_iflags & SB_I_RETIRED))
>
> SB_I_PERSB_BDI can't be set for noop_backing_dev_info, so that check
> should not be needed.  Which also conveniently fixes the overly long
> line.
>
> Also this should clear SB_I_PERSB_BDI as the only place that checks
> it is the unregistration.
>
> >       spin_lock(&sb_lock);
> > -     /* should be initialized for __put_super_and_need_restart() */
>
> This is a completely unrelated change.  While the function is gone
> it might be worth to check what it got renamed to or folded in, or
> if the initialization is still needed.  But all that is for a separate
> patch.
>
> >       up_write(&sb->s_umount);
> >       if (sb->s_bdi != &noop_backing_dev_info) {
> > -             if (sb->s_iflags & SB_I_PERSB_BDI)
> > +             /* retire should have already unregistered bdi */
> > +             if (sb->s_iflags & SB_I_PERSB_BDI && !(sb->s_iflags & SB_I_RETIRED))
> >                       bdi_unregister(sb->s_bdi);
> >               bdi_put(sb->s_bdi);
>
> And once SB_I_PERSB_BDI is dropped when retiring we don't need this
> change.