MTD Partition problems

MTD Partition problems

[Malik, Vipin [FRCO/HOU]]

Hi Folks,

I'm just getting back into playing with this.

Is there something broken with the mtd partitioning code?

I defined 3 mtd partitions on one 8MB flash. The partitions look fine on the
debug log and a "cat /proc/mtd" shows three devices with the proper sizes
and names as defined in my "mtd_partition" map structure in my map file.

However, it seems that 6 mtd partitions have been added (even though just 3
show up in /proc/mtd)!

Each of mtd[0,1], mtd[2,3] and mtd[4,5] pairs point to partitions 1,2&3
respectively. Each of the pairs point to the same partitions (have the same
stuff in them).

However mtdblock0,1,2 point to the correct partitions!

And I've checked that my /dev/mtd0,1,2,3,4,5 devices are pointing to char
devices 30,1...5

Any clues?

Vipin
 

Re: MTD Partition problems

[Jörn Engel]

On Fri, 2 August 2002 16:59:07 -0500, Malik, Vipin [FRCO/HOU] wrote:
> I'm just getting back into playing with this.
> 
> Is there something broken with the mtd partitioning code?

Not really broken, but maybe useless.

> I defined 3 mtd partitions on one 8MB flash. The partitions look fine on the
> debug log and a "cat /proc/mtd" shows three devices with the proper sizes
> and names as defined in my "mtd_partition" map structure in my map file.
> 
> However, it seems that 6 mtd partitions have been added (even though just 3
> show up in /proc/mtd)!
> 
> Each of mtd[0,1], mtd[2,3] and mtd[4,5] pairs point to partitions 1,2&3
> respectively. Each of the pairs point to the same partitions (have the same
> stuff in them).

One device of each pair is read-only. This is by design and scheduled
to be ripped out sometime.

David, 2.4.19 is released, but I am right in my exam period. Does end
of August and/or beginning of September sound ok for the merge?

Jörn

-- 
Beware of bugs in the above code; I have only proved it correct, but
not tried it.
-- Donald Knuth

Re: MTD Partition problems

[Vipin Malik]

At 11:46 AM 8/3/2002 +0200, Jörn Engel wrote:
>On Fri, 2 August 2002 16:59:07 -0500, Malik, Vipin [FRCO/HOU] wrote:
> > I'm just getting back into playing with this.
> >
> > Is there something broken with the mtd partitioning code?
>
>Not really broken, but maybe useless.


Hmm, useless as in "that functionality has been depricated- a better way is 
available- use <state better way here>" or useless as in "it's good stuff 
but currently not working".

I am (almost) sure that it was working and allowed me to setup different 
read/write partitions on the same flash chip (about 9-12 months ago).



> > I defined 3 mtd partitions on one 8MB flash. The partitions look fine 
> on the
> > debug log and a "cat /proc/mtd" shows three devices with the proper sizes
> > and names as defined in my "mtd_partition" map structure in my map file.
> >
> > However, it seems that 6 mtd partitions have been added (even though just 3
> > show up in /proc/mtd)!
> >
> > Each of mtd[0,1], mtd[2,3] and mtd[4,5] pairs point to partitions 1,2&3
> > respectively. Each of the pairs point to the same partitions (have the same
> > stuff in them).
>
>One device of each pair is read-only. This is by design and scheduled
>to be ripped out sometime.


Sorry for the stupid question: If it's by design, why is it going to be 
ripped out sometime soon?


>David, 2.4.19 is released, but I am right in my exam period. Does end
>of August and/or beginning of September sound ok for the merge?


Should I be using the main CVS code or some branch. The above was with 
2.4.19-rc3.

Thanks

Vipin
P.S. I noticed that my original mail did show up, but not at work. Hmmm. 
Sorry for the duplicate post.



>Jörn
>
>--
>Beware of bugs in the above code; I have only proved it correct, but
>not tried it.
>-- Donald Knuth
>
>______________________________________________________
>Linux MTD discussion mailing list
>http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: MTD Partition problems

[Jörn Engel]

On Sat, 3 August 2002 07:47:11 -0500, Vipin Malik wrote:
> >> Is there something broken with the mtd partitioning code?
> >
> >Not really broken, but maybe useless.
> 
> Hmm, useless as in "that functionality has been depricated- a better way is 
> available- use <state better way here>" or useless as in "it's good stuff 
> but currently not working".

Useless as in useless, really. Functionality is added, that is
available anyway.

> >One device of each pair is read-only. This is by design and scheduled
> >to be ripped out sometime.
> 
> Sorry for the stupid question: If it's by design, why is it going to be 
> ripped out sometime soon?

It might have made sense, when it was designed. It might just have
been a relatively new developer making the usual mistakes. Whatever.

Fact is that it is useless, the superuser can create a read only
device simply by 'chmod -w /dev/mtd0', if she wishes. That is all
those extra devices do, except for causing confusion and eating up
minor numbers.

People would get even more confused when suddenly the device nodes
have different behaviour, which will definitely happen as soon as the
ro-devices are gone. But I have some patches in my queue that will
create confusion anyway. You do the math.

> >David, 2.4.19 is released, but I am right in my exam period. Does end
> >of August and/or beginning of September sound ok for the merge?
> 
> Should I be using the main CVS code or some branch. The above was with 
> 2.4.19-rc3.

Currently, there is no branch that tackles this problem. There was
one, but David never took a look, was busy, and rescheduled to 'when
2.4.19 is out'. Now I am busy,...

Jörn

-- 
Public Domain  - Free as in Beer
General Public - Free as in Speech
BSD License    - Free as in Enterprise
Shared Source  - Free as in "Work will make you..."

Re: MTD Partition problems

[Vipin Malik]

> >
> > Sorry for the stupid question: If it's by design, why is it going to be
> > ripped out sometime soon?
>
>It might have made sense, when it was designed. It might just have
>been a relatively new developer making the usual mistakes. Whatever.
>
>Fact is that it is useless, the superuser can create a read only
>device simply by 'chmod -w /dev/mtd0', if she wishes. That is all
>those extra devices do, except for causing confusion and eating up
>minor numbers.

So if I read you correctly, you are saying that the partitioning code 
really works but in a quirky way: the presence of duplicate read only 
paired devices, which really provide no useful functionality.

However, if one ignores the duplicated devices, one can still use 
partitioning and mount a JFFS2 fs on one of the partitions while having 
something completely different on another and device access locking will be 
taken care of by the mtd driver (which is essentially what partitioning 
provides vs. just calling add_mtd() NUM_PARTITION times in your map file on 
the same flash chip).

Additionally, (say) /dev/mtdblock2 will really provide a handle to 
/dev/mtd4 the writable third partition on the partitioned flash chip, thus 
mounting the JFFS2 fs on the partition originally intended (the third 
partition).

Unfortunately, my tests indicate that this is not happening. I know that 
/dev/mtdblock2 (the intended third partition) is formatted. However JFFS2 
finds data here and refuses to mount on it. I think it is really being 
passes a handle to and accesses /dev/mtd2, which is the wrong partition.

/dev/mtdblock[4,5] do not exist, even though /dev/mtd[4,5] do and /dev/mtd4 
is the correct partition that I want!

Argh! I'll try your patches if you like.

Thanks for your answers.

Vipin




