Message ID | 160537468016.3082569.17243477803724537224.stgit@warthog.procyon.org.uk (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | afs: Fix afs_write_end() when called with copied == 0 [ver #2] | expand |
David Howells <dhowells@redhat.com> wrote: > - SetPageUptodate(page); > + SetPageUptoodate(page); I'm not sure what happened there. Will try again :-(
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
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
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)) {
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(-)