linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] btrfs: Embed sector size check into BTRFS_MAX_INLINE_DATA_SIZE()
@ 2018-03-02  2:09 Qu Wenruo
  2018-03-02  2:09 ` [PATCH 2/3] btrfs: Unify inline extent creation condition for plain and compressed data Qu Wenruo
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Qu Wenruo @ 2018-03-02  2:09 UTC (permalink / raw)
  To: linux-btrfs; +Cc: dsterba

We have extra sector size check in cow_file_range_inline(), but doesn't
implement it in BTRFS_MAX_INLINE_DATA_SIZE().

The biggest reason is that btrfs_symlink() also uses this macro to check
name length.

In fact such behavior makes max_inline calculation quite confusing, and
cause unexpected large extent for symbol link.

Here we embed sector size check into BTRFS_MAX_INLINE_DATA_SIZE() so
that it will never exceed sector size.

The downside is, for symbol link, we will reduce max symbol link length
from 16K- to 4095, but it won't affect current system using that long
name, but only prevent later creation.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/ctree.h | 5 +++--
 fs/btrfs/inode.c | 1 -
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 13c260b525a1..90948096c00f 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1297,8 +1297,9 @@ static inline u32 BTRFS_NODEPTRS_PER_BLOCK(const struct btrfs_fs_info *info)
 		(offsetof(struct btrfs_file_extent_item, disk_bytenr))
 static inline u32 BTRFS_MAX_INLINE_DATA_SIZE(const struct btrfs_fs_info *info)
 {
-	return BTRFS_MAX_ITEM_SIZE(info) -
-	       BTRFS_FILE_EXTENT_INLINE_DATA_START;
+	return min_t(u32, info->sectorsize - 1,
+		     BTRFS_MAX_ITEM_SIZE(info) -
+		     BTRFS_FILE_EXTENT_INLINE_DATA_START);
 }
 
 static inline u32 BTRFS_MAX_XATTR_SIZE(const struct btrfs_fs_info *info)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index e1a7f3cb5be9..0f7041e10c67 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -299,7 +299,6 @@ static noinline int cow_file_range_inline(struct btrfs_root *root,
 
 	if (start > 0 ||
 	    actual_end > fs_info->sectorsize ||
-	    data_len > BTRFS_MAX_INLINE_DATA_SIZE(fs_info) ||
 	    (!compressed_size &&
 	    (actual_end & (fs_info->sectorsize - 1)) == 0) ||
 	    end + 1 < isize ||
-- 
2.16.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/3] btrfs: Unify inline extent creation condition for plain and compressed data
  2018-03-02  2:09 [PATCH 1/3] btrfs: Embed sector size check into BTRFS_MAX_INLINE_DATA_SIZE() Qu Wenruo
@ 2018-03-02  2:09 ` Qu Wenruo
  2018-03-02  2:09 ` [PATCH 3/3] btrfs: Show more accurate max_inline Qu Wenruo
  2018-03-02  2:34 ` [PATCH 1/3] btrfs: Embed sector size check into BTRFS_MAX_INLINE_DATA_SIZE() Qu Wenruo
  2 siblings, 0 replies; 4+ messages in thread
From: Qu Wenruo @ 2018-03-02  2:09 UTC (permalink / raw)
  To: linux-btrfs; +Cc: dsterba

cow_file_range_inline() used different condition for plain and
compressed data.

For compressed data, it's allowed to have inline extent equal to sectorsize,
while for plain data, it's not allowed to have inline extent equal to
sectorsize.

