Bug 9365

Summary: BFS: When overwriting an existing file with nonzero size, the filesystem driver allocates a new block thus leaking blocks.
Product: File System Reporter: Dmitri Vorobiev (dmitri.vorobiev)
Component: OtherAssignee: Dmitri Vorobiev (dmitri.vorobiev)
Status: RESOLVED CODE_FIX    
Severity: high CC: dmitri.vorobiev
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 2.6.24-rc2 Subsystem:
Regression: --- Bisected commit-id:

Description Dmitri Vorobiev 2007-11-13 06:58:45 UTC 
Most recent kernel where this bug did not occur: N/A
Distribution: this bug does not depend on distribution.
Hardware Environment: x86
Software Environment:

>>>

# lsmod
Module                  Size  Used by
bfs                    14008  1
#

<<<

Problem Description:

Blocks are leaking when an existing file is extended.

Steps to reproduce:

1. Mount an empty BFS partition. The examples below assume that the partition is mounted to `/mnt'.

2. Create two regular files, the first file occupying two BFS blocks, while the second file taking the rest of space on the BFS partition minus one block:

>>>

root@codedot:/mnt# mount | grep mnt
/dev/loop0 on /mnt type bfs (rw)
root@codedot:/mnt# df -B 512 | grep loop
/dev/loop0               65536        81     65455   1% /mnt
root@codedot:/mnt# dd if=/dev/urandom of=1 bs=512 count=2
2+0 records in
2+0 records out
1024 bytes (1.0 kB) copied, 0.00986342 seconds, 104 kB/s
root@codedot:/mnt# df -B 512 | grep loop
/dev/loop0               65536        83     65453   1% /mnt
root@codedot:/mnt# dd if=/dev/urandom of=2 bs=512 count=65452
65452+0 records in
65452+0 records out
33511424 bytes (34 MB) copied, 56.66 seconds, 591 kB/s
root@codedot:/mnt# df -B 512 | grep loop
/dev/loop0               65536     65535         1 100% /mnt
root@codedot:/mnt# 

<<<

3. Now try to write a few bytes to the beginning of the first file and see the last free block disappearing:

>>>

root@codedot:/mnt# echo "hello" > 1
root@codedot:/mnt# df -B 512 | grep loop
/dev/loop0               65536     65536         0 100% /mnt
root@codedot:/mnt# 

<<<
Comment 1 Dmitri Vorobiev 2007-11-13 06:59:54 UTC 
The bfs_get_block() routine needs to use the `s_block' field of the inode to determine if the file alreasy exists. Currently, this function makes use of the `i_size' field of the in-core inode structure and assumes that the file does not exist if this field is zero. This assumption is incorrect, because when an existing file gets overwritten, this field can still be equal zero.

I am working on a fix to this problem, so I am assigning this bug to myself.
Comment 2 Dmitri Vorobiev 2007-11-13 08:47:46 UTC 
The patch proposed in the following LKML message

http://lkml.org/lkml/2007/11/13/185

fixes the error reported in the context of this bug. What follows is a console session, which proves that the error is gone:

>>>

debian:~# mount -t bfs /dev/loop0 /mnt
debian:~# cd /mnt/
debian:/mnt# df -B 512 | grep loop
/dev/loop0               65536        81     65455   1% /mnt
debian:/mnt# dd if=/dev/urandom of=1 bs=512 count=2
2+0 records in
2+0 records out
1024 bytes (1.0 kB) copied, 0.0116998 seconds, 87.5 kB/s
debian:/mnt# df -B 512 | grep loop
/dev/loop0               65536        83     65453   1% /mnt
debian:/mnt# dd if=/dev/urandom of=2 bs=512 count=65452
65452+0 records in
65452+0 records out
33511424 bytes (34 MB) copied, 115.517 seconds, 290 kB/s
debian:/mnt# echo "hello" > 1
debian:/mnt# df -B 512 | grep loop
/dev/loop0               65536     65535         1 100% /mnt
debian:/mnt#

<<<
Comment 3 Andrew Morton 2007-11-13 11:54:22 UTC 
I merged Dmitry's fix.

It wasn't necessary to create all these bugzilla reports, really.  Simply
emailing the patch is sufficient.  We're not very formal.