Re: [PATCH] 2.3.41 - cleanup file_operations structs

From: James Manning (jmm@raleigh.ibm.com)
Date: Sun Jan 30 2000 - 13:26:00 EST


[ Sunday, January 30, 2000 ] Ingo Oeser wrote:
> On Sat, 29 Jan 2000, James Manning wrote:
>
> > Changes:
> > - updated to 2.3.41
> > - cleaned up instances of leading spaces rather than tabs
> > - removed some benign garbage added the first time
> > - removed the 3 files from patch that were accepted
> > (arch/i386/kernel/apm.c, drivers/pci/proc.c, fs/udf/file.c)
> > - file count is now down from 171 to 168 :)
> > - 2429 LOC deleted - 980 LOC added = 1449 less kernel LOC
> >
> > http://sublogic.com/patches/file_operations-2.3.41.patch
>
> Nice patch. But why only file_operations? There are some/many
> other struct-initializers that need the same optimization.

I forgot to re-mention that I'd be doing the others (inode_operations
is next, BTW) as this patch does or doesn't get accepted. Since the
initializers are so close together, putting out multiple versions (diff
structs) at the same time would give lots of conflicting patches (a bad
thing :) so rather than confuse the issue, I think the best chance of
acceptance is to keep it simple (Linus/Alan may even want it broken
down into small sub-patches and if so I hope they ask :).

file_operations was first mainly because I saw the Arla group thinking
about adding a couple of entries, which made me start going through
what code was based on the current ordering and realizing that the
vast majority didn't even make use of this gcc-ism. :)

> The only places, where we _shouln't_ do this, are places not
> embedded in "#ifdef __KERNEL_"/"#endif /* __KERNEL__ */"
> protections, because this _can_ be used from user code, which is
> not required to use gcc (but kernel compilation is!).

Absolutely. Any user-space-visible structs should keep ordering
anyway just to keep the ABI stable, so that we're not forcing gcc
on user-space is just a side benefit.

> This feature is not ANSI-C, so it is gcc specific.

Yup, and since the kernel is gcc-specific anyway, we might as well
get some benefit from it :)

> Would you accept additional/incremental patches, to make this a
> "better_struct_initializer"-patch?

I'd rather keep it simple and see if we can get this accepted first.
Once the first one goes through and is accepted, the following ones should
be easy enough (I'm already done with inode_operations, for instance,
but want to create non-conflicting patches as per the above).

As it stands, even just file_operations is a very large patch, and
as the first time through only got 1.75% accepted, it's best to (imho)
work out any issues Linus/Alan/etc have with it now rather than flood
the list with patches that'll never get accepted.

Until then, I'll keep my fingers crossed and hope the savings of
~1.5k lines of code will show up on Linus' radar :)

James

-- 
Miscellaneous Engineer --- IBM Netfinity Performance Development

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Mon Jan 31 2000 - 21:00:25 EST