[PATCH] NFS: Fix a typo in include/linux/nfs_fs.h

Previous thread: [006/165] hwmon: (coretemp) Skip duplicate CPU entries by Greg KH on Friday, July 30, 2010 - 10:13 am. (1 message)

Next thread: [PATCH] regulator: tps6586x: fix millivolt return values and SM2 table by Mike Rapoport on Friday, July 30, 2010 - 12:58 pm. (3 messages)
From: Trond Myklebust
Date: Friday, July 30, 2010 - 12:43 pm

Hi Linus,

Please pull from the "bugfixes" branch of the repository at

   git pull git://git.linux-nfs.org/projects/trondmy/nfs-2.6.git bugfixes

This will update the following files through the appended changesets.

  Cheers,
    Trond

----
 fs/nfs/file.c          |   13 +++++++++++--
 fs/nfs/nfsroot.c       |    2 +-
 fs/nfs/write.c         |   27 +++++++++++++++++++--------
 include/linux/nfs_fs.h |    1 +
 4 files changed, 32 insertions(+), 11 deletions(-)

commit cfb506e1d330387dfaf334dd493b3773d388863d
Author: Trond Myklebust <Trond.Myklebust@netapp.com>
Date:   Fri Jul 30 15:31:57 2010 -0400

    NFS: Ensure that writepage respects the nonblock flag
    
    Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>

commit b608b283a962caaa280756bc8563016a71712acf
Author: Trond Myklebust <Trond.Myklebust@netapp.com>
Date:   Fri Jul 30 15:31:54 2010 -0400

    NFS: kswapd must not block in nfs_release_page
    
    See https://bugzilla.kernel.org/show_bug.cgi?id=16056
    
    If other processes are blocked waiting for kswapd to free up some memory so
    that they can make progress, then we cannot allow kswapd to block on those
    processes.
    
    Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
    Cc: stable@kernel.org

commit 674b2222920012244ca59978b356b25412a8dcc7
Author: Dan Carpenter <error27@gmail.com>
Date:   Tue Jul 13 13:34:59 2010 +0200

    nfs: include space for the NUL in root path
    
    In root_nfs_name() it does the following:
    
            if (strlen(buf) + strlen(cp) > NFS_MAXPATHLEN) {
                    printk(KERN_ERR "Root-NFS: Pathname for remote directory too long.\n");
                    return -1;
            }
            sprintf(nfs_export_path, buf, cp);
    
    In the original code if (strlen(buf) + strlen(cp) == NFS_MAXPATHLEN)
    then the sprintf() would lead to an overflow.  Generally the rest of the
    code assumes that the path can have NFS_MAXPATHLEN (1024) characters and
    a ...
From: Ingo Molnar
Date: Saturday, July 31, 2010 - 11:44 pm

FYI, these commits introduced a build failure on x86:

  fs/nfs/file.c:508: error: implicit declaration of function ‘nfs_commit_inode’

The nfs_commit_inode() prototype:

 #if defined(CONFIG_NFS_V3) || defined(CONFIG_NFS_V4)
 extern int  nfs_commit_inode(struct inode *, int);
 extern struct nfs_write_data *nfs_commitdata_alloc(void);
 extern void nfs_commit_free(struct nfs_write_data *wdata);
 #endif

is missing if the kernel is configured for legacy NFS:

 CONFIG_NFS_FS=y
 # CONFIG_NFS_V3 is not set
 # CONFIG_NFS_V4 is not set

The fix below solves the build problem here, but I have not tested whether NFS 
still works fine for this config.

Thanks,

	Ingo

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 include/linux/nfs_fs.h |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index f6e2455..bf145c6 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -492,11 +492,9 @@ extern int nfs_writeback_done(struct rpc_task *, struct nfs_write_data *);
 extern int nfs_wb_all(struct inode *inode);
 extern int nfs_wb_page(struct inode *inode, struct page* page);
 extern int nfs_wb_page_cancel(struct inode *inode, struct page* page);
-#if defined(CONFIG_NFS_V3) || defined(CONFIG_NFS_V4)
 extern int  nfs_commit_inode(struct inode *, int);
 extern struct nfs_write_data *nfs_commitdata_alloc(void);
 extern void nfs_commit_free(struct nfs_write_data *wdata);
-#endif
 
 static inline int
 nfs_have_writebacks(struct inode *inode)
--

From: Linus Torvalds
Date: Sunday, August 1, 2010 - 10:10 am

No, I think perhaps the '#if' line should just be moved down one line.
The other function still only exist for V3/V4, it's just
nfs_commit_inode() that is available (as a stub that just returns 0)
for V2.

But maybe that stub should be made an inline function or #define?

Trond?

                  Linus
--

From: Trond Myklebust
Date: Sunday, August 1, 2010 - 10:35 am

Sorry. I was intending to put that declaration outside the #ifdef. The

I'll convert it into an inlined stub just as it was prior to commit
8fc795f703c5138e1a8bfb88c69f52632031aa6a.

Trond
--

From: Trond Myklebust
Date: Sunday, August 1, 2010 - 10:40 am

nfs_commit_inode() needs to be defined irrespectively of whether or not
we are supporting NFSv3 and NFSv4.
Allow the compiler to optimise away code in the NFSv2-only case by
converting it into an inlined stub function.

Reported-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/write.c         |    5 -----
 include/linux/nfs_fs.h |    6 ++++++
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index bb72ad3..9f81bdd 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -1454,11 +1454,6 @@ out_mark_dirty:
 	return ret;
 }
 #else
-int nfs_commit_inode(struct inode *inode, int how)
-{
-	return 0;
-}
-
 static int nfs_commit_unstable_pages(struct inode *inode, struct writeback_control *wbc)
 {
 	return 0;
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index f6e2455..bad4d12 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -496,6 +496,12 @@ extern int nfs_wb_page_cancel(struct inode *inode, struct page* page);
 extern int  nfs_commit_inode(struct inode *, int);
 extern struct nfs_write_data *nfs_commitdata_alloc(void);
 extern void nfs_commit_free(struct nfs_write_data *wdata);
+#else
+static inline int
+nfs_commit_inode(struct inode *inode, int how)
+{
+	return 0;
+}
 #endif
 
 static inline int
-- 
1.7.2

--

From: Ingo Molnar
Date: Sunday, August 1, 2010 - 12:32 pm

I tested this patch too and it solves the problem (and doesnt introduce new 
problems).

Thanks,

	Ingo
--

From: Christoph Hellwig
Date: Sunday, August 1, 2010 - 12:45 pm

Btw, is there a reason to allow not building NFSv3?  If at all
building without v2 support would be more useful.

--

From: Myklebust, Trond
Date: Sunday, August 1, 2010 - 12:56 pm

I am planning to separate nfsv2, nfsv3 and nfsv4 into loadable submodules at some point in the not-too-distant future. That seems like a better solution than these CONFIG suboptions that keep tripping us up.

Cheers
    Trond--

Previous thread: [006/165] hwmon: (coretemp) Skip duplicate CPU entries by Greg KH on Friday, July 30, 2010 - 10:13 am. (1 message)

Next thread: [PATCH] regulator: tps6586x: fix millivolt return values and SM2 table by Mike Rapoport on Friday, July 30, 2010 - 12:58 pm. (3 messages)