From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kai Bankett Subject: Re: [PATCH]QNX6 filesystem (RO) driver Date: Wed, 15 Feb 2012 03:52:01 +0100 Message-ID: <4F3B1DD1.3000202@ontika.net> References: <4F29A37F.6070909@ontika.net> <20120212045629.GO23916@ZenIV.linux.org.uk> <4F3839CE.8040805@ontika.net> <20120212224358.GW23916@ZenIV.linux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: linux-fsdevel@vger.kernel.org To: Al Viro Return-path: Received: from mailout05.t-online.de ([194.25.134.82]:35969 "EHLO mailout05.t-online.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757536Ab2BOBwA (ORCPT ); Tue, 14 Feb 2012 20:52:00 -0500 In-Reply-To: <20120212224358.GW23916@ZenIV.linux.org.uk> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 02/12/2012 11:43 PM, Al Viro wrote: > On Sun, Feb 12, 2012 at 11:14:38PM +0100, Kai Bankett wrote: > > re junk removal from qnx4 - sure, just make is a separate patch. > >>> qnx6_iget() - what, they really don't allow named pipes/sockets/device >>> nodes on the filesystem? If not, you can just use init_special_inode() >>> instead of that printk+iget_failed(). >> At least so far I could not find any additional extras. >> I followed your advise and used init_special_inode() to get rid of >> that printk+stuff. > Umm... How does qnx encode device numbers for block/character devices? Actually I have to check again. My statement was not meant to say "there are none", but so far I have not seen any. I did my reverse engineering on test qnx6 filesystems created with QNX. So far I have not analyzed these special cases. I will have to try to generate some. Will come back on that one after some analysis. Maybe I will have time tomorrow to dig a bit deeper into that ... Anyways, the init_special_inode() at the final end seems to be right there, so at least one point less on the list. > A few more things: > * duplicating di_block_ptr[] array seems to be pointless Again, well spotted. Just removed the "raw" thing. To me just taking the required stuff into qnx6_inode_info gives better control and more speed. > * qnx6_get_inode_loc(), qnx6_dir_lfile_block() and qnx6_block_map() > seem to have a lot of duplicated code, at the very least. Looks like > a missing helper function... Yep. With the write code it will become even more. Solution found. Just block_map() survived. > Incidentally, I'd switched qnx6_get_devblock() > and qnx6_check_blockptr() to __fs32 argument, along with making QNX6_PTR_UNSET > (~(__fs32)0) That was my exactly first approach. However, I finally decided to move away from that. My thinking was that maybe qnx6_get_devblock() in the future may be called from with a non-__fs32 value holding function. Also, If I see it right, it won't save any cpu cycles + getting to a "uniformed" endianess directly at the fs address pointer reading functions makes the endianess border very clear when reading the source. Feedback very welcome. Easy to change, if you think __fs32 arguments make more sense. > * what is actually stored in ->de_size for long entries in case > of big-endian fs? 4 bytes of de_inode - fine, as as for short ones, but > then what? If it's really 32bit big-endian 0xff, it will have the first byte > equal to 0, which would confuse the living hell out of your code. Or is > it actually __u8 + 3 bytes of padding? Then it would be compatible with > short dir entries, but conversions would be the wrong thing to do there... Checked against a hexdump. The de_size for long filename direntries is just a single byte. Corrected - thanks. > * I seriously suspect that you want to cook yourself a couple of > unhashed in-core struct inode for Longfile and Inode ones; then get_inode_loc(), > dir_lfile_block() and block_map() would become identical *and* you just might > be able to use page cache instead of all that messing with buffer_heads in > readdir et.al. Just do new_inode() and fill di_block_ptr/levels/i_mode/i_size > and ->a_ops. iput() both in ->put_super() and that's it... For directories > in pagecache see how e.g. ext2 is doing it - or minixfs, for that matter. I had a look at that point. After successfully switching find_enty() to pagemap I spend quite some time on switching long_match() to pagemap cache. At the end I gave up. Things just became too complicated. Far more complicated than bh stuff. At least if I get no further clue on how to manage the different pagesizes efficient - compared to the very efficient bh code - I cannot see light at the end of the tunnel. For long Filenames, the Filename always is stored in a seperate block. Whatever I tried, I did not get the pagemap code as efficient, as it abstracts from blocksize. (that's where it got it's stength - IHMO) Ok, I could save the block adressing steps, but I will add so many other steps on the way ... I am really unsure if that pays off at the end. At least code complexity rises extremely. Extremley happy if you could give me some additional guidance an that point. > * mmu_private is never used. Just lose it... > It's history. Updated Patch: http://a6.ontika.net/patches/patch-3.2.5-qnx6-v2.gz Signed-off-by: Kai Bankett