LinuxLists.cc - [PATCH 0/7] BKL removal follow-up

2010-11-17 15:27:27

Subject: [PATCH 0/7] BKL removal follow-up

2010-11-21 14:12:53

by Boaz Harrosh

[permalink] [raw]

Subject: Re: [PATCH 0/7] BKL removal follow-up

On 11/19/2010 01:40 AM, Linus Torvalds wrote:
> On Thu, Nov 18, 2010 at 3:34 PM, Jan Kara <[email protected]> wrote:
>>
>> Just for info, UDF BKL removal patches seem to work fine but I want to
>> give them some final SMP testing on Monday before pushing them to -next.
>> I'm not sure how much people hurry with disabling the lock so if I should
>> push them ASAP or whether the next merge window is fine...
>
> I don't think I can reasonably do it in 37, we're late enough in the
> -rc series. So UDF and really saying 'n' to BKL by default will have
> to be for the next merge window.
>
> I hate to do it, but I'd be too nervous about it any other way.
>
> Linus

Could it not default to "n" but be SELECTed by the like of UDF. Or
that too big of a Kconfig change for 37-rcX? I understand it does
not really buy us anything for distro's with allmodconfig style,
but it should help with defconfig on all the other ARCH's.

Thanks
Boaz

2010-11-18 23:41:35

by Linus Torvalds

[permalink] [raw]

Subject: Re: [PATCH 0/7] BKL removal follow-up

On Thu, Nov 18, 2010 at 3:34 PM, Jan Kara <[email protected]> wrote:
>
> ?Just for info, UDF BKL removal patches seem to work fine but I want to
> give them some final SMP testing on Monday before pushing them to -next.
> I'm not sure how much people hurry with disabling the lock so if I should
> push them ASAP or whether the next merge window is fine...

I don't think I can reasonably do it in 37, we're late enough in the
-rc series. So UDF and really saying 'n' to BKL by default will have
to be for the next merge window.

I hate to do it, but I'd be too nervous about it any other way.

Linus

2010-11-22 15:17:45

by Nick Bowler

[permalink] [raw]

Subject: Re: [PATCH 0/7] BKL removal follow-up

On 2010-11-21 09:45 -0800, Linus Torvalds wrote:
> Yes, I'd be ok with UDF doing a "select BKL" along with a "default n"
> for BKL itself.
>
> I think UDF currently is the only sane reason to have BKL enabled any
> more, and yes, it would probably make it easier to configure things.

UFS (which I use) also relies on BKL.

--
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)

2010-11-17 15:27:18

by Arnd Bergmann

[permalink] [raw]

Subject: [PATCH 4/7] BKL: remove references to lock_kernel from comments

Lock_kernel is gone from the code, so the comments
should be updated, too. nfsd now uses lock_flocks
instead of lock_kernel to protect against posix
file locks.

Signed-off-by: Arnd Bergmann <[email protected]>
Cc: "J. Bruce Fields" <[email protected]>
Cc: [email protected]
---
fs/nfsd/nfs4state.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index f1e5ec6..36db9cc 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2254,7 +2254,7 @@ nfs4_file_downgrade(struct nfs4_file *fp, unsigned int share_access)
* Spawn a thread to perform a recall on the delegation represented
* by the lease (file_lock)
*
- * Called from break_lease() with lock_kernel() held.
+ * Called from break_lease() with lock_flocks() held.
* Note: we assume break_lease will only call this *once* for any given
* lease.
*/
@@ -2278,7 +2278,7 @@ void nfsd_break_deleg_cb(struct file_lock *fl)
list_add_tail(&dp->dl_recall_lru, &del_recall_lru);
spin_unlock(&recall_lock);

- /* only place dl_time is set. protected by lock_kernel*/
+ /* only place dl_time is set. protected by lock_flocks*/
dp->dl_time = get_seconds();

/*
@@ -2295,7 +2295,7 @@ void nfsd_break_deleg_cb(struct file_lock *fl)
/*
* The file_lock is being reapd.
*
- * Called by locks_free_lock() with lock_kernel() held.
+ * Called by locks_free_lock() with lock_flocks() held.
*/
static
void nfsd_release_deleg_cb(struct file_lock *fl)
@@ -2310,7 +2310,7 @@ void nfsd_release_deleg_cb(struct file_lock *fl)
}

