2011-04-03 23:55:55

by Sedat Dilek

[permalink] [raw]
Subject: [PATCH] ubifs: Use empty_iops and empty_fops consistently

Follow the nomenclature as we did for empty_aops and rename
to empty_iops and empty_fops.

This patch is on top of linux-2.6-block.git#for-linus.

Signed-off-by: Sedat Dilek <[email protected]>
CC: Artem Bityutskiy <[email protected]>
CC: Adrian Hunter <[email protected]>
CC: Jens Axboe <[email protected]>
---
fs/ubifs/xattr.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c
index 3299f46..16f19f5 100644
--- a/fs/ubifs/xattr.c
+++ b/fs/ubifs/xattr.c
@@ -80,8 +80,8 @@ enum {
SECURITY_XATTR,
};

-static const struct inode_operations none_inode_operations;
-static const struct file_operations none_file_operations;
+static const struct inode_operations empty_iops;
+static const struct file_operations empty_fops;

/**
* create_xattr - create an extended attribute.
@@ -131,8 +131,8 @@ static int create_xattr(struct ubifs_info *c, struct inode *host,

/* Re-define all operations to be "nothing" */
inode->i_mapping->a_ops = &empty_aops;
- inode->i_op = &none_inode_operations;
- inode->i_fop = &none_file_operations;
+ inode->i_op = &empty_iops;
+ inode->i_fop = &empty_fops;

inode->i_flags |= S_SYNC | S_NOATIME | S_NOCMTIME | S_NOQUOTA;
ui = ubifs_inode(inode);
--
1.7.4.3


2011-04-04 07:06:47

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: [PATCH] ubifs: Use empty_iops and empty_fops consistently

On Mon, 2011-04-04 at 01:55 +0200, Sedat Dilek wrote:
> Follow the nomenclature as we did for empty_aops and rename
> to empty_iops and empty_fops.
>
> This patch is on top of linux-2.6-block.git#for-linus.
>
> Signed-off-by: Sedat Dilek <[email protected]>
> CC: Artem Bityutskiy <[email protected]>
> CC: Adrian Hunter <[email protected]>
> CC: Jens Axboe <[email protected]>
> ---
> fs/ubifs/xattr.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c
> index 3299f46..16f19f5 100644
> --- a/fs/ubifs/xattr.c
> +++ b/fs/ubifs/xattr.c
> @@ -80,8 +80,8 @@ enum {
> SECURITY_XATTR,
> };
>
> -static const struct inode_operations none_inode_operations;
> -static const struct file_operations none_file_operations;
> +static const struct inode_operations empty_iops;
> +static const struct file_operations empty_fops;
>
> /**
> * create_xattr - create an extended attribute.
> @@ -131,8 +131,8 @@ static int create_xattr(struct ubifs_info *c, struct inode *host,
>
> /* Re-define all operations to be "nothing" */
> inode->i_mapping->a_ops = &empty_aops;
> - inode->i_op = &none_inode_operations;
> - inode->i_fop = &none_file_operations;
> + inode->i_op = &empty_iops;
> + inode->i_fop = &empty_fops;
>
> inode->i_flags |= S_SYNC | S_NOATIME | S_NOCMTIME | S_NOQUOTA;
> ui = ubifs_inode(inode);

Well, this re-naming looks strange for me. I could understand if you had
a global exported empty_*ops which you could share and shrink the .data
size a bit. But this re-naming just looks useles, if you take into
account the breakage and the men-hours we spend to write e-mails to this
thread - really did not worth it. Or I miss something?

--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

2011-04-04 09:38:44

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH] ubifs: Use empty_iops and empty_fops consistently

On Mon, Apr 4, 2011 at 9:04 AM, Artem Bityutskiy <[email protected]> wrote:
> On Mon, 2011-04-04 at 01:55 +0200, Sedat Dilek wrote:
>> Follow the nomenclature as we did for empty_aops and rename
>> to empty_iops and empty_fops.
>>
>> This patch is on top of linux-2.6-block.git#for-linus.
>>
>> Signed-off-by: Sedat Dilek <[email protected]>
>> CC: Artem Bityutskiy <[email protected]>
>> CC: Adrian Hunter <[email protected]>
>> CC: Jens Axboe <[email protected]>
>> ---
>>  fs/ubifs/xattr.c |    8 ++++----
>>  1 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c
>> index 3299f46..16f19f5 100644
>> --- a/fs/ubifs/xattr.c
>> +++ b/fs/ubifs/xattr.c
>> @@ -80,8 +80,8 @@ enum {
>>       SECURITY_XATTR,
>>  };
>>
>> -static const struct inode_operations none_inode_operations;
>> -static const struct file_operations none_file_operations;
>> +static const struct inode_operations empty_iops;
>> +static const struct file_operations empty_fops;
>>
>>  /**
>>   * create_xattr - create an extended attribute.
>> @@ -131,8 +131,8 @@ static int create_xattr(struct ubifs_info *c, struct inode *host,
>>
>>       /* Re-define all operations to be "nothing" */
>>       inode->i_mapping->a_ops = &empty_aops;
>> -     inode->i_op = &none_inode_operations;
>> -     inode->i_fop = &none_file_operations;
>> +     inode->i_op = &empty_iops;
>> +     inode->i_fop = &empty_fops;
>>
>>       inode->i_flags |= S_SYNC | S_NOATIME | S_NOCMTIME | S_NOQUOTA;
>>       ui = ubifs_inode(inode);
>
> Well, this re-naming looks strange for me. I could understand if you had
> a global exported empty_*ops which you could share and shrink the .data
> size a bit. But this re-naming just looks useles, if you take into
> account the breakage and the men-hours we spend to write e-mails to this
> thread - really did not worth it. Or I miss something?
>

