Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Andre Noll
On 22:13, Boaz Harrosh wrote: > All the scsi calls do not need any locks. The scsi LLDS never > see these threads since commands are queued through the block > layer. That's what everybody believes, but nobody seems to know for sure. Therefore I did what Andi suggested: Make a zero-semantics

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Boaz Harrosh
On Thu, Jan 10 2008 at 21:45 +0200, Andre Noll <[EMAIL PROTECTED]> wrote: > On 20:29, Andi Kleen wrote: > >>> Sure, I can do that if James likes the idea. Since not all case >>> statements need the BKL, we could add it only to those for which it >>> isn't clear that it is unnecessary. >>> >>> And

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread James Bottomley
On Thu, 2008-01-10 at 20:45 +0100, Andre Noll wrote: > On 20:29, Andi Kleen wrote: > > > > Sure, I can do that if James likes the idea. Since not all case > > > statements need the BKL, we could add it only to those for which it > > > isn't clear that it is unnecessary. > > > > > > And this

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Andre Noll
On 20:29, Andi Kleen wrote: > > Sure, I can do that if James likes the idea. Since not all case > > statements need the BKL, we could add it only to those for which it > > isn't clear that it is unnecessary. > > > > And this would actually improve something. > > I still think it would be a good

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Andi Kleen
> If your goal is to get rid of the BKL everywhere, sure. It's not clear > to me this is the most productive way of spending our time though. I believe eventually eliminating ->ioctl (as opposed to ->unlocked_ioctl) would be a productive thing to do. Still having implicit BKL semantics in such

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Matthew Wilcox
On Thu, Jan 10, 2008 at 08:32:27PM +0100, Andi Kleen wrote: > Not many really in the core kernel. Hardly any. Grep for > lock_kernel to be sure, but there is not much. > > It's mostly drivers that still need it. > > How about the low level SCSI drivers that might called from the high > level

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Andi Kleen
On Thu, Jan 10, 2008 at 01:03:48PM -0600, James Bottomley wrote: > > On Thu, 2008-01-10 at 19:59 +0100, Andi Kleen wrote: > > > Really, all this is doing is open coding what the ioctl handler is doing > > > anyway, isn't it? in which case, why bother to change it at all? > > > > Because once

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Andi Kleen
On Thu, Jan 10, 2008 at 08:07:48PM +0100, Andre Noll wrote: > On 19:59, Andi Kleen wrote: > > > But perhaps for such a long ioctl handler it would be better to move > > the lock/unlock_kernel()s into the individual case ...: statements; > > then it could be eliminated step by step. > > Sure, I

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Andi Kleen
On Thu, Jan 10, 2008 at 12:03:24PM -0700, Matthew Wilcox wrote: > On Thu, Jan 10, 2008 at 07:59:44PM +0100, Andi Kleen wrote: > > > Really, all this is doing is open coding what the ioctl handler is doing > > > anyway, isn't it? in which case, why bother to change it at all? > > > > Because once

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Andre Noll
On 19:59, Andi Kleen wrote: > But perhaps for such a long ioctl handler it would be better to move > the lock/unlock_kernel()s into the individual case ...: statements; > then it could be eliminated step by step. Sure, I can do that if James likes the idea. Since not all case statements need the

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread James Bottomley
On Thu, 2008-01-10 at 19:59 +0100, Andi Kleen wrote: > > Really, all this is doing is open coding what the ioctl handler is doing > > anyway, isn't it? in which case, why bother to change it at all? > > Because once it's open coded it is visible and can then be eliminated. > Does SCSI need the

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Matthew Wilcox
On Thu, Jan 10, 2008 at 07:59:44PM +0100, Andi Kleen wrote: > > Really, all this is doing is open coding what the ioctl handler is doing > > anyway, isn't it? in which case, why bother to change it at all? > > Because once it's open coded it is visible and can then be eliminated. > Does SCSI need

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Andi Kleen
> Really, all this is doing is open coding what the ioctl handler is doing > anyway, isn't it? in which case, why bother to change it at all? Because once it's open coded it is visible and can then be eliminated. Does SCSI need the BKL at all? But perhaps for such a long ioctl handler it would

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread James Bottomley
On Thu, 2008-01-10 at 19:05 +0100, Andre Noll wrote: > [Resent with proper subject and to additional recipients] > > This patch against linus-current is compile-tested on x86 and x86-64. > > Please review This is rather long. For the utility of what you've just done, what's wrong with just

[patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Andre Noll
[Resent with proper subject and to additional recipients] This patch against linus-current is compile-tested on x86 and x86-64. Please review Andre --- Change sg.c to use the unlocked_ioctl instead of the ioctl handler. The patch moves the BKL into sg.c and is thus a first step to get rid of

[patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Andre Noll
[Resent with proper subject and to additional recipients] This patch against linus-current is compile-tested on x86 and x86-64. Please review Andre --- Change sg.c to use the unlocked_ioctl instead of the ioctl handler. The patch moves the BKL into sg.c and is thus a first step to get rid of

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread James Bottomley
On Thu, 2008-01-10 at 19:05 +0100, Andre Noll wrote: [Resent with proper subject and to additional recipients] This patch against linus-current is compile-tested on x86 and x86-64. Please review This is rather long. For the utility of what you've just done, what's wrong with just making

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Matthew Wilcox
On Thu, Jan 10, 2008 at 07:59:44PM +0100, Andi Kleen wrote: Really, all this is doing is open coding what the ioctl handler is doing anyway, isn't it? in which case, why bother to change it at all? Because once it's open coded it is visible and can then be eliminated. Does SCSI need the

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread James Bottomley
On Thu, 2008-01-10 at 19:59 +0100, Andi Kleen wrote: Really, all this is doing is open coding what the ioctl handler is doing anyway, isn't it? in which case, why bother to change it at all? Because once it's open coded it is visible and can then be eliminated. Does SCSI need the BKL at

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Andre Noll
On 19:59, Andi Kleen wrote: But perhaps for such a long ioctl handler it would be better to move the lock/unlock_kernel()s into the individual case ...: statements; then it could be eliminated step by step. Sure, I can do that if James likes the idea. Since not all case statements need the

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Andi Kleen
On Thu, Jan 10, 2008 at 12:03:24PM -0700, Matthew Wilcox wrote: On Thu, Jan 10, 2008 at 07:59:44PM +0100, Andi Kleen wrote: Really, all this is doing is open coding what the ioctl handler is doing anyway, isn't it? in which case, why bother to change it at all? Because once it's open

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Andi Kleen
On Thu, Jan 10, 2008 at 08:07:48PM +0100, Andre Noll wrote: On 19:59, Andi Kleen wrote: But perhaps for such a long ioctl handler it would be better to move the lock/unlock_kernel()s into the individual case ...: statements; then it could be eliminated step by step. Sure, I can do that

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Andi Kleen
Really, all this is doing is open coding what the ioctl handler is doing anyway, isn't it? in which case, why bother to change it at all? Because once it's open coded it is visible and can then be eliminated. Does SCSI need the BKL at all? But perhaps for such a long ioctl handler it would be

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Andi Kleen
On Thu, Jan 10, 2008 at 01:03:48PM -0600, James Bottomley wrote: On Thu, 2008-01-10 at 19:59 +0100, Andi Kleen wrote: Really, all this is doing is open coding what the ioctl handler is doing anyway, isn't it? in which case, why bother to change it at all? Because once it's open coded

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Matthew Wilcox
On Thu, Jan 10, 2008 at 08:32:27PM +0100, Andi Kleen wrote: Not many really in the core kernel. Hardly any. Grep for lock_kernel to be sure, but there is not much. It's mostly drivers that still need it. How about the low level SCSI drivers that might called from the high level SCSI

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Andi Kleen
If your goal is to get rid of the BKL everywhere, sure. It's not clear to me this is the most productive way of spending our time though. I believe eventually eliminating -ioctl (as opposed to -unlocked_ioctl) would be a productive thing to do. Still having implicit BKL semantics in such an

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread James Bottomley
On Thu, 2008-01-10 at 20:45 +0100, Andre Noll wrote: On 20:29, Andi Kleen wrote: Sure, I can do that if James likes the idea. Since not all case statements need the BKL, we could add it only to those for which it isn't clear that it is unnecessary. And this would actually

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Boaz Harrosh
On Thu, Jan 10 2008 at 21:45 +0200, Andre Noll [EMAIL PROTECTED] wrote: On 20:29, Andi Kleen wrote: Sure, I can do that if James likes the idea. Since not all case statements need the BKL, we could add it only to those for which it isn't clear that it is unnecessary. And this would

Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl

2008-01-10 Thread Andre Noll
On 22:13, Boaz Harrosh wrote: All the scsi calls do not need any locks. The scsi LLDS never see these threads since commands are queued through the block layer. That's what everybody believes, but nobody seems to know for sure. Therefore I did what Andi suggested: Make a zero-semantics change