From: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
To: "linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>
Cc: Chris Mason <clm@fb.com>, Filipe Manana <fdmanana@suse.com>
Subject: [PATCH 1/4] btrfs: correct empty compression property behavior
Date: Fri, 19 Sep 2014 17:45:49 +0900 [thread overview]
Message-ID: <541BED3D.5020803@jp.fujitsu.com> (raw)
From: Naohiro Aota <naota@elisp.net>
In the current implementation, compression property == "" has
the two different meanings: one is with BTRFS_INODE_NOCOMPRESS,
and the other is without this flag.
So, even if the two files a and b have the same compression
property, "", and the same contents, one file seems to be
compressed and the other is not. It's difficult to understand
for users and also confuses them.
Here is the real example. Let assume the following two cases.
a) A file created freshly (under a directory without both
COMPRESS and NOCOMPRESS flag.)
b) A existing file which is explicitly set ""
to compression property.
In addition, here is the command log (I attached the source of
"getflags" program in this patch.)
=======
$ rm -f a b; touch a b
$ btrfs prop set b compression ""
# both a and b have the same compression property: ""
$ btrfs prop get a compression
$ btrfs prop get b compression
# but ... let's take a look at inode flags
$ ./getflags a
0x0
$ ./getflags b
0x400 # 0x400 (FS_NOCOMP_FL) corresponds to BTRFS_INODE_NOCOMPRESS
=======
So both these two files have their compression property == "",
but have different NOCOMPRESS flag state leading to different
behavior.
case | BTRFS_INODE_NOCOMPRESS | behavior
=====+========================+=========================
a | unset | might be compressed
b | set | never be compressed
I consider that we should not expect users to remember
whether their files are case a or b and should introduce
another value for compress property anyway.
getflags.c:
===================
#include <sys/ioctl.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <stdio.h>
#include <linux/fs.h>
int main(int argc, char const* argv[])
{
const char *name = argv[1];
int fd = open(name, O_RDONLY);
long x;
ioctl(fd, FS_IOC_GETFLAGS, &x);
printf("0x%lx\n", x);
return 0;
}
===================
Signed-off-by: Naohiro Aota <naota@elisp.net>
Signed-off-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
---
fs/btrfs/props.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
index 129b1dd..bf005f4 100644
--- a/fs/btrfs/props.c
+++ b/fs/btrfs/props.c
@@ -393,8 +393,8 @@ static int prop_compression_apply(struct inode *inode,
int type;
if (len == 0) {
- BTRFS_I(inode)->flags |= BTRFS_INODE_NOCOMPRESS;
- BTRFS_I(inode)->flags &= ~BTRFS_INODE_COMPRESS;
+ BTRFS_I(inode)->flags &=
+ ~(BTRFS_INODE_COMPRESS | BTRFS_INODE_NOCOMPRESS);
BTRFS_I(inode)->force_compress = BTRFS_COMPRESS_NONE;
return 0;
-- 1.8.3.1
next reply other threads:[~2014-09-19 8:46 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-19 8:45 Satoru Takeuchi [this message]
2014-09-19 8:48 ` [PATCH 2/4] btrfs: introduce new compression property to disable compression at all Satoru Takeuchi
2014-09-19 8:52 ` [PATCH 3/4] btrfs: export __btrfs_set_prop Satoru Takeuchi
2014-09-19 9:05 ` [PATCH 4/4] btrfs: Fix compression related ioctl to run atomic operations in one transaction Satoru Takeuchi
2014-09-25 5:57 ` [PATCH v2 " Satoru Takeuchi
2014-09-22 12:01 ` [PATCH 3/4] btrfs: export __btrfs_set_prop David Sterba
2014-09-25 5:55 ` [PATCH v2 3/4] btrfs: Rename and export __btrfs_set_prop to be called from running transaction Satoru Takeuchi
2014-09-29 16:23 ` David Sterba
2014-09-29 16:36 ` [PATCH 1/4] btrfs: correct empty compression property behavior David Sterba
2014-10-16 1:37 ` Satoru Takeuchi
2014-10-16 7:01 ` Filipe David Manana
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=541BED3D.5020803@jp.fujitsu.com \
--to=takeuchi_satoru@jp.fujitsu.com \
--cc=clm@fb.com \
--cc=fdmanana@suse.com \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).