>People would get even more confused when suddenly the device nodes
>have different behaviour, which will definitely happen as soon as the
>ro-devices are gone. But I have some patches in my queue that will
>create confusion anyway. You do the math.
>
> > >David, 2.4.19 is released, but I am right in my exam period. Does end
> > >of August and/or beginning of September sound ok for the merge?
> >
> > Should I be using the main CVS code or some branch. The above was with
> > 2.4.19-rc3.
>
>Currently, there is no branch that tackles this problem. There was
>one, but David never took a look, was busy, and rescheduled to 'when
>2.4.19 is out'. Now I am busy,...
>
>Jörn
>
>--
>Public Domain  - Free as in Beer
>General Public - Free as in Speech
>BSD License    - Free as in Enterprise
>Shared Source  - Free as in "Work will make you..."
>
>______________________________________________________
>Linux MTD discussion mailing list
>http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: MTD Partition problems

[Jörn Engel]

On Sat, 3 August 2002 14:24:32 -0500, Vipin Malik wrote:
> >Fact is that it is useless, the superuser can create a read only
> >device simply by 'chmod -w /dev/mtd0', if she wishes. That is all
> >those extra devices do, except for causing confusion and eating up
> >minor numbers.
> 
> So if I read you correctly, you are saying that the partitioning code 
> really works but in a quirky way: the presence of duplicate read only 
> paired devices, which really provide no useful functionality.

Yes.

> However, if one ignores the duplicated devices, one can still use 
> partitioning and mount a JFFS2 fs on one of the partitions while having 
> something completely different on another and device access locking will be 
> taken care of by the mtd driver (which is essentially what partitioning 
> provides vs. just calling add_mtd() NUM_PARTITION times in your map file on 
> the same flash chip).

Yes. The partitioning code basically checks the limits and hands the
work to the base device with adjusted offset.

> Additionally, (say) /dev/mtdblock2 will really provide a handle to 
> /dev/mtd4 the writable third partition on the partitioned flash chip, thus 
> mounting the JFFS2 fs on the partition originally intended (the third 
> partition).

Almost. /dev/mtd4 should be minor number 8. If you check
/usr/src/linux/Documentation/devices.txt, you will read the following:
                  0 = /dev/mtd0         First MTD (rw)
                  1 = /dev/mtdr0        First MTD (ro)
Thus mtd0 has minor 0, mtd1 has minor 2, ...

Also, the third partition is /dev/mtd3, as /dev/mtd0 is the real
device. Look at the minor numbers for /dev/hda*, just the same.

But what you *meant* is correct.

> Unfortunately, my tests indicate that this is not happening. I know that 
> /dev/mtdblock2 (the intended third partition) is formatted. However JFFS2 
> finds data here and refuses to mount on it. I think it is really being 
> passes a handle to and accesses /dev/mtd2, which is the wrong partition.

See above.

> /dev/mtdblock[4,5] do not exist, even though /dev/mtd[4,5] do and /dev/mtd4 
> is the correct partition that I want!

See above.

> Argh! I'll try your patches if you like.

I would be happy if you did. But you don't need my patches, unless you
have more than one device that you want to partition. With the two
thinkos out of the way, you should be doing fine. :)

Jörn

-- 
Mundie uses a textbook tactic of manipulation: start with some
reasonable talk, and lead the audience to an unreasonable conclusion.
-- Bruce Perens

Re: MTD Partition problems

[David Woodhouse]

joern@wohnheim.fh-wedel.de said:
> > Sorry for the stupid question: If it's by design, why is it going to
> > be ripped out sometime soon?

> It might have made sense, when it was designed. It might just have
> been a relatively new developer making the usual mistakes. Whatever.

> Fact is that it is useless, the superuser can create a read only
> device simply by 'chmod -w /dev/mtd0', if she wishes. That is all
> those extra devices do, except for causing confusion and eating up
> minor numbers. 

I inherited the major number from elsewhere, and it was already set up like 
that. I haven't changed it because people have only just started to read 
the bloody documentation and set it up right -- with the occasional 
exception :)

>  Currently, there is no branch that tackles this problem. There was
> one, but David never took a look, was busy, and rescheduled to 'when
> 2.4.19 is out'. Now I am busy,...

I want to get the remaining changes in the tree into 2.4.20-pre, then 
update the code for 2.5 and start doing new exciting stuff. 

--
dwmw2

Disk blocks for long periods

[Joakim Tjernlund]

Hi 

I have noticed that if i copy a "big" file(580K) it sometimes take up to 42 seconds before
it's finished. Normally it takes about 3-4 seconds. When this long copy happen, top reports
that kupdate and the copy(cp) process is in D state, the rest is sleeping.
The FS is a 45% usage. FS is about 63MB in size. Using the stable branch.

Why does it block for so long time?

Also, if I fill the FS up to 100% and later reboots, it kan take 10-15 minutes before
init starts to run. The root fs mount takes about 20 seconds and the system just
stops between the last message from the kernel and when init prints its version.

It is just the first reboot after I fill the FS, once I have gotten past the first reboot and
reboots again, things are back to normal.

Any ideas?

   Jocke

Re: Disk blocks for long periods

[David Woodhouse]

joakim.tjernlund@lumentis.se said:
>  I have noticed that if i copy a "big" file(580K) it sometimes take up
> to 42 seconds before it's finished. Normally it takes about 3-4
> seconds. When this long copy happen, top reports that kupdate and the
> copy(cp) process is in D state, the rest is sleeping. The FS is a 45%
> usage. FS is about 63MB in size. Using the stable branch.

> Why does it block for so long time?

Dunno. What's it doing? Where are cp and kupdate sleeping? (SysRq-T).

>  Also, if I fill the FS up to 100% and later reboots, it kan take
> 10-15 minutes before init starts to run. The root fs mount takes about
> 20 seconds and the system just stops between the last message from the
> kernel and when init prints its version.

> It is just the first reboot after I fill the FS, once I have gotten
> past the first reboot and reboots again, things are back to normal. 

More logs and SysRq-T required. Sounds like it's erasing blocks and for 
some reason not allowing anything else to happen while it's doing it.

--
dwmw2

RE: Disk blocks for long periods

[Joakim Tjernlund]

> joakim.tjernlund@lumentis.se said:
> >  I have noticed that if i copy a "big" file(580K) it sometimes take up
> > to 42 seconds before it's finished. Normally it takes about 3-4
> > seconds. When this long copy happen, top reports that kupdate and the
> > copy(cp) process is in D state, the rest is sleeping. The FS is a 45%
> > usage. FS is about 63MB in size. Using the stable branch.
> 
> > Why does it block for so long time?
> 
> Dunno. What's it doing? Where are cp and kupdate sleeping? (SysRq-T).

