| Submitter | Sedat Dilek |
|---|---|
| Date | April 3, 2011, 11:55 p.m. |
| Message ID | <1301874939-2630-1-git-send-email-sedat.dilek@gmail.com> |
| Download | mbox | patch |
| Permalink | /patch/89571/ |
| State | New |
| Headers | show |
Comments
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 <sedat.dilek@gmail.com> > CC: Artem Bityutskiy <dedekind1@gmail.com> > CC: Adrian Hunter <adrian.hunter@nokia.com> > CC: Jens Axboe <jaxboe@fusionio.com> > --- > 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?
On Mon, Apr 4, 2011 at 9:04 AM, Artem Bityutskiy <dedekind1@gmail.com> 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 <sedat.dilek@gmail.com> >> CC: Artem Bityutskiy <dedekind1@gmail.com> >> CC: Adrian Hunter <adrian.hunter@nokia.com> >> CC: Jens Axboe <jaxboe@fusionio.com> >> --- >> 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 -
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.
On Mon, Apr 4, 2011 at 12:06 PM, Artem Bityutskiy <dedekind1@gmail.com> 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 -
On Mon, Apr 4, 2011 at 12:29 PM, Sedat Dilek <sedat.dilek@googlemail.com> wrote: > On Mon, Apr 4, 2011 at 12:06 PM, Artem Bityutskiy <dedekind1@gmail.com> 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 -
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
On Tue, Apr 19, 2011 at 12:02 PM, Artem Bityutskiy <dedekind1@gmail.com> 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 (Артём Битюцкий) > >
Patch
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);
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 <sedat.dilek@gmail.com> CC: Artem Bityutskiy <dedekind1@gmail.com> CC: Adrian Hunter <adrian.hunter@nokia.com> CC: Jens Axboe <jaxboe@fusionio.com> --- fs/ubifs/xattr.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)