Re: [f2fs-dev] [PATCH v4 2/2] f2fs: add F2FS_IOC_SET_COMPRESS_OPTION ioctl

2020-10-28 Thread Chao Yu

Daeho,

On 2020/10/29 9:21, Daeho Jeong wrote:

Chao,

Do you want to print out a kernel warning message in this case? like
"XX compression algorithm is set for this inode, but current mount
option doesn't support this algorithm."?


Yup, something like that,

Change 'current mount option' to 'current kernel' or 'current f2fs module' 
would be
better?



2020년 10월 28일 (수) 오후 3:47, Chao Yu 님이 작성:


On 2020/10/27 13:38, Daeho Jeong wrote:

From: Daeho Jeong 

Added a new F2FS_IOC_SET_COMPRESS_OPTION ioctl to change file
compression option of a file.

struct f2fs_comp_option {
  u8 algorithm; => compression algorithm
=> 0:lzo, 1:lz4, 2:zstd, 3:lzorle
  u8 log_cluster_size;  => log scale cluster size
=> 2 ~ 8
};

struct f2fs_comp_option option;

option.algorithm = 1;
option.log_cluster_size = 7;

ioctl(fd, F2FS_IOC_SET_COMPRESS_OPTION, &option);

Signed-off-by: Daeho Jeong 
---

v4: changed commit message.
v3: changed the error number more specific.
  folded in fix for build breakage reported by kernel test robot
   and Dan Carpenter .
v2: added ioctl description.
---
   fs/f2fs/compress.c |  5 +
   fs/f2fs/f2fs.h |  7 +++
   fs/f2fs/file.c | 52 ++
   3 files changed, 64 insertions(+)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 7895186cc765..816d7adc914c 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -514,6 +514,11 @@ bool f2fs_is_compress_backend_ready(struct inode *inode)
   return f2fs_cops[F2FS_I(inode)->i_compress_algorithm];
   }

+bool f2fs_is_compress_algorithm_ready(unsigned char algorithm)
+{
+ return algorithm < COMPRESS_MAX && f2fs_cops[algorithm] != NULL;
+}
+
   static mempool_t *compress_page_pool;
   static int num_compress_pages = 512;
   module_param(num_compress_pages, uint, 0444);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index a33c90cf979b..cc38afde6c04 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -435,6 +435,8 @@ static inline bool __has_cursum_space(struct f2fs_journal 
*journal,
   struct f2fs_sectrim_range)
   #define F2FS_IOC_GET_COMPRESS_OPTION_IOR(F2FS_IOCTL_MAGIC, 21,  \
   struct f2fs_comp_option)
+#define F2FS_IOC_SET_COMPRESS_OPTION _IOW(F2FS_IOCTL_MAGIC, 22,  \
+ struct f2fs_comp_option)

   /*
* should be same as XFS_IOC_GOINGDOWN.
@@ -3915,6 +3917,7 @@ bool f2fs_compress_write_end(struct inode *inode, void 
*fsdata,
   int f2fs_truncate_partial_cluster(struct inode *inode, u64 from, bool lock);
   void f2fs_compress_write_end_io(struct bio *bio, struct page *page);
   bool f2fs_is_compress_backend_ready(struct inode *inode);
+bool f2fs_is_compress_algorithm_ready(unsigned char algorithm);
   int f2fs_init_compress_mempool(void);
   void f2fs_destroy_compress_mempool(void);
   void f2fs_decompress_pages(struct bio *bio, struct page *page, bool verity);
@@ -3945,6 +3948,10 @@ static inline bool f2fs_is_compress_backend_ready(struct 
inode *inode)
   /* not support compression */
   return false;
   }
+static inline bool f2fs_is_compress_algorithm_ready(unsigned char algorithm)
+{
+ return false;
+}
   static inline struct page *f2fs_compress_control_page(struct page *page)
   {
   WARN_ON_ONCE(1);
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 8922ab191a9d..8048b150e43b 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -3963,6 +3963,55 @@ static int f2fs_ioc_get_compress_option(struct file 
*filp, unsigned long arg)
   return 0;
   }

