Re: [PATCH] one-liner fix for bforget() honoring BH_Protected; was: Re: Patch (repost): cramfs memory corruption fix

Re: [PATCH] one-liner fix for bforget() honoring BH_Protected; was: Re: Patch (repost): cramfs memory corruption fix

[Adam J. Richter]

>From: "David L. Parsley" <parsley@linuxjedi.org>

>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
adam@yggdrasil.com     \ /                  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 majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/

Re: Patch (repost): cramfs memory corruption fix

[Linus Torvalds]

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 majordomo@vger.kernel.org
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

[David L. Parsley]

[-- Attachment #1: Type: text/plain, Size: 986 bytes --]

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

[-- Attachment #2: bforgetfix.diff --]
[-- Type: text/plain, Size: 684 bytes --]

--- 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] one-liner fix for bforget() honoring BH_Protected; was: Re: Patch (repost): cramfs memory corruption fix

[Linus Torvalds]

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 majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/