On Tue, Mar 10, 2020 at 05:30:24PM +0100, Christoph Hellwig wrote:
> On Mon, Mar 09, 2020 at 02:32:28PM -0700, Omar Sandoval wrote:
> > +static void btrfs_submit_direct_hook(struct btrfs_dio_private *dip)
>
> Just curious: why is this routine called btrfs_submit_direct_hook?
No idea. The name goes back to commit e65e15355429 ("btrfs: fix panic
caused by direct IO"), and it didn't make any sense then, either.
> it doesn't seem to hook up anything, but just contains the guts of
> btrfs_submit_direct. Any reason to keep the two functions separate?
The only reason I didn't combine them is that it would make for a pretty
big, unwieldy function. Maybe folding btrfs_submit_direct_hook() into
btrfs_submit_direct() and splitting the dip setup into something like
btrfs_create_dio_private() would be more natural.
> Also instead of the separate bip allocation and the bio clone, why
> not clone into a private bio_set that contains the private data
> as part of the allocation?
Mainly because dip is not tied to any one bio, as we might need to split
up or retry bios. We also already clone the bios from a btrfs bioset,
although that doesn't have everything we need for direct I/O.
Thanks!
