public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
* kernel command line arguments for block2mtd do not work
@ 2006-07-04 14:38 Ville Herva
  2006-07-05  6:52 ` Artem B. Bityutskiy
  0 siblings, 1 reply; 5+ messages in thread
From: Ville Herva @ 2006-07-04 14:38 UTC (permalink / raw)
  To: linux-mtd

There seems to be a problem in 2.6.17.3 drivers/mtd/devices/block2mtd.c .
If I compile the driver in statically (to avoid the hassle with modules),
passing in the block2mtd parameter always fails. I'm using kernel command
line parameter

        block2mtd.block2mtd=/dev/hdc2,65536

The
        name = kmalloc(len, GFP_KERNEL);
allocation in block2mtd.c::parse_name() always return NULL (len is 10), even
if I try GFP_ATOMIC.

I tried allocating the name statically (char sname[50]; *pname =
(char*)sname), but that only gave me a screenful of

        unknown interrupt or fault at EIP 00000060 c0126251 00006250

I hardcoded name to be "/dev/hdc2", but it failed in add_device() in similar
manner.

I can only conclude block2mtd_setup() is called too early when the driver is
compiled

After boot
        echo "/dev/hdc2" > /sys/module/block2mtd/parameters/block2mtd
seems to work


As far as I can see, doing kmalloc() for name in parse_name() is superfluous
anyway; block2mtd_setup() calls parse_name(), and the returned name is only used
by add_device() a couple of lines later. add_device() duplicates the string
but never kfrees it as far as I can see. So it's a memory leak at least.

I can workaround this by saving the parameter into static char[] during
block2mtd_setup() (called by module_param_call() macro) and only doing the  
real block2mtd_setup() call in block2mtd_init(). Even then, block2mtd fails 
to find the device (/dev/hdc2). (Note that after boot, echo "/dev/hdc2" >   
/sys/module/block2mtd/parameters/block2mtd does work fully.)             

My next try was to only call real block2mtd_setup() from late_initcall().     
That is still earlier than rootfs mount, so it can't find /dev/hdc2. 

For the rough idea, see 
        http://iki.fi/v/tmp/block2mtd.c                      
(Sorry for the ugliness - it's just a workaround hack.)


The final and ugliest hack wast to do

block2mtd.c::add_device()
        if (strcmp(devname, "/dev/hdc2") == 0)
        {
                INFO("Opening dev \"%s\" via open_by_devnum(MKDEV(22, 2)...\n",
                bdev = open_by_devnum(MKDEV(22, 2), FMODE_WRITE | FMODE_READ);
                INFO("... got bdev=0x%08x\n", bdev);
        }   
        else
                bdev = open_bdev_excl(devname, O_RDWR, NULL);

and that "works". Better ideas would be welcome. At least if (and since)
passing module params to block2mtd via kernel command line is not supposed
to work, the kernel might give an error saying so...

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

* Re: kernel command line arguments for block2mtd do not work
  2006-07-04 14:38 kernel command line arguments for block2mtd do not work Ville Herva
@ 2006-07-05  6:52 ` Artem B. Bityutskiy
  2006-07-05  7:05   ` Ville Herva
  0 siblings, 1 reply; 5+ messages in thread
From: Artem B. Bityutskiy @ 2006-07-05  6:52 UTC (permalink / raw)
  To: linux-mtd, vherva

On Tue, 2006-07-04 at 17:38 +0300, Ville Herva wrote:
> For the rough idea, see 
>         http://iki.fi/v/tmp/block2mtd.c                      
> (Sorry for the ugliness - it's just a workaround hack.)

Hello,

I'd suggest you to just send a patch. See "Patch submission" at
http://www.linux-mtd.infradead.org/source.html

-- 
Best Regards,
Artem B. Bityutskiy,
St.-Petersburg, Russia.

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

* Re: kernel command line arguments for block2mtd do not work
  2006-07-05  6:52 ` Artem B. Bityutskiy
@ 2006-07-05  7:05   ` Ville Herva
  2006-07-05  7:13     ` Ville Herva
  0 siblings, 1 reply; 5+ messages in thread
From: Ville Herva @ 2006-07-05  7:05 UTC (permalink / raw)
  To: Artem B. Bityutskiy; +Cc: linux-mtd

