[PATCH 0/2] zonefs fixes

[PATCH 0/2] zonefs fixes

[Damien Le Moal]

A couple of fixes:
- prevent use of sequerntial zone files as swap files
- Fix write offset initialization of asynchronous append write operation
  (for sequential files open with O_APPEND or aio writes issued with
  RWF_APPEND)

Damien Le Moal (2):
  zonefs: prevent use of seq files as swap file
  zonefs: Fix O_APPEND async write handling

 fs/zonefs/super.c | 92 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 82 insertions(+), 10 deletions(-)

-- 
2.30.2

[PATCH 1/2] zonefs: prevent use of seq files as swap file

[Damien Le Moal]

The sequential write constraint of sequential zone file prevent their
use as swap files. Only allow conventional zone files to be used as swap
files.

Fixes: 8dcc1a9d90c1 ("fs: New zonefs file system")
Cc: <stable@vger.kernel.org>
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 fs/zonefs/super.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c
index 0fe76f376dee..a3d074f98660 100644
--- a/fs/zonefs/super.c
+++ b/fs/zonefs/super.c
@@ -165,6 +165,21 @@ static int zonefs_writepages(struct address_space *mapping,
 	return iomap_writepages(mapping, wbc, &wpc, &zonefs_writeback_ops);
 }
 
+static int zonefs_swap_activate(struct swap_info_struct *sis,
+				struct file *swap_file, sector_t *span)
+{
+	struct inode *inode = file_inode(swap_file);
+	struct zonefs_inode_info *zi = ZONEFS_I(inode);
+
+	if (zi->i_ztype != ZONEFS_ZTYPE_CNV) {
+		zonefs_err(inode->i_sb,
+			   "swap file: not a conventional zone file\n");
+		return -EINVAL;
+	}
+
+	return iomap_swapfile_activate(sis, swap_file, span, &zonefs_iomap_ops);
+}
+
 static const struct address_space_operations zonefs_file_aops = {
 	.readpage		= zonefs_readpage,
 	.readahead		= zonefs_readahead,
@@ -177,6 +192,7 @@ static const struct address_space_operations zonefs_file_aops = {
 	.is_partially_uptodate	= iomap_is_partially_uptodate,
 	.error_remove_page	= generic_error_remove_page,
 	.direct_IO		= noop_direct_IO,
+	.swap_activate		= zonefs_swap_activate,
 };
 
 static void zonefs_update_stats(struct inode *inode, loff_t new_isize)
-- 
2.30.2

Re: [PATCH 1/2] zonefs: prevent use of seq files as swap file

[Johannes Thumshirn]

On 15/03/2021 04:49, Damien Le Moal wrote:
> The sequential write constraint of sequential zone file prevent their
> use as swap files. Only allow conventional zone files to be used as swap
> files.

That would be super useful to test in in zonefs tests as well. I can take
care if you want.

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH 1/2] zonefs: prevent use of seq files as swap file

[Damien Le Moal]

On 2021/03/15 15:46, Johannes Thumshirn wrote:
> On 15/03/2021 04:49, Damien Le Moal wrote:
>> The sequential write constraint of sequential zone file prevent their
>> use as swap files. Only allow conventional zone files to be used as swap
>> files.
> 
> That would be super useful to test in in zonefs tests as well. I can take
> care if you want.

Yep. Adding a test case for that.

> 
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> 


-- 
Damien Le Moal
Western Digital Research

[PATCH 2/2] zonefs: Fix O_APPEND async write handling

[Damien Le Moal]

zonefs updates the size of a sequential zone file inode only on
completion of direct writes. When executing asynchronous append writes
(with a file open with O_APPEND or using RWF_APPEND), the use of the
current inode size in generic_write_checks() to set an iocb offset thus
leads to unaligned write if an application issues an append write
operation with another write already being executed.

Fix this problem by introducing zonefs_write_checks() as a modified
version of generic_write_checks() using the file inode wp_offset for an
append write iocb offset. Also introduce zonefs_write_check_limits() to
replace generic_write_check_limits() call. This zonefs special helper
makes sure that the maximum file limit used is the maximum size of the
file being accessed.

Since zonefs_write_checks() already truncates the iov_iter, the calls
to iov_iter_truncate() in zonefs_file_dio_write() and
zonefs_file_buffered_write() are removed.

