Message ID | 1358173111-10511-1-git-send-email-wenqing.lz@taobao.com |
---|---|
State | Superseded, archived |
Headers | show |
On Mon, Jan 14, 2013 at 10:18:30PM +0800, Zheng Liu wrote: > > ext2fs_file_llseek_data/hole() seem to be weird because ext2_file_t > structure is hidden by a typedef. The caller can not dereference > it. So I define a marco called EXT2_SEEK_OFFSET_INVALID to let the > caller indicate that it find the data/hole from ext2_file_t->pos or > from offset. What do you think? Is the problem you're worried about is that the user can't get current location? That's pretty easy to solve. You can get it the same way it works with the lseek(2) system call. retval = ext2fs_file_llseek(file, 0, SEEK_CUR, &pos); Upon the return, pos will be contain the current file offset. - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 14, 2013 at 10:23:29PM -0500, Theodore Ts'o wrote: > On Mon, Jan 14, 2013 at 10:18:30PM +0800, Zheng Liu wrote: > > > > ext2fs_file_llseek_data/hole() seem to be weird because ext2_file_t > > structure is hidden by a typedef. The caller can not dereference > > it. So I define a marco called EXT2_SEEK_OFFSET_INVALID to let the > > caller indicate that it find the data/hole from ext2_file_t->pos or > > from offset. What do you think? > > Is the problem you're worried about is that the user can't get current > location? > > That's pretty easy to solve. You can get it the same way it works > with the lseek(2) system call. > > retval = ext2fs_file_llseek(file, 0, SEEK_CUR, &pos); > > Upon the return, pos will be contain the current file offset. Ah, I see. I will remove EXT2_SEEK_OFFSET_INVALID flag in next version. Thanks, - Zheng -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 14, 2013 at 10:18:30PM +0800, Zheng Liu wrote: > +static errcode_t ext2fs_file_llseek_data(ext2_file_t file, __u64 offset) > +{ > + int ret_flags, flag = 1; > + ext2_filsys fs = file->fs; > + errcode_t retval; > + > + /* > + * If offset == EXT2_SEEK_OFFSET_INVALID, that means that > + * the caller wants to find the data from file->pos. > + */ > + offset = offset != EXT2_SEEK_OFFSET_INVALID ? offset : file->pos; > + file->blockno = offset / fs->blocksize; > + while (file->blockno * fs->blocksize < EXT2_I_SIZE(&file->inode)) { > + retval = ext2fs_bmap2(fs, file->ino, &file->inode, > + BMAP_BUFFER, 0, file->blockno, > + &ret_flags, &file->physblock); Using bmap will certainly work, although as an optimization we can do much better for extents if we use the extents interface. I won't insist on this, though. (We can always optimize this later). - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 15, 2013 at 01:55:09PM -0500, Theodore Ts'o wrote: > On Mon, Jan 14, 2013 at 10:18:30PM +0800, Zheng Liu wrote: > > +static errcode_t ext2fs_file_llseek_data(ext2_file_t file, __u64 offset) > > +{ > > + int ret_flags, flag = 1; > > + ext2_filsys fs = file->fs; > > + errcode_t retval; > > + > > + /* > > + * If offset == EXT2_SEEK_OFFSET_INVALID, that means that > > + * the caller wants to find the data from file->pos. > > + */ > > + offset = offset != EXT2_SEEK_OFFSET_INVALID ? offset : file->pos; > > + file->blockno = offset / fs->blocksize; > > + while (file->blockno * fs->blocksize < EXT2_I_SIZE(&file->inode)) { > > + retval = ext2fs_bmap2(fs, file->ino, &file->inode, > > + BMAP_BUFFER, 0, file->blockno, > > + &ret_flags, &file->physblock); > > Using bmap will certainly work, although as an optimization we can do > much better for extents if we use the extents interface. I won't > insist on this, though. (We can always optimize this later). Yeah, it allows us to skip to the next data/hole directly if the extents interface is used. But if we do that, we will need to handle extent-based file and indirect-based file resptively like this. if (inode->i_flags & EXT4_EXTENTS_FL) { ext2fs_file_ext_llseek_data(); ... } else { ext2fs_file_ind_llseek_data(); ... } I am not sure whether it is too complicated or not for us. What do you think? Thanks, - Zheng -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 16, 2013 at 08:04:56PM +0800, Zheng Liu wrote: > Yeah, it allows us to skip to the next data/hole directly if the extents > interface is used. But if we do that, we will need to handle > extent-based file and indirect-based file resptively like this. > > if (inode->i_flags & EXT4_EXTENTS_FL) { > ext2fs_file_ext_llseek_data(); > ... > } else { > ext2fs_file_ind_llseek_data(); > ... > } > > I am not sure whether it is too complicated or not for us. What do you > think? I'm not too worried about the performance issues for debugfs. But for clients who are accessing ext[234] using libext2fs and FUSE, they would probably notice in at least some circumstances. I'm not that worried about the complexity, but it's also not a high priority thing, either --- it's a "nice to have", not a "must have". Regards, - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jan 16, 2013 at 09:59:18AM -0500, Theodore Ts'o wrote: > On Wed, Jan 16, 2013 at 08:04:56PM +0800, Zheng Liu wrote: > > Yeah, it allows us to skip to the next data/hole directly if the extents > > interface is used. But if we do that, we will need to handle > > extent-based file and indirect-based file resptively like this. > > > > if (inode->i_flags & EXT4_EXTENTS_FL) { > > ext2fs_file_ext_llseek_data(); > > ... > > } else { > > ext2fs_file_ind_llseek_data(); > > ... > > } > > > > I am not sure whether it is too complicated or not for us. What do you > > think? > > I'm not too worried about the performance issues for debugfs. But for > clients who are accessing ext[234] using libext2fs and FUSE, they > would probably notice in at least some circumstances. I'm not that > worried about the complexity, but it's also not a high priority thing, > either --- it's a "nice to have", not a "must have". Understood. Let me try it. Thanks, - Zheng -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/lib/ext2fs/ext2_err.et.in b/lib/ext2fs/ext2_err.et.in index c99a167..f763938 100644 --- a/lib/ext2fs/ext2_err.et.in +++ b/lib/ext2fs/ext2_err.et.in @@ -473,4 +473,7 @@ ec EXT2_ET_UNKNOWN_CSUM, ec EXT2_ET_MMP_CSUM_INVALID, "MMP block checksum does not match MMP block" +ec EXT2_ET_SEEK_BEYOND_EOF, + "lseek beyond the EOF" + end diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h index 7139b4d..474049f 100644 --- a/lib/ext2fs/ext2fs.h +++ b/lib/ext2fs/ext2fs.h @@ -160,6 +160,10 @@ typedef struct ext2_file *ext2_file_t; #define EXT2_SEEK_SET 0 #define EXT2_SEEK_CUR 1 #define EXT2_SEEK_END 2 +#define EXT2_SEEK_DATA 3 +#define EXT2_SEEK_HOLE 4 + +#define EXT2_SEEK_OFFSET_INVALID -1 /* * Flags for the ext2_filsys structure and for ext2fs_open() diff --git a/lib/ext2fs/fileio.c b/lib/ext2fs/fileio.c index 1f7002c..ab33290 100644 --- a/lib/ext2fs/fileio.c +++ b/lib/ext2fs/fileio.c @@ -312,10 +312,84 @@ fail: return retval; } +static errcode_t ext2fs_file_llseek_data(ext2_file_t file, __u64 offset) +{ + int ret_flags, flag = 1; + ext2_filsys fs = file->fs; + errcode_t retval; + + /* + * If offset == EXT2_SEEK_OFFSET_INVALID, that means that + * the caller wants to find the data from file->pos. + */ + offset = offset != EXT2_SEEK_OFFSET_INVALID ? offset : file->pos; + file->blockno = offset / fs->blocksize; + while (file->blockno * fs->blocksize < EXT2_I_SIZE(&file->inode)) { + retval = ext2fs_bmap2(fs, file->ino, &file->inode, + BMAP_BUFFER, 0, file->blockno, + &ret_flags, &file->physblock); + if (retval) + return retval; + if (file->physblock == 0 || (ret_flags & BMAP_RET_UNINIT)) { + file->blockno++; + flag = 0; + } else { + if (flag) + file->pos = offset; + else + file->pos = file->blockno * fs->blocksize; + break; + } + } + + /* notify the caller that there is no any data */ + if (file->blockno * fs->blocksize >= EXT2_I_SIZE(&file->inode)) + return EXT2_ET_SEEK_BEYOND_EOF; + + return 0; +} + +static errcode_t ext2fs_file_llseek_hole(ext2_file_t file, __u64 offset) +{ + int ret_flags, flag = 1; + ext2_filsys fs = file->fs; + errcode_t retval; + + /* + * If offset == EXT2_SEEK_OFFSET_INVALID, that means that + * the caller wants to find the hole from file->pos. + */ + offset = offset != EXT2_SEEK_OFFSET_INVALID ? offset : file->pos; + if (offset >= EXT2_I_SIZE(&file->inode)) + return EXT2_ET_SEEK_BEYOND_EOF; + + file->blockno = offset / fs->blocksize; + while (file->blockno * fs->blocksize < EXT2_I_SIZE(&file->inode)) { + retval = ext2fs_bmap2(fs, file->ino, &file->inode, + BMAP_BUFFER, 0, file->blockno, + &ret_flags, &file->physblock); + if (retval) + return retval; + if (file->physblock == 0 || (ret_flags & BMAP_RET_UNINIT)) { + if (flag) + file->pos = offset; + else + file->pos = file->blockno * fs->blocksize; + break; + } else { + file->blockno++; + flag = 0; + } + } + + return 0; +} + errcode_t ext2fs_file_llseek(ext2_file_t file, __u64 offset, int whence, __u64 *ret_pos) { EXT2_CHECK_MAGIC(file, EXT2_ET_MAGIC_EXT2_FILE); + errcode_t retval = 0; if (whence == EXT2_SEEK_SET) file->pos = offset; @@ -323,13 +397,17 @@ errcode_t ext2fs_file_llseek(ext2_file_t file, __u64 offset, file->pos += offset; else if (whence == EXT2_SEEK_END) file->pos = EXT2_I_SIZE(&file->inode) + offset; + else if (whence == EXT2_SEEK_DATA) + retval = ext2fs_file_llseek_data(file, offset); + else if (whence == EXT2_SEEK_HOLE) + retval = ext2fs_file_llseek_hole(file, offset); else return EXT2_ET_INVALID_ARGUMENT; if (ret_pos) *ret_pos = file->pos; - return 0; + return retval; } errcode_t ext2fs_file_lseek(ext2_file_t file, ext2_off_t offset,