From: Frank Filz <ffilzlnx@us.ibm.com>
Subject: Found issue with BKL not held during call
	to	nfs_asynch_unlink_release()
Date: Thu, 05 Oct 2006 10:46:04 -0700
Message-ID: <1160070364.3376.71.camel@dyn9047022153>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
To: NFS List <nfs@lists.sourceforge.net>
Sender: nfs-bounces@lists.sourceforge.net
Errors-To: nfs-bounces@lists.sourceforge.net

We have found a problem with the BKL not being held during a call to
nfs_asynch_unlink_release(), the call stack (this is an rpciod process)
is:

nfs_async_unlink_release (called via tk_ops->rpc_release())
rpc_release_task
__rpc_execute
run_workqueue
worker_thread
kthread
kernel_thread

I have been talking to Chuck Lever about this because I had noted one of
his patches for removing the BKL added a spin lock to protect the
nfs_deletes list used in fs/nfs/unlink.c. This patch happens to fix the
specific problem we are seeing, but discussion with Chuck suggests there
may be a larger problem.

Chuck pointed out that tk_ops is a recent addition, and that perhaps the
think to do would be to add a lock_kernel() around the call to 
tk_ops->rpc_release().

When I had been investigating removing the BKL, I had noted that it
seemed inconsistent whether the BKL was held for calls to rpc_execute(),
and in some cases, the BKL was held ONLY for the call to rpc_execute().
My investigations found that rpciod does not hold the BKL when calling
__rpc_execute(), and I also think some of the direct I/O code in
fs/nfs/direct.c does not hold the BKL, or possible the asynch code (I
didn't keep good notes - I had made a cursory examination, then noted
for sure there were code paths where rpc_execute was called without the
BKL, and naively assumed it was not needed, and didn't pursue exactly
which calls did and did not hold the BKL - it seems that assumption may
be wrong).

At first, I thought that using Chuck's patch to unlink.c would be a good
fix since it would not introduce new lock_kernel() calls, however, if
there may be other holes, it might be simpler to put the proper
lock_kernel() calls in, and then cleanly remove all uses of the BKL when
Chuck's patch set is adopted.

Frank Filz


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs