Re: [PATCH 51/56] drivers/char/mem: support compiling out splice

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Thu, Nov 13, 2014 at 04:19:48PM -0800, josh@joshtriplett.org wrote:
> On Thu, Nov 13, 2014 at 03:34:16PM -0800, Greg Kroah-Hartman wrote:
> > On Thu, Nov 13, 2014 at 02:31:50PM -0800, josh@joshtriplett.org wrote:
> > > [Please don't top-post.]
> > > 
> > > On Thu, Nov 13, 2014 at 11:23:46PM +0100, Pieter Smith wrote:
> > > > Okay with moving the relevant functions to a new translation unit and
> > > > squashing it out in the Makefile
> > > 
> > > No, you don't need to do that either.  Mark pipe_to_null and
> > > splice_write_null as __maybe_unused, then wrap the initialization of
> > > .splice_write = splice_write_null to make it .splice_write =
> > > splice_p(splice_write_null).  That will avoid adding a single ifdef.
> > 
> > Again, ick, no.  You aren't saving anything "real" at all, just take out
> > the splice core code, leave the file pointer alone, and never do that
> > horrid "splice_p" stuff, ick ick ick.
> 
> Without doing the splice_p change (which should add zero lines of code,
> total diffstat of -3+3 in this case, just a couple of __maybe_unused
> tokens and a splice_p() in the initializer), the actual splice
> implementations for filesystems and drivers won't get thrown away.  I
> certainly agree that #ifdefs for those would be painful and not worth
> it.  However, what problem would the proposed __maybe_unused / splice_p
> cause?

I don't see what it buys you except churn and a constant need to keep
fixing up code that doesn't use it as new drivers get added.

It's also "not normal" when compared to all of the other function
pointers in the filesystem structure, what makes splice "special" here
that everyone now needs to know it should be treated differently?

> On the other hand, I can *definitely* understand not bothering with
> changing filesystems that nobody will use on a space-constrained system
> (e.g.  cluster filesystems); the patch series could likely be narrowed
> to just a half-dozen likely filesystems and drivers, all of which could
> be done separately from the initial series removing the core splice
> code.  Would that be more appealing?

As long as you aren't forcing every call-place to change, like this
patch series did, it would be better.

Also, no one ever posted how much space savings overall there was here,
so until that happens, I'm going to assume it isn't even worth the
effort, right?

thanks,

greg k-h