Re: [PATCH 2/5] [FAT] Let the block device know when sectors can be discarded

[OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>]

David Woodhouse <dwmw2@infradead.org> writes:

> On Thu, 2008-08-07 at 01:40 +0900, OGAWA Hirofumi wrote:
>> Perhaps, after checking cluster-chain corruption is better. Thanks.
>
> That can be done, but not like that, I think. At the point you added the
> blkdev_issue_discard() call, the value of 'cluster' has already changed.
>
> So if the chain being freed is clusters 10, 11 and 12, your version of
> the patch will attempt to discard clusters 11, 12 and 0xFFFF (EOF).

+				     fat_clus_to_blknr(sbi, fatent.entry),

No, no. I used fatent.entry, not cluster.

> On Thu, 2008-08-07 at 02:14 +0900, OGAWA Hirofumi wrote:
>> Ah, blkdev_issue_discard() assumes blocksize is 512bytes, um...
>
> Doesn't everything? I did consider using q->hardsect_size, but decided
> that was probably the wrong thing to do. I don't know. Jens?

My point is fs-blocksize vs hard sector size.

In FAT patch, it's passing the number of fs-blocks, not number of hard
sectors.

>> blkdev_issue_discard() takes bytes,
>
> -EPARSE. It takes sectors at the moment -- do you mean you _want_ it to
> take bytes? 

Yes and no. I guess bytes or _fs_-blocksize is good interface for fs.

>> and instead add some helpers for sb or inode?
>
> I suppose we could; I'm not really so sure we need them.
> inode->i_sb->s_bdev isn't exactly hard...
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>