/*
- * Called from setlease() with lock_kernel() held
+ * Called from setlease() with lock_flocks() held
*/
static
int nfsd_same_client_deleg_cb(struct file_lock *onlist, struct file_lock *try)
--
1.7.1

2010-11-21 17:53:52

by Linus Torvalds

[permalink] [raw]

Subject: Re: [PATCH 0/7] BKL removal follow-up

On Sun, Nov 21, 2010 at 6:12 AM, Boaz Harrosh <[email protected]> wrote:
> On 11/19/2010 01:40 AM, Linus Torvalds wrote:
>>
>> I don't think I can reasonably do it in 37, we're late enough in the
>> -rc series. So UDF and really saying 'n' to BKL by default will have
>> to be for the next merge window.
>
> Could it not default to "n" but be SELECTed by the like of UDF. Or
> that too big of a Kconfig change for 37-rcX? I understand it does
> not really buy us anything for distro's with allmodconfig style,
> but it should help with defconfig on all the other ARCH's.

Yes, I'd be ok with UDF doing a "select BKL" along with a "default n"
for BKL itself.

I think UDF currently is the only sane reason to have BKL enabled any
more, and yes, it would probably make it easier to configure things.

Tested patch?

Linus

2010-11-18 23:34:44

by Jan Kara

[permalink] [raw]

Subject: Re: [PATCH 0/7] BKL removal follow-up

Hi,

On Wed 17-11-10 16:26:52, Arnd Bergmann wrote:
> It seems the v4l and udf code has been worked out now and patches
> to kill the BKL there are finally making it upstream.
>
> As promised, here are the patches I did since the kernel summit
> to turn off the BKL by default. Given that -rc2 is out now, I don't
> know how much of these you still want for 2.6.37, so just pick
> the ones you like and I'll send a pull request for the rest
> once the 2.6.38 merge window opens.
Just for info, UDF BKL removal patches seem to work fine but I want to
give them some final SMP testing on Monday before pushing them to -next.
I'm not sure how much people hurry with disabling the lock so if I should
push them ASAP or whether the next merge window is fine...

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2010-11-17 15:40:21

by J. Bruce Fields

[permalink] [raw]

Subject: Re: [PATCH 4/7] BKL: remove references to lock_kernel from comments

On Wed, Nov 17, 2010 at 04:26:56PM +0100, Arnd Bergmann wrote:
> Lock_kernel is gone from the code, so the comments
> should be updated, too. nfsd now uses lock_flocks
> instead of lock_kernel to protect against posix
> file locks.
>
> Signed-off-by: Arnd Bergmann <[email protected]>
> Cc: "J. Bruce Fields" <[email protected]>

Acked-by: J. Bruce Fields <[email protected]>

I want to get rid of most of these callbacks, actually; but that can
wait for the next merge window.

--b.

> Cc: [email protected]
> ---
> fs/nfsd/nfs4state.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index f1e5ec6..36db9cc 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2254,7 +2254,7 @@ nfs4_file_downgrade(struct nfs4_file *fp, unsigned int share_access)
> * Spawn a thread to perform a recall on the delegation represented
> * by the lease (file_lock)
> *
> - * Called from break_lease() with lock_kernel() held.
> + * Called from break_lease() with lock_flocks() held.
> * Note: we assume break_lease will only call this *once* for any given
> * lease.
> */
> @@ -2278,7 +2278,7 @@ void nfsd_break_deleg_cb(struct file_lock *fl)
> list_add_tail(&dp->dl_recall_lru, &del_recall_lru);
> spin_unlock(&recall_lock);
>
> - /* only place dl_time is set. protected by lock_kernel*/
> + /* only place dl_time is set. protected by lock_flocks*/
> dp->dl_time = get_seconds();
>
> /*
> @@ -2295,7 +2295,7 @@ void nfsd_break_deleg_cb(struct file_lock *fl)
> /*
> * The file_lock is being reapd.
> *
> - * Called by locks_free_lock() with lock_kernel() held.
> + * Called by locks_free_lock() with lock_flocks() held.
> */
> static
> void nfsd_release_deleg_cb(struct file_lock *fl)
> @@ -2310,7 +2310,7 @@ void nfsd_release_deleg_cb(struct file_lock *fl)
> }
>
> /*
> - * Called from setlease() with lock_kernel() held
> + * Called from setlease() with lock_flocks() held
> */
> static
> int nfsd_same_client_deleg_cb(struct file_lock *onlist, struct file_lock *try)
> --
> 1.7.1
>