2.5.2-pre7 still missing bits of kdev_t
Merging Andries' changes for these files gets me a full build:
./fs/reiserfs/inode.c./fs/reiserfs/super.c./fs/reiserfs/journal.c./fs/ext3/super.c./include/linux/reiserfs_fs.h
--alessandro
"this machine will, will not communicate
these thoughts and the strain I am under
be a world child, form a circle before we all go under"
(Radiohead, "Street Spirit [fade out]")
Quoted 4 lines · Alessandro Suardi
./fs/reiserfs/inode.c./fs/reiserfs/super.c./fs/reiserfs/journal.c./include/linux/reiserfs_fs.h
reiserfs is blindly storing the kernel's kdev_t value raw to disk.
AFAICS this will need a policy decision not just cleanup, before it works in 2.5.2 properly. If we switch the kernel to 12:20 major:minor numbers, suddenly the reiserfs disk format changes based on kernel version, and earlier kernels see corrupted major:minor numbers.
For many filesystems with just 16-bits of major/minor storage, they store the raw kdev_t value as well, but have different problems when 12:20 comes around.
If reiserfs guys had planned ahead they would be decoding the major/minor number before it hits disk, and already storing it in 12:20 fashion or somesuch.
Unless I am missing something...
Jeff
Signature · 3 lines
Jeff Garzik | Only so many songs can be sung Building 1024 | with two lips, two lungs, and one tongue. MandrakeSoft | - nomeansno
Quoted 2 lines · Jeff Garzik
reiserfs is blindly storing the kernel's
kdev_tvalue raw to disk.
Well, it won't do that. You have to use "kdev_t_to_nr()", which (whenever the format of kdev_t changes) will still be identical in the low 16 bits.
Now, if somebody actually has the raw "kdev_t" in their on-disk structures, that's a real problem, but I don't think anybody does. Certainly I didn't see reiserfs do it (but it may well be missing a few "kdev_t_to_nr()" calls)
Linus
Quoted 5 lines · Linus Torvalds
Quoted 2 lines · Jeff Garzik
reiserfs is blindly storing the kernel's
kdev_tvalue raw to disk.
Quoted 7 lines
Well, it won't do that. You have to use "
kdev_t_to_nr()", which (whenever the format ofkdev_tchanges) will still be identical in the low 16 bits.Now, if somebody actually has the raw "
kdev_t" in their on-disk structures, that's a real problem, but I don't think anybody does. Certainly I didn't see reiserfs do it (but it may well be missing a few "kdev_t_to_nr()" calls)
AFAICS it does:
include/linux/reiserfs.h:
#define sd_v1_rdev(sdp) (le32_to_cpu((sdp)->u.sd_rdev))
#define set_sd_v1_rdev(sdp,v) ((sdp)->u.sd_rdev = cpu_to_le32(v))
[jgarzik@rum reiserfs]$ grep v1_rdev *.c
inode.c: rdev = sd_v1_rdev(sd); inode.c: set_sd_v1_rdev(sd_v1, inode->i_rdev );
In the first inode.c line shown here, it passes the value received directly to init_special_inode.
Jeff
Signature · 3 lines
Jeff Garzik | Only so many songs can be sung Building 1024 | with two lips, two lungs, and one tongue. MandrakeSoft | - nomeansno
Quoted 11 lines · Jeff Garzik
Quoted 5 lines
Now, if somebody actually has the raw "
kdev_t" in their on-disk structures, that's a real problem, but I don't think anybody does. Certainly I didn't see reiserfs do it (but it may well be missing a few "kdev_t_to_nr()" calls)AFAICS it does:
include/linux/reiserfs.h:
#definesd_v1_rdev(sdp)(le32_to_cpu((sdp)->u.sd_rdev))
#defineset_sd_v1_rdev(sdp,v)((sdp)->u.sd_rdev =cpu_to_le32(v))
Ok, just add the proper conversion functions, ie a "to_kdev_t()" and "kdev_t_to_nr()".
Linus
Quoted 8 lines · Linus Torvalds
Quoted 6 lines · Jeff Garzik
Quoted 5 lines
Now, if somebody actually has the raw "
kdev_t" in their on-disk structures, that's a real problem, but I don't think anybody does. Certainly I didn't see reiserfs do it (but it may well be missing a few "kdev_t_to_nr()" calls)
Quoted 8 lines
Quoted 5 lines
AFAICS it does:
include/linux/reiserfs.h:
#definesd_v1_rdev(sdp)(le32_to_cpu((sdp)->u.sd_rdev))
#defineset_sd_v1_rdev(sdp,v)((sdp)->u.sd_rdev =cpu_to_le32(v))Ok, just add the proper conversion functions, ie a "
to_kdev_t()" and "kdev_t_to_nr()".
Actually, result of sd_v1_rdev() is only passed to init_special_inode() which takes int rather than kdev_t.
Isn't this a bit strange, because file system backend has to convert kdev_t to u32 on write, but not on read?
Quoted 2 lines
Linus
Nikita.
Quoted 1 line
Quoted 16 lines · Nikita Danilov
Quoted 14 lines · Linus Torvalds
Quoted 11 lines · Jeff Garzik
Quoted 5 lines
Now, if somebody actually has the raw "
kdev_t" in their on-disk structures, that's a real problem, but I don't think anybody does. Certainly I didn't see reiserfs do it (but it may well be missing a few "kdev_t_to_nr()" calls)AFAICS it does:
include/linux/reiserfs.h:
#definesd_v1_rdev(sdp)(le32_to_cpu((sdp)->u.sd_rdev))
#defineset_sd_v1_rdev(sdp,v)((sdp)->u.sd_rdev =cpu_to_le32(v))
Quoted 8 lines
Quoted 2 lines
Ok, just add the proper conversion functions, ie a "
to_kdev_t()" and "kdev_t_to_nr()".Actually, result of
sd_v1_rdev()is only passed toinit_special_inode()which takes int rather thankdev_t.Isn't this a bit strange, because file system backend has to convert
kdev_tto u32 on write, but not on read?
As mentioned to viro on IRC, I think init_special_inode should take major and minor arguments, to nudge the filesystem implementors into thinking that major and minor should be treated separately, and be given additional thought as to how they are encoded on-disk.
(I suggested having init_special_inode taking a kdev_t argument as its third arg, but viro yelled at me :))
Jeff
Signature · 3 lines
Jeff Garzik | Only so many songs can be sung Building 1024 | with two lips, two lungs, and one tongue. MandrakeSoft | - nomeansno
Quoted 5 lines · Jeff Garzik
As mentioned to viro on IRC, I think
init_special_inodeshould take major and minor arguments, to nudge the filesystem implementors into thinking that major and minor should be treated separately, and be given additional thought as to how they are encoded on-disk.
Yes. If somebody sends me a patch, I'll apply it in a jiffy.
Linus
Quoted 9 lines · Linus Torvalds
Quoted 5 lines · Jeff Garzik
As mentioned to viro on IRC, I think
init_special_inodeshould take major and minor arguments, to nudge the filesystem implementors into thinking that major and minor should be treated separately, and be given additional thought as to how they are encoded on-disk.Yes. If somebody sends me a patch, I'll apply it in a jiffy.
Guys, wait a minute with that. There is a related issue (->i_rdev becoming dev_t) and I'd rather see it handled first.