LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* bdi: register sysfs bdi device only once per queue
@ 2008-11-03 15:53 Kay Sievers
  2008-11-03 16:08 ` Peter Zijlstra
  0 siblings, 1 reply; 6+ messages in thread
From: Kay Sievers @ 2008-11-03 15:53 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Folkert van Heusden, Peter Korsgaard, linux-kernel

Andrew, can you pick this up this please?

Thanks,
Kay


From: Kay Sievers <kay.sievers@vrfy.org>
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 <jacmet@sunsite.dk>
Acked-By: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-Off-By: Kay Sievers <kay.sievers@vrfy.org>
---

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);



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: bdi: register sysfs bdi device only once per queue
  2008-11-03 15:53 bdi: register sysfs bdi device only once per queue Kay Sievers
@ 2008-11-03 16:08 ` Peter Zijlstra
  2008-11-05 20:10   ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2008-11-03 16:08 UTC (permalink / raw)
  To: Kay Sievers
  Cc: Andrew Morton, Folkert van Heusden, Peter Korsgaard,
	linux-kernel, David Woodhouse

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 <kay.sievers@vrfy.org>
> 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 <jacmet@sunsite.dk>
> Acked-By: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Signed-Off-By: Kay Sievers <kay.sievers@vrfy.org>
> ---
> 
> 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?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: bdi: register sysfs bdi device only once per queue
  2008-11-03 16:08 ` Peter Zijlstra
@ 2008-11-05 20:10   ` Andrew Morton
  2008-11-05 22:10     ` David Woodhouse
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2008-11-05 20:10 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: kay.sievers, folkert, jacmet, linux-kernel, dwmw2

On Mon, 03 Nov 2008 17:08:50 +0100
Peter Zijlstra <peterz@infradead.org> 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 <kay.sievers@vrfy.org>
> > 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 <jacmet@sunsite.dk>
> > Acked-By: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > Signed-Off-By: Kay Sievers <kay.sievers@vrfy.org>
> > ---
> > 
> > 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.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: bdi: register sysfs bdi device only once per queue
  2008-11-05 20:10   ` Andrew Morton
@ 2008-11-05 22:10     ` David Woodhouse
  2008-11-05 22:21       ` Peter Zijlstra
  2008-11-07 14:11       ` Peter Zijlstra
  0 siblings, 2 replies; 6+ messages in thread
From: David Woodhouse @ 2008-11-05 22:10 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Peter Zijlstra, kay.sievers, folkert, jacmet, linux-kernel

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
David.Woodhouse@intel.com                              Intel Corporation


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: bdi: register sysfs bdi device only once per queue
  2008-11-05 22:10     ` David Woodhouse
@ 2008-11-05 22:21       ` Peter Zijlstra
  2008-11-07 14:11       ` Peter Zijlstra
  1 sibling, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2008-11-05 22:21 UTC (permalink / raw)
  To: David Woodhouse; +Cc: Andrew Morton, kay.sievers, folkert, jacmet, linux-kernel

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...

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: bdi: register sysfs bdi device only once per queue
  2008-11-05 22:10     ` David Woodhouse
  2008-11-05 22:21       ` Peter Zijlstra
@ 2008-11-07 14:11       ` Peter Zijlstra
  1 sibling, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2008-11-07 14:11 UTC (permalink / raw)
  To: David Woodhouse; +Cc: Andrew Morton, kay.sievers, folkert, jacmet, linux-kernel

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...

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2008-11-07 14:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-11-03 15:53 bdi: register sysfs bdi device only once per queue Kay Sievers
2008-11-03 16:08 ` Peter Zijlstra
2008-11-05 20:10   ` Andrew Morton
2008-11-05 22:10     ` David Woodhouse
2008-11-05 22:21       ` Peter Zijlstra
2008-11-07 14:11       ` Peter Zijlstra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).