linux-kernel - Re: [PATCH, RFC] xfs: batched discard support

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090821124638.GC10263@elte.hu>
Date:	Fri, 21 Aug 2009 14:46:38 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Christoph Hellwig <hch@...radead.org>
Cc:	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Paul Mackerras <paulus@...ba.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	xfs@....sgi.com, linux-fsdevel@...r.kernel.org,
	linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
	liml@....ca, jens.axboe@...cle.com
Subject: Re: [PATCH, RFC] xfs: batched discard support


* Christoph Hellwig <hch@...radead.org> wrote:

> On Wed, Aug 19, 2009 at 10:39:16PM +0200, Ingo Molnar wrote:
> > A general interface design question: you added a new 
> > ioctl XFS_IOC_TRIM case. It's a sub-case of an 
> > ugly-looking demultiplexing xfs_file_ioctl().
> 
> ioctl is per defintion a multiplexer.

Yes, and? There's two variants of multiplexing:

  - multiplex something rather straightforward and related
  - multiplex unrelated fields, data, structures

I consider the second one 'ugly', the first one 'ok-ish'. YMMV.

> > What is your threshold for turning something into a syscall? 
> > When are ioctls acceptable in your opinion?
> > 
> > I'm asking this because we are facing a similar problem with 
> > perfcounters: we need to extend the ioctl functionality there 
> > but introducing a new syscall looks wasteful.
> > 
> > So i'm torn about the 'syscall versus ioctl' issue, i'd like to 
> > avoid making interface design mistakes and i'd like to solicit 
> > some opinions about this. I've attached the perfcounters ioctl 
> > patch below.
> 
> Only add a syscall if it has _one_ clear defined purpose, which 
> has kernel-wide meaning.
> 
> Do not add an syscall that is just another multiplexer without 
> structure. Most likely it will just be even worse than sys_ioctl.
> 
> Also really don't bother adding a system call that is specific to 
> one singler driver or filesystem.  Besides horrible logistics - 
> you'd need some always built-in stub calling out to the possibly 
> modular drivers/filesystem - it also simply doesn't make any 
> semantical sense.  I can't say I like the ioctl use in 
> perfcounters much, but adding a special syscalls instead would be 
> even more horrible.
> 
> As for the trim support this really just was an RFC to start 
> bringing some code into play instead of the endless masturbation 
> about hat code that doesn't exist happens on hardware most people 
> don't have.  The interface will most ceetainly change and I hope 
> we will have a common interface for all filesystems (or at least 
> those that care).

Okay, i'm confused.

I'd like to understand the technical basis of your critisism and i'd 
like to address any deficiencies of the perfcounters code. You said 
you dont like the ioctl solution we have, but that you'd like a 
separate syscall even less.

Perfcounters are a kernel-wide concept, encompassing 100% of all 
Linux installations, not just some special hardware.

So by your own standard above they seem to be more than eligible for 
system calls (i hope i'm not mis-stating it), as long as they are 
cleanly structured. Yet you dont like the interface nor any future 
pushing of the currently ioctl bits of the interface into syscalls.

Is there any other interface form you'd like more?

Thanks,

	Ingo
--
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/