- 08 Dec, 2020 40 commits
-
-
David Sterba authored
After the rwsem replaced the tree lock implementation, the extent buffer got smaller but leaving some holes behind. By changing log_index type and reordering, we can squeeze the size further to 240 bytes, measured on release config on x86_64. Log_index spans only 3 values and needs to be signed. Before: struct extent_buffer { u64 start; /* 0 8 */ long unsigned int len; /* 8 8 */ long unsigned int bflags; /* 16 8 */ struct btrfs_fs_info * fs_info; /* 24 8 */ spinlock_t refs_lock; /* 32 4 */ atomic_t refs; /* 36 4 */ atomic_t io_pages; /* 40 4 */ int read_mirror; /* 44 4 */ struct callback_head callback_head __attribute__((__aligned__(8))); /* 48 16 */ /* --- cacheline 1 boundary (64 bytes) --- */ pid_t lock_owner; /* 64 4 */ bool lock_recursed; /* 68 1 */ /* XXX 3 bytes hole, try to pack */ struct rw_semaphore lock; /* 72 40 */ short int log_index; /* 112 2 */ /* XXX 6 bytes hole, try to pack */ struct page * pages[16]; /* 120 128 */ /* size: 248, cachelines: 4, members: 14 */ /* sum members: 239, holes: 2, sum holes: 9 */ /* forced alignments: 1 */ /* last cacheline: 56 bytes */ } __attribute__((__aligned__(8))); After: struct extent_buffer { u64 start; /* 0 8 */ long unsigned int len; /* 8 8 */ long unsigned int bflags; /* 16 8 */ struct btrfs_fs_info * fs_info; /* 24 8 */ spinlock_t refs_lock; /* 32 4 */ atomic_t refs; /* 36 4 */ atomic_t io_pages; /* 40 4 */ int read_mirror; /* 44 4 */ struct callback_head callback_head __attribute__((__aligned__(8))); /* 48 16 */ /* --- cacheline 1 boundary (64 bytes) --- */ pid_t lock_owner; /* 64 4 */ bool lock_recursed; /* 68 1 */ s8 log_index; /* 69 1 */ /* XXX 2 bytes hole, try to pack */ struct rw_semaphore lock; /* 72 40 */ struct page * pages[16]; /* 112 128 */ /* size: 240, cachelines: 4, members: 14 */ /* sum members: 238, holes: 1, sum holes: 2 */ /* forced alignments: 1 */ /* last cacheline: 48 bytes */ } __attribute__((__aligned__(8))); Reviewed-by:Josef Bacik <josef@toxicpanda.com> Signed-off-by:
David Sterba <dsterba@suse.com>
-
Josef Bacik authored
We no longer distinguish between blocking and spinning, so rip out all this code. Signed-off-by:
-
Josef Bacik authored
Now that we're using a rw_semaphore we no longer need to indicate if a lock is blocking or not, nor do we need to flip the entire path from blocking to spinning. Remove these helpers and all the places they are called. Signed-off-by:
-
David Sterba authored
The context structure unnecessarily stores copy of the checksum size, that can be now easily obtained from fs_info. Reviewed-by:
Johannes Thumshirn <johannes.thumshirn@wdc.com> Signed-off-by:
-
David Sterba authored
The state structure unnecessarily stores copy of the checksum size, that can be now easily obtained from fs_info. Reviewed-by:
-
David Sterba authored
Remove local variable that is then used just once and replace it with fs_info::csum_size. Reviewed-by:
-
David Sterba authored
The fs_info value is 32bit, switch also the local u16 variables. This leads to a better assembly code generated due to movzwl. This simple change will shave some bytes on x86_64 and release config: text data bss dec hex filename 1090000 17980 14912 1122892 11224c pre/btrfs.ko 1089794 17980 14912 1122686 11217e post/btrfs.ko DELTA: -206 Reviewed-by:
-
David Sterba authored
btrfs_get_16 shows up in the system performance profiles (helper to read 16bit values from on-disk structures). This is partially because of the checksum size that's frequently read along with data reads/writes, other u16 uses are from item size or directory entries. Replace all calls to btrfs_super_csum_size by the cached value from fs_info. Reviewed-by:
Qu Wenruo <wqu@suse.com> Signed-off-by:
-
David Sterba authored
btrfs_csum_bytes_to_leaves shows up in system profiles, which makes it a candidate for optimizations. After the 64bit division has been replaced by shift, there's still a calculation done each time the function is called: checksums per leaf. As this is a constant value for the entire filesystem lifetime, we can calculate it once at mount time and reuse. This also allows to reduce the division to 64bit/32bit as we know the constant will always fit the 32bit type. Replace the open-coded rounding up with a macro that internally handles the 64bit division and as it's now a short function, make it static inline (slight code increase, slight stack usage reduction). Reviewed-by:
-
David Sterba authored
In many places we need the checksum size and it is inefficient to read it from the raw superblock. Store the value into fs_info, actual use will be in followup patches. The size is u32 as it allows to generate better assembly than with u16. Reviewed-by:
-
David Sterba authored
The value of super_block::s_blocksize_bits is the same as fs_info::sectorsize_bits, but we don't need to do the extra dereferences in many functions and storing the bits as u32 (in fs_info) generates shorter assembly. Reviewed-by:
-
David Sterba authored
Change free_space_bitmap_size to take btrfs_fs_info so we can get the sectorsize_bits to do calculations. Reviewed-by:
-
David Sterba authored
We do a lot of calculations where we divide or multiply by sectorsize. We also know and make sure that sectorsize is a power of two, so this means all divisions can be turned to shifts and avoid eg. expensive u64/u32 divisions. The type is u32 as it's more register friendly on x86_64 compared to u8 and the resulting assembly is smaller (movzbl vs movl). There's also superblock s_blocksize_bits but it's usually one more pointer dereference farther than fs_info. Reviewed-by:
-
Qu Wenruo authored
The variable @page_size in submit_extent_page() is not related to page size. It can already be smaller than PAGE_SIZE, so rename it to io_size to reduce confusion, this is especially important for later subpage support. Signed-off-by:
-
Qu Wenruo authored
If we're reading partial page, btrfs will warn about this as read/write is always done in sector size, which now equals page size. But for the upcoming subpage read-only support, our data read is only aligned to sectorsize, which can be smaller than page size. Thus here we change the warning condition to check it against sectorsize, the behavior is not changed for regular sectorsize == PAGE_SIZE case, and won't report error for subpage read. Also, pass the proper start/end with bv_offset for check_data_csum() to handle. Signed-off-by:
-
Qu Wenruo authored
Function process_pages_contig() does not only handle page locking but also other operations. Rename the local variable pages_locked to pages_processed to reduce confusion. Signed-off-by:
-
Qu Wenruo authored
For check_data_csum(), the page we're using is directly from the inode mapping, thus it has valid page_offset(). We can use (page_offset() + pg_off) to replace @start parameter completely, while the @len should always be sectorsize. Since we're here, also add some comment, as there are quite some confusion in words like start/offset, without explaining whether it's file_offset or logical bytenr. This should not affect the existing behavior, as for current sectorsize == PAGE_SIZE case, @pgoff should always be 0, and len is always PAGE_SIZE (or sectorsize from the dio read path). Reviewed-by:
Goldwyn Rodrigues <rgoldwyn@suse.com> Signed-off-by:
-
Qu Wenruo authored
All callers of btrfs_wq_submit_bio() pass struct inode as @private_data, so there is no need for it to be (void *), replace it with "struct inode *inode". While we can extract fs_info from struct inode, also remove the @fs_info parameter. Since we're here, also replace all the (void *private_data) into (struct inode *inode). Reviewed-by:
-
Qu Wenruo authored
The @failed_start parameter is only paired with @exclusive_bits, and those parameters are only used for EXTENT_LOCKED bit, which have their own wrappers lock_extent_bits(). Thus for regular set_extent_bit() calls, the failed_start makes no sense, just sink the parameter. Also, since @failed_start and @exclusive_bits are used in pairs, add an assert to make it obvious. Signed-off-by:
-
Qu Wenruo authored
The pitfall here is, if the parameter @bits has multiple bits set, we will return the first range which just has one of the specified bits set. This is a little tricky if we want an exact match. Anyway, update the comment to make that clear. Signed-off-by:
-
Qu Wenruo authored
The return value of that function is completely wrong. That function only returns 0 if the extent buffer doesn't need to be submitted. The "ret = 1" and "ret = 0" are determined by the return value of "test_and_clear_bit(EXTENT_BUFFER_DIRTY, &eb->bflags)". And if we get ret == 1, it's because the extent buffer is dirty, and we set its status to EXTENT_BUFFER_WRITE_BACK, and continue to page locking. While if we get ret == 0, it means the extent is not dirty from the beginning, so we don't need to write it back. The caller also follows this, in btree_write_cache_pages(), if lock_extent_buffer_for_io() returns 0, we just skip the extent buffer completely. So the comment is completely wrong. Since we're here, also change the description a little. The write bio flushing won't be visible to the caller, thus it's not an major feature. In the main description, only describe the locking part to make the point more clear. For reference, added in commit 2e3c2513 ("btrfs: extent_io: add proper error handling to lock_extent_buffer_for_io()") Signed-off-by:
-
David Sterba authored
Long time ago the explicit casts were necessary for u64 but we don't need it. Remove casts where the type matches, leaving only cases that cast sector_t or loff_t. Reviewed-by:
-
David Sterba authored
The drop_level member is used directly unlike all the other int types in root_item. Add the definition and use it everywhere. The type is u8 so there's no conversion necessary and the helpers are properly inlined, this is for consistency. Reviewed-by:
-
David Sterba authored
For consistency use the available helpers to set flags and limit. Reviewed-by:
-
David Sterba authored
There's one raw use of le->cpu conversion but we have a helper to do that for us, so use it. Reviewed-by:
-
David Sterba authored
We have helpers to access the on-disk item members, use that for root_item::ctransid instead of raw le64_to_cpu. Reviewed-by:
-
David Sterba authored
The names in btrfs_lockdep_keysets are generated from a simple pattern using snprintf but we can generate them directly with some macro magic and remove the helpers. Reviewed-by:
-
David Sterba authored
BTRFS_MAX_LEVEL is 8 and the keyset table is supposed to have a key for each level, but we'll never have more than 8 levels. The values passed to btrfs_set_buffer_lockdep_class are always derived from a valid extent buffer. Set the array sizes to the right value. Reviewed-by:
-
Goldwyn Rodrigues authored
This effectively reverts 09745ff88d93 ("btrfs: dio iomap DSYNC workaround") now that the iomap API has been updated to allow iomap_dio_complete() not to be called under i_rwsem anymore. Reviewed-by: -
Goldwyn Rodrigues authored
If direct writes are called with O_DIRECT | O_DSYNC, it will result in a deadlock because iomap_dio_rw() is called under i_rwsem which calls: iomap_dio_complete() generic_write_sync() btrfs_sync_file() btrfs_sync_file() requires i_rwsem, so call __iomap_dio_rw() with the i_rwsem locked, and call iomap_dio_complete() after unlocking i_rwsem. Reviewed-by: -
Goldwyn Rodrigues authored
The inode dio_sem can be eliminated because all DIO synchronization is now performed through inode->i_rwsem that provides the same guarantees. This reduces btrfs_inode size by 40 bytes. Reviewed-by:
-
Goldwyn Rodrigues authored
Direct writes within EOF are safe to be performed with inode shared lock to improve parallelization with other direct writes or reads because EOF is not changed and there is no race with truncate(). Direct reads are already performed under shared inode lock. This patch is precursor to removing btrfs_inode->dio_sem. Signed-off-by:
-
Goldwyn Rodrigues authored
Push inode locking and unlocking closer to where we perform the I/O. For this we need to move the write checks inside the respective functions as well. pos is evaluated after generic_write_checks because O_APPEND can change iocb->ki_pos. Reviewed-by:
-
Goldwyn Rodrigues authored
btrfs_inode_lock/unlock() are wrappers around inode locks, separating the type of lock and actual locking. - 0 - default, exclusive lock - BTRFS_ILOCK_SHARED - for shared locks, for possible parallel DIO - BTRFS_ILOCK_TRY - for the RWF_NOWAIT sequence The bits SHARED and TRY can be combined together. Reviewed-by:
-
Goldwyn Rodrigues authored
btrfs_write_check() checks write parameters in one place before beginning a write. This does away with inode_unlock() after every check. In the later patches, it will help push inode_lock/unlock() in buffered and direct write functions respectively. generic_write_checks needs to be called before as it could truncate iov_iter and its return used as count. Signed-off-by:
-
Goldwyn Rodrigues authored
fs_info::fs_state is a filesystem bit check as opposed to inode and can be performed before we begin with write checks. This eliminates inode lock/unlock in case the error bit is set. Reviewed-by:
-
Goldwyn Rodrigues authored
While we do this, correct the call to pagecache_isize_extended: - pagecache_isize_extended needs to be called to the start of the write as opposed to i_size - we don't need to check range before the call, this is done in the function Reviewed-by:
Christoph Hellwig <hch@lst.de> Reviewed-by:
-
Goldwyn Rodrigues authored
The read and write DIO don't have anything in common except for the call to iomap_dio_rw. Extract the write call into a new function to get rid of conditional statements for direct write. Reviewed-by:
-
Anand Jain authored
Add /sys/fs/btrfs/UUID/read_policy attribute so that the read policy for the raid1, raid1c34 and raid10 can be tuned. When this attribute is read, it will show all available policies, with active policy in [ ]. The read_policy attribute can be written using one of the items listed in there. For example: $ cat /sys/fs/btrfs/UUID/read_policy [pid] $ echo pid > /sys/fs/btrfs/UUID/read_policy Reviewed-by:
Anand Jain <anand.jain@oracle.com> Reviewed-by:
-
Anand Jain authored
As of now, we use the pid method to read striped mirrored data, which means process id determines the stripe id to read. This type of routing typically helps in a system with many small independent processes tying to read random data. On the other hand, the pid based read IO policy is inefficient because if there is a single process trying to read a large file, the overall disk bandwidth remains underutilized. So this patch introduces a read policy framework so that we could add more read policies, such as IO routing based on the device's wait-queue or manual when we have a read-preferred device or a policy based on the target storage caching. Reviewed-by:
-