I am on PPC and SysRq-T does not display anything useful :-(
Perhaps readprofile?

> 
> >  Also, if I fill the FS up to 100% and later reboots, it kan take
> > 10-15 minutes before init starts to run. The root fs mount takes about
> > 20 seconds and the system just stops between the last message from the
> > kernel and when init prints its version.
> 
> > It is just the first reboot after I fill the FS, once I have gotten
> > past the first reboot and reboots again, things are back to normal. 
> 
> More logs and SysRq-T required. Sounds like it's erasing blocks and for 
> some reason not allowing anything else to happen while it's doing it.

Perhaps kupdate is holding some VFS specific block? I am on a MV 2.4.2 kernel 

> 
> --
> dwmw2
> 
> 
> 

Re: Disk blocks for long periods

[David Woodhouse]

joakim.tjernlund@lumentis.se said:
>  I am on PPC and SysRq-T does not display anything useful :-( Perhaps
> readprofile?

SysRq-T should work on PPC. It's mostly generic code. What's wrong with it?

The profile _may_ help but it's most likely to tell us that the CPU spend
most time sleeping, without actually telling us _where_ each process was
sleeping.

>  Perhaps kupdate is holding some VFS specific block? I am on a MV
> 2.4.2 kernel 

I'd care more if you could reproduce it with something a little more 
recent. I'm assuming you're using v1.100 of cfi_cmdset_0001.c, since you 
committed that change.


--
dwmw2

RE: Disk blocks for long periods

[Joakim Tjernlund]

> 
> joakim.tjernlund@lumentis.se said:
> >  I am on PPC and SysRq-T does not display anything useful :-( Perhaps
> > readprofile?
> 
> SysRq-T should work on PPC. It's mostly generic code. What's wrong with it?
Maybe I spoke too soon, but I can not make sense of it:
SysRq: Show State                                                               
                                                                                
                         free                        sibling                    
  task             PC    stack   pid father child younger older                 
init      S 0FF86418     0     1      0   159  (NOTLB)                          
keventd   S 00000000     0     2      1        (L-TLB)       3                  
kswapd    S 00000000     0     3      1        (L-TLB)       4     2            
kreclaimd  S 00000000     0     4      1        (L-TLB)       5     3           
bdflush   S 00000000     0     5      1        (L-TLB)       6     4            
kupdate   D 00000000     0     6      1        (L-TLB)       7     5            
mtdblockd  S 00000000     0     7      1        (L-TLB)       8     6           
jffs2_gcd_mtd4  S 00000000     0     8      1        (L-TLB)      51     7      
syslogd   S 0FF86418    28    51      1        (NOTLB)      53     8            
klogd     R 0FF7EB04   144    53      1        (NOTLB)      58    51            
inetd     S 0FF86418     0    58      1   163  (NOTLB)      72    53            
te_server  S 0FF42A84     0    72      1        (NOTLB)      75    58           
te_log    S 0FF3C418    28    75      1        (NOTLB)      77    72            
te        S 0FF3C418     0    77      1        (NOTLB)      79    75            
ntpd      S 0FDE4418     0    79      1        (NOTLB)      81    77
alib_psupd  S 0FED6418     0    81      1        (NOTLB)      83    79          
icn_server  S 0FF18418     0    83      1        (NOTLB)      85    81          
cp_cm     S 0FED6418     0    85      1        (NOTLB)      87    83            
cp_cop    S 0FED6418     0    87      1        (NOTLB)      89    85            
cp_pbh    S 0FED6418     0    89      1        (NOTLB)      91    87            
alib_swusd  S 0FED6418     0    91      1        (NOTLB)      93    89          
mgmt_named  S 0FEAC418     0    93      1        (NOTLB)      95    91          
mgmt_invd  S 0FD6A418  3520    95      1        (NOTLB)      97    93           
eq_rh     S 0FD6A418     0    97      1        (NOTLB)      99    95            
mgmt_pmd  S 0FC45418     0    99      1        (NOTLB)     101    97            
eq_bh_superviso  S 0FE3E418  3520   101      1        (NOTLB)     103    99     
eq_bh     S 0FE3E418     0   103      1        (NOTLB)     105   101            
mgmt_backupd  S 0FC45418  3520   105      1        (NOTLB)     107   103        
mgmt_alarmd  S 0FD6A418   124   107      1        (NOTLB)     109   105         
tosv_server  S 0FED6418     0   109      1        (NOTLB)     111   107         
tosv_supervisor  S 0FED6418     0   111      1        (NOTLB)     113   109     
snmpd     S 0FB4B418     8   113      1   161  (NOTLB)     115   111            
sys_sysconfd  S 0FD6A418  3168   115      1        (NOTLB)     117   113        
sys_ipconfd  S 0FD6A418     0   117      1        (NOTLB)     120   115         
umi_sim   S 0FED6418    76   120      1        (NOTLB)     122   117            
uxi_sim   S 0FED6418     0   122      1        (NOTLB)     124   120            
upi_sim   S 0FED6418     0   124      1        (NOTLB)     126   122            
cp_sim    S 0FED6418   260   126      1        (NOTLB)     128   124            
eq_tm_mgr  S 0FD6A418     0   128      1        (NOTLB)     130   126
eq_mxm_mgr  S 0FD6A418     0   130      1        (NOTLB)     132   128          
eq_gbem_mgr  S 0FD6A418    24   132      1        (NOTLB)     134   130         
eq_xm_mgr  S 0FD6A418     0   134      1        (NOTLB)     136   132           
eq_cpm_mgr  S 0FD6A418     4   136      1   160  (NOTLB)     138   134          
eq_pbm_mgr  S 0FD6A418     4   138      1        (NOTLB)     140   136          
eq_bm_mgr  S 0FD6A418     0   140      1        (NOTLB)     142   138           
eq_topm_mgr  S 0FD6A418     0   142      1        (NOTLB)     144   140         
eq_spfm_mgr  S 0FD6A418  3136   144      1        (NOTLB)     146   142         
eq_pm_mgr  S 0FD6A418     0   146      1        (NOTLB)     148   144           
ne_swumd  S 0FED6418     0   148      1        (NOTLB)     150   146            
ne_rc     S 0FED6418   428   150      1        (NOTLB)     152   148            
ne_rc_superviso  S 0FF3C418     0   152      1        (NOTLB)     154   150     
pppd      S 0FF86418   152   154      1        (NOTLB)     156   152            
zebra     S 0FD29418   128   156      1        (NOTLB)     158   154            
ospfd     S 0FD97418  3520   158      1        (NOTLB)     159   156            
bash      S 0FEC35AC   416   159      1   181  (NOTLB)           158            
eq_cpm_child  S 0FD6A418    68   160    136        (NOTLB)                      
snmpd     S 0FB51B84     0   161    113   162  (NOTLB)                          
snmpd     S 0FAB3370     0   162    161        (NOTLB)                          
in.telnetd  S 0FF64418     0   163     58   164  (NOTLB)                        
bash      S 0FEE0B04     0   164    163        (NOTLB)                          
bash      S 0FEC35AC  3156   180    159   182  (NOTLB)     181                  
cp        D 0FF7EB14  2868   181    159        (NOTLB)           180            
cp        D 0FF7EB14  1996   182    180        (NOTLB)



> 
> The profile _may_ help but it's most likely to tell us that the CPU spend
> most time sleeping, without actually telling us _where_ each process was
> sleeping.
> 
> >  Perhaps kupdate is holding some VFS specific block? I am on a MV
> > 2.4.2 kernel 
> 
> I'd care more if you could reproduce it with something a little more 
> recent. I'm assuming you're using v1.100 of cfi_cmdset_0001.c, since you 
> committed that change.
> 

Sorry, but It's a major job to upgrade our kernel, so that will have to wait.
No, I am not running the v1.100 of cfi_cmdset_0001.c but I have made sure that
the relevant fixes are in.

When I last looked(many months ago) there was no 2.4 branch of the MTD layer and there
was much NAND activity so I kept the one I had. Is the latest CVS of drivers/mtd the stable one?
Is it in 2.4.19?

   Jocke 

Re: Disk blocks for long periods

[David Woodhouse]

joakim.tjernlund@lumentis.se said:
> > SysRq-T should work on PPC. It's mostly generic code. What's wrong
> > with it? 

> Maybe I spoke too soon, but I can not make sense of it:

That's because you're using 2.4.2. A newer kernel would print the relevant 
call trace from the kernel stack too, and we'd be able to see what kernel 
function(s) it's sleeping in.

--
dwmw2

RE: Disk blocks for long periods

[Dave Ellis]

joakim.tjernlund@lumentis.se said:

> I have noticed that if i copy a "big" file(580K) it sometimes 
> take up to 42 seconds before it's finished. Normally it takes 
> about 3-4 seconds. When this long copy happen, top reports 
> that kupdate and the copy(cp) process is in D state, the rest 
> is sleeping. The FS is a 45% usage. FS is about 63MB in size. 
> Using the stable branch.
> 
> Why does it block for so long time?

I've been working on what may be the same problem and I think I 
finally understand it. I've seen it with 2.4.4, 2.4.18 and
with 2.4.18 with the 2.4.19 jffs2 and mtd code. I am using an 
AM29LV641, which uses cfi_cmdset_0002.c, but the code in 
cfi_cmdset_0001.c is similar. I have a possible solution
but I'd like some feedback on it.

The problem occurs when do_erase_oneblock() tries to lock the 
flash while cfi_amdstd_write() is writing a lot of data. The 
erasing thread locks the chip mutex when do_write_oneword() does the
    cfi_spin_unlock(chip->mutex);
    cfi_udelay(chip->word_write_time);
    cfi_spinlock(chip->mutex);
sequence. It sees the state is FL_WRITING, so it puts itself 
back on the wait queue. do_write_oneword() continues and eventually
sets the state back to FL_READY and wakes up the queue, but the
erasing thread doesn't actually run until the cfi_udelay() in 
do_write_oneword() calls schedule() while writing the next word
to the flash. The state is FL_WRITING, so the erasing thread goes
back on the wait queue. This continues until the entire write is
finished, then the erasing thread finally starts the erase.

The effect of all this sheduling is to write exactly one word
to flash for each jiffie! My flash is 16 bits wide, so a single
write of 2400 bytes was sometimes taking 1200 jiffies, or
12 seconds.

This patch to cfi_cmdset_0002.c 1.56 seems solve the problem, but
I am not sure if this is right way to do it. Comments?

Dave Ellis
dge@sixnetio.com

BTW - If I make similar changes to 1.55 or before it solves this
problem, but the write fails occasionally. I am guessing that
with my change it gets to the write completion check faster and the
old check fails, but the new write completion polling works better.

--- cfi_cmdset_0002.c	Mon Jul 15 11:13:25 2002
+++ cfi_cmdset_0002.fixed.c	Mon Aug  5 14:28:40 2002
@@ -386,9 +384,7 @@
 
 	cfi_write(map, datum, adr);
 
-	cfi_spin_unlock(chip->mutex);
-	cfi_udelay(chip->word_write_time);
-	cfi_spin_lock(chip->mutex);
+	udelay(chip->word_write_time);
 
 
 	/* Polling toggle bits instead of reading back many times
@@ -447,6 +444,7 @@
 	chip->state = FL_READY;
 	wake_up(&chip->wq);
 	cfi_spin_unlock(chip->mutex);
+	cfi_udelay(1);	/* just a chance to schedule() */
 	
 	return ret;
 }

Re: MTD Partition problems

[Vipin Malik]

> > Additionally, (say) /dev/mtdblock2 will really provide a handle to
> > /dev/mtd4 the writable third partition on the partitioned flash chip, thus
> > mounting the JFFS2 fs on the partition originally intended (the third
> > partition).
>
>Almost. /dev/mtd4 should be minor number 8. If you check
>/usr/src/linux/Documentation/devices.txt, you will read the following:
>                   0 = /dev/mtd0         First MTD (rw)
>                   1 = /dev/mtdr0        First MTD (ro)
>Thus mtd0 has minor 0, mtd1 has minor 2, ...
>
>Also, the third partition is /dev/mtd3, as /dev/mtd0 is the real
>device. Look at the minor numbers for /dev/hda*, just the same.
>
>But what you *meant* is correct.
>
> > Unfortunately, my tests indicate that this is not happening. I know that
> > /dev/mtdblock2 (the intended third partition) is formatted. However JFFS2
> > finds data here and refuses to mount on it. I think it is really being
> > passes a handle to and accesses /dev/mtd2, which is the wrong partition.
>
>See above.


Well, I think there is still a problem somewhere. Regardless of which 
/dev/mtd<n> device is the actual physical partition (that was a good 
academic discussion), JFFS2 never gets passed the /dev/mtd<n> device by me- 
only the /dev/mtdblock<n> device.

I can do a 'less -f /dev/mtdblock2' and see that's the partition that I 
want to mount JFFS2 on (it's all formatted correctly). However, a 'mount -t 
jffs2 /dev/mtdblock2 /mnt/flash' fails as JFFS2 sees a bunch of data to 
whichever mtd device handle its been passed (by mtdblock2) and refuses to 
mount as it cannot find any valid JFFS2 nodes there.

Vipin



> > /dev/mtdblock[4,5] do not exist, even though /dev/mtd[4,5] do and 
> /dev/mtd4
> > is the correct partition that I want!
>
>See above.
>
> > Argh! I'll try your patches if you like.
>
>I would be happy if you did. But you don't need my patches, unless you
>have more than one device that you want to partition. With the two
>thinkos out of the way, you should be doing fine. :)
>
>Jörn
>
>--
>Mundie uses a textbook tactic of manipulation: start with some
>reasonable talk, and lead the audience to an unreasonable conclusion.
>-- Bruce Perens
>
>______________________________________________________
>Linux MTD discussion mailing list
>http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: Disk blocks for long periods

[Jasmine Strong]

On Monday, August 5, 2002, at 03:42 PM, David Woodhouse wrote:

>
> joakim.tjernlund@lumentis.se said:
>>> SysRq-T should work on PPC. It's mostly generic code. What's wrong
>>> with it?
>
>> Maybe I spoke too soon, but I can not make sense of it:
>
> That's because you're using 2.4.2.

...2.4.2 was the last-but-one Redwood PPC kernel release from Montavista;
they have a 2.4.17 out now which is much more stable and faster.

On the 2.4.2 kernel we had lots of odd little problems with stuff like
this that Just Went Away with 2.4.17.

(in ppc405gp land) Jas.

Re: MTD Partition problems

[Jörn Engel]

On Mon, 5 August 2002 14:10:24 -0500, Vipin Malik wrote:
> Well, I think there is still a problem somewhere. Regardless of which 
> /dev/mtd<n> device is the actual physical partition (that was a good 
> academic discussion), JFFS2 never gets passed the /dev/mtd<n> device by me- 
> only the /dev/mtdblock<n> device.

I fail to see what was academic about that discussion. Is it academic,
if you pass /dev/hda1 (empty partition) or /dev/hda (whole disk with
root partition) to mke2fs? Try it and tell me again. :-)

> I can do a 'less -f /dev/mtdblock2' and see that's the partition that I 
> want to mount JFFS2 on (it's all formatted correctly). However, a 'mount -t 
> jffs2 /dev/mtdblock2 /mnt/flash' fails as JFFS2 sees a bunch of data to 
> whichever mtd device handle its been passed (by mtdblock2) and refuses to 
> mount as it cannot find any valid JFFS2 nodes there.

If it was formatted correctly, it should get mounted. But I have no
idea, what "it's all formatted correctly" or "JFFS2 sees a bunch of
data" means. Try doing a trivial format (eraseall /dev/mtd<n>) and
mount that. If that fails, show me that the partition has all 0xff and
the jffs2 messages and I will have a look. Until then, I consider this
to be pebkac.

Jörn

-- 
There's nothing better for promoting creativity in a medium than
making an audience feel "Hmm ­ I could do better than that!"
-- Douglas Adams in a slashdot interview

RE: Disk blocks for long periods

[Joakim Tjernlund]

Hi Dave

This is interesting ...

> I've been working on what may be the same problem and I think I 
> finally understand it. I've seen it with 2.4.4, 2.4.18 and
> with 2.4.18 with the 2.4.19 jffs2 and mtd code. I am using an 
> AM29LV641, which uses cfi_cmdset_0002.c, but the code in 
> cfi_cmdset_0001.c is similar. I have a possible solution
> but I'd like some feedback on it.

Yes, cfi_cmdset_0001.c has the same problem and I think do_write_buffer() is
even more sensitive since is has an extra unlock,udelay,lock pair in it.

> 
> The problem occurs when do_erase_oneblock() tries to lock the 
> flash while cfi_amdstd_write() is writing a lot of data. The 
> erasing thread locks the chip mutex when do_write_oneword() does the
>     cfi_spin_unlock(chip->mutex);
>     cfi_udelay(chip->word_write_time);
>     cfi_spinlock(chip->mutex);
> sequence. It sees the state is FL_WRITING, so it puts itself 
> back on the wait queue. do_write_oneword() continues and eventually
> sets the state back to FL_READY and wakes up the queue, but the
> erasing thread doesn't actually run until the cfi_udelay() in 
> do_write_oneword() calls schedule() while writing the next word
> to the flash. The state is FL_WRITING, so the erasing thread goes
> back on the wait queue. This continues until the entire write is
> finished, then the erasing thread finally starts the erase.
> 
> The effect of all this sheduling is to write exactly one word
> to flash for each jiffie! My flash is 16 bits wide, so a single
> write of 2400 bytes was sometimes taking 1200 jiffies, or
> 12 seconds.
> 
> This patch to cfi_cmdset_0002.c 1.56 seems solve the problem, but
> I am not sure if this is right way to do it. Comments?

I am not sure this is the the rigth way either, but I will test and see what happens.

BTW, is it neccesary to use spin_lock_bh()? Can we not get away with just spin_lock()?
I am not very good at locking, but I think xxx_bh is only needed when interrupts can
can execute the locked code and currently there are no interrupts in this code path(I think).

Is it not true that xxx_bh() also disables interrupts? If so, there must be rather long
periods with interrupts turned off in cfi_cmdset_xxxx.c?

Have you tried to use cfi_udelay(chip->word_write_time) instead of udelay(chip->word_write_time)?
That will at least let other processes run, won't it? 

Perhaps David can comment?

> 
> Dave Ellis
> dge@sixnetio.com
> 
> BTW - If I make similar changes to 1.55 or before it solves this
> problem, but the write fails occasionally. I am guessing that
> with my change it gets to the write completion check faster and the
> old check fails, but the new write completion polling works better.
> 
> --- cfi_cmdset_0002.c	Mon Jul 15 11:13:25 2002
> +++ cfi_cmdset_0002.fixed.c	Mon Aug  5 14:28:40 2002
> @@ -386,9 +384,7 @@
>  
>  	cfi_write(map, datum, adr);
>  
> -	cfi_spin_unlock(chip->mutex);
> -	cfi_udelay(chip->word_write_time);
> -	cfi_spin_lock(chip->mutex);
> +	udelay(chip->word_write_time);
>  
>  
>  	/* Polling toggle bits instead of reading back many times
> @@ -447,6 +444,7 @@
>  	chip->state = FL_READY;
>  	wake_up(&chip->wq);
>  	cfi_spin_unlock(chip->mutex);
> +	cfi_udelay(1);	/* just a chance to schedule() */
>  	
>  	return ret;
>  }
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 

Re: Disk blocks for long periods

[David Woodhouse]

joakim.tjernlund@lumentis.se said:
>  BTW, is it neccesary to use spin_lock_bh()? Can we not get away with
> just spin_lock()? I am not very good at locking, but I think xxx_bh is
> only needed when interrupts can can execute the locked code and
> currently there are no interrupts in this code path(I think).

> Is it not true that xxx_bh() also disables interrupts? If so, there
> must be rather long periods with interrupts turned off in
> cfi_cmdset_xxxx.c?

spin_lock_bh() should not disable interrupts, only bottom-halves. It was 
used to protect against timers, because we intend(ed) to have the erase 
completion done from a timer. At the moment there's no real need to use 
anything but spin_lock().


--
dwmw2

Re: Disk blocks for long periods

[Jörn Engel]

Hi!

Just a wild thought:
With three command sets already implemented (0020 is not in the tree
yet), wouldn't it make sense to go through them and try to collect as
much common code, as possible? I really dislike code multiplication,
especially bug multiplication.

Jörn

-- 
Those who come seeking peace without a treaty are plotting.
-- Sun Tzu

Re: Disk blocks for long periods

[David Woodhouse]

joern@wohnheim.fh-wedel.de said:
> Just a wild thought: With three command sets already implemented (0020
> is not in the tree yet), wouldn't it make sense to go through them and
> try to collect as much common code, as possible? I really dislike code
> multiplication, especially bug multiplication.

Yes, it would. We've done some of it already -- the cfi_cmd() bits, etc. 
Some helpful stuff for handling the state machine and scheduling 
appropriately would also be good.

--
dwmw2

RE: Disk blocks for long periods

[Dave Ellis]

joakim.tjernlund@lumentis.se said:

> Have you tried to use cfi_udelay(chip->word_write_time) 
> instead of udelay(chip->word_write_time)? That will at least 
> let other processes run, won't it? 

The cfi_udelay() call is part of the problem, since it lets other
processes run, and the one that runs is the erasing process, but
it can't do anything except slow things down since the flash 
is busy writing. What the erasing process does seem to do (and 
I don't understand why) is cause current->need_resched to be set
for every call to cfi_udelay(), so it calls schedule_timeout(1) 
and sleeps for a whole jiffie.

By calling cfi_udelay() at the _end_ of the write of each word,
it still allows the call to schedule_timeout() to happen, but
at a time when the chip state allows other mtd processes operations,
such as erase, to use it, so the one jiffie doesn't happen
for every word written. The word writes are short (10 us typical,
360 us worst case for my chip), so not scheduling during the write
is not so bad.

My change seems to solve the problem for me, but I don't 
understand the scheduling well enough to understand why 
current->need_resched gets set each time.

Maybe cfi_udelay() should be changed so for short delays it
does schedule(); udelay(delay); instead of schedule_timeout(1).
If I get some time later I will try it.
 
Dave Ellis
dge@sixnetio.com

> > --- cfi_cmdset_0002.c	Mon Jul 15 11:13:25 2002
> > +++ cfi_cmdset_0002.fixed.c	Mon Aug  5 14:28:40 2002
> > @@ -386,9 +384,7 @@
> >  
> >  	cfi_write(map, datum, adr);
> >  
> > -	cfi_spin_unlock(chip->mutex);
> > -	cfi_udelay(chip->word_write_time);
> > -	cfi_spin_lock(chip->mutex);
> > +	udelay(chip->word_write_time);
> >  
> >  
> >  	/* Polling toggle bits instead of reading back many 
> times @@ -447,6 
> > +444,7 @@
> >  	chip->state = FL_READY;
> >  	wake_up(&chip->wq);
> >  	cfi_spin_unlock(chip->mutex);
> > +	cfi_udelay(1);	/* just a chance to schedule() */
> >  	
> >  	return ret;
> >  }

RE: Disk blocks for long periods

[Joakim Tjernlund]

> 
> joakim.tjernlund@lumentis.se said:
> 
> > Have you tried to use cfi_udelay(chip->word_write_time) 
> > instead of udelay(chip->word_write_time)? That will at least 
> > let other processes run, won't it? 
> 
> The cfi_udelay() call is part of the problem, since it lets other
> processes run, and the one that runs is the erasing process, but
> it can't do anything except slow things down since the flash 
> is busy writing. What the erasing process does seem to do (and 
> I don't understand why) is cause current->need_resched to be set
> for every call to cfi_udelay(), so it calls schedule_timeout(1) 
> and sleeps for a whole jiffie.

hmm, I just noticed that the BIG kernel lock is held while the
erasing thread executes, since kupdate takes it in sync_old_buffers()
(if I understand this code correctly). Maybe that is causing
current->need_resched to be set?

Also, is it not a bad idea to hold the kernel lock for, possibly, seconds while
erasing sectors?

> 
> By calling cfi_udelay() at the _end_ of the write of each word,
> it still allows the call to schedule_timeout() to happen, but
> at a time when the chip state allows other mtd processes operations,
> such as erase, to use it, so the one jiffie doesn't happen
> for every word written. The word writes are short (10 us typical,
> 360 us worst case for my chip), so not scheduling during the write
> is not so bad.

I assume your chip does not support buffered writes? or is it just do_write_one_word()
that is causing the log delays?

 Jocke

RE: Disk blocks for long periods

[Joakim Tjernlund]

> 
> spin_lock_bh() should not disable interrupts, only bottom-halves. It was 
> used to protect against timers, because we intend(ed) to have the erase 
> completion done from a timer. At the moment there's no real need to use 
> anything but spin_lock().

Can the JFFS2 layer also use spin_lock() instead of spin_lock_bh()?
Shold we try not to disable bottom-halves or does it not matter?

BTW,
     I just did a test, ran this some 20 times:
     #> time cp tstfile tmp1 & time cp file tmp2 & time cp file tmp3 &
     sometimes I removed the tmp1,tmp2 and tmp3 files before running it.

     if I rm the files first it takes 10 seconds to complete and if I don't
     it takes 20 seconds. It's a bit much I think.

I have set kupdate to run every .5 second   
tstfile is 581K

 Jocke



> 
> 
> --
> dwmw2
> 
> 
> 

Re: MTD Partition problems

[Vipin Malik]

>
> > However, if one ignores the duplicated devices, one can still use
> > partitioning and mount a JFFS2 fs on one of the partitions while having
> > something completely different on another and device access locking 
> will be
> > taken care of by the mtd driver (which is essentially what partitioning
> > provides vs. just calling add_mtd() NUM_PARTITION times in your map 
> file on
> > the same flash chip).
>
>Yes. The partitioning code basically checks the limits and hands the
>work to the base device with adjusted offset.
>
> > Additionally, (say) /dev/mtdblock2 will really provide a handle to
> > /dev/mtd4 the writable third partition on the partitioned flash chip, thus
> > mounting the JFFS2 fs on the partition originally intended (the third
> > partition).
>
>Almost. /dev/mtd4 should be minor number 8. If you check
>/usr/src/linux/Documentation/devices.txt, you will read the following:
>                   0 = /dev/mtd0         First MTD (rw)
>                   1 = /dev/mtdr0        First MTD (ro)
>Thus mtd0 has minor 0, mtd1 has minor 2, ...
>
>Also, the third partition is /dev/mtd3, as /dev/mtd0 is the real
>device. Look at the minor numbers for /dev/hda*, just the same.


I got the JFFS2 to mount on /dev/mtdblock2 (most of the partition was 
erased. I must have passed the wrong size to the mtd_debug erase. There was 
some garbage at the end).

Anyway, thanks for the explanations.

Vipin


>But what you *meant* is correct.
>
> > Unfortunately, my tests indicate that this is not happening. I know that
> > /dev/mtdblock2 (the intended third partition) is formatted. However JFFS2
> > finds data here and refuses to mount on it. I think it is really being
> > passes a handle to and accesses /dev/mtd2, which is the wrong partition.
>
>See above.
>
> > /dev/mtdblock[4,5] do not exist, even though /dev/mtd[4,5] do and 
> /dev/mtd4
> > is the correct partition that I want!
>
>See above.
>
> > Argh! I'll try your patches if you like.
>
>I would be happy if you did. But you don't need my patches, unless you
>have more than one device that you want to partition. With the two
>thinkos out of the way, you should be doing fine. :)
>
>Jörn
>
>--
>Mundie uses a textbook tactic of manipulation: start with some
>reasonable talk, and lead the audience to an unreasonable conclusion.
>-- Bruce Perens
>
>______________________________________________________
>Linux MTD discussion mailing list
>http://lists.infradead.org/mailman/listinfo/linux-mtd/

RE: Disk blocks for long periods

[Dave Ellis]

joakim.tjernlund@lumentis.se said:

> hmm, I just noticed that the BIG kernel lock is held while 
> the erasing thread executes, since kupdate takes it in 
> sync_old_buffers() (if I understand this code correctly). 
> Maybe that is causing current->need_resched to be set?
> 
> Also, is it not a bad idea to hold the kernel lock for, 
> possibly, seconds while erasing sectors?

If it is doing that, it doesn't seem right to me. But I 
thought jffs2/mtd bypassed the kernel buffers, so maybe 
kupdate and sync_old_buffers() are not involved here? I don't
know much about this.
 
> I assume your chip does not support buffered writes? or is it 
> just do_write_one_word() that is causing the log delays?

No, it doesn't support buffered writes. The long delay I saw
comes from sleeping for a jiffie during each call to
do_write_oneword().

Dave Ellis
dge@sixnetio.com

Re: Disk blocks for long periods

[Joakim Tjernlund]

> joakim.tjernlund@lumentis.se said:
> 
> > hmm, I just noticed that the BIG kernel lock is held while 
> > the erasing thread executes, since kupdate takes it in 
> > sync_old_buffers() (if I understand this code correctly). 
> > Maybe that is causing current->need_resched to be set?
> > 
> > Also, is it not a bad idea to hold the kernel lock for, 
> > possibly, seconds while erasing sectors?
> 
> If it is doing that, it doesn't seem right to me. But I 
> thought jffs2/mtd bypassed the kernel buffers, so maybe 
> kupdate and sync_old_buffers() are not involved here? I don't
> know much about this.

hmm, just remembered that a spin_lock() is NOP on Uni Processor computers.

Kupdate does the erasing and puts a CLEANMARKER on it when its 
done.  Just follow sync_old_buffers()  until it ends up in jffs2_write_super()

BTW
          jffs2_mark_erased_blocks() also checks the newly erased block for
         0xffffffff by reading the EB back. Removing this test improves my litte test case 
         I wrote about in my previous post to the mtd list. The max copy time is reduced
         from 20 seconds to 15 seconds.

        Jocke 

>  
> > I assume your chip does not support buffered writes? or is it 
> > just do_write_one_word() that is causing the log delays?
> 
> No, it doesn't support buffered writes. The long delay I saw
> comes from sleeping for a jiffie during each call to
> do_write_oneword().

I tried your patch and it improved my write times as well. Thanks

> 
> Dave Ellis
> dge@sixnetio.com

Re: Disk blocks for long periods

[David Woodhouse]

joakim.tjernlund@lumentis.se said:
>  Can the JFFS2 layer also use spin_lock() instead of spin_lock_bh()?
> Shold we try not to disable bottom-halves or does it not matter?

Well, the MTD API documentation (in my head) says that the erase completion 
callback may be called in bottom-half context (i.e. from a timer), which is 
why JFFS2 uses spin_lock_bh(). 

I suppose we could change that.

--
dwmw2

Re: Disk blocks for long periods

[David Woodhouse]

DGE@sixnetio.com said:
> The cfi_udelay() call is part of the problem, since it lets other
> processes run, and the one that runs is the erasing process, but it
> can't do anything except slow things down since the flash  is busy
> writing.

The erasing process should be sitting in TASK_UNINTERRUPTIBLE and on the 
chip's wait queue, waiting to be woken when the chip becomes available. Why 
is it running?

> What the erasing process does seem to do (and  I don't understand why)
> is cause current->need_resched to be set for every call to
> cfi_udelay(), so it calls schedule_timeout(1)  and sleeps for a whole
> jiffie. 

Sleeping for a whole jiffie is bad. We should udelay() and yield() as you 
point out; there's no reason to use schedule_timeout() and hence stop 
ourself from being woken immediately if there's not really anything else to 
run. I've committed that change to CVS.

I don't understand why need_resched is being set either, though. 

Adding cfi_udelay() to do_write_oneword() is dangerous. You may trigger a
bug that's actually already there. ISTR there's a time limit on the 'unlock
bypass' mode, and if you schedule() you may find that the chip is no longer 
in that mode when you return.

Another thing you might want to do if you really care about write 
performance is copy the logic for calculating word_write_time from the 
cfi_cmdset_0001 driver.

--
dwmw2

RE: Disk blocks for long periods

[Dave Ellis]

David Woodhouse said:
> DGE@sixnetio.com said:
> > The cfi_udelay() call is part of the problem, since it lets other 
> > processes run, and the one that runs is the erasing process, but it 
> > can't do anything except slow things down since the flash  is busy 
> > writing.
> 
> The erasing process should be sitting in TASK_UNINTERRUPTIBLE and on the 
> chip's wait queue, waiting to be woken when the chip becomes available.
Why 
> is it running?

It is running because of the
        wake_up(&chip->wq);
at the end of do_write_oneword(). So it wakes up for every word written.

With the old code (cfi_cmdset_0002.c 1.56 and before) the next chance it 
gets to run is when cfi_udelay() is called during the next 
do_write_oneword(). Unfortunately, it can't start erasing because 
chip->state is FL_WRITING, so it puts itself back on the wait queue. The
wake_up() at the end of do_write_oneword() wakes it up again, but it
doesn't run until the cfi_delay() in the _next_ call to do_write_oneword().
So it runs and puts itself back on the wait queue for every call to 
do_write_oneword(). Eventually all the words are written, it wakes up to 
find FL_READY and starts the erase. Without the one jiffie delay in 
cfi_delay() each time it wouldn't have been as bad, but erase going off 
and back on the wait queue several thousand times still doesn't seem right.

> > What the erasing process does seem to do (and  I don't understand why) 
> > is cause current->need_resched to be set for every call to 
> > cfi_udelay(), so it calls schedule_timeout(1) and sleeps for a whole 
> > jiffie.
> 
> Sleeping for a whole jiffie is bad. We should udelay() and yield() as you 
> point out; there's no reason to use schedule_timeout() and hence stop 
> ourself from being woken immediately if there's not really anything else
to 
> run. I've committed that change to CVS.

Thanks, I looked at the change and seems much better to me.

> Adding cfi_udelay() to do_write_oneword() is dangerous. You 
> may trigger a bug that's actually already there. ISTR there's 
> a time limit on the 'unlock bypass' mode, and if you 
> schedule() you may find that the chip is no longer 
> in that mode when you return.

Actually I didn't add it, I just moved it to the end, where chip->state
is FL_READY, so the erase can actually start if it wakes up. I think
both the old code and your latest change to cfi_cmdset_0002.c
would have the same problem with a time limit, since they call 
cfi_udelay() or yield() while each word is writing. I didn't
find any time limit for my chip.

I think my change (which really should be cond_resched(), not 
cfi_udelay() since I don't need the delay) would cause a 
different problem with 'unlock bypass' (which I am not using).
The chip is tagged as FL_READY when it really (I guess) 
isn't. But if I schedule() between words with the chip not 
FL_READY, I am right back to the original problem. And if I
don't schedule() until the whole write is done nothing else
can run for a long time.

Maybe if 'unlock bypass' is used the
  chip->state = FL_READY; wake_up(&chip->wq);
should only happen after leaving 'unlock bypass' mode?

I wonder if 'unlock bypass' is worth all the trouble. I think
I'll just not use it and call cond_resched() instead of 
cfi_udelay() at the end of do_write_oneword().
 
> Another thing you might want to do if you really care about write 
> performance is copy the logic for calculating word_write_time 
> from the cfi_cmdset_0001 driver.

Thanks for the suggestion, but the write performance with my
fix is fine so I may not try it for a while. The long delays
(up to 60 seconds) on single write() were the problem. They 
were making a communications protocol time out and making total
load times very long. Without them, the average performance is 
fine.

I am very impressed with jffs2 and mtd. With this one problem
fixed it is exactly what we need and works very well.

Dave Ellis
dge@sixnetio.com

RE: Disk blocks for long periods

[Joakim Tjernlund]

> David Woodhouse said:
> > DGE@sixnetio.com said:
> > > The cfi_udelay() call is part of the problem, since it lets other 
> > > processes run, and the one that runs is the erasing process, but it 
> > > can't do anything except slow things down since the flash  is busy 
> > > writing.
> > 
> > The erasing process should be sitting in TASK_UNINTERRUPTIBLE and on the 
> > chip's wait queue, waiting to be woken when the chip becomes available.
> Why 
> > is it running?
> 
> It is running because of the
>         wake_up(&chip->wq);
> at the end of do_write_oneword(). So it wakes up for every word written.
> 
> With the old code (cfi_cmdset_0002.c 1.56 and before) the next chance it 
> gets to run is when cfi_udelay() is called during the next 
> do_write_oneword(). Unfortunately, it can't start erasing because 
> chip->state is FL_WRITING, so it puts itself back on the wait queue. The
> wake_up() at the end of do_write_oneword() wakes it up again, but it
> doesn't run until the cfi_delay() in the _next_ call to do_write_oneword().
> So it runs and puts itself back on the wait queue for every call to 
> do_write_oneword(). Eventually all the words are written, it wakes up to 
> find FL_READY and starts the erase. Without the one jiffie delay in 
> cfi_delay() each time it wouldn't have been as bad, but erase going off 
> and back on the wait queue several thousand times still doesn't seem right.

You can move the wake_up() call up one step(xxxx_write_words()) and call it just before 
you leave that function. I tried that and it's a improvement(note, I using buffer writes, so
word writes should see an even better improvement), but that will also
hold reads back(not that I noticed anything) until xxx_write_words has completed. 
We can fix that by having separate wait queues for read and erase.

   Jocke

RE: Disk blocks for long periods

[Joakim Tjernlund]

> 
> joakim.tjernlund@lumentis.se said:
> >  Can the JFFS2 layer also use spin_lock() instead of spin_lock_bh()?
> > Shold we try not to disable bottom-halves or does it not matter?
> 
> Well, the MTD API documentation (in my head) says that the erase completion 
> callback may be called in bottom-half context (i.e. from a timer), which is 
> why JFFS2 uses spin_lock_bh().

I noticed that erase_callback() in erase.c useses spin_lock(), not spin_lock_bh().
Should it?
 
> 
> I suppose we could change that.

I dont understand what using a timer would give us. I suppose that one could have
more the one erase simontainously(one per bank) but that won't give a significant
improvement and erase.c must be made "bank aware" as well. 

Unless there is some other advantage, I think you should change the spec.

 Jocke

> 
> --
> dwmw2
> 
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
> 

Re: Disk blocks for long periods

[David Woodhouse]

joakim.tjernlund@lumentis.se said:
>  I noticed that erase_callback() in erase.c useses spin_lock(), not
> spin_lock_bh(). Should it?

It expects to actually be called in bottom-half context, and if it isn't 
then nothing else will be. So it's fine to use spin_lock() there.

>  I dont understand what using a timer would give us. I suppose that
> one could have more the one erase simontainously(one per bank) but
> that won't give a significant improvement and erase.c must be made
> "bank aware" as well. 

> Unless there is some other advantage, I think you should change the
> spec.

Using a timer to check for erase completion means that the process which 
submitted the erase doesn't have to be blocked waiting for it. The callback 
can happen some time later. In the case of JFFS2, it's kupdated that ends 
up stuck in the erase routine for ages.

But using a timer to actually do the check doesn't mean we have to call the 
completion callback in timer context -- we could use schedule_task() to run 
it in process context.

--
dwmw2

Re: Disk blocks for long periods

[David Woodhouse]

joakim.tjernlund@lumentis.se said:
>  You can move the wake_up() call up one step(xxxx_write_words()) and
> call it just before  you leave that function. I tried that and it's a
> improvement(note, I using buffer writes, so word writes should see an
> even better improvement), but that will also hold reads back(not that
> I noticed anything) until xxx_write_words has completed.  We can fix
> that by having separate wait queues for read and erase. 

Aren't reads held back anyway for the same reason that erases are -- by the 
time we actually schedule after waking them, the chip is in FL_WRITING 
again?

What happens if you stick a cond_resched() immediately after the wake_up() 
(ok, after the spin_unlock).?

--
dwmw2

RE: Disk blocks for long periods

[Joakim Tjernlund]

> 
> joakim.tjernlund@lumentis.se said:
> >  I noticed that erase_callback() in erase.c useses spin_lock(), not
> > spin_lock_bh(). Should it?
> 
> It expects to actually be called in bottom-half context, and if it isn't 
> then nothing else will be. So it's fine to use spin_lock() there.
OK.
> 
> >  I dont understand what using a timer would give us. I suppose that
> > one could have more the one erase simontainously(one per bank) but
> > that won't give a significant improvement and erase.c must be made
> > "bank aware" as well. 
> 
> > Unless there is some other advantage, I think you should change the
> > spec.
> 
> Using a timer to check for erase completion means that the process which 
> submitted the erase doesn't have to be blocked waiting for it. The callback 
> can happen some time later. In the case of JFFS2, it's kupdated that ends 
> up stuck in the erase routine for ages.

Maybe erasing should be moved away from kupdate then, to the gc thread or a new
erase thread that does not mind beeing blocked?
> 
> But using a timer to actually do the check doesn't mean we have to call the 
> completion callback in timer context -- we could use schedule_task() to run 
> it in process context.

OK, so if schedule_task() is used instead its possible to replace spin_lock_bh
with spin_lock everywhere?

I think we should change erase.c to do atmost 3 erase & mark in one go.
That will free up space more quickly when there is a lot of pending erases.

 Jocke
> 
> --
> dwmw2
> 
> 
> 

Re: Disk blocks for long periods

[David Woodhouse]

joakim.tjernlund@lumentis.se said:
> Maybe erasing should be moved away from kupdate then, to the gc thread
> or a new erase thread that does not mind beeing blocked?

Maybe. I want to keep the GC thread optional though. I think it's 
sufficient just to fix the chip drivers to really do asynchronous erases, 
so the thread calling ->erase() doesn't actually have to block. Then we can 
leave it in kupdate.

>  OK, so if schedule_task() is used instead its possible to replace
> spin_lock_bh with spin_lock everywhere?

Yes.

> I think we should change erase.c to do atmost 3 erase & mark in one
> go. That will free up space more quickly when there is a lot of
> pending erases. 

Seems reasonable.

--
dwmw2

RE: Disk blocks for long periods

[Joakim Tjernlund]

> 
> joakim.tjernlund@lumentis.se said:
> >  You can move the wake_up() call up one step(xxxx_write_words()) and
> > call it just before  you leave that function. I tried that and it's a
> > improvement(note, I using buffer writes, so word writes should see an
> > even better improvement), but that will also hold reads back(not that
> > I noticed anything) until xxx_write_words has completed.  We can fix
> > that by having separate wait queues for read and erase. 
> 
> Aren't reads held back anyway for the same reason that erases are -- by the 
> time we actually schedule after waking them, the chip is in FL_WRITING 
> again?

Yes, that why you need 2 queues. Then you do wake_up()&cond_resched(). 
The read queue get woken after every word/buffer write and
the erase queue after the whole read/write has completed.

> 
> What happens if you stick a cond_resched() immediately after the wake_up() 
> (ok, after the spin_unlock).?

I had that before I moved wake_up one step. My copy test was improved
with almost 1 sec(from ~10 to ~9) when the wake_up()&cond_resched() was one step up.

 Jocke
> 
> --
> dwmw2
> 
> 
> 

Re: Disk blocks for long periods

[David Woodhouse]

joakim.tjernlund@lumentis.se said:
>  Yes, that why you need 2 queues. Then you do wake_up()&cond_resched().
>   The read queue get woken after every word/buffer write and the erase
> queue after the whole read/write has completed. 

OK, right. That makes sense.

--
dwmw2