+static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long arg)
+{
+ struct inode *inode = file_inode(filp);
+ struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+ struct f2fs_comp_option option;
+ int ret = 0;
+
+ if (!f2fs_sb_has_compression(sbi))
+ return -EOPNOTSUPP;
+
+ if (!(filp->f_mode & FMODE_WRITE))
+ return -EBADF;
+
+ if (copy_from_user(&option, (struct f2fs_comp_option __user *)arg,
+ sizeof(option)))
+ return -EFAULT;
+
+ if (!f2fs_compressed_file(inode) ||
+ option.log_cluster_size < MIN_COMPRESS_LOG_SIZE ||
+ option.log_cluster_size > MAX_COMPRESS_LOG_SIZE)
+ return -EINVAL;
+
+ if (!f2fs_is_compress_algorithm_ready(option.algorithm))
+ return -ENOPKG;


As we allow to mount image with different kernel which supports different 
compress
algorithms, so I guess we can support to change algorithm to one other which 
current
kernel doesn't support, since we have add f2fs_is_compress_backend_ready() in 
all
foreground operations to disallow user to operate such inode's data.

IMO, just add to print one warnning message is fine.

Thanks,


+
+ file_start_write(filp);
+ inode_lock(inode);
+
+ if (f

Re: [f2fs-dev] [PATCH v4 2/2] f2fs: add F2FS_IOC_SET_COMPRESS_OPTION ioctl

2020-10-28 Thread Chao Yu

On 2020/10/27 13:38, Daeho Jeong wrote:

From: Daeho Jeong 

Added a new F2FS_IOC_SET_COMPRESS_OPTION ioctl to change file
compression option of a file.

struct f2fs_comp_option {
 u8 algorithm; => compression algorithm
   => 0:lzo, 1:lz4, 2:zstd, 3:lzorle
 u8 log_cluster_size;  => log scale cluster size
   => 2 ~ 8
};

struct f2fs_comp_option option;

option.algorithm = 1;
option.log_cluster_size = 7;

ioctl(fd, F2FS_IOC_SET_COMPRESS_OPTION, &option);

Signed-off-by: Daeho Jeong 
---

v4: changed commit message.
v3: changed the error number more specific.
 folded in fix for build breakage reported by kernel test robot
  and Dan Carpenter .
v2: added ioctl description.
---
  fs/f2fs/compress.c |  5 +
  fs/f2fs/f2fs.h |  7 +++
  fs/f2fs/file.c | 52 ++
  3 files changed, 64 insertions(+)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 7895186cc765..816d7adc914c 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -514,6 +514,11 @@ bool f2fs_is_compress_backend_ready(struct inode *inode)
return f2fs_cops[F2FS_I(inode)->i_compress_algorithm];
  }
  
+bool f2fs_is_compress_algorithm_ready(unsigned char algorithm)

+{
+   return algorithm < COMPRESS_MAX && f2fs_cops[algorithm] != NULL;
+}
+
  static mempool_t *compress_page_pool;
  static int num_compress_pages = 512;
  module_param(num_compress_pages, uint, 0444);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index a33c90cf979b..cc38afde6c04 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -435,6 +435,8 @@ static inline bool __has_cursum_space(struct f2fs_journal 
*journal,
struct f2fs_sectrim_range)
  #define F2FS_IOC_GET_COMPRESS_OPTION  _IOR(F2FS_IOCTL_MAGIC, 21,  \
struct f2fs_comp_option)
+#define F2FS_IOC_SET_COMPRESS_OPTION   _IOW(F2FS_IOCTL_MAGIC, 22,  \
+   struct f2fs_comp_option)
  
  /*

   * should be same as XFS_IOC_GOINGDOWN.
@@ -3915,6 +3917,7 @@ bool f2fs_compress_write_end(struct inode *inode, void 
*fsdata,
  int f2fs_truncate_partial_cluster(struct inode *inode, u64 from, bool lock);
  void f2fs_compress_write_end_io(struct bio *bio, struct page *page);
  bool f2fs_is_compress_backend_ready(struct inode *inode);
+bool f2fs_is_compress_algorithm_ready(unsigned char algorithm);
  int f2fs_init_compress_mempool(void);
  void f2fs_destroy_compress_mempool(void);
  void f2fs_decompress_pages(struct bio *bio, struct page *page, bool verity);
@@ -3945,6 +3948,10 @@ static inline bool f2fs_is_compress_backend_ready(struct 
inode *inode)
/* not support compression */
return false;
  }
