> Hi Andi,
> I grepped and tried to do what you suggested.
> The first file that git grep reported was:
> arch/arm/common/rtctime.c:static const struct file_operations rtc_fops = {
>
> So I cooked up the following patch (probably mangled, just to give you
> a rough idea of what I did):
> diff
Hi Andi,
I grepped and tried to do what you suggested.
The first file that git grep reported was:
arch/arm/common/rtctime.c:static const struct file_operations rtc_fops = {
So I cooked up the following patch (probably mangled, just to give you
a rough idea of what I did):
diff --git
On Thursday 10 January 2008 03:39, Alasdair G Kergon wrote:
> On Thu, Jan 10, 2008 at 01:49:15AM -0800, Daniel Phillips wrote:
> > So what stops you from changing to unlocked_ioctl for the main
> > device mapper ctl_ioctl?
>
> Nothing - patches to do this are queued for 2.6.25:
Nice. This
On Thu, Jan 10, 2008 at 01:49:15AM -0800, Daniel Phillips wrote:
> So what stops you from changing to unlocked_ioctl for the main device
> mapper ctl_ioctl?
Nothing - patches to do this are queued for 2.6.25:
> So if I write my own driver and have never heard of ioctls running on BKL
> before I can rather be sure that I just can change the interface of the ioctl
> function, put it in unlocked_ioctl and are fine? Cool.
If you know the BKL is not needed in your code you should use unlocked_ioctl
Andi Kleen wrote:
> > Can you explain the rationale behind that running on the BKL? What type
> > of
>
> It used to always run with the BKL because everything used to
> and originally nobody wanted to review all ioctl handlers in tree to see if
> they can run with more fine grained locking. A lot
Hi Alasdair,
On Wednesday 09 January 2008 14:40, Alasdair G Kergon wrote:
> Device-mapper for example in dm_blk_ioctl(), has no need for BKL so
> drops it immediately, but it does need the inode parameter, so it is
> unable to switch as things stand.
So what stops you from changing to
> Can you explain the rationale behind that running on the BKL? What type of
It used to always run with the BKL because everything used to
and originally nobody wanted to review all ioctl handlers in tree to see if
they can run with more fine grained locking. A lot probably can though.
>
Andi Kleen wrote:
> Here's a proposal for some useful code transformations the kernel janitors
> could do as opposed to running checkpatch.pl.
>
> Most ioctl handlers still running implicitely under the big kernel
> lock (BKL). But long term Linux is trying to get away from that. There is a
> new
On Wed, Jan 09, 2008 at 10:45:13PM +, Alasdair G Kergon wrote:
> On Wed, Jan 09, 2008 at 11:46:03PM +0100, Andi Kleen wrote:
> > struct inode *inode = file->f_dentry->d_inode;
>
> And oops if that's not defined?
For file_operations which we talk about here it always is defined.
Block_device
On Wed, Jan 09, 2008 at 10:45:13PM +, Alasdair G Kergon wrote:
On Wed, Jan 09, 2008 at 11:46:03PM +0100, Andi Kleen wrote:
struct inode *inode = file-f_dentry-d_inode;
And oops if that's not defined?
For file_operations which we talk about here it always is defined.
Block_device is a
Andi Kleen wrote:
Here's a proposal for some useful code transformations the kernel janitors
could do as opposed to running checkpatch.pl.
Most ioctl handlers still running implicitely under the big kernel
lock (BKL). But long term Linux is trying to get away from that. There is a
new
Can you explain the rationale behind that running on the BKL? What type of
It used to always run with the BKL because everything used to
and originally nobody wanted to review all ioctl handlers in tree to see if
they can run with more fine grained locking. A lot probably can though.
things
Hi Alasdair,
On Wednesday 09 January 2008 14:40, Alasdair G Kergon wrote:
Device-mapper for example in dm_blk_ioctl(), has no need for BKL so
drops it immediately, but it does need the inode parameter, so it is
unable to switch as things stand.
So what stops you from changing to
Andi Kleen wrote:
Can you explain the rationale behind that running on the BKL? What type
of
It used to always run with the BKL because everything used to
and originally nobody wanted to review all ioctl handlers in tree to see if
they can run with more fine grained locking. A lot probably
So if I write my own driver and have never heard of ioctls running on BKL
before I can rather be sure that I just can change the interface of the ioctl
function, put it in unlocked_ioctl and are fine? Cool.
If you know the BKL is not needed in your code you should use unlocked_ioctl
correct.
On Thu, Jan 10, 2008 at 01:49:15AM -0800, Daniel Phillips wrote:
So what stops you from changing to unlocked_ioctl for the main device
mapper ctl_ioctl?
Nothing - patches to do this are queued for 2.6.25:
On Thursday 10 January 2008 03:39, Alasdair G Kergon wrote:
On Thu, Jan 10, 2008 at 01:49:15AM -0800, Daniel Phillips wrote:
So what stops you from changing to unlocked_ioctl for the main
device mapper ctl_ioctl?
Nothing - patches to do this are queued for 2.6.25:
Nice. This removes a
On Wednesday 09 January 2008 04:00:43 pm Alasdair G Kergon wrote:
> On Wed, Jan 09, 2008 at 03:31:00PM -0800, Vadim Lobanov wrote:
> > From 2.6.23's fs/ioctl.c - do_ioctl():
>
> Ah - you're talking about struct file_operations of course;
> I was talking about struct block_device_operations.
>
>
On Wed, Jan 09, 2008 at 03:31:00PM -0800, Vadim Lobanov wrote:
> From 2.6.23's fs/ioctl.c - do_ioctl():
Ah - you're talking about struct file_operations of course;
I was talking about struct block_device_operations.
Alasdair
--
[EMAIL PROTECTED]
--
To unsubscribe from this list: send the line
On Wednesday 09 January 2008 03:05:45 pm Alasdair G Kergon wrote:
> On Wed, Jan 09, 2008 at 04:58:46PM -0600, Chris Friesen wrote:
> > Alasdair G Kergon wrote:
> > >On Wed, Jan 09, 2008 at 11:46:03PM +0100, Andi Kleen wrote:
> > >>struct inode *inode = file->f_dentry->d_inode;
> > >
> > >And oops
On Wed, Jan 09, 2008 at 04:58:46PM -0600, Chris Friesen wrote:
> Alasdair G Kergon wrote:
> >On Wed, Jan 09, 2008 at 11:46:03PM +0100, Andi Kleen wrote:
> >>struct inode *inode = file->f_dentry->d_inode;
> >And oops if that's not defined?
> Isn't this basically identical to what was being passed
Alasdair G Kergon wrote:
On Wed, Jan 09, 2008 at 11:46:03PM +0100, Andi Kleen wrote:
struct inode *inode = file->f_dentry->d_inode;
And oops if that's not defined?
Isn't this basically identical to what was being passed in to .ioctl()?
Chris
--
To unsubscribe from this list: send the
On Wed, Jan 09, 2008 at 11:46:03PM +0100, Andi Kleen wrote:
> struct inode *inode = file->f_dentry->d_inode;
And oops if that's not defined?
Alasdair
--
[EMAIL PROTECTED]
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
On Wed, Jan 09, 2008 at 10:40:26PM +, Alasdair G Kergon wrote:
> On Wed, Jan 09, 2008 at 02:12:40PM -0600, Matt Mackall wrote:
> > You'll need to change the prototype, the unlocked version doesn't take
> > an inode. And you'll need to make sure that nothing in the function uses
> > the inode,
On Wed, Jan 09, 2008 at 02:12:40PM -0600, Matt Mackall wrote:
> You'll need to change the prototype, the unlocked version doesn't take
> an inode. And you'll need to make sure that nothing in the function uses
> the inode, which I think Andi forgot to mention.
This old chestnut again. Perhaps
On Tue, 2008-01-08 at 20:58 +0100, Paolo Ciarrocchi wrote:
> On Jan 8, 2008 5:40 PM, Andi Kleen <[EMAIL PROTECTED]> wrote:
> So I cooked up the following patch (probably mangled, just to give you
> a rough idea of what I did):
> diff --git a/arch/arm/common/rtctime.c b/arch/arm/common/rtctime.c
>
> Yes, very good point. Does checkpatch.pl enforce diff -p already?
> If not, should it?
I don't think it should. That would reject a lot of perfectly good
patches.
-Andi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More
On 14:17, Richard Knutsson wrote:
> Would had preferred:
>
> if (x) {
>result = -E;
>goto out;
> }
>
> then:
>
> result = -E;
> if (x)
>goto out;
>
AFAIK, the second form is preferred in Linux because it is better
readable and it generates slightly better code.
Thanks
Andre Noll wrote:
On 17:40, Andi Kleen wrote:
Here's a proposal for some useful code transformations the kernel janitors
could do as opposed to running checkpatch.pl.
Here's my take on drivers/scsi/sg.c. It's only compile-tested on x86-64.
... and x86 with all(yes|mod)config. :)
On 17:40, Andi Kleen wrote:
> Here's a proposal for some useful code transformations the kernel janitors
> could do as opposed to running checkpatch.pl.
Here's my take on drivers/scsi/sg.c. It's only compile-tested on x86-64.
Please review.
Andre
---
Convert sg.c to the new unlocked_ioctl
Andi Kleen <[EMAIL PROTECTED]> writes:
> I imagined it would check for
>
> +struct file_operations ... = {
> + ...
> + .ioctl = ...
>
> That wouldn't catch the case of someone adding only .ioctl to an
> already existing file_operations which is not visible in the patch context,
> but
On Wed, Jan 09, 2008 at 02:41:52AM +0100, Andi Kleen wrote:
> There are a few like scsi_host_template that don't have a unlocked_ioctl yet,
> but that is just something that needs to be fixed.
There's few enough scsi LLDDs with an ioctl method that ->ioctl should
be switched over in a single
On Wed, Jan 09, 2008 at 02:41:52AM +0100, Andi Kleen wrote:
There are a few like scsi_host_template that don't have a unlocked_ioctl yet,
but that is just something that needs to be fixed.
There's few enough scsi LLDDs with an ioctl method that -ioctl should
be switched over in a single patch.
Andi Kleen [EMAIL PROTECTED] writes:
I imagined it would check for
+struct file_operations ... = {
+ ...
+ .ioctl = ...
That wouldn't catch the case of someone adding only .ioctl to an
already existing file_operations which is not visible in the patch context,
but that
On 17:40, Andi Kleen wrote:
Here's a proposal for some useful code transformations the kernel janitors
could do as opposed to running checkpatch.pl.
Here's my take on drivers/scsi/sg.c. It's only compile-tested on x86-64.
Please review.
Andre
---
Convert sg.c to the new unlocked_ioctl entry
Andre Noll wrote:
On 17:40, Andi Kleen wrote:
Here's a proposal for some useful code transformations the kernel janitors
could do as opposed to running checkpatch.pl.
Here's my take on drivers/scsi/sg.c. It's only compile-tested on x86-64.
... and x86 with all(yes|mod)config. :)
On 14:17, Richard Knutsson wrote:
Would had preferred:
if (x) {
result = -E;
goto out;
}
then:
result = -E;
if (x)
goto out;
AFAIK, the second form is preferred in Linux because it is better
readable and it generates slightly better code.
Thanks for the review.
Yes, very good point. Does checkpatch.pl enforce diff -p already?
If not, should it?
I don't think it should. That would reject a lot of perfectly good
patches.
-Andi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More
On Tue, 2008-01-08 at 20:58 +0100, Paolo Ciarrocchi wrote:
On Jan 8, 2008 5:40 PM, Andi Kleen [EMAIL PROTECTED] wrote:
So I cooked up the following patch (probably mangled, just to give you
a rough idea of what I did):
diff --git a/arch/arm/common/rtctime.c b/arch/arm/common/rtctime.c
index
On Wed, Jan 09, 2008 at 02:12:40PM -0600, Matt Mackall wrote:
You'll need to change the prototype, the unlocked version doesn't take
an inode. And you'll need to make sure that nothing in the function uses
the inode, which I think Andi forgot to mention.
This old chestnut again. Perhaps we
On Wed, Jan 09, 2008 at 10:40:26PM +, Alasdair G Kergon wrote:
On Wed, Jan 09, 2008 at 02:12:40PM -0600, Matt Mackall wrote:
You'll need to change the prototype, the unlocked version doesn't take
an inode. And you'll need to make sure that nothing in the function uses
the inode, which I
On Wed, Jan 09, 2008 at 11:46:03PM +0100, Andi Kleen wrote:
struct inode *inode = file-f_dentry-d_inode;
And oops if that's not defined?
Alasdair
--
[EMAIL PROTECTED]
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More
Alasdair G Kergon wrote:
On Wed, Jan 09, 2008 at 11:46:03PM +0100, Andi Kleen wrote:
struct inode *inode = file-f_dentry-d_inode;
And oops if that's not defined?
Isn't this basically identical to what was being passed in to .ioctl()?
Chris
--
To unsubscribe from this list: send the line
On Wed, Jan 09, 2008 at 04:58:46PM -0600, Chris Friesen wrote:
Alasdair G Kergon wrote:
On Wed, Jan 09, 2008 at 11:46:03PM +0100, Andi Kleen wrote:
struct inode *inode = file-f_dentry-d_inode;
And oops if that's not defined?
Isn't this basically identical to what was being passed in to
On Wednesday 09 January 2008 03:05:45 pm Alasdair G Kergon wrote:
On Wed, Jan 09, 2008 at 04:58:46PM -0600, Chris Friesen wrote:
Alasdair G Kergon wrote:
On Wed, Jan 09, 2008 at 11:46:03PM +0100, Andi Kleen wrote:
struct inode *inode = file-f_dentry-d_inode;
And oops if that's not
On Wed, Jan 09, 2008 at 03:31:00PM -0800, Vadim Lobanov wrote:
From 2.6.23's fs/ioctl.c - do_ioctl():
Ah - you're talking about struct file_operations of course;
I was talking about struct block_device_operations.
Alasdair
--
[EMAIL PROTECTED]
--
To unsubscribe from this list: send the line
On Wednesday 09 January 2008 04:00:43 pm Alasdair G Kergon wrote:
On Wed, Jan 09, 2008 at 03:31:00PM -0800, Vadim Lobanov wrote:
From 2.6.23's fs/ioctl.c - do_ioctl():
Ah - you're talking about struct file_operations of course;
I was talking about struct block_device_operations.
Alasdair
On Tue, Jan 08, 2008 at 09:31:24PM -0400, Kevin Winchester wrote:
> Arnd Bergmann wrote:
> > On Wednesday 09 January 2008, Andi Kleen wrote:
> >> I imagined it would check for
> >>
> >> +struct file_operations ... = {
> >> + ...
> >> + .ioctl = ...
> >>
> >> That wouldn't catch the
Arnd Bergmann wrote:
> On Wednesday 09 January 2008, Andi Kleen wrote:
>> I imagined it would check for
>>
>> +struct file_operations ... = {
>> + ...
>> + .ioctl = ...
>>
>> That wouldn't catch the case of someone adding only .ioctl to an
>> already existing file_operations which is
On Wednesday 09 January 2008, Andi Kleen wrote:
> I imagined it would check for
>
> +struct file_operations ... = {
> + ...
> + .ioctl = ...
>
> That wouldn't catch the case of someone adding only .ioctl to an
> already existing file_operations which is not visible in the patch
On Wed, Jan 09, 2008 at 01:40:58AM +0100, Arnd Bergmann wrote:
> On Tuesday 08 January 2008, Andi Kleen wrote:
> > > Thanks, Andi! I think it'd very useful change.
> >
> > Reminds me this is something that should be actually flagged
> > in checkpatch.pl too
> >
> > Andy, it would be good if
On Tuesday 08 January 2008, Andi Kleen wrote:
> > Thanks, Andi! I think it'd very useful change.
>
> Reminds me this is something that should be actually flagged
> in checkpatch.pl too
>
> Andy, it would be good if checkpatch.pl complained about .ioctl =
> as opposed to .unlocked_ioctl = ...
> Sorry about the noise here - I now notice that not all .ioctl function
> pointers have the option of changing to .unlocked_ioctl. In this case,
> the ioctl is in the struct scsi_host_template, rather than struct
> file_operations.
>
> I'll try to be a little more careful about the git grepping
Andi Kleen wrote:
> On Tue, Jan 08, 2008 at 07:50:47PM -0400, Kevin Winchester wrote:
>> Andi Kleen wrote:
>>> Here's a proposal for some useful code transformations the kernel janitors
>>> could do as opposed to running checkpatch.pl.
>>>
>>
>>
>> I notice that every driver in drivers/ata uses a
On Tue, Jan 08, 2008 at 07:50:47PM -0400, Kevin Winchester wrote:
> Andi Kleen wrote:
> > Here's a proposal for some useful code transformations the kernel janitors
> > could do as opposed to running checkpatch.pl.
> >
>
>
> I notice that every driver in drivers/ata uses a .ioctl that points to
> [EMAIL PROTECTED]:~/linux-2.6/mm$ grep "struct file_operations" *
> shmem.c:static const struct file_operations shmem_file_operations;
> shmem.c:static const struct file_operations shmem_file_operations = {
> swapfile.c:static const struct file_operations proc_swaps_operations = {
>
> Am I
Matthew Wilcox пишет:
> On Tue, Jan 08, 2008 at 01:16:06PM -0700, Matthew Wilcox wrote:
>> On Tue, Jan 08, 2008 at 09:03:13PM +0100, Paolo Ciarrocchi wrote:
>>> Yes of course, I've been silly in didn't verify whether the file compile
>>> but I would appreciate to know whether I'm on the right
Andi Kleen wrote:
> Here's a proposal for some useful code transformations the kernel janitors
> could do as opposed to running checkpatch.pl.
>
I notice that every driver in drivers/ata uses a .ioctl that points to
ata_scsi_ioctl(). I could add the BKL to that function, and then change
all of
On Jan 9, 2008 12:06 AM, Andi Kleen <[EMAIL PROTECTED]> wrote:
> > I would suggest to only work on files that compile. e.g. do a
> >
> > make allyesconfig
> > make -j$[$(grep -c processor /proc/cpuinfo)*2] &1 |tee LOG
> > (will probably take a long time)
> >
> > first and then only
> I would suggest to only work on files that compile. e.g. do a
>
> make allyesconfig
> make -j$[$(grep -c processor /proc/cpuinfo)*2] &1 |tee LOG (will
> probably take a long time)
>
> first and then only modify files when are mentioned in "LOG"
Actually since this will probably
On Jan 8, 2008 9:42 PM, Andi Kleen <[EMAIL PROTECTED]> wrote:
> > I grepped and tried to do what you suggested.
>
> First a quick question: how would you rate your C knowledge? Did you
> ever write a program yourself?
Yes I did but I probably beeing inactive for too much time,
I need to back
> I grepped and tried to do what you suggested.
First a quick question: how would you rate your C knowledge? Did you
ever write a program yourself?
My proposal assumes that you have at least basic C knowledge.
> The first file that git grep reported was:
> arch/arm/common/rtctime.c:static
On Jan 8, 2008 9:21 PM, Matthew Wilcox <[EMAIL PROTECTED]> wrote:
> On Tue, Jan 08, 2008 at 01:16:06PM -0700, Matthew Wilcox wrote:
> > On Tue, Jan 08, 2008 at 09:03:13PM +0100, Paolo Ciarrocchi wrote:
> > > Yes of course, I've been silly in didn't verify whether the file compile
> > > but I would
On Tue, 8 Jan 2008 20:58:04 +0100
"Paolo Ciarrocchi" <[EMAIL PROTECTED]> wrote:
> -static const struct file_operations rtc_fops = {
> +static long rtc_fioctl(struct file_operations rtc_fops)
> +{
> + lock_kernel();
> .owner = THIS_MODULE,
> .llseek = no_llseek,
>
On Tue, Jan 08, 2008 at 01:16:06PM -0700, Matthew Wilcox wrote:
> On Tue, Jan 08, 2008 at 09:03:13PM +0100, Paolo Ciarrocchi wrote:
> > Yes of course, I've been silly in didn't verify whether the file compile
> > but I would appreciate to know whether I'm on the right track or not.
>
> Well ...
On Tue, Jan 08, 2008 at 09:03:13PM +0100, Paolo Ciarrocchi wrote:
> On Jan 8, 2008 9:00 PM, Matthew Wilcox <[EMAIL PROTECTED]> wrote:
> > On Tue, Jan 08, 2008 at 08:58:04PM +0100, Paolo Ciarrocchi wrote:
> > > -static const struct file_operations rtc_fops = {
> > > +static long rtc_fioctl(struct
On Jan 8, 2008 9:00 PM, Matthew Wilcox <[EMAIL PROTECTED]> wrote:
> On Tue, Jan 08, 2008 at 08:58:04PM +0100, Paolo Ciarrocchi wrote:
> > -static const struct file_operations rtc_fops = {
> > +static long rtc_fioctl(struct file_operations rtc_fops)
> > +{
> > + lock_kernel();
> > .owner
On Tue, Jan 08, 2008 at 08:58:04PM +0100, Paolo Ciarrocchi wrote:
> -static const struct file_operations rtc_fops = {
> +static long rtc_fioctl(struct file_operations rtc_fops)
> +{
> + lock_kernel();
> .owner = THIS_MODULE,
You should probably restrict yourself to files that
On Jan 8, 2008 5:40 PM, Andi Kleen <[EMAIL PROTECTED]> wrote:
>
> Here's a proposal for some useful code transformations the kernel janitors
> could do as opposed to running checkpatch.pl.
>
> Most ioctl handlers still running implicitely under the big kernel
> lock (BKL). But long term Linux is
> Thanks, Andi! I think it'd very useful change.
Reminds me this is something that should be actually flagged
in checkpatch.pl too
Andy, it would be good if checkpatch.pl complained about .ioctl =
as opposed to .unlocked_ioctl = ...
Also perhaps if a whole new file_operations with a ioctl is
On Tue, Jan 08, 2008 at 05:40:15PM +0100, Andi Kleen wrote:
> Here's a proposal for some useful code transformations the kernel janitors
> could do as opposed to running checkpatch.pl.
>
> Most ioctl handlers still running implicitely under the big kernel
> lock (BKL). But long term Linux is
[Andi Kleen - Tue, Jan 08, 2008 at 05:40:15PM +0100]
|
| Here's a proposal for some useful code transformations the kernel janitors
| could do as opposed to running checkpatch.pl.
|
[...snip...]
| -Andi
|
|
got it, thanks
- Cyrill -
--
To unsubscribe from this list: send the
Here's a proposal for some useful code transformations the kernel janitors
could do as opposed to running checkpatch.pl.
Most ioctl handlers still running implicitely under the big kernel
lock (BKL). But long term Linux is trying to get away from that. There is a new
->unlocked_ioctl entry
Here's a proposal for some useful code transformations the kernel janitors
could do as opposed to running checkpatch.pl.
Most ioctl handlers still running implicitely under the big kernel
lock (BKL). But long term Linux is trying to get away from that. There is a new
-unlocked_ioctl entry point
[Andi Kleen - Tue, Jan 08, 2008 at 05:40:15PM +0100]
|
| Here's a proposal for some useful code transformations the kernel janitors
| could do as opposed to running checkpatch.pl.
|
[...snip...]
| -Andi
|
|
got it, thanks
- Cyrill -
--
To unsubscribe from this list: send the
On Tue, Jan 08, 2008 at 05:40:15PM +0100, Andi Kleen wrote:
Here's a proposal for some useful code transformations the kernel janitors
could do as opposed to running checkpatch.pl.
Most ioctl handlers still running implicitely under the big kernel
lock (BKL). But long term Linux is trying
Thanks, Andi! I think it'd very useful change.
Reminds me this is something that should be actually flagged
in checkpatch.pl too
Andy, it would be good if checkpatch.pl complained about .ioctl =
as opposed to .unlocked_ioctl = ...
Also perhaps if a whole new file_operations with a ioctl is
On Tue, 8 Jan 2008 20:58:04 +0100
Paolo Ciarrocchi [EMAIL PROTECTED] wrote:
-static const struct file_operations rtc_fops = {
+static long rtc_fioctl(struct file_operations rtc_fops)
+{
+ lock_kernel();
.owner = THIS_MODULE,
.llseek = no_llseek,
.read
On Tue, Jan 08, 2008 at 01:16:06PM -0700, Matthew Wilcox wrote:
On Tue, Jan 08, 2008 at 09:03:13PM +0100, Paolo Ciarrocchi wrote:
Yes of course, I've been silly in didn't verify whether the file compile
but I would appreciate to know whether I'm on the right track or not.
Well ... you're
On Jan 8, 2008 9:00 PM, Matthew Wilcox [EMAIL PROTECTED] wrote:
On Tue, Jan 08, 2008 at 08:58:04PM +0100, Paolo Ciarrocchi wrote:
-static const struct file_operations rtc_fops = {
+static long rtc_fioctl(struct file_operations rtc_fops)
+{
+ lock_kernel();
.owner =
On Jan 8, 2008 5:40 PM, Andi Kleen [EMAIL PROTECTED] wrote:
Here's a proposal for some useful code transformations the kernel janitors
could do as opposed to running checkpatch.pl.
Most ioctl handlers still running implicitely under the big kernel
lock (BKL). But long term Linux is trying to
On Tue, Jan 08, 2008 at 08:58:04PM +0100, Paolo Ciarrocchi wrote:
-static const struct file_operations rtc_fops = {
+static long rtc_fioctl(struct file_operations rtc_fops)
+{
+ lock_kernel();
.owner = THIS_MODULE,
You should probably restrict yourself to files that you
On Tue, Jan 08, 2008 at 09:03:13PM +0100, Paolo Ciarrocchi wrote:
On Jan 8, 2008 9:00 PM, Matthew Wilcox [EMAIL PROTECTED] wrote:
On Tue, Jan 08, 2008 at 08:58:04PM +0100, Paolo Ciarrocchi wrote:
-static const struct file_operations rtc_fops = {
+static long rtc_fioctl(struct
On Jan 8, 2008 9:42 PM, Andi Kleen [EMAIL PROTECTED] wrote:
I grepped and tried to do what you suggested.
First a quick question: how would you rate your C knowledge? Did you
ever write a program yourself?
Yes I did but I probably beeing inactive for too much time,
I need to back studing a
On Jan 8, 2008 9:21 PM, Matthew Wilcox [EMAIL PROTECTED] wrote:
On Tue, Jan 08, 2008 at 01:16:06PM -0700, Matthew Wilcox wrote:
On Tue, Jan 08, 2008 at 09:03:13PM +0100, Paolo Ciarrocchi wrote:
Yes of course, I've been silly in didn't verify whether the file compile
but I would appreciate
I grepped and tried to do what you suggested.
First a quick question: how would you rate your C knowledge? Did you
ever write a program yourself?
My proposal assumes that you have at least basic C knowledge.
The first file that git grep reported was:
arch/arm/common/rtctime.c:static const
On Jan 9, 2008 12:06 AM, Andi Kleen [EMAIL PROTECTED] wrote:
I would suggest to only work on files that compile. e.g. do a
make allyesconfig
make -j$[$(grep -c processor /proc/cpuinfo)*2] 1 |tee LOG
(will probably take a long time)
first and then only modify files when
I would suggest to only work on files that compile. e.g. do a
make allyesconfig
make -j$[$(grep -c processor /proc/cpuinfo)*2] 1 |tee LOG (will
probably take a long time)
first and then only modify files when are mentioned in LOG
Actually since this will probably take very
Matthew Wilcox пишет:
On Tue, Jan 08, 2008 at 01:16:06PM -0700, Matthew Wilcox wrote:
On Tue, Jan 08, 2008 at 09:03:13PM +0100, Paolo Ciarrocchi wrote:
Yes of course, I've been silly in didn't verify whether the file compile
but I would appreciate to know whether I'm on the right track or not.
[EMAIL PROTECTED]:~/linux-2.6/mm$ grep struct file_operations *
shmem.c:static const struct file_operations shmem_file_operations;
shmem.c:static const struct file_operations shmem_file_operations = {
swapfile.c:static const struct file_operations proc_swaps_operations = {
Am I right in
Andi Kleen wrote:
Here's a proposal for some useful code transformations the kernel janitors
could do as opposed to running checkpatch.pl.
snip
I notice that every driver in drivers/ata uses a .ioctl that points to
ata_scsi_ioctl(). I could add the BKL to that function, and then change
all
On Tue, Jan 08, 2008 at 07:50:47PM -0400, Kevin Winchester wrote:
Andi Kleen wrote:
Here's a proposal for some useful code transformations the kernel janitors
could do as opposed to running checkpatch.pl.
snip
I notice that every driver in drivers/ata uses a .ioctl that points to
Andi Kleen wrote:
On Tue, Jan 08, 2008 at 07:50:47PM -0400, Kevin Winchester wrote:
Andi Kleen wrote:
Here's a proposal for some useful code transformations the kernel janitors
could do as opposed to running checkpatch.pl.
snip
I notice that every driver in drivers/ata uses a .ioctl that
Sorry about the noise here - I now notice that not all .ioctl function
pointers have the option of changing to .unlocked_ioctl. In this case,
the ioctl is in the struct scsi_host_template, rather than struct
file_operations.
I'll try to be a little more careful about the git grepping in
On Tuesday 08 January 2008, Andi Kleen wrote:
Thanks, Andi! I think it'd very useful change.
Reminds me this is something that should be actually flagged
in checkpatch.pl too
Andy, it would be good if checkpatch.pl complained about .ioctl =
as opposed to .unlocked_ioctl = ...
This is
On Wed, Jan 09, 2008 at 01:40:58AM +0100, Arnd Bergmann wrote:
On Tuesday 08 January 2008, Andi Kleen wrote:
Thanks, Andi! I think it'd very useful change.
Reminds me this is something that should be actually flagged
in checkpatch.pl too
Andy, it would be good if checkpatch.pl
On Wednesday 09 January 2008, Andi Kleen wrote:
I imagined it would check for
+struct file_operations ... = {
+ ...
+ .ioctl = ...
That wouldn't catch the case of someone adding only .ioctl to an
already existing file_operations which is not visible in the patch context,
Arnd Bergmann wrote:
On Wednesday 09 January 2008, Andi Kleen wrote:
I imagined it would check for
+struct file_operations ... = {
+ ...
+ .ioctl = ...
That wouldn't catch the case of someone adding only .ioctl to an
already existing file_operations which is not visible in
On Tue, Jan 08, 2008 at 09:31:24PM -0400, Kevin Winchester wrote:
Arnd Bergmann wrote:
On Wednesday 09 January 2008, Andi Kleen wrote:
I imagined it would check for
+struct file_operations ... = {
+ ...
+ .ioctl = ...
That wouldn't catch the case of someone adding
100 matches
Mail list logo
