On Tue, 20 Sep 2011, Christoph Hellwig wrote:
> >
> > I am not sure what do you mean ? There already is a check when both
> > start and len are huge numbers. I am not sure if we can do more without
> > significantly complicating the test to cover various start, or len
> > numbers where can the fsblock->group_number overflow for various file
> > systems.
>
> Add a testcase where start is a relatively small number (smaller than an
> AG/BG), but start + len is outside the fs.
Already done in the second version of the xfstests patch.
>
> > @@ -145,7 +145,7 @@ xfs_ioc_trim(
> > struct request_queue *q = mp->m_ddev_targp->bt_bdev->bd_disk->queue;
> > unsigned int granularity = q->limits.discard_granularity;
> > struct fstrim_range range;
> > + xfs_fsblock_t start, end, minlen;
> > xfs_agnumber_t start_agno, end_agno, agno;
> > __uint64_t blocks_trimmed = 0;
> > int error, last_error = 0;
> > @@ -165,19 +165,21 @@ xfs_ioc_trim(
> > * matter as trimming blocks is an advisory interface.
> > */
> > start = XFS_B_TO_FSBT(mp, range.start);
> > + end = start + XFS_B_TO_FSBT(mp, range.len) - 1;
> > minlen = XFS_B_TO_FSB(mp, max_t(u64, granularity, range.minlen));
> >
> > + if (start >= mp->m_sb.sb_dblocks)
> > return -XFS_ERROR(EINVAL);
> > + start_agno = XFS_FSB_TO_AGNO(mp, start);
> >
> > + if (end >= mp->m_sb.sb_dblocks) {
> > + end = mp->m_sb.sb_dblocks - 1;
> > end_agno = mp->m_sb.sb_agcount - 1;
> > + } else
> > + end_agno = XFS_FSB_TO_AGNO(mp, end);
>
> I'd rather do something like:
>
> if (start >= mp->m_sb.sb_dblocks)
> return -XFS_ERROR(EINVAL);
> if (end > mp->m_sb.sb_dblocks - 1)
> end = mp->m_sb.sb_dblocks - 1;
>
>
> start_agno = XFS_FSB_TO_AGNO(mp, start);
> end_agno = XFS_FSB_TO_AGNO(mp, end)
>
> here.
>
> Otherwise the patch looks fine.
>
Thanks!
-Lukas
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