+static inline bool f2fs_is_compress_algorithm_ready(unsigned char algorithm)
+{
+   return false;
+}
  static inline struct page *f2fs_compress_control_page(struct page *page)
  {
WARN_ON_ONCE(1);
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 8922ab191a9d..8048b150e43b 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -3963,6 +3963,55 @@ static int f2fs_ioc_get_compress_option(struct file 
*filp, unsigned long arg)
return 0;
  }
  
+static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long arg)

+{
+   struct inode *inode = file_inode(filp);
+   struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+   struct f2fs_comp_option option;
+   int ret = 0;
+
+   if (!f2fs_sb_has_compression(sbi))
+   return -EOPNOTSUPP;
+
+   if (!(filp->f_mode & FMODE_WRITE))
+   return -EBADF;
+
+   if (copy_from_user(&option, (struct f2fs_comp_option __user *)arg,
+   sizeof(option)))
+   return -EFAULT;
+
+   if (!f2fs_compressed_file(inode) ||
+   option.log_cluster_size < MIN_COMPRESS_LOG_SIZE ||
+   option.log_cluster_size > MAX_COMPRESS_LOG_SIZE)
+   return -EINVAL;
+
+   if (!f2fs_is_compress_algorithm_ready(option.algorithm))
+   return -ENOPKG;


As we allow to mount image with different kernel which supports different 
compress
algorithms, so I guess we can support to change algorithm to one other which 
current
kernel doesn't support, since we have add f2fs_is_compress_backend_ready() in 
all
foreground operations to disallow user to operate such inode's data.

IMO, just add to print one warnning message is fine.

Thanks,


+
+   file_start_write(filp);
+   inode_lock(inode);
+
+   if (f2fs_is_mmap_file(inode) || get_dirty_pages(inode)) {
+   ret = -EBUSY;
+   goto out;
+   }
+
+   if (inode->i_size != 0) {
+   ret = -EFBIG;
+   goto out;
+   }
+
+   F2FS_I(inode)->i_compress_algorithm = option.algorithm;
+   F2FS_I(inode)->i_log_cluster_size = option.log_cluster_size;
+   F2FS_I

Re: [f2fs-dev] [PATCH v4 2/2] f2fs: add F2FS_IOC_SET_COMPRESS_OPTION ioctl

2020-10-28 Thread Daeho Jeong
Chao,

Do you want to print out a kernel warning message in this case? like
"XX compression algorithm is set for this inode, but current mount
option doesn't support this algorithm."?

2020년 10월 28일 (수) 오후 3:47, Chao Yu 님이 작성:
>
> On 2020/10/27 13:38, Daeho Jeong wrote:
> > From: Daeho Jeong 
> >
> > Added a new F2FS_IOC_SET_COMPRESS_OPTION ioctl to change file
> > compression option of a file.
> >
> > struct f2fs_comp_option {
> >  u8 algorithm; => compression algorithm
> >=> 0:lzo, 1:lz4, 2:zstd, 3:lzorle
> >  u8 log_cluster_size;  => log scale cluster size
> >=> 2 ~ 8
> > };
> >
> > struct f2fs_comp_option option;
> >
> > option.algorithm = 1;
> > option.log_cluster_size = 7;
> >
> > ioctl(fd, F2FS_IOC_SET_COMPRESS_OPTION, &option);
> >
> > Signed-off-by: Daeho Jeong 
> > ---
> >
> > v4: changed commit message.
> > v3: changed the error number more specific.
> >  folded in fix for build breakage reported by kernel test robot
> >   and Dan Carpenter .
> > v2: added ioctl description.
> > ---
> >   fs/f2fs/compress.c |  5 +
> >   fs/f2fs/f2fs.h |  7 +++
> >   fs/f2fs/file.c | 52 ++
> >   3 files changed, 64 insertions(+)
> >
> > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> > index 7895186cc765..816d7adc914c 100644
> > --- a/fs/f2fs/compress.c
> > +++ b/fs/f2fs/compress.c
> > @@ -514,6 +514,11 @@ bool f2fs_is_compress_backend_ready(struct inode 
> > *inode)
> >   return f2fs_cops[F2FS_I(inode)->i_compress_algorithm];
> >   }
> >
> > +bool f2fs_is_compress_algorithm_ready(unsigned char algorithm)
> > +{
> > + return algorithm < COMPRESS_MAX && f2fs_cops[algorithm] != NULL;
> > +}
> > +
> >   static mempool_t *compress_page_pool;
> >   static int num_compress_pages = 512;
> >   module_param(num_compress_pages, uint, 0444);
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index a33c90cf979b..cc38afde6c04 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -435,6 +435,8 @@ static inline bool __has_cursum_space(struct 
> > f2fs_journal *journal,
> >   struct f2fs_sectrim_range)
> >   #define F2FS_IOC_GET_COMPRESS_OPTION_IOR(F2FS_IOCTL_MAGIC, 21,
> >   \
> >   struct f2fs_comp_option)
> > +#define F2FS_IOC_SET_COMPRESS_OPTION _IOW(F2FS_IOCTL_MAGIC, 22,  \
> > + struct f2fs_comp_option)
> >
> >   /*
> >* should be same as XFS_IOC_GOINGDOWN.
> > @@ -3915,6 +3917,7 @@ bool f2fs_compress_write_end(struct inode *inode, 
> > void *fsdata,
> >   int f2fs_truncate_partial_cluster(struct inode *inode, u64 from, bool 
> > lock);
> >   void f2fs_compress_write_end_io(struct bio *bio, struct page *page);
> >   bool f2fs_is_compress_backend_ready(struct inode *inode);
> > +bool f2fs_is_compress_algorithm_ready(unsigned char algorithm);
> >   int f2fs_init_compress_mempool(void);
> >   void f2fs_destroy_compress_mempool(void);
> >   void f2fs_decompress_pages(struct bio *bio, struct page *page, bool 
> > verity);
> > @@ -3945,6 +3948,10 @@ static inline bool 
> > f2fs_is_compress_backend_ready(struct inode *inode)
> >   /* not support compression */
> >   return false;
> >   }
> > +static inline bool f2fs_is_compress_algorithm_ready(unsigned char 
> > algorithm)
> > +{
> > + return false;
> > +}
> >   static inline struct page *f2fs_compress_control_page(struct page *page)
> >   {
> >   WARN_ON_ONCE(1);
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index 8922ab191a9d..8048b150e43b 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -3963,6 +3963,55 @@ static int f2fs_ioc_get_compress_option(struct file 
> > *filp, unsigned long arg)
> >   return 0;
> >   }
> >
> > +static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long 
> > arg)
> > +{
> > + struct inode *inode = file_inode(filp);
> > + struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> > + struct f2fs_comp_option option;
> > + int ret = 0;
> > +
> > + if (!f2fs_sb_has_compression(sbi))
> > + return -EOPNOTSUPP;
> > +
> > + if (!(filp->f_mode & FMODE_WRITE))
> > + return -EBADF;
> > +
> > + if (copy_from_user(&option, (struct f2fs_comp_option __user *)arg,
> > + sizeof(option)))
> > + return -EFAULT;
> > +
> > + if (!f2fs_compressed_file(inode) ||
> > + option.log_cluster_size < MIN_COMPRESS_LOG_SIZE ||
> > + option.log_cluster_size > MAX_COMPRESS_LOG_SIZE)
> > + return -EINVAL;
> > +
> > + if (!f2fs_is_compress_algorithm_ready(option.algorithm))
> > + return -ENOPKG;
>
> As we allow to mount image with different kernel which supports different 
> compress
> algorithms, so I guess we can support to change algorithm to one other which 
> curren

[PATCH v4 2/2] f2fs: add F2FS_IOC_SET_COMPRESS_OPTION ioctl

2020-10-26 Thread Daeho Jeong
From: Daeho Jeong 

Added a new F2FS_IOC_SET_COMPRESS_OPTION ioctl to change file
compression option of a file.

struct f2fs_comp_option {
u8 algorithm; => compression algorithm
  => 0:lzo, 1:lz4, 2:zstd, 3:lzorle
u8 log_cluster_size;  => log scale cluster size
  => 2 ~ 8
};

struct f2fs_comp_option option;

option.algorithm = 1;
option.log_cluster_size = 7;

ioctl(fd, F2FS_IOC_SET_COMPRESS_OPTION, &option);

Signed-off-by: Daeho Jeong 
---

v4: changed commit message.
v3: changed the error number more specific.
folded in fix for build breakage reported by kernel test robot
 and Dan Carpenter .
v2: added ioctl description.
---
 fs/f2fs/compress.c |  5 +
 fs/f2fs/f2fs.h |  7 +++
 fs/f2fs/file.c | 52 ++
 3 files changed, 64 insertions(+)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 7895186cc765..816d7adc914c 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -514,6 +514,11 @@ bool f2fs_is_compress_backend_ready(struct inode *inode)
return f2fs_cops[F2FS_I(inode)->i_compress_algorithm];
 }
 