On Wed, Jul 05, 2006 at 10:52:15AM +0400, you [Artem B. Bityutskiy] wrote:
> On Tue, 2006-07-04 at 17:38 +0300, Ville Herva wrote:
> > For the rough idea, see 
> >         http://iki.fi/v/tmp/block2mtd.c                      
> > (Sorry for the ugliness - it's just a workaround hack.)
> 
> Hello,
> 
> I'd suggest you to just send a patch. See "Patch submission" at
> http://www.linux-mtd.infradead.org/source.html

I don't think any of the ideas I listed to "fix" the problem are
satisfactory. They all run into the same dead-end of not having access to
the rootfs when parsing the arguments, and hence, not being able to open the
underlying blockdev. 

These days the kernel afaik does not have builtin
device name -> device number mapping (I think it used to - for rootfs
device mounting - but isn't that gone?). The device we are mounting might be
the rootfs, so /dev is not present. Hence, 
	open_bdev_excl(devname, O_RDWR, NULL); 
can't succeed.

What I _was_ able to do (see http://iki.fi/v/tmp/block2mtd.c) is to move the
argument parsing later in boot process - it doesn't fail anymore, but that
doesn't help much, since the device can't be opened. This means the error
message is much more helpful (and it won't crash), but is it worth it?

As a stop-gap measure I suggested making the "out of memory" message more
informative (e.g. "can't parse arguments - not possible from kernel command
line, see Documentation/block2dev.txt" or something.



-- v -- 

v@iki.fi

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

* Re: kernel command line arguments for block2mtd do not work
  2006-07-05  7:05   ` Ville Herva
@ 2006-07-05  7:13     ` Ville Herva
  2006-07-05  7:21       ` Artem B. Bityutskiy
  0 siblings, 1 reply; 5+ messages in thread
From: Ville Herva @ 2006-07-05  7:13 UTC (permalink / raw)
  To: Artem B. Bityutskiy; +Cc: linux-mtd

On Wed, Jul 05, 2006 at 10:05:10AM +0300, you [Ville Herva] wrote:
> On Wed, Jul 05, 2006 at 10:52:15AM +0400, you [Artem B. Bityutskiy] wrote:
> > On Tue, 2006-07-04 at 17:38 +0300, Ville Herva wrote:
> > > For the rough idea, see 
> > >         http://iki.fi/v/tmp/block2mtd.c                      
> > > (Sorry for the ugliness - it's just a workaround hack.)
> > 
> > Hello,
> > 
> > I'd suggest you to just send a patch. See "Patch submission" at
> > http://www.linux-mtd.infradead.org/source.html
> 
> I don't think any of the ideas I listed to "fix" the problem are
> satisfactory. They all run into the same dead-end of not having access to
> the rootfs when parsing the arguments, and hence, not being able to open the
> underlying blockdev. 
> 
> These days the kernel afaik does not have builtin
> device name -> device number mapping (I think it used to - for rootfs
> device mounting - but isn't that gone?). The device we are mounting might be
> the rootfs, so /dev is not present. Hence, 
> 	open_bdev_excl(devname, O_RDWR, NULL); 
> can't succeed.
> 
> What I _was_ able to do (see http://iki.fi/v/tmp/block2mtd.c) is to move the
> argument parsing later in boot process - it doesn't fail anymore, but that
> doesn't help much, since the device can't be opened. This means the error
> message is much more helpful (and it won't crash), but is it worth it?

Wait a minute, should I use init/do_mounts.c::name_to_dev_t() ?

	/*      Convert a name into device number.  We accept the following
	 *      variants:
	 *
	 *      1) device number in hexadecimal represents itself
	 *      2) /dev/nfs represents Root_NFS (0xff)
	 *      3) /dev/<disk_name> represents the device number of disk
	 *      4) /dev/<disk_name><decimal> represents the device number
	 *         of partition - device number of disk plus the partition number
	 *      5) /dev/<disk_name>p<decimal> - same as the above, that form is
	 *         used when disk name of partitioned disk ends on a digit.
	*/
	dev_t name_to_dev_t(char *name)

That might work... Even if you can't pass a normal device name in, you could
specify device number.

So something like

block2mtd.c::add_device()

        bdev = open_bdev_excl(devname, O_RDWR, NULL);
	if (!bdev) {
		dev_t dev = name_to_dev_t(devname);
		if (dev != 0) {
                	bdev = open_by_devnum(dev, FMODE_WRITE | FMODE_READ);
		}
	}

? 

What do you think?




-- v -- 

v@iki.fi

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

* Re: kernel command line arguments for block2mtd do not work
  2006-07-05  7:13     ` Ville Herva
@ 2006-07-05  7:21       ` Artem B. Bityutskiy
  0 siblings, 0 replies; 5+ messages in thread
From: Artem B. Bityutskiy @ 2006-07-05  7:21 UTC (permalink / raw)
  To: vherva; +Cc: linux-mtd

On Wed, 2006-07-05 at 10:13 +0300, Ville Herva wrote:
> What do you think?

Probably, I have not time to delve into this, sorry. I just wanted to
say that the sad truth of life is that you'll barely be heard with
lengthy mails like this (my experience shows this) unless you make a
patch, test it, then bug this mailing list with it :-)

-- 
Best Regards,
Artem B. Bityutskiy,
St.-Petersburg, Russia.

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

end of thread, other threads:[~2006-07-05  7:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-07-04 14:38 kernel command line arguments for block2mtd do not work Ville Herva
2006-07-05  6:52 ` Artem B. Bityutskiy
2006-07-05  7:05   ` Ville Herva
2006-07-05  7:13     ` Ville Herva
2006-07-05  7:21       ` Artem B. Bityutskiy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox