BKL still required for what functions?

BKL still required for what functions?

[Richard Holden]

I'm doing some driver cleanup on the ixj telephony driver, mostly copy/ 
paste and common code refactoring. I'm also looking at trying to get  
rid of the BKL from the 2 places its used, so I was wondering if we  
have a list of kernel calls that still require the BKL to be held, and  
if we don't have a list would it be worthwhile to put one into  
Documentation, I'd be willing to maintain the list as we removed the  
BKL from the last core kernel places.

-Richard

Re: BKL still required for what functions?

[Andi Kleen]

Richard Holden <aciddeath@gmail.com> writes:

> I'm doing some driver cleanup on the ixj telephony driver, mostly
> copy/ paste and common code refactoring. I'm also looking at trying to
> get  rid of the BKL from the 2 places its used, so I was wondering if
> we  have a list of kernel calls that still require the BKL to be held,
> and  if we don't have a list would it be worthwhile to put one into
> Documentation, I'd be willing to maintain the list as we removed the
> BKL from the last core kernel places.

The standard answer is to check the callers. 

But in general there are very little (in fact I cannot think of any)
general kernel utility functions left that need the BKL and if they
then usually take it on their own. So you should be ok
assuming that general kernel code (e.g. code living in kernel/*
or lib/*) doesn't need it.

If you rely on other driver frameworks that might be different,
but a good rule of thumb is to just check that directly if it 
uses lock_kernel at all.

The reason that a lot of driver code still uses BKL implicitely
(e.g. in ioctl code) is more because it always needs someone 
to audit.

-Andi

-- 
ak@linux.intel.com

Re: BKL still required for what functions?

[Alan Cox]

On Mon, 6 Oct 2008 22:24:12 -0600
Richard Holden <aciddeath@gmail.com> wrote:

> I'm doing some driver cleanup on the ixj telephony driver, mostly copy/ 
> paste and common code refactoring. I'm also looking at trying to get  
> rid of the BKL from the 2 places its used, so I was wondering if we  
> have a list of kernel calls that still require the BKL to be held, and  
> if we don't have a list would it be worthwhile to put one into  
> Documentation, I'd be willing to maintain the list as we removed the  
> BKL from the last core kernel places.

Its never so simple - a lot of the functions it depends what data you are
using or passing to them whether they are BKL safe.

Which bits use the BKL and what do you need to know is safe ?

Re: BKL still required for what functions?

[Richard Holden]

On Oct 7, 2008, at 10:35 AM, Alan Cox wrote:
>
> Its never so simple - a lot of the functions it depends what data  
> you are
> using or passing to them whether they are BKL safe.
>
> Which bits use the BKL and what do you need to know is safe ?

Actually, it looks like I was looking at 2 different places, the  
simple removal is in phonedev.c that you authored, as far as I can  
tell the only thing the BKL is protecting is a call to request_module,  
it may be some nesting rules that I don't understand but I think if it  
is still needed there we could push the lock_kernel call down to the  
site of the request_module call. See patch below.

Thanks,
Richard Holden

Signed-off-by: Richard Holden <aciddeath@gmail.com>

diff --git a/drivers/telephony/phonedev.c b/drivers/telephony/phonedev.c
index 4d74ba3..fd1424c 100644
--- a/drivers/telephony/phonedev.c
+++ b/drivers/telephony/phonedev.c
@@ -54,14 +54,16 @@ static int phone_open(struct inode *inode, struct  
file *file)
  	if (minor >= PHONE_NUM_DEVICES)
  		return -ENODEV;

-	lock_kernel();
+	
  	mutex_lock(&phone_lock);
  	p = phone_device[minor];
  	if (p)
  		new_fops = fops_get(p->f_op);
  	if (!new_fops) {
  		mutex_unlock(&phone_lock);
+		lock_kernel();
  		request_module("char-major-%d-%d", PHONE_MAJOR, minor);
+		unlock_kernel();
  		mutex_lock(&phone_lock);
  		p = phone_device[minor];
  		if (p == NULL || (new_fops = fops_get(p->f_op)) == NULL)
@@ -81,7 +83,6 @@ static int phone_open(struct inode *inode, struct  
file *file)
  	fops_put(old_fops);
  end:
  	mutex_unlock(&phone_lock);
-	unlock_kernel();
  	return err;
  }

Re: BKL still required for what functions?

[Alan Cox]

> -	lock_kernel();
> +	
>   	mutex_lock(&phone_lock);
>   	p = phone_device[minor];
>   	if (p)
>   		new_fops = fops_get(p->f_op);
>   	if (!new_fops) {
>   		mutex_unlock(&phone_lock);
> +		lock_kernel();
>   		request_module("char-major-%d-%d", PHONE_MAJOR, minor);
> +		unlock_kernel();
>   		mutex_lock(&phone_lock);
>   		p = phone_device[minor];
>   		if (p == NULL || (new_fops = fops_get(p->f_op)) == NULL)
> @@ -81,7 +83,6 @@ static int phone_open(struct inode *inode, struct  
> file *file)
>   	fops_put(old_fops);
>   end:
>   	mutex_unlock(&phone_lock);
> -	unlock_kernel();
>   	return err;
>   }

Looks like the BKL can go entirely there providing phone_device[] is
always rechecked back in the phone mutex, request_module is happy nowdays
without the BKL