Re: bvec_iter.bi_sector -> loff_t?

Hongbo Li
2024-06-21T03:13:36Z

On 2024/6/20 22:49, Matthew Wilcox wrote:
> On Thu, Jun 20, 2024 at 10:16:02AM -0400, Kent Overstreet wrote:
>> That's really just descriptive, not prescriptive.
>>
>> The intent of O_DIRECT is "bypass the page cache", the alignment
>> restrictions are just a side effect of that. Applications just care
>> about is having predictable performance characteristics.
>
> But any application that has been written to use O_DIRECT already has the
> alignment & size guarantees in place. What this patch is attempting to do
> is make it "more friendly" to use, and I'm not sure that's a great idea.
> Not without buy-in from a large cross-section of filesystem people.

Indeed, the purpose of O_DIRECT is to bypass the page cache. Either the
file system can handle the unaligned offset or memory, or it should
directly return EINVAL (or ENOTSUP for file cannot be read by direct
I/O?). But I have observed that some file systems have
this fallback logic (in ext4: if `ext4_should_use_dio` not true, it will
fallback to buffer I/O. in f2fs: if `f2fs_should_use_dio` not true, it
will fallback to buffer I/O.). Does O_DIRECT flag need prescriptive
definition to standardize I/O behavior?

Thanks,
Hongbo
>
> I'm more sympathetic to "lets relax the alignment requirements", since
> most IO devices actually can do IO to arbitrary boundaries (or at least
> reasonable boundaries, eg cacheline alignment or 4-byte alignment).
> The 512 byte alignment doesn't seem particularly rooted in any hardware
> restrictions.
>
> But size? Fundamentally, we're asking the device to do IO directly to
> this userspace address. That means you get to do the entire IO, not
> just the part of it that you want. I know some devices have bitbucket
> descriptors, but many don't.
>
>>> I'm against it. Block devices only do sector-aligned IO and we should
>>> not pretend otherwise.
>>
>> Eh?
>>
>> bio isn't really specific to the block layer anyways, given that an
>> iov_iter can be a bio underneath. We _really_ should be trying for
>> better commonality of data structures.
>
> bio is absolutely specific to the block layer. Look at it:
>
> /*
> * main unit of I/O for the block layer and lower layers (ie drivers and
> * stacking drivers)
> */
>
> struct block_device *bi_bdev;
> unsigned short bi_flags; /* BIO_* below */
> unsigned short bi_ioprio;
> blk_status_t bi_status;
>
> Filesystems get to use it to interact with the block layer. The iov_iter
> isn't an abstraction over the bio, it's an abstraction over the bio_vec.
>