[f2fs-dev] [bug report] f2fs: introduce periodic iostat io latency traces

[f2fs-dev] [bug report] f2fs: introduce periodic iostat io latency traces

[Dan Carpenter]

Hello Daeho Jeong,

The patch f4b05791dda9: "f2fs: introduce periodic iostat io latency
traces" from Aug 3, 2021, leads to the following
Smatch static checker warnings:

fs/f2fs/f2fs.h:3365 __update_iostat_latency() error: buffer overflow 'sbi->rd_sum_lat' 3 <= 3
fs/f2fs/f2fs.h:3366 __update_iostat_latency() error: buffer overflow 'sbi->rd_bio_cnt' 3 <= 3
fs/f2fs/f2fs.h:3367 __update_iostat_latency() error: buffer overflow 'sbi->rd_peak_lat' 3 <= 3
fs/f2fs/f2fs.h:3368 __update_iostat_latency() error: buffer overflow 'sbi->rd_peak_lat' 3 <= 3
fs/f2fs/f2fs.h:3370 __update_iostat_latency() error: buffer overflow 'sbi->wr_sum_lat[sync]' 3 <= 3
fs/f2fs/f2fs.h:3371 __update_iostat_latency() error: buffer overflow 'sbi->wr_bio_cnt[sync]' 3 <= 3
fs/f2fs/f2fs.h:3372 __update_iostat_latency() error: buffer overflow 'sbi->wr_peak_lat[sync]' 3 <= 3
fs/f2fs/f2fs.h:3373 __update_iostat_latency() error: buffer overflow 'sbi->wr_peak_lat[sync]' 3 <= 3

fs/f2fs/f2fs.h
    3348 static inline void __update_iostat_latency(struct bio_iostat_ctx *iostat_ctx,
    3349 				int rw, int sync)
    3350 {
    3351 	unsigned long ts_diff;
    3352 	unsigned int iotype = iostat_ctx->type;
    3353 	unsigned long flags;
    3354 	struct f2fs_sb_info *sbi = iostat_ctx->sbi;
    3355 
    3356 	if (!sbi->iostat_enable)
    3357 		return;
    3358 
    3359 	ts_diff = jiffies - iostat_ctx->submit_ts;
    3360 	if (iotype >= META_FLUSH)
                    ^^^^^^^^^^^^^^^^^^^^
This means the highest value of "iotype" is NR_PAGE_TYPE.

    3361 		iotype = META;
    3362 
    3363 	spin_lock_irqsave(&sbi->iostat_lat_lock, flags);
    3364 	if (rw == 0) {
--> 3365 		sbi->rd_sum_lat[iotype] += ts_diff;
                        ^^^^^^^^^^^^^^^^^^^^^^^
    3366 		sbi->rd_bio_cnt[iotype]++;
                        ^^^^^^^^^^^^^^^^^^^^^^
These arrays have NR_PAGE_TYPE elements so it's off by one.

    3367 		if (ts_diff > sbi->rd_peak_lat[iotype])
    3368 			sbi->rd_peak_lat[iotype] = ts_diff;
    3369 	} else {
    3370 		sbi->wr_sum_lat[sync][iotype] += ts_diff;
    3371 		sbi->wr_bio_cnt[sync][iotype]++;
    3372 		if (ts_diff > sbi->wr_peak_lat[sync][iotype])
    3373 			sbi->wr_peak_lat[sync][iotype] = ts_diff;
    3374 	}
    3375 	spin_unlock_irqrestore(&sbi->iostat_lat_lock, flags);
    3376 }

regards,
dan carpenter


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [bug report] f2fs: introduce periodic iostat io latency traces

[Daeho Jeong]

Hi Dan,

We don't use NR_PAGE_TYPE for the index.

Thank you,

On Mon, Aug 16, 2021 at 12:37 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> Hello Daeho Jeong,
>
> The patch f4b05791dda9: "f2fs: introduce periodic iostat io latency
> traces" from Aug 3, 2021, leads to the following
> Smatch static checker warnings:
>
> fs/f2fs/f2fs.h:3365 __update_iostat_latency() error: buffer overflow 'sbi->rd_sum_lat' 3 <= 3
> fs/f2fs/f2fs.h:3366 __update_iostat_latency() error: buffer overflow 'sbi->rd_bio_cnt' 3 <= 3
> fs/f2fs/f2fs.h:3367 __update_iostat_latency() error: buffer overflow 'sbi->rd_peak_lat' 3 <= 3
> fs/f2fs/f2fs.h:3368 __update_iostat_latency() error: buffer overflow 'sbi->rd_peak_lat' 3 <= 3
> fs/f2fs/f2fs.h:3370 __update_iostat_latency() error: buffer overflow 'sbi->wr_sum_lat[sync]' 3 <= 3
> fs/f2fs/f2fs.h:3371 __update_iostat_latency() error: buffer overflow 'sbi->wr_bio_cnt[sync]' 3 <= 3
> fs/f2fs/f2fs.h:3372 __update_iostat_latency() error: buffer overflow 'sbi->wr_peak_lat[sync]' 3 <= 3
> fs/f2fs/f2fs.h:3373 __update_iostat_latency() error: buffer overflow 'sbi->wr_peak_lat[sync]' 3 <= 3
>
> fs/f2fs/f2fs.h
>     3348 static inline void __update_iostat_latency(struct bio_iostat_ctx *iostat_ctx,
>     3349                                int rw, int sync)
>     3350 {
>     3351        unsigned long ts_diff;
>     3352        unsigned int iotype = iostat_ctx->type;
>     3353        unsigned long flags;
>     3354        struct f2fs_sb_info *sbi = iostat_ctx->sbi;
>     3355
>     3356        if (!sbi->iostat_enable)
>     3357                return;
>     3358
>     3359        ts_diff = jiffies - iostat_ctx->submit_ts;
>     3360        if (iotype >= META_FLUSH)
>                     ^^^^^^^^^^^^^^^^^^^^
> This means the highest value of "iotype" is NR_PAGE_TYPE.
>
>     3361                iotype = META;
>     3362
>     3363        spin_lock_irqsave(&sbi->iostat_lat_lock, flags);
>     3364        if (rw == 0) {
> --> 3365                sbi->rd_sum_lat[iotype] += ts_diff;
>                         ^^^^^^^^^^^^^^^^^^^^^^^
>     3366                sbi->rd_bio_cnt[iotype]++;
>                         ^^^^^^^^^^^^^^^^^^^^^^
> These arrays have NR_PAGE_TYPE elements so it's off by one.
>
>     3367                if (ts_diff > sbi->rd_peak_lat[iotype])
>     3368                        sbi->rd_peak_lat[iotype] = ts_diff;
>     3369        } else {
>     3370                sbi->wr_sum_lat[sync][iotype] += ts_diff;
>     3371                sbi->wr_bio_cnt[sync][iotype]++;
>     3372                if (ts_diff > sbi->wr_peak_lat[sync][iotype])
>     3373                        sbi->wr_peak_lat[sync][iotype] = ts_diff;
>     3374        }
>     3375        spin_unlock_irqrestore(&sbi->iostat_lat_lock, flags);
>     3376 }
>
> regards,
> dan carpenter
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel