From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754235AbaHLRnR (ORCPT ); Tue, 12 Aug 2014 13:43:17 -0400 Received: from mail-qg0-f42.google.com ([209.85.192.42]:41628 "EHLO mail-qg0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753929AbaHLRnP (ORCPT ); Tue, 12 Aug 2014 13:43:15 -0400 From: Jeff Layton X-Google-Original-From: Jeff Layton Date: Tue, 12 Aug 2014 13:43:13 -0400 To: linux-fsdevel@vger.kernel.org Cc: linux-kernel@vger.kernel.org, linux-nfs@vger.kernel.org, trond.myklebust@primarydata.com Subject: Re: [PATCH 0/5] locks: move most locks_release_private calls outside of i_lock Message-ID: <20140812134313.1273dc63@tlielax.poochiereds.net> In-Reply-To: <1407854893-2698-1-git-send-email-jlayton@primarydata.com> References: <1407854893-2698-1-git-send-email-jlayton@primarydata.com> X-Mailer: Claws Mail 3.10.1 (GTK+ 2.24.22; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 12 Aug 2014 10:48:08 -0400 Jeff Layton wrote: > In the days of yore, the file locking code was primarily protected by > the BKL. That changed in commit 72f98e72551fa (locks: turn lock_flocks > into a spinlock), at which point the code was changed to be protected > by a conventional spinlock (mostly due to a push to finally eliminate > the BKL). Since then, the code has been changed to use the i_lock > instead of a global spinlock, but it's still under a spinlock. > > With that change, several functions now no longer can block when they > originally could. This is a particular problem with the > fl_release_private operation. In NFSv4, that operation is used to kick > off a RELEASE_LOCKOWNER or FREE_STATEID call, and that requires being > able to do an allocation. > > This was reported by Josh Stone here: > > https://bugzilla.redhat.com/show_bug.cgi?id=1089092 > > My initial stab at fixing this involved moving this to a workqueue, but > Trond pointed out the above change was technically a regression with the > way the spinlocking in the file locking code works, and suggested an > alternate approach to fixing it. > > This set focuses on moving most of the locks_release_private calls > outside of the inode->i_lock. There are still a few that are done > under the i_lock in the lease handling code. Cleaning up the use of > the i_lock in the lease code is a larger project which we'll have to > tackle at some point, but there are some other cleanups that will > need to happen first. > > Absent any objections, I'll plan to merge these for 3.18. > Erm...make that v3.17... As Trond points out, the fact that we end up doing sleeping allocations under spinlock can allow an unprivileged user to crash a NFSv4 client. So I may see about merging these sooner rather than later after they've had a little soak time in linux-next. -- Jeff Layton From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Layton Subject: Re: [PATCH 0/5] locks: move most locks_release_private calls outside of i_lock Date: Tue, 12 Aug 2014 13:43:13 -0400 Message-ID: <20140812134313.1273dc63@tlielax.poochiereds.net> References: <1407854893-2698-1-git-send-email-jlayton@primarydata.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, trond.myklebust-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org To: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Return-path: In-Reply-To: <1407854893-2698-1-git-send-email-jlayton-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org> Sender: linux-nfs-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-fsdevel.vger.kernel.org On Tue, 12 Aug 2014 10:48:08 -0400 Jeff Layton wrote: > In the days of yore, the file locking code was primarily protected by > the BKL. That changed in commit 72f98e72551fa (locks: turn lock_flocks > into a spinlock), at which point the code was changed to be protected > by a conventional spinlock (mostly due to a push to finally eliminate > the BKL). Since then, the code has been changed to use the i_lock > instead of a global spinlock, but it's still under a spinlock. > > With that change, several functions now no longer can block when they > originally could. This is a particular problem with the > fl_release_private operation. In NFSv4, that operation is used to kick > off a RELEASE_LOCKOWNER or FREE_STATEID call, and that requires being > able to do an allocation. > > This was reported by Josh Stone here: > > https://bugzilla.redhat.com/show_bug.cgi?id=1089092 > > My initial stab at fixing this involved moving this to a workqueue, but > Trond pointed out the above change was technically a regression with the > way the spinlocking in the file locking code works, and suggested an > alternate approach to fixing it. > > This set focuses on moving most of the locks_release_private calls > outside of the inode->i_lock. There are still a few that are done > under the i_lock in the lease handling code. Cleaning up the use of > the i_lock in the lease code is a larger project which we'll have to > tackle at some point, but there are some other cleanups that will > need to happen first. > > Absent any objections, I'll plan to merge these for 3.18. > Erm...make that v3.17... As Trond points out, the fact that we end up doing sleeping allocations under spinlock can allow an unprivileged user to crash a NFSv4 client. So I may see about merging these sooner rather than later after they've had a little soak time in linux-next. -- Jeff Layton -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html