On Fri, Jan 04, 2008 at 03:53:04PM -0500, J. Bruce Fields wrote: > So, the problem is that fcntl_setlease() does > > vfs_setlease() > fasync_helper() > > which the bkl held over both, and you want to preserve that? > > But what that BKL is doing is a mystery to me--the very first thing that > fasync_helper() does is kmem_cache_alloc(., GFP_KERNEL). So you won't > be introducing any new problem if you lock those two operations > separately. Unless I'm totally missing something. A very good point. So yet another race caused by using the BKL rather than thinking ... but maybe it's an inconsequential race. The consequences are that (if the kmalloc in fasync_helper sleeps) a lease appears that isn't fully set-up yet (and may be removed if the kmalloc fails). Actually, it seems bad if the kmalloc eventually succeeds -- there's a window while kmalloc is sleeping where another process could open the file, break the lease, fl_fasync will be NULL, so no signal is sent. Then 30 seconds later the lease is removed without the leaseholder being sent a signal. Bad. How can we fix this situation? I think we need a better interface than fasync_helper() -- fasync_alloc() and fasync_setup() would seem to do the trick. -- Intel are signing my paycheques ... these opinions are still mine "Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step." - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
