All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] hpfs: fix warnings when the filesystem fills up
@ 2013-06-08 23:25 Mikulas Patocka
  2013-07-04 16:42 ` [PATCH] hpfs: better test for errors Mikulas Patocka
  0 siblings, 1 reply; 5+ messages in thread
From: Mikulas Patocka @ 2013-06-08 23:25 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-fsdevel

hpfs: fix warnings when the filesystem fills up

This patch fixes warnings due to missing lock on write error path.

[11624.793312] ------------[ cut here ]------------
[11624.795305] WARNING: at fs/hpfs/hpfs_fn.h:353 hpfs_truncate+0x75/0x80 [hpfs]()
[11624.797483] Hardware name: empty
[11624.797547] Pid: 26563, comm: dd Tainted: P           O 3.9.4 #12
[11624.797548] Call Trace:
[11624.797556]  [<ffffffff81035ed9>] ? warn_slowpath_common+0x79/0xc0
[11624.797559]  [<ffffffffa0033775>] ? hpfs_truncate+0x75/0x80 [hpfs]
[11624.797561]  [<ffffffffa0033804>] ? hpfs_write_begin+0x84/0x90 [hpfs]
[11624.797563]  [<ffffffffa0033490>] ? _hpfs_bmap+0x10/0x10 [hpfs]
[11624.797568]  [<ffffffff810c0771>] ? generic_file_buffered_write+0x121/0x2c0
[11624.797570]  [<ffffffff810c18e7>] ? __generic_file_aio_write+0x1c7/0x3f0
[11624.797572]  [<ffffffff810c1b8c>] ? generic_file_aio_write+0x7c/0x100
[11624.797576]  [<ffffffff81115f38>] ? do_sync_write+0x98/0xd0
[11624.797579]  [<ffffffffa00336bd>] ? hpfs_file_write+0xd/0x50 [hpfs]
[11624.797581]  [<ffffffff81116522>] ? vfs_write+0xa2/0x160
[11624.797582]  [<ffffffff81116801>] ? sys_write+0x51/0xa0
[11624.797586]  [<ffffffff813515e2>] ? page_fault+0x22/0x30
[11624.797588]  [<ffffffff81351b16>] ? system_call_fastpath+0x1a/0x1f
[11624.797589] ---[ end trace 073c81ffe88d3716 ]---

This should be backported to stable kernels 2.6.39 and newer.

Signed-off-by: Mikulas Patocka <mikulas@artax.karlin.mff.cuni.cz>
Cc: stable@kernel.org

---
 fs/hpfs/file.c |    4 ++++
 1 file changed, 4 insertions(+)

Index: linux-3.9.4-fast/fs/hpfs/file.c
===================================================================
--- linux-3.9.4-fast.orig/fs/hpfs/file.c	2013-06-07 03:01:45.000000000 +0200
+++ linux-3.9.4-fast/fs/hpfs/file.c	2013-06-07 03:02:44.000000000 +0200
@@ -109,10 +109,14 @@ static void hpfs_write_failed(struct add
 {
 	struct inode *inode = mapping->host;
 
+	hpfs_lock(inode->i_sb);
+
 	if (to > inode->i_size) {
 		truncate_pagecache(inode, to, inode->i_size);
 		hpfs_truncate(inode);
 	}
+
+	hpfs_unlock(inode->i_sb);
 }
 
 static int hpfs_write_begin(struct file *file, struct address_space *mapping,

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] hpfs: better test for errors
  2013-06-08 23:25 [PATCH] hpfs: fix warnings when the filesystem fills up Mikulas Patocka
@ 2013-07-04 16:42 ` Mikulas Patocka
  2013-07-04 16:44   ` [PATCH] hpfs: use mpage Mikulas Patocka
  2013-07-04 18:21   ` [PATCH] hpfs: better test for errors Linus Torvalds
  0 siblings, 2 replies; 5+ messages in thread
From: Mikulas Patocka @ 2013-07-04 16:42 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-fsdevel

hpfs: better test for errors

The test if bitmap access is out of bound could errorneously pass if the
device size is divisible by 16384 sectors and we are asking for one bitmap
after the end.

Check for invalid size in the superblock. Invalid size could cause integer
overflows in the rest of the code.

Signed-off-by: Mikulas Patocka <mpatocka@artax.karlin.mff.cuni.cz>
Cc: stable@kernel.org

---
 fs/hpfs/map.c   |    3 ++-
 fs/hpfs/super.c |    8 +++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

