| 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
| ||
|
Message-ID: <46F22EF4.3060404@openvz.org>
Date: Thu, 20 Sep 2007 12:27:32 +0400
From: Pavel Emelyanov <xemul@...nvz.org>
To: "J. Bruce Fields" <bfields@...ldses.org>
CC: Andrew Morton <akpm@...l.org>,
Trond Myklebust <trond.myklebust@....uio.no>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
devel@...nvz.org
Subject: Re: [PATCH] Rework /proc/locks via seq_files and seq_list helpers
J. Bruce Fields wrote:
> On Wed, Sep 19, 2007 at 03:35:27PM +0400, Pavel Emelyanov wrote:
>> Currently /proc/locks is shown with a proc_read function, but
>> its behavior is rather complex as it has to manually handle
>> current offset and buffer length. On the other hand, files
>> that show objects from lists can be easily reimplemented using
>> the sequential files and the seq_list_XXX() helpers.
>>
>> This saves (as usually) 16 lines of code and more than 200 from
>> the .text section.
>>
>> This patch looks rather ugly, as diff often uses curly braces
>> as not-changed lines, but I haven't managed to organize the
>> code to make diff look better. Except for move the whole proc
>> related stuff upper/lower in the locks.c file...
>
> Fine by me. I've lost track--are you assuming some earlier patches are
> applied? It doesn't seem to apply to any copy of locks.c I have.
Yes, this applies the the set of patches that cleanup the
MANDATORY_LOCK macro (5 patches just accepted by Andrew).
> --b.
>
>> Signed-off-by: Pavel Emelyanov <xemul@...nvz.org>
>>
>> ---
>>
>> fs/locks.c | 124 ++++++++++++++++++++++------------------------------
>> fs/proc/proc_misc.c | 20 ++++----
>> 2 files changed, 64 insertions(+), 80 deletions(-)
>>
>> diff --git a/fs/locks.c b/fs/locks.c
>> index 8e849ed..746dc70 100644
>> --- a/fs/locks.c
>> +++ b/fs/locks.c
>> @@ -2032,134 +2032,116 @@ int vfs_cancel_lock(struct file *filp, s
>>
>> EXPORT_SYMBOL_GPL(vfs_cancel_lock);
>>
>> -static void lock_get_status(char* out, struct file_lock *fl, int id, char *pfx)
>> +#ifdef CONFIG_PROC_FS
>> +#include <linux/seq_file.h>
>> +
>> +static void lock_get_status(struct seq_file *f, struct file_lock *fl,
>> + int id, char *pfx)
>> {
>> struct inode *inode = NULL;
>>
>> if (fl->fl_file != NULL)
>> inode = fl->fl_file->f_path.dentry->d_inode;
>>
>> - out += sprintf(out, "%d:%s ", id, pfx);
>> + seq_printf(f, "%d:%s ", id, pfx);
>> if (IS_POSIX(fl)) {
>> - out += sprintf(out, "%6s %s ",
>> + seq_printf(f, "%6s %s ",
>> (fl->fl_flags & FL_ACCESS) ? "ACCESS" : "POSIX ",
>> (inode == NULL) ? "*NOINODE*" :
>> mandatory_lock(inode) ? "MANDATORY" : "ADVISORY ");
>> } else if (IS_FLOCK(fl)) {
>> if (fl->fl_type & LOCK_MAND) {
>> - out += sprintf(out, "FLOCK MSNFS ");
>> + seq_printf(f, "FLOCK MSNFS ");
>> } else {
>> - out += sprintf(out, "FLOCK ADVISORY ");
>> + seq_printf(f, "FLOCK ADVISORY ");
>> }
>> } else if (IS_LEASE(fl)) {
>> - out += sprintf(out, "LEASE ");
>> + seq_printf(f, "LEASE ");
>> if (fl->fl_type & F_INPROGRESS)
>> - out += sprintf(out, "BREAKING ");
>> + seq_printf(f, "BREAKING ");
>> else if (fl->fl_file)
>> - out += sprintf(out, "ACTIVE ");
>> + seq_printf(f, "ACTIVE ");
>> else
>> - out += sprintf(out, "BREAKER ");
>> + seq_printf(f, "BREAKER ");
>> } else {
>> - out += sprintf(out, "UNKNOWN UNKNOWN ");
>> + seq_printf(f, "UNKNOWN UNKNOWN ");
>> }
>> if (fl->fl_type & LOCK_MAND) {
>> - out += sprintf(out, "%s ",
>> + seq_printf(f, "%s ",
>> (fl->fl_type & LOCK_READ)
>> ? (fl->fl_type & LOCK_WRITE) ? "RW " : "READ "
>> : (fl->fl_type & LOCK_WRITE) ? "WRITE" : "NONE ");
>> } else {
>> - out += sprintf(out, "%s ",
>> + seq_printf(f, "%s ",
>> (fl->fl_type & F_INPROGRESS)
>> ? (fl->fl_type & F_UNLCK) ? "UNLCK" : "READ "
>> : (fl->fl_type & F_WRLCK) ? "WRITE" : "READ ");
>> }
>> if (inode) {
>> #ifdef WE_CAN_BREAK_LSLK_NOW
>> - out += sprintf(out, "%d %s:%ld ", fl->fl_pid,
>> + seq_printf(f, "%d %s:%ld ", fl->fl_pid,
>> inode->i_sb->s_id, inode->i_ino);
>> #else
>> /* userspace relies on this representation of dev_t ;-( */
>> - out += sprintf(out, "%d %02x:%02x:%ld ", fl->fl_pid,
>> + seq_printf(f, "%d %02x:%02x:%ld ", fl->fl_pid,
>> MAJOR(inode->i_sb->s_dev),
>> MINOR(inode->i_sb->s_dev), inode->i_ino);
>> #endif
>> } else {
>> - out += sprintf(out, "%d <none>:0 ", fl->fl_pid);
>> + seq_printf(f, "%d <none>:0 ", fl->fl_pid);
>> }
>> if (IS_POSIX(fl)) {
>> if (fl->fl_end == OFFSET_MAX)
>> - out += sprintf(out, "%Ld EOF\n", fl->fl_start);
>> + seq_printf(f, "%Ld EOF\n", fl->fl_start);
>> else
>> - out += sprintf(out, "%Ld %Ld\n", fl->fl_start,
>> - fl->fl_end);
>> + seq_printf(f, "%Ld %Ld\n", fl->fl_start, fl->fl_end);
>> } else {
>> - out += sprintf(out, "0 EOF\n");
>> + seq_printf(f, "0 EOF\n");
>> }
>> }
>>
>> -static void move_lock_status(char **p, off_t* pos, off_t offset)
>> +static int locks_show(struct seq_file *f, void *v)
>> {
>> - int len;
>> - len = strlen(*p);
>> - if(*pos >= offset) {
>> - /* the complete line is valid */
>> - *p += len;
>> - *pos += len;
>> - return;
>> - }
>> - if(*pos+len > offset) {
>> - /* use the second part of the line */
>> - int i = offset-*pos;
>> - memmove(*p,*p+i,len-i);
>> - *p += len-i;
>> - *pos += len;
>> - return;
>> - }
>> - /* discard the complete line */
>> - *pos += len;
>> -}
>> + int idx;
>> + struct file_lock *fl, *bfl;
>>
>> -/**
>> - * get_locks_status - reports lock usage in /proc/locks
>> - * @buffer: address in userspace to write into
>> - * @start: ?
>> - * @offset: how far we are through the buffer
>> - * @length: how much to read
>> - */
>> + fl = list_entry(v, struct file_lock, fl_link);
>> + idx = (int)f->private;
>>
>> -int get_locks_status(char *buffer, char **start, off_t offset, int length)
>> -{
>> - struct file_lock *fl;
>> - char *q = buffer;
>> - off_t pos = 0;
>> - int i = 0;
>> + lock_get_status(f, fl, idx, "");
>>
>> - lock_kernel();
>> - list_for_each_entry(fl, &file_lock_list, fl_link) {
>> - struct file_lock *bfl;
>> + list_for_each_entry(bfl, &fl->fl_block, fl_block)
>> + lock_get_status(f, bfl, idx, " ->");
>>
>> - lock_get_status(q, fl, ++i, "");
>> - move_lock_status(&q, &pos, offset);
>> + f->private = (void *)(idx + 1);
>> + return 0;
>> +}
>>
>> - if(pos >= offset+length)
>> - goto done;
>> +static void *locks_start(struct seq_file *f, loff_t *pos)
>> +{
>> + lock_kernel();
>> + f->private = (void *)1;
>> + return seq_list_start(&file_lock_list, *pos);
>> +}
>>
>> - list_for_each_entry(bfl, &fl->fl_block, fl_block) {
>> - lock_get_status(q, bfl, i, " ->");
>> - move_lock_status(&q, &pos, offset);
>> +static void *locks_next(struct seq_file *f, void *v, loff_t *pos)
>> +{
>> + return seq_list_next(v, &file_lock_list, pos);
>> +}
>>
>> - if(pos >= offset+length)
>> - goto done;
>> - }
>> - }
>> -done:
>> +static void locks_stop(struct seq_file *f, void *v)
>> +{
>> unlock_kernel();
>> - *start = buffer;
>> - if(q-buffer < length)
>> - return (q-buffer);
>> - return length;
>> }
>>
>> +struct seq_operations locks_seq_operations = {
>> + .start = locks_start,
>> + .next = locks_next,
>> + .stop = locks_stop,
>> + .show = locks_show,
>> +};
>> +#endif
>> +
>> /**
>> * lock_may_read - checks that the region is free of locks
>> * @inode: the inode that is being read
>> diff --git a/fs/proc/proc_misc.c b/fs/proc/proc_misc.c
>> index 166a6db..043621c 100644
>> --- a/fs/proc/proc_misc.c
>> +++ b/fs/proc/proc_misc.c
>> @@ -68,7 +68,6 @@ extern int get_stram_list(char *);
>> extern int get_filesystem_list(char *);
>> extern int get_exec_domain_list(char *);
>> extern int get_dma_list(char *);
>> -extern int get_locks_status (char *, char **, off_t, int);
>>
>> static int proc_calc_metrics(char *page, char **start, off_t off,
>> int count, int *eof, int len)
>> @@ -630,16 +629,19 @@ static int cmdline_read_proc(char *page,
>> return proc_calc_metrics(page, start, off, count, eof, len);
>> }
>>
>> -static int locks_read_proc(char *page, char **start, off_t off,
>> - int count, int *eof, void *data)
>> +extern struct seq_operations locks_seq_operations;
>> +static int locks_open(struct inode *inode, struct file *filp)
>> {
>> - int len = get_locks_status(page, start, off, count);
>> -
>> - if (len < count)
>> - *eof = 1;
>> - return len;
>> + return seq_open(filp, &locks_seq_operations);
>> }
>>
>> +static const struct file_operations proc_locks_operations = {
>> + .open = locks_open,
>> + .read = seq_read,
>> + .llseek = seq_lseek,
>> + .release = seq_release,
>> +};
>> +
>> static int execdomains_read_proc(char *page, char **start, off_t off,
>> int count, int *eof, void *data)
>> {
>> @@ -916,7 +918,6 @@ void __init proc_misc_init(void)
>> #endif
>> {"filesystems", filesystems_read_proc},
>> {"cmdline", cmdline_read_proc},
>> - {"locks", locks_read_proc},
>> {"execdomains", execdomains_read_proc},
>> {NULL,}
>> };
>> @@ -934,6 +935,7 @@ void __init proc_misc_init(void)
>> entry->proc_fops = &proc_kmsg_operations;
>> }
>> #endif
>> + create_seq_entry("locks", 0, &proc_locks_operations);
>> create_seq_entry("devices", 0, &proc_devinfo_operations);
>> create_seq_entry("cpuinfo", 0, &proc_cpuinfo_operations);
>> #ifdef CONFIG_BLOCK
>
-
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/
Powered by blists - more mailing lists