Fixes: 8dcc1a9d90c1 ("fs: New zonefs file system")
Cc: <stable@vger.kernel.org>
Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 fs/zonefs/super.c | 76 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 66 insertions(+), 10 deletions(-)

diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c
index a3d074f98660..81836a5b436e 100644
--- a/fs/zonefs/super.c
+++ b/fs/zonefs/super.c
@@ -743,6 +743,68 @@ static ssize_t zonefs_file_dio_append(struct kiocb *iocb, struct iov_iter *from)
 	return ret;
 }
 
+/*
+ * Do not exceed the LFS limits nor the file zone size. If pos is under the
+ * limit it becomes a short access. If it exceeds the limit, return -EFBIG.
+ */
+static loff_t zonefs_write_check_limits(struct file *file, loff_t pos,
+					loff_t count)
+{
+	struct inode *inode = file_inode(file);
+	struct zonefs_inode_info *zi = ZONEFS_I(inode);
+	loff_t limit = rlimit(RLIMIT_FSIZE);
+	loff_t max_size = zi->i_max_size;
+
+	if (limit != RLIM_INFINITY) {
+		if (pos >= limit) {
+			send_sig(SIGXFSZ, current, 0);
+			return -EFBIG;
+		}
+		count = min(count, limit - pos);
+	}
+
+	if (!(file->f_flags & O_LARGEFILE))
+		max_size = min_t(loff_t, MAX_NON_LFS, max_size);
+
+	if (unlikely(pos >= max_size))
+		return -EFBIG;
+
+	return min(count, max_size - pos);
+}
+
+static ssize_t zonefs_write_checks(struct kiocb *iocb, struct iov_iter *from)
+{
+	struct file *file = iocb->ki_filp;
+	struct inode *inode = file_inode(file);
+	struct zonefs_inode_info *zi = ZONEFS_I(inode);
+	loff_t count;
+
+	if (IS_SWAPFILE(inode))
+		return -ETXTBSY;
+
+	if (!iov_iter_count(from))
+		return 0;
+
+	if ((iocb->ki_flags & IOCB_NOWAIT) && !(iocb->ki_flags & IOCB_DIRECT))
+		return -EINVAL;
+
+	if (iocb->ki_flags & IOCB_APPEND) {
+		if (zi->i_ztype != ZONEFS_ZTYPE_SEQ)
+			return -EINVAL;
+		mutex_lock(&zi->i_truncate_mutex);
+		iocb->ki_pos = zi->i_wpoffset;
+		mutex_unlock(&zi->i_truncate_mutex);
+	}
+
+	count = zonefs_write_check_limits(file, iocb->ki_pos,
+					  iov_iter_count(from));
+	if (count < 0)
+		return count;
+
+	iov_iter_truncate(from, count);
+	return iov_iter_count(from);
+}
+
 /*
  * Handle direct writes. For sequential zone files, this is the only possible
  * write path. For these files, check that the user is issuing writes
@@ -760,8 +822,7 @@ static ssize_t zonefs_file_dio_write(struct kiocb *iocb, struct iov_iter *from)
 	struct super_block *sb = inode->i_sb;
 	bool sync = is_sync_kiocb(iocb);
 	bool append = false;
-	size_t count;
-	ssize_t ret;
+	ssize_t ret, count;
 
 	/*
 	 * For async direct IOs to sequential zone files, refuse IOCB_NOWAIT
@@ -779,13 +840,10 @@ static ssize_t zonefs_file_dio_write(struct kiocb *iocb, struct iov_iter *from)
 		inode_lock(inode);
 	}
 
-	ret = generic_write_checks(iocb, from);
-	if (ret <= 0)
+	count = zonefs_write_checks(iocb, from);
+	if (count <= 0)
 		goto inode_unlock;
 
-	iov_iter_truncate(from, zi->i_max_size - iocb->ki_pos);
-	count = iov_iter_count(from);
-
 	if ((iocb->ki_pos | count) & (sb->s_blocksize - 1)) {
 		ret = -EINVAL;
 		goto inode_unlock;
@@ -844,12 +902,10 @@ static ssize_t zonefs_file_buffered_write(struct kiocb *iocb,
 		inode_lock(inode);
 	}
 
-	ret = generic_write_checks(iocb, from);
+	ret = zonefs_write_checks(iocb, from);
 	if (ret <= 0)
 		goto inode_unlock;
 
-	iov_iter_truncate(from, zi->i_max_size - iocb->ki_pos);
-
 	ret = iomap_file_buffered_write(iocb, from, &zonefs_iomap_ops);
 	if (ret > 0)
 		iocb->ki_pos += ret;
-- 
2.30.2

Re: [PATCH 2/2] zonefs: Fix O_APPEND async write handling

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn <jhannes.thumshirn@wdc.com>

Re: [PATCH 2/2] zonefs: Fix O_APPEND async write handling

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 5167 bytes --]

Hi Damien,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.12-rc3 next-20210315]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Damien-Le-Moal/zonefs-fixes/20210315-115123
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 1e28eed17697bcf343c6743f0028cc3b5dd88bf0
config: x86_64-randconfig-a013-20210315 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project a28facba1ccdc957f386b7753f4958307f1bfde8)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/1e8bb76723bbf4072c032334104026a611b2634e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Damien-Le-Moal/zonefs-fixes/20210315-115123
        git checkout 1e8bb76723bbf4072c032334104026a611b2634e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/zonefs/super.c:844:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (count <= 0)
               ^~~~~~~~~~
   fs/zonefs/super.c:881:9: note: uninitialized use occurs here
           return ret;
                  ^~~
   fs/zonefs/super.c:844:2: note: remove the 'if' if its condition is always false
           if (count <= 0)
           ^~~~~~~~~~~~~~~
   fs/zonefs/super.c:825:13: note: initialize the variable 'ret' to silence this warning
           ssize_t ret, count;
                      ^
                       = 0
   1 warning generated.


vim +844 fs/zonefs/super.c

   807	
   808	/*
   809	 * Handle direct writes. For sequential zone files, this is the only possible
   810	 * write path. For these files, check that the user is issuing writes
   811	 * sequentially from the end of the file. This code assumes that the block layer
   812	 * delivers write requests to the device in sequential order. This is always the
   813	 * case if a block IO scheduler implementing the ELEVATOR_F_ZBD_SEQ_WRITE
   814	 * elevator feature is being used (e.g. mq-deadline). The block layer always
   815	 * automatically select such an elevator for zoned block devices during the
   816	 * device initialization.
   817	 */
   818	static ssize_t zonefs_file_dio_write(struct kiocb *iocb, struct iov_iter *from)
   819	{
   820		struct inode *inode = file_inode(iocb->ki_filp);
   821		struct zonefs_inode_info *zi = ZONEFS_I(inode);
   822		struct super_block *sb = inode->i_sb;
   823		bool sync = is_sync_kiocb(iocb);
   824		bool append = false;
   825		ssize_t ret, count;
   826	
   827		/*
   828		 * For async direct IOs to sequential zone files, refuse IOCB_NOWAIT
   829		 * as this can cause write reordering (e.g. the first aio gets EAGAIN
   830		 * on the inode lock but the second goes through but is now unaligned).
   831		 */
   832		if (zi->i_ztype == ZONEFS_ZTYPE_SEQ && !sync &&
   833		    (iocb->ki_flags & IOCB_NOWAIT))
   834			return -EOPNOTSUPP;
   835	
   836		if (iocb->ki_flags & IOCB_NOWAIT) {
   837			if (!inode_trylock(inode))
   838				return -EAGAIN;
   839		} else {
   840			inode_lock(inode);
   841		}
   842	
   843		count = zonefs_write_checks(iocb, from);
 > 844		if (count <= 0)
   845			goto inode_unlock;
   846	
   847		if ((iocb->ki_pos | count) & (sb->s_blocksize - 1)) {
   848			ret = -EINVAL;
   849			goto inode_unlock;
   850		}
   851	
   852		/* Enforce sequential writes (append only) in sequential zones */
   853		if (zi->i_ztype == ZONEFS_ZTYPE_SEQ) {
   854			mutex_lock(&zi->i_truncate_mutex);
   855			if (iocb->ki_pos != zi->i_wpoffset) {
   856				mutex_unlock(&zi->i_truncate_mutex);
   857				ret = -EINVAL;
   858				goto inode_unlock;
   859			}
   860			mutex_unlock(&zi->i_truncate_mutex);
   861			append = sync;
   862		}
   863	
   864		if (append)
   865			ret = zonefs_file_dio_append(iocb, from);
   866		else
   867			ret = iomap_dio_rw(iocb, from, &zonefs_iomap_ops,
   868					   &zonefs_write_dio_ops, 0);
   869		if (zi->i_ztype == ZONEFS_ZTYPE_SEQ &&
   870		    (ret > 0 || ret == -EIOCBQUEUED)) {
   871			if (ret > 0)
   872				count = ret;
   873			mutex_lock(&zi->i_truncate_mutex);
   874			zi->i_wpoffset += count;
   875			mutex_unlock(&zi->i_truncate_mutex);
   876		}
   877	
   878	inode_unlock:
   879		inode_unlock(inode);
   880	
   881		return ret;
   882	}
   883	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 34930 bytes --]

