Ok guys get ready to flame me....
The attached patch removes the lock/unlock in this function. Now I am 80%
sure of this one, but would like a word from the kmod maintainer about
whether request_module needs the BKL or not. do_nfsservctl already takes
the BKL inside the function so as long as request_module is safe this pair
can be removed -- effectively making do_nfsservctl responsible for it's own
locking scheme.
So whoever knows for SURE about request_module, please reply.
========NOT A PATCH --- filesystems.c::sys_nfsservctl================
long
asmlinkage sys_nfsservctl(int cmd, void *argp, void *resp)
{
int ret = -ENOSYS;
lock_kernel();
if (nfsd_linkage ||
(request_module ("nfsd") == 0 && nfsd_linkage))
ret = nfsd_linkage->do_nfsservctl(cmd, argp, resp);
unlock_kernel();
return ret;
}
==================PATCH ATTACHED==========================
>>>>> " " == Craig Christophel <[email protected]> writes:
> Ok guys get ready to flame me....
> The attached patch removes the lock/unlock in this function.
> Now I am 80%
> sure of this one, but would like a word from the kmod
> maintainer about whether request_module needs the BKL or not.
> do_nfsservctl already takes the BKL inside the function so as
> long as request_module is safe this pair can be removed --
> effectively making do_nfsservctl responsible for it's own
> locking scheme.
What would remain to protect 'nfsd_linkage' if you removed the BKL?
Cheers,
Trond
> What would remain to protect 'nfsd_linkage' if you removed the BKL?
ok here is a patch with that changed. inlined for commenting and attached for
patching. (kmail screws up my inlines occasionally)
All of this extra checking is only in place if nfsd is a module. rw_sems
shouldn't impost that much overhead in the normal case.
The while is there to catch extremely unlikely cases of the module unloading
during this loop, ie: the cleanup_module section of nfsd is waiting for the
semaphore.
===== fs/filesystems.c 1.4 vs edited =====
--- 1.4/fs/filesystems.c Fri Feb 8 22:10:55 2002
+++ edited/fs/filesystems.c Wed Feb 13 17:43:41 2002
@@ -16,19 +16,28 @@
#if defined(CONFIG_NFSD_MODULE)
struct nfsd_linkage *nfsd_linkage = NULL;
+DECLARE_RWSEM(nfsd_linkage_lock);
long
asmlinkage sys_nfsservctl(int cmd, void *argp, void *resp)
{
int ret = -ENOSYS;
- lock_kernel();
-
- if (nfsd_linkage ||
- (request_module ("nfsd") == 0 && nfsd_linkage))
- ret = nfsd_linkage->do_nfsservctl(cmd, argp, resp);
-
- unlock_kernel();
+ down_read(&nfsd_linkage_lock);
+ while(!nfsd_linkage) {
+ up_read(&nfsd_linkage_lock);
+ down_write(&nfsd_linkage_lock);
+ if (!nfsd_linkage)
+ if (request_module ("nfsd") != 0) {
+ up_write(&nfsd_linkage_lock);
+ goto out;
+ }
+ up_write(&nfsd_linkage_lock);
+ down_read(&nfsd_linkage_lock);
+ }
+ ret = nfsd_linkage->do_nfsservctl(cmd, argp, resp);
+ up_read(&nfsd_linkage_lock);
+out:
return ret;
}
EXPORT_SYMBOL(nfsd_linkage);
===== fs/nfsd/nfsctl.c 1.10 vs edited =====
--- 1.10/fs/nfsd/nfsctl.c Fri Feb 8 22:10:55 2002
+++ edited/fs/nfsd/nfsctl.c Wed Feb 13 17:33:17 2002
@@ -290,6 +290,7 @@
do_nfsservctl: handle_sys_nfsservctl,
};
+extern struct nfsd_linkage_lock;
/*
* Initialize the module
*/
@@ -307,7 +308,9 @@
void
cleanup_module(void)
{
+ down_write(&nfsd_linkage_lock);
nfsd_linkage = NULL;
+ up_write(&nfsd_linkage_lock);
nfsd_export_shutdown();
nfsd_cache_shutdown();
remove_proc_entry("fs/nfs/exports", NULL);
On Wednesday February 13, [email protected] wrote:
> Ok guys get ready to flame me....
>
> The attached patch removes the lock/unlock in this function. Now I am 80%
> sure of this one, but would like a word from the kmod maintainer about
> whether request_module needs the BKL or not. do_nfsservctl already takes
> the BKL inside the function so as long as request_module is safe this pair
> can be removed -- effectively making do_nfsservctl responsible for it's own
> locking scheme.
>
> So whoever knows for SURE about request_module, please reply.
Please see
http://www.cse.unsw.edu.au/~neilb/patches/linux-devel/2.5.4-pre5/
for current nfsd patches.
patch-E-NfsdSyscallCleanup
might be of particular interest.
When I have finished testing these (sometime next week I hope) I will
be submitting them to Linux for 2.5, and then backporting them to 2.4,
and hopefully submitting the least intrusive ones to Marcello shortly
after 2.4.18 comes out.
These patches:
remove most of the BKL
enable TCP support
plus assorted other things.
Comments, and testing, most welcome.
NeilBrown
Hi,
I'm wondering what encoding is used for NFSv{2,3} file handles under
knfsd. In particular, can I upgrade kernel from 2.4.x to 2.4.y without
my clients accumulating stale filehandles?
Cheers,
Matt
On Friday February 15, [email protected] wrote:
> Hi,
>
> I'm wondering what encoding is used for NFSv{2,3} file handles under
> knfsd. In particular, can I upgrade kernel from 2.4.x to 2.4.y without
> my clients accumulating stale filehandles?
In general, you should be able to upgrade a minor version (e.g. 2.2 to
2.4) and up or down grade a patch leverl (e.g. 2.4.2 to 2.4.10 and
back to 2.4.2) without and filehandle compatability problems.
This only applies from 2.2.16 or there abouts. Before that it was a
bit of a mess.
In particular, 2.4 introduced a new file handle format, but still
recognises the old file handle format. Further, in response to a
request that contains an old style file handle, it generates an old
style file handle so that the client can compare filehandles and so
that thos filehandles can still be used in the server is down graded.
Some time in 2.4 I will probably introduce a new export option
(fs=nnnn) which will cause the filehandles to look slightly different
(i.e. not have any device numbers in them). This will be optional
(options are like that...) but even if you do change a filesystem to
be exported with the new option, clients that currently have it
mounted will still work without getting stale file handles.
So, you should be safe (but if something does go wrong, let me know
... there could always be a bug in there somewhere).
NeilBrown