Messages in this thread | | | Date | Thu, 13 Nov 2014 14:24:22 -0800 | From | josh@joshtrip ... | Subject | Re: [PATCH 04/56] fs: Macros to define splice file_operations |
| |
On Thu, Nov 13, 2014 at 10:49:07PM +0100, Richard Weinberger wrote: > On Thu, Nov 13, 2014 at 10:22 PM, Pieter Smith <pieter@boesman.nl> wrote: > > Provides a CONFIG_SYSCALL_SPLICE compatible way of defining the .splice_read > > and .splice_write file_operations so that they can later be compiled out when > > the kernel is configured without the splice-family syscalls [...] > > --- a/include/linux/fs.h > > +++ b/include/linux/fs.h > > @@ -1512,6 +1512,32 @@ struct file_operations { > > int (*show_fdinfo)(struct seq_file *m, struct file *f); > > }; > > > > +#ifdef CONFIG_SYSCALL_SPLICE > > + > > +/* > > + * Define and init the splice_read member of a file_operations struct > > + */ > > +#define SPLICE_READ_INIT(read) .splice_read = read, > > + > > +/* > > + * Define and init the splice_read member of a file_operations struct > > + */ > > +#define SPLICE_WRITE_INIT(write) .splice_write = write, > > This is ugly like hell. > Why can't you do something like __exit_p()?
On Thu, Nov 13, 2014 at 09:51:39PM +0000, Al Viro wrote: > This (and subsequent stuff making use of that) is bloody pointless. You > save 2 words per file_operations instance, at the cost of making things > uglier and harder to grep. NAK.
Given the large number of uses of these, I agree that it doesn't seem worth the tradeoff, particularly since very few file_operations structures will exist on any individual tiny configuration. I think we should go with a wrapper similar to __exit_p (splice_p?), which just becomes NULL when !CONFIG_SYSCALL_SPLICE. Removing the actual pointers from file_operations can wait until we have compiler support for tagging specific fields in a structure (like splice_read and splice_write) as dead.
Similarly, you shouldn't wrap the functions that get assigned to those pointers with #ifdef; instead, mark them as __maybe_unused, which doesn't even add any lines of code. The compiler will then automatically throw them out when not used, without emiting a warning.
That should drastically reduce the number of changes, and in particular eliminate almost all of the ifdefs.
- Josh Triplett
| |