Skip to content

Commit

Permalink
[PATCH] holepunch: fix shmem_truncate_range punching too far
Browse files Browse the repository at this point in the history
Miklos Szeredi observes BUG_ON(!entry) in shmem_writepage() triggered in rare
circumstances, because shmem_truncate_range() erroneously removes partially
truncated directory pages at the end of the range: later reclaim on pages
pointing to these removed directories triggers the BUG.  Indeed, and it can
also cause data loss beyond the hole.

Fix this as in the patch proposed by Miklos, but distinguish between "limit"
(how far we need to search: ignore truncation's next_index optimization in the
holepunch case - if there are races it's more consistent to act on the whole
range specified) and "upper_limit" (how far we can free directory pages:
generally we must be careful to keep partially punched pages, but can relax at
end of file - i_size being held stable by i_mutex).

Signed-off-by: Hugh Dickins <[email protected]>
Cc: Miklos Szeredi <[email protected]>
Cc: Badari Pulavarty <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
Hugh Dickins authored and Linus Torvalds committed Mar 29, 2007
1 parent 96fac9d commit a2646d1
Showing 1 changed file with 21 additions and 11 deletions.
32 changes: 21 additions & 11 deletions mm/shmem.c
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,8 @@ static void shmem_truncate_range(struct inode *inode, loff_t start, loff_t end)
long nr_swaps_freed = 0;
int offset;
int freed;
int punch_hole = 0;
int punch_hole;
unsigned long upper_limit;

inode->i_ctime = inode->i_mtime = CURRENT_TIME;
idx = (start + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;
Expand All @@ -492,11 +493,18 @@ static void shmem_truncate_range(struct inode *inode, loff_t start, loff_t end)
info->flags |= SHMEM_TRUNCATE;
if (likely(end == (loff_t) -1)) {
limit = info->next_index;
upper_limit = SHMEM_MAX_INDEX;
info->next_index = idx;
punch_hole = 0;
} else {
limit = (end + PAGE_CACHE_SIZE - 1) >> PAGE_CACHE_SHIFT;
if (limit > info->next_index)
limit = info->next_index;
if (end + 1 >= inode->i_size) { /* we may free a little more */
limit = (inode->i_size + PAGE_CACHE_SIZE - 1) >>
PAGE_CACHE_SHIFT;
upper_limit = SHMEM_MAX_INDEX;
} else {
limit = (end + 1) >> PAGE_CACHE_SHIFT;
upper_limit = limit;
}
punch_hole = 1;
}

Expand All @@ -520,10 +528,10 @@ static void shmem_truncate_range(struct inode *inode, loff_t start, loff_t end)
* If there are no indirect blocks or we are punching a hole
* below indirect blocks, nothing to be done.
*/
if (!topdir || (punch_hole && (limit <= SHMEM_NR_DIRECT)))
if (!topdir || limit <= SHMEM_NR_DIRECT)
goto done2;

BUG_ON(limit <= SHMEM_NR_DIRECT);
upper_limit -= SHMEM_NR_DIRECT;
limit -= SHMEM_NR_DIRECT;
idx = (idx > SHMEM_NR_DIRECT)? (idx - SHMEM_NR_DIRECT): 0;
offset = idx % ENTRIES_PER_PAGE;
Expand All @@ -543,7 +551,7 @@ static void shmem_truncate_range(struct inode *inode, loff_t start, loff_t end)
if (*dir) {
diroff = ((idx - ENTRIES_PER_PAGEPAGE/2) %
ENTRIES_PER_PAGEPAGE) / ENTRIES_PER_PAGE;
if (!diroff && !offset) {
if (!diroff && !offset && upper_limit >= stage) {
*dir = NULL;
nr_pages_to_free++;
list_add(&middir->lru, &pages_to_free);
Expand All @@ -570,9 +578,11 @@ static void shmem_truncate_range(struct inode *inode, loff_t start, loff_t end)
}
stage = idx + ENTRIES_PER_PAGEPAGE;
middir = *dir;
*dir = NULL;
nr_pages_to_free++;
list_add(&middir->lru, &pages_to_free);
if (upper_limit >= stage) {
*dir = NULL;
nr_pages_to_free++;
list_add(&middir->lru, &pages_to_free);
}
shmem_dir_unmap(dir);
cond_resched();
dir = shmem_dir_map(middir);
Expand All @@ -598,7 +608,7 @@ static void shmem_truncate_range(struct inode *inode, loff_t start, loff_t end)
}
if (offset)
offset = 0;
else if (subdir && !page_private(subdir)) {
else if (subdir && upper_limit - idx >= ENTRIES_PER_PAGE) {
dir[diroff] = NULL;
nr_pages_to_free++;
list_add(&subdir->lru, &pages_to_free);
Expand Down

0 comments on commit a2646d1

Please sign in to comment.