On 10/11/23 2:50 PM, Mike Snitzer wrote:
On Wed, Oct 11 2023 at 4:20P -0400, Jens Axboe axboe@kernel.dk wrote:
On 10/11/23 2:12 PM, Sarthak Kukreti wrote:
Only call truncate_bdev_range() if the fallocate mode is supported. This fixes a bug where data in the pagecache could be invalidated if the fallocate() was called on the block device with an invalid mode.
Fix looks fine, but would be nicer if we didn't have to duplicate the truncate_bdev_range() in each switch clause. Can we check this upfront instead?
No, if you look at the function (rather than just the patch in isolation) we need to make the call for each case rather than collapse to a single call at the front (that's the reason for this fix, because otherwise the default: error case will invalidate the page cache too).
Yes that part is clear, but it might look cleaner to check a valid mask first rather than have 3 duplicate calls.
Just so you're aware, I also had this feedback that shaped the patch a bit back in April: https://listman.redhat.com/archives/dm-devel/2023-April/053986.html
Also, please wrap commit messages at 72-74 chars.
Not seeing where the header should be wrapped. You referring to the Fixes: line? I've never seen those wrapped.
I'm referring to the commit message itself.