Re: [PATCH 2/2] zonefs: Fix O_APPEND async write handling

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 5299 bytes --]

Hi Damien,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.12-rc3 next-20210315]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Damien-Le-Moal/zonefs-fixes/20210315-115123
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 1e28eed17697bcf343c6743f0028cc3b5dd88bf0
config: x86_64-randconfig-a013-20210315 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project a28facba1ccdc957f386b7753f4958307f1bfde8)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/1e8bb76723bbf4072c032334104026a611b2634e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Damien-Le-Moal/zonefs-fixes/20210315-115123
        git checkout 1e8bb76723bbf4072c032334104026a611b2634e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> fs/zonefs/super.c:844:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (count <= 0)
               ^~~~~~~~~~
   fs/zonefs/super.c:881:9: note: uninitialized use occurs here
           return ret;
                  ^~~
   fs/zonefs/super.c:844:2: note: remove the 'if' if its condition is always false
           if (count <= 0)
           ^~~~~~~~~~~~~~~
   fs/zonefs/super.c:825:13: note: initialize the variable 'ret' to silence this warning
           ssize_t ret, count;
                      ^
                       = 0
   1 warning generated.


vim +844 fs/zonefs/super.c

   807	
   808	/*
   809	 * Handle direct writes. For sequential zone files, this is the only possible
   810	 * write path. For these files, check that the user is issuing writes
   811	 * sequentially from the end of the file. This code assumes that the block layer
   812	 * delivers write requests to the device in sequential order. This is always the
   813	 * case if a block IO scheduler implementing the ELEVATOR_F_ZBD_SEQ_WRITE
   814	 * elevator feature is being used (e.g. mq-deadline). The block layer always
   815	 * automatically select such an elevator for zoned block devices during the
   816	 * device initialization.
   817	 */
   818	static ssize_t zonefs_file_dio_write(struct kiocb *iocb, struct iov_iter *from)
   819	{
   820		struct inode *inode = file_inode(iocb->ki_filp);
   821		struct zonefs_inode_info *zi = ZONEFS_I(inode);
   822		struct super_block *sb = inode->i_sb;
   823		bool sync = is_sync_kiocb(iocb);
   824		bool append = false;
   825		ssize_t ret, count;
   826	
   827		/*
   828		 * For async direct IOs to sequential zone files, refuse IOCB_NOWAIT
   829		 * as this can cause write reordering (e.g. the first aio gets EAGAIN
   830		 * on the inode lock but the second goes through but is now unaligned).
   831		 */
   832		if (zi->i_ztype == ZONEFS_ZTYPE_SEQ && !sync &&
   833		    (iocb->ki_flags & IOCB_NOWAIT))
   834			return -EOPNOTSUPP;
   835	
   836		if (iocb->ki_flags & IOCB_NOWAIT) {
   837			if (!inode_trylock(inode))
   838				return -EAGAIN;
   839		} else {
   840			inode_lock(inode);
   841		}
   842	
   843		count = zonefs_write_checks(iocb, from);
 > 844		if (count <= 0)
   845			goto inode_unlock;
   846	
   847		if ((iocb->ki_pos | count) & (sb->s_blocksize - 1)) {
   848			ret = -EINVAL;
   849			goto inode_unlock;
   850		}
   851	
   852		/* Enforce sequential writes (append only) in sequential zones */
   853		if (zi->i_ztype == ZONEFS_ZTYPE_SEQ) {
   854			mutex_lock(&zi->i_truncate_mutex);
   855			if (iocb->ki_pos != zi->i_wpoffset) {
   856				mutex_unlock(&zi->i_truncate_mutex);
   857				ret = -EINVAL;
   858				goto inode_unlock;
   859			}
   860			mutex_unlock(&zi->i_truncate_mutex);
   861			append = sync;
   862		}
   863	
   864		if (append)
   865			ret = zonefs_file_dio_append(iocb, from);
   866		else
   867			ret = iomap_dio_rw(iocb, from, &zonefs_iomap_ops,
   868					   &zonefs_write_dio_ops, 0);
   869		if (zi->i_ztype == ZONEFS_ZTYPE_SEQ &&
   870		    (ret > 0 || ret == -EIOCBQUEUED)) {
   871			if (ret > 0)
   872				count = ret;
   873			mutex_lock(&zi->i_truncate_mutex);
   874			zi->i_wpoffset += count;
   875			mutex_unlock(&zi->i_truncate_mutex);
   876		}
   877	
   878	inode_unlock:
   879		inode_unlock(inode);
   880	
   881		return ret;
   882	}
   883	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 34930 bytes --]

