Andrew, can you pick this up this please?
Thanks,
Kay
From: Kay Sievers <[email protected]>
Subject: bdi: register sysfs bdi device only once per queue
Devices which share the same queue, like floppies and mtd devices,
get registered multiple times in the bdi interface, but bdi accounts
only the last registered device of the devices sharing one queue.
On remove, all earlier registered devices leak, stay around in
sysfs, and cause "duplicate filename" errors if the devices are
re-created.
This prevents the creation of multiple bdi interfaces per queue,
and the bdi device will carry the dev_t name of the block device
which is the first one registered, of the pool of devices using
the same queue.
Tested-By: Peter Korsgaard <[email protected]>
Acked-By: Peter Zijlstra <[email protected]>
Signed-Off-By: Kay Sievers <[email protected]>
---
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index f2e574d..e6676e5 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -176,6 +176,9 @@ int bdi_register(struct backing_dev_info *bdi, struct device *parent,
int ret = 0;
struct device *dev;
+ if (bdi->dev)
+ goto exit;
+
va_start(args, fmt);
dev = device_create_vargs(bdi_class, parent, MKDEV(0, 0), bdi, fmt, args);
va_end(args);
On Mon, 2008-11-03 at 16:53 +0100, Kay Sievers wrote:
> Andrew, can you pick this up this please?
>
> Thanks,
> Kay
>
>
> From: Kay Sievers <[email protected]>
> Subject: bdi: register sysfs bdi device only once per queue
>
> Devices which share the same queue, like floppies and mtd devices,
> get registered multiple times in the bdi interface, but bdi accounts
> only the last registered device of the devices sharing one queue.
David, is there any reason the mtd devices do this?
> On remove, all earlier registered devices leak, stay around in
> sysfs, and cause "duplicate filename" errors if the devices are
> re-created.
>
> This prevents the creation of multiple bdi interfaces per queue,
> and the bdi device will carry the dev_t name of the block device
> which is the first one registered, of the pool of devices using
> the same queue.
>
> Tested-By: Peter Korsgaard <[email protected]>
> Acked-By: Peter Zijlstra <[email protected]>
> Signed-Off-By: Kay Sievers <[email protected]>
> ---
>
> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> index f2e574d..e6676e5 100644
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -176,6 +176,9 @@ int bdi_register(struct backing_dev_info *bdi, struct device *parent,
> int ret = 0;
> struct device *dev;
>
> + if (bdi->dev)
> + goto exit;
> +
> va_start(args, fmt);
> dev = device_create_vargs(bdi_class, parent, MKDEV(0, 0), bdi, fmt, args);
> va_end(args);
Would it make sense to make it print something like, please fix me?
On Mon, 03 Nov 2008 17:08:50 +0100
Peter Zijlstra <[email protected]> wrote:
> On Mon, 2008-11-03 at 16:53 +0100, Kay Sievers wrote:
> > Andrew, can you pick this up this please?
> >
> > Thanks,
> > Kay
> >
> >
> > From: Kay Sievers <[email protected]>
> > Subject: bdi: register sysfs bdi device only once per queue
> >
> > Devices which share the same queue, like floppies and mtd devices,
> > get registered multiple times in the bdi interface, but bdi accounts
> > only the last registered device of the devices sharing one queue.
>
> David, is there any reason the mtd devices do this?
[tap tap - is this thing turned on?]
> > On remove, all earlier registered devices leak, stay around in
> > sysfs, and cause "duplicate filename" errors if the devices are
> > re-created.
> >
> > This prevents the creation of multiple bdi interfaces per queue,
> > and the bdi device will carry the dev_t name of the block device
> > which is the first one registered, of the pool of devices using
> > the same queue.
> >
> > Tested-By: Peter Korsgaard <[email protected]>
> > Acked-By: Peter Zijlstra <[email protected]>
> > Signed-Off-By: Kay Sievers <[email protected]>
> > ---
> >
> > diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> > index f2e574d..e6676e5 100644
> > --- a/mm/backing-dev.c
> > +++ b/mm/backing-dev.c
> > @@ -176,6 +176,9 @@ int bdi_register(struct backing_dev_info *bdi, struct device *parent,
> > int ret = 0;
> > struct device *dev;
> >
> > + if (bdi->dev)
> > + goto exit;
> > +
> > va_start(args, fmt);
> > dev = device_create_vargs(bdi_class, parent, MKDEV(0, 0), bdi, fmt, args);
> > va_end(args);
>
> Would it make sense to make it print something like, please fix me?
Yes, I think it would.
--- a/mm/backing-dev.c~bdi-register-sysfs-bdi-device-only-once-per-queue-fix
+++ a/mm/backing-dev.c
@@ -176,7 +176,7 @@ int bdi_register(struct backing_dev_info
int ret = 0;
struct device *dev;
- if (bdi->dev)
+ if (WARN_ON(bdi->dev))
goto exit;
va_start(args, fmt);
_
It's a bit cheeky to add a known-to-trigger WARN_ON into -rc4 but if we
don't do this then the issue will just get swept under the carpet.
On Wed, 2008-11-05 at 12:10 -0800, Andrew Morton wrote:
> > > Devices which share the same queue, like floppies and mtd devices,
> > > get registered multiple times in the bdi interface, but bdi accounts
> > > only the last registered device of the devices sharing one queue.
> >
> > David, is there any reason the mtd devices do this?
>
> [tap tap - is this thing turned on?]
No particular reason. It's been done that way since the dawn of time,
when it was perfectly normal. Nobody's yet bothered to change that, when
changing the rules.
--
David Woodhouse Open Source Technology Centre
[email protected] Intel Corporation
On Wed, 2008-11-05 at 23:10 +0100, David Woodhouse wrote:
> On Wed, 2008-11-05 at 12:10 -0800, Andrew Morton wrote:
> > > > Devices which share the same queue, like floppies and mtd devices,
> > > > get registered multiple times in the bdi interface, but bdi accounts
> > > > only the last registered device of the devices sharing one queue.
> > >
> > > David, is there any reason the mtd devices do this?
> >
> > [tap tap - is this thing turned on?]
>
> No particular reason. It's been done that way since the dawn of time,
> when it was perfectly normal. Nobody's yet bothered to change that, when
> changing the rules.
I don't think it ever made sense, or was intended that way, but yeah it
didn't really matter in any substantial way.
I can remember touching some mtd stuff back when I did the
per-bdi-writeback stuff, but I can't seem to recall all the details.
Might be time to stare at the mtd code a bit...
On Wed, 2008-11-05 at 23:10 +0100, David Woodhouse wrote:
> On Wed, 2008-11-05 at 12:10 -0800, Andrew Morton wrote:
> > > > Devices which share the same queue, like floppies and mtd devices,
> > > > get registered multiple times in the bdi interface, but bdi accounts
> > > > only the last registered device of the devices sharing one queue.
> > >
> > > David, is there any reason the mtd devices do this?
> >
> > [tap tap - is this thing turned on?]
>
> No particular reason. It's been done that way since the dawn of time,
> when it was perfectly normal. Nobody's yet bothered to change that, when
> changing the rules.
It looks to be the consequence of using one queue for a bunch of devices
(which seems odd), and its all tied into this translation layer which
starts a thread per queue.
Do you want me to rip this up and make it do one queue/thread per
device? - seems invasive...