Re: [PATCH 14/21] FS-Cache: Avoid ENFILE checking for kernel-specific open files

[David Howells <dhowells@redhat.com>]

Christoph Hellwig <hch@infradead.org> wrote:

> > Make it possible to avoid ENFILE checking for kernel specific open files,
> > such as are used by the CacheFiles module.
> > 
> > After, for example, tarring up a kernel source tree over the network, the
> > CacheFiles module may easily have 20000+ files open in the backing
> > filesystem, thus causing all non-root processes to be given error ENFILE
> > when they try to open a file, socket, pipe, etc..
> 
> Big fat NACK/.  If you don't want file accouting don't use files.

File accounting is to prevent *userspace* from executing a DoS attack.

"struct file" is the interface I have to use.

I'd prefer to only have to deal with inodes or maybe dentries and inodes - but
various interfaces seem to require it.  I'm trying to keep the memory usage
down.

> The actual users of this unfortunately wasn't posted,

That's not my fault.  Trond or the mailing list seems to have lost patch 18/21
somewhere.

> but it's using files only as the optional arguments to ->readpage, to call
> ->flush and for ->setattr.  You could do all that aswell at the fs level.

No, I couldn't.  I'm trying to keep the VFS API changes down for the moment.

Besides, you're list of ops that seem to require it is incomplete.

> Besides that unposted patch adds various odd exports,

Yes, and?

> another silly write something variant in filemap.c, etc..

And how am I supposed to do this otherwise?  I don't want to muck about with
kiocb's and iovecs aren't any use, also I'm going to write one page and
exactly one page, so I can optimise it quite well.

I could move this function into cachefiles, but judging from past performance
you'd blow a gasket if I did.  Also, I'm not sure that all the functions it
calls are globally available.

> Also the documentation claims it needs FIBMAP support from userspace which
> is a big no-way.

Actually, I don't use FIBMAP at all.  I was just using that to guide people to
what I meant by bmap().

There isn't any way around using bmap() at the moment.  The current AIO
interface just isn't usable for this as is, and I have to detect holes somehow
(I'd *like* to add O_NOHOLE and use O_DIRECT, but that looks like a major
upheaval will be required in the VM/VFS).

This works for the moment, permitting the other bits to be tested more widely.

> Can we please get a clear description why all this bloat is needed?

You have one.  Read the description.  Then go and re-read the discussion back
on the 20th of April.

I'm sorry you don't have a new copy of the CacheFiles patch.  You can find it
in:

	http://people.redhat.com/~dhowells/nfs/nfs+fscache.tar.bz2

> Also please don't add _MODULE ifdefs in headers.

What choice do I have?  CONFIG_FSCACHE is not defined if CONFIG_FSCACHE_MODULE
is.

David