* [PATCH] udf: add extent cache support in case of file reading
@ 2013-01-12 6:13 Namjae Jeon
2013-01-14 14:56 ` Jan Kara
0 siblings, 1 reply; 13+ messages in thread
From: Namjae Jeon @ 2013-01-12 6:13 UTC (permalink / raw)
To: jack
Cc: linux-fsdevel, linux-kernel, Namjae Jeon, Namjae Jeon,
Ashish Sangwan, Bonggil Bak
From: Namjae Jeon <namjae.jeon@samsung.com>
This patch implements extent caching in case of file reading.
While reading a file, currently, UDF reads metadata serially
which takes a lot of time depending on the number of extents present
in the file. Caching last accessd extent improves metadata read time.
Instead of reading file metadata from start, now we read from
the cached extent.
This patch considerably improves the time spent by CPU in kernel mode.
For example, while reading a 10.9 GB file using dd:
Time before applying patch:
11677022208 bytes (10.9GB) copied, 1529.748921 seconds, 7.3MB/s
real 25m 29.85s
user 0m 12.41s
sys 15m 34.75s
Time after applying patch:
11677022208 bytes (10.9GB) copied, 1469.338231 seconds, 7.6MB/s
real 24m 29.44s
user 0m 15.73s
sys 3m 27.61s
Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Ashish Sangwan <a.sangwan@samsung.com>
Signed-off-by: Bonggil Bak <bgbak@samsung.com>
---
fs/udf/ialloc.c | 2 ++
fs/udf/inode.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++-----
fs/udf/udf_i.h | 17 +++++++++++++
fs/udf/udfdecl.h | 10 ++++----
4 files changed, 92 insertions(+), 11 deletions(-)
diff --git a/fs/udf/ialloc.c b/fs/udf/ialloc.c
index 7e5aae4..7dd86a4a 100644
--- a/fs/udf/ialloc.c
+++ b/fs/udf/ialloc.c
@@ -117,6 +117,8 @@ struct inode *udf_new_inode(struct inode *dir, umode_t mode, int *err)
iinfo->i_lenAlloc = 0;
iinfo->i_use = 0;
iinfo->i_checkpoint = 1;
+ memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
+ mutex_init(&(iinfo->i_extent_cache_lock));
if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_AD_IN_ICB))
iinfo->i_alloc_type = ICBTAG_FLAG_AD_IN_ICB;
else if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_SHORT_AD))
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index cbae1ed..9980381 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -90,6 +90,7 @@ void udf_evict_inode(struct inode *inode)
}
kfree(iinfo->i_ext.i_data);
iinfo->i_ext.i_data = NULL;
+ udf_clear_extent_cache(iinfo);
if (want_delete) {
udf_free_inode(inode);
}
@@ -105,6 +106,7 @@ static void udf_write_failed(struct address_space *mapping, loff_t to)
truncate_pagecache(inode, to, isize);
if (iinfo->i_alloc_type != ICBTAG_FLAG_AD_IN_ICB) {
down_write(&iinfo->i_data_sem);
+ udf_clear_extent_cache(iinfo);
udf_truncate_extents(inode);
up_write(&iinfo->i_data_sem);
}
@@ -372,7 +374,7 @@ static int udf_get_block(struct inode *inode, sector_t block,
iinfo->i_next_alloc_goal++;
}
-
+ udf_clear_extent_cache(iinfo);
phys = inode_getblk(inode, block, &err, &new);
if (!phys)
goto abort;
@@ -1171,6 +1173,7 @@ set_size:
} else {
if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB) {
down_write(&iinfo->i_data_sem);
+ udf_clear_extent_cache(iinfo);
memset(iinfo->i_ext.i_data + iinfo->i_lenEAttr + newsize,
0x00, bsize - newsize -
udf_file_entry_alloc_offset(inode));
@@ -1184,6 +1187,7 @@ set_size:
if (err)
return err;
down_write(&iinfo->i_data_sem);
+ udf_clear_extent_cache(iinfo);
truncate_setsize(inode, newsize);
udf_truncate_extents(inode);
up_write(&iinfo->i_data_sem);
@@ -1301,6 +1305,8 @@ static void udf_fill_inode(struct inode *inode, struct buffer_head *bh)
iinfo->i_lenAlloc = 0;
iinfo->i_next_alloc_block = 0;
iinfo->i_next_alloc_goal = 0;
+ memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
+ mutex_init(&(iinfo->i_extent_cache_lock));
if (fe->descTag.tagIdent == cpu_to_le16(TAG_IDENT_EFE)) {
iinfo->i_efe = 1;
iinfo->i_use = 0;
@@ -2156,11 +2162,12 @@ int8_t inode_bmap(struct inode *inode, sector_t block,
struct udf_inode_info *iinfo;
iinfo = UDF_I(inode);
- pos->offset = 0;
- pos->block = iinfo->i_location;
- pos->bh = NULL;
+ if (!udf_read_extent_cache(inode, bcount, &lbcount, pos)) {
+ pos->offset = 0;
+ pos->block = iinfo->i_location;
+ pos->bh = NULL;
+ }
*elen = 0;
-
do {
etype = udf_next_aext(inode, pos, eloc, elen, 1);
if (etype == -1) {
@@ -2170,7 +2177,8 @@ int8_t inode_bmap(struct inode *inode, sector_t block,
}
lbcount += *elen;
} while (lbcount <= bcount);
-
+ /* update extent cache */
+ udf_update_extent_cache(inode, lbcount - *elen, pos, 1);
*offset = (bcount + *elen - lbcount) >> blocksize_bits;
return etype;
@@ -2200,3 +2208,57 @@ long udf_block_map(struct inode *inode, sector_t block)
else
return ret;
}
+int udf_read_extent_cache(struct inode *inode, loff_t bcount,
+ loff_t *lbcount, struct extent_position *pos)
+{
+ struct udf_inode_info *iinfo = UDF_I(inode);
+ mutex_lock(&iinfo->i_extent_cache_lock);
+ if ((iinfo->cached_extent.lstart <= bcount) &&
+ (iinfo->cached_extent.valid)) {
+ /* Cache hit */
+ *lbcount = iinfo->cached_extent.lstart;
+ memcpy(pos, &iinfo->cached_extent.epos,
+ sizeof(struct extent_position));
+ mutex_unlock(&iinfo->i_extent_cache_lock);
+ return 1;
+ } else
+ udf_clear_extent_cache(iinfo);
+ mutex_unlock(&iinfo->i_extent_cache_lock);
+ return 0;
+}
+void udf_update_extent_cache(struct inode *inode, loff_t estart,
+ struct extent_position *pos, int next_epos)
+{
+ struct udf_inode_info *iinfo = UDF_I(inode);
+ mutex_lock(&iinfo->i_extent_cache_lock);
+ if (pos->bh != NULL)
+ /* Increase ref count */
+ get_bh(pos->bh);
+ memcpy(&iinfo->cached_extent.epos, pos,
+ sizeof(struct extent_position));
+ iinfo->cached_extent.lstart = estart;
+ iinfo->cached_extent.valid = 1;
+ if (next_epos)
+ switch (iinfo->i_alloc_type) {
+ case ICBTAG_FLAG_AD_SHORT:
+ iinfo->cached_extent.epos.offset -=
+ sizeof(struct short_ad);
+ break;
+ case ICBTAG_FLAG_AD_LONG:
+ iinfo->cached_extent.epos.offset -=
+ sizeof(struct long_ad);
+ }
+ mutex_unlock(&iinfo->i_extent_cache_lock);
+}
+
+/* This function should be called after i_data_sem is
+ * held for writing OR i_extent_cache_lock is taken.
+ */
+void udf_clear_extent_cache(struct udf_inode_info *iinfo)
+{
+ if (iinfo->cached_extent.valid) {
+ brelse(iinfo->cached_extent.epos.bh);
+ memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
+ }
+}
+
diff --git a/fs/udf/udf_i.h b/fs/udf/udf_i.h
index bb8309d..95e5e17 100644
--- a/fs/udf/udf_i.h
+++ b/fs/udf/udf_i.h
@@ -1,6 +1,20 @@
#ifndef _UDF_I_H
#define _UDF_I_H
+struct extent_position {
+ struct buffer_head *bh;
+ uint32_t offset;
+ struct kernel_lb_addr block;
+};
+
+struct udf_ext_cache {
+ /* Extent position */
+ struct extent_position epos;
+ /* Start logical offset in bytes */
+ loff_t lstart;
+ bool valid;
+};
+
/*
* The i_data_sem and i_mutex serve for protection of allocation information
* of a regular files and symlinks. This includes all extents belonging to
@@ -35,6 +49,9 @@ struct udf_inode_info {
__u8 *i_data;
} i_ext;
struct rw_semaphore i_data_sem;
+ struct udf_ext_cache cached_extent;
+ /* Mutex for protecting extent cache */
+ struct mutex i_extent_cache_lock;
struct inode vfs_inode;
};
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index de038da..db4a22e 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -113,11 +113,6 @@ struct ustr {
uint8_t u_len;
};
-struct extent_position {
- struct buffer_head *bh;
- uint32_t offset;
- struct kernel_lb_addr block;
-};
/* super.c */
@@ -164,6 +159,11 @@ extern int8_t udf_next_aext(struct inode *, struct extent_position *,
struct kernel_lb_addr *, uint32_t *, int);
extern int8_t udf_current_aext(struct inode *, struct extent_position *,
struct kernel_lb_addr *, uint32_t *, int);
+int udf_read_extent_cache(struct inode *inode, loff_t bcount, loff_t *lbcount,
+ struct extent_position *pos);
+void udf_update_extent_cache(struct inode *inode, loff_t estart,
+ struct extent_position *pos, int next_epos);
+void udf_clear_extent_cache(struct udf_inode_info *iinfo);
/* misc.c */
extern struct buffer_head *udf_tgetblk(struct super_block *, int);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] udf: add extent cache support in case of file reading
2013-01-12 6:13 [PATCH] udf: add extent cache support in case of file reading Namjae Jeon
@ 2013-01-14 14:56 ` Jan Kara
2013-01-15 2:05 ` Namjae Jeon
0 siblings, 1 reply; 13+ messages in thread
From: Jan Kara @ 2013-01-14 14:56 UTC (permalink / raw)
To: Namjae Jeon
Cc: jack, linux-fsdevel, linux-kernel, Namjae Jeon, Ashish Sangwan,
Bonggil Bak
On Sat 12-01-13 15:13:08, Namjae Jeon wrote:
> From: Namjae Jeon <namjae.jeon@samsung.com>
>
> This patch implements extent caching in case of file reading.
> While reading a file, currently, UDF reads metadata serially
> which takes a lot of time depending on the number of extents present
> in the file. Caching last accessd extent improves metadata read time.
> Instead of reading file metadata from start, now we read from
> the cached extent.
>
> This patch considerably improves the time spent by CPU in kernel mode.
> For example, while reading a 10.9 GB file using dd:
> Time before applying patch:
> 11677022208 bytes (10.9GB) copied, 1529.748921 seconds, 7.3MB/s
> real 25m 29.85s
> user 0m 12.41s
> sys 15m 34.75s
>
> Time after applying patch:
> 11677022208 bytes (10.9GB) copied, 1469.338231 seconds, 7.6MB/s
> real 24m 29.44s
> user 0m 15.73s
> sys 3m 27.61s
Thanks for the patch! I have just three minor comments below.
> Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
> Signed-off-by: Ashish Sangwan <a.sangwan@samsung.com>
> Signed-off-by: Bonggil Bak <bgbak@samsung.com>
> ---
> fs/udf/ialloc.c | 2 ++
> fs/udf/inode.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++-----
> fs/udf/udf_i.h | 17 +++++++++++++
> fs/udf/udfdecl.h | 10 ++++----
> 4 files changed, 92 insertions(+), 11 deletions(-)
>
> diff --git a/fs/udf/ialloc.c b/fs/udf/ialloc.c
> index 7e5aae4..7dd86a4a 100644
> --- a/fs/udf/ialloc.c
> +++ b/fs/udf/ialloc.c
> @@ -117,6 +117,8 @@ struct inode *udf_new_inode(struct inode *dir, umode_t mode, int *err)
> iinfo->i_lenAlloc = 0;
> iinfo->i_use = 0;
> iinfo->i_checkpoint = 1;
> + memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
> + mutex_init(&(iinfo->i_extent_cache_lock));
> if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_AD_IN_ICB))
> iinfo->i_alloc_type = ICBTAG_FLAG_AD_IN_ICB;
> else if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_SHORT_AD))
> diff --git a/fs/udf/inode.c b/fs/udf/inode.c
> index cbae1ed..9980381 100644
> --- a/fs/udf/inode.c
> +++ b/fs/udf/inode.c
> @@ -90,6 +90,7 @@ void udf_evict_inode(struct inode *inode)
> }
> kfree(iinfo->i_ext.i_data);
> iinfo->i_ext.i_data = NULL;
> + udf_clear_extent_cache(iinfo);
> if (want_delete) {
> udf_free_inode(inode);
> }
> @@ -105,6 +106,7 @@ static void udf_write_failed(struct address_space *mapping, loff_t to)
> truncate_pagecache(inode, to, isize);
> if (iinfo->i_alloc_type != ICBTAG_FLAG_AD_IN_ICB) {
> down_write(&iinfo->i_data_sem);
> + udf_clear_extent_cache(iinfo);
> udf_truncate_extents(inode);
> up_write(&iinfo->i_data_sem);
> }
> @@ -372,7 +374,7 @@ static int udf_get_block(struct inode *inode, sector_t block,
> iinfo->i_next_alloc_goal++;
> }
>
> -
> + udf_clear_extent_cache(iinfo);
> phys = inode_getblk(inode, block, &err, &new);
> if (!phys)
> goto abort;
> @@ -1171,6 +1173,7 @@ set_size:
> } else {
> if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB) {
> down_write(&iinfo->i_data_sem);
> + udf_clear_extent_cache(iinfo);
> memset(iinfo->i_ext.i_data + iinfo->i_lenEAttr + newsize,
> 0x00, bsize - newsize -
> udf_file_entry_alloc_offset(inode));
> @@ -1184,6 +1187,7 @@ set_size:
> if (err)
> return err;
> down_write(&iinfo->i_data_sem);
> + udf_clear_extent_cache(iinfo);
> truncate_setsize(inode, newsize);
> udf_truncate_extents(inode);
> up_write(&iinfo->i_data_sem);
> @@ -1301,6 +1305,8 @@ static void udf_fill_inode(struct inode *inode, struct buffer_head *bh)
> iinfo->i_lenAlloc = 0;
> iinfo->i_next_alloc_block = 0;
> iinfo->i_next_alloc_goal = 0;
> + memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
> + mutex_init(&(iinfo->i_extent_cache_lock));
> if (fe->descTag.tagIdent == cpu_to_le16(TAG_IDENT_EFE)) {
> iinfo->i_efe = 1;
> iinfo->i_use = 0;
> @@ -2156,11 +2162,12 @@ int8_t inode_bmap(struct inode *inode, sector_t block,
> struct udf_inode_info *iinfo;
>
> iinfo = UDF_I(inode);
> - pos->offset = 0;
> - pos->block = iinfo->i_location;
> - pos->bh = NULL;
> + if (!udf_read_extent_cache(inode, bcount, &lbcount, pos)) {
> + pos->offset = 0;
> + pos->block = iinfo->i_location;
> + pos->bh = NULL;
> + }
> *elen = 0;
> -
> do {
> etype = udf_next_aext(inode, pos, eloc, elen, 1);
> if (etype == -1) {
> @@ -2170,7 +2177,8 @@ int8_t inode_bmap(struct inode *inode, sector_t block,
> }
> lbcount += *elen;
> } while (lbcount <= bcount);
> -
> + /* update extent cache */
> + udf_update_extent_cache(inode, lbcount - *elen, pos, 1);
> *offset = (bcount + *elen - lbcount) >> blocksize_bits;
>
> return etype;
> @@ -2200,3 +2208,57 @@ long udf_block_map(struct inode *inode, sector_t block)
> else
> return ret;
> }
> +int udf_read_extent_cache(struct inode *inode, loff_t bcount,
> + loff_t *lbcount, struct extent_position *pos)
> +{
> + struct udf_inode_info *iinfo = UDF_I(inode);
Use empty line after declarations please...
> + mutex_lock(&iinfo->i_extent_cache_lock);
> + if ((iinfo->cached_extent.lstart <= bcount) &&
> + (iinfo->cached_extent.valid)) {
> + /* Cache hit */
> + *lbcount = iinfo->cached_extent.lstart;
> + memcpy(pos, &iinfo->cached_extent.epos,
> + sizeof(struct extent_position));
> + mutex_unlock(&iinfo->i_extent_cache_lock);
> + return 1;
> + } else
> + udf_clear_extent_cache(iinfo);
> + mutex_unlock(&iinfo->i_extent_cache_lock);
> + return 0;
> +}
> +void udf_update_extent_cache(struct inode *inode, loff_t estart,
> + struct extent_position *pos, int next_epos)
> +{
> + struct udf_inode_info *iinfo = UDF_I(inode);
> + mutex_lock(&iinfo->i_extent_cache_lock);
> + if (pos->bh != NULL)
> + /* Increase ref count */
> + get_bh(pos->bh);
> + memcpy(&iinfo->cached_extent.epos, pos,
> + sizeof(struct extent_position));
> + iinfo->cached_extent.lstart = estart;
> + iinfo->cached_extent.valid = 1;
> + if (next_epos)
> + switch (iinfo->i_alloc_type) {
> + case ICBTAG_FLAG_AD_SHORT:
> + iinfo->cached_extent.epos.offset -=
> + sizeof(struct short_ad);
> + break;
> + case ICBTAG_FLAG_AD_LONG:
> + iinfo->cached_extent.epos.offset -=
> + sizeof(struct long_ad);
> + }
> + mutex_unlock(&iinfo->i_extent_cache_lock);
> +}
> +
> +/* This function should be called after i_data_sem is
> + * held for writing OR i_extent_cache_lock is taken.
> + */
> +void udf_clear_extent_cache(struct udf_inode_info *iinfo)
> +{
> + if (iinfo->cached_extent.valid) {
> + brelse(iinfo->cached_extent.epos.bh);
> + memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
> + }
> +}
> +
> diff --git a/fs/udf/udf_i.h b/fs/udf/udf_i.h
> index bb8309d..95e5e17 100644
> --- a/fs/udf/udf_i.h
> +++ b/fs/udf/udf_i.h
> @@ -1,6 +1,20 @@
> #ifndef _UDF_I_H
> #define _UDF_I_H
>
> +struct extent_position {
> + struct buffer_head *bh;
> + uint32_t offset;
> + struct kernel_lb_addr block;
> +};
> +
> +struct udf_ext_cache {
> + /* Extent position */
> + struct extent_position epos;
> + /* Start logical offset in bytes */
> + loff_t lstart;
> + bool valid;
I'd just remove the 'valid' entry and use lstart == -1 as an indication
for invalid cache.
> +};
> +
> /*
> * The i_data_sem and i_mutex serve for protection of allocation information
> * of a regular files and symlinks. This includes all extents belonging to
> @@ -35,6 +49,9 @@ struct udf_inode_info {
> __u8 *i_data;
> } i_ext;
> struct rw_semaphore i_data_sem;
> + struct udf_ext_cache cached_extent;
> + /* Mutex for protecting extent cache */
> + struct mutex i_extent_cache_lock;
This should be spinlock not mutex. We don't need to sleep while holding
the lock and spinlocks are both faster and smaller...
> struct inode vfs_inode;
> };
>
> diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
> index de038da..db4a22e 100644
> --- a/fs/udf/udfdecl.h
> +++ b/fs/udf/udfdecl.h
> @@ -113,11 +113,6 @@ struct ustr {
> uint8_t u_len;
> };
>
> -struct extent_position {
> - struct buffer_head *bh;
> - uint32_t offset;
> - struct kernel_lb_addr block;
> -};
>
> /* super.c */
>
> @@ -164,6 +159,11 @@ extern int8_t udf_next_aext(struct inode *, struct extent_position *,
> struct kernel_lb_addr *, uint32_t *, int);
> extern int8_t udf_current_aext(struct inode *, struct extent_position *,
> struct kernel_lb_addr *, uint32_t *, int);
> +int udf_read_extent_cache(struct inode *inode, loff_t bcount, loff_t *lbcount,
> + struct extent_position *pos);
> +void udf_update_extent_cache(struct inode *inode, loff_t estart,
> + struct extent_position *pos, int next_epos);
> +void udf_clear_extent_cache(struct udf_inode_info *iinfo);
>
> /* misc.c */
> extern struct buffer_head *udf_tgetblk(struct super_block *, int);
> --
> 1.7.9.5
>
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] udf: add extent cache support in case of file reading
2013-01-14 14:56 ` Jan Kara
@ 2013-01-15 2:05 ` Namjae Jeon
0 siblings, 0 replies; 13+ messages in thread
From: Namjae Jeon @ 2013-01-15 2:05 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, linux-kernel, Namjae Jeon, Ashish Sangwan, Bonggil Bak
2013/1/14, Jan Kara <jack@suse.cz>:
> On Sat 12-01-13 15:13:08, Namjae Jeon wrote:
>> From: Namjae Jeon <namjae.jeon@samsung.com>
>>
>> This patch implements extent caching in case of file reading.
>> While reading a file, currently, UDF reads metadata serially
>> which takes a lot of time depending on the number of extents present
>> in the file. Caching last accessd extent improves metadata read time.
>> Instead of reading file metadata from start, now we read from
>> the cached extent.
>>
>> This patch considerably improves the time spent by CPU in kernel mode.
>> For example, while reading a 10.9 GB file using dd:
>> Time before applying patch:
>> 11677022208 bytes (10.9GB) copied, 1529.748921 seconds, 7.3MB/s
>> real 25m 29.85s
>> user 0m 12.41s
>> sys 15m 34.75s
>>
>> Time after applying patch:
>> 11677022208 bytes (10.9GB) copied, 1469.338231 seconds, 7.6MB/s
>> real 24m 29.44s
>> user 0m 15.73s
>> sys 3m 27.61s
> Thanks for the patch! I have just three minor comments below.
Hi. Jan.
After fixing, I will resend v2 patch again.
Thanks for review!
>
>> Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
>> Signed-off-by: Ashish Sangwan <a.sangwan@samsung.com>
>> Signed-off-by: Bonggil Bak <bgbak@samsung.com>
>> ---
>> fs/udf/ialloc.c | 2 ++
>> fs/udf/inode.c | 74
>> +++++++++++++++++++++++++++++++++++++++++++++++++-----
>> fs/udf/udf_i.h | 17 +++++++++++++
>> fs/udf/udfdecl.h | 10 ++++----
>> 4 files changed, 92 insertions(+), 11 deletions(-)
>>
>> diff --git a/fs/udf/ialloc.c b/fs/udf/ialloc.c
>> index 7e5aae4..7dd86a4a 100644
>> --- a/fs/udf/ialloc.c
>> +++ b/fs/udf/ialloc.c
>> @@ -117,6 +117,8 @@ struct inode *udf_new_inode(struct inode *dir, umode_t
>> mode, int *err)
>> iinfo->i_lenAlloc = 0;
>> iinfo->i_use = 0;
>> iinfo->i_checkpoint = 1;
>> + memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
>> + mutex_init(&(iinfo->i_extent_cache_lock));
>> if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_AD_IN_ICB))
>> iinfo->i_alloc_type = ICBTAG_FLAG_AD_IN_ICB;
>> else if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_SHORT_AD))
>> diff --git a/fs/udf/inode.c b/fs/udf/inode.c
>> index cbae1ed..9980381 100644
>> --- a/fs/udf/inode.c
>> +++ b/fs/udf/inode.c
>> @@ -90,6 +90,7 @@ void udf_evict_inode(struct inode *inode)
>> }
>> kfree(iinfo->i_ext.i_data);
>> iinfo->i_ext.i_data = NULL;
>> + udf_clear_extent_cache(iinfo);
>> if (want_delete) {
>> udf_free_inode(inode);
>> }
>> @@ -105,6 +106,7 @@ static void udf_write_failed(struct address_space
>> *mapping, loff_t to)
>> truncate_pagecache(inode, to, isize);
>> if (iinfo->i_alloc_type != ICBTAG_FLAG_AD_IN_ICB) {
>> down_write(&iinfo->i_data_sem);
>> + udf_clear_extent_cache(iinfo);
>> udf_truncate_extents(inode);
>> up_write(&iinfo->i_data_sem);
>> }
>> @@ -372,7 +374,7 @@ static int udf_get_block(struct inode *inode, sector_t
>> block,
>> iinfo->i_next_alloc_goal++;
>> }
>>
>> -
>> + udf_clear_extent_cache(iinfo);
>> phys = inode_getblk(inode, block, &err, &new);
>> if (!phys)
>> goto abort;
>> @@ -1171,6 +1173,7 @@ set_size:
>> } else {
>> if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB) {
>> down_write(&iinfo->i_data_sem);
>> + udf_clear_extent_cache(iinfo);
>> memset(iinfo->i_ext.i_data + iinfo->i_lenEAttr + newsize,
>> 0x00, bsize - newsize -
>> udf_file_entry_alloc_offset(inode));
>> @@ -1184,6 +1187,7 @@ set_size:
>> if (err)
>> return err;
>> down_write(&iinfo->i_data_sem);
>> + udf_clear_extent_cache(iinfo);
>> truncate_setsize(inode, newsize);
>> udf_truncate_extents(inode);
>> up_write(&iinfo->i_data_sem);
>> @@ -1301,6 +1305,8 @@ static void udf_fill_inode(struct inode *inode,
>> struct buffer_head *bh)
>> iinfo->i_lenAlloc = 0;
>> iinfo->i_next_alloc_block = 0;
>> iinfo->i_next_alloc_goal = 0;
>> + memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
>> + mutex_init(&(iinfo->i_extent_cache_lock));
>> if (fe->descTag.tagIdent == cpu_to_le16(TAG_IDENT_EFE)) {
>> iinfo->i_efe = 1;
>> iinfo->i_use = 0;
>> @@ -2156,11 +2162,12 @@ int8_t inode_bmap(struct inode *inode, sector_t
>> block,
>> struct udf_inode_info *iinfo;
>>
>> iinfo = UDF_I(inode);
>> - pos->offset = 0;
>> - pos->block = iinfo->i_location;
>> - pos->bh = NULL;
>> + if (!udf_read_extent_cache(inode, bcount, &lbcount, pos)) {
>> + pos->offset = 0;
>> + pos->block = iinfo->i_location;
>> + pos->bh = NULL;
>> + }
>> *elen = 0;
>> -
>> do {
>> etype = udf_next_aext(inode, pos, eloc, elen, 1);
>> if (etype == -1) {
>> @@ -2170,7 +2177,8 @@ int8_t inode_bmap(struct inode *inode, sector_t
>> block,
>> }
>> lbcount += *elen;
>> } while (lbcount <= bcount);
>> -
>> + /* update extent cache */
>> + udf_update_extent_cache(inode, lbcount - *elen, pos, 1);
>> *offset = (bcount + *elen - lbcount) >> blocksize_bits;
>>
>> return etype;
>> @@ -2200,3 +2208,57 @@ long udf_block_map(struct inode *inode, sector_t
>> block)
>> else
>> return ret;
>> }
>> +int udf_read_extent_cache(struct inode *inode, loff_t bcount,
>> + loff_t *lbcount, struct extent_position *pos)
>> +{
>> + struct udf_inode_info *iinfo = UDF_I(inode);
> Use empty line after declarations please...
>
>> + mutex_lock(&iinfo->i_extent_cache_lock);
>> + if ((iinfo->cached_extent.lstart <= bcount) &&
>> + (iinfo->cached_extent.valid)) {
>> + /* Cache hit */
>> + *lbcount = iinfo->cached_extent.lstart;
>> + memcpy(pos, &iinfo->cached_extent.epos,
>> + sizeof(struct extent_position));
>> + mutex_unlock(&iinfo->i_extent_cache_lock);
>> + return 1;
>> + } else
>> + udf_clear_extent_cache(iinfo);
>> + mutex_unlock(&iinfo->i_extent_cache_lock);
>> + return 0;
>> +}
>> +void udf_update_extent_cache(struct inode *inode, loff_t estart,
>> + struct extent_position *pos, int next_epos)
>> +{
>> + struct udf_inode_info *iinfo = UDF_I(inode);
>> + mutex_lock(&iinfo->i_extent_cache_lock);
>> + if (pos->bh != NULL)
>> + /* Increase ref count */
>> + get_bh(pos->bh);
>> + memcpy(&iinfo->cached_extent.epos, pos,
>> + sizeof(struct extent_position));
>> + iinfo->cached_extent.lstart = estart;
>> + iinfo->cached_extent.valid = 1;
>> + if (next_epos)
>> + switch (iinfo->i_alloc_type) {
>> + case ICBTAG_FLAG_AD_SHORT:
>> + iinfo->cached_extent.epos.offset -=
>> + sizeof(struct short_ad);
>> + break;
>> + case ICBTAG_FLAG_AD_LONG:
>> + iinfo->cached_extent.epos.offset -=
>> + sizeof(struct long_ad);
>> + }
>> + mutex_unlock(&iinfo->i_extent_cache_lock);
>> +}
>> +
>> +/* This function should be called after i_data_sem is
>> + * held for writing OR i_extent_cache_lock is taken.
>> + */
>> +void udf_clear_extent_cache(struct udf_inode_info *iinfo)
>> +{
>> + if (iinfo->cached_extent.valid) {
>> + brelse(iinfo->cached_extent.epos.bh);
>> + memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
>> + }
>> +}
>> +
>> diff --git a/fs/udf/udf_i.h b/fs/udf/udf_i.h
>> index bb8309d..95e5e17 100644
>> --- a/fs/udf/udf_i.h
>> +++ b/fs/udf/udf_i.h
>> @@ -1,6 +1,20 @@
>> #ifndef _UDF_I_H
>> #define _UDF_I_H
>>
>> +struct extent_position {
>> + struct buffer_head *bh;
>> + uint32_t offset;
>> + struct kernel_lb_addr block;
>> +};
>> +
>> +struct udf_ext_cache {
>> + /* Extent position */
>> + struct extent_position epos;
>> + /* Start logical offset in bytes */
>> + loff_t lstart;
>> + bool valid;
> I'd just remove the 'valid' entry and use lstart == -1 as an indication
> for invalid cache.
>
>> +};
>> +
>> /*
>> * The i_data_sem and i_mutex serve for protection of allocation
>> information
>> * of a regular files and symlinks. This includes all extents belonging
>> to
>> @@ -35,6 +49,9 @@ struct udf_inode_info {
>> __u8 *i_data;
>> } i_ext;
>> struct rw_semaphore i_data_sem;
>> + struct udf_ext_cache cached_extent;
>> + /* Mutex for protecting extent cache */
>> + struct mutex i_extent_cache_lock;
> This should be spinlock not mutex. We don't need to sleep while holding
> the lock and spinlocks are both faster and smaller...
>
>> struct inode vfs_inode;
>> };
>>
>> diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
>> index de038da..db4a22e 100644
>> --- a/fs/udf/udfdecl.h
>> +++ b/fs/udf/udfdecl.h
>> @@ -113,11 +113,6 @@ struct ustr {
>> uint8_t u_len;
>> };
>>
>> -struct extent_position {
>> - struct buffer_head *bh;
>> - uint32_t offset;
>> - struct kernel_lb_addr block;
>> -};
>>
>> /* super.c */
>>
>> @@ -164,6 +159,11 @@ extern int8_t udf_next_aext(struct inode *, struct
>> extent_position *,
>> struct kernel_lb_addr *, uint32_t *, int);
>> extern int8_t udf_current_aext(struct inode *, struct extent_position *,
>> struct kernel_lb_addr *, uint32_t *, int);
>> +int udf_read_extent_cache(struct inode *inode, loff_t bcount, loff_t
>> *lbcount,
>> + struct extent_position *pos);
>> +void udf_update_extent_cache(struct inode *inode, loff_t estart,
>> + struct extent_position *pos, int next_epos);
>> +void udf_clear_extent_cache(struct udf_inode_info *iinfo);
>>
>> /* misc.c */
>> extern struct buffer_head *udf_tgetblk(struct super_block *, int);
>> --
>> 1.7.9.5
>>
> --
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] udf: add extent cache support in case of file reading
2013-02-04 10:21 ` Jan Kara
@ 2013-02-04 10:28 ` Namjae Jeon
0 siblings, 0 replies; 13+ messages in thread
From: Namjae Jeon @ 2013-02-04 10:28 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, linux-kernel, Namjae Jeon, Ashish Sangwan, Bonggil Bak
2013/2/4, Jan Kara <jack@suse.cz>:
> On Sat 02-02-13 15:21:09, Namjae Jeon wrote:
>> Hi. Jan.
>>
>> Sorry for interrupt.
>> Have you taken this patch to your tree ? I can not find it..
>> or Is there any issue regarding this patch ?
> I had it in my tree but not in the for_next branch. Did it now so you
> should see the patch in tomorrow's linux-next.
Okay, I see.
Thanks Jan!
>
> Honza
>> 2013/1/22, Namjae Jeon <linkinjeon@gmail.com>:
>> > 2013/1/22, Jan Kara <jack@suse.cz>:
>> >> On Tue 22-01-13 09:45:09, Namjae Jeon wrote:
>> >>> 2013/1/21, Jan Kara <jack@suse.cz>:
>> >>> > @@ -2222,6 +2219,8 @@ int udf_read_extent_cache(struct inode
>> >>> > *inode,
>> >>> > loff_t
>> >>> > bcount,
>> >>> > *lbcount = iinfo->cached_extent.lstart;
>> >>> > memcpy(pos, &iinfo->cached_extent.epos,
>> >>> > sizeof(struct extent_position));
>> >>> > + if (pos->bh)
>> >>> > + get_bh(pos->bh);
>> >>> > spin_unlock(&iinfo->i_extent_cache_lock);
>> >>> > return 1;
>> >>> > } else
>> >>> > This is the most important - we should give buffer reference to
>> >>> > pos->bh.
>> >>> > Caller will eventually free it right?
>> >>> This change is not required as we give buffer reference to pos->bh at
>> >>> the time of cache update.
>> >>> When we start reading a file, first we try to read the cache which
>> >>> will lead to cache miss.
>> >>> So, we would really access the pos->bh in udf_update_extent_cache for
>> >>> the first time, and this is where the buffer reference is
>> >>> incremented.
>> >>> Calling get_bh at 2 places will eventually lead to mem leak.
>> >>> Let me know your opinion.
>> >> Yes, udf_update_extent_cache() gets its own reference to bh but that
>> >> is
>> >> dropped in udf_clear_extent_cache(). So I think
>> >> udf_read_extent_cache()
>> >> needs to get a reference to the caller (as the caller will eventually
>> >> free
>> >> the bh via brelse(epos.bh) e.g. in udf_extend_file(). Also I realized
>> >> udf_update_extent_cache() needs to first clear the cache if it is
>> >> valid.
>> >> Otherwise it just overwrites bh pointer and reference is leaked. Is it
>> >> clearer now?
>> > Yes, you're right. Also, this patch looks good to me.
>> >>
>> >> I've also changed locking of udf_clear_extent_cache() so that
>> >> i_extent_cache_lock is always taken for that function - it makes the
>> >> locking rules obvious at the first sight.
>> > Yes, right. it is needed.
>> > When we test with this patch, working fine.
>> > Thanks Jan!
>> >>
>> >> Attached is the patch I currently carry.
>> >>
>> >> Honza
>> >>
>> >> --
>> >> Jan Kara <jack@suse.cz>
>> >> SUSE Labs, CR
>> >>
>> >
> --
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] udf: add extent cache support in case of file reading
2013-02-02 6:21 ` Namjae Jeon
@ 2013-02-04 10:21 ` Jan Kara
2013-02-04 10:28 ` Namjae Jeon
0 siblings, 1 reply; 13+ messages in thread
From: Jan Kara @ 2013-02-04 10:21 UTC (permalink / raw)
To: Namjae Jeon
Cc: Jan Kara, linux-fsdevel, linux-kernel, Namjae Jeon,
Ashish Sangwan, Bonggil Bak
On Sat 02-02-13 15:21:09, Namjae Jeon wrote:
> Hi. Jan.
>
> Sorry for interrupt.
> Have you taken this patch to your tree ? I can not find it..
> or Is there any issue regarding this patch ?
I had it in my tree but not in the for_next branch. Did it now so you
should see the patch in tomorrow's linux-next.
Honza
> 2013/1/22, Namjae Jeon <linkinjeon@gmail.com>:
> > 2013/1/22, Jan Kara <jack@suse.cz>:
> >> On Tue 22-01-13 09:45:09, Namjae Jeon wrote:
> >>> 2013/1/21, Jan Kara <jack@suse.cz>:
> >>> > @@ -2222,6 +2219,8 @@ int udf_read_extent_cache(struct inode *inode,
> >>> > loff_t
> >>> > bcount,
> >>> > *lbcount = iinfo->cached_extent.lstart;
> >>> > memcpy(pos, &iinfo->cached_extent.epos,
> >>> > sizeof(struct extent_position));
> >>> > + if (pos->bh)
> >>> > + get_bh(pos->bh);
> >>> > spin_unlock(&iinfo->i_extent_cache_lock);
> >>> > return 1;
> >>> > } else
> >>> > This is the most important - we should give buffer reference to
> >>> > pos->bh.
> >>> > Caller will eventually free it right?
> >>> This change is not required as we give buffer reference to pos->bh at
> >>> the time of cache update.
> >>> When we start reading a file, first we try to read the cache which
> >>> will lead to cache miss.
> >>> So, we would really access the pos->bh in udf_update_extent_cache for
> >>> the first time, and this is where the buffer reference is incremented.
> >>> Calling get_bh at 2 places will eventually lead to mem leak.
> >>> Let me know your opinion.
> >> Yes, udf_update_extent_cache() gets its own reference to bh but that is
> >> dropped in udf_clear_extent_cache(). So I think udf_read_extent_cache()
> >> needs to get a reference to the caller (as the caller will eventually
> >> free
> >> the bh via brelse(epos.bh) e.g. in udf_extend_file(). Also I realized
> >> udf_update_extent_cache() needs to first clear the cache if it is valid.
> >> Otherwise it just overwrites bh pointer and reference is leaked. Is it
> >> clearer now?
> > Yes, you're right. Also, this patch looks good to me.
> >>
> >> I've also changed locking of udf_clear_extent_cache() so that
> >> i_extent_cache_lock is always taken for that function - it makes the
> >> locking rules obvious at the first sight.
> > Yes, right. it is needed.
> > When we test with this patch, working fine.
> > Thanks Jan!
> >>
> >> Attached is the patch I currently carry.
> >>
> >> Honza
> >>
> >> --
> >> Jan Kara <jack@suse.cz>
> >> SUSE Labs, CR
> >>
> >
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] udf: add extent cache support in case of file reading
2013-01-22 11:49 ` Namjae Jeon
@ 2013-02-02 6:21 ` Namjae Jeon
2013-02-04 10:21 ` Jan Kara
0 siblings, 1 reply; 13+ messages in thread
From: Namjae Jeon @ 2013-02-02 6:21 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, linux-kernel, Namjae Jeon, Ashish Sangwan, Bonggil Bak
Hi. Jan.
Sorry for interrupt.
Have you taken this patch to your tree ? I can not find it..
or Is there any issue regarding this patch ?
Thanks!
2013/1/22, Namjae Jeon <linkinjeon@gmail.com>:
> 2013/1/22, Jan Kara <jack@suse.cz>:
>> On Tue 22-01-13 09:45:09, Namjae Jeon wrote:
>>> 2013/1/21, Jan Kara <jack@suse.cz>:
>>> > @@ -2222,6 +2219,8 @@ int udf_read_extent_cache(struct inode *inode,
>>> > loff_t
>>> > bcount,
>>> > *lbcount = iinfo->cached_extent.lstart;
>>> > memcpy(pos, &iinfo->cached_extent.epos,
>>> > sizeof(struct extent_position));
>>> > + if (pos->bh)
>>> > + get_bh(pos->bh);
>>> > spin_unlock(&iinfo->i_extent_cache_lock);
>>> > return 1;
>>> > } else
>>> > This is the most important - we should give buffer reference to
>>> > pos->bh.
>>> > Caller will eventually free it right?
>>> This change is not required as we give buffer reference to pos->bh at
>>> the time of cache update.
>>> When we start reading a file, first we try to read the cache which
>>> will lead to cache miss.
>>> So, we would really access the pos->bh in udf_update_extent_cache for
>>> the first time, and this is where the buffer reference is incremented.
>>> Calling get_bh at 2 places will eventually lead to mem leak.
>>> Let me know your opinion.
>> Yes, udf_update_extent_cache() gets its own reference to bh but that is
>> dropped in udf_clear_extent_cache(). So I think udf_read_extent_cache()
>> needs to get a reference to the caller (as the caller will eventually
>> free
>> the bh via brelse(epos.bh) e.g. in udf_extend_file(). Also I realized
>> udf_update_extent_cache() needs to first clear the cache if it is valid.
>> Otherwise it just overwrites bh pointer and reference is leaked. Is it
>> clearer now?
> Yes, you're right. Also, this patch looks good to me.
>>
>> I've also changed locking of udf_clear_extent_cache() so that
>> i_extent_cache_lock is always taken for that function - it makes the
>> locking rules obvious at the first sight.
> Yes, right. it is needed.
> When we test with this patch, working fine.
> Thanks Jan!
>>
>> Attached is the patch I currently carry.
>>
>> Honza
>>
>> --
>> Jan Kara <jack@suse.cz>
>> SUSE Labs, CR
>>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] udf: add extent cache support in case of file reading
2013-01-22 10:04 ` Jan Kara
@ 2013-01-22 11:49 ` Namjae Jeon
2013-02-02 6:21 ` Namjae Jeon
0 siblings, 1 reply; 13+ messages in thread
From: Namjae Jeon @ 2013-01-22 11:49 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, linux-kernel, Namjae Jeon, Ashish Sangwan, Bonggil Bak
2013/1/22, Jan Kara <jack@suse.cz>:
> On Tue 22-01-13 09:45:09, Namjae Jeon wrote:
>> 2013/1/21, Jan Kara <jack@suse.cz>:
>> > @@ -2222,6 +2219,8 @@ int udf_read_extent_cache(struct inode *inode,
>> > loff_t
>> > bcount,
>> > *lbcount = iinfo->cached_extent.lstart;
>> > memcpy(pos, &iinfo->cached_extent.epos,
>> > sizeof(struct extent_position));
>> > + if (pos->bh)
>> > + get_bh(pos->bh);
>> > spin_unlock(&iinfo->i_extent_cache_lock);
>> > return 1;
>> > } else
>> > This is the most important - we should give buffer reference to
>> > pos->bh.
>> > Caller will eventually free it right?
>> This change is not required as we give buffer reference to pos->bh at
>> the time of cache update.
>> When we start reading a file, first we try to read the cache which
>> will lead to cache miss.
>> So, we would really access the pos->bh in udf_update_extent_cache for
>> the first time, and this is where the buffer reference is incremented.
>> Calling get_bh at 2 places will eventually lead to mem leak.
>> Let me know your opinion.
> Yes, udf_update_extent_cache() gets its own reference to bh but that is
> dropped in udf_clear_extent_cache(). So I think udf_read_extent_cache()
> needs to get a reference to the caller (as the caller will eventually free
> the bh via brelse(epos.bh) e.g. in udf_extend_file(). Also I realized
> udf_update_extent_cache() needs to first clear the cache if it is valid.
> Otherwise it just overwrites bh pointer and reference is leaked. Is it
> clearer now?
Yes, you're right. Also, this patch looks good to me.
>
> I've also changed locking of udf_clear_extent_cache() so that
> i_extent_cache_lock is always taken for that function - it makes the
> locking rules obvious at the first sight.
Yes, right. it is needed.
When we test with this patch, working fine.
Thanks Jan!
>
> Attached is the patch I currently carry.
>
> Honza
>
> --
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] udf: add extent cache support in case of file reading
2013-01-22 0:45 ` Namjae Jeon
@ 2013-01-22 10:04 ` Jan Kara
2013-01-22 11:49 ` Namjae Jeon
0 siblings, 1 reply; 13+ messages in thread
From: Jan Kara @ 2013-01-22 10:04 UTC (permalink / raw)
To: Namjae Jeon
Cc: Jan Kara, linux-fsdevel, linux-kernel, Namjae Jeon,
Ashish Sangwan, Bonggil Bak
[-- Attachment #1: Type: text/plain, Size: 1721 bytes --]
On Tue 22-01-13 09:45:09, Namjae Jeon wrote:
> 2013/1/21, Jan Kara <jack@suse.cz>:
> > @@ -2222,6 +2219,8 @@ int udf_read_extent_cache(struct inode *inode, loff_t
> > bcount,
> > *lbcount = iinfo->cached_extent.lstart;
> > memcpy(pos, &iinfo->cached_extent.epos,
> > sizeof(struct extent_position));
> > + if (pos->bh)
> > + get_bh(pos->bh);
> > spin_unlock(&iinfo->i_extent_cache_lock);
> > return 1;
> > } else
> > This is the most important - we should give buffer reference to pos->bh.
> > Caller will eventually free it right?
> This change is not required as we give buffer reference to pos->bh at
> the time of cache update.
> When we start reading a file, first we try to read the cache which
> will lead to cache miss.
> So, we would really access the pos->bh in udf_update_extent_cache for
> the first time, and this is where the buffer reference is incremented.
> Calling get_bh at 2 places will eventually lead to mem leak.
> Let me know your opinion.
Yes, udf_update_extent_cache() gets its own reference to bh but that is
dropped in udf_clear_extent_cache(). So I think udf_read_extent_cache()
needs to get a reference to the caller (as the caller will eventually free
the bh via brelse(epos.bh) e.g. in udf_extend_file(). Also I realized
udf_update_extent_cache() needs to first clear the cache if it is valid.
Otherwise it just overwrites bh pointer and reference is leaked. Is it
clearer now?
I've also changed locking of udf_clear_extent_cache() so that
i_extent_cache_lock is always taken for that function - it makes the
locking rules obvious at the first sight.
Attached is the patch I currently carry.
Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
[-- Attachment #2: 0001-udf-add-extent-cache-support-in-case-of-file-reading.patch --]
[-- Type: text/x-patch, Size: 7355 bytes --]
>From 99600051b04bc4ec8bd4d16a8bf993ca54042db6 Mon Sep 17 00:00:00 2001
From: Namjae Jeon <namjae.jeon@samsung.com>
Date: Sat, 19 Jan 2013 11:17:14 +0900
Subject: [PATCH] udf: add extent cache support in case of file reading
This patch implements extent caching in case of file reading.
While reading a file, currently, UDF reads metadata serially
which takes a lot of time depending on the number of extents present
in the file. Caching last accessd extent improves metadata read time.
Instead of reading file metadata from start, now we read from
the cached extent.
This patch considerably improves the time spent by CPU in kernel mode.
For example, while reading a 10.9 GB file using dd:
Time before applying patch:
11677022208 bytes (10.9GB) copied, 1529.748921 seconds, 7.3MB/s
real 25m 29.85s
user 0m 12.41s
sys 15m 34.75s
Time after applying patch:
11677022208 bytes (10.9GB) copied, 1469.338231 seconds, 7.6MB/s
real 24m 29.44s
user 0m 15.73s
sys 3m 27.61s
[JK: Fix bh refcounting issues, simplify initialization]
Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Ashish Sangwan <a.sangwan@samsung.com>
Signed-off-by: Bonggil Bak <bgbak@samsung.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
fs/udf/inode.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++----
fs/udf/super.c | 2 +
fs/udf/udf_i.h | 16 ++++++++++
fs/udf/udfdecl.h | 5 ---
4 files changed, 98 insertions(+), 11 deletions(-)
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index cbae1ed..7a12e48 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -67,6 +67,74 @@ static void udf_update_extents(struct inode *,
struct extent_position *);
static int udf_get_block(struct inode *, sector_t, struct buffer_head *, int);
+static void __udf_clear_extent_cache(struct inode *inode)
+{
+ struct udf_inode_info *iinfo = UDF_I(inode);
+
+ if (iinfo->cached_extent.lstart != -1) {
+ brelse(iinfo->cached_extent.epos.bh);
+ iinfo->cached_extent.lstart = -1;
+ }
+}
+
+/* Invalidate extent cache */
+static void udf_clear_extent_cache(struct inode *inode)
+{
+ struct udf_inode_info *iinfo = UDF_I(inode);
+
+ spin_lock(&iinfo->i_extent_cache_lock);
+ __udf_clear_extent_cache(inode);
+ spin_unlock(&iinfo->i_extent_cache_lock);
+}
+
+/* Return contents of extent cache */
+static int udf_read_extent_cache(struct inode *inode, loff_t bcount,
+ loff_t *lbcount, struct extent_position *pos)
+{
+ struct udf_inode_info *iinfo = UDF_I(inode);
+ int ret = 0;
+
+ spin_lock(&iinfo->i_extent_cache_lock);
+ if ((iinfo->cached_extent.lstart <= bcount) &&
+ (iinfo->cached_extent.lstart != -1)) {
+ /* Cache hit */
+ *lbcount = iinfo->cached_extent.lstart;
+ memcpy(pos, &iinfo->cached_extent.epos,
+ sizeof(struct extent_position));
+ if (pos->bh)
+ get_bh(pos->bh);
+ ret = 1;
+ }
+ spin_unlock(&iinfo->i_extent_cache_lock);
+ return ret;
+}
+
+/* Add extent to extent cache */
+static void udf_update_extent_cache(struct inode *inode, loff_t estart,
+ struct extent_position *pos, int next_epos)
+{
+ struct udf_inode_info *iinfo = UDF_I(inode);
+
+ spin_lock(&iinfo->i_extent_cache_lock);
+ /* Invalidate previously cached extent */
+ __udf_clear_extent_cache(inode);
+ if (pos->bh)
+ get_bh(pos->bh);
+ memcpy(&iinfo->cached_extent.epos, pos,
+ sizeof(struct extent_position));
+ iinfo->cached_extent.lstart = estart;
+ if (next_epos)
+ switch (iinfo->i_alloc_type) {
+ case ICBTAG_FLAG_AD_SHORT:
+ iinfo->cached_extent.epos.offset -=
+ sizeof(struct short_ad);
+ break;
+ case ICBTAG_FLAG_AD_LONG:
+ iinfo->cached_extent.epos.offset -=
+ sizeof(struct long_ad);
+ }
+ spin_unlock(&iinfo->i_extent_cache_lock);
+}
void udf_evict_inode(struct inode *inode)
{
@@ -90,6 +158,7 @@ void udf_evict_inode(struct inode *inode)
}
kfree(iinfo->i_ext.i_data);
iinfo->i_ext.i_data = NULL;
+ udf_clear_extent_cache(inode);
if (want_delete) {
udf_free_inode(inode);
}
@@ -105,6 +174,7 @@ static void udf_write_failed(struct address_space *mapping, loff_t to)
truncate_pagecache(inode, to, isize);
if (iinfo->i_alloc_type != ICBTAG_FLAG_AD_IN_ICB) {
down_write(&iinfo->i_data_sem);
+ udf_clear_extent_cache(inode);
udf_truncate_extents(inode);
up_write(&iinfo->i_data_sem);
}
@@ -372,7 +442,7 @@ static int udf_get_block(struct inode *inode, sector_t block,
iinfo->i_next_alloc_goal++;
}
-
+ udf_clear_extent_cache(inode);
phys = inode_getblk(inode, block, &err, &new);
if (!phys)
goto abort;
@@ -1171,6 +1241,7 @@ set_size:
} else {
if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB) {
down_write(&iinfo->i_data_sem);
+ udf_clear_extent_cache(inode);
memset(iinfo->i_ext.i_data + iinfo->i_lenEAttr + newsize,
0x00, bsize - newsize -
udf_file_entry_alloc_offset(inode));
@@ -1184,6 +1255,7 @@ set_size:
if (err)
return err;
down_write(&iinfo->i_data_sem);
+ udf_clear_extent_cache(inode);
truncate_setsize(inode, newsize);
udf_truncate_extents(inode);
up_write(&iinfo->i_data_sem);
@@ -2156,11 +2228,12 @@ int8_t inode_bmap(struct inode *inode, sector_t block,
struct udf_inode_info *iinfo;
iinfo = UDF_I(inode);
- pos->offset = 0;
- pos->block = iinfo->i_location;
- pos->bh = NULL;
+ if (!udf_read_extent_cache(inode, bcount, &lbcount, pos)) {
+ pos->offset = 0;
+ pos->block = iinfo->i_location;
+ pos->bh = NULL;
+ }
*elen = 0;
-
do {
etype = udf_next_aext(inode, pos, eloc, elen, 1);
if (etype == -1) {
@@ -2170,7 +2243,8 @@ int8_t inode_bmap(struct inode *inode, sector_t block,
}
lbcount += *elen;
} while (lbcount <= bcount);
-
+ /* update extent cache */
+ udf_update_extent_cache(inode, lbcount - *elen, pos, 1);
*offset = (bcount + *elen - lbcount) >> blocksize_bits;
return etype;
diff --git a/fs/udf/super.c b/fs/udf/super.c
index 186adbf..da8ce9f 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -134,6 +134,8 @@ static struct inode *udf_alloc_inode(struct super_block *sb)
ei->i_next_alloc_goal = 0;
ei->i_strat4096 = 0;
init_rwsem(&ei->i_data_sem);
+ ei->cached_extent.lstart = -1;
+ spin_lock_init(&ei->i_extent_cache_lock);
return &ei->vfs_inode;
}
diff --git a/fs/udf/udf_i.h b/fs/udf/udf_i.h
index bb8309d..b5cd8ed 100644
--- a/fs/udf/udf_i.h
+++ b/fs/udf/udf_i.h
@@ -1,6 +1,19 @@
#ifndef _UDF_I_H
#define _UDF_I_H
+struct extent_position {
+ struct buffer_head *bh;
+ uint32_t offset;
+ struct kernel_lb_addr block;
+};
+
+struct udf_ext_cache {
+ /* Extent position */
+ struct extent_position epos;
+ /* Start logical offset in bytes */
+ loff_t lstart;
+};
+
/*
* The i_data_sem and i_mutex serve for protection of allocation information
* of a regular files and symlinks. This includes all extents belonging to
@@ -35,6 +48,9 @@ struct udf_inode_info {
__u8 *i_data;
} i_ext;
struct rw_semaphore i_data_sem;
+ struct udf_ext_cache cached_extent;
+ /* Spinlock for protecting extent cache */
+ spinlock_t i_extent_cache_lock;
struct inode vfs_inode;
};
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index de038da..be7dabb 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -113,11 +113,6 @@ struct ustr {
uint8_t u_len;
};
-struct extent_position {
- struct buffer_head *bh;
- uint32_t offset;
- struct kernel_lb_addr block;
-};
/* super.c */
--
1.7.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] udf: add extent cache support in case of file reading
2013-01-21 11:04 ` Jan Kara
@ 2013-01-22 0:45 ` Namjae Jeon
2013-01-22 10:04 ` Jan Kara
0 siblings, 1 reply; 13+ messages in thread
From: Namjae Jeon @ 2013-01-22 0:45 UTC (permalink / raw)
To: Jan Kara
Cc: linux-fsdevel, linux-kernel, Namjae Jeon, Ashish Sangwan, Bonggil Bak
2013/1/21, Jan Kara <jack@suse.cz>:
> On Sat 19-01-13 11:17:14, Namjae Jeon wrote:
>> From: Namjae Jeon <namjae.jeon@samsung.com>
>>
>> This patch implements extent caching in case of file reading.
>> While reading a file, currently, UDF reads metadata serially
>> which takes a lot of time depending on the number of extents present
>> in the file. Caching last accessd extent improves metadata read time.
>> Instead of reading file metadata from start, now we read from
>> the cached extent.
>>
>> This patch considerably improves the time spent by CPU in kernel mode.
>> For example, while reading a 10.9 GB file using dd:
>> Time before applying patch:
>> 11677022208 bytes (10.9GB) copied, 1529.748921 seconds, 7.3MB/s
>> real 25m 29.85s
>> user 0m 12.41s
>> sys 15m 34.75s
>>
>> Time after applying patch:
>> 11677022208 bytes (10.9GB) copied, 1469.338231 seconds, 7.6MB/s
>> real 24m 29.44s
>> user 0m 15.73s
>> sys 3m 27.61s
>>
>> Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
>> Signed-off-by: Ashish Sangwan <a.sangwan@samsung.com>
>> Signed-off-by: Bonggil Bak <bgbak@samsung.com>
> Thanks for the patch Namjae. I did a few more changes to the patch.
> Please check them whether you think they are OK.
>
> diff --git a/fs/udf/ialloc.c b/fs/udf/ialloc.c
> index 0cb208e..7e5aae4 100644
> --- a/fs/udf/ialloc.c
> +++ b/fs/udf/ialloc.c
> @@ -117,10 +117,6 @@ struct inode *udf_new_inode(struct inode *dir, umode_t
> mode, int *err)
> iinfo->i_lenAlloc = 0;
> iinfo->i_use = 0;
> iinfo->i_checkpoint = 1;
> - memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
> - spin_lock_init(&(iinfo->i_extent_cache_lock));
> - /* Mark extent cache as invalid for now */
> - iinfo->cached_extent.lstart = -1;
> if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_AD_IN_ICB))
> iinfo->i_alloc_type = ICBTAG_FLAG_AD_IN_ICB;
> else if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_SHORT_AD))
> diff --git a/fs/udf/inode.c b/fs/udf/inode.c
> index 8494b8c..fb0c4c4 100644
> --- a/fs/udf/inode.c
> +++ b/fs/udf/inode.c
> @@ -1305,9 +1305,6 @@ static void udf_fill_inode(struct inode *inode, struct
> buffer_head *bh)
> iinfo->i_lenAlloc = 0;
> iinfo->i_next_alloc_block = 0;
> iinfo->i_next_alloc_goal = 0;
> - memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
> - spin_lock_init(&(iinfo->i_extent_cache_lock));
> - iinfo->cached_extent.lstart = -1;
> if (fe->descTag.tagIdent == cpu_to_le16(TAG_IDENT_EFE)) {
> iinfo->i_efe = 1;
> iinfo->i_use = 0;
>
Hi Jan.
> Initialization now happens in udf_alloc_inode(). Also it's not necessary
> to initialized cached_extent.epos when lstart == -1 - noone should look at
> that.
This change is okay.
>
> @@ -2222,6 +2219,8 @@ int udf_read_extent_cache(struct inode *inode, loff_t
> bcount,
> *lbcount = iinfo->cached_extent.lstart;
> memcpy(pos, &iinfo->cached_extent.epos,
> sizeof(struct extent_position));
> + if (pos->bh)
> + get_bh(pos->bh);
> spin_unlock(&iinfo->i_extent_cache_lock);
> return 1;
> } else
> This is the most important - we should give buffer reference to pos->bh.
> Caller will eventually free it right?
This change is not required as we give buffer reference to pos->bh at
the time of cache update.
When we start reading a file, first we try to read the cache which
will lead to cache miss.
So, we would really access the pos->bh in udf_update_extent_cache for
the first time, and this is where the buffer reference is incremented.
Calling get_bh at 2 places will eventually lead to mem leak.
Let me know your opinion.
Thanks for review and change!
>
> @@ -2236,8 +2235,7 @@ void udf_update_extent_cache(struct inode *inode,
> loff_t estart,
> struct udf_inode_info *iinfo = UDF_I(inode);
>
> spin_lock(&iinfo->i_extent_cache_lock);
> - if (pos->bh != NULL)
> - /* Increase ref count */
> + if (pos->bh)
> get_bh(pos->bh);
> memcpy(&iinfo->cached_extent.epos, pos,
> sizeof(struct extent_position));
> @@ -2266,4 +2264,3 @@ void udf_clear_extent_cache(struct udf_inode_info
> *iinfo)
> iinfo->cached_extent.lstart = -1;
> }
> }
> -
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index 186adbf..da8ce9f 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -134,6 +134,8 @@ static struct inode *udf_alloc_inode(struct super_block
> *sb)
> ei->i_next_alloc_goal = 0;
> ei->i_strat4096 = 0;
> init_rwsem(&ei->i_data_sem);
> + ei->cached_extent.lstart = -1;
> + spin_lock_init(&ei->i_extent_cache_lock);
>
> return &ei->vfs_inode;
> }
>
> Honza
> --
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] udf: add extent cache support in case of file reading
2013-01-19 2:17 Namjae Jeon
2013-01-19 2:29 ` Cong Ding
@ 2013-01-21 11:04 ` Jan Kara
2013-01-22 0:45 ` Namjae Jeon
1 sibling, 1 reply; 13+ messages in thread
From: Jan Kara @ 2013-01-21 11:04 UTC (permalink / raw)
To: Namjae Jeon
Cc: jack, linux-fsdevel, linux-kernel, Namjae Jeon, Ashish Sangwan,
Bonggil Bak
On Sat 19-01-13 11:17:14, Namjae Jeon wrote:
> From: Namjae Jeon <namjae.jeon@samsung.com>
>
> This patch implements extent caching in case of file reading.
> While reading a file, currently, UDF reads metadata serially
> which takes a lot of time depending on the number of extents present
> in the file. Caching last accessd extent improves metadata read time.
> Instead of reading file metadata from start, now we read from
> the cached extent.
>
> This patch considerably improves the time spent by CPU in kernel mode.
> For example, while reading a 10.9 GB file using dd:
> Time before applying patch:
> 11677022208 bytes (10.9GB) copied, 1529.748921 seconds, 7.3MB/s
> real 25m 29.85s
> user 0m 12.41s
> sys 15m 34.75s
>
> Time after applying patch:
> 11677022208 bytes (10.9GB) copied, 1469.338231 seconds, 7.6MB/s
> real 24m 29.44s
> user 0m 15.73s
> sys 3m 27.61s
>
> Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
> Signed-off-by: Ashish Sangwan <a.sangwan@samsung.com>
> Signed-off-by: Bonggil Bak <bgbak@samsung.com>
Thanks for the patch Namjae. I did a few more changes to the patch.
Please check them whether you think they are OK.
diff --git a/fs/udf/ialloc.c b/fs/udf/ialloc.c
index 0cb208e..7e5aae4 100644
--- a/fs/udf/ialloc.c
+++ b/fs/udf/ialloc.c
@@ -117,10 +117,6 @@ struct inode *udf_new_inode(struct inode *dir, umode_t mode, int *err)
iinfo->i_lenAlloc = 0;
iinfo->i_use = 0;
iinfo->i_checkpoint = 1;
- memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
- spin_lock_init(&(iinfo->i_extent_cache_lock));
- /* Mark extent cache as invalid for now */
- iinfo->cached_extent.lstart = -1;
if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_AD_IN_ICB))
iinfo->i_alloc_type = ICBTAG_FLAG_AD_IN_ICB;
else if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_SHORT_AD))
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 8494b8c..fb0c4c4 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -1305,9 +1305,6 @@ static void udf_fill_inode(struct inode *inode, struct buffer_head *bh)
iinfo->i_lenAlloc = 0;
iinfo->i_next_alloc_block = 0;
iinfo->i_next_alloc_goal = 0;
- memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
- spin_lock_init(&(iinfo->i_extent_cache_lock));
- iinfo->cached_extent.lstart = -1;
if (fe->descTag.tagIdent == cpu_to_le16(TAG_IDENT_EFE)) {
iinfo->i_efe = 1;
iinfo->i_use = 0;
Initialization now happens in udf_alloc_inode(). Also it's not necessary
to initialized cached_extent.epos when lstart == -1 - noone should look at
that.
@@ -2222,6 +2219,8 @@ int udf_read_extent_cache(struct inode *inode, loff_t bcount,
*lbcount = iinfo->cached_extent.lstart;
memcpy(pos, &iinfo->cached_extent.epos,
sizeof(struct extent_position));
+ if (pos->bh)
+ get_bh(pos->bh);
spin_unlock(&iinfo->i_extent_cache_lock);
return 1;
} else
This is the most important - we should give buffer reference to pos->bh.
Caller will eventually free it right?
@@ -2236,8 +2235,7 @@ void udf_update_extent_cache(struct inode *inode, loff_t estart,
struct udf_inode_info *iinfo = UDF_I(inode);
spin_lock(&iinfo->i_extent_cache_lock);
- if (pos->bh != NULL)
- /* Increase ref count */
+ if (pos->bh)
get_bh(pos->bh);
memcpy(&iinfo->cached_extent.epos, pos,
sizeof(struct extent_position));
@@ -2266,4 +2264,3 @@ void udf_clear_extent_cache(struct udf_inode_info *iinfo)
iinfo->cached_extent.lstart = -1;
}
}
-
diff --git a/fs/udf/super.c b/fs/udf/super.c
index 186adbf..da8ce9f 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -134,6 +134,8 @@ static struct inode *udf_alloc_inode(struct super_block *sb)
ei->i_next_alloc_goal = 0;
ei->i_strat4096 = 0;
init_rwsem(&ei->i_data_sem);
+ ei->cached_extent.lstart = -1;
+ spin_lock_init(&ei->i_extent_cache_lock);
return &ei->vfs_inode;
}
Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] udf: add extent cache support in case of file reading
2013-01-19 2:29 ` Cong Ding
@ 2013-01-21 2:18 ` Namjae Jeon
0 siblings, 0 replies; 13+ messages in thread
From: Namjae Jeon @ 2013-01-21 2:18 UTC (permalink / raw)
To: Cong Ding
Cc: jack, linux-fsdevel, linux-kernel, Namjae Jeon, Ashish Sangwan,
Bonggil Bak
2013/1/19, Cong Ding <dinggnu@gmail.com>:
> On Sat, Jan 19, 2013 at 11:17:14AM +0900, Namjae Jeon wrote:
>> From: Namjae Jeon <namjae.jeon@samsung.com>
>>
>> This patch implements extent caching in case of file reading.
>> While reading a file, currently, UDF reads metadata serially
>> which takes a lot of time depending on the number of extents present
>> in the file. Caching last accessd extent improves metadata read time.
>> Instead of reading file metadata from start, now we read from
>> the cached extent.
>>
>> This patch considerably improves the time spent by CPU in kernel mode.
>> For example, while reading a 10.9 GB file using dd:
>> Time before applying patch:
>> 11677022208 bytes (10.9GB) copied, 1529.748921 seconds, 7.3MB/s
>> real 25m 29.85s
>> user 0m 12.41s
>> sys 15m 34.75s
>>
>> Time after applying patch:
>> 11677022208 bytes (10.9GB) copied, 1469.338231 seconds, 7.6MB/s
>> real 24m 29.44s
>> user 0m 15.73s
>> sys 3m 27.61s
> did you have any test on lots of small files?
Hi. Cong.
I created 2048 files of each 512KB size for testing performance
drpping by extent cache feature.
Used this script to read every file:
index=0
while [ $index != 2048 ]
do
dd if=file.$index of=/dev/zero 1> /dev/null 2>/dev/null
index=$(($index + 1))
done
Performance without patch:
VDLinux#> echo 3 > /proc/sys/vm/drop_caches
VDLinux#> time ./script2.sh
real 0m 55.13s
user 0m 1.40s
sys 0m 25.17s
Performace with patch =>
VDLinux#> time ./script2.sh
real 0m 53.70s
user 0m 1.60s
sys 0m 25.11s
I can not find any performance dropping with extent cache patch.
Thanks.
>
> - cong
>>
>> Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
>> Signed-off-by: Ashish Sangwan <a.sangwan@samsung.com>
>> Signed-off-by: Bonggil Bak <bgbak@samsung.com>
>> ---
>> fs/udf/ialloc.c | 4 +++
>> fs/udf/inode.c | 79
>> +++++++++++++++++++++++++++++++++++++++++++++++++-----
>> fs/udf/udf_i.h | 16 +++++++++++
>> fs/udf/udfdecl.h | 10 +++----
>> 4 files changed, 98 insertions(+), 11 deletions(-)
>>
>> diff --git a/fs/udf/ialloc.c b/fs/udf/ialloc.c
>> index 7e5aae4..0cb208e 100644
>> --- a/fs/udf/ialloc.c
>> +++ b/fs/udf/ialloc.c
>> @@ -117,6 +117,10 @@ struct inode *udf_new_inode(struct inode *dir,
>> umode_t mode, int *err)
>> iinfo->i_lenAlloc = 0;
>> iinfo->i_use = 0;
>> iinfo->i_checkpoint = 1;
>> + memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
>> + spin_lock_init(&(iinfo->i_extent_cache_lock));
>> + /* Mark extent cache as invalid for now */
>> + iinfo->cached_extent.lstart = -1;
>> if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_AD_IN_ICB))
>> iinfo->i_alloc_type = ICBTAG_FLAG_AD_IN_ICB;
>> else if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_SHORT_AD))
>> diff --git a/fs/udf/inode.c b/fs/udf/inode.c
>> index e78ef48..86e0469 100644
>> --- a/fs/udf/inode.c
>> +++ b/fs/udf/inode.c
>> @@ -91,6 +91,7 @@ void udf_evict_inode(struct inode *inode)
>> }
>> kfree(iinfo->i_ext.i_data);
>> iinfo->i_ext.i_data = NULL;
>> + udf_clear_extent_cache(iinfo);
>> if (want_delete) {
>> udf_free_inode(inode);
>> }
>> @@ -106,6 +107,7 @@ static void udf_write_failed(struct address_space
>> *mapping, loff_t to)
>> truncate_pagecache(inode, to, isize);
>> if (iinfo->i_alloc_type != ICBTAG_FLAG_AD_IN_ICB) {
>> down_write(&iinfo->i_data_sem);
>> + udf_clear_extent_cache(iinfo);
>> udf_truncate_extents(inode);
>> up_write(&iinfo->i_data_sem);
>> }
>> @@ -373,7 +375,7 @@ static int udf_get_block(struct inode *inode, sector_t
>> block,
>> iinfo->i_next_alloc_goal++;
>> }
>>
>> -
>> + udf_clear_extent_cache(iinfo);
>> phys = inode_getblk(inode, block, &err, &new);
>> if (!phys)
>> goto abort;
>> @@ -1172,6 +1174,7 @@ set_size:
>> } else {
>> if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB) {
>> down_write(&iinfo->i_data_sem);
>> + udf_clear_extent_cache(iinfo);
>> memset(iinfo->i_ext.i_data + iinfo->i_lenEAttr + newsize,
>> 0x00, bsize - newsize -
>> udf_file_entry_alloc_offset(inode));
>> @@ -1185,6 +1188,7 @@ set_size:
>> if (err)
>> return err;
>> down_write(&iinfo->i_data_sem);
>> + udf_clear_extent_cache(iinfo);
>> truncate_setsize(inode, newsize);
>> udf_truncate_extents(inode);
>> up_write(&iinfo->i_data_sem);
>> @@ -1302,6 +1306,9 @@ static void udf_fill_inode(struct inode *inode,
>> struct buffer_head *bh)
>> iinfo->i_lenAlloc = 0;
>> iinfo->i_next_alloc_block = 0;
>> iinfo->i_next_alloc_goal = 0;
>> + memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
>> + spin_lock_init(&(iinfo->i_extent_cache_lock));
>> + iinfo->cached_extent.lstart = -1;
>> if (fe->descTag.tagIdent == cpu_to_le16(TAG_IDENT_EFE)) {
>> iinfo->i_efe = 1;
>> iinfo->i_use = 0;
>> @@ -2157,11 +2164,12 @@ int8_t inode_bmap(struct inode *inode, sector_t
>> block,
>> struct udf_inode_info *iinfo;
>>
>> iinfo = UDF_I(inode);
>> - pos->offset = 0;
>> - pos->block = iinfo->i_location;
>> - pos->bh = NULL;
>> + if (!udf_read_extent_cache(inode, bcount, &lbcount, pos)) {
>> + pos->offset = 0;
>> + pos->block = iinfo->i_location;
>> + pos->bh = NULL;
>> + }
>> *elen = 0;
>> -
>> do {
>> etype = udf_next_aext(inode, pos, eloc, elen, 1);
>> if (etype == -1) {
>> @@ -2171,7 +2179,8 @@ int8_t inode_bmap(struct inode *inode, sector_t
>> block,
>> }
>> lbcount += *elen;
>> } while (lbcount <= bcount);
>> -
>> + /* update extent cache */
>> + udf_update_extent_cache(inode, lbcount - *elen, pos, 1);
>> *offset = (bcount + *elen - lbcount) >> blocksize_bits;
>>
>> return etype;
>> @@ -2201,3 +2210,61 @@ long udf_block_map(struct inode *inode, sector_t
>> block)
>> else
>> return ret;
>> }
>> +
>> +int udf_read_extent_cache(struct inode *inode, loff_t bcount,
>> + loff_t *lbcount, struct extent_position *pos)
>> +{
>> + struct udf_inode_info *iinfo = UDF_I(inode);
>> +
>> + spin_lock(&iinfo->i_extent_cache_lock);
>> + if ((iinfo->cached_extent.lstart <= bcount) &&
>> + (iinfo->cached_extent.lstart != -1)) {
>> + /* Cache hit */
>> + *lbcount = iinfo->cached_extent.lstart;
>> + memcpy(pos, &iinfo->cached_extent.epos,
>> + sizeof(struct extent_position));
>> + spin_unlock(&iinfo->i_extent_cache_lock);
>> + return 1;
>> + } else
>> + udf_clear_extent_cache(iinfo);
>> + spin_unlock(&iinfo->i_extent_cache_lock);
>> + return 0;
>> +}
>> +
>> +void udf_update_extent_cache(struct inode *inode, loff_t estart,
>> + struct extent_position *pos, int next_epos)
>> +{
>> + struct udf_inode_info *iinfo = UDF_I(inode);
>> +
>> + spin_lock(&iinfo->i_extent_cache_lock);
>> + if (pos->bh != NULL)
>> + /* Increase ref count */
>> + get_bh(pos->bh);
>> + memcpy(&iinfo->cached_extent.epos, pos,
>> + sizeof(struct extent_position));
>> + iinfo->cached_extent.lstart = estart;
>> + if (next_epos)
>> + switch (iinfo->i_alloc_type) {
>> + case ICBTAG_FLAG_AD_SHORT:
>> + iinfo->cached_extent.epos.offset -=
>> + sizeof(struct short_ad);
>> + break;
>> + case ICBTAG_FLAG_AD_LONG:
>> + iinfo->cached_extent.epos.offset -=
>> + sizeof(struct long_ad);
>> + }
>> + spin_unlock(&iinfo->i_extent_cache_lock);
>> +}
>> +
>> +/* This function should be called after i_data_sem is
>> + * held for writing OR i_extent_cache_lock is taken.
>> + */
>> +void udf_clear_extent_cache(struct udf_inode_info *iinfo)
>> +{
>> + if (iinfo->cached_extent.lstart != -1) {
>> + brelse(iinfo->cached_extent.epos.bh);
>> + memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
>> + iinfo->cached_extent.lstart = -1;
>> + }
>> +}
>> +
>> diff --git a/fs/udf/udf_i.h b/fs/udf/udf_i.h
>> index bb8309d..b5cd8ed 100644
>> --- a/fs/udf/udf_i.h
>> +++ b/fs/udf/udf_i.h
>> @@ -1,6 +1,19 @@
>> #ifndef _UDF_I_H
>> #define _UDF_I_H
>>
>> +struct extent_position {
>> + struct buffer_head *bh;
>> + uint32_t offset;
>> + struct kernel_lb_addr block;
>> +};
>> +
>> +struct udf_ext_cache {
>> + /* Extent position */
>> + struct extent_position epos;
>> + /* Start logical offset in bytes */
>> + loff_t lstart;
>> +};
>> +
>> /*
>> * The i_data_sem and i_mutex serve for protection of allocation
>> information
>> * of a regular files and symlinks. This includes all extents belonging
>> to
>> @@ -35,6 +48,9 @@ struct udf_inode_info {
>> __u8 *i_data;
>> } i_ext;
>> struct rw_semaphore i_data_sem;
>> + struct udf_ext_cache cached_extent;
>> + /* Spinlock for protecting extent cache */
>> + spinlock_t i_extent_cache_lock;
>> struct inode vfs_inode;
>> };
>>
>> diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
>> index de038da..db4a22e 100644
>> --- a/fs/udf/udfdecl.h
>> +++ b/fs/udf/udfdecl.h
>> @@ -113,11 +113,6 @@ struct ustr {
>> uint8_t u_len;
>> };
>>
>> -struct extent_position {
>> - struct buffer_head *bh;
>> - uint32_t offset;
>> - struct kernel_lb_addr block;
>> -};
>>
>> /* super.c */
>>
>> @@ -164,6 +159,11 @@ extern int8_t udf_next_aext(struct inode *, struct
>> extent_position *,
>> struct kernel_lb_addr *, uint32_t *, int);
>> extern int8_t udf_current_aext(struct inode *, struct extent_position *,
>> struct kernel_lb_addr *, uint32_t *, int);
>> +int udf_read_extent_cache(struct inode *inode, loff_t bcount, loff_t
>> *lbcount,
>> + struct extent_position *pos);
>> +void udf_update_extent_cache(struct inode *inode, loff_t estart,
>> + struct extent_position *pos, int next_epos);
>> +void udf_clear_extent_cache(struct udf_inode_info *iinfo);
>>
>> /* misc.c */
>> extern struct buffer_head *udf_tgetblk(struct super_block *, int);
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel"
>> in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] udf: add extent cache support in case of file reading
2013-01-19 2:17 Namjae Jeon
@ 2013-01-19 2:29 ` Cong Ding
2013-01-21 2:18 ` Namjae Jeon
2013-01-21 11:04 ` Jan Kara
1 sibling, 1 reply; 13+ messages in thread
From: Cong Ding @ 2013-01-19 2:29 UTC (permalink / raw)
To: Namjae Jeon
Cc: jack, linux-fsdevel, linux-kernel, Namjae Jeon, Ashish Sangwan,
Bonggil Bak
On Sat, Jan 19, 2013 at 11:17:14AM +0900, Namjae Jeon wrote:
> From: Namjae Jeon <namjae.jeon@samsung.com>
>
> This patch implements extent caching in case of file reading.
> While reading a file, currently, UDF reads metadata serially
> which takes a lot of time depending on the number of extents present
> in the file. Caching last accessd extent improves metadata read time.
> Instead of reading file metadata from start, now we read from
> the cached extent.
>
> This patch considerably improves the time spent by CPU in kernel mode.
> For example, while reading a 10.9 GB file using dd:
> Time before applying patch:
> 11677022208 bytes (10.9GB) copied, 1529.748921 seconds, 7.3MB/s
> real 25m 29.85s
> user 0m 12.41s
> sys 15m 34.75s
>
> Time after applying patch:
> 11677022208 bytes (10.9GB) copied, 1469.338231 seconds, 7.6MB/s
> real 24m 29.44s
> user 0m 15.73s
> sys 3m 27.61s
did you have any test on lots of small files?
- cong
>
> Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
> Signed-off-by: Ashish Sangwan <a.sangwan@samsung.com>
> Signed-off-by: Bonggil Bak <bgbak@samsung.com>
> ---
> fs/udf/ialloc.c | 4 +++
> fs/udf/inode.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++-----
> fs/udf/udf_i.h | 16 +++++++++++
> fs/udf/udfdecl.h | 10 +++----
> 4 files changed, 98 insertions(+), 11 deletions(-)
>
> diff --git a/fs/udf/ialloc.c b/fs/udf/ialloc.c
> index 7e5aae4..0cb208e 100644
> --- a/fs/udf/ialloc.c
> +++ b/fs/udf/ialloc.c
> @@ -117,6 +117,10 @@ struct inode *udf_new_inode(struct inode *dir, umode_t mode, int *err)
> iinfo->i_lenAlloc = 0;
> iinfo->i_use = 0;
> iinfo->i_checkpoint = 1;
> + memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
> + spin_lock_init(&(iinfo->i_extent_cache_lock));
> + /* Mark extent cache as invalid for now */
> + iinfo->cached_extent.lstart = -1;
> if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_AD_IN_ICB))
> iinfo->i_alloc_type = ICBTAG_FLAG_AD_IN_ICB;
> else if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_SHORT_AD))
> diff --git a/fs/udf/inode.c b/fs/udf/inode.c
> index e78ef48..86e0469 100644
> --- a/fs/udf/inode.c
> +++ b/fs/udf/inode.c
> @@ -91,6 +91,7 @@ void udf_evict_inode(struct inode *inode)
> }
> kfree(iinfo->i_ext.i_data);
> iinfo->i_ext.i_data = NULL;
> + udf_clear_extent_cache(iinfo);
> if (want_delete) {
> udf_free_inode(inode);
> }
> @@ -106,6 +107,7 @@ static void udf_write_failed(struct address_space *mapping, loff_t to)
> truncate_pagecache(inode, to, isize);
> if (iinfo->i_alloc_type != ICBTAG_FLAG_AD_IN_ICB) {
> down_write(&iinfo->i_data_sem);
> + udf_clear_extent_cache(iinfo);
> udf_truncate_extents(inode);
> up_write(&iinfo->i_data_sem);
> }
> @@ -373,7 +375,7 @@ static int udf_get_block(struct inode *inode, sector_t block,
> iinfo->i_next_alloc_goal++;
> }
>
> -
> + udf_clear_extent_cache(iinfo);
> phys = inode_getblk(inode, block, &err, &new);
> if (!phys)
> goto abort;
> @@ -1172,6 +1174,7 @@ set_size:
> } else {
> if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB) {
> down_write(&iinfo->i_data_sem);
> + udf_clear_extent_cache(iinfo);
> memset(iinfo->i_ext.i_data + iinfo->i_lenEAttr + newsize,
> 0x00, bsize - newsize -
> udf_file_entry_alloc_offset(inode));
> @@ -1185,6 +1188,7 @@ set_size:
> if (err)
> return err;
> down_write(&iinfo->i_data_sem);
> + udf_clear_extent_cache(iinfo);
> truncate_setsize(inode, newsize);
> udf_truncate_extents(inode);
> up_write(&iinfo->i_data_sem);
> @@ -1302,6 +1306,9 @@ static void udf_fill_inode(struct inode *inode, struct buffer_head *bh)
> iinfo->i_lenAlloc = 0;
> iinfo->i_next_alloc_block = 0;
> iinfo->i_next_alloc_goal = 0;
> + memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
> + spin_lock_init(&(iinfo->i_extent_cache_lock));
> + iinfo->cached_extent.lstart = -1;
> if (fe->descTag.tagIdent == cpu_to_le16(TAG_IDENT_EFE)) {
> iinfo->i_efe = 1;
> iinfo->i_use = 0;
> @@ -2157,11 +2164,12 @@ int8_t inode_bmap(struct inode *inode, sector_t block,
> struct udf_inode_info *iinfo;
>
> iinfo = UDF_I(inode);
> - pos->offset = 0;
> - pos->block = iinfo->i_location;
> - pos->bh = NULL;
> + if (!udf_read_extent_cache(inode, bcount, &lbcount, pos)) {
> + pos->offset = 0;
> + pos->block = iinfo->i_location;
> + pos->bh = NULL;
> + }
> *elen = 0;
> -
> do {
> etype = udf_next_aext(inode, pos, eloc, elen, 1);
> if (etype == -1) {
> @@ -2171,7 +2179,8 @@ int8_t inode_bmap(struct inode *inode, sector_t block,
> }
> lbcount += *elen;
> } while (lbcount <= bcount);
> -
> + /* update extent cache */
> + udf_update_extent_cache(inode, lbcount - *elen, pos, 1);
> *offset = (bcount + *elen - lbcount) >> blocksize_bits;
>
> return etype;
> @@ -2201,3 +2210,61 @@ long udf_block_map(struct inode *inode, sector_t block)
> else
> return ret;
> }
> +
> +int udf_read_extent_cache(struct inode *inode, loff_t bcount,
> + loff_t *lbcount, struct extent_position *pos)
> +{
> + struct udf_inode_info *iinfo = UDF_I(inode);
> +
> + spin_lock(&iinfo->i_extent_cache_lock);
> + if ((iinfo->cached_extent.lstart <= bcount) &&
> + (iinfo->cached_extent.lstart != -1)) {
> + /* Cache hit */
> + *lbcount = iinfo->cached_extent.lstart;
> + memcpy(pos, &iinfo->cached_extent.epos,
> + sizeof(struct extent_position));
> + spin_unlock(&iinfo->i_extent_cache_lock);
> + return 1;
> + } else
> + udf_clear_extent_cache(iinfo);
> + spin_unlock(&iinfo->i_extent_cache_lock);
> + return 0;
> +}
> +
> +void udf_update_extent_cache(struct inode *inode, loff_t estart,
> + struct extent_position *pos, int next_epos)
> +{
> + struct udf_inode_info *iinfo = UDF_I(inode);
> +
> + spin_lock(&iinfo->i_extent_cache_lock);
> + if (pos->bh != NULL)
> + /* Increase ref count */
> + get_bh(pos->bh);
> + memcpy(&iinfo->cached_extent.epos, pos,
> + sizeof(struct extent_position));
> + iinfo->cached_extent.lstart = estart;
> + if (next_epos)
> + switch (iinfo->i_alloc_type) {
> + case ICBTAG_FLAG_AD_SHORT:
> + iinfo->cached_extent.epos.offset -=
> + sizeof(struct short_ad);
> + break;
> + case ICBTAG_FLAG_AD_LONG:
> + iinfo->cached_extent.epos.offset -=
> + sizeof(struct long_ad);
> + }
> + spin_unlock(&iinfo->i_extent_cache_lock);
> +}
> +
> +/* This function should be called after i_data_sem is
> + * held for writing OR i_extent_cache_lock is taken.
> + */
> +void udf_clear_extent_cache(struct udf_inode_info *iinfo)
> +{
> + if (iinfo->cached_extent.lstart != -1) {
> + brelse(iinfo->cached_extent.epos.bh);
> + memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
> + iinfo->cached_extent.lstart = -1;
> + }
> +}
> +
> diff --git a/fs/udf/udf_i.h b/fs/udf/udf_i.h
> index bb8309d..b5cd8ed 100644
> --- a/fs/udf/udf_i.h
> +++ b/fs/udf/udf_i.h
> @@ -1,6 +1,19 @@
> #ifndef _UDF_I_H
> #define _UDF_I_H
>
> +struct extent_position {
> + struct buffer_head *bh;
> + uint32_t offset;
> + struct kernel_lb_addr block;
> +};
> +
> +struct udf_ext_cache {
> + /* Extent position */
> + struct extent_position epos;
> + /* Start logical offset in bytes */
> + loff_t lstart;
> +};
> +
> /*
> * The i_data_sem and i_mutex serve for protection of allocation information
> * of a regular files and symlinks. This includes all extents belonging to
> @@ -35,6 +48,9 @@ struct udf_inode_info {
> __u8 *i_data;
> } i_ext;
> struct rw_semaphore i_data_sem;
> + struct udf_ext_cache cached_extent;
> + /* Spinlock for protecting extent cache */
> + spinlock_t i_extent_cache_lock;
> struct inode vfs_inode;
> };
>
> diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
> index de038da..db4a22e 100644
> --- a/fs/udf/udfdecl.h
> +++ b/fs/udf/udfdecl.h
> @@ -113,11 +113,6 @@ struct ustr {
> uint8_t u_len;
> };
>
> -struct extent_position {
> - struct buffer_head *bh;
> - uint32_t offset;
> - struct kernel_lb_addr block;
> -};
>
> /* super.c */
>
> @@ -164,6 +159,11 @@ extern int8_t udf_next_aext(struct inode *, struct extent_position *,
> struct kernel_lb_addr *, uint32_t *, int);
> extern int8_t udf_current_aext(struct inode *, struct extent_position *,
> struct kernel_lb_addr *, uint32_t *, int);
> +int udf_read_extent_cache(struct inode *inode, loff_t bcount, loff_t *lbcount,
> + struct extent_position *pos);
> +void udf_update_extent_cache(struct inode *inode, loff_t estart,
> + struct extent_position *pos, int next_epos);
> +void udf_clear_extent_cache(struct udf_inode_info *iinfo);
>
> /* misc.c */
> extern struct buffer_head *udf_tgetblk(struct super_block *, int);
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] udf: add extent cache support in case of file reading
@ 2013-01-19 2:17 Namjae Jeon
2013-01-19 2:29 ` Cong Ding
2013-01-21 11:04 ` Jan Kara
0 siblings, 2 replies; 13+ messages in thread
From: Namjae Jeon @ 2013-01-19 2:17 UTC (permalink / raw)
To: jack
Cc: linux-fsdevel, linux-kernel, Namjae Jeon, Namjae Jeon,
Ashish Sangwan, Bonggil Bak
From: Namjae Jeon <namjae.jeon@samsung.com>
This patch implements extent caching in case of file reading.
While reading a file, currently, UDF reads metadata serially
which takes a lot of time depending on the number of extents present
in the file. Caching last accessd extent improves metadata read time.
Instead of reading file metadata from start, now we read from
the cached extent.
This patch considerably improves the time spent by CPU in kernel mode.
For example, while reading a 10.9 GB file using dd:
Time before applying patch:
11677022208 bytes (10.9GB) copied, 1529.748921 seconds, 7.3MB/s
real 25m 29.85s
user 0m 12.41s
sys 15m 34.75s
Time after applying patch:
11677022208 bytes (10.9GB) copied, 1469.338231 seconds, 7.6MB/s
real 24m 29.44s
user 0m 15.73s
sys 3m 27.61s
Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Ashish Sangwan <a.sangwan@samsung.com>
Signed-off-by: Bonggil Bak <bgbak@samsung.com>
---
fs/udf/ialloc.c | 4 +++
fs/udf/inode.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++-----
fs/udf/udf_i.h | 16 +++++++++++
fs/udf/udfdecl.h | 10 +++----
4 files changed, 98 insertions(+), 11 deletions(-)
diff --git a/fs/udf/ialloc.c b/fs/udf/ialloc.c
index 7e5aae4..0cb208e 100644
--- a/fs/udf/ialloc.c
+++ b/fs/udf/ialloc.c
@@ -117,6 +117,10 @@ struct inode *udf_new_inode(struct inode *dir, umode_t mode, int *err)
iinfo->i_lenAlloc = 0;
iinfo->i_use = 0;
iinfo->i_checkpoint = 1;
+ memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
+ spin_lock_init(&(iinfo->i_extent_cache_lock));
+ /* Mark extent cache as invalid for now */
+ iinfo->cached_extent.lstart = -1;
if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_AD_IN_ICB))
iinfo->i_alloc_type = ICBTAG_FLAG_AD_IN_ICB;
else if (UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_USE_SHORT_AD))
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index e78ef48..86e0469 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -91,6 +91,7 @@ void udf_evict_inode(struct inode *inode)
}
kfree(iinfo->i_ext.i_data);
iinfo->i_ext.i_data = NULL;
+ udf_clear_extent_cache(iinfo);
if (want_delete) {
udf_free_inode(inode);
}
@@ -106,6 +107,7 @@ static void udf_write_failed(struct address_space *mapping, loff_t to)
truncate_pagecache(inode, to, isize);
if (iinfo->i_alloc_type != ICBTAG_FLAG_AD_IN_ICB) {
down_write(&iinfo->i_data_sem);
+ udf_clear_extent_cache(iinfo);
udf_truncate_extents(inode);
up_write(&iinfo->i_data_sem);
}
@@ -373,7 +375,7 @@ static int udf_get_block(struct inode *inode, sector_t block,
iinfo->i_next_alloc_goal++;
}
-
+ udf_clear_extent_cache(iinfo);
phys = inode_getblk(inode, block, &err, &new);
if (!phys)
goto abort;
@@ -1172,6 +1174,7 @@ set_size:
} else {
if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB) {
down_write(&iinfo->i_data_sem);
+ udf_clear_extent_cache(iinfo);
memset(iinfo->i_ext.i_data + iinfo->i_lenEAttr + newsize,
0x00, bsize - newsize -
udf_file_entry_alloc_offset(inode));
@@ -1185,6 +1188,7 @@ set_size:
if (err)
return err;
down_write(&iinfo->i_data_sem);
+ udf_clear_extent_cache(iinfo);
truncate_setsize(inode, newsize);
udf_truncate_extents(inode);
up_write(&iinfo->i_data_sem);
@@ -1302,6 +1306,9 @@ static void udf_fill_inode(struct inode *inode, struct buffer_head *bh)
iinfo->i_lenAlloc = 0;
iinfo->i_next_alloc_block = 0;
iinfo->i_next_alloc_goal = 0;
+ memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
+ spin_lock_init(&(iinfo->i_extent_cache_lock));
+ iinfo->cached_extent.lstart = -1;
if (fe->descTag.tagIdent == cpu_to_le16(TAG_IDENT_EFE)) {
iinfo->i_efe = 1;
iinfo->i_use = 0;
@@ -2157,11 +2164,12 @@ int8_t inode_bmap(struct inode *inode, sector_t block,
struct udf_inode_info *iinfo;
iinfo = UDF_I(inode);
- pos->offset = 0;
- pos->block = iinfo->i_location;
- pos->bh = NULL;
+ if (!udf_read_extent_cache(inode, bcount, &lbcount, pos)) {
+ pos->offset = 0;
+ pos->block = iinfo->i_location;
+ pos->bh = NULL;
+ }
*elen = 0;
-
do {
etype = udf_next_aext(inode, pos, eloc, elen, 1);
if (etype == -1) {
@@ -2171,7 +2179,8 @@ int8_t inode_bmap(struct inode *inode, sector_t block,
}
lbcount += *elen;
} while (lbcount <= bcount);
-
+ /* update extent cache */
+ udf_update_extent_cache(inode, lbcount - *elen, pos, 1);
*offset = (bcount + *elen - lbcount) >> blocksize_bits;
return etype;
@@ -2201,3 +2210,61 @@ long udf_block_map(struct inode *inode, sector_t block)
else
return ret;
}
+
+int udf_read_extent_cache(struct inode *inode, loff_t bcount,
+ loff_t *lbcount, struct extent_position *pos)
+{
+ struct udf_inode_info *iinfo = UDF_I(inode);
+
+ spin_lock(&iinfo->i_extent_cache_lock);
+ if ((iinfo->cached_extent.lstart <= bcount) &&
+ (iinfo->cached_extent.lstart != -1)) {
+ /* Cache hit */
+ *lbcount = iinfo->cached_extent.lstart;
+ memcpy(pos, &iinfo->cached_extent.epos,
+ sizeof(struct extent_position));
+ spin_unlock(&iinfo->i_extent_cache_lock);
+ return 1;
+ } else
+ udf_clear_extent_cache(iinfo);
+ spin_unlock(&iinfo->i_extent_cache_lock);
+ return 0;
+}
+
+void udf_update_extent_cache(struct inode *inode, loff_t estart,
+ struct extent_position *pos, int next_epos)
+{
+ struct udf_inode_info *iinfo = UDF_I(inode);
+
+ spin_lock(&iinfo->i_extent_cache_lock);
+ if (pos->bh != NULL)
+ /* Increase ref count */
+ get_bh(pos->bh);
+ memcpy(&iinfo->cached_extent.epos, pos,
+ sizeof(struct extent_position));
+ iinfo->cached_extent.lstart = estart;
+ if (next_epos)
+ switch (iinfo->i_alloc_type) {
+ case ICBTAG_FLAG_AD_SHORT:
+ iinfo->cached_extent.epos.offset -=
+ sizeof(struct short_ad);
+ break;
+ case ICBTAG_FLAG_AD_LONG:
+ iinfo->cached_extent.epos.offset -=
+ sizeof(struct long_ad);
+ }
+ spin_unlock(&iinfo->i_extent_cache_lock);
+}
+
+/* This function should be called after i_data_sem is
+ * held for writing OR i_extent_cache_lock is taken.
+ */
+void udf_clear_extent_cache(struct udf_inode_info *iinfo)
+{
+ if (iinfo->cached_extent.lstart != -1) {
+ brelse(iinfo->cached_extent.epos.bh);
+ memset(&iinfo->cached_extent, 0, sizeof(struct udf_ext_cache));
+ iinfo->cached_extent.lstart = -1;
+ }
+}
+
diff --git a/fs/udf/udf_i.h b/fs/udf/udf_i.h
index bb8309d..b5cd8ed 100644
--- a/fs/udf/udf_i.h
+++ b/fs/udf/udf_i.h
@@ -1,6 +1,19 @@
#ifndef _UDF_I_H
#define _UDF_I_H
+struct extent_position {
+ struct buffer_head *bh;
+ uint32_t offset;
+ struct kernel_lb_addr block;
+};
+
+struct udf_ext_cache {
+ /* Extent position */
+ struct extent_position epos;
+ /* Start logical offset in bytes */
+ loff_t lstart;
+};
+
/*
* The i_data_sem and i_mutex serve for protection of allocation information
* of a regular files and symlinks. This includes all extents belonging to
@@ -35,6 +48,9 @@ struct udf_inode_info {
__u8 *i_data;
} i_ext;
struct rw_semaphore i_data_sem;
+ struct udf_ext_cache cached_extent;
+ /* Spinlock for protecting extent cache */
+ spinlock_t i_extent_cache_lock;
struct inode vfs_inode;
};
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index de038da..db4a22e 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -113,11 +113,6 @@ struct ustr {
uint8_t u_len;
};
-struct extent_position {
- struct buffer_head *bh;
- uint32_t offset;
- struct kernel_lb_addr block;
-};
/* super.c */
@@ -164,6 +159,11 @@ extern int8_t udf_next_aext(struct inode *, struct extent_position *,
struct kernel_lb_addr *, uint32_t *, int);
extern int8_t udf_current_aext(struct inode *, struct extent_position *,
struct kernel_lb_addr *, uint32_t *, int);
+int udf_read_extent_cache(struct inode *inode, loff_t bcount, loff_t *lbcount,
+ struct extent_position *pos);
+void udf_update_extent_cache(struct inode *inode, loff_t estart,
+ struct extent_position *pos, int next_epos);
+void udf_clear_extent_cache(struct udf_inode_info *iinfo);
/* misc.c */
extern struct buffer_head *udf_tgetblk(struct super_block *, int);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2013-02-04 10:28 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-12 6:13 [PATCH] udf: add extent cache support in case of file reading Namjae Jeon
2013-01-14 14:56 ` Jan Kara
2013-01-15 2:05 ` Namjae Jeon
2013-01-19 2:17 Namjae Jeon
2013-01-19 2:29 ` Cong Ding
2013-01-21 2:18 ` Namjae Jeon
2013-01-21 11:04 ` Jan Kara
2013-01-22 0:45 ` Namjae Jeon
2013-01-22 10:04 ` Jan Kara
2013-01-22 11:49 ` Namjae Jeon
2013-02-02 6:21 ` Namjae Jeon
2013-02-04 10:21 ` Jan Kara
2013-02-04 10:28 ` Namjae Jeon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).