Updated patch: http://a6.ontika.net/patches/patch-3.2.5-qnx6.gz
Signed-off-by: Kai Bankett <chaosman@xxxxxxxxxx>
On 02/12/2012 05:56 AM, Al Viro wrote:
On Wed, Feb 01, 2012 at 09:41:35PM +0100, Kai Bankett wrote:
- Endianness support
QNX6 offers to create filesystems either little- or big endian. That's
an advantage if the created filesystem later is used on an architecture
that got a different endianness.
I introduced some TO_CPUxx() macros and a superblock detection mechanism
for detecting the FS endianness.
Could not test it on a different architecture than x86 and therefore I'm
unsure if it works on a different endian linux system, but from (my)
theory it should do.
At least on x86 it's no problem to mount a different endian QNX6 FS and
read from it - that's what I've tested.
If the macros are the right approach ... well, happy to receive feedback ;)
Nope, that's not the right way to do it. Don't use __leNN for that; define
your own types. Take a look at e.g. fs/sysv/sysv.h - __fs{16,32} in there
and functions for work with those. And duplicate that. What's more, you
really want the direction of conversion spelled out; sure, it's an involution
and you'll get the same code in both directions. But it's much easier to
verify that conversions are right that way. sparse is useful for that, BTW.
And don't use the same variable for host- and fs-endian values - it's not
worth the PITA.
Thanks alot. That was exactly what I was looking for when implementing
that TO_CPU stuff.
Looked around in some other fs drivers, but did not have a look in the sysv.
You are right, using the alignment functions asymetric makes
understanding the code far easier as one can directly see in which
direction things are moved.
At least sparse seems to be fine with my work ...
Don't bother with struct qnx6_ptr - sparse _will_ complain about all places
where you do arithmetics on __bitwise types.
While we are at it, please use explicitly-sized type when you are dealing
with disk block numbers. And pick unsigned one...
I decided to use unsigned int. All adressing is using 32bit pointers. So
this seems to make most sense to me.
Note that unsigned long is target-dependent; on some it's 64bit, on some
32bit. For some things that's exactly what we want, for some...
Fixed. I decided to stay with unsigned int. Just a couple of blocks (12
blocks of 32bit).
So, in case someone is really creating a full 32bit * blocksize fs,
there could be a problem.
I currently can not test how QNX would deal with that special situation
- sorry.
I hope that's ok ...
qnx6_find_bits(): what's wrong with ilog2()? Or order_base_2(), if you
care about the case when argument itself may be not a power of 2. Both
are in linux/log2.h...
Great. Exactly what I was looking for!
Fixed.
sb_set_blocksize() may fail; check return value...
Fixed
WTH are writepage and friends doing in read-only fs driver, especially
seeing that you don't seem to have anything resembling a block allocator?
Fixed
Those were leftovers from the qnx4 fs driver.
Maybe it would be a good idea if I create a patch for the qnx4 driver
for removing those?
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.
_Never_ do strlen() on unsanitized on-disk data; you can't guarantee a
terminating '\0' in there.
Fixed.
Full ack. One more leftover from the qnx4 driver.
However, it seems that the qnx4 driver cannot live without. (at least
from a quick check, I'm not deep enough in the qnx4 fs)
+ /* "" means "." ---> so paths like "/usr/lib//libc.a" work */
+ if (!len&& (lf->lf_fname[0] == '.')&& (lf->lf_fname[1] == '\0'))
+ return TO_CPU32(dir->i_sb, de->de_inode);
Huh? That thing is not going to get called with len == 0 and paths like
that are handled in fs/namei.c just fine without forcing every fs driver
to do that kind of contortions. Moreover, . and .. are _also_ handled
there - ->lookup() never has to deal with those.
Removed.
Leftover from qnx4 driver.
In my eyes that one could well be also removed from there.
Incidentally, don't you have the name length right in the directory
entry? Or is it just an upper limit? If it's exact, it would be faster
to reject mismatches without looking at the name contents (and do memcmp()
in case of match).
Yes, I have the name length in the directory entry, but only for short
filenames.
For long filenames the length field in the directory entry is always 0xff.
I just moved the length check up by one if-clause.
Personally I don't think that moving it around further makes that much
sense, as the length for short filenames is also checked before copying
the filename.
The way it currently is it's at least symmetrical between short und long
filename handling code.
I hope I got all the points and fixes right?
Additionally I removed the old bitmap.c. Currently it was not used anyways.
I also cleaned up the previously (at least in my eyes) ugly
qnx6_super_block and mmi superblock structure.
As the superblock root nodes (inode, bitmap and longfilename) all got
the same structure, I've added a qnx6_root_inode structure for each root
node in the superblock.
Thank you very much for the extremely helpful feedback!
Kai
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
