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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20141114032722.GA31174@kroah.com>
Date:	Thu, 13 Nov 2014 19:27:22 -0800
From:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:	josh@...htriplett.org
Cc:	Pieter Smith <pieter@...sman.nl>, Arnd Bergmann <arnd@...db.de>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 51/56] drivers/char/mem: support compiling out splice

On Thu, Nov 13, 2014 at 04:19:48PM -0800, josh@...htriplett.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@...htriplett.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/