Re: Amiga SFS file system Linux driver

[Roman Z. <zi...@li...>]

Hi,

(Sorry for the delay, I was ill for a few days last week.)

Marek Szyprowski wrote:

> > - wrap the access to asfs_inode_info/asfs_sb_info into macros, this will
> > make it easier to port to 2.5/6.
> 
> Same as in for example AFFS driver?

Yes, in 2.5 you'll find this in pretty much every driver.

> > - you likely want to implement a get_block function like other file
> > systems do instead of implementing the readpage function.
> 
> I implemented this, but... The new version is terribly slow! Kernel waits
> few second before it reads almost every single block! I don't know where is
> the problem. My routines are simmilar to the routines used in other file
> systems (I based on AFFS driver). I thinks that I forgot to set-up
> something, but I don't know what. Could you help me finding the bug?

I had the time now to play a little with and you need to add the
sync_page function to asfs_aops, otherwise the I/O is started rather
randomly.

> > - it's probably also better to avoid the from32be/from16be macros and
> > convert the values as needed.
> 
> Is it really the problem? I decided to correct all values in reading
> functions, so the other functions do not need to care wheather the values
> are in big or little endian...

It makes further development simpler, at some point you have to remove
it anyway. First, it makes reading the source easier, if one knows which
structures are on disk and which are in memory and each should be in its
native order. Right now you don't cache any data, but as soon as you do
that, you will need separate structures to cache disk data. It becomes
even more important, if you want to add write support.

I'll commit a version with a few small fixes. Here a few more things I
noticed while going over the driver:
- You should read the inode via the read_inode2 function (use iget4 and
you can pass it the fsObject), this avoids creating inodes with
duplicate inode numbers.
- asfs_search_BNodeTree shouldn't return structures, use a pointer and
return a proper error value instead.
- if asfs is case-insensitive, you'll probably want to add
dentry_operations similiar to affs.
- please make the lines shorter, they don't have to be exactly 80
characters, but it's a good goal.

bye, Roman