[PATCH 0/1] ns: introduce proc_get_ns_by_fd()
Konstantin Khlebnikov
khlebnikov at yandex-team.ru
Mon Sep 28 08:21:07 UTC 2015
On 25.09.2015 20:56, Oleg Nesterov wrote:
> On 09/25, Konstantin Khlebnikov wrote:
>>
>> +struct ns_common *proc_ns_fdget(int fd, int nstype, struct fd *fd_ref)
>> {
>> - struct file *file;
>> + struct ns_common *ns;
>> + struct fd f;
>>
>> - file = fget(fd);
>> - if (!file)
>> + f = fdget(fd);
>> + if (!f.file)
>> return ERR_PTR(-EBADF);
>>
>> - if (file->f_op != &ns_file_operations)
>> + if (f.file->f_op != &ns_file_operations)
>> + goto out_invalid;
>> +
>> + ns = get_proc_ns(file_inode(f.file));
>> + if (nstype && (ns->ops->type != nstype))
>> goto out_invalid;
>>
>> - return file;
>> + *fd_ref = f;
>> + return ns;
>>
>> out_invalid:
>> - fput(file);
>> + fdput(f);
>> return ERR_PTR(-EINVAL);
>> }
>
> Well yes, fdget() makes sense but this is minor.
>
> Honestly, I do not really like the new helper... I understand this
> is subjective, so I won't insist. But how about 1/1? We do not need
> fd/file at all. With this patch your sys_getvpid() can just use
> proc_get_ns_by_fd(fd, CLONE_NEWPID) and put_pid_ns().
Hmm. My version has 0 or 2 atomic ops per get-put sequence.
Your version: 2 or 4 atomic ops. Plus even in worst case pinning by
struct fd theoretically scales better because it touches refcount at
struct file: there're might be many of them for one namespace.
>
> Eric, what do you think?
>
> See also "TODO" in the changelog.
>
> Oleg.
>
--
Konstantin
More information about the Containers
mailing list