Re: [PATCH 2/2] zonefs: Fix O_APPEND async write handling

[Johannes Thumshirn]

[-- Attachment #1: Type: text/plain, Size: 774 bytes --]

On 15/03/2021 08:16, kernel test robot wrote:
> 818	static ssize_t zonefs_file_dio_write(struct kiocb *iocb, struct iov_iter *from)
>    819	{
>    820		struct inode *inode = file_inode(iocb->ki_filp);
>    821		struct zonefs_inode_info *zi = ZONEFS_I(inode);
>    822		struct super_block *sb = inode->i_sb;
>    823		bool sync = is_sync_kiocb(iocb);
>    824		bool append = false;
>    825		ssize_t ret, count;

>    843		count = zonefs_write_checks(iocb, from);
>  > 844		if (count <= 0)
>    845			goto inode_unlock;

Args that needs to be:
			if (count <= 0) {
				ret = count;
				goto inode_unlock;
			}

Sorry for not spotting it.

>    878	inode_unlock:
>    879		inode_unlock(inode);
>    880	
>    881		return ret;
>    882	}
>    883	

Re: [PATCH 2/2] zonefs: Fix O_APPEND async write handling

[Johannes Thumshirn]

On 15/03/2021 08:16, kernel test robot wrote:
> 818	static ssize_t zonefs_file_dio_write(struct kiocb *iocb, struct iov_iter *from)
>    819	{
>    820		struct inode *inode = file_inode(iocb->ki_filp);
>    821		struct zonefs_inode_info *zi = ZONEFS_I(inode);
>    822		struct super_block *sb = inode->i_sb;
>    823		bool sync = is_sync_kiocb(iocb);
>    824		bool append = false;
>    825		ssize_t ret, count;

>    843		count = zonefs_write_checks(iocb, from);
>  > 844		if (count <= 0)
>    845			goto inode_unlock;

Args that needs to be:
			if (count <= 0) {
				ret = count;
				goto inode_unlock;
			}

Sorry for not spotting it.

>    878	inode_unlock:
>    879		inode_unlock(inode);
>    880	
>    881		return ret;
>    882	}
>    883	

Re: [PATCH 2/2] zonefs: Fix O_APPEND async write handling

[Damien Le Moal]

[-- Attachment #1: Type: text/plain, Size: 992 bytes --]

On 2021/03/15 16:21, Johannes Thumshirn wrote:
> On 15/03/2021 08:16, kernel test robot wrote:
>> 818	static ssize_t zonefs_file_dio_write(struct kiocb *iocb, struct iov_iter *from)
>>    819	{
>>    820		struct inode *inode = file_inode(iocb->ki_filp);
>>    821		struct zonefs_inode_info *zi = ZONEFS_I(inode);
>>    822		struct super_block *sb = inode->i_sb;
>>    823		bool sync = is_sync_kiocb(iocb);
>>    824		bool append = false;
>>    825		ssize_t ret, count;
> 
>>    843		count = zonefs_write_checks(iocb, from);
>>  > 844		if (count <= 0)
>>    845			goto inode_unlock;
> 
> Args that needs to be:
> 			if (count <= 0) {
> 				ret = count;
> 				goto inode_unlock;
> 			}
> 
> Sorry for not spotting it.

Yep. Sending v2. Weird that gcc does not complain on my local compile...

> 
>>    878	inode_unlock:
>>    879		inode_unlock(inode);
>>    880	
>>    881		return ret;
>>    882	}
>>    883	
> 

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 2/2] zonefs: Fix O_APPEND async write handling

[Damien Le Moal]

On 2021/03/15 16:21, Johannes Thumshirn wrote:
> On 15/03/2021 08:16, kernel test robot wrote:
>> 818	static ssize_t zonefs_file_dio_write(struct kiocb *iocb, struct iov_iter *from)
>>    819	{
>>    820		struct inode *inode = file_inode(iocb->ki_filp);
>>    821		struct zonefs_inode_info *zi = ZONEFS_I(inode);
>>    822		struct super_block *sb = inode->i_sb;
>>    823		bool sync = is_sync_kiocb(iocb);
>>    824		bool append = false;
>>    825		ssize_t ret, count;
> 
>>    843		count = zonefs_write_checks(iocb, from);
>>  > 844		if (count <= 0)
>>    845			goto inode_unlock;
> 
> Args that needs to be:
> 			if (count <= 0) {
> 				ret = count;
> 				goto inode_unlock;
> 			}
> 
> Sorry for not spotting it.

Yep. Sending v2. Weird that gcc does not complain on my local compile...

> 
>>    878	inode_unlock:
>>    879		inode_unlock(inode);
>>    880	
>>    881		return ret;
>>    882	}
>>    883	
> 

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 2/2] zonefs: Fix O_APPEND async write handling

