* [PATCH] btrfs: property: Set incompat flag of lzo/zstd compression [not found] <cover.1526370458.git.misono.tomohiro@jp.fujitsu.com> @ 2018-05-15 7:51 ` Misono Tomohiro 2018-05-15 8:05 ` Su Yue ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Misono Tomohiro @ 2018-05-15 7:51 UTC (permalink / raw) To: linux-btrfs Incompat flag of lzo/zstd compression should be set at: 1. mount time (-o compress/compress-force) 2. when defrag is done 3. when property is set Currently 3. is missing and this commit adds this. Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com> --- fs/btrfs/props.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c index 53a8c95828e3..dc6140013ae8 100644 --- a/fs/btrfs/props.c +++ b/fs/btrfs/props.c @@ -380,6 +380,7 @@ static int prop_compression_apply(struct inode *inode, const char *value, size_t len) { + struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb); int type; if (len == 0) { @@ -390,14 +391,17 @@ static int prop_compression_apply(struct inode *inode, return 0; } - if (!strncmp("lzo", value, 3)) + if (!strncmp("lzo", value, 3)) { type = BTRFS_COMPRESS_LZO; - else if (!strncmp("zlib", value, 4)) + btrfs_set_fs_incompat(fs_info, COMPRESS_LZO); + } else if (!strncmp("zlib", value, 4)) { type = BTRFS_COMPRESS_ZLIB; - else if (!strncmp("zstd", value, len)) + } else if (!strncmp("zstd", value, len)) { type = BTRFS_COMPRESS_ZSTD; - else + btrfs_set_fs_incompat(fs_info, COMPRESS_ZSTD); + } else { return -EINVAL; + } BTRFS_I(inode)->flags &= ~BTRFS_INODE_NOCOMPRESS; BTRFS_I(inode)->flags |= BTRFS_INODE_COMPRESS; -- 2.14.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: property: Set incompat flag of lzo/zstd compression 2018-05-15 7:51 ` [PATCH] btrfs: property: Set incompat flag of lzo/zstd compression Misono Tomohiro @ 2018-05-15 8:05 ` Su Yue 2018-05-15 8:34 ` Su Yue 2018-05-15 8:35 ` Duncan 2018-05-15 8:20 ` Anand Jain 2018-05-15 14:07 ` David Sterba 2 siblings, 2 replies; 7+ messages in thread From: Su Yue @ 2018-05-15 8:05 UTC (permalink / raw) To: Misono Tomohiro, linux-btrfs [-- Attachment #1: Type: text/plain, Size: 1746 bytes --] On 05/15/2018 03:51 PM, Misono Tomohiro wrote: > Incompat flag of lzo/zstd compression should be set at: > 1. mount time (-o compress/compress-force) > 2. when defrag is done > 3. when property is set > > Currently 3. is missing and this commit adds this. > If I don't misunderstand, compression property of an inode is only apply for *the* inode, not the whole filesystem. So the original logical should be okay. Thanks, Su > Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com> > --- > fs/btrfs/props.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c > index 53a8c95828e3..dc6140013ae8 100644 > --- a/fs/btrfs/props.c > +++ b/fs/btrfs/props.c > @@ -380,6 +380,7 @@ static int prop_compression_apply(struct inode *inode, > const char *value, > size_t len) > { > + struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb); > int type; > > if (len == 0) { > @@ -390,14 +391,17 @@ static int prop_compression_apply(struct inode *inode, > return 0; > } > > - if (!strncmp("lzo", value, 3)) > + if (!strncmp("lzo", value, 3)) { > type = BTRFS_COMPRESS_LZO; > - else if (!strncmp("zlib", value, 4)) > + btrfs_set_fs_incompat(fs_info, COMPRESS_LZO); > + } else if (!strncmp("zlib", value, 4)) { > type = BTRFS_COMPRESS_ZLIB; > - else if (!strncmp("zstd", value, len)) > + } else if (!strncmp("zstd", value, len)) { > type = BTRFS_COMPRESS_ZSTD; > - else > + btrfs_set_fs_incompat(fs_info, COMPRESS_ZSTD); > + } else { > return -EINVAL; > + } > > BTRFS_I(inode)->flags &= ~BTRFS_INODE_NOCOMPRESS; > BTRFS_I(inode)->flags |= BTRFS_INODE_COMPRESS; > [-- Attachment #2: pEpkey.asc --] [-- Type: application/pgp-keys, Size: 1791 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: property: Set incompat flag of lzo/zstd compression 2018-05-15 8:05 ` Su Yue @ 2018-05-15 8:34 ` Su Yue 2018-05-15 8:35 ` Duncan 1 sibling, 0 replies; 7+ messages in thread From: Su Yue @ 2018-05-15 8:34 UTC (permalink / raw) To: Misono Tomohiro, linux-btrfs [-- Attachment #1: Type: text/plain, Size: 1970 bytes --] On 05/15/2018 04:05 PM, Su Yue wrote: > > > On 05/15/2018 03:51 PM, Misono Tomohiro wrote: >> Incompat flag of lzo/zstd compression should be set at: >> 1. mount time (-o compress/compress-force) >> 2. when defrag is done >> 3. when property is set >> >> Currently 3. is missing and this commit adds this. >> > > If I don't misunderstand, compression property of an inode is only Embarrassed for bad memory about btrfs_set_fs_incompat(). The patch is fine. Just ignore this thread. > apply for *the* inode, not the whole filesystem. > So the original logical should be okay. > > Thanks, > Su > >> Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com> >> --- >> fs/btrfs/props.c | 12 ++++++++---- >> 1 file changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c >> index 53a8c95828e3..dc6140013ae8 100644 >> --- a/fs/btrfs/props.c >> +++ b/fs/btrfs/props.c >> @@ -380,6 +380,7 @@ static int prop_compression_apply(struct inode *inode, >> const char *value, >> size_t len) >> { >> + struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb); >> int type; >> >> if (len == 0) { >> @@ -390,14 +391,17 @@ static int prop_compression_apply(struct inode *inode, >> return 0; >> } >> >> - if (!strncmp("lzo", value, 3)) >> + if (!strncmp("lzo", value, 3)) { >> type = BTRFS_COMPRESS_LZO; >> - else if (!strncmp("zlib", value, 4)) >> + btrfs_set_fs_incompat(fs_info, COMPRESS_LZO); >> + } else if (!strncmp("zlib", value, 4)) { >> type = BTRFS_COMPRESS_ZLIB; >> - else if (!strncmp("zstd", value, len)) >> + } else if (!strncmp("zstd", value, len)) { >> type = BTRFS_COMPRESS_ZSTD; >> - else >> + btrfs_set_fs_incompat(fs_info, COMPRESS_ZSTD); >> + } else { >> return -EINVAL; >> + } >> >> BTRFS_I(inode)->flags &= ~BTRFS_INODE_NOCOMPRESS; >> BTRFS_I(inode)->flags |= BTRFS_INODE_COMPRESS; >> > > [-- Attachment #2: pEpkey.asc --] [-- Type: application/pgp-keys, Size: 1791 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: property: Set incompat flag of lzo/zstd compression 2018-05-15 8:05 ` Su Yue 2018-05-15 8:34 ` Su Yue @ 2018-05-15 8:35 ` Duncan 2018-05-15 8:50 ` Su Yue 1 sibling, 1 reply; 7+ messages in thread From: Duncan @ 2018-05-15 8:35 UTC (permalink / raw) To: linux-btrfs Su Yue posted on Tue, 15 May 2018 16:05:01 +0800 as excerpted: > > On 05/15/2018 03:51 PM, Misono Tomohiro wrote: >> Incompat flag of lzo/zstd compression should be set at: >> 1. mount time (-o compress/compress-force) >> 2. when defrag is done 3. when property is set >> >> Currently 3. is missing and this commit adds this. >> >> > If I don't misunderstand, compression property of an inode is only apply > for *the* inode, not the whole filesystem. > So the original logical should be okay. But the inode is on the filesystem, and if it's compressed with lzo/zstd, the incompat flag should be set to avoid mounting with an earlier kernel that doesn't understand that compression and would therefore, if we're lucky, simply fail to read the data compressed in that file/inode. (If we're unlucky it could blow up with kernel memory corruption like James Harvey's current case of unexpected, corrupted compressed data in a nocow file that being nocow, doesn't have csum validation to fail and abort the decompression, and shouldn't be compressed at all.) So better to set the incompat flag and refuse to mount at all on kernels that don't have the required compression support. -- Duncan - List replies preferred. No HTML msgs. "Every nonfree program has a lord, a master -- and if you use the program, he is your master." Richard Stallman ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: property: Set incompat flag of lzo/zstd compression 2018-05-15 8:35 ` Duncan @ 2018-05-15 8:50 ` Su Yue 0 siblings, 0 replies; 7+ messages in thread From: Su Yue @ 2018-05-15 8:50 UTC (permalink / raw) To: Duncan, linux-btrfs [-- Attachment #1: Type: text/plain, Size: 1405 bytes --] On 05/15/2018 04:35 PM, Duncan wrote: > Su Yue posted on Tue, 15 May 2018 16:05:01 +0800 as excerpted: > > >> >> On 05/15/2018 03:51 PM, Misono Tomohiro wrote: >>> Incompat flag of lzo/zstd compression should be set at: >>> 1. mount time (-o compress/compress-force) >>> 2. when defrag is done 3. when property is set >>> >>> Currently 3. is missing and this commit adds this. >>> >>> >> If I don't misunderstand, compression property of an inode is only apply >> for *the* inode, not the whole filesystem. >> So the original logical should be okay. > > But the inode is on the filesystem, and if it's compressed with lzo/zstd, > the incompat flag should be set to avoid mounting with an earlier kernel > that doesn't understand that compression and would therefore, if we're > lucky, simply fail to read the data compressed in that file/inode. (If > we're unlucky it could blow up with kernel memory corruption like James > Harvey's current case of unexpected, corrupted compressed data in a nocow > file that being nocow, doesn't have csum validation to fail and abort the > decompression, and shouldn't be compressed at all.) > > So better to set the incompat flag and refuse to mount at all on kernels > that don't have the required compression support. > Get it. As your conclusion, it's indeed better to set the incompat flag. Thanks, Su [-- Attachment #2: pEpkey.asc --] [-- Type: application/pgp-keys, Size: 1791 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: property: Set incompat flag of lzo/zstd compression 2018-05-15 7:51 ` [PATCH] btrfs: property: Set incompat flag of lzo/zstd compression Misono Tomohiro 2018-05-15 8:05 ` Su Yue @ 2018-05-15 8:20 ` Anand Jain 2018-05-15 14:07 ` David Sterba 2 siblings, 0 replies; 7+ messages in thread From: Anand Jain @ 2018-05-15 8:20 UTC (permalink / raw) To: Misono Tomohiro, linux-btrfs On 05/15/2018 03:51 PM, Misono Tomohiro wrote: > Incompat flag of lzo/zstd compression should be set at: > 1. mount time (-o compress/compress-force) > 2. when defrag is done > 3. when property is set > > Currently 3. is missing and this commit adds this. > > Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com> Reviewed-by: Anand Jain <anand.jain@oracle.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: property: Set incompat flag of lzo/zstd compression 2018-05-15 7:51 ` [PATCH] btrfs: property: Set incompat flag of lzo/zstd compression Misono Tomohiro 2018-05-15 8:05 ` Su Yue 2018-05-15 8:20 ` Anand Jain @ 2018-05-15 14:07 ` David Sterba 2 siblings, 0 replies; 7+ messages in thread From: David Sterba @ 2018-05-15 14:07 UTC (permalink / raw) To: Misono Tomohiro; +Cc: linux-btrfs On Tue, May 15, 2018 at 04:51:26PM +0900, Misono Tomohiro wrote: > Incompat flag of lzo/zstd compression should be set at: > 1. mount time (-o compress/compress-force) > 2. when defrag is done > 3. when property is set > > Currently 3. is missing and this commit adds this. That was missed during the review, thanks for catching it. > Signed-off-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com> Reviewed-by: David Sterba <dsterba@suse.com> For 4.17-rc and for stable 4.14+. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-05-15 14:09 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <cover.1526370458.git.misono.tomohiro@jp.fujitsu.com> 2018-05-15 7:51 ` [PATCH] btrfs: property: Set incompat flag of lzo/zstd compression Misono Tomohiro 2018-05-15 8:05 ` Su Yue 2018-05-15 8:34 ` Su Yue 2018-05-15 8:35 ` Duncan 2018-05-15 8:50 ` Su Yue 2018-05-15 8:20 ` Anand Jain 2018-05-15 14:07 ` David Sterba
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.