It is concerning that a FLUSH may fail but the blkdev_issue_flush
callers assume it will always succeed.
Each blkdev_issue_flush caller should come to terms with the reality
that a FLUSH may fail -- the file_operations' .fsync methods in
particular. nilfs2 is the only filesystem that checks
blkdev_issue_flush's return.
Signed-off-by: Mike Snitzer <[email protected]>
---
include/linux/blkdev.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index ba43f40..76d6e48 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -968,7 +968,7 @@ static inline struct request *blk_map_queue_find_tag(struct blk_queue_tag *bqt,
#define BLKDEV_DISCARD_SECURE 0x01 /* secure discard */
-extern int blkdev_issue_flush(struct block_device *, gfp_t, sector_t *);
+extern int __must_check blkdev_issue_flush(struct block_device *, gfp_t, sector_t *);
extern int blkdev_issue_discard(struct block_device *bdev, sector_t sector,
sector_t nr_sects, gfp_t gfp_mask, unsigned long flags);
extern int blkdev_issue_zeroout(struct block_device *bdev, sector_t sector,
--
1.7.1
Mike Snitzer <[email protected]> writes:
> It is concerning that a FLUSH may fail but the blkdev_issue_flush
> callers assume it will always succeed.
>
> Each blkdev_issue_flush caller should come to terms with the reality
> that a FLUSH may fail -- the file_operations' .fsync methods in
> particular. nilfs2 is the only filesystem that checks
> blkdev_issue_flush's return.
Yeah, as it stands, it looks like in many cases fsync won't return an
error if a flush fails. That's bad.
On Tue, Jun 26, 2012 at 11:27:25AM -0400, Mike Snitzer wrote:
> It is concerning that a FLUSH may fail but the blkdev_issue_flush
> callers assume it will always succeed.
>
> Each blkdev_issue_flush caller should come to terms with the reality
> that a FLUSH may fail -- the file_operations' .fsync methods in
> particular. nilfs2 is the only filesystem that checks
> blkdev_issue_flush's return.
Good spot, but it would be way better if you actually provided patches
to fix this instead of just adding more compiler warnings.
On Tue, Jun 26 2012 at 11:51am -0400,
Christoph Hellwig <[email protected]> wrote:
> On Tue, Jun 26, 2012 at 11:27:25AM -0400, Mike Snitzer wrote:
> > It is concerning that a FLUSH may fail but the blkdev_issue_flush
> > callers assume it will always succeed.
> >
> > Each blkdev_issue_flush caller should come to terms with the reality
> > that a FLUSH may fail -- the file_operations' .fsync methods in
> > particular. nilfs2 is the only filesystem that checks
> > blkdev_issue_flush's return.
>
> Good spot, but it would be way better if you actually provided patches
> to fix this instead of just adding more compiler warnings.
Alasdair pointed this issue out in response to me asserting that
blkdev_issue_flush does return non-void. But anyway, others knowing
about this issue is half the battle. ;)
Most .fsync methods are straight-forward to convert but I'd prefer each
filesystem maintainer actively audit all blkdev_issue_flush calls.