Thread: Re: [Pramfs-devel] pramfstools
Status: Beta
Brought to you by:
vdavydov825
From: Marco S. <mar...@gm...> - 2013-08-26 15:15:40
|
Hi, Il 25/08/2013 20:28, Vladimir Davydov ha scritto: > Hello, > > I've written the fsck utility for PRAM FS. You can clone it from > git://github.com/locker/pramfstools > > Note that it requires read (write in case of repair) access to /dev/mem, > because AFAIK this is currently the only way to operate with PRAM FS > image, so STRICT_DEVMEM kernel config option must be turned off. > > Also note that currently it does not support PRAM FS xattrs, but I'm > planning to work this out. > > For more information and example of use cases, see README. > > I would appreciate if you could look at this. Perhaps it'll be worth > merging this to the PRAM FS repo at sourceforge if you find the code > acceptable, because IMHO it would be nice to have all the bits regarding > PRAM FS gathered in one place. Anyway, if you have any comments > regarding the tool's interface/coding style/etc, please let me know. > > Thanks. Great. A really good work. I create a new git repo: git clone git://git.code.sf.net/p/pramfs/Tools pramfs-Tools If you have got a sourceforge account I can add you to the project and I can give you write access. I didn't read the code in a deep way but my first comments: 1) I think it's a good idea to check at the beginning if the fs is already mounted. On line check and repair can be dangerous. We can read the proc files to know it; 2) The pram_fs.h shouldn't be tracked in the repo. When you compile you should have a reference in your kernel tree. Indeed this file isn't in the private fs folder just for that; 3) With the next version we have to change a couple of things: crc is now 32 bit because it has got better performance and now the structures are big enough to require a 32 bit crc. In addition I think we have to manage the O_TMPFILE, we could have inode without any links (but maybe this case is already covered in "orphaned inodes", I didn't see it yet). Marco |
From: Marco S. <mar...@gm...> - 2013-08-27 15:33:30
|
Il 27/08/2013 09:37, Vladimir Davydov ha scritto: > On 08/26/2013 07:08 PM, Marco Stornelli wrote: >> Hi, >> >> Il 25/08/2013 20:28, Vladimir Davydov ha scritto: >>> Hello, >>> >>> I've written the fsck utility for PRAM FS. You can clone it from >>> git://github.com/locker/pramfstools >>> >>> Note that it requires read (write in case of repair) access to /dev/mem, >>> because AFAIK this is currently the only way to operate with PRAM FS >>> image, so STRICT_DEVMEM kernel config option must be turned off. >>> >>> Also note that currently it does not support PRAM FS xattrs, but I'm >>> planning to work this out. >>> >>> For more information and example of use cases, see README. >>> >>> I would appreciate if you could look at this. Perhaps it'll be worth >>> merging this to the PRAM FS repo at sourceforge if you find the code >>> acceptable, because IMHO it would be nice to have all the bits regarding >>> PRAM FS gathered in one place. Anyway, if you have any comments >>> regarding the tool's interface/coding style/etc, please let me know. >>> >>> Thanks. >> >> Great. A really good work. I create a new git repo: >> >> git clone git://git.code.sf.net/p/pramfs/Tools pramfs-Tools >> >> If you have got a sourceforge account I can add you to the project and >> I can give you write access. > > I've just joined :-) username is vdavydov825 > >> I didn't read the code in a deep way but my first comments: >> >> 1) I think it's a good idea to check at the beginning if the fs is >> already mounted. On line check and repair can be dangerous. We can >> read the proc files to know it; > > Agree. > >> >> 2) The pram_fs.h shouldn't be tracked in the repo. When you compile >> you should have a reference in your kernel tree. Indeed this file >> isn't in the private fs folder just for that; > > Agree. > >> >> 3) With the next version we have to change a couple of things: crc is >> now 32 bit because it has got better performance and now the >> structures are big enough to require a 32 bit crc.In addition I think >> we have to manage the O_TMPFILE, we could have inode without any links >> (but maybe this case is already covered in "orphaned inodes", I didn't >> see it yet). > > I guess check_orphaned_inodes() should handle O_TMPFILE properly > although initially I implemented it for another reason. This is how FS > tree check currently works: > > 1. Walk over FS tree starting from the root and descending to leafs > marking all found inodes as 'reachable' > > 2. Iterate over the inode table checking if there are active (i.e. not > deleted) inodes that are not marked as 'reachable' - I call them > orphaned inodes. For each orphaned inode, first try to find its parent > as specified by i_d.d_parent. If found, try to repair it, i.e. link back > to the parent dir. If there is no active parent or parent link is > invalid, prompt to delete it, because it's unrecoverable. Ok. Here actually we can follow another way, but it can be an item on our todo list. Instead to delete it, we can create a lost+found folder and attach them to this new folder. > > Initially, check for orphaned inodes was implemented, because PRAM FS > directory entry structure is rather unreliable - corruption of an inode > will result in loss of all inodes that follow it in the same directory. > To avoid this, I try to recover inodes that were not found reachable. > > However, this should also work for O_TMPFILE. AFAIU, O_TMPFILE is in > fact the same as an open unlinked file, i.e. it has i_links_count=0 and > i_d zeroed out, but i_dtime is not set and i_mode is not cleared yet so > that it does not appear as deleted. So it won't be found and marked as > 'reachable' during walk over FS tree. Thus it will be later found as > orphaned and unrecoverable since i_d.d_parent=0, and prompted to be > deleted. The deletion won't free blocks allocated by the inode > immediately, but the blocks should be freed in check_block_alloc() > automatically, because they will also become orphaned with the deletion > of the inode. > Yep, I quite agree. > I will fix the code according to your comments above and notify you when > it's done. > > Thanks. Thanks again. Marco |