All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: Dave Chinner <david@fromorbit.com>,
	Eric Biggers <ebiggers@kernel.org>,
	Christoph Hellwig <hch@infradead.org>,
	linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-ext4@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net,
	linux-xfs@vger.kernel.org, Theodore Ts'o <tytso@mit.edu>,
	Jaegeuk Kim <jaegeuk@kernel.org>, Chao Yu <chao@kernel.org>
Subject: Re: [PATCH v10 0/5] add support for direct I/O with fscrypt using blk-crypto
Date: Thu, 20 Jan 2022 23:12:56 -0800	[thread overview]
Message-ID: <Yepc+JcZiICsJfTQ@infradead.org> (raw)
In-Reply-To: <20220121023603.GH13563@magnolia>

On Thu, Jan 20, 2022 at 06:36:03PM -0800, Darrick J. Wong wrote:
> Sure.  How's this?  I couldn't think of a real case of directio
> requiring different alignments for pos and bytecount, so the only real
> addition here is the alignment requirements for best performance.

While I see some benefits of adding the information to a catchall like
statx we really need to be careful to not bloat the structure like
crazy.

> struct statx {
> ...
> 	/* 0x90 */
> 	__u64	stx_mnt_id;
> 
> 	/* Memory buffer alignment required for directio, in bytes. */
> 	__u32	stx_dio_mem_align;
> 
> 	/* File range alignment required for directio, in bytes. */
> 	__u32	stx_dio_fpos_align_min;

So this really needs a good explanation why we need both iven that we
had no real use case for this.

> 	/* File range alignment needed for best performance, in bytes. */
> 	__u32	stx_dio_fpos_align_opt;

And why we really care about this.  I guess you want to allow sector
size dio in reflink setups, but discourage it.  But is this really as
important?

> 	/* Maximum size of a directio request, in bytes. */
> 	__u32	stx_dio_max_iosize;

I know XFS_IOC_DIOINFO had this, but does it really make much sense?
Why do we need it for direct I/O and not buffered I/O?

WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>,
	Theodore Ts'o <tytso@mit.edu>, Dave Chinner <david@fromorbit.com>,
	linux-f2fs-devel@lists.sourceforge.net,
	Eric Biggers <ebiggers@kernel.org>,
	linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Jaegeuk Kim <jaegeuk@kernel.org>,
	linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org
Subject: Re: [f2fs-dev] [PATCH v10 0/5] add support for direct I/O with fscrypt using blk-crypto
Date: Thu, 20 Jan 2022 23:12:56 -0800	[thread overview]
Message-ID: <Yepc+JcZiICsJfTQ@infradead.org> (raw)
In-Reply-To: <20220121023603.GH13563@magnolia>

On Thu, Jan 20, 2022 at 06:36:03PM -0800, Darrick J. Wong wrote:
> Sure.  How's this?  I couldn't think of a real case of directio
> requiring different alignments for pos and bytecount, so the only real
> addition here is the alignment requirements for best performance.

While I see some benefits of adding the information to a catchall like
statx we really need to be careful to not bloat the structure like
crazy.

> struct statx {
> ...
> 	/* 0x90 */
> 	__u64	stx_mnt_id;
> 
> 	/* Memory buffer alignment required for directio, in bytes. */
> 	__u32	stx_dio_mem_align;
> 
> 	/* File range alignment required for directio, in bytes. */
> 	__u32	stx_dio_fpos_align_min;

So this really needs a good explanation why we need both iven that we
had no real use case for this.

> 	/* File range alignment needed for best performance, in bytes. */
> 	__u32	stx_dio_fpos_align_opt;

And why we really care about this.  I guess you want to allow sector
size dio in reflink setups, but discourage it.  But is this really as
important?

> 	/* Maximum size of a directio request, in bytes. */
> 	__u32	stx_dio_max_iosize;

I know XFS_IOC_DIOINFO had this, but does it really make much sense?
Why do we need it for direct I/O and not buffered I/O?


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

  reply	other threads:[~2022-01-21  7:12 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-20  7:12 [PATCH v10 0/5] add support for direct I/O with fscrypt using blk-crypto Eric Biggers
2022-01-20  7:12 ` [f2fs-dev] " Eric Biggers
2022-01-20  7:12 ` [PATCH v10 1/5] fscrypt: add functions for direct I/O support Eric Biggers
2022-01-20  7:12   ` [f2fs-dev] " Eric Biggers
2022-01-20  8:27   ` Christoph Hellwig
2022-01-20  8:27     ` [f2fs-dev] " Christoph Hellwig
2022-01-20  9:04     ` Eric Biggers
2022-01-20  9:04       ` [f2fs-dev] " Eric Biggers
2022-01-21  7:10       ` Christoph Hellwig
2022-01-21  7:10         ` [f2fs-dev] " Christoph Hellwig
2022-01-20  7:12 ` [PATCH v10 2/5] iomap: support direct I/O with fscrypt using blk-crypto Eric Biggers
2022-01-20  7:12   ` [f2fs-dev] " Eric Biggers
2022-01-20  8:28   ` Christoph Hellwig
2022-01-20  8:28     ` [f2fs-dev] " Christoph Hellwig
2022-01-20  7:12 ` [PATCH v10 3/5] ext4: " Eric Biggers
2022-01-20  7:12   ` [f2fs-dev] " Eric Biggers
2022-01-20  7:12 ` [PATCH v10 4/5] f2fs: " Eric Biggers
2022-01-20  7:12   ` [f2fs-dev] " Eric Biggers
2022-01-20  7:12 ` [PATCH v10 5/5] fscrypt: update documentation for direct I/O support Eric Biggers
2022-01-20  7:12   ` [f2fs-dev] " Eric Biggers
2022-01-20  8:30 ` [PATCH v10 0/5] add support for direct I/O with fscrypt using blk-crypto Christoph Hellwig
2022-01-20  8:30   ` [f2fs-dev] " Christoph Hellwig
2022-01-20 17:10   ` Darrick J. Wong
2022-01-20 17:10     ` [f2fs-dev] " Darrick J. Wong
2022-01-20 20:39     ` Eric Biggers
2022-01-20 20:39       ` [f2fs-dev] " Eric Biggers
2022-01-20 21:00       ` Darrick J. Wong
2022-01-20 21:00         ` [f2fs-dev] " Darrick J. Wong
2022-01-20 22:04         ` Dave Chinner
2022-01-20 22:04           ` [f2fs-dev] " Dave Chinner
2022-01-20 22:48           ` Eric Biggers
2022-01-20 22:48             ` [f2fs-dev] " Eric Biggers
2022-01-20 23:57             ` Dave Chinner
2022-01-20 23:57               ` [f2fs-dev] " Dave Chinner
2022-01-21  2:36               ` Darrick J. Wong
2022-01-21  2:36                 ` [f2fs-dev] " Darrick J. Wong
2022-01-21  7:12                 ` Christoph Hellwig [this message]
2022-01-21  7:12                   ` Christoph Hellwig
2022-01-23 23:03                 ` Dave Chinner
2022-01-23 23:03                   ` [f2fs-dev] " Dave Chinner
2022-02-09  1:10                   ` Eric Biggers
2022-02-09  1:10                     ` [f2fs-dev] " Eric Biggers
2022-02-10  4:03                     ` Dave Chinner
2022-02-10  4:03                       ` [f2fs-dev] " Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Yepc+JcZiICsJfTQ@infradead.org \
    --to=hch@infradead.org \
    --cc=chao@kernel.org \
    --cc=david@fromorbit.com \
    --cc=djwong@kernel.org \
    --cc=ebiggers@kernel.org \
    --cc=jaegeuk@kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-fscrypt@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.