OpenVZ Forum - RDF feed
type: channel
items: [PATCH] Rewrite proc seq operations via seq_list_xxx ones
items: Re: [PATCH] Rewrite proc seq operations via seq_list_xxx ones

[PATCH] Rewrite proc seq operations via seq_list_xxx ones
type: item
description: Some time ago I posted a set of patches that consolidated the seq files, walking the list_head-s. This set was merged in the mm tree. /proc/diskstats and /proc/partitions files were included in this set, but a bit later this part was dropped because of conflicts with some other changes. Here's the fixed version against the latest git block repo. Signed-off-by: Pavel Emelyanov <xemul@openvz.org> --- diff --git a/block/genhd.c b/block/genhd.c index 3af1e7a..27f7061 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -264,39 +264,34 @@ void __init printk_all_partitions(void) #ifdef CONFIG_PROC_FS /* iterator */ -static void *part_start(struct seq_file *part, loff_t *pos) +static void *disk_start(struct seq_file *part, loff_t *pos) { - struct list_head *p; - loff_t l = *pos; - mutex_lock(&block_subsys_lock); - list_for_each(p, &block_subsys.list) - if (!l--) - return list_entry(p, struct gendisk, kobj.entry); - return NULL; + return seq_list_start_head(&block_subsys.list, *pos); } -static void *part_next(struct seq_file *part, void *v, loff_t *pos) +static void *disk_next(struct seq_file *part, void *v, loff_t *pos) { - struct list_head *p = ((struct gendisk *)v)->kobj.entry.next; - ++*pos; - return p==&block_subsys.list ? NULL : - list_entry(p, struct gendisk, kobj.entry); + return seq_list_next(v, &block_subsys.list, pos); } -static void part_stop(struct seq_file *part, void *v) +static void disk_stop(struct seq_file *part, void *v) { mutex_unlock(&block_subsys_lock); } static int show_partition(struct seq_file *part, void *v) { - struct gendisk *sgp = v; + struct gendisk *sgp; int n; char buf[BDEVNAME_SIZE]; - if (&sgp->kobj.entry == block_subsys.list.next) + if (v == &block_subsys.list) { seq_puts(part, "major minor #blocks name\n\n"); + return 0; + } + + sgp = list_entry(v, struct gendisk, kobj.entry); /* Don't show non-partitionable removeable devices or empty devices */ if (!get_capacity(sgp) || @@ -325,9 +320,9 @@ static int show_partition(struct seq_fil } struct …
creator: Pavel Emelianov
date: 2007-10-10T11:10:40-00:00

Re: [PATCH] Rewrite proc seq operations via seq_list_xxx ones
type: item
description: * Pavel Emelyanov (xemul@openvz.org) wrote: > Some time ago I posted a set of patches that consolidated > the seq files, walking the list_head-s. This set was merged > in the mm tree. /proc/diskstats and /proc/partitions files > were included in this set, but a bit later this part was > dropped because of conflicts with some other changes. > > Here's the fixed version against the latest git block repo. > Hi Pavel, You might want to try implementing this patch on top of the sorted seq list patch I proposed there. It deals with a race between proc file read and list modification between iterations. http://lkml.org/lkml/2007/9/6/175 See this patch for a user implementation example and test code (/proc/modules) : http://lkml.org/lkml/2007/9/6/176 Mathieu > Signed-off-by: Pavel Emelyanov <xemul@openvz.org> > > --- > > diff --git a/block/genhd.c b/block/genhd.c > index 3af1e7a..27f7061 100644 > --- a/block/genhd.c > +++ b/block/genhd.c > @@ -264,39 +264,34 @@ void __init printk_all_partitions(void) > > #ifdef CONFIG_PROC_FS > /* iterator */ > -static void *part_start(struct seq_file *part, loff_t *pos) > +static void *disk_start(struct seq_file *part, loff_t *pos) > { > - struct list_head *p; > - loff_t l = *pos; > - > mutex_lock(&block_subsys_lock); > - list_for_each(p, &block_subsys.list) > - if (!l--) > - return list_entry(p, struct gendisk, kobj.entry); > - return NULL; > + return seq_list_start_head(&block_subsys.list, *pos); > } > > -static void *part_next(struct seq_file *part, void *v, loff_t *pos) > +static void *disk_next(struct seq_file *part, void *v, loff_t *pos) > { > - struct list_head *p = ((struct gendisk *)v)->kobj.entry.next; > - ++*pos; > - return p==&block_subsys.list ? NULL : > - list_entry(p, struct gendisk, kobj.entry); > + return seq_list_next(v, &block_subsys.list, pos); > } > > -static void part_stop(struct seq_file *part, void *v) > +static void disk_stop(struct seq_file *part, void *v) > { > mutex_unlock(&block_subsys_lock); > } > > static int…
creator: Mathieu Desnoyers
date: 2007-10-10T14:45:26-00:00

Re: [PATCH] Rewrite proc seq operations via seq_list_xxx ones
type: item
description: Mathieu Desnoyers wrote: > * Pavel Emelyanov (xemul@openvz.org) wrote: >> Some time ago I posted a set of patches that consolidated >> the seq files, walking the list_head-s. This set was merged >> in the mm tree. /proc/diskstats and /proc/partitions files >> were included in this set, but a bit later this part was >> dropped because of conflicts with some other changes. >> >> Here's the fixed version against the latest git block repo. >> > > Hi Pavel, > > You might want to try implementing this patch on top of the sorted seq > list patch I proposed there. It deals with a race between proc file read > and list modification between iterations. AFAIS there's no race - everything is protected with the mutex. OTOH, If you mean the problem that the list can change while the seq file reallocates its buffers, then this problem exists for all the kernel and I don't think it's a good idea to stick to the block proc files only. > http://lkml.org/lkml/2007/9/6/175 > > See this patch for a user implementation example and test code > (/proc/modules) : > > http://lkml.org/lkml/2007/9/6/176 > > Mathieu > >> Signed-off-by: Pavel Emelyanov <xemul@openvz.org> >> >> --- >> >> diff --git a/block/genhd.c b/block/genhd.c >> index 3af1e7a..27f7061 100644 >> --- a/block/genhd.c >> +++ b/block/genhd.c >> @@ -264,39 +264,34 @@ void __init printk_all_partitions(void) >> >> #ifdef CONFIG_PROC_FS >> /* iterator */ >> -static void *part_start(struct seq_file *part, loff_t *pos) >> +static void *disk_start(struct seq_file *part, loff_t *pos) >> { >> - struct list_head *p; >> - loff_t l = *pos; >> - >> mutex_lock(&block_subsys_lock); >> - list_for_each(p, &block_subsys.list) >> - if (!l--) >> - return list_entry(p, struct gendisk, kobj.entry); >> - return NULL; >> + return seq_list_start_head(&block_subsys.list, *pos); >> } >> >> -static void *part_next(struct seq_file *part, void *v, loff_t *pos) >> +static void *disk_next(struct seq_file *part, void *v, loff_t *pos) >> { >> - struct list_head *p…
creator: Pavel Emelianov
date: 2007-10-10T14:56:42-00:00

Re: [PATCH] Rewrite proc seq operations via seq_list_xxx ones
type: item
description: * Pavel Emelyanov (xemul@openvz.org) wrote: > Mathieu Desnoyers wrote: > > * Pavel Emelyanov (xemul@openvz.org) wrote: > >> Some time ago I posted a set of patches that consolidated > >> the seq files, walking the list_head-s. This set was merged > >> in the mm tree. /proc/diskstats and /proc/partitions files > >> were included in this set, but a bit later this part was > >> dropped because of conflicts with some other changes. > >> > >> Here's the fixed version against the latest git block repo. > >> > > > > Hi Pavel, > > > > You might want to try implementing this patch on top of the sorted seq > > list patch I proposed there. It deals with a race between proc file read > > and list modification between iterations. > > AFAIS there's no race - everything is protected with the mutex. > > OTOH, If you mean the problem that the list can change while the > seq file reallocates its buffers, then this problem exists for all > the kernel and I don't think it's a good idea to stick to the block > proc files only. > The problem is that the list can change between two consecutive reads. And yes, all proc files using lists are affected, but changing every proc file users is a quite large task. A problematic sequence would be: 1st procfs read() *_start() - mutex down - seq list start *_next() - seq list next *_stop() - mutex up end of 1st procfs read() mutex down -> Removing a list entry mutex up 2nd procfs read() *_start() - mutex down ... we now skip a list entry depending on the relative position of our iterator and the deletion. The mutex is taken/released at each and every read, therefore not protecting against modifications across reads. Mathieu > > http://lkml.org/lkml/2007/9/6/175 > > > > See this patch for a user implementation example and test code > > (/proc/modules) : > > > > http://lkml.org/lkml/2007/9/6/176 > > > > Mathieu > > > >> Signed-off-by: Pavel Emelyanov <xemul@openvz.org> > >> > >> --- > >> > >> diff --git a/block/genhd.c b/block/genhd.c > >> index 3af1e…
creator: Mathieu Desnoyers
date: 2007-10-10T15:10:49-00:00