Re: [Q] ext

[Toshiyuki Okajima <toshi.okajima@jp.fujitsu.com>]

Hi.

(2010/08/10 15:35), Andreas Dilger wrote:
>
> On 2010-08-10, at 10:23, Toshiyuki Okajima wrote:
<snip>
>> +loff_t ext4_llseek(struct file *file, loff_t offset, int origin)
>> +{
>> +	struct inode *inode = file->f_mapping->host;
>> +	loff_t maxbytes;
>> +
>> +	mutex_lock(&inode->i_mutex);
>> +	if (!(ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)))
>> +		maxbytes = EXT4_SB(inode->i_sb)->s_bitmap_maxbytes;
>> +	else
>> +		maxbytes = inode->i_sb->s_maxbytes;
>
> This part doesn't really have to be under i_mutex.  Otherwise, the patch looks reasonable.
OK.

>
>> +	switch (origin) {
>> +	case SEEK_END:
>> +		offset += inode->i_size;
>> +		break;
>> +	case SEEK_CUR:
>> +		if (offset == 0) {
>> +			mutex_unlock(&inode->i_mutex);
>> +			return file->f_pos;
>> +		}
>> +		offset += file->f_pos;
>> +		break;
>> +       	}
>> +
>> +	if (offset<  0 || offset>  maxbytes) {
>> +		mutex_unlock(&inode->i_mutex);
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (offset != file->f_pos) {
>> +		file->f_pos = offset;
>> +		file->f_version = 0;
>> +	}
>> +	mutex_unlock(&inode->i_mutex);
>> +
>> +	return offset;
>> +}
>
> It's too bad that we have to duplicate the whole generic_file_llseek() code here, but I don't think there is a better solution.  However, it is worthwhile to add a comment to this function like:
>
> /* copied from generic_file_llseek() to handle both block-mapped and
>   * extent-mapped maxbytes values.  Should otherwise be identical. */
I understand it.

I apply your comments and then rebuild my patch.
Immediately I'll send it.

Thanks,
Toshiyuki Okajima