But since we limit BTRFS_MAX_INLINE_DATA_SIZE() to (sectorsize - 1),
there is no such difference any long, just remove the extra check.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/inode.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 0f7041e10c67..8c5e69bdbfb5 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -299,8 +299,6 @@ static noinline int cow_file_range_inline(struct btrfs_root *root,
 
 	if (start > 0 ||
 	    actual_end > fs_info->sectorsize ||
-	    (!compressed_size &&
-	    (actual_end & (fs_info->sectorsize - 1)) == 0) ||
 	    end + 1 < isize ||
 	    data_len > fs_info->max_inline) {
 		return 1;
-- 
2.16.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 3/3] btrfs: Show more accurate max_inline
  2018-03-02  2:09 [PATCH 1/3] btrfs: Embed sector size check into BTRFS_MAX_INLINE_DATA_SIZE() Qu Wenruo
  2018-03-02  2:09 ` [PATCH 2/3] btrfs: Unify inline extent creation condition for plain and compressed data Qu Wenruo
@ 2018-03-02  2:09 ` Qu Wenruo
  2018-03-02  2:34 ` [PATCH 1/3] btrfs: Embed sector size check into BTRFS_MAX_INLINE_DATA_SIZE() Qu Wenruo
  2 siblings, 0 replies; 4+ messages in thread
From: Qu Wenruo @ 2018-03-02  2:09 UTC (permalink / raw)
  To: linux-btrfs; +Cc: dsterba

Btrfs shows max_inline option into kernel message, but for
max_inline=4096, btrfs won't really inline 4096 bytes inline data if
it's not compressed.

Since we have unified the behavior and now BTRFS_MAX_INLINE_DATA_SIZE()
should handle most of the condition check, just limit
fs_info->max_inline to BTRFS_MAX_INLINE_DATA_SIZE(), so we could have
more accurate max_inline output.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/super.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 3a4dce153645..6685016bc0ec 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -618,8 +618,8 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
 
 				if (info->max_inline) {
 					info->max_inline = min_t(u64,
-						info->max_inline,
-						info->sectorsize);
+					info->max_inline,
+					BTRFS_MAX_INLINE_DATA_SIZE(info));
 				}
 				btrfs_info(info, "max_inline at %llu",
 					   info->max_inline);
-- 
2.16.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/3] btrfs: Embed sector size check into BTRFS_MAX_INLINE_DATA_SIZE()
  2018-03-02  2:09 [PATCH 1/3] btrfs: Embed sector size check into BTRFS_MAX_INLINE_DATA_SIZE() Qu Wenruo
  2018-03-02  2:09 ` [PATCH 2/3] btrfs: Unify inline extent creation condition for plain and compressed data Qu Wenruo
  2018-03-02  2:09 ` [PATCH 3/3] btrfs: Show more accurate max_inline Qu Wenruo
@ 2018-03-02  2:34 ` Qu Wenruo
  2 siblings, 0 replies; 4+ messages in thread
From: Qu Wenruo @ 2018-03-02  2:34 UTC (permalink / raw)
  To: Qu Wenruo, linux-btrfs; +Cc: dsterba


[-- Attachment #1.1: Type: text/plain, Size: 2445 bytes --]

This patch along with any one uses sector/node size in
btrfs_parse_options() should only use them after it get initialized.

Unfortunately at this point, nodesize is not initialized in open_ctree().

So unfortunately, this patch needs extra work to move
btrfs_parse_options() after basic fs_info initialization, and please
ignore this version.

Thanks,
Qu

On 2018年03月02日 10:09, Qu Wenruo wrote:
> We have extra sector size check in cow_file_range_inline(), but doesn't
> implement it in BTRFS_MAX_INLINE_DATA_SIZE().
> 
> The biggest reason is that btrfs_symlink() also uses this macro to check
> name length.
> 
> In fact such behavior makes max_inline calculation quite confusing, and
> cause unexpected large extent for symbol link.
> 
> Here we embed sector size check into BTRFS_MAX_INLINE_DATA_SIZE() so
> that it will never exceed sector size.
> 
> The downside is, for symbol link, we will reduce max symbol link length
> from 16K- to 4095, but it won't affect current system using that long
> name, but only prevent later creation.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/ctree.h | 5 +++--
>  fs/btrfs/inode.c | 1 -
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 13c260b525a1..90948096c00f 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -1297,8 +1297,9 @@ static inline u32 BTRFS_NODEPTRS_PER_BLOCK(const struct btrfs_fs_info *info)
>  		(offsetof(struct btrfs_file_extent_item, disk_bytenr))
>  static inline u32 BTRFS_MAX_INLINE_DATA_SIZE(const struct btrfs_fs_info *info)
>  {
> -	return BTRFS_MAX_ITEM_SIZE(info) -
> -	       BTRFS_FILE_EXTENT_INLINE_DATA_START;
> +	return min_t(u32, info->sectorsize - 1,
> +		     BTRFS_MAX_ITEM_SIZE(info) -
> +		     BTRFS_FILE_EXTENT_INLINE_DATA_START);
>  }
>  
>  static inline u32 BTRFS_MAX_XATTR_SIZE(const struct btrfs_fs_info *info)
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index e1a7f3cb5be9..0f7041e10c67 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -299,7 +299,6 @@ static noinline int cow_file_range_inline(struct btrfs_root *root,
>  
>  	if (start > 0 ||
>  	    actual_end > fs_info->sectorsize ||
> -	    data_len > BTRFS_MAX_INLINE_DATA_SIZE(fs_info) ||
>  	    (!compressed_size &&
>  	    (actual_end & (fs_info->sectorsize - 1)) == 0) ||
>  	    end + 1 < isize ||
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-03-02  2:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-02  2:09 [PATCH 1/3] btrfs: Embed sector size check into BTRFS_MAX_INLINE_DATA_SIZE() Qu Wenruo
2018-03-02  2:09 ` [PATCH 2/3] btrfs: Unify inline extent creation condition for plain and compressed data Qu Wenruo
2018-03-02  2:09 ` [PATCH 3/3] btrfs: Show more accurate max_inline Qu Wenruo
2018-03-02  2:34 ` [PATCH 1/3] btrfs: Embed sector size check into BTRFS_MAX_INLINE_DATA_SIZE() Qu Wenruo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).