+bool f2fs_is_compress_algorithm_ready(unsigned char algorithm)
+{
+   return algorithm < COMPRESS_MAX && f2fs_cops[algorithm] != NULL;
+}
+
 static mempool_t *compress_page_pool;
 static int num_compress_pages = 512;
 module_param(num_compress_pages, uint, 0444);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index a33c90cf979b..cc38afde6c04 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -435,6 +435,8 @@ static inline bool __has_cursum_space(struct f2fs_journal 
*journal,
struct f2fs_sectrim_range)
 #define F2FS_IOC_GET_COMPRESS_OPTION   _IOR(F2FS_IOCTL_MAGIC, 21,  \
struct f2fs_comp_option)
+#define F2FS_IOC_SET_COMPRESS_OPTION   _IOW(F2FS_IOCTL_MAGIC, 22,  \
+   struct f2fs_comp_option)
 
 /*
  * should be same as XFS_IOC_GOINGDOWN.
@@ -3915,6 +3917,7 @@ bool f2fs_compress_write_end(struct inode *inode, void 
*fsdata,
 int f2fs_truncate_partial_cluster(struct inode *inode, u64 from, bool lock);
 void f2fs_compress_write_end_io(struct bio *bio, struct page *page);
 bool f2fs_is_compress_backend_ready(struct inode *inode);
+bool f2fs_is_compress_algorithm_ready(unsigned char algorithm);
 int f2fs_init_compress_mempool(void);
 void f2fs_destroy_compress_mempool(void);
 void f2fs_decompress_pages(struct bio *bio, struct page *page, bool verity);
@@ -3945,6 +3948,10 @@ static inline bool f2fs_is_compress_backend_ready(struct 
inode *inode)
/* not support compression */
return false;
 }