[Nathan Chancellor]

On Mon, Mar 15, 2021 at 07:22:56AM +0000, Damien Le Moal wrote:
> On 2021/03/15 16:21, Johannes Thumshirn wrote:
> > On 15/03/2021 08:16, kernel test robot wrote:
> >> 818	static ssize_t zonefs_file_dio_write(struct kiocb *iocb, struct iov_iter *from)
> >>    819	{
> >>    820		struct inode *inode = file_inode(iocb->ki_filp);
> >>    821		struct zonefs_inode_info *zi = ZONEFS_I(inode);
> >>    822		struct super_block *sb = inode->i_sb;
> >>    823		bool sync = is_sync_kiocb(iocb);
> >>    824		bool append = false;
> >>    825		ssize_t ret, count;
> > 
> >>    843		count = zonefs_write_checks(iocb, from);
> >>  > 844		if (count <= 0)
> >>    845			goto inode_unlock;
> > 
> > Args that needs to be:
> > 			if (count <= 0) {
> > 				ret = count;
> > 				goto inode_unlock;
> > 			}
> > 
> > Sorry for not spotting it.
> 
> Yep. Sending v2. Weird that gcc does not complain on my local compile...

Unfortunately, GCC's version of this warning was disabled for default
compiles by Linus in commit 78a5255ffb6a ("Stop the ad-hoc games with
-Wno-maybe-initialized"). W=2 is required, which can be quite noisy from
my understanding. KCFLAGS=-Wmaybe-uninitialized is a good option.

Cheers,
Nathan

> > 
> >>    878	inode_unlock:
> >>    879		inode_unlock(inode);
> >>    880	
> >>    881		return ret;
> >>    882	}
> >>    883	
> > 
> 
> -- 
> Damien Le Moal
> Western Digital Research

Re: [PATCH 2/2] zonefs: Fix O_APPEND async write handling

[Nathan Chancellor]

[-- Attachment #1: Type: text/plain, Size: 1431 bytes --]

On Mon, Mar 15, 2021 at 07:22:56AM +0000, Damien Le Moal wrote:
> On 2021/03/15 16:21, Johannes Thumshirn wrote:
> > On 15/03/2021 08:16, kernel test robot wrote:
> >> 818	static ssize_t zonefs_file_dio_write(struct kiocb *iocb, struct iov_iter *from)
> >>    819	{
> >>    820		struct inode *inode = file_inode(iocb->ki_filp);
> >>    821		struct zonefs_inode_info *zi = ZONEFS_I(inode);
> >>    822		struct super_block *sb = inode->i_sb;
> >>    823		bool sync = is_sync_kiocb(iocb);
> >>    824		bool append = false;
> >>    825		ssize_t ret, count;
> > 
> >>    843		count = zonefs_write_checks(iocb, from);
> >>  > 844		if (count <= 0)
> >>    845			goto inode_unlock;
> > 
> > Args that needs to be:
> > 			if (count <= 0) {
> > 				ret = count;
> > 				goto inode_unlock;
> > 			}
> > 
> > Sorry for not spotting it.
> 
> Yep. Sending v2. Weird that gcc does not complain on my local compile...

Unfortunately, GCC's version of this warning was disabled for default
compiles by Linus in commit 78a5255ffb6a ("Stop the ad-hoc games with
-Wno-maybe-initialized"). W=2 is required, which can be quite noisy from
my understanding. KCFLAGS=-Wmaybe-uninitialized is a good option.

Cheers,
Nathan

> > 
> >>    878	inode_unlock:
> >>    879		inode_unlock(inode);
> >>    880	
> >>    881		return ret;
> >>    882	}
> >>    883	
> > 
> 
> -- 
> Damien Le Moal
> Western Digital Research

Re: [PATCH 2/2] zonefs: Fix O_APPEND async write handling

[Damien Le Moal]

[-- Attachment #1: Type: text/plain, Size: 1384 bytes --]

On 2021/03/16 2:09, Nathan Chancellor wrote:
> On Mon, Mar 15, 2021 at 07:22:56AM +0000, Damien Le Moal wrote:
>> On 2021/03/15 16:21, Johannes Thumshirn wrote:
>>> On 15/03/2021 08:16, kernel test robot wrote:
>>>> 818	static ssize_t zonefs_file_dio_write(struct kiocb *iocb, struct iov_iter *from)
>>>>    819	{
>>>>    820		struct inode *inode = file_inode(iocb->ki_filp);
>>>>    821		struct zonefs_inode_info *zi = ZONEFS_I(inode);
>>>>    822		struct super_block *sb = inode->i_sb;
>>>>    823		bool sync = is_sync_kiocb(iocb);
>>>>    824		bool append = false;
>>>>    825		ssize_t ret, count;
>>>
>>>>    843		count = zonefs_write_checks(iocb, from);
>>>>  > 844		if (count <= 0)
>>>>    845			goto inode_unlock;
>>>
>>> Args that needs to be:
>>> 			if (count <= 0) {
>>> 				ret = count;
>>> 				goto inode_unlock;
>>> 			}
>>>
>>> Sorry for not spotting it.
>>
>> Yep. Sending v2. Weird that gcc does not complain on my local compile...
> 
> Unfortunately, GCC's version of this warning was disabled for default
> compiles by Linus in commit 78a5255ffb6a ("Stop the ad-hoc games with
> -Wno-maybe-initialized"). W=2 is required, which can be quite noisy from
> my understanding. KCFLAGS=-Wmaybe-uninitialized is a good option.

I was not aware of that change. Thanks for the information !


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 2/2] zonefs: Fix O_APPEND async write handling

[Damien Le Moal]

On 2021/03/16 2:09, Nathan Chancellor wrote:
> On Mon, Mar 15, 2021 at 07:22:56AM +0000, Damien Le Moal wrote:
>> On 2021/03/15 16:21, Johannes Thumshirn wrote:
>>> On 15/03/2021 08:16, kernel test robot wrote:
>>>> 818	static ssize_t zonefs_file_dio_write(struct kiocb *iocb, struct iov_iter *from)
>>>>    819	{
>>>>    820		struct inode *inode = file_inode(iocb->ki_filp);
>>>>    821		struct zonefs_inode_info *zi = ZONEFS_I(inode);
>>>>    822		struct super_block *sb = inode->i_sb;
>>>>    823		bool sync = is_sync_kiocb(iocb);
>>>>    824		bool append = false;
>>>>    825		ssize_t ret, count;
>>>
>>>>    843		count = zonefs_write_checks(iocb, from);
>>>>  > 844		if (count <= 0)
>>>>    845			goto inode_unlock;
>>>
>>> Args that needs to be:
>>> 			if (count <= 0) {
>>> 				ret = count;
>>> 				goto inode_unlock;
>>> 			}
>>>
>>> Sorry for not spotting it.
>>
>> Yep. Sending v2. Weird that gcc does not complain on my local compile...
> 
> Unfortunately, GCC's version of this warning was disabled for default
> compiles by Linus in commit 78a5255ffb6a ("Stop the ad-hoc games with
> -Wno-maybe-initialized"). W=2 is required, which can be quite noisy from
> my understanding. KCFLAGS=-Wmaybe-uninitialized is a good option.

I was not aware of that change. Thanks for the information !


-- 
Damien Le Moal
Western Digital Research