On Mon, 17 Feb 2014 10:06:56 +0100, Andreas Rohner wrote:
> Hi Ryusuke,
>
> On 2014-02-17 04:00, Ryusuke Konishi wrote:
>>> + segnum >= nilfs->ns_nsegments ||
>>
>> This is bad too, because userland programs usually don't know the
>> segment structure of NILFS. When we specify the partition size to
>> range->len, FITRIM can fail due to this check.
>>
>> The upper limit of (range->start + range->len) should be
>> the block device size.
>
> ext4 also checks the range structure like that. Besides couldn't it be
> possible, that the block device is bigger than the file system?
As I mentioned in my second mail, I misunderstood the meaning of this
check at first.
As you pointed out, the block device size may be bigger than the file
system size after shrinking the file system. So, sbp->s_dev_size
should be used for for this check. But it is a bit complicated since
the current nilfs object doesn't have on-memory copy of it.
It is OK to use nilfs->ns_nsegments as its alternative.
>>> + ret = blkdev_issue_discard(nilfs->ns_bdev,
>>> + start * sects_per_block,
>>> + nblocks * sects_per_block,
>>> + GFP_NOFS, 0);
>>> + if (ret < 0) {
>>> + kunmap(kaddr);
>>> + put_bh(su_bh);
>>> + goto out_sem;
>>> + }
>>> +
>>> + trimmed += nblocks;
>>> + nblocks = 0;
>
> This should be:
>
> + start = seg_start;
> + nblocks = seg_end - seg_start + 1;
Yes, that's right.
Regards,
Ryusuke Konishi
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
