FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Redhat > Device-mapper Development

 
 
LinkBack Thread Tools
 
Old 06-26-2012, 03:51 PM
Christoph Hellwig
 
Default block: all callers should check blkdev_issue_flush's return

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.

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 06-26-2012, 03:57 PM
Mike Snitzer
 
Default block: all callers should check blkdev_issue_flush's return

On Tue, Jun 26 2012 at 11:51am -0400,
Christoph Hellwig <hch@infradead.org> 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.

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 07-01-2012, 07:28 AM
Joel Becker
 
Default block: all callers should check blkdev_issue_flush's return

On Tue, Jun 26, 2012 at 11:57:50AM -0400, Mike Snitzer wrote:
> On Tue, Jun 26 2012 at 11:51am -0400,
> Christoph Hellwig <hch@infradead.org> 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.

So send it out with maintainers on cc: and get Acks. That way we have a
coherent patch series cleaning up the in-tree filesystems, rather than a
bunch of warnings for every compile until the maintainers notice.

Joel


--

"To announce that there must be no criticism of them president, or
that we are to stand by the president, right or wrong, is not only
unpatriotic and servile, but is morally treasonable to the American
public."
- Theodore Roosevelt

http://www.jlbec.org/
jlbec@evilplan.org

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 
Old 07-02-2012, 02:35 PM
Mike Snitzer
 
Default block: all callers should check blkdev_issue_flush's return

On Sun, Jul 01 2012 at 3:28am -0400,
Joel Becker <jlbec@evilplan.org> wrote:

> On Tue, Jun 26, 2012 at 11:57:50AM -0400, Mike Snitzer wrote:
> > On Tue, Jun 26 2012 at 11:51am -0400,
> > Christoph Hellwig <hch@infradead.org> 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.
>
> So send it out with maintainers on cc: and get Acks. That way we have a
> coherent patch series cleaning up the in-tree filesystems, rather than a
> bunch of warnings for every compile until the maintainers notice.

Hi Joel,

I shouldn't have sent an RFC patch at all; a normal mail would've
sufficed.

My intent wasn't to have that patch go upstream. I explained as much to
Jens when I saw him last week: I just wanted to get the issue on
filesystem developers' radar (hence the RFC).

But just because someone reports something doesn't implicitly mean they
own fixing it -- I'm unfortunately quite busy with other work.

Given you have more filesystem experience and may be more inclined to
pick over the nuance of each blkdev_issue_flush caller (and how
short-circuiting on blkdev_issue_flush failure should be handled):
please feel free to get a coherent patchset going.

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
 

Thread Tools




All times are GMT. The time now is 04:27 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org