[PATCH] afs: Fix afs_write_end() when called with copied == 0 [ver #2]

[PATCH] afs: Fix afs_write_end() when called with copied == 0 [ver #2]

[David Howells]

When afs_write_end() is called with copied == 0, it tries to set the dirty
region, but there's no way to actually encode a 0-length region in the
encoding in page->private.  "0,0", for example, indicates a 1-byte region
at offset 0.  The maths miscalculates this and sets it incorrectly.

Fix it to just do nothing but unlock and put the page in this case.  We
don't actually need to mark the page dirty as nothing presumably changed.

Fixes: 65dd2d6072d3 ("afs: Alter dirty range encoding in page->private")
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/afs/write.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/afs/write.c b/fs/afs/write.c
index 50371207f327..07f29c5be771 100644
--- a/fs/afs/write.c
+++ b/fs/afs/write.c
@@ -169,11 +169,14 @@ int afs_write_end(struct file *file, struct address_space *mapping,
 	unsigned int f, from = pos & (PAGE_SIZE - 1);
 	unsigned int t, to = from + copied;
 	loff_t i_size, maybe_i_size;
-	int ret;
+	int ret = 0;
 
 	_enter("{%llx:%llu},{%lx}",
 	       vnode->fid.vid, vnode->fid.vnode, page->index);
 
+	if (copied == 0)
+		goto out;
+
 	maybe_i_size = pos + copied;
 
 	i_size = i_size_read(&vnode->vfs_inode);
@@ -196,7 +199,7 @@ int afs_write_end(struct file *file, struct address_space *mapping,
 			if (ret < 0)
 				goto out;
 		}
-		SetPageUptodate(page);
+		SetPageUptoodate(page);
 	}
 
 	if (PagePrivate(page)) {

Re: [PATCH] afs: Fix afs_write_end() when called with copied == 0 [ver #2]

[David Howells]

David Howells <dhowells@redhat.com> wrote:

> -		SetPageUptodate(page);
> +		SetPageUptoodate(page);

I'm not sure what happened there.  Will try again :-(

Re: [PATCH] afs: Fix afs_write_end() when called with copied == 0 [ver #2]

[kernel test robot]

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

Hi David,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on linux/master v5.10-rc3 next-20201113]
[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/David-Howells/afs-Fix-afs_write_end-when-called-with-copied-0-ver-2/20201115-012626
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git f01c30de86f1047e9bae1b1b1417b0ce8dcd15b1
config: microblaze-randconfig-r022-20201115 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 9.3.0
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
        # https://github.com/0day-ci/linux/commit/3486f1e413fba9587ced6c768d75e993ef78ce9d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review David-Howells/afs-Fix-afs_write_end-when-called-with-copied-0-ver-2/20201115-012626
        git checkout 3486f1e413fba9587ced6c768d75e993ef78ce9d
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=microblaze 

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

All errors (new ones prefixed by >>):

   fs/afs/write.c: In function 'afs_write_end':
>> fs/afs/write.c:202:3: error: implicit declaration of function 'SetPageUptoodate'; did you mean 'SetPageUptodate'? [-Werror=implicit-function-declaration]
     202 |   SetPageUptoodate(page);
         |   ^~~~~~~~~~~~~~~~
         |   SetPageUptodate
   cc1: some warnings being treated as errors

vim +202 fs/afs/write.c

   158	
   159	/*
   160	 * finalise part of a write to a page
   161	 */
   162	int afs_write_end(struct file *file, struct address_space *mapping,
   163			  loff_t pos, unsigned len, unsigned copied,
   164			  struct page *page, void *fsdata)
   165	{
   166		struct afs_vnode *vnode = AFS_FS_I(file_inode(file));
   167		struct key *key = afs_file_key(file);
   168		unsigned long priv;
   169		unsigned int f, from = pos & (PAGE_SIZE - 1);
   170		unsigned int t, to = from + copied;
   171		loff_t i_size, maybe_i_size;
   172		int ret = 0;
   173	
   174		_enter("{%llx:%llu},{%lx}",
   175		       vnode->fid.vid, vnode->fid.vnode, page->index);
   176	
   177		if (copied == 0)
   178			goto out;
   179	
   180		maybe_i_size = pos + copied;
   181	
   182		i_size = i_size_read(&vnode->vfs_inode);
   183		if (maybe_i_size > i_size) {
   184			write_seqlock(&vnode->cb_lock);
   185			i_size = i_size_read(&vnode->vfs_inode);
   186			if (maybe_i_size > i_size)
   187				i_size_write(&vnode->vfs_inode, maybe_i_size);
   188			write_sequnlock(&vnode->cb_lock);
   189		}
   190	
   191		if (!PageUptodate(page)) {
   192			if (copied < len) {
   193				/* Try and load any missing data from the server.  The
   194				 * unmarshalling routine will take care of clearing any
   195				 * bits that are beyond the EOF.
   196				 */
   197				ret = afs_fill_page(vnode, key, pos + copied,
   198						    len - copied, page);
   199				if (ret < 0)
   200					goto out;
   201			}
 > 202			SetPageUptoodate(page);
   203		}
   204	
   205		if (PagePrivate(page)) {
   206			priv = page_private(page);
   207			f = afs_page_dirty_from(priv);
   208			t = afs_page_dirty_to(priv);
   209			if (from < f)
   210				f = from;
   211			if (to > t)
   212				t = to;
   213			priv = afs_page_dirty(f, t);
   214			set_page_private(page, priv);
   215			trace_afs_page_dirty(vnode, tracepoint_string("dirty+"),
   216					     page->index, priv);
   217		} else {
   218			priv = afs_page_dirty(from, to);
   219			attach_page_private(page, (void *)priv);
   220			trace_afs_page_dirty(vnode, tracepoint_string("dirty"),
   221					     page->index, priv);
   222		}
   223	
   224		set_page_dirty(page);
   225		if (PageDirty(page))
   226			_debug("dirtied");
   227		ret = copied;
   228	
   229	out:
   230		unlock_page(page);
   231		put_page(page);
   232		return ret;
   233	}
   234	

---
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: 39028 bytes --]

Re: [PATCH] afs: Fix afs_write_end() when called with copied == 0 [ver #2]

[kernel test robot]

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

Hi David,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on linux/master v5.10-rc3 next-20201113]
[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/David-Howells/afs-Fix-afs_write_end-when-called-with-copied-0-ver-2/20201115-012626
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git f01c30de86f1047e9bae1b1b1417b0ce8dcd15b1
config: x86_64-randconfig-a006-20201115 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 9a85643cd357e412cff69067bb5c4840e228c2ab)
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/3486f1e413fba9587ced6c768d75e993ef78ce9d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review David-Howells/afs-Fix-afs_write_end-when-called-with-copied-0-ver-2/20201115-012626
        git checkout 3486f1e413fba9587ced6c768d75e993ef78ce9d
        # 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 errors (new ones prefixed by >>):

>> fs/afs/write.c:202:3: error: implicit declaration of function 'SetPageUptoodate' [-Werror,-Wimplicit-function-declaration]
                   SetPageUptoodate(page);
                   ^
   fs/afs/write.c:202:3: note: did you mean 'SetPageUptodate'?
   include/linux/page-flags.h:539:29: note: 'SetPageUptodate' declared here
   static __always_inline void SetPageUptodate(struct page *page)
                               ^
   1 error generated.

vim +/SetPageUptoodate +202 fs/afs/write.c

   158	
   159	/*
   160	 * finalise part of a write to a page
   161	 */
   162	int afs_write_end(struct file *file, struct address_space *mapping,
   163			  loff_t pos, unsigned len, unsigned copied,
   164			  struct page *page, void *fsdata)
   165	{
   166		struct afs_vnode *vnode = AFS_FS_I(file_inode(file));
   167		struct key *key = afs_file_key(file);
   168		unsigned long priv;
   169		unsigned int f, from = pos & (PAGE_SIZE - 1);
   170		unsigned int t, to = from + copied;
   171		loff_t i_size, maybe_i_size;
   172		int ret = 0;
   173	
   174		_enter("{%llx:%llu},{%lx}",
   175		       vnode->fid.vid, vnode->fid.vnode, page->index);
   176	
   177		if (copied == 0)
   178			goto out;
   179	
   180		maybe_i_size = pos + copied;
   181	
   182		i_size = i_size_read(&vnode->vfs_inode);
   183		if (maybe_i_size > i_size) {
   184			write_seqlock(&vnode->cb_lock);
   185			i_size = i_size_read(&vnode->vfs_inode);
   186			if (maybe_i_size > i_size)
   187				i_size_write(&vnode->vfs_inode, maybe_i_size);
   188			write_sequnlock(&vnode->cb_lock);
   189		}
   190	
   191		if (!PageUptodate(page)) {
   192			if (copied < len) {
   193				/* Try and load any missing data from the server.  The
   194				 * unmarshalling routine will take care of clearing any
   195				 * bits that are beyond the EOF.
   196				 */
   197				ret = afs_fill_page(vnode, key, pos + copied,
   198						    len - copied, page);
   199				if (ret < 0)
   200					goto out;
   201			}
 > 202			SetPageUptoodate(page);
   203		}
   204	
   205		if (PagePrivate(page)) {
   206			priv = page_private(page);
   207			f = afs_page_dirty_from(priv);
   208			t = afs_page_dirty_to(priv);
   209			if (from < f)
   210				f = from;
   211			if (to > t)
   212				t = to;
   213			priv = afs_page_dirty(f, t);
   214			set_page_private(page, priv);
   215			trace_afs_page_dirty(vnode, tracepoint_string("dirty+"),
   216					     page->index, priv);
   217		} else {
   218			priv = afs_page_dirty(from, to);
   219			attach_page_private(page, (void *)priv);
   220			trace_afs_page_dirty(vnode, tracepoint_string("dirty"),
   221					     page->index, priv);
   222		}
   223	
   224		set_page_dirty(page);
   225		if (PageDirty(page))
   226			_debug("dirtied");
   227		ret = copied;
   228	
   229	out:
   230		unlock_page(page);
   231		put_page(page);
   232		return ret;
   233	}
   234	

---
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: 37120 bytes --]