On Wed, Apr 28, 2021 at 10:58:44AM -0600, Jens Axboe wrote:

On 4/28/21 10:50 AM, Matthew Wilcox wrote:

quoted

bio_add_page() has its arguments in the wrong order:

extern int bio_add_page(struct bio *, struct page *, unsigned int,unsigned int);

Oh, right, and the prototype commits the cardinal sin of just giving you
a pair of unsigned ints and doesn't bother to tell you what they mean.
I'll send a patch for that ... anyway:

int bio_add_page(struct bio *bio, struct page *page,
                 unsigned int len, unsigned int offset)

This fails to follow #4: https://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html

Here's what I want to do for the folio equivalent:

size_t bio_add_folio(struct bio *bio, struct folio *folio, size_t off,
                size_t len)

This will make the transition more painful, but it does remove an irritant
for the future.  Any objections?

What's the point in shuffling len and offset around?

That almost everything else does (off, len).  Grepping include, here's
the (object, len, off) examples I found (will miss multi-line examples,
tried to use my best judgement, tried to exclude loff_t):

bio_add_zone_append_page
__bio_try_merge_page
__bio_add_page
bio_integrity_add_page
fscrypt_encrypt_block_inplace
fscrypt_decrypt_block_inplace
sg_set_page
sg_copy_buffer
sg_pcopy_from_buffer
sg_pcopy_to_buffer
sg_zero_buffer
copy_linear_skb

versus (object, off, len):

bpf_ctx_copy_t
->is_partially_uptodate
memcpy_from_page
memcpy_to_page
memzero_page
(basically all of the iomap functions)
kvm_write_guest_page
kvm_vcpu_write_guest_page
skb_gro_remcsum_process
nf_checksum_partial
perf_copy_f
read_module_eeprom
skb_frag_foreach_page
skb_to_sgvec_nomark
skb_to_sgvec
skb_send_sock
various skb_checksum_ops
sk_msg_clone
various gss_krb functions
xdr_process_buf
usercopy_warn
various network getfrag functions

Not _quite_ as one-sided as I thought, but there are basically three
families of functions (bio, fscrypt, sg) that are len-first (plus
copy_linear_skb() is out-of-family), then networking, filesystems,
highmem, perf, bpf, kvm are off-first.

So, you're the maintainer, it's your call ... do you want bio_add_folio()
to resemble the bio_add_page() calls as much as possible, or do you want
to migrate towards how the rest of the world works?