linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] btrfs: correct empty compression property behavior
@ 2014-09-19  8:45 Satoru Takeuchi
  2014-09-19  8:48 ` [PATCH 2/4] btrfs: introduce new compression property to disable compression at all Satoru Takeuchi
  2014-09-29 16:36 ` [PATCH 1/4] btrfs: correct empty compression property behavior David Sterba
  0 siblings, 2 replies; 11+ messages in thread
From: Satoru Takeuchi @ 2014-09-19  8:45 UTC (permalink / raw)
  To: linux-btrfs@vger.kernel.org; +Cc: Chris Mason, Filipe Manana

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 


^ permalink raw reply related	[flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-10-16  7:01 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-19  8:45 [PATCH 1/4] btrfs: correct empty compression property behavior Satoru Takeuchi
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

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).