All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: 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: Mon, 24 Jan 2022 10:03:32 +1100	[thread overview]
Message-ID: <20220123230332.GL59729@dread.disaster.area> (raw)
In-Reply-To: <20220121023603.GH13563@magnolia>

On Thu, Jan 20, 2022 at 06:36:03PM -0800, Darrick J. Wong wrote:
> On Fri, Jan 21, 2022 at 10:57:55AM +1100, Dave Chinner 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.
> 
> struct statx {
> ...
> 	/* 0x90 */
> 	__u64	stx_mnt_id;
> 
> 	/* Memory buffer alignment required for directio, in bytes. */
> 	__u32	stx_dio_mem_align;

	__32	stx_mem_align_dio;

(for consistency with suggestions below)

> 
> 	/* File range alignment required for directio, in bytes. */
> 	__u32	stx_dio_fpos_align_min;

"fpos" is not really a user term - "offset" is the userspace term for
file position, and it's much less of a random letter salad if it's
named that way. Also, we don't need "min" in the name; the
description of the field in the man page can give all the gory
details about it being the minimum required alignment.

	__u32	stx_offset_align_dio;

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

This is a common property of both DIO and buffered IO, so no need
for it to be dio-only property.

	__u32	stx_offset_align_optimal;

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

Unnecessary, it will always be the syscall max IO size, because the
internal DIO code will slice and dice it down to the max sizes the
hardware supports.

> #define STATX_DIRECTIO	0x00001000U	/* Want/got directio geometry */
> 
> How about that?

Mostly seems reasonable at a first look.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

WARNING: multiple messages have this Message-ID (diff)
From: Dave Chinner <david@fromorbit.com>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>,
	Theodore Ts'o <tytso@mit.edu>,
	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: Mon, 24 Jan 2022 10:03:32 +1100	[thread overview]
Message-ID: <20220123230332.GL59729@dread.disaster.area> (raw)
In-Reply-To: <20220121023603.GH13563@magnolia>

On Thu, Jan 20, 2022 at 06:36:03PM -0800, Darrick J. Wong wrote:
> On Fri, Jan 21, 2022 at 10:57:55AM +1100, Dave Chinner 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.
> 
> struct statx {
> ...
> 	/* 0x90 */
> 	__u64	stx_mnt_id;
> 
> 	/* Memory buffer alignment required for directio, in bytes. */
> 	__u32	stx_dio_mem_align;

	__32	stx_mem_align_dio;

(for consistency with suggestions below)

> 
> 	/* File range alignment required for directio, in bytes. */
> 	__u32	stx_dio_fpos_align_min;

"fpos" is not really a user term - "offset" is the userspace term for
file position, and it's much less of a random letter salad if it's
named that way. Also, we don't need "min" in the name; the
description of the field in the man page can give all the gory
details about it being the minimum required alignment.

	__u32	stx_offset_align_dio;

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

This is a common property of both DIO and buffered IO, so no need
for it to be dio-only property.

	__u32	stx_offset_align_optimal;

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

Unnecessary, it will always be the syscall max IO size, because the
internal DIO code will slice and dice it down to the max sizes the
hardware supports.

> #define STATX_DIRECTIO	0x00001000U	/* Want/got directio geometry */
> 
> How about that?

Mostly seems reasonable at a first look.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com


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

  parent reply	other threads:[~2022-01-23 23:03 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
2022-01-21  7:12                   ` [f2fs-dev] " Christoph Hellwig
2022-01-23 23:03                 ` Dave Chinner [this message]
2022-01-23 23:03                   ` 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=20220123230332.GL59729@dread.disaster.area \
    --to=david@fromorbit.com \
    --cc=chao@kernel.org \
    --cc=djwong@kernel.org \
    --cc=ebiggers@kernel.org \
    --cc=hch@infradead.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.