Following bug was uncovered by compiling with '-W' flag:
CC [M] fs/afs/write.o
fs/afs/write.c: In function ‘afs_write_back_from_locked_page’:
fs/afs/write.c:398: warning: comparison of unsigned expression >= 0 is always true
Loop variable 'n' is unsigned, so wraps around happily as far as I can
see. Trival fix attached (compile tested only).
Signed-Off-By: Mika Kukkonen <mikukkon@iki.fi>
Signed-off-by: David Howells <dhowells@redhat.com>
---
fs/afs/write.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/afs/write.c b/fs/afs/write.c
index 67ae4db..28f3751 100644
--- a/fs/afs/write.c
+++ b/fs/afs/write.c
@@ -395,8 +395,9 @@ static int afs_write_back_from_locked_page(struct afs_writeback *wb,
if (n == 0)
goto no_more;
if (pages[0]->index != start) {
- for (n--; n >= 0; n--)
- put_page(pages[n]);
+ do {
+ put_page(pages[--n]);
+ } while (n > 0);
goto no_more;
}
-
Fix a couple of problems with unlinking AFS files.
(1) The parent directory wasn't being updated properly between unlink() and
the following lookup().
It seems that, for some reason, invalidate_remote_inode() wasn't
discarding the directory contents correctly, so this patch calls
invalidate_inode_pages2() instead on non-regular files.
(2) afs_vnode_deleted_remotely() should handle vnodes that don't have a
source server recorded without oopsing.
Signed-off-by: David Howells <dhowells@redhat.com>
---
fs/afs/file.c | 2 +-
fs/afs/inode.c | 10 +++++++---
fs/afs/super.c | 3 ++-
fs/afs/vnode.c | 33 +++++++++++++++++++++------------
4 files changed, 31 insertions(+), 17 deletions(-)
diff --git a/fs/afs/file.c b/fs/afs/file.c
index 3e25795..9c0e721 100644
--- a/fs/afs/file.c
+++ b/fs/afs/file.c
@@ -236,7 +236,7 @@ static void afs_invalidatepage(struct page *page, unsigned long offset)
{
int ret = 1;
- kenter("{%lu},%lu", page->index, offset);
+ _enter("{%lu},%lu", page->index, offset);
BUG_ON(!PageLocked(page));
diff --git a/fs/afs/inode.c b/fs/afs/inode.c
index 515a5d1..47f5fed 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -209,11 +209,15 @@ bad_inode:
*/
void afs_zap_data(struct afs_vnode *vnode)
{
- _enter("zap data {%x:%u}", vnode->fid.vid, vnode->fid.vnode);
+ _enter("{%x:%u}", vnode->fid.vid, vnode->fid.vnode);
/* nuke all the non-dirty pages that aren't locked, mapped or being
- * written back */
- invalidate_remote_inode(&vnode->vfs_inode);
+ * written back in a regular file and completely discard the pages in a
+ * directory or symlink */
+ if (S_ISREG(vnode->vfs_inode.i_mode))
+ invalidate_remote_inode(&vnode->vfs_inode);
+ else
+ invalidate_inode_pages2(vnode->vfs_inode.i_mapping);
}
/*
diff --git a/fs/afs/super.c b/fs/afs/super.c
index d24be33..422f532 100644
--- a/fs/afs/super.c
+++ b/fs/afs/super.c
@@ -488,6 +488,7 @@ static struct inode *afs_alloc_inode(struct ...On Thu, 10 May 2007 15:33:34 +0100 gcc -W finds a number of fairly scary bugs. More than one would expect, given that it is recommended in Documentation/SubmitChecklist, which everyone reads ;) -
Do you mean in my code specifically? Or in the kernel in general? As far as I can tell -W only finds an eye-glazingly large quantity of 'unused parameter' warnings in AFS and AF_RXRPC. David -
Yes, it's a shame that there doesn't seem to be a fine-grained way of turning on -W's useful bits. -
You can turn off -W's undesirable bits. For net/rxrpc/ and fs/afs/ at least, adding: CFLAGS += -W -Wno-unused-parameter to the Makefile generates no warnings. Perhaps this should be added to the master Makefile. Adding -Wsign-compare finds some stuff that I will fix. It also finds some stuff in the main and the networking headers. This is a really useful option and found some tricky bugs in CacheFiles. I would endorse adding this generally too. David -
Which states incorrectly: | 22: Newly-added code has been compiled with `gcc -W'. This will generate | lots of noise, but is good for finding bugs like "warning: comparison | between signed and unsigned". -W does not imply -Wsign-compare, at least not on my gcc. David -
| Greg KH | Og dreams of kernels |
| Jens Axboe | [PATCH 31/33] Fusion: sg chaining support |
| Arnd Bergmann | Re: finding your own dead "CONFIG_" variables |
| Mark Brown | [PATCH 2/2] Subject: natsemi: Allow users to disable workaround for DspCfg reset |
| Tony Breeds | [LGUEST] Look in object dir for .config |
git: | |
| Brian Downing | Re: Git in a Nutshell guide |
| John Benes | Re: master has some toys |
| Matthias Lederhofer | [PATCH 4/7] introduce GIT_WORK_TREE to specify the work tree |
| Alexander Sulfrian | [RFC/PATCH] RE: git calls SSH_ASKPASS even if DISPLAY is not set |
| Junio C Hamano | Re: Rss produced by git is not valid xml? |
| Linux Kernel Mailing List | iSeries: fix section mismatch in iseries_veth |
| Linux Kernel Mailing List | ixbge: remove TX lock and redo TX accounting. |
| Linux Kernel Mailing List | ixgbe: fix several counter register errata |
| Linux Kernel Mailing List | b43: fix build with CONFIG_SSB_PCI |
