Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <183bdfcf876e7a33586e3236d73b7ad5ba88f293.camel@kernel.org>
Subject: Re: [PATCH][RESEND] vfs: serialize updates to file->f_sb_err with
 f_lock
From:   Jeff Layton <jlayton@kernel.org>
To:     Al Viro <viro@zeniv.linux.org.uk>
Cc:     linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        sargun@sargun.me, amir73il@gmail.com, vgoyal@redhat.com
Date:   Mon, 04 Jan 2021 14:00:18 -0500
In-Reply-To: <20210104185717.GK3579531@ZenIV.linux.org.uk>
References: <20210104184347.90598-1-jlayton@kernel.org>
         <20210104185717.GK3579531@ZenIV.linux.org.uk>
Content-Type: text/plain; charset="ISO-8859-15"
User-Agent: Evolution 3.38.2 (3.38.2-1.fc33) 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Mon, 2021-01-04 at 18:57 +0000, Al Viro wrote:
> On Mon, Jan 04, 2021 at 01:43:47PM -0500, Jeff Layton wrote:
> > @@ -172,7 +172,12 @@ SYSCALL_DEFINE1(syncfs, int, fd)
> > ?	ret = sync_filesystem(sb);
> > ?	up_read(&sb->s_umount);
> > ?
> > 
> > -	ret2 = errseq_check_and_advance(&sb->s_wb_err, &f.file->f_sb_err);
> > +	if (errseq_check(&sb->s_wb_err, f.file->f_sb_err)) {
> > +		/* Something changed, must use slow path */
> > +		spin_lock(&f.file->f_lock);
> > +		ret2 = errseq_check_and_advance(&sb->s_wb_err, &f.file->f_sb_err);
> > +		spin_unlock(&f.file->f_lock);
> > +	}
> 
> 	Is there any point bothering with the fastpath here?
> I mean, look at the up_read() immediately prior to that thing...

It is a micro-optimization, but the vastly common case is that we will
avoid the spinlock there. That said, I'm fine with dropping the fastpath
if you prefer.

-- 
Jeff Layton <jlayton@kernel.org>