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

Andrew Morton
2006-04-21T00:08:58Z

David Howells <dhowells@redhat.com> 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..
>
> ...
>
>
> static struct percpu_counter nr_files __cacheline_aligned_in_smp;
> +static atomic_t nr_kernel_files;

So it's not performance-critical.

> -struct file *get_empty_filp(void)
> +struct file *get_empty_filp(int kernel)

I'd suggest a new get_empty_kernel_filp(void) rather than providing a magic
argument. (we can still have the magic argument in the new
__get_empty_filp(int), but it shouldn't be part of the caller-visible API).

> + if (!kernel) {
> + if (get_nr_files() >= files_stat.max_files &&
> + !capable(CAP_SYS_ADMIN)
> + ) {

ugly.

> + f->f_kernel_flags = kernel ? FKFLAGS_KERNEL : 0;

It would be more flexible to make the caller pass in the flags directly.

> f->f_uid = tsk->fsuid;
> f->f_gid = tsk->fsgid;
> eventpoll_init_file(f);
> @@ -235,6 +250,7 @@ struct file fastcall *fget_light(unsigne
> return file;
> }
>
> +EXPORT_SYMBOL(fget_light);

fget_light is not otherwise referenced in this patch.

this change is not changelogged.

why non-GPL?

> +EXPORT_SYMBOL(dentry_open_kernel);

_GPL?

> @@ -640,6 +643,7 @@ struct file {
> atomic_t f_count;
> unsigned int f_flags;
> mode_t f_mode;
> + unsigned short f_kernel_flags;
> loff_t f_pos;
> struct fown_struct f_owner;
> unsigned int f_uid, f_gid;

That's unfortunate. There's still room in f_flags. Was it hard to use that?

> @@ -943,7 +943,7 @@ static ctl_table fs_table[] = {
> .ctl_name = FS_NRFILE,
> .procname = "file-nr",
> .data = &files_stat,
> - .maxlen = 3*sizeof(int),
> + .maxlen = 4*sizeof(int),
> .mode = 0444,
> .proc_handler = &proc_nr_files,

This changes the format of /proc/sys/fs/file-nr. What will break?