Re: First kernel patch (optimization)

[Theodore Ts'o]

On Sat, Sep 19, 2015 at 07:47:22PM +0200, Alexander Holler wrote:
> No. I don't want to lower the standards. Maybe in regard to silly style
> stuff, but not in regard to code quality (and I mean real bugs like races,
> deadlocks or such, and not if a line has more than 80 characters). I would
> have liked some comments like "good or bad idea" but this list is imho the
> wrong place to search for such useful comments. I haven't searched for
> comments on the code, as I was FULLY aware that the code is ugly and NOT
> ready for inclusion),

That was asked and answered on the original thread, but perhaps it got
lost amongst some of the noise --- some of which was contributed by
yourself, but be that as it may.

The *feature* is in and of itself not an insane idea.  "Secure delete"
is something that Linux has had before (in fact I did the original
implementation for ext2 a decade+ ago), and indeed the interface,
using the FS_SECRM_FL flag, is still present in Linux even if it is
not supported by any of the file systems in the tree at the moment.
It got ripped out because doing it right in the face of ongoing
interface changes and performance improvements (going back to the
pre-2.0, and possibly the pre-1.2 days), it was simpler to rip it out
rather than to reimplement it.

I wasn't responsible for ripping it out, back in mists of pre-history,
but I didn't care enough to reimplement it --- or complain about it,
either.  And no one else cared enough to complain to Linus about this
being a user-visible change that broke them.  So it's a great example
of how a feature and functionality that no one cares about *can* get
ripped out of the kernel.

Perhaps not so surprisingly, over a decade later, it is not currently
at the top of the priority list of any of the current file system or
VFS developers, as far as I know.  One of the reasons for that is that
there are a number of other ways of achieving the same functionality.
These include using tmpfs, or using file system level encryption.
They require a bit more system administrator setup than just being
able to set the FS_SECR_FL flag, true, but just because it's more
convenient doesn't mean that it's worth doing.

So.... this is a feature request.  It's a reasonable feature request,
in that if someone would like to pay $$$ for some consultant to
implement it in a way that is bug-free, I suspect it could go
upstream.  Someone who was very motivated and with the sufficient
skills could also invest their own effort to make a patch that can go
upstream too.  You've elected not, to because you believe it would
take you months of "unpaid time".  That's purely within your rights to
do.  But you don't have the right to try to tell other people what
work to do on their behalf --- not unless you are paying their salary.


And of course, if you want to maintain an out-of-tree patch, there's
nothing wrong with that.  I've implemented code that has never gone
upstream because the weight of the other file system developers were
afraid that Enterise Linux customers would use the feature
incorrectly, and it would cause potentially cause a security failure
if used inappropriately, which would be a PR and support nightmare for
them.

Seeing that the weight of the other file system developers are against
the patch, it's never gone into the mainline Linux kernel, even though
I could have forced the feature into ext4.  However, this patch is in
active use in practically every single data center kernel for Google,
and it's in use in at least one other very large publically traded
company that uses cluster file systems such as Hadoopfs.  And if
someone wants a copy of the FALLOC_FL_NO_HIDE_STALE patch for ext4,
I'm happy to give it to them.  But it hasn't gone upstream, and I'm OK
with that.

As far as what you want to do next, you have a personal "proof of
concept" patch that seems to work well enough for you.  Great!  I'm
sure you can keep using it for your own purposes.  If you can convince
someone with the skills to get the patch to an upstreamable state, it
is my judgement that this is doable, so this puts your feature in a
much better state than the FALLOC_FL_NO_HIDE_STALE flag.  However,
there is still a non-trivial amount of work left to do to turn your
"proof of concept" patch into something that is upstremable, including
changing the interface to using the FS_SECRM_FL flag.  And your
whining that other people should change *their* priorities to match
*yours* is not likely to help.

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