Hi David,
Just want to bring this patch to your notice. It fixes all the previously commented issues. Thanks, Anand On 04/06/2017 11:22 AM, Anand Jain wrote:
These patches adds cleanup of device barrier, q and flush codes. This was needed for the following reasons.. - First of all, Qu approach on the chunk based degradable check wanted to know what devices failed rather than total count of failed devices. So the patch 3/7 adds a function check_barrier_error() in which Qu can add such a logic. Irrespective whether those Qu patches passes through all the reviews, and if it does not, still this patch is good to have as it does not change the commit performance since check_barrier_error() is only called when there is an error in any of the device. However I believe a volume manager when deciding an action of the device error, it should rather have a broader view and state of other devices instead of action based on just individual device, this just a step towards that. (Earlier the patch [1] was also on the similar idea, its yet to be separated from the feature patch, as time permits and priority rises. [1] https://patchwork.kernel.org/patch/9058361/) [PATCH v4 3/7] btrfs: cleanup barrier_all_devices() to check dev stat flush error - However barrier_all_devices() wasn't capable of supporting 3/7 (above). So clean up of write_dev_flush() is needed, and if it uses blkdev_issue_flush() then the way we count the error device especially the err_send++ part can be avoided. Patch 1/7 provides this cleanup. [PATCH v4 1/7] btrfs: use blkdev_issue_flush to flush the device cache - There are some static error checking in barrier_all_devices(), such as !dev->bdev && errors_send++, these errors would remain same before or after the flush. As of now we do not have any code which will bring a null bdev back, also we are in device_mutex. So consolidated all those errors as dropouts at the place after the flush. This clean up is in 2/7. [PATCH v4 2/7] btrfs: cleanup barrier_all_devices() unify dev error count Now with 1/7 and 2/7 in place, 3/7 can be applied. These 3 are dependent patches. - Patch 4/7 is not related to above 3 patches, however it adds up a cleanup around the REQ_PREFLUSH flag. It is found that we do not have any bio with the flag REQ_PREFLUSH set. So an end io checking for that flag is not required. Concerned what if there is a bio in future which will set REQ_PREFLUSH ? I am not too sure about that. However the write_dev_flush code with the patch 3/7 (above) is still unaffected with or without patch 4/7, thats because 3/7 does not depend on BTRFS_DEV_STAT_FLUSH_ERRS, instead it uses a new per device last_flush_err member to track the error. But it would have been better or the right way if it had checked BTRFS_DEV_STAT_FLUSH_ERRS instead. But as it seeks much bigger change, it can be optimized to use BTRFS_DEV_STAT_FLUSH_ERRS at a later point of time. [PATCH v4 4/7] btrfs: REQ_PREFLUSH does not use btrfs_end_bio() completion callback And rest of the patches are completely a cleanup patches which are found good to have when around these parts of the code. They are - [PATCH v4 5/7] btrfs: use q which is already obtained from bdev_get_queue [PATCH v4 6/7] btrfs: delete unused member nobarriers [PATCH v4 7/7] btrfs: check if the device is flush capable Anand Jain (7): btrfs: use blkdev_issue_flush to flush the device cache btrfs: cleanup barrier_all_devices() unify dev error count btrfs: cleanup barrier_all_devices() to check dev stat flush error btrfs: REQ_PREFLUSH does not use btrfs_end_bio() completion callback btrfs: use q which is already obtained from bdev_get_queue btrfs: delete unused member nobarriers btrfs: check if the device is flush capable fs/btrfs/disk-io.c | 112 +++++++++++++++++++++++++---------------------------- fs/btrfs/volumes.c | 10 ++--- fs/btrfs/volumes.h | 4 +- 3 files changed, 58 insertions(+), 68 deletions(-)
-- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