Sorry for having flooded your inbox.
The last days might be the first time I noticed UBIFS at all and
looked into its code.
I did so because I fell over a build-error when pulling
linux-block-2.6.git#for-linus into linux-next (next-20110404).

The current code in linux-next (next-20110404) looks in the meantime like this:

[ fs/ubifs/xattr.c ]
...
static const struct inode_operations none_inode_operations;
static const struct file_operations none_file_operations;
...
/* Re-define all operations to be "nothing" */
inode->i_mapping->a_ops = &empty_aops;
inode->i_op = &none_inode_operations;
inode->i_fop = &none_file_operations;
...

First seeing this was strange to me, so you try via code-grep-ping to
understand the correlations in the complete fs-tree.
So, let's do some code-grep:

$ egrep 'empty_aops|mpty_iops|empty_fops' -r fs/ include/linux/
fs/ubifs/xattr.c: inode->i_mapping->a_ops = &empty_aops;
fs/open.c: static const struct file_operations empty_fops = {};
fs/open.c: f->f_op = &empty_fops;
fs/inode.c:const struct address_space_operations empty_aops = {
fs/inode.c:EXPORT_SYMBOL(empty_aops);
fs/inode.c: static const struct inode_operations empty_iops;
fs/inode.c: static const struct file_operations empty_fops;
fs/inode.c: inode->i_op = &empty_iops;
fs/inode.c: inode->i_fop = &empty_fops;
fs/inode.c: mapping->a_ops = &empty_aops;
fs/nilfs2/page.c: mapping->a_ops = &empty_aops;
include/linux/fs.h:extern const struct address_space_operations empty_aops;

Hope you can understand now, why I did this.

It's up to you how you name your variables, function-names etc.

- Sedat -

2011-04-04 10:09:17

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: [PATCH] ubifs: Use empty_iops and empty_fops consistently

On Mon, 2011-04-04 at 11:38 +0200, Sedat Dilek wrote:
> Sorry for having flooded your inbox.

That's ok! :-) I think I did not look carefully enough. Your patch is
just an additional clean-up which makes the names to be consistent. I
can pick it, but probably it is better if it goes in with the
'empty_aops' changes? Whatever you prefer.

--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

2011-04-04 10:29:49

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH] ubifs: Use empty_iops and empty_fops consistently

On Mon, Apr 4, 2011 at 12:06 PM, Artem Bityutskiy <[email protected]> wrote:
> On Mon, 2011-04-04 at 11:38 +0200, Sedat Dilek wrote:
>> Sorry for having flooded your inbox.
>
> That's ok! :-) I think I did not look carefully enough. Your patch is
> just an additional clean-up which makes the names to be consistent. I
> can pick it, but probably it is better if it goes in with the
> 'empty_aops' changes? Whatever you prefer.
>

I dunno the internal organization between block-tree and fs-sub-tree
maintainers.
As I was in contact first with Jens in this issue, it might be a good
idea to let this patch go through his block-tree.

- Sedat -

2011-04-19 09:50:24

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH] ubifs: Use empty_iops and empty_fops consistently

On Mon, Apr 4, 2011 at 12:29 PM, Sedat Dilek <[email protected]> wrote:
> On Mon, Apr 4, 2011 at 12:06 PM, Artem Bityutskiy <[email protected]> wrote:
>> On Mon, 2011-04-04 at 11:38 +0200, Sedat Dilek wrote:
>>> Sorry for having flooded your inbox.
>>
>> That's ok! :-) I think I did not look carefully enough. Your patch is
>> just an additional clean-up which makes the names to be consistent. I
>> can pick it, but probably it is better if it goes in with the
>> 'empty_aops' changes? Whatever you prefer.
>>
>
> I dunno the internal organization between block-tree and fs-sub-tree
> maintainers.
> As I was in contact first with Jens in this issue, it might be a good
> idea to let this patch go through his block-tree.
>
> - Sedat -
>

Hi Artem,

can you take this patch into your UBIFS tree?
Thanks in advance.

- Sedat -

2011-04-19 10:04:57

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: [PATCH] ubifs: Use empty_iops and empty_fops consistently

On Tue, 2011-04-19 at 11:50 +0200, Sedat Dilek wrote:
> can you take this patch into your UBIFS tree?
> Thanks in advance.

Sorry, forgot about this patch. Pushed to ubifs-2.6.git tree, but I
re-wrote the commit message because I found yours a bit confusing. Is
this OK?

http://git.infradead.org/ubifs-2.6.git/commit/8c355ad269c9da635fad8367e9a6d52bf3fe08c8

--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

2011-04-19 10:21:41

by Sedat Dilek

[permalink] [raw]
Subject: Re: [PATCH] ubifs: Use empty_iops and empty_fops consistently

On Tue, Apr 19, 2011 at 12:02 PM, Artem Bityutskiy <[email protected]> wrote:
> On Tue, 2011-04-19 at 11:50 +0200, Sedat Dilek wrote:
>> can you take this patch into your UBIFS tree?
>> Thanks in advance.
>
> Sorry, forgot about this patch. Pushed to ubifs-2.6.git tree, but I
> re-wrote the commit message because I found yours a bit confusing. Is
> this OK?
>

Thanks.
Nice wording, fire it up :-).

> http://git.infradead.org/ubifs-2.6.git/commit/8c355ad269c9da635fad8367e9a6d52bf3fe08c8
>
> --
> Best Regards,
> Artem Bityutskiy (Артём Битюцкий)
>
>