Btrfs: add basic DIO read/write support V3

V1->V2
-Use __blockdev_direct_IO instead of helper
-Use KM_IRQ0 for kmap instead of KM_USER0

V2->V3
-Update the submit function to work with my submit hook changes -Add DIO write support

This provides basic DIO support for reads only. It does not do any of the work to recover from mismatching checksums, that will come later. A few design changes have been made from Jim's code (sorry Jim!)

1) Use the generic direct-io code. Jim originally re-wrote all the generic DIO code in order to account for all of BTRFS's oddities, but thanks to that work it seems like the best bet is to just ignore compression and such and just opt to fallback on buffered IO.

2) Fallback on buffered IO for compressed or inline extents. Jim's code did it's own buffering to make dio with compressed extents work. Now we just fallback onto normal buffered IO.

3) Lock the entire range during DIO. I originally had it so we would lock the extents as get_block was called, and then unlock them as the endio function was called, which worked great, but if we ever had an error in the submit_io hook, we could have locked an extent that would never be submitted for IO, so we wouldn't be able to unlock it, so this solution fixed that problem and made it a bit cleaner.

I've tested this with fsx and everything works great. This patch depends on my dio and filemap.c patches to work. Thanks,

---
 fs/btrfs/ctree.h     |    7 +
 fs/btrfs/file-item.c |  258 +++++++++++++++++++++++++++++++-
 fs/btrfs/file.c      |   69 ++++++++-
 fs/btrfs/inode.c     |  414 +++++++++++++++++++++++++++++++++++++++++++++++---
 4 files changed, 716 insertions(+), 32 deletions(-)

index 746a724..8f0c202 100644
@@ -2257,6 +2257,8 @@ int btrfs_del_csums(struct btrfs_trans_handle *trans,
 		    struct btrfs_root *root, u64 bytenr, u64 len);
 int btrfs_lookup_bio_sums(struct btrfs_root *root, struct inode *inode,
 			  struct bio *bio, u32 *dst);
+int btrfs_lookup_bio_sums_dio(struct btrfs_root *root, struct inode *inode,
+			      struct bio *bio, u64 logical_offset, u32 *dst);
 int btrfs_insert_file_extent(struct btrfs_trans_handle *trans,
 			     struct btrfs_root *root,
 			     u64 objectid, u64 pos,

@@ -2270,8 +2272,13 @@ int btrfs_lookup_file_extent(struct btrfs_trans_handle *trans,
 int btrfs_csum_file_blocks(struct btrfs_trans_handle *trans,
 			   struct btrfs_root *root,
 			   struct btrfs_ordered_sum *sums);
+int btrfs_csum_file_blocks_dio(struct btrfs_trans_handle *trans,
+			       struct btrfs_root *root, u64 disk_bytenr,
+			       u64 bytes, u32 *csums);
 int btrfs_csum_one_bio(struct btrfs_root *root, struct inode *inode,
 		       struct bio *bio, u64 file_start, int contig);
+int btrfs_csum_one_bio_dio(struct btrfs_root *root, struct inode *inode,
+			   struct bio *bio, u32 *csums);
 int btrfs_csum_file_bytes(struct btrfs_root *root, struct inode *inode,
 			  u64 start, unsigned long len);
 struct btrfs_csum_item *btrfs_lookup_csum(struct btrfs_trans_handle *trans,

1.6.6.1

AIO's aio_complete does kmap with KM_IRQ0/1 and it gets called in the same context as the btrfs completion handler, so if it's ok for aio_complete it should be ok for btrfs right? Thanks,

Josef

Using KM_USERx from irq context is a horrid, horrid bug.

Using KM_IRQx with local interrupts enabled is a ditto.

It's OK to use KM_IRQx from process context as long as local interrupts are disabled.

kmap slots are just per-cpu variables. Each one has a particular usage
convention: process-context-only, must-be-irq-safe, etc.

aio_complete does a spin_lock_irqsave before that, which disables interrupts on the local CPU.

Ok so looking at debug_kmap_atomic it seems that KM_IRQ0 is ok to use in all cases, in_irq(), !irqs_disabled() and !in_interrupt(), so I should be safe. If you still don't like it, would something like this be acceptable?

if (in_interrupt() || irqs_disabled()) {
	if (in_irq())
		type = KM_IRQ0;
	else if (!irqs_disabled())
		type = KM_SOFTIRQ0;
	else
		type = KM_IRQ0;
} else {
	type = KM_USER0;
}

Hrm, you'd think somebody would have had this problem before and already come up with a nice helper, oh well. Thanks,

Josef

Ok how about I just do

local_irq_disable()
kmap(KM_IRQ0)
local_irq_enable()

would that be acceptable? Thanks,

Josef

yup.

local_irq_disable() (or local_irq_save())
kmap_atomic(KM_IRQx);
<stuff>
kunmap_atomic(KM_IRQx);
local_irq_enable() (or local_irq_restore()).

then perhaps flush_dcache_page().

Great, thank you, I will fix this up and resend.

Josef