Skip to content

Commit

Permalink
btrfs: skip compression property for anything other than files and dirs
Browse files Browse the repository at this point in the history
The compression property only has effect on regular files and directories
(so that it's propagated to files and subdirectories created inside a
directory). For any other inode type (symlink, fifo, device, socket),
it's pointless to set the compression property because it does nothing
and ends up unnecessarily wasting leaf space due to the pointless xattr
(75 or 76 bytes, depending on the compression value). Symlinks in
particular are very common (for example, I have almost 10k symlinks under
/etc, /usr and /var alone) and therefore it's worth to avoid wasting
leaf space with the compression xattr.

For example, the compression property can end up on a symlink or character
device implicitly, through inheritance from a parent directory

  $ mkdir /mnt/testdir
  $ btrfs property set /mnt/testdir compression lzo

  $ ln -s yadayada /mnt/testdir/lnk
  $ mknod /mnt/testdir/dev c 0 0

Or explicitly like this:

  $ ln -s yadayda /mnt/lnk
  $ setfattr -h -n btrfs.compression -v lzo /mnt/lnk

So skip the compression property on inodes that are neither a regular
file nor a directory.

CC: [email protected] # 5.4+
Signed-off-by: Filipe Manana <[email protected]>
Reviewed-by: David Sterba <[email protected]>
Signed-off-by: David Sterba <[email protected]>
  • Loading branch information
fdmanana authored and kdave committed Apr 27, 2022
1 parent 193b4e8 commit 4b73c55
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 0 deletions.
43 changes: 43 additions & 0 deletions fs/btrfs/props.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ struct prop_handler {
size_t len);
int (*apply)(struct inode *inode, const char *value, size_t len);
const char *(*extract)(struct inode *inode);
bool (*ignore)(const struct btrfs_inode *inode);
int inheritable;
};

Expand Down Expand Up @@ -74,6 +75,28 @@ int btrfs_validate_prop(const struct btrfs_inode *inode, const char *name,
return handler->validate(inode, value, value_len);
}

/*
* Check if a property should be ignored (not set) for an inode.
*
* @inode: The target inode.
* @name: The property's name.
*
* The caller must be sure the given property name is valid, for example by
* having previously called btrfs_validate_prop().
*
* Returns: true if the property should be ignored for the given inode
* false if the property must not be ignored for the given inode
*/
bool btrfs_ignore_prop(const struct btrfs_inode *inode, const char *name)
{
const struct prop_handler *handler;

handler = find_prop_handler(name, NULL);
ASSERT(handler != NULL);

return handler->ignore(inode);
}

int btrfs_set_prop(struct btrfs_trans_handle *trans, struct inode *inode,
const char *name, const char *value, size_t value_len,
int flags)
Expand Down Expand Up @@ -316,6 +339,22 @@ static int prop_compression_apply(struct inode *inode, const char *value,
return 0;
}

static bool prop_compression_ignore(const struct btrfs_inode *inode)
{
/*
* Compression only has effect for regular files, and for directories
* we set it just to propagate it to new files created inside them.
* Everything else (symlinks, devices, sockets, fifos) is pointless as
* it will do nothing, so don't waste metadata space on a compression
* xattr for anything that is neither a file nor a directory.
*/
if (!S_ISREG(inode->vfs_inode.i_mode) &&
!S_ISDIR(inode->vfs_inode.i_mode))
return true;

return false;
}

static const char *prop_compression_extract(struct inode *inode)
{
switch (BTRFS_I(inode)->prop_compress) {
Expand All @@ -336,6 +375,7 @@ static struct prop_handler prop_handlers[] = {
.validate = prop_compression_validate,
.apply = prop_compression_apply,
.extract = prop_compression_extract,
.ignore = prop_compression_ignore,
.inheritable = 1
},
};
Expand All @@ -362,6 +402,9 @@ static int inherit_props(struct btrfs_trans_handle *trans,
if (!h->inheritable)
continue;

if (h->ignore(BTRFS_I(inode)))
continue;

value = h->extract(parent);
if (!value)
continue;
Expand Down
1 change: 1 addition & 0 deletions fs/btrfs/props.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ int btrfs_set_prop(struct btrfs_trans_handle *trans, struct inode *inode,
int flags);
int btrfs_validate_prop(const struct btrfs_inode *inode, const char *name,
const char *value, size_t value_len);
bool btrfs_ignore_prop(const struct btrfs_inode *inode, const char *name);

int btrfs_load_inode_props(struct inode *inode, struct btrfs_path *path);

Expand Down
3 changes: 3 additions & 0 deletions fs/btrfs/xattr.c
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,9 @@ static int btrfs_xattr_handler_set_prop(const struct xattr_handler *handler,
if (ret)
return ret;

if (btrfs_ignore_prop(BTRFS_I(inode), name))
return 0;

trans = btrfs_start_transaction(root, 2);
if (IS_ERR(trans))
return PTR_ERR(trans);
Expand Down

0 comments on commit 4b73c55

Please sign in to comment.