lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <202210020436.9lyAheSp-lkp@intel.com>
Date:   Sun, 2 Oct 2022 04:39:35 +0800
From:   kernel test robot <lkp@...el.com>
To:     Jeff Layton <jlayton@...nel.org>, tytso@....edu,
        adilger.kernel@...ger.ca, djwong@...nel.org, david@...morbit.com,
        trondmy@...merspace.com, neilb@...e.de, viro@...iv.linux.org.uk,
        zohar@...ux.ibm.com, xiubli@...hat.com, chuck.lever@...cle.com,
        lczerner@...hat.com, jack@...e.cz, bfields@...ldses.org,
        brauner@...nel.org, fweimer@...hat.com
Cc:     kbuild-all@...ts.01.org, linux-btrfs@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        ceph-devel@...r.kernel.org, linux-ext4@...r.kernel.org,
        linux-nfs@...r.kernel.org, linux-xfs@...r.kernel.org
Subject: Re: [PATCH v6 8/9] vfs: update times after copying data in
 __generic_file_write_iter

Hi Jeff,

I love your patch! Perhaps something to improve:

[auto build test WARNING on next-20220929]
[cannot apply to tytso-ext4/dev trondmy-nfs/linux-next ceph-client/for-linus linus/master v6.0-rc7 v6.0-rc6 v6.0-rc5 v6.0-rc7]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jeff-Layton/vfs-nfsd-clean-up-handling-of-i_version-counter/20220930-192844
base:    1c6c4f42b3de4f18ea96d62950d0e266ca35a055
config: i386-randconfig-a001
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/b305ff6981591917824b59e5ac7f833afea77ce6
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jeff-Layton/vfs-nfsd-clean-up-handling-of-i_version-counter/20220930-192844
        git checkout b305ff6981591917824b59e5ac7f833afea77ce6
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@...el.com>

All warnings (new ones prefixed by >>):

   In file included from arch/x86/include/asm/bug.h:87,
                    from include/linux/bug.h:5,
                    from include/linux/thread_info.h:13,
                    from arch/x86/include/asm/preempt.h:7,
                    from include/linux/preempt.h:78,
                    from include/linux/spinlock.h:56,
                    from include/linux/wait.h:9,
                    from include/linux/wait_bit.h:8,
                    from include/linux/fs.h:6,
                    from include/linux/dax.h:5,
                    from mm/filemap.c:15:
   mm/filemap.c: In function '__generic_file_write_iter':
>> mm/filemap.c:3892:32: warning: format '%ld' expects argument of type 'long int', but argument 2 has type 'ssize_t' {aka 'int'} [-Wformat=]
    3892 |                 WARN_ONCE(err, "Failed to update m/ctime after write: %ld\n", err);
         |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  ~~~
         |                                                                               |
         |                                                                               ssize_t {aka int}
   include/asm-generic/bug.h:105:31: note: in definition of macro '__WARN_printf'
     105 |                 __warn_printk(arg);                                     \
         |                               ^~~
   include/linux/once_lite.h:31:25: note: in expansion of macro 'WARN'
      31 |                         func(__VA_ARGS__);                              \
         |                         ^~~~
   include/asm-generic/bug.h:151:9: note: in expansion of macro 'DO_ONCE_LITE_IF'
     151 |         DO_ONCE_LITE_IF(condition, WARN, 1, format)
         |         ^~~~~~~~~~~~~~~
   mm/filemap.c:3892:17: note: in expansion of macro 'WARN_ONCE'
    3892 |                 WARN_ONCE(err, "Failed to update m/ctime after write: %ld\n", err);
         |                 ^~~~~~~~~
   mm/filemap.c:3892:73: note: format string is defined here
    3892 |                 WARN_ONCE(err, "Failed to update m/ctime after write: %ld\n", err);
         |                                                                       ~~^
         |                                                                         |
         |                                                                         long int
         |                                                                       %d