Index: linux-3.10-fast/fs/hpfs/map.c
===================================================================
--- linux-3.10-fast.orig/fs/hpfs/map.c	2013-07-03 23:56:43.000000000 +0200
+++ linux-3.10-fast/fs/hpfs/map.c	2013-07-03 23:57:55.000000000 +0200
@@ -17,7 +17,8 @@ __le32 *hpfs_map_bitmap(struct super_blo
 			 struct quad_buffer_head *qbh, char *id)
 {
 	secno sec;
-	if (hpfs_sb(s)->sb_chk) if (bmp_block * 16384 > hpfs_sb(s)->sb_fs_size) {
+	unsigned n_bands = (hpfs_sb(s)->sb_fs_size + 0x3fff) >> 14;
+	if (hpfs_sb(s)->sb_chk) if (bmp_block >= n_bands) {
 		hpfs_error(s, "hpfs_map_bitmap called with bad parameter: %08x at %s", bmp_block, id);
 		return NULL;
 	}
Index: linux-3.10-fast/fs/hpfs/super.c
===================================================================
--- linux-3.10-fast.orig/fs/hpfs/super.c	2013-07-04 00:05:17.000000000 +0200
+++ linux-3.10-fast/fs/hpfs/super.c	2013-07-04 00:07:23.000000000 +0200
@@ -560,7 +560,13 @@ static int hpfs_fill_super(struct super_
 	sbi->sb_cp_table = NULL;
 	sbi->sb_c_bitmap = -1;
 	sbi->sb_max_fwd_alloc = 0xffffff;
-	
+
+	if (sbi->sb_fs_size >= 0x80000000) {
+		hpfs_error(s, "invalid size in superblock: %08x",
+			(unsigned)sbi->sb_fs_size);
+		goto bail4;
+	}
+
 	/* Load bitmap directory */
 	if (!(sbi->sb_bmp_dir = hpfs_load_bitmap_directory(s, le32_to_cpu(superblock->bitmaps))))
 		goto bail4;


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] hpfs: use mpage
  2013-07-04 16:42 ` [PATCH] hpfs: better test for errors Mikulas Patocka
@ 2013-07-04 16:44   ` Mikulas Patocka
  2013-07-04 17:04     ` [PATCH] hpfs: implement prefetch to improve performance Mikulas Patocka
  2013-07-04 18:21   ` [PATCH] hpfs: better test for errors Linus Torvalds
  1 sibling, 1 reply; 5+ messages in thread
From: Mikulas Patocka @ 2013-07-04 16:44 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-fsdevel

hpfs: use mpage

Use the mpage interface to improve performance.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 fs/hpfs/file.c |   40 ++++++++++++++++++++++++++++++++++------
 1 file changed, 34 insertions(+), 6 deletions(-)

Index: linux-3.10-fast/fs/hpfs/file.c
===================================================================
--- linux-3.10-fast.orig/fs/hpfs/file.c	2013-07-02 23:59:16.000000000 +0200
+++ linux-3.10-fast/fs/hpfs/file.c	2013-07-03 00:01:22.000000000 +0200
@@ -7,6 +7,7 @@
  */
 
 #include "hpfs_fn.h"
+#include <linux/mpage.h>
 
 #define BLOCKS(size) (((size) + 511) >> 9)
 
@@ -34,7 +35,7 @@ int hpfs_file_fsync(struct file *file, l
  * so we must ignore such errors.
  */
 
-static secno hpfs_bmap(struct inode *inode, unsigned file_secno)
+static secno hpfs_bmap(struct inode *inode, unsigned file_secno, unsigned *n_secs)
 {
 	struct hpfs_inode_info *hpfs_inode = hpfs_i(inode);
 	unsigned n, disk_secno;
@@ -42,11 +43,20 @@ static secno hpfs_bmap(struct inode *ino
 	struct buffer_head *bh;
 	if (BLOCKS(hpfs_i(inode)->mmu_private) <= file_secno) return 0;
 	n = file_secno - hpfs_inode->i_file_sec;
-	if (n < hpfs_inode->i_n_secs) return hpfs_inode->i_disk_sec + n;
+	if (n < hpfs_inode->i_n_secs) {
+		*n_secs = hpfs_inode->i_n_secs - n;
+		return hpfs_inode->i_disk_sec + n;
+	}
 	if (!(fnode = hpfs_map_fnode(inode->i_sb, inode->i_ino, &bh))) return 0;
 	disk_secno = hpfs_bplus_lookup(inode->i_sb, inode, &fnode->btree, file_secno, bh);
 	if (disk_secno == -1) return 0;
 	if (hpfs_chk_sectors(inode->i_sb, disk_secno, 1, "bmap")) return 0;
+	n = file_secno - hpfs_inode->i_file_sec;
+	if (n < hpfs_inode->i_n_secs) {
+		*n_secs = hpfs_inode->i_n_secs - n;
+		return hpfs_inode->i_disk_sec + n;
+	}
+	*n_secs = 1;
 	return disk_secno;
 }
 
@@ -67,10 +77,14 @@ static int hpfs_get_block(struct inode *
 {
 	int r;
 	secno s;
+	unsigned n_secs;
 	hpfs_lock(inode->i_sb);
-	s = hpfs_bmap(inode, iblock);
+	s = hpfs_bmap(inode, iblock, &n_secs);
 	if (s) {
+		if (bh_result->b_size >> 9 < n_secs)
+			n_secs = bh_result->b_size >> 9;
 		map_bh(bh_result, inode->i_sb, s);
+		bh_result->b_size = n_secs << 9;
 		goto ret_0;
 	}
 	if (!create) goto ret_0;
@@ -95,14 +109,26 @@ static int hpfs_get_block(struct inode *
 	return r;
 }
 
+static int hpfs_readpage(struct file *file, struct page *page)
+{
+	return mpage_readpage(page, hpfs_get_block);
+}
+
 static int hpfs_writepage(struct page *page, struct writeback_control *wbc)
 {
-	return block_write_full_page(page,hpfs_get_block, wbc);
+	return block_write_full_page(page, hpfs_get_block, wbc);
 }
 
-static int hpfs_readpage(struct file *file, struct page *page)
+static int hpfs_readpages(struct file *file, struct address_space *mapping,
+			  struct list_head *pages, unsigned nr_pages)
+{
+	return mpage_readpages(mapping, pages, nr_pages, hpfs_get_block);
+}
+
+static int hpfs_writepages(struct address_space *mapping,
+			   struct writeback_control *wbc)
 {
-	return block_read_full_page(page,hpfs_get_block);
+	return mpage_writepages(mapping, wbc, hpfs_get_block);
 }
 
 static void hpfs_write_failed(struct address_space *mapping, loff_t to)
@@ -161,6 +187,8 @@ static sector_t _hpfs_bmap(struct addres
 const struct address_space_operations hpfs_aops = {
 	.readpage = hpfs_readpage,
 	.writepage = hpfs_writepage,
+	.readpages = hpfs_readpages,
+	.writepages = hpfs_writepages,
 	.write_begin = hpfs_write_begin,
 	.write_end = hpfs_write_end,
 	.bmap = _hpfs_bmap


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] hpfs: implement prefetch to improve performance
  2013-07-04 16:44   ` [PATCH] hpfs: use mpage Mikulas Patocka
@ 2013-07-04 17:04     ` Mikulas Patocka
  0 siblings, 0 replies; 5+ messages in thread
From: Mikulas Patocka @ 2013-07-04 17:04 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-fsdevel

hpfs: implement prefetch to improve performance

This patch implements prefetch to improve performance.
It helps mostly when scanning the bitmaps to calculate free space.

Signed-off-by: Mikulas Patocka <mpatocka@artax.karlin.mff.cuni.cz>

---
 fs/hpfs/buffer.c  |   33 +++++++++++++++++++++++++++++++++
 fs/hpfs/hpfs_fn.h |    7 +++++--
 fs/hpfs/map.c     |   19 ++++++++++++++++++-
 fs/hpfs/super.c   |    9 +++++++--
 4 files changed, 63 insertions(+), 5 deletions(-)

Index: linux-3.9.9-fast/fs/hpfs/buffer.c
===================================================================
--- linux-3.9.9-fast.orig/fs/hpfs/buffer.c	2013-07-04 02:42:13.000000000 +0200
+++ linux-3.9.9-fast/fs/hpfs/buffer.c	2013-07-04 18:54:25.000000000 +0200
@@ -7,8 +7,37 @@
  */
 #include <linux/sched.h>
 #include <linux/slab.h>
+#include <linux/blkdev.h>
 #include "hpfs_fn.h"
 
+void hpfs_prefetch_sectors(struct super_block *s, unsigned secno, int n)
+{
+	struct buffer_head *bh;
+	struct blk_plug plug;
+
+	if (n <= 0 || unlikely(secno >= hpfs_sb(s)->sb_fs_size))
+		return;
+
+	bh = sb_find_get_block(s, secno);
+	if (bh) {
+		if (buffer_uptodate(bh)) {
+			brelse(bh);
+			return;
+		}
+		brelse(bh);
+	};
+
+	blk_start_plug(&plug);
+	while (n > 0) {
+		if (unlikely(secno >= hpfs_sb(s)->sb_fs_size))
+			break;
+		sb_breadahead(s, secno);
+		secno++;
+		n--;
+	}
+	blk_finish_plug(&plug);
+}
+
 /* Map a sector into a buffer and return pointers to it and to the buffer. */
 
 void *hpfs_map_sector(struct super_block *s, unsigned secno, struct buffer_head **bhp,
@@ -18,6 +47,8 @@ void *hpfs_map_sector(struct super_block
 
 	hpfs_lock_assert(s);
 
+	hpfs_prefetch_sectors(s, secno, ahead);
+
 	cond_resched();
 
 	*bhp = bh = sb_bread(s, secno);
@@ -67,6 +98,8 @@ void *hpfs_map_4sectors(struct super_blo
 		return NULL;
 	}
 
+	hpfs_prefetch_sectors(s, secno, 4 + ahead);
+
 	qbh->data = data = kmalloc(2048, GFP_NOFS);
 	if (!data) {
 		printk("HPFS: hpfs_map_4sectors: out of memory\n");
Index: linux-3.9.9-fast/fs/hpfs/hpfs_fn.h
===================================================================
--- linux-3.9.9-fast.orig/fs/hpfs/hpfs_fn.h	2013-07-04 02:42:13.000000000 +0200
+++ linux-3.9.9-fast/fs/hpfs/hpfs_fn.h	2013-07-04 18:58:28.000000000 +0200
@@ -27,8 +27,9 @@
 #define ALLOC_FWD_MAX	128
 #define ALLOC_M		1
 #define FNODE_RD_AHEAD	16
-#define ANODE_RD_AHEAD	16
-#define DNODE_RD_AHEAD	4
+#define ANODE_RD_AHEAD	0
+#define DNODE_RD_AHEAD	72
+#define COUNT_RD_AHEAD	62
 
 #define FREE_DNODES_ADD	58
 #define FREE_DNODES_DEL	29
@@ -207,6 +208,7 @@ void hpfs_remove_fnode(struct super_bloc
 
 /* buffer.c */
 
+void hpfs_prefetch_sectors(struct super_block *, unsigned, int);
 void *hpfs_map_sector(struct super_block *, unsigned, struct buffer_head **, int);
 void *hpfs_get_sector(struct super_block *, unsigned, struct buffer_head **);
 void *hpfs_map_4sectors(struct super_block *, unsigned, struct quad_buffer_head *, int);
@@ -271,6 +273,7 @@ void hpfs_evict_inode(struct inode *);
 
 __le32 *hpfs_map_dnode_bitmap(struct super_block *, struct quad_buffer_head *);
 __le32 *hpfs_map_bitmap(struct super_block *, unsigned, struct quad_buffer_head *, char *);
+void hpfs_prefetch_bitmap(struct super_block *, unsigned);
 unsigned char *hpfs_load_code_page(struct super_block *, secno);
 __le32 *hpfs_load_bitmap_directory(struct super_block *, secno bmp);
 struct fnode *hpfs_map_fnode(struct super_block *s, ino_t, struct buffer_head **);
Index: linux-3.9.9-fast/fs/hpfs/super.c
===================================================================
--- linux-3.9.9-fast.orig/fs/hpfs/super.c	2013-07-04 01:20:32.000000000 +0200
+++ linux-3.9.9-fast/fs/hpfs/super.c	2013-07-04 18:56:34.000000000 +0200
@@ -123,7 +123,7 @@ unsigned hpfs_count_one_bitmap(struct su
 	unsigned long *bits;
 	unsigned count;
 
-	bits = hpfs_map_4sectors(s, secno, &qbh, 4);
+	bits = hpfs_map_4sectors(s, secno, &qbh, 0);
 	if (!bits)
 		return 0;
 	count = bitmap_weight(bits, 2048 * BITS_PER_BYTE);
@@ -136,8 +136,13 @@ static unsigned count_bitmaps(struct sup
 	unsigned n, count, n_bands;
 	n_bands = (hpfs_sb(s)->sb_fs_size + 0x3fff) >> 14;
 	count = 0;
-	for (n = 0; n < n_bands; n++)
+	for (n = 0; n < COUNT_RD_AHEAD; n++) {
+		hpfs_prefetch_bitmap(s, n);
+	}
+	for (n = 0; n < n_bands; n++) {
+		hpfs_prefetch_bitmap(s, n + COUNT_RD_AHEAD);
 		count += hpfs_count_one_bitmap(s, le32_to_cpu(hpfs_sb(s)->sb_bmp_dir[n]));
+	}
 	return count;
 }
 
Index: linux-3.9.9-fast/fs/hpfs/map.c
===================================================================
--- linux-3.9.9-fast.orig/fs/hpfs/map.c	2013-07-04 01:20:32.000000000 +0200
+++ linux-3.9.9-fast/fs/hpfs/map.c	2013-07-04 01:20:32.000000000 +0200
@@ -17,6 +17,7 @@ __le32 *hpfs_map_bitmap(struct super_blo
 			 struct quad_buffer_head *qbh, char *id)
 {
 	secno sec;
+	__le32 *ret;
 	unsigned n_bands = (hpfs_sb(s)->sb_fs_size + 0x3fff) >> 14;
 	if (hpfs_sb(s)->sb_chk) if (bmp_block >= n_bands) {
 		hpfs_error(s, "hpfs_map_bitmap called with bad parameter: %08x at %s", bmp_block, id);
@@ -27,7 +28,23 @@ __le32 *hpfs_map_bitmap(struct super_blo
 		hpfs_error(s, "invalid bitmap block pointer %08x -> %08x at %s", bmp_block, sec, id);
 		return NULL;
 	}
-	return hpfs_map_4sectors(s, sec, qbh, 4);
+	ret = hpfs_map_4sectors(s, sec, qbh, 4);
+	if (ret) hpfs_prefetch_bitmap(s, bmp_block + 1);
+	return ret;
+}
+
+void hpfs_prefetch_bitmap(struct super_block *s, unsigned bmp_block)
+{
+	unsigned to_prefetch, next_prefetch;
+	unsigned n_bands = (hpfs_sb(s)->sb_fs_size + 0x3fff) >> 14;
+	if (unlikely(bmp_block >= n_bands))
+		return;
+	to_prefetch = le32_to_cpu(hpfs_sb(s)->sb_bmp_dir[bmp_block]);
+	if (unlikely(bmp_block + 1 >= n_bands))
+		next_prefetch = 0;
+	else
+		next_prefetch = le32_to_cpu(hpfs_sb(s)->sb_bmp_dir[bmp_block + 1]);
+	hpfs_prefetch_sectors(s, to_prefetch, 4 + 4 * (to_prefetch + 4 == next_prefetch));
 }
 
 /*


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] hpfs: better test for errors
  2013-07-04 16:42 ` [PATCH] hpfs: better test for errors Mikulas Patocka
  2013-07-04 16:44   ` [PATCH] hpfs: use mpage Mikulas Patocka
@ 2013-07-04 18:21   ` Linus Torvalds
  1 sibling, 0 replies; 5+ messages in thread
From: Linus Torvalds @ 2013-07-04 18:21 UTC (permalink / raw)
  To: Mikulas Patocka; +Cc: linux-fsdevel

On Thu, Jul 4, 2013 at 9:42 AM, Mikulas Patocka
<mikulas@artax.karlin.mff.cuni.cz> wrote:
> hpfs: better test for errors
>
> The test if bitmap access is out of bound could errorneously pass if the
> device size is divisible by 16384 sectors and we are asking for one bitmap
[...]

Mikulas, *please* fix whatever scripts you (and apparently some other
people) use that causes the subject line to be repeated as the first
line of the body of the email too..

Whatever it is, it's annoying. I can edit the emails, but I know for a
fact that others don't, and sometimes I miss it too. Resulting in
commit messages that have that silly duplication. There are too many
examples of this in the kernel logs already, let's put an end to it.

The rule is:

 - the subject line is the summary line of the patch

 - if you want to explicitly add a summary line in the body (because
your subject line is something else - maybe you're replying in a
thread and don't want to break the thread subject), you can do it by
having

     Subject: summary-line-goes-here

the more common cases being that you want to specify authorship or
original date (which will obviously be set specifically and perhaps
not the way you like it in the mail headers).

But just repeating the summary line is *BROKEN*. What broken
scripts/scms do this crap?

               Linus

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-07-04 18:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-08 23:25 [PATCH] hpfs: fix warnings when the filesystem fills up Mikulas Patocka
2013-07-04 16:42 ` [PATCH] hpfs: better test for errors Mikulas Patocka
2013-07-04 16:44   ` [PATCH] hpfs: use mpage Mikulas Patocka
2013-07-04 17:04     ` [PATCH] hpfs: implement prefetch to improve performance Mikulas Patocka
2013-07-04 18:21   ` [PATCH] hpfs: better test for errors Linus Torvalds

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.