get_mtd_device()

get_mtd_device()

[Alexander Larsson]

Why was get_mtd_device() removed?

I need it or something like it in jffs to get the mtd device from the
minor device of the mtdblock device i shall mount.

/ Alex




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: get_mtd_device()

[David Woodhouse]

alex@cendio.se said:
> Why was get_mtd_device() removed?

Locking issues.

I need to change the __MOD_INC_USE_COUNT(mtd->module) calls in the user 
modules to use try_inc_mod_count() on 2.3, and to deal appropriately with 
failure. This will make me mostly happy about the race conditions which
currently exist with drivers being unloaded at the same time as a user
module's open() function is being called on a different CPU.

This means that the driver module unload path goes...

  Acquire unload_lock in kernel/module.c
  call cleanup_module()
  ->	Acquire mtd_table_mutex
	Remove the mtd_info from the list
	Drop mtd_table_mutex
  Drop unload_lock
  
Now, I'm unhappy that I've just noticed that the unload_lock is a spinlock and 
hence perhaps cleanup_module() shouldn't be allowed to sleep, and hence can't 
try to acquire the mtd_table_mutex semaphore.

But besides this problem, which is going to bite a _lot_ of things, the
behaviour of get_mtd_device would have to be something like...

  Acquire mtd_table_mutex
  go through the mtd_table to get the one we want
  call try_inc_mod_count()
  -->	Acquire unload_lock
	increase the use count
	Drop unload_lock
  Drop mtd_table_mutex


Spot the deadlock. If you can tell me how to fix it, you can have 
get_mtd_device() back :)

>  I need it or something like it in jffs to get the mtd device from the
> minor device of the mtdblock device i shall mount. 

Could we add an ioctl() to mtdblock instead, to allow jffs to get direct 
access to the mtd_info struct that way? 

Actually, I'd prefer in the end that mtdblock not be required for jffs to work 
- after all, it's not really necessary except to keep mount(8) happy. 
We ought to be able to use 'mount /dev/mtd0 -tjffs /mnt/wherever' 



--
dwmw2




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

Re: get_mtd_device()

[Alexander Larsson]

On Fri, 7 Apr 2000, David Woodhouse wrote:

> alex@cendio.se said:
> > Why was get_mtd_device() removed?
> 
> Locking issues.
> 
> I need to change the __MOD_INC_USE_COUNT(mtd->module) calls in the user 
> modules to use try_inc_mod_count() on 2.3, and to deal appropriately with 
> failure. This will make me mostly happy about the race conditions which
> currently exist with drivers being unloaded at the same time as a user
> module's open() function is being called on a different CPU.
> 
> This means that the driver module unload path goes...
> 
>   Acquire unload_lock in kernel/module.c
>   call cleanup_module()
>   ->	Acquire mtd_table_mutex
> 	Remove the mtd_info from the list
> 	Drop mtd_table_mutex
>   Drop unload_lock
>   
> Now, I'm unhappy that I've just noticed that the unload_lock is a spinlock and 
> hence perhaps cleanup_module() shouldn't be allowed to sleep, and hence can't 
> try to acquire the mtd_table_mutex semaphore.
 Heh. Seems a bit bad yes.

> But besides this problem, which is going to bite a _lot_ of things, the
> behaviour of get_mtd_device would have to be something like...
> 
>   Acquire mtd_table_mutex
>   go through the mtd_table to get the one we want
>   call try_inc_mod_count()
>   -->	Acquire unload_lock
> 	increase the use count
> 	Drop unload_lock
>   Drop mtd_table_mutex
> 
> 
> Spot the deadlock. If you can tell me how to fix it, you can have 
> get_mtd_device() back :)

Hmmm, no obvious solution presents itself. I've just added a private
get_mtd_device() in my tree. When the rest of the fs works I'll try to
come up with a better solution.
 
> >  I need it or something like it in jffs to get the mtd device from the
> > minor device of the mtdblock device i shall mount. 
> 
> Could we add an ioctl() to mtdblock instead, to allow jffs to get direct 
> access to the mtd_info struct that way? 
> 
> Actually, I'd prefer in the end that mtdblock not be required for jffs to work 
> - after all, it's not really necessary except to keep mount(8) happy. 
> We ought to be able to use 'mount /dev/mtd0 -tjffs /mnt/wherever' 

I haven't actually tested this, I just assumed mount would check if
it was a block-device. Maybe it'll just work. I'll test.

/ Alex




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org

get_mtd_device()

[David Woodhouse]

I've been thinking about this over the weekend.

I think we can restore get_mtd_device(), but is _has_ to increase the MTD 
device's use count - hence it has to be coupled with a put_mtd_device().

Also, users like FTL cannot assume that the mtd_info struct will be present 
for a given MTD when ftl_open() is called - it may be in the process of 
being removed on another processor.

So in the _open() routine, they must attempt try_inc_mod_count(), which I'm 
going to wrap in another function mtd_inc_use_count() so that I can later 
deal with non-modular drivers which can go away (PCMCIA, anything else 
hotpluggable).

User modules _must_ be able to deal with mtd_inc_use_count() failing. If 
this happens, the mtd_info struct must be considered invalid, and not 
dereferenced at all.

So I reckon we want something like the following:

struct mtd_info *get_mtd_device(struct mtd_info *mtd, int num);

This _must_ be called with the mtd_table_mutex not held, i.e. not from an 
add/remove notify function. If <mtd> is NULL, it returns the MTD at minor 
<num>, and if <num> is -1, it scans the list for the MTD driver which has 
its struct mtd_info at location <mtd>.

On success, it returns a pointer to the struct mtd_info, on failure it 
returns NULL.

If both <mtd> and <num> are set, it checks that the MTD driver with minor 
<num> has its struct mtd_info at <mtd>, and returns NULL if they don't 
match.

The reason for having two parameters rather than just the number is as 
follows:

We need the number, because things like jffs, mtdblock and mtdchar when I 
split it from the core code will want to get an MTD driver directly from 
the minor number without using the notify functions to maintain a copy of 
the mtd_table (mtdblock currently does this and it's horrible).

We cannot make do with _just_ the number, because on SMP machines, a driver 
may have gone away and another been loaded in the same place - so user 
modules can't just use the number to get a handle on the same MTD later. 
They have to check that they've actually got the same one back.

We could actually provide just the number and force all the MTD user 
modules to do the check themselves, I suppose, but doing the check for them 
is safer.

---------
Couple it with 

int put_mtd_device(struct mtd_info *mtd);

I can't actually think of any possible reason for failure that the caller 
would be able to do anything about, so I may switch that to return void.

---------

If I don't hear any complaints, I'll implement this just as soon as I've 
provided some paperwork to get the boss off my back.

--
dwmw2




To unsubscribe, send "unsubscribe mtd" to majordomo@infradead.org