Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Date:   Fri, 20 Oct 2023 20:09:53 +0200
From:   Jan Kara <jack@suse.cz>
To:     Linus Torvalds <torvalds@linux-foundation.org>
Cc:     Andy Shevchenko <andriy.shevchenko@intel.com>,
        Baokun Li <libaokun1@huawei.com>,
        Josh Poimboeuf <jpoimboe@kernel.org>, Jan Kara <jack@suse.cz>,
        Nathan Chancellor <nathan@kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Kees Cook <keescook@chromium.org>,
        Ferry Toth <ftoth@exalondelft.nl>,
        linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org
Subject: Re: [GIT PULL] ext2, quota, and udf fixes for 6.6-rc1
Message-ID: <20231020180953.wdehmwqoqndk5shq@quack3>
References: <ZTFh0NeYtvgcjSv8@smile.fi.intel.com>
 <CAHk-=wjXG52UNKCwwEU1A+QWHYfvKOieV0uFOpPkLR0NSvOjtg@mail.gmail.com>
 <CAHk-=whis2BJF2fv1xySAg2NTQ+C5fViNSGkLNCOqGzi-3y+8w@mail.gmail.com>
 <ZTFxEcjo4d6vXbo5@smile.fi.intel.com>
 <ZTFydEbdEYlxOxc1@smile.fi.intel.com>
 <CAHk-=wh_gbZE_ZsQ6+9gSPdXfoCtmuK-MFmBkO3ywMKFQEvb6g@mail.gmail.com>
 <ZTKUDzONVHXnWAJc@smile.fi.intel.com>
 <ZTKXbbSS2Pvmc-Fh@smile.fi.intel.com>
 <ZTKY6nRGWoYsEJjj@smile.fi.intel.com>
 <CAHk-=whzn2AVM6iSfy64h8TPjL6DtirO-YKW9o8afEw1s9nbjw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAHk-=whzn2AVM6iSfy64h8TPjL6DtirO-YKW9o8afEw1s9nbjw@mail.gmail.com>
Precedence: bulk

On Fri 20-10-23 10:26:26, Linus Torvalds wrote:
> On Fri, 20 Oct 2023 at 08:12, Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> >
> > > > --- a/fs/quota/dquot.c
> > > > +++ b/fs/quota/dquot.c
> > > > @@ -632,8 +632,10 @@ static inline int dquot_write_dquot(struct dquot *dquot)
> > > >  {
> > > >         int ret = dquot->dq_sb->dq_op->write_dquot(dquot);
> > > >         if (ret < 0) {
> > > > +#if 0
> > > >                 quota_error(dquot->dq_sb, "Can't write quota structure "
> > > >                             "(error %d). Quota may get out of sync!", ret);
> > > > +#endif
> > > >                 /* Clear dirty bit anyway to avoid infinite loop. */
> > > >                 clear_dquot_dirty(dquot);
> > > >         }
> >
> > Doing the same on the my branch based on top of v6.6-rc6 does not help.
> > So looks like a race condition somewhere happening related to that dirty bit
> > (as comment states it needs to be cleaned to avoid infinite loop, that's
> >  probably what happens).
> 
> Hmm. Normally, dirty bits should always be cleared *before* the
> write-back, not after it. Otherwise you might lose a dirty event that
> happened *during* writeback.

Yes, and normally we clear the dirty bit in dquot_commit() before writing
the dquot. However if there is an error in fs-private ->write_dquot()
helper before calling back into dquot_commit() (e.g. ext4 fails to start a
transaction), ->write_dquot() can return without clearing the dirty bit.
For dqput() to not loop indefinitely trying to clean the dquot, we clear
the dirty bit here just to be sure in case of error.

> But I don't know the quota code.
> 
> ... the fact that the #if 0 doesn't help in another case does say that
> it's not the quota_error() call itself. Which it really couldn't have
> been (apart from timing and compiler bugs), but it's still a data
> point, I guess.

Yeah, that's a bit weird. I'm really curious what the problem is.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR