| Message ID | 20170313074214.24123-2-anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Mar 13, 2017 at 03:42:11PM +0800, Anand Jain wrote: > REQ_PREFLUSH bio to flush dev cache uses btrfs_end_empty_barrier() > completion callback only, as of now, and there it accounts for dev > stat flush errors BTRFS_DEV_STAT_FLUSH_ERRS, so remove it from the > btrfs_end_bio(). Can you please be more specific? I was trying to find the code path that does the supposedly duplicate accounting for BTRFS_DEV_STAT_FLUSH_ERRS, but still not there after half an hour. submit_stripe_bio btrfsic_submit_bio btrfs_end_bio -> stats accounting write_dev_flush btrfsic_submit_bio btrfs_end_empty_barrier complete now here it wakes up flush_wait, that is only waited for in write_dev_flush, there the FLUSH_ERRS accounting happens, but ... I'm not sure if I haven't lost in the bio handling maze. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/28/2017 11:19 PM, David Sterba wrote: > On Mon, Mar 13, 2017 at 03:42:11PM +0800, Anand Jain wrote: >> REQ_PREFLUSH bio to flush dev cache uses btrfs_end_empty_barrier() >> completion callback only, as of now, and there it accounts for dev >> stat flush errors BTRFS_DEV_STAT_FLUSH_ERRS, so remove it from the >> btrfs_end_bio(). > > Can you please be more specific? I was trying to find the code path that > does the supposedly duplicate accounting for BTRFS_DEV_STAT_FLUSH_ERRS, > but still not there after half an hour. > > submit_stripe_bio > btrfsic_submit_bio > btrfs_end_bio > -> stats accounting > > write_dev_flush > btrfsic_submit_bio > btrfs_end_empty_barrier > complete > > now here it wakes up flush_wait, that is only waited for in > write_dev_flush, there the FLUSH_ERRS accounting happens, but ... I'm > not sure if I haven't lost in the bio handling maze. As I am checking again, we aren't using REQ_PREFLUSH flag for any other bio and would suffice for its correctness IMO. Thanks, Anand -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/29/2017 06:00 PM, Anand Jain wrote: > > > On 03/28/2017 11:19 PM, David Sterba wrote: >> On Mon, Mar 13, 2017 at 03:42:11PM +0800, Anand Jain wrote: >>> REQ_PREFLUSH bio to flush dev cache uses btrfs_end_empty_barrier() >>> completion callback only, as of now, and there it accounts for dev >>> stat flush errors BTRFS_DEV_STAT_FLUSH_ERRS, so remove it from the >>> btrfs_end_bio(). >> >> Can you please be more specific? I was trying to find the code path that >> does the supposedly duplicate accounting for BTRFS_DEV_STAT_FLUSH_ERRS, >> but still not there after half an hour. >> >> submit_stripe_bio >> btrfsic_submit_bio >> btrfs_end_bio >> -> stats accounting >> >> write_dev_flush >> btrfsic_submit_bio >> btrfs_end_empty_barrier >> complete >> >> now here it wakes up flush_wait, that is only waited for in >> write_dev_flush, there the FLUSH_ERRS accounting happens, but ... I'm >> not sure if I haven't lost in the bio handling maze. > As I am checking again, we aren't using REQ_PREFLUSH flag for > any other bio and would suffice for its correctness IMO. I mean for this patch correctness. > Thanks, Anand > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 73d56eef5e60..1563ae03079b 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -6010,9 +6010,6 @@ static void btrfs_end_bio(struct bio *bio) else btrfs_dev_stat_inc(dev, BTRFS_DEV_STAT_READ_ERRS); - if (bio->bi_opf & REQ_PREFLUSH) - btrfs_dev_stat_inc(dev, - BTRFS_DEV_STAT_FLUSH_ERRS); btrfs_dev_stat_print_on_error(dev); } }
REQ_PREFLUSH bio to flush dev cache uses btrfs_end_empty_barrier() completion callback only, as of now, and there it accounts for dev stat flush errors BTRFS_DEV_STAT_FLUSH_ERRS, so remove it from the btrfs_end_bio(). Signed-off-by: Anand Jain <anand.jain@oracle.com> --- fs/btrfs/volumes.c | 3 --- 1 file changed, 3 deletions(-)