| Message ID | 55B5A7E4.6050600@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 27 Jul 2015 11:39:16 +0800 Joseph Qi <joseph.qi@huawei.com> wrote: > cluster pos is defined as u32, when calculate corresponding sector it > should be converted to u64 first, otherwise it may overflow. What are the runtime effects of this change? Please always include this info when fixing bugs so that others can work out which kernel version(s) should be patched.
On 2015/7/29 4:51, Andrew Morton wrote: > On Mon, 27 Jul 2015 11:39:16 +0800 Joseph Qi <joseph.qi@huawei.com> wrote: > >> cluster pos is defined as u32, when calculate corresponding sector it >> should be converted to u64 first, otherwise it may overflow. > > What are the runtime effects of this change? > The issue is when using large volume, for example, 9T volume with 2T already used, frequently create small files with O_DIRECT and the IO is not cluster aligned, it may clear sectors in the wrong place. > Please always include this info when fixing bugs so that others can > work out which kernel version(s) should be patched. > Sorry, I'll bear it in mind. > >
On Wed, 29 Jul 2015 08:30:59 +0800 Joseph Qi <joseph.qi@huawei.com> wrote: > On 2015/7/29 4:51, Andrew Morton wrote: > > On Mon, 27 Jul 2015 11:39:16 +0800 Joseph Qi <joseph.qi@huawei.com> wrote: > > > >> cluster pos is defined as u32, when calculate corresponding sector it > >> should be converted to u64 first, otherwise it may overflow. > > > > What are the runtime effects of this change? > > > The issue is when using large volume, for example, 9T volume with 2T already > used, frequently create small files with O_DIRECT and the IO is not cluster > aligned, it may clear sectors in the wrong place. You mean it corrupts the filesystem? So this wants to be merged asap and backported as far as possible?
On 2015/7/29 8:40, Andrew Morton wrote: > On Wed, 29 Jul 2015 08:30:59 +0800 Joseph Qi <joseph.qi@huawei.com> wrote: > >> On 2015/7/29 4:51, Andrew Morton wrote: >>> On Mon, 27 Jul 2015 11:39:16 +0800 Joseph Qi <joseph.qi@huawei.com> wrote: >>> >>>> cluster pos is defined as u32, when calculate corresponding sector it >>>> should be converted to u64 first, otherwise it may overflow. >>> >>> What are the runtime effects of this change? >>> >> The issue is when using large volume, for example, 9T volume with 2T already >> used, frequently create small files with O_DIRECT and the IO is not cluster >> aligned, it may clear sectors in the wrong place. > > You mean it corrupts the filesystem? > > So this wants to be merged asap and backported as far as possible? > Yes, it only affects kernel 4.0+. >
diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c index b57f0c7..b36dcad 100644 --- a/fs/ocfs2/aops.c +++ b/fs/ocfs2/aops.c @@ -688,7 +688,7 @@ static int ocfs2_direct_IO_zero_extend(struct ocfs2_super *osb, if (p_cpos && !(ext_flags & OCFS2_EXT_UNWRITTEN)) { u64 s = i_size_read(inode); - sector_t sector = (p_cpos << (osb->s_clustersize_bits - 9)) + + sector_t sector = ((u64)p_cpos << (osb->s_clustersize_bits - 9)) + (do_div(s, osb->s_clustersize) >> 9); ret = blkdev_issue_zeroout(osb->sb->s_bdev, sector, @@ -916,7 +916,7 @@ static ssize_t ocfs2_direct_IO_write(struct kiocb *iocb, BUG_ON(!p_cpos || (ext_flags & OCFS2_EXT_UNWRITTEN)); ret = blkdev_issue_zeroout(osb->sb->s_bdev, - p_cpos << (osb->s_clustersize_bits - 9), + (u64)p_cpos << (osb->s_clustersize_bits - 9), zero_len_head >> 9, GFP_NOFS, false); if (ret < 0) mlog_errno(ret);
cluster pos is defined as u32, when calculate corresponding sector it should be converted to u64 first, otherwise it may overflow. Signed-off-by: Joseph Qi <joseph.qi@huawei.com> --- fs/ocfs2/aops.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)