lkml.org 
[lkml]   [2007]   [Oct]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: per BDI dirty limit (was Re: -mm merge plans for 2.6.24)
    From
    Date
    On Sat, 2007-10-27 at 23:08 +0200, Kay Sievers wrote:
    > On Sat, 2007-10-27 at 09:02 -0700, Greg KH wrote:

    > > Ah, I see a few problems. Here, try this version instead. It's
    > > compile-tested only, and should be a lot simpler.
    > >
    > > Note, we still are not setting the parent to the new bdi structure
    > > properly, so the devices will show up in /sys/devices/virtual/ instead
    > > of in their proper location. To do this, we need the parent of the
    > > device, which I'm not so sure what it should be (block device? block
    > > device controller?)
    >
    > Assigning a parent device will only work with the upcoming conversion of
    > the raw kobjects in the block subsystem to "struct device".
    >
    > A few comments to the patch:
    >
    > > --- a/include/linux/string.h
    > > +++ b/include/linux/string.h
    > > @@ -8,6 +8,7 @@
    > > #include <linux/compiler.h> /* for inline */
    > > #include <linux/types.h> /* for size_t */
    > > #include <linux/stddef.h> /* for NULL */
    > > +#include <stdarg.h>
    > >
    > > #ifdef __cplusplus
    > > extern "C" {
    > > @@ -111,6 +112,9 @@ extern void *kmemdup(const void *src, si
    > > extern char **argv_split(gfp_t gfp, const char *str, int *argcp);
    > > extern void argv_free(char **argv);
    > >
    > > +char *kvprintf(const char *fmt, va_list args);
    > > +char *kprintf(const char *fmt, ...);
    >
    > Why is that here? I don't think we need this when we use the existing:
    > kvasprintf(GFP_KERNEL, fmt, args)

    Ignorance of the existance of said function. Thanks for pointing it out.
    (kobject_set_name ought to use it too I guess)

    > > --- a/mm/backing-dev.c
    > > +++ b/mm/backing-dev.c
    >
    > > +
    > > +static struct device_attribute bdi_dev_attrs[] = {
    > > + __ATTR(readahead, 0644, readahead_show, readahead_store),
    > > + __ATTR_RO(reclaimable),
    > > + __ATTR_RO(writeback),
    > > + __ATTR_RO(dirty),
    > > + __ATTR_RO(bdi_dirty),
    > > +};
    >
    > Default attributes will need the NULL termination back (see below).
    >
    > > +static __init int bdi_class_init(void)
    > > +{
    > > + bdi_class = class_create(THIS_MODULE, "bdi");
    > > + return 0;
    > > +}
    > > +
    > > +__initcall(bdi_class_init);
    > > +
    > > +int bdi_register(struct backing_dev_info *bdi, const char *fmt, ...)
    >
    > This function should accept a: "struct device *parent" and all callers
    > just pass NULL until the block layer conversion gets merged.

    Yeah, you're right, but I wanted to just get something working before
    bothering with the parent thing.

    > > +{
    > > + char *name;
    > > + va_list args;
    > > + int ret = -ENOMEM;
    > > + int i;
    > > +
    > > + va_start(args, fmt);
    > > + name = kvprintf(fmt, args);
    >
    > kvasprintf(GFP_KERNEL, fmt, args);
    >
    > > + va_end(args);
    > > +
    > > + if (!name)
    > > + return -ENOMEM;
    > > +
    > > + bdi->dev = device_create(bdi_class, NULL, MKDEV(0,0), name);
    >
    > The parent should be passed here.
    >
    > > + for (i = 0; i < ARRAY_SIZE(bdi_dev_attrs); i++) {
    > > + ret = device_create_file(bdi->dev, &bdi_dev_attrs[i]);
    > > + if (ret)
    > > + break;
    > > + }
    > > + if (ret) {
    > > + while (--i >= 0)
    > > + device_remove_file(bdi->dev, &bdi_dev_attrs[i]);
    > > + device_unregister(bdi->dev);
    > > + bdi->dev = NULL;
    > > + }
    >
    > All this open-coded attribute stuff should go away and be replaced by:
    > bdi_class->dev_attrs = bdi_dev_attrs;
    > Otherwise at event time the attributes are not created and stuff hooking
    > into the events will not be able to set values. Also, the core will do
    > proper add/remove and error handling then.

    ok, that's good to know. someone ought to write a book on how to use all
    this... really, even the functions are bare of documentation or
    comments.

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2007-10-27 23:39    [from the cache]
    ©2003-2014 Jasper Spaans. hosted at Digital Ocean