From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5711C2F60DF for ; Fri, 24 Oct 2025 20:37:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1761338253; cv=none; b=q+tEB7xv+Wy9RNN0yMAT0z63FNYg14lw1ucTMDEdeo2blC83CM5EFEDnEviaB4OjD3RLx5buJ4F/+pX67Acs/yyjVEQJPS9jv+v9CEl9m5X3YR7akJEQfLW6yZsnlscz1YJYsdn5yAfdVXoLcoZIQ93eOgUOWOQQmD6dKhv38Lc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1761338253; c=relaxed/simple; bh=q7wOtUQ3w/xyhiTk/FxfT6i7WPIIu7UijsYHUvqAWOg=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=GZ8nog/kWMMIzyTrEkN+PYOWqYqdfMpVWhZx22VRBGQ9hqurOy9JsyQ+xTgkDnHN1oLY8LJF3EdxwAvv4tvvqNNF81A7kYL7dyqZdjl2vvrXbhcYfgWfSKsTzJhiHC5nbP8OpLKwGom6xEkRrpWnLq61qdO6CeJfJ15EFv8eLEY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=WVxjMRL/; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="WVxjMRL/" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 89506C4CEF1; Fri, 24 Oct 2025 20:37:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1761338252; bh=q7wOtUQ3w/xyhiTk/FxfT6i7WPIIu7UijsYHUvqAWOg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=WVxjMRL//Z+ZAiLF2YNIzp5ninMCHaRXuAmh2+Yeu3P5T1SR8SYcLARTYUFB1qiZl Gqrf9D6xWYeOb0ruHj6EK8dRwqBzqtp8jLVF0o6cS/KUhrktii9tjgCqNLGB4+gsE5 MFWu0afvjV/zdsLIkYApr8Lt+IcDtNBuHjEU4y4LHkYABxvVEx5kUBKWvcbQZWgkQU 4aZP2jc71/fQLETjzumMovs4ar23ny6O/bqhuDqegVfgg7IqAIXjI4Otvf9jLqQa/0 xhBVSXhTBxzYFZ6dsaMvBqPTdEvlqAALYQt0JTovL7uGaaH3o9NhchVMqZEtAYdXV7 yQY0p7mR1I9fA== Date: Fri, 24 Oct 2025 16:37:31 -0400 From: Mike Snitzer To: Chuck Lever Cc: NeilBrown , Jeff Layton , Olga Kornievskaia , Dai Ngo , Tom Talpey , linux-nfs@vger.kernel.org, Chuck Lever , Christoph Hellwig Subject: Re: [PATCH v7 14/14] NFSD: Initialize separate ki_flags Message-ID: References: <20251024144306.35652-1-cel@kernel.org> <20251024144306.35652-15-cel@kernel.org> Precedence: bulk X-Mailing-List: linux-nfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Fri, Oct 24, 2025 at 03:34:00PM -0400, Chuck Lever wrote: > On 10/24/25 2:13 PM, Mike Snitzer wrote: > >> /* > >> - * IOCB_SYNC + IOCB_DIRECT requests that iter_write should persist > >> - * both written data and dirty time stamps. > >> - * > >> - * When falling back to buffered I/O or handling the unaligned > >> - * first and last segments, the data and time stamps must be > >> - * durable before nfsd_vfs_write() returns to its caller, matching > >> - * the behavior of direct I/O. > >> + * IOCB_SYNC + IOCB_DIRECT requests that iter_write should > >> + * persist both written data and dirty time stamps. > >> */ > >> - kiocb->ki_flags |= IOCB_SYNC | IOCB_DSYNC; > >> + args.flags_direct = kiocb->ki_flags | IOCB_SYNC | IOCB_DIRECT; > > AFAIK we still need: IOCB_DIRECT | IOCB_DSYNC | IOCB_SYNC > > > > IOCB_DIRECT | IOCB_DSYNC was recently put under a microscope relative > > to XFS performance and the resulting improvement was merged for 6.18 > > with commit c91d38b57f ("xfs: rework datasync tracking and execution") > > This looks like an xfs-specific fix. I'm reluctant to apply a fix for > a specific file system implementation in what's supposed to be generic > code. > > If (IOCB_DIRECT | IOCB_DSYNC | IOCB_SYNC) /is/ correct for all file > systems, then it needs an explanatory code comment, which I'm not yet > qualified to write. I don't see any textual material in previous > incarnations of this code that might help get me started. The XFS specific performance improvement isn't the point. The point is that applications (like I think DB2 is what started all this with Jan Kara and the XFS filesystem) results in the use of O_DIRECT+O_DSYNC. It is a clear reality that other filesystems are catering to O_DIRECT+O_DSYNC. And given our findings with Christoph that buffered IO needs O_DSYNC+O_SYNC, I'd rather we not expose ourselves to not having O_DSYNC set. Particularly because any filesystem NFSD is writing to _can_ also fallback to using buffered IO if O_DIRECT set (NFSD is doing exactly that). Which we _know_ (from Christoph) that having O_DSYNC set is important when we fallback to using buffered IO (like we do for the misaligned head and/or tail). Please let's not make the same mistake twice.