+static inline bool f2fs_is_compress_algorithm_ready(unsigned char algorithm)
+{
+   return false;
+}
 static inline struct page *f2fs_compress_control_page(struct page *page)
 {
WARN_ON_ONCE(1);
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 8922ab191a9d..8048b150e43b 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -3963,6 +3963,55 @@ static int f2fs_ioc_get_compress_option(struct file 
*filp, unsigned long arg)
return 0;
 }
 
+static int f2fs_ioc_set_compress_option(struct file *filp, unsigned long arg)
+{
+   struct inode *inode = file_inode(filp);
+   struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+   struct f2fs_comp_option option;
+   int ret = 0;
+
+   if (!f2fs_sb_has_compression(sbi))
+   return -EOPNOTSUPP;
+
+   if (!(filp->f_mode & FMODE_WRITE))
+   return -EBADF;
+
+   if (copy_from_user(&option, (struct f2fs_comp_option __user *)arg,
+   sizeof(option)))
+   return -EFAULT;
+
+   if (!f2fs_compressed_file(inode) ||
+   option.log_cluster_size < MIN_COMPRESS_LOG_SIZE ||
+   option.log_cluster_size > MAX_COMPRESS_LOG_SIZE)
+   return -EINVAL;
+
+   if (!f2fs_is_compress_algorithm_ready(option.algorithm))
+   return -ENOPKG;
+
+   file_start_write(filp);
+   inode_lock(inode);
+
+   if (f2fs_is_mmap_file(inode) || get_dirty_pages(inode)) {
+   ret = -EBUSY;
+   goto out;
+   }
+
+   if (inode->i_size != 0) {
+   ret = -EFBIG;
+   goto out;
+   }
+
+   F2FS_I(inode)->i_compress_algorithm = option.algorithm;
+   F2FS_I(inode)->i_log_cluster_size = option.log_cluster_size;
+   F2FS_I(inode)->i_cluster_size = 1 << option.log_cluster_size;
+   f2fs_mark_inode_dirty_sync(inode, true);
+out:
+   inode_unlock(inode);
+   file_end_write(filp);
+
+   return ret;
+}
+
 long f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
if (unlikely(f2fs_cp_error(F2FS_I_SB(file_inode(filp)
@@ -4053,6 +4102,8 @@ long f2fs_ioctl(struct file *filp, unsigned int cmd, 
unsigned long arg)
return f2fs_s