Re: [PATCH] exportfs: be careful to only return expected errors.
On Thu, Aug 04 2016, NeilBrown wrote: > > > When nfsd calls fh_to_dentry, it expect ESTALE or ENOMEM as errors. > In particular it can be tempting to return ENOENT, but this is not > handled well by nfsd. > > Rather than requiring strict adherence to error code code filesystems, > treat all unexpected error codes the same as ESTALE. This is safest. > > Signed-off-by: NeilBrown > --- > > I didn't add a dprintk for unexpected error messages, partly > because dprintk isn't usable in exportfs. I could have used pr_debug() > but I really didn't see much value. > > This has been tested together with the btrfs change, and it restores > correct functionality. > > Thanks, > NeilBrown Hi Bruce, I notice that this patch isn't in -next, but the btrfs change which motivated it is. That means, unless there is some other work around somewhere, btrfs might start having problems with nfs export. Can we get this patch into 4.9-rc?? Or has someone fixed it a different way? Thanks, NeilBrown > > fs/exportfs/expfs.c | 10 ++ > include/linux/exportfs.h | 13 +++-- > 2 files changed, 13 insertions(+), 10 deletions(-) > > diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c > index 207ba8d627ca..a4b531be9168 100644 > --- a/fs/exportfs/expfs.c > +++ b/fs/exportfs/expfs.c > @@ -428,10 +428,10 @@ struct dentry *exportfs_decode_fh(struct vfsmount *mnt, > struct fid *fid, > if (!nop || !nop->fh_to_dentry) > return ERR_PTR(-ESTALE); > result = nop->fh_to_dentry(mnt->mnt_sb, fid, fh_len, fileid_type); > - if (!result) > - result = ERR_PTR(-ESTALE); > - if (IS_ERR(result)) > - return result; > + if (PTR_ERR(result) == -ENOMEM) > + return ERR_CAST(result); > + if (IS_ERR_OR_NULL(result)) > + return ERR_PTR(-ESTALE); > > if (d_is_dir(result)) { > /* > @@ -541,6 +541,8 @@ struct dentry *exportfs_decode_fh(struct vfsmount *mnt, > struct fid *fid, > > err_result: > dput(result); > + if (err != -ENOMEM) > + err = -ESTALE; > return ERR_PTR(err); > } > EXPORT_SYMBOL_GPL(exportfs_decode_fh); > diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h > index b03c0625fa6e..5ab958cdc50b 100644 > --- a/include/linux/exportfs.h > +++ b/include/linux/exportfs.h > @@ -157,12 +157,13 @@ struct fid { > *@fh_to_dentry is given a &struct super_block (@sb) and a file handle > *fragment (@fh, @fh_len). It should return a &struct dentry which refers > *to the same file that the file handle fragment refers to. If it > cannot, > - *it should return a %NULL pointer if the file was found but no > acceptable > - *&dentries were available, or an %ERR_PTR error code indicating why it > - *couldn't be found (e.g. %ENOENT or %ENOMEM). Any suitable dentry can > be > - *returned including, if necessary, a new dentry created with > d_alloc_root. > - *The caller can then find any other extant dentries by following the > - *d_alias links. > + *it should return a %NULL pointer if the file cannot be found, or an > + *%ERR_PTR error code of %ENOMEM if a memory allocation failure occurred. > + *Any other error code is treated like %NULL, and will cause an %ESTALE > error > + *for callers of exportfs_decode_fh(). > + *Any suitable dentry can be returned including, if necessary, a new > dentry > + *created with d_alloc_root. The caller can then find any other extant > + *dentries by following the d_alias links. > * > * fh_to_parent: > *Same as @fh_to_dentry, except that it returns a pointer to the parent > -- > 2.9.2 signature.asc Description: PGP signature
Re: [PATCH 2/2] btrfs: fix false enospc for compression
Hi, On 10/06/2016 10:51 AM, Wang Xiaoguang wrote: When testing btrfs compression, sometimes we got ENOSPC error, though fs still has much free space, xfstests generic/171, generic/172, generic/173, generic/174, generic/175 can reveal this bug in my test environment when compression is enabled. Sorry, here xfstests generic/171, generic/172, generic/173, generic/174, generic/175 have some modifications by me, in original codes, they use "nr_free=$(stat -f -c '%f' $testdir)" to count fs free space and then write the nr_free blocks, but if fs has compression enabled, this operation will not fill fs, so I use "dd if=/dev/zero of=testfile" to fill fs, just this modifications :) Also you can execute below steps to check: # sudo mkfs.btrfs -f -n 64K -b 320M /dev/sdc6 # sudo mount -o compress-force=lzo /dev/sdc6 mntpoint/ # cd mntpoint # sudo dd if=/dev/zero of=testfile In my test environment, it'll get enospc quickly, but fs is not full. My virtual machine settings: [lege@localhost mntpoint]$ free -m totalusedfree shared buff/cache available Mem: 1949 2211141 21 5871597 Swap: 1019 71012 [lege@localhost mntpoint]$ lscpu Architecture: x86_64 CPU op-mode(s):32-bit, 64-bit Byte Order:Little Endian CPU(s):4 On-line CPU(s) list: 0-3 Thread(s) per core:1 Core(s) per socket:1 Socket(s): 4 NUMA node(s): 1 Vendor ID: GenuineIntel CPU family:6 Model: 42 Model name:Intel Xeon E312xx (Sandy Bridge) Stepping: 1 CPU MHz: 3192.816 BogoMIPS: 6445.42 Hypervisor vendor: KVM Virtualization type: full L1d cache: 32K L1i cache: 32K L2 cache: 4096K NUMA node0 CPU(s): 0-3 Regards, Xiaoguang Wang After some debuging work, we found that it's btrfs_delalloc_reserve_metadata() which sometimes tries to reserve plenty of metadata space, even for very small data range. In btrfs_delalloc_reserve_metadata(), the number of metadata bytes we try to reserve is calculated by the difference between outstanding_extents and reserved_extents. Please see below case for how ENOSPC occurs: 1, Buffered write 128MB data in unit of 128KB, so finially we'll have inode outstanding extents be 1, and reserved_extents be 1024. Note it's btrfs_merge_extent_hook() that merges these 128KB units into one big outstanding extent, but do not change reserved_extents. 2, When writing dirty pages, for compression, cow_file_range_async() will split above big extent in unit of 128KB(compression extent size is 128KB). When first split opeartion finishes, we'll have 2 outstanding extents and 1024 reserved extents, and just right now the currently generated ordered extent is dispatched to run and complete, then btrfs_delalloc_release_metadata()(see btrfs_finish_ordered_io()) will be called to release metadata, after that we will have 1 outstanding extents and 1 reserved extents(also see logic in drop_outstanding_extent()). Later cow_file_range_async() continues to handles left data range[128KB, 128MB), and if no other ordered extent was dispatched to run, there will be 1023 outstanding extents and 1 reserved extent. 3, Now if another bufferd write for this file enters, then btrfs_delalloc_reserve_metadata() will at least try to reserve metadata for 1023 outstanding extents' metadata, for 16KB node size, it'll be 1023*16384*2*8, about 255MB, for 64K node size, it'll be 1023*65536*8*2, about 1GB metadata, so obviously it's not sane and can easily result in enospc error. The root cause is that for compression, its max extent size will no longer be BTRFS_MAX_EXTENT_SIZE(128MB), it'll be 128KB, so current metadata reservation method in btrfs is not appropriate or correct, here we introduce: enum btrfs_metadata_reserve_type { BTRFS_RESERVE_NORMAL, BTRFS_RESERVE_COMPRESS, }; and expand btrfs_delalloc_reserve_metadata() and btrfs_delalloc_reserve_space() by adding a new enum btrfs_metadata_reserve_type argument. When a data range will go through compression, we use BTRFS_RESERVE_COMPRESS to reserve metatata. Meanwhile we introduce EXTENT_COMPRESS flag to mark a data range that will go through compression path. With this patch, we can fix these false enospc error for compression. Signed-off-by: Wang Xiaoguang --- fs/btrfs/ctree.h | 31 ++-- fs/btrfs/extent-tree.c | 55 + fs/btrfs/extent_io.c | 59 +- fs/btrfs/extent_io.h | 2 + fs/btrfs/file.c | 26 +-- fs/btrfs/free-space-cache.c | 6 +- fs/btrfs/inode-map.c | 5 +- fs/btrfs/inode.c | 181 --- fs/btrfs/ioctl.c | 12 ++- fs/btrfs/relocation.c| 14 +++- fs/btrfs/t
Re: BTRFS: space_info 4 has 18446742286429913088 free, is not full
Hi, On 09/29/2016 03:27 PM, Stefan Priebe - Profihost AG wrote: Am 29.09.2016 um 09:13 schrieb Wang Xiaoguang: I found that compress sometime report ENOSPC error even in 4.8-rc8, currently I cannot confirm that as i do not have anough space to test this without compression ;-( But yes i've compression enabled. I might not get you, my poor english :) You mean that you only get ENOSPC error when compression is enabled? And when compression is not enabled, you do not get ENOSPC error? I can't tell you. I cannot test with compression not enabled. I do not have anough free space on this disk. I had just sent two patches to fix false enospc error for compression, please have a try, they fix false enospc error in my test environment. btrfs: fix false enospc for compression btrfs: improve inode's outstanding_extents computation I apply these two patchs in linux upstream tree, the latest commit is 41844e36206be90cd4d962ea49b0abc3612a99d0. Regards, Xiaoguang Wang I'm trying to fix it. That sounds good but do you also get the BTRFS: space_info 4 has 18446742286429913088 free, is not full kernel messages on umount? if not you might have found another problem. Yes, I seem similar messages, you can paste you whole dmesg info here. [ cut here ] WARNING: CPU: 2 PID: 5187 at fs/btrfs/extent-tree.c:5790 btrfs_free_block_groups+0x346/0x430 [btrfs]() Modules linked in: netconsole xt_multiport iptable_filter ip_tables x_tables 8021q garp bonding x86_pkg_temp_thermal coretemp kvm_intel kvm irqbypass sb_edac crc32_pclmul edac_core i2c_i801 i40e(O) vxlan ip6_udp_tunnel udp_tunnel shpchp ipmi_si ipmi_msghandler button loop btrfs dm_mod raid10 raid0 multipath linear raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor raid6_pq igb i2c_algo_bit i2c_core usbhid raid1 md_mod xhci_pci sg ehci_pci xhci_hcd ehci_hcd sd_mod ahci usbcore ptp libahci usb_common pps_core aacraid CPU: 2 PID: 5187 Comm: umount Tainted: G O 4.4.22+63-ph #1 Hardware name: Supermicro Super Server/X10SRi-F, BIOS 2.0 12/17/2015 880fda777d00 813b69c3 c067a099 880fda777d38 810821c6 880074bf0a00 88103c10c088 88103c10c000 88103c10c098 Call Trace: [] dump_stack+0x63/0x90 [] warn_slowpath_common+0x86/0xc0 [] warn_slowpath_null+0x1a/0x20 [] btrfs_free_block_groups+0x346/0x430 [btrfs] [] close_ctree+0x15d/0x330 [btrfs] [] btrfs_put_super+0x19/0x20 [btrfs] [] generic_shutdown_super+0x6f/0x100 [] kill_anon_super+0x12/0x20 [] btrfs_kill_super+0x16/0xa0 [btrfs] [] deactivate_locked_super+0x43/0x70 [] deactivate_super+0x5c/0x60 [] cleanup_mnt+0x3f/0x90 [] __cleanup_mnt+0x12/0x20 [] task_work_run+0x81/0xa0 [] exit_to_usermode_loop+0xb0/0xc0 [] syscall_return_slowpath+0xd4/0x130 [] int_ret_from_sys_call+0x25/0x8f ---[ end trace cee6ace13018e13e ]--- [ cut here ] WARNING: CPU: 2 PID: 5187 at fs/btrfs/extent-tree.c:5791 btrfs_free_block_groups+0x365/0x430 [btrfs]() Modules linked in: netconsole xt_multiport iptable_filter ip_tables x_tables 8021q garp bonding x86_pkg_temp_thermal coretemp kvm_intel kvm irqbypass sb_edac crc32_pclmul edac_core i2c_i801 i40e(O) vxlan ip6_udp_tunnel udp_tunnel shpchp ipmi_si ipmi_msghandler button loop btrfs dm_mod raid10 raid0 multipath linear raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor raid6_pq igb i2c_algo_bit i2c_core usbhid raid1 md_mod xhci_pci sg ehci_pci xhci_hcd ehci_hcd sd_mod ahci usbcore ptp libahci usb_common pps_core aacraid CPU: 2 PID: 5187 Comm: umount Tainted: G W O 4.4.22+63-ph #1 Hardware name: Supermicro Super Server/X10SRi-F, BIOS 2.0 12/17/2015 880fda777d00 813b69c3 c067a099 880fda777d38 810821c6 880074bf0a00 88103c10c088 88103c10c000 88103c10c098 Call Trace: [] dump_stack+0x63/0x90 [] warn_slowpath_common+0x86/0xc0 [] warn_slowpath_null+0x1a/0x20 [] btrfs_free_block_groups+0x365/0x430 [btrfs] [] close_ctree+0x15d/0x330 [btrfs] [] btrfs_put_super+0x19/0x20 [btrfs] [] generic_shutdown_super+0x6f/0x100 [] kill_anon_super+0x12/0x20 [] btrfs_kill_super+0x16/0xa0 [btrfs] [] deactivate_locked_super+0x43/0x70 [] deactivate_super+0x5c/0x60 [] cleanup_mnt+0x3f/0x90 [] __cleanup_mnt+0x12/0x20 [] task_work_run+0x81/0xa0 [] exit_to_usermode_loop+0xb0/0xc0 [] syscall_return_slowpath+0xd4/0x130 [] int_ret_from_sys_call+0x25/0x8f ---[ end trace cee6ace13018e13f ]--- [ cut here ] WARNING: CPU: 2 PID: 5187 at fs/btrfs/extent-tree.c:10151 btrfs_free_block_groups+0x291/0x430 [btrfs]() Modules linked in: netconsole xt_multiport iptable_filter ip_tables x_tables 8021q garp bonding x86_pkg_temp_thermal coretemp kvm_intel kvm irqbypass sb_edac crc32_pclmul edac_core i2c_i801 i40e(O) vxlan ip6_udp_tunnel udp_tunnel shpchp ipmi_si ipmi_msghandler button loop btrfs dm_mod raid10 raid0 multipath linear raid456 as
[PATCH 2/2] btrfs: fix false enospc for compression
When testing btrfs compression, sometimes we got ENOSPC error, though fs still has much free space, xfstests generic/171, generic/172, generic/173, generic/174, generic/175 can reveal this bug in my test environment when compression is enabled. After some debuging work, we found that it's btrfs_delalloc_reserve_metadata() which sometimes tries to reserve plenty of metadata space, even for very small data range. In btrfs_delalloc_reserve_metadata(), the number of metadata bytes we try to reserve is calculated by the difference between outstanding_extents and reserved_extents. Please see below case for how ENOSPC occurs: 1, Buffered write 128MB data in unit of 128KB, so finially we'll have inode outstanding extents be 1, and reserved_extents be 1024. Note it's btrfs_merge_extent_hook() that merges these 128KB units into one big outstanding extent, but do not change reserved_extents. 2, When writing dirty pages, for compression, cow_file_range_async() will split above big extent in unit of 128KB(compression extent size is 128KB). When first split opeartion finishes, we'll have 2 outstanding extents and 1024 reserved extents, and just right now the currently generated ordered extent is dispatched to run and complete, then btrfs_delalloc_release_metadata()(see btrfs_finish_ordered_io()) will be called to release metadata, after that we will have 1 outstanding extents and 1 reserved extents(also see logic in drop_outstanding_extent()). Later cow_file_range_async() continues to handles left data range[128KB, 128MB), and if no other ordered extent was dispatched to run, there will be 1023 outstanding extents and 1 reserved extent. 3, Now if another bufferd write for this file enters, then btrfs_delalloc_reserve_metadata() will at least try to reserve metadata for 1023 outstanding extents' metadata, for 16KB node size, it'll be 1023*16384*2*8, about 255MB, for 64K node size, it'll be 1023*65536*8*2, about 1GB metadata, so obviously it's not sane and can easily result in enospc error. The root cause is that for compression, its max extent size will no longer be BTRFS_MAX_EXTENT_SIZE(128MB), it'll be 128KB, so current metadata reservation method in btrfs is not appropriate or correct, here we introduce: enum btrfs_metadata_reserve_type { BTRFS_RESERVE_NORMAL, BTRFS_RESERVE_COMPRESS, }; and expand btrfs_delalloc_reserve_metadata() and btrfs_delalloc_reserve_space() by adding a new enum btrfs_metadata_reserve_type argument. When a data range will go through compression, we use BTRFS_RESERVE_COMPRESS to reserve metatata. Meanwhile we introduce EXTENT_COMPRESS flag to mark a data range that will go through compression path. With this patch, we can fix these false enospc error for compression. Signed-off-by: Wang Xiaoguang --- fs/btrfs/ctree.h | 31 ++-- fs/btrfs/extent-tree.c | 55 + fs/btrfs/extent_io.c | 59 +- fs/btrfs/extent_io.h | 2 + fs/btrfs/file.c | 26 +-- fs/btrfs/free-space-cache.c | 6 +- fs/btrfs/inode-map.c | 5 +- fs/btrfs/inode.c | 181 --- fs/btrfs/ioctl.c | 12 ++- fs/btrfs/relocation.c| 14 +++- fs/btrfs/tests/inode-tests.c | 15 ++-- 11 files changed, 309 insertions(+), 97 deletions(-) diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index 16885f6..fa6a19a 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -97,6 +97,19 @@ static const int btrfs_csum_sizes[] = { 4 }; #define BTRFS_DIRTY_METADATA_THRESHSZ_32M +/* + * for compression, max file extent size would be limited to 128K, so when + * reserving metadata for such delalloc writes, pass BTRFS_RESERVE_COMPRESS to + * btrfs_delalloc_reserve_metadata() or btrfs_delalloc_reserve_space() to + * calculate metadata, for none-compression, use BTRFS_RESERVE_NORMAL. + */ +enum btrfs_metadata_reserve_type { + BTRFS_RESERVE_NORMAL, + BTRFS_RESERVE_COMPRESS, +}; +int inode_need_compress(struct inode *inode); +u64 btrfs_max_extent_size(enum btrfs_metadata_reserve_type reserve_type); + #define BTRFS_MAX_EXTENT_SIZE SZ_128M struct btrfs_mapping_tree { @@ -2677,10 +2690,14 @@ int btrfs_subvolume_reserve_metadata(struct btrfs_root *root, void btrfs_subvolume_release_metadata(struct btrfs_root *root, struct btrfs_block_rsv *rsv, u64 qgroup_reserved); -int btrfs_delalloc_reserve_metadata(struct inode *inode, u64 num_bytes); -void btrfs_delalloc_release_metadata(struct inode *inode, u64 num_bytes); -int btrfs_delalloc_reserve_space(struct inode *inode, u64 start, u64 len); -void btrfs_delalloc_release_space(struct inode *inode, u64 start, u64 len); +int btrfs_delalloc_reserve_metadata(struct inode *inode, u64 num_bytes, + enum btrfs_metadata_reserve_type reserve_type); +void btrfs_delalloc_release_metadata(struct i
[PATCH 1/2] btrfs: improve inode's outstanding_extents computation
This issue was revealed by modifying BTRFS_MAX_EXTENT_SIZE(128MB) to 64KB, When modifying BTRFS_MAX_EXTENT_SIZE(128MB) to 64KB, fsstress test often gets these warnings from btrfs_destroy_inode(): WARN_ON(BTRFS_I(inode)->outstanding_extents); WARN_ON(BTRFS_I(inode)->reserved_extents); Simple test program below can reproduce this issue steadily. Note: you need to modify BTRFS_MAX_EXTENT_SIZE to 64KB to have test, otherwise there won't be such WARNING. #include #include #include #include #include int main(void) { int fd; char buf[68 *1024]; memset(buf, 0, 68 * 1024); fd = open("testfile", O_CREAT | O_EXCL | O_RDWR); pwrite(fd, buf, 68 * 1024, 64 * 1024); return; } When BTRFS_MAX_EXTENT_SIZE is 64KB, and buffered data range is: 64KB128K132KB |---|---| 64 + 4KB 1) for above data range, btrfs_delalloc_reserve_metadata() will reserve metadata and set BTRFS_I(inode)->outstanding_extents to 2. (68KB + 64KB - 1) / 64KB == 2 Outstanding_extents: 2 2) then btrfs_dirty_page() will be called to dirty pages and set EXTENT_DELALLOC flag. In this case, btrfs_set_bit_hook() will be called twice. The 1st set_bit_hook() call will set DEALLOC flag for the first 64K. 64KB128KB |---| 64KB DELALLOC Outstanding_extents: 2 Set_bit_hooks() uses FIRST_DELALLOC flag to avoid re-increase outstanding_extents counter. So for 1st set_bit_hooks() call, it won't modify outstanding_extents, it's still 2. Then FIRST_DELALLOC flag is *CLEARED*. 3) 2nd btrfs_set_bit_hook() call. Because FIRST_DELALLOC have been cleared by previous set_bit_hook(), btrfs_set_bit_hook() will increase BTRFS_I(inode)->outstanding_extents by one, so now BTRFS_I(inode)->outstanding_extents is 3. 64KB128KB132KB |---|| 64K DELALLOC 4K DELALLOC Outstanding_extents: 3 But the correct outstanding_extents number should be 2, not 3. The 2nd btrfs_set_bit_hook() call just screwed up this, and leads to the WARN_ON(). Normally, we can solve it by only increasing outstanding_extents in set_bit_hook(). But the problem is for delalloc_reserve/release_metadata(), we only have a 'length' parameter, and calculate in-accurate outstanding_extents. If we only rely on set_bit_hook() release_metadata() will crew things up as it will decrease inaccurate number. So the fix we use is: 1) Increase *INACCURATE* outstanding_extents at delalloc_reserve_meta Just as a place holder. 2) Increase *accurate* outstanding_extents at set_bit_hooks() This is the real increaser. 3) Decrease *INACCURATE* outstanding_extents before returning This makes outstanding_extents to correct value. For 128M BTRFS_MAX_EXTENT_SIZE, due to limitation of __btrfs_buffered_write(), each iteration will only handle about 2MB data. So btrfs_dirty_pages() won't need to handle cases cross 2 extents. Signed-off-by: Wang Xiaoguang --- fs/btrfs/ctree.h | 2 ++ fs/btrfs/inode.c | 65 ++-- fs/btrfs/ioctl.c | 6 ++ 3 files changed, 62 insertions(+), 11 deletions(-) diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index 33fe035..16885f6 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -3119,6 +3119,8 @@ int btrfs_start_delalloc_roots(struct btrfs_fs_info *fs_info, int delay_iput, int nr); int btrfs_set_extent_delalloc(struct inode *inode, u64 start, u64 end, struct extent_state **cached_state); +int btrfs_set_extent_defrag(struct inode *inode, u64 start, u64 end, + struct extent_state **cached_state); int btrfs_create_subvol_root(struct btrfs_trans_handle *trans, struct btrfs_root *new_root, struct btrfs_root *parent_root, diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index e6811c4..a7193b1 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -1590,6 +1590,9 @@ static void btrfs_split_extent_hook(struct inode *inode, if (!(orig->state & EXTENT_DELALLOC)) return; + if (btrfs_is_free_space_inode(inode)) + return; + size = orig->end - orig->start + 1; if (size > BTRFS_MAX_EXTENT_SIZE) { u64 num_extents; @@ -1632,6 +1635,9 @@ static void btrfs_merge_extent_hook(struct inode *inode, if (!(other->state & EXTENT_DELALLOC)) return; + if (btrfs_is_free_space_inode(inode)) + return; + if (new->start > other->start)
Re: Btrfs progs release 4.8 (32bit builds broken)
Oops, I mistook the intent of that check. Please disregard. On Wed, Oct 5, 2016 at 3:25 PM, Justin Maggard wrote: > I saw a 32-bit build failure, but it looked like a legitimate bug, > unrelated to the compiler version. Here's the trivial fix: > > diff --git a/ioctl.h b/ioctl.h > index a7235c0..26a3a5a 100644 > --- a/ioctl.h > +++ b/ioctl.h > @@ -606,7 +606,7 @@ struct btrfs_ioctl_send_args { > * Size of structure depends on pointer width, was not caught. Kernel > handles > * pointer width differences transparently > */ > -BUILD_ASSERT(sizeof(__u64 *) == 8 > +BUILD_ASSERT(sizeof(__u64) == 8 > ? sizeof(struct btrfs_ioctl_send_args) == 72 > : (sizeof(void *) == 4 > ? sizeof(struct btrfs_ioctl_send_args) == 68 > > -Justin > > On Wed, Oct 5, 2016 at 6:33 AM, David Sterba wrote: >> I got a report that the 32bit builds are broken. This seems to be caused >> by padding inserted (or not) into the structures and depends on a >> compiler version. The error messages may look cryptic, but if you see >> something like >> >> ioctl.h:570:1: note: in expansion of macro 'BUILD_ASSERT' >> BUILD_ASSERT(sizeof(struct btrfs_ioctl_received_subvol_args) == 200); >> >> that means that the given structure has an unexpected size. Fixing that >> properly will probably lead to some tricks to force the exact size >> regardless of arch bits and compiler. >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in >> the body of a message to majord...@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Btrfs progs release 4.8 (32bit builds broken)
I saw a 32-bit build failure, but it looked like a legitimate bug, unrelated to the compiler version. Here's the trivial fix: diff --git a/ioctl.h b/ioctl.h index a7235c0..26a3a5a 100644 --- a/ioctl.h +++ b/ioctl.h @@ -606,7 +606,7 @@ struct btrfs_ioctl_send_args { * Size of structure depends on pointer width, was not caught. Kernel handles * pointer width differences transparently */ -BUILD_ASSERT(sizeof(__u64 *) == 8 +BUILD_ASSERT(sizeof(__u64) == 8 ? sizeof(struct btrfs_ioctl_send_args) == 72 : (sizeof(void *) == 4 ? sizeof(struct btrfs_ioctl_send_args) == 68 -Justin On Wed, Oct 5, 2016 at 6:33 AM, David Sterba wrote: > I got a report that the 32bit builds are broken. This seems to be caused > by padding inserted (or not) into the structures and depends on a > compiler version. The error messages may look cryptic, but if you see > something like > > ioctl.h:570:1: note: in expansion of macro 'BUILD_ASSERT' > BUILD_ASSERT(sizeof(struct btrfs_ioctl_received_subvol_args) == 200); > > that means that the given structure has an unexpected size. Fixing that > properly will probably lead to some tricks to force the exact size > regardless of arch bits and compiler. > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] btrfs: opencode chunk locking, remove helpers
On 10/5/16 8:23 AM, David Sterba wrote: > The helpers are trivial and we don't use them consistently. This one is going to conflict with the root->fs_info removal patchset in every chunk since the helper does nothing with the root. Otherwise, the idea is sound. -Jeff > Signed-off-by: David Sterba > --- > fs/btrfs/disk-io.c | 4 +-- > fs/btrfs/extent-tree.c | 8 +++--- > fs/btrfs/free-space-cache.c | 4 +-- > fs/btrfs/volumes.c | 70 > ++--- > fs/btrfs/volumes.h | 10 --- > 5 files changed, 43 insertions(+), 53 deletions(-) > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index 54bc8c7c6bcd..6091663a9ed3 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -3989,7 +3989,7 @@ void close_ctree(struct btrfs_root *root) > __btrfs_free_block_rsv(root->orphan_block_rsv); > root->orphan_block_rsv = NULL; > > - lock_chunks(root); > + mutex_lock(&fs_info->chunk_mutex); > while (!list_empty(&fs_info->pinned_chunks)) { > struct extent_map *em; > > @@ -3998,7 +3998,7 @@ void close_ctree(struct btrfs_root *root) > list_del_init(&em->list); > free_extent_map(em); > } > - unlock_chunks(root); > + mutex_unlock(&fs_info->chunk_mutex); > } > > int btrfs_buffer_uptodate(struct extent_buffer *buf, u64 parent_transid, > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index 665da8f66ff1..00e5655cfd84 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -9617,9 +9617,9 @@ int btrfs_inc_block_group_ro(struct btrfs_root *root, > out: > if (cache->flags & BTRFS_BLOCK_GROUP_SYSTEM) { > alloc_flags = update_block_group_flags(root, cache->flags); > - lock_chunks(root->fs_info->chunk_root); > + mutex_lock(&root->fs_info->chunk_mutex); > check_system_chunk(trans, root, alloc_flags); > - unlock_chunks(root->fs_info->chunk_root); > + mutex_unlock(&root->fs_info->chunk_mutex); > } > mutex_unlock(&root->fs_info->ro_block_group_mutex); > > @@ -10647,7 +10647,7 @@ int btrfs_remove_block_group(struct > btrfs_trans_handle *trans, > > memcpy(&key, &block_group->key, sizeof(key)); > > - lock_chunks(root); > + mutex_lock(&root->fs_info->chunk_mutex); > if (!list_empty(&em->list)) { > /* We're in the transaction->pending_chunks list. */ > free_extent_map(em); > @@ -10715,7 +10715,7 @@ int btrfs_remove_block_group(struct > btrfs_trans_handle *trans, > free_extent_map(em); > } > > - unlock_chunks(root); > + mutex_unlock(&root->fs_info->chunk_mutex); > > ret = remove_block_group_free_space(trans, root->fs_info, block_group); > if (ret) > diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c > index d571bd2b697b..0e33d3d84870 100644 > --- a/fs/btrfs/free-space-cache.c > +++ b/fs/btrfs/free-space-cache.c > @@ -3328,7 +3328,7 @@ void btrfs_put_block_group_trimming(struct > btrfs_block_group_cache *block_group) > spin_unlock(&block_group->lock); > > if (cleanup) { > - lock_chunks(block_group->fs_info->chunk_root); > + mutex_lock(&block_group->fs_info->chunk_mutex); > em_tree = &block_group->fs_info->mapping_tree.map_tree; > write_lock(&em_tree->lock); > em = lookup_extent_mapping(em_tree, block_group->key.objectid, > @@ -3340,7 +3340,7 @@ void btrfs_put_block_group_trimming(struct > btrfs_block_group_cache *block_group) >*/ > remove_extent_mapping(em_tree, em); > write_unlock(&em_tree->lock); > - unlock_chunks(block_group->fs_info->chunk_root); > + mutex_unlock(&block_group->fs_info->chunk_mutex); > > /* once for us and once for the tree */ > free_extent_map(em); > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 035efce603a9..cc31b39160be 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -1878,10 +1878,10 @@ int btrfs_rm_device(struct btrfs_root *root, char > *device_path, u64 devid) > } > > if (device->writeable) { > - lock_chunks(root); > + mutex_lock(&root->fs_info->chunk_mutex); > list_del_init(&device->dev_alloc_list); > device->fs_devices->rw_devices--; > - unlock_chunks(root); > + mutex_unlock(&root->fs_info->chunk_mutex); > dev_name = kstrdup(device->name->str, GFP_KERNEL); > if (!dev_name) { > ret = -ENOMEM; > @@ -1985,11 +1985,11 @@ int btrfs_rm_device(struct btrfs_root *root, char > *device_path, u64 devid) > > error_undo: > if (device->writeable) { > - lock_chunks(root); > + mutex_lock(&root->fs_info->chunk_mutex); >
Re: Btrfs progs release 4.8 (32bit builds broken)
I got a report that the 32bit builds are broken. This seems to be caused by padding inserted (or not) into the structures and depends on a compiler version. The error messages may look cryptic, but if you see something like ioctl.h:570:1: note: in expansion of macro 'BUILD_ASSERT' BUILD_ASSERT(sizeof(struct btrfs_ioctl_received_subvol_args) == 200); that means that the given structure has an unexpected size. Fixing that properly will probably lead to some tricks to force the exact size regardless of arch bits and compiler. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Btrfs-progs: subvol_uuid_search: Return error code on memory allocation failure
From: Prasanth K S R This commit fixes coverity defect CID 1328695. Signed-off-by: Prasanth K S R --- send-utils.c | 5 + 1 file changed, 5 insertions(+) diff --git a/send-utils.c b/send-utils.c index a85fa08..6f80b6f 100644 --- a/send-utils.c +++ b/send-utils.c @@ -486,6 +486,11 @@ struct subvol_info *subvol_uuid_search(struct subvol_uuid_search *s, info->path = strdup(path); } else { info->path = malloc(PATH_MAX); + if (!info->path) { + fprintf(stderr, "Memory allocation failed\n"); + ret = -ENOMEM; + goto out; + } ret = btrfs_subvolid_resolve(s->mnt_fd, info->path, PATH_MAX, root_id); } -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] btrfs: opencode chunk locking, remove helpers
The helpers are trivial and we don't use them consistently. Signed-off-by: David Sterba --- fs/btrfs/disk-io.c | 4 +-- fs/btrfs/extent-tree.c | 8 +++--- fs/btrfs/free-space-cache.c | 4 +-- fs/btrfs/volumes.c | 70 ++--- fs/btrfs/volumes.h | 10 --- 5 files changed, 43 insertions(+), 53 deletions(-) diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 54bc8c7c6bcd..6091663a9ed3 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -3989,7 +3989,7 @@ void close_ctree(struct btrfs_root *root) __btrfs_free_block_rsv(root->orphan_block_rsv); root->orphan_block_rsv = NULL; - lock_chunks(root); + mutex_lock(&fs_info->chunk_mutex); while (!list_empty(&fs_info->pinned_chunks)) { struct extent_map *em; @@ -3998,7 +3998,7 @@ void close_ctree(struct btrfs_root *root) list_del_init(&em->list); free_extent_map(em); } - unlock_chunks(root); + mutex_unlock(&fs_info->chunk_mutex); } int btrfs_buffer_uptodate(struct extent_buffer *buf, u64 parent_transid, diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 665da8f66ff1..00e5655cfd84 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -9617,9 +9617,9 @@ int btrfs_inc_block_group_ro(struct btrfs_root *root, out: if (cache->flags & BTRFS_BLOCK_GROUP_SYSTEM) { alloc_flags = update_block_group_flags(root, cache->flags); - lock_chunks(root->fs_info->chunk_root); + mutex_lock(&root->fs_info->chunk_mutex); check_system_chunk(trans, root, alloc_flags); - unlock_chunks(root->fs_info->chunk_root); + mutex_unlock(&root->fs_info->chunk_mutex); } mutex_unlock(&root->fs_info->ro_block_group_mutex); @@ -10647,7 +10647,7 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans, memcpy(&key, &block_group->key, sizeof(key)); - lock_chunks(root); + mutex_lock(&root->fs_info->chunk_mutex); if (!list_empty(&em->list)) { /* We're in the transaction->pending_chunks list. */ free_extent_map(em); @@ -10715,7 +10715,7 @@ int btrfs_remove_block_group(struct btrfs_trans_handle *trans, free_extent_map(em); } - unlock_chunks(root); + mutex_unlock(&root->fs_info->chunk_mutex); ret = remove_block_group_free_space(trans, root->fs_info, block_group); if (ret) diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c index d571bd2b697b..0e33d3d84870 100644 --- a/fs/btrfs/free-space-cache.c +++ b/fs/btrfs/free-space-cache.c @@ -3328,7 +3328,7 @@ void btrfs_put_block_group_trimming(struct btrfs_block_group_cache *block_group) spin_unlock(&block_group->lock); if (cleanup) { - lock_chunks(block_group->fs_info->chunk_root); + mutex_lock(&block_group->fs_info->chunk_mutex); em_tree = &block_group->fs_info->mapping_tree.map_tree; write_lock(&em_tree->lock); em = lookup_extent_mapping(em_tree, block_group->key.objectid, @@ -3340,7 +3340,7 @@ void btrfs_put_block_group_trimming(struct btrfs_block_group_cache *block_group) */ remove_extent_mapping(em_tree, em); write_unlock(&em_tree->lock); - unlock_chunks(block_group->fs_info->chunk_root); + mutex_unlock(&block_group->fs_info->chunk_mutex); /* once for us and once for the tree */ free_extent_map(em); diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 035efce603a9..cc31b39160be 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -1878,10 +1878,10 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path, u64 devid) } if (device->writeable) { - lock_chunks(root); + mutex_lock(&root->fs_info->chunk_mutex); list_del_init(&device->dev_alloc_list); device->fs_devices->rw_devices--; - unlock_chunks(root); + mutex_unlock(&root->fs_info->chunk_mutex); dev_name = kstrdup(device->name->str, GFP_KERNEL); if (!dev_name) { ret = -ENOMEM; @@ -1985,11 +1985,11 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path, u64 devid) error_undo: if (device->writeable) { - lock_chunks(root); + mutex_lock(&root->fs_info->chunk_mutex); list_add(&device->dev_alloc_list, &root->fs_info->fs_devices->alloc_list); device->fs_devices->rw_devices++; - unlock_chunks(root); + mutex_unlock(&root->fs_info->chunk_mutex); } goto out; } @@ -2216,9 +2216,9 @@ static int btrfs_prepare
[PATCH] btrfs: remove stale comment from btrfs_statfs
Signed-off-by: David Sterba --- fs/btrfs/super.c | 4 1 file changed, 4 deletions(-) diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index 4071fe2bd098..2b852ca4fea5 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -2076,10 +2076,6 @@ static int btrfs_statfs(struct dentry *dentry, struct kstatfs *buf) u64 thresh = 0; int mixed = 0; - /* -* holding chunk_mutex to avoid allocating new chunks, holding -* device_list_mutex to avoid the device being removed -*/ rcu_read_lock(); list_for_each_entry_rcu(found, head, list) { if (found->flags & BTRFS_BLOCK_GROUP_DATA) { -- 2.10.0 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] btrfs: remove unused headers, statfs.h
Signed-off-by: David Sterba --- fs/btrfs/file.c | 1 - fs/btrfs/inode.c | 1 - fs/btrfs/ioctl.c | 1 - 3 files changed, 3 deletions(-) diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index fea31a4a6e36..887ed6ac016e 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -27,7 +27,6 @@ #include #include #include -#include #include #include #include diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index e6811c42e41e..3241dfd14db3 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -30,7 +30,6 @@ #include #include #include -#include #include #include #include diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 7fd939bfbd99..a6959c7d9807 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -33,7 +33,6 @@ #include #include #include -#include #include #include #include -- 2.10.0 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Btrfs progs release 4.8
Hi, btrfs-progs 4.8 have been released. Though it's a major version release, there are no shiny new features. The theme was error handling improvements and addressing some fuzzer bugz. There are several fuzzer bugs open and some tests might report errors with ASAN or UBSAN enabled. Fixing needs more time, as some bugons happen deep in call chains. At this moment, bugs triggered by fuzzed images are not considered release blockers. Changes: * error handling improvements all over the place * new fuzzed images, test updates * doc fixups * minor cleanups and improvements * kernel library helpers moved to own directory * qgroup: fix regression leading to incorrect status after check, introduced in 4.7 Tarballs: https://www.kernel.org/pub/linux/kernel/people/kdave/btrfs-progs/ Git: git://git.kernel.org/pub/scm/linux/kernel/git/kdave/btrfs-progs.git Shortlog: Adam Borowski (1): btrfs-progs: docs: document exit codes from scrub David Sterba (75): btrfs-progs: scrub: improved error handling in scrub_read_file btrfs-progs: fi usage: improved error handling in load_device_info btrfs-progs: dump-tree: improved error handling in print_extents btrfs-progs: dump-tree: improved error handling in cmd_inspect_dump_tree btrfs-progs: switch BUG_ON to ASSERT in reserve_free_space btrfs-progs: more verbose error handling in creation helpers btrfs-progs: mkfs: check for sane sectorsize earlier btrfs-progs: handle errors from btrfs_alloc_path btrfs-progs: receive: improved error handling in process_snapshot btrfs-progs: check: improved error handling in calc_extent_flag btrfs-progs: switch column values to asserts btrfs-progs: chunk-recover: improve error handling in insert_stripe btrfs-progs: mkfs: handle block ordering errors in make_btrfs btrfs-progs: convert: improve error handling in create_image_file_range btrfs-progs: catch invalid flags in open_ctree_fd btrfs-progs: remove trivial helpers for filtering functions btrfs-progs: qgroup: switch to common message helpers btrfs-progs: improved error handling in btrfs_print_tree btrfs-progs: inspect: improved error handling btrfs-progs: fi du: catch bogus extent lengths btrfs-progs: fi du: improved error handling in mark_inode_seen btrfs-progs: tree-stats: switch to common message helpers btrfs-progs: prop: simplify help printing code btrfs-progs: remove redundant check in btrfs_add_to_fsid btrfs-progs: improve error handling in btrfs_alloc_data_chunk btrfs-progs: dump-super: switch to common message helpers btrfs-progs: corrupt-block: improved error handling in corrupt_item_nocow btrfs-progs: improve error handling in btrfs_add_to_fsid btrfs-progs: use standard allocation functions in non-kenrel code btrfs-progs: check: handle errors returned by add_extent_rec_nolookup btrfs-progs: check: improve error handling in add_extent_rec_nolookup btrfs-progs: convert: improve error handling in do_rollback btrfs-progs: image: switch to common message helpers btrfs-progs: check: switch some messages to common helpers btrfs-progs: corrupt-block: fix assertion condition btrfs-progs: improve error handling in clone_inode_rec btrfs-progs: cleanup, kill trivial btrfs_set_key_type helper btrfs-progs: cleanup, kill trivial btrfs_key_type helper btrfs-progs: remove unused variable in add_inode_items btrfs-progs: use PATH_MAX in cmd_inspect_logical_resolve btrfs-progs: mkfs: remove useless helper btrfs-progs: tests: iterate over fuzzed images and test various tools btrfs-progs: tests: add script to scan results for some known runtime errors btrfs-progs: build: add basic support for subdirectory build btrfs-progs: move 3rd party kernel library modules to own directory btrfs-progs: restore: update help text btrfs-progs: remove stray function declaration btrfs-progs: don't write to optarg in btrfs_qgroup_parse_sort_string btrfs-progs: constify string arguments where appropriate btrfs-progs: image: use common message helpers btrfs-progs: btrfstune: use common message helpers btrfs-progs: more sanity checks in read_tree_block_fs_info btrfs-progs: tests: add fuzzed images with bad blocksize/lengh of eb btrfs-progs: check: better error handling in find_parent_roots btrfs-progs: tests: add fuzzed image with bad parent refs, qgroup-verify btrfs-progs: image: catch zero length extents, avoid endless loop btrfs-progs: image: return negativer error from all paths in mdrestore_init btrfs-progs: image: drop useless bug_on btrfs-progs: print value when assertion fails btrfs-progs: chunk-recover: handle duplicate cache entries btrfs-progs: don't access freed memory in btrfs_close_devices btrfs-progs: tests: split test
[PATCH] btrfs-progs: image: fix compiler warning
In v4.8-rc1, gcc 5.3.1 gives following warning. Fixed it. [CC] btrfs-image.o btrfs-image.c: In function 'flush_pending': btrfs-image.c:708:17: warning: 'start' may be used uninitialized in this function [-Wmaybe-uninitialized] header->bytenr = cpu_to_le64(start); ^ btrfs-image.c:927:6: note: 'start' was declared here u64 start; ^ Signed-off-by: Tsutomu Itoh --- btrfs-image.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/btrfs-image.c b/btrfs-image.c index 0d410f8..47f36b9 100644 --- a/btrfs-image.c +++ b/btrfs-image.c @@ -924,7 +924,7 @@ static int flush_pending(struct metadump_struct *md, int done) struct async_work *async = NULL; struct extent_buffer *eb; u64 blocksize = md->root->nodesize; - u64 start; + u64 start = 0; u64 size; size_t offset; int ret = 0; -- 2.9.3 Tsutomu Itoh t-i...@jp.fujitsu.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html