vim +3892 mm/filemap.c

  3793	
  3794	/**
  3795	 * __generic_file_write_iter - write data to a file
  3796	 * @iocb:	IO state structure (file, offset, etc.)
  3797	 * @from:	iov_iter with data to write
  3798	 *
  3799	 * This function does all the work needed for actually writing data to a
  3800	 * file. It does all basic checks, removes SUID from the file, updates
  3801	 * modification times and calls proper subroutines depending on whether we
  3802	 * do direct IO or a standard buffered write.
  3803	 *
  3804	 * It expects i_rwsem to be grabbed unless we work on a block device or similar
  3805	 * object which does not need locking at all.
  3806	 *
  3807	 * This function does *not* take care of syncing data in case of O_SYNC write.
  3808	 * A caller has to handle it. This is mainly due to the fact that we want to
  3809	 * avoid syncing under i_rwsem.
  3810	 *
  3811	 * Return:
  3812	 * * number of bytes written, even for truncated writes
  3813	 * * negative error code if no data has been written at all
  3814	 */
  3815	ssize_t __generic_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
  3816	{
  3817		struct file *file = iocb->ki_filp;
  3818		struct address_space *mapping = file->f_mapping;
  3819		struct inode 	*inode = mapping->host;
  3820		ssize_t		written = 0;
  3821		ssize_t		err;
  3822		ssize_t		status;
  3823	
  3824		/* We can write back this queue in page reclaim */
  3825		current->backing_dev_info = inode_to_bdi(inode);
  3826		err = file_remove_privs(file);
  3827		if (err)
  3828			goto out;
  3829	
  3830		if (iocb->ki_flags & IOCB_DIRECT) {
  3831			loff_t pos, endbyte;
  3832	
  3833			written = generic_file_direct_write(iocb, from);
  3834			/*
  3835			 * If the write stopped short of completing, fall back to
  3836			 * buffered writes.  Some filesystems do this for writes to
  3837			 * holes, for example.  For DAX files, a buffered write will
  3838			 * not succeed (even if it did, DAX does not handle dirty
  3839			 * page-cache pages correctly).
  3840			 */
  3841			if (written < 0 || !iov_iter_count(from) || IS_DAX(inode))
  3842				goto out;
  3843	
  3844			pos = iocb->ki_pos;
  3845			status = generic_perform_write(iocb, from);
  3846			/*
  3847			 * If generic_perform_write() returned a synchronous error
  3848			 * then we want to return the number of bytes which were
  3849			 * direct-written, or the error code if that was zero.  Note
  3850			 * that this differs from normal direct-io semantics, which
  3851			 * will return -EFOO even if some bytes were written.
  3852			 */
  3853			if (unlikely(status < 0)) {
  3854				err = status;
  3855				goto out;
  3856			}
  3857			/*
  3858			 * We need to ensure that the page cache pages are written to
  3859			 * disk and invalidated to preserve the expected O_DIRECT
  3860			 * semantics.
  3861			 */
  3862			endbyte = pos + status - 1;
  3863			err = filemap_write_and_wait_range(mapping, pos, endbyte);
  3864			if (err == 0) {
  3865				iocb->ki_pos = endbyte + 1;
  3866				written += status;
  3867				invalidate_mapping_pages(mapping,
  3868							 pos >> PAGE_SHIFT,
  3869							 endbyte >> PAGE_SHIFT);
  3870			} else {
  3871				/*
  3872				 * We don't know how much we wrote, so just return
  3873				 * the number of bytes which were direct-written
  3874				 */
  3875			}
  3876		} else {
  3877			written = generic_perform_write(iocb, from);
  3878			if (likely(written > 0))
  3879				iocb->ki_pos += written;
  3880		}
  3881	out:
  3882		if (written > 0) {
  3883			err = file_update_time(file);
  3884			/*
  3885			 * There isn't much we can do at this point if updating the
  3886			 * times fails after a successful write. The times and i_version
  3887			 * should still be updated in the inode, and it should still be
  3888			 * marked dirty, so hopefully the next inode update will catch it.
  3889			 * Log a warning once so we have a record that something untoward
  3890			 * has occurred.
  3891			 */
> 3892			WARN_ONCE(err, "Failed to update m/ctime after write: %ld\n", err);
  3893		}
  3894	
  3895		current->backing_dev_info = NULL;
  3896		return written ? written : err;
  3897	}
  3898	EXPORT_SYMBOL(__generic_file_write_iter);
  3899	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

View attachment "config" of type "text/plain" (152328 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