Re: [PATCH] one-liner fix for bforget() honoring BH_Protected; was:Re: Patch (repost): cramfs memory corruption fix
On Wed, 10 Jan 2001, David L. Parsley wrote: > > Yup, I backed out Adam's one-liner in favor of the attached one-liner. > Tested on 2.4.0, but should patch cleanly to just about anything. ;-) > > BTW Linus - you were of course right on the cramfs wanting 4096 > blocksize... but without this fix, that doesn't matter much. ;-) So everything works the way you want it now? Just checking that there aren't any outstanding issues.. Linus - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] one-liner fix for bforget() honoring BH_Protected; was: Re: Patch (repost): cramfs memory corruption fix
>From: "David L. Parsley" <[EMAIL PROTECTED]> >Linus Torvalds wrote: >> On Sat, 6 Jan 2001, Adam J. Richter wrote: >> > >> > This sounds like a bug that I posted a fix for a long time ago. >> > cramfs calls bforget on the superblock area, destroying that block of >> > the ramdisk, even when the ramdisk does not contain a cramfs file system. >> > Normally, bforget is called on block that really can be trashed, >> > such as blocks release by truncate or unlink. >> >> I'd really prefer just not letting bforget() touch BH_Protected buffers. >> bforget() is also used by other things than unlink/truncate: it's used by >> various partition codes etc, and it's used by the raid logic. >Yup, I backed out Adam's one-liner in favor of the attached one-liner. >Tested on 2.4.0, but should patch cleanly to just about anything. ;-) Applying Linus's patch is fine, but I think my patch should also be applied (in addition), although for a less important reason. The bforget in cramfs is going to force unnecessary device IO if cramfs is in the list of file systems that you are trying to detect when mounting a file system from a physical device. Adam J. Richter __ __ 4880 Stevens Creek Blvd, Suite 104 [EMAIL PROTECTED] \ / San Jose, California 95129-1034 +1 408 261-6630 | g g d r a s i l United States of America fax +1 408 261-6631 "Free Software For The Rest Of Us." - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
[PATCH] one-liner fix for bforget() honoring BH_Protected; was: Re: Patch (repost): cramfs memory corruption fix
Linus Torvalds wrote:
>
> On Sat, 6 Jan 2001, Adam J. Richter wrote:
> >
> > This sounds like a bug that I posted a fix for a long time ago.
> > cramfs calls bforget on the superblock area, destroying that block of
> > the ramdisk, even when the ramdisk does not contain a cramfs file system.
> > Normally, bforget is called on block that really can be trashed,
> > such as blocks release by truncate or unlink.
>
> I'd really prefer just not letting bforget() touch BH_Protected buffers.
> bforget() is also used by other things than unlink/truncate: it's used by
> various partition codes etc, and it's used by the raid logic.
Yup, I backed out Adam's one-liner in favor of the attached one-liner.
Tested on 2.4.0, but should patch cleanly to just about anything. ;-)
BTW Linus - you were of course right on the cramfs wanting 4096
blocksize... but without this fix, that doesn't matter much. ;-)
regards,
David
--
David L. Parsley
Network Administrator
Roanoke College
--- linux.linus/fs/buffer.c Wed Jan 3 23:45:26 2001
+++ linux/fs/buffer.c Wed Jan 10 15:49:36 2001
@@ -1145,13 +1145,15 @@
* free list if it can.. We can NOT free the buffer if:
* - there are other users of it
* - it is locked and thus can have active IO
+ * - it is marked BH_Protected
*/
void __bforget(struct buffer_head * buf)
{
/* grab the lru lock here to block bdflush. */
spin_lock(&lru_list_lock);
write_lock(&hash_table_lock);
- if (!atomic_dec_and_test(&buf->b_count) || buffer_locked(buf))
+ if (!atomic_dec_and_test(&buf->b_count) || buffer_locked(buf) ||
+ buffer_protected(buf))
goto in_use;
__hash_unlink(buf);
remove_inode_queue(buf);
Re: Patch (repost): cramfs memory corruption fix
On Sun, 7 Jan 2001, David L. Parsley wrote: > > 2.4.0 ramfs with the one-liner does it's job for me already; what I'd > really love to fool with is _cramfs_. ;-) In case you missed the > beginning of this thread: all my cramfs initrd's fail to mount as > /dev/ram0 with 'wrong magic'; their romfs counterparts work fine. I did > manage to pivot_root into a cramfs root, but it blew up pretty quick > with 'attempt to access beyond end of device', and killed my init > shell. Then there's the wierdness where cramfs compiled in the kernel > corrupts the romfs. Adam's one-liner (bforget -> brelse on superblock) > didn't fix this. Uhhuh. I've never used cramfs on a ramdisk, but I have a guess: cramfs _really_ wants to have 4kB blocks. I bet your ramdisk has 1kB blocks (the default). Have you tried using ramdisk_blocksize=4096 as a boot option? > The cramfs docs are contradictory btw; the kernel help says 'doesn't > support hardlinks', and Documentation/filesystems/cramfs.txt says > 'Hardlinks are supported, but...'. I made my cramfs with and without > hardlinks (to busybox); but that didn't affect whether it mounted. I > haven't tested whether this fixes the 'access beyond end of device' > problems. The documentation is right. cramfs does not support hardlinks. HOWEVER, cramfs gives you the _effect_ of hardlinks by having mkcramfs notice that two files are the same (whether hardlinked or not), and re-using the data block pointer. This gives you all the space savings of hard links, with a twist: you can actually have two separate files, with separate permissions and owners etc, and they will be "linked" in the data linking meaning if the contents are the same. So you could think of the cramfs links as being a superset of hardlinks. Or a "corruption" of hardlinks. The file shows up as two different files as far as unix tools are concerned (nlink == 1, different inode numbers etc), but becasue of the data sharing they have the disk usage pattern of a hardlink. The reason you don't have (and cannot have) _true_ hardlinks is obvious: ramfs does not have inodes. It only has directory entries with enough information to fill in a linux inode structure. Kind of like FAT, in that respect. This, btw, also means that it's counterproductive to try to save space on the source tree that is crammed by trying to find identical files and hardlinking them - mkcramfs will do the same thing regardless. Linus - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: Patch (repost): cramfs memory corruption fix
Hi Alan, On Mon, 8 Jan 2001, Alan Cox wrote: > I have been thinking about this. I think we should merge the size > limiting code with the example clean ramfs code. Having spent a > while debugging the LFS checks and some other funnies I realised one > problem with the ramfs in 2.4.0 as an example. It does not > demonstrate error cases, which the new one does. For demonstration purposes perhaps. But I do not see a lot of value of using ramfs if shmem could do read and write. Greetings Christoph - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: Patch (repost): cramfs memory corruption fix
Hi Christoph, On Mon, 8 Jan 2001, Christoph Hellwig wrote: > I had a prototype tmpfs in -test10 (ro so) times. It based on ramfs > for all the metadata stuff and used the (old) shmfs code for > swap-backed data. The only real problem the code had, was that it > needed a ->allocpage address_space method in place of > page_cache_alloc() to directly swap-in pages in ->read. IF anyone > is interested I could forward port it to 2.4.0 and the new shmfs. Be aware that there is nothing left from the old shm memory/swap handling in 2.4.0. It is a copy of ramfs plus additions for swap and resource limits If you would be willing to add the read and write functions to the new coding I would be very happy. Greetings Christoph - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: Patch (repost): cramfs memory corruption fix
> On Sun, 7 Jan 2001, Linus Torvalds wrote: > > I wonder what to do about this - the limits are obviously useful, as > > would the "use swap-space as a backing store" thing be. At the same > > time I'd really hate to lose the lean-mean-clean ramfs. > > Let me repeat on this issue: shmem.c has everything needed for this > despite read and write and they should be really easy to add. > > I did not plan to write them in the near future because I did not > think that this is a really wanted feature. But I can look into it. I have been thinking about this. I think we should merge the size limiting code with the example clean ramfs code. Having spent a while debugging the LFS checks and some other funnies I realised one problem with the ramfs in 2.4.0 as an example. It does not demonstrate error cases, which the new one does. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: Patch (repost): cramfs memory corruption fix
[EMAIL PROTECTED] said: > While the topic is raised..., I've hacked up cramfs for linear > addressing to kill the "double buffering" effiect. However as David > mentions the block device support thing is an issue here. What is a > reasonable way to allow a cramfs partition to access the device > directly, like the patch that I wrote, and be picked up in a > reasonable way by the init system? So far, I just cheat. JFFS doesn't need a block device, but uses the minor# to determine which MTD device to use. You could use the same trick. -- dwmw2 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: Patch (repost): cramfs memory corruption fix
In article <[EMAIL PROTECTED]> you wrote: > Hi Linus, > > On Sun, 7 Jan 2001, Linus Torvalds wrote: >> I wonder what to do about this - the limits are obviously useful, as >> would the "use swap-space as a backing store" thing be. At the same >> time I'd really hate to lose the lean-mean-clean ramfs. > > Let me repeat on this issue: shmem.c has everything needed for this > despite read and write and they should be really easy to add. > > I did not plan to write them in the near future because I did not > think that this is a really wanted feature. But I can look into it. > I had a prototype tmpfs in -test10 (ro so) times. It based on ramfs for all the metadata stuff and used the (old) shmfs code for swap-backed data. The only real problem the code had, was that it needed a ->allocpage address_space method in place of page_cache_alloc() to directly swap-in pages in ->read. IF anyone is interested I could forward port it to 2.4.0 and the new shmfs. Christoph -- Whip me. Beat me. Make me maintain AIX. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: Patch (repost): cramfs memory corruption fix
On Monday 08 January 2001 13:11, David Woodhouse wrote: > [EMAIL PROTECTED] said: > > Also, if you care about memory usage, you're likely to be much better > > off using ramfs rather than something like "ext2 on ramdisk". You > > won't get the double buffering. > > That'll be even more useful once we can completely configure out all > support for block devices too. > While the topic is raised..., I've hacked up cramfs for linear addressing to kill the "double buffering" effiect. However as David mentions the block device support thing is an issue here. What is a reasonable way to allow a cramfs partition to access the device directly, like the patch that I wrote, and be picked up in a reasonable way by the init system? (Current system does some non-cool things like find out it's cramfs from /dev/rom , and within cramfs never accesses that block device anymore..., it's sort of silly, and _not_ the right way to do it.) Thanks, Shane Nay. (Patches referenced here can be found at: ftp://ftp.agendacomputing.com/pub/agenda/testing/CES/patches/ , contributed by various authors: Rob Leslie, Brad Laronde, and myself. If you want the mkcramfs with xip, just ask.) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: Patch (repost): cramfs memory corruption fix
Hi Linus, On Sun, 7 Jan 2001, Linus Torvalds wrote: > I wonder what to do about this - the limits are obviously useful, as > would the "use swap-space as a backing store" thing be. At the same > time I'd really hate to lose the lean-mean-clean ramfs. Let me repeat on this issue: shmem.c has everything needed for this despite read and write and they should be really easy to add. I did not plan to write them in the near future because I did not think that this is a really wanted feature. But I can look into it. Greetings Christoph - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: Patch (repost): cramfs memory corruption fix
[EMAIL PROTECTED] said: > Also, if you care about memory usage, you're likely to be much better > off using ramfs rather than something like "ext2 on ramdisk". You > won't get the double buffering. That'll be even more useful once we can completely configure out all support for block devices too. -- dwmw2 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: Patch (repost): cramfs memory corruption fix
Rik van Riel <[EMAIL PROTECTED]> writes: > On Sun, 7 Jan 2001, Linus Torvalds wrote: > > On Sun, 7 Jan 2001, Alan Cox wrote: > > > > > -ac has the rather extended ramfs with resource limits and stuff. That one > > > also has rather more extended bugs 8). AFAIK none of those are in the > vanilla > > > > ramfs code > > > This is actually where I agree with whoever it was that said that ramfs as > > it stands now (without the limit checking etc) is much nicer simply > > because it can act as an example of how to do a simple filesystem. > > > > I wonder what to do about this - the limits are obviously useful, as would > > the "use swap-space as a backing store" thing be. At the same time I'd > > really hate to lose the lean-mean-clean ramfs. > > Sounds like a job for ... ... tmpfs!! If you need tmpfs the VFS layer is broken. For 99% of everything performance is determined by VFS layer caching. A fs that uses swap space as a backing store is not a big win. You just have a fs that doesn't support sync and you can add a mount option to a normal fs if you want that. I've written the filesystem and it was a dumb idea. Ramfs with (maybe) some basic limits has a place. tmpfs is just extra code to maintain. Eric - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: Patch (repost): cramfs memory corruption fix
On Sat, 6 Jan 2001, Adam J. Richter wrote: > > This sounds like a bug that I posted a fix for a long time ago. > cramfs calls bforget on the superblock area, destroying that block of > the ramdisk, even when the ramdisk does not contain a cramfs file system. > Normally, bforget is called on block that really can be trashed, > such as blocks release by truncate or unlink. I'd really prefer just not letting bforget() touch BH_Protected buffers. bforget() is also used by other things than unlink/truncate: it's used by various partition codes etc, and it's used by the raid logic. Now, nobody wants to use RAID on a ramdisk, but the fact is that "bforget()" does not mean "forget the contents of this buffer", but it really means "you can forget this cached copy even if it is dirty, we're not likely to need it in the near future and can read it back in". Also, if you care about memory usage, you're likely to be much better off using ramfs rather than something like "ext2 on ramdisk". You won't get the double buffering. Linus - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: Patch (repost): cramfs memory corruption fix
> Sounds like a job for ... ... tmpfs!! > > (and yes, I share your opinion that ramfs is nice _because_ > it's an easy example for filesystem code teaching) The resource tracking ramfs isnt that much uglier to be honest. One that went off using backing store would be, but ramfs with limits simply ensures that dd if=/dev/zero of=/mnt/ram/foo doesnt crash your box - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: Patch (repost): cramfs memory corruption fix
On Sun, 7 Jan 2001, Linus Torvalds wrote: > On Sun, 7 Jan 2001, Alan Cox wrote: > > > -ac has the rather extended ramfs with resource limits and stuff. That one > > also has rather more extended bugs 8). AFAIK none of those are in the vanilla > > ramfs code > This is actually where I agree with whoever it was that said that ramfs as > it stands now (without the limit checking etc) is much nicer simply > because it can act as an example of how to do a simple filesystem. > > I wonder what to do about this - the limits are obviously useful, as would > the "use swap-space as a backing store" thing be. At the same time I'd > really hate to lose the lean-mean-clean ramfs. Sounds like a job for ... ... tmpfs!! (and yes, I share your opinion that ramfs is nice _because_ it's an easy example for filesystem code teaching) cheers, Rik -- Virtual memory is like a game you can't win; However, without VM there's truly nothing to lose... http://www.surriel.com/ http://www.conectiva.com/ http://distro.conectiva.com.br/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: Patch (repost): cramfs memory corruption fix
Alan Cox wrote: > -ac has the rather extended ramfs with resource limits and stuff. That one > also has rather more extended bugs 8). AFAIK none of those are in the vanilla > ramfs code Nifty stuff, too; it's nice for a ramfs mount to show up in 'df' with useful figures. Shame I can't put anything there. ;-) 2.4.0 ramfs with the one-liner does it's job for me already; what I'd really love to fool with is _cramfs_. ;-) In case you missed the beginning of this thread: all my cramfs initrd's fail to mount as /dev/ram0 with 'wrong magic'; their romfs counterparts work fine. I did manage to pivot_root into a cramfs root, but it blew up pretty quick with 'attempt to access beyond end of device', and killed my init shell. Then there's the wierdness where cramfs compiled in the kernel corrupts the romfs. Adam's one-liner (bforget -> brelse on superblock) didn't fix this. The cramfs docs are contradictory btw; the kernel help says 'doesn't support hardlinks', and Documentation/filesystems/cramfs.txt says 'Hardlinks are supported, but...'. I made my cramfs with and without hardlinks (to busybox); but that didn't affect whether it mounted. I haven't tested whether this fixes the 'access beyond end of device' problems. regards, David -- David L. Parsley Network Administrator Roanoke College - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: Patch (repost): cramfs memory corruption fix
On Sun, 7 Jan 2001, Alan Cox wrote: > > > I'll take a look at the ramfs one. I may have broken something else when fixing > > > everything else with ramfs (like unlink) crashing > > > > Ehh.. Plain 2.4.0 ramfs is fine, assuming you add a "UnlockPage()" to > > ramfs_writepage(). So what do you mean by "fixing everything else"? > > -ac has the rather extended ramfs with resource limits and stuff. That one > also has rather more extended bugs 8). AFAIK none of those are in the vanilla > ramfs code Ahh, ok. This is actually where I agree with whoever it was that said that ramfs as it stands now (without the limit checking etc) is much nicer simply because it can act as an example of how to do a simple filesystem. I wonder what to do about this - the limits are obviously useful, as would the "use swap-space as a backing store" thing be. At the same time I'd really hate to lose the lean-mean-clean ramfs. Anyway, I'm quite sure that the resource stuff would not be 2.4.1 material (adding the UnlockPage() is), so I won't have to worry about this issue for a while. Linus - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: Patch (repost): cramfs memory corruption fix
> > I'll take a look at the ramfs one. I may have broken something else when fixing > > everything else with ramfs (like unlink) crashing > > Ehh.. Plain 2.4.0 ramfs is fine, assuming you add a "UnlockPage()" to > ramfs_writepage(). So what do you mean by "fixing everything else"? -ac has the rather extended ramfs with resource limits and stuff. That one also has rather more extended bugs 8). AFAIK none of those are in the vanilla ramfs code - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: Patch (repost): cramfs memory corruption fix
On Sun, 7 Jan 2001, Alan Cox wrote: > > >ramfs croaks with 'kernel BUG in filemap.c line 2559' anytime I make a > > >file in ac2 and ac3. Works fine in 2.4.0 vanilla. Should be quite > > >repeatable... > > I'll take a look at the ramfs one. I may have broken something else when fixing > everything else with ramfs (like unlink) crashing Ehh.. Plain 2.4.0 ramfs is fine, assuming you add a "UnlockPage()" to ramfs_writepage(). So what do you mean by "fixing everything else"? Linus - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Re: Patch (repost): cramfs memory corruption fix
> >ramfs croaks with 'kernel BUG in filemap.c line 2559' anytime I make a > >file in ac2 and ac3. Works fine in 2.4.0 vanilla. Should be quite > >repeatable... I'll take a look at the ramfs one. I may have broken something else when fixing everything else with ramfs (like unlink) crashing > This sounds like a bug that I posted a fix for a long time ago. > cramfs calls bforget on the superblock area, destroying that block of > the ramdisk, even when the ramdisk does not contain a cramfs file system. > Normally, bforget is called on block that really can be trashed, > such as blocks release by truncate or unlink. If it worked for > you before, you were just getting lucky. Here is the patch. > > Linus, please consider applying this. Thank you. This isnt the fix. If -ac also fails well it contains this cramfs fix. So there must be other problems Alan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] Please read the FAQ at http://www.tux.org/lkml/
Patch (repost): cramfs memory corruption fix
>From: "David L. Parsley" <[EMAIL PROTECTED]>
>
>Using root=/dev/ram0 and a cramfs initrd gives me 'wrong magic' when it
>tries to boot. Even more bizarre, if cramfs is compiled in the kernel
>when I use a romfs root, it says 'wrong magic' then mounts the romfs but
>can't find init. If I take cramfs out of the kernel, the romfs mounts &
>init runs fine. I just saw this with ac3.
>
>ramfs croaks with 'kernel BUG in filemap.c line 2559' anytime I make a
>file in ac2 and ac3. Works fine in 2.4.0 vanilla. Should be quite
>repeatable...
This sounds like a bug that I posted a fix for a long time ago.
cramfs calls bforget on the superblock area, destroying that block of
the ramdisk, even when the ramdisk does not contain a cramfs file system.
Normally, bforget is called on block that really can be trashed,
such as blocks release by truncate or unlink. If it worked for
you before, you were just getting lucky. Here is the patch.
Linus, please consider applying this. Thank you.
By the way, the other approach to fixing this problem would
be to change bforget not to trash blocks marked with BH_Protected
(I think that is just ramdisk blocks), but that would waste memory,
because we really can release blocks from things like truncating
or unlinking files.
--
Adam J. Richter __ __ 4880 Stevens Creek Blvd, Suite 104
[EMAIL PROTECTED] \ / San Jose, California 95129-1034
+1 408 261-6630 | g g d r a s i l United States of America
fax +1 408 261-6631 "Free Software For The Rest Of Us."
--- /tmp/adam/linux-2.4.0/fs/cramfs/inode.c Fri Dec 29 14:07:57 2000
+++ linux/fs/cramfs/inode.c Sat Dec 30 02:12:06 2000
@@ -138,7 +138,7 @@
struct buffer_head * bh = bh_array[i];
if (bh) {
memcpy(data, bh->b_data, PAGE_CACHE_SIZE);
- bforget(bh);
+ brelse(bh);
} else
memset(data, 0, PAGE_CACHE_SIZE);
data += PAGE_CACHE_SIZE;
