Messages in this thread | | | Date | Sun, 07 Jul 2002 15:42:56 -0700 | From | Dave Hansen <> | Subject | Re: BKL removal |
| |
Thunder from the hill wrote:
>> "As long as I comment [and understand] why I am using the BKL." >> would be a bit more accurate. How many places in the kernel have >> you seen comments about what the BKL is actually doing? Could >> you point me to some of your comments where _you_ are using the >> BKL? Once you fully understand why it is there, the extra step >> of removal is usually very small. > > Old Blue, could you please bring me an example on where in USB the > bkl shouldn't be used, but is? And can you explain why it's wrong > to use bkl there?
Old Blue? 23 isn't _that_ old!
BKL use isn't right or wrong -- it isn't a case of creating a deadlock or a race. I'm picking a relatively random function from "grep -r lock_kernel * | grep /usb/". I'll show what I think isn't optimal about it.
"up" is a local variable. There is no point in protecting its allocation. If the goal is to protect data inside "up", there should probably be a subsystem-level lock for all "struct uhci_hcd"s or a lock contained inside of the structure itself. Is this the kind of example you're looking for?
static int uhci_proc_open(struct inode *inode, struct file *file) { const struct proc_dir_entry *dp = PDE(inode); struct uhci_hcd *uhci = dp->data; struct uhci_proc *up; unsigned long flags; int ret = -ENOMEM;
- lock_kernel();
up = kmalloc(sizeof(*up), GFP_KERNEL); if (!up) goto out;
up->data = kmalloc(MAX_OUTPUT, GFP_KERNEL); if (!up->data) { kfree(up); goto out; }
+ lock_kernel(); spin_lock_irqsave(&uhci->frame_list_lock, flags); up->size = uhci_sprint_schedule(uhci, up->data, MAX_OUTPUT); spin_unlock_irqrestore(&uhci->frame_list_lock, flags);
file->private_data = up; + unlock_kernel();
ret = 0; out: - unlock_kernel(); return ret; }
-- Dave Hansen haveblue@us.ibm.com
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |