On Wed, Sep 18, 2019 at 06:49:38PM +0200, Christoph Hellwig wrote: > On Mon, Sep 16, 2019 at 08:53:11AM -0400, Brian Foster wrote: > > The caller might not care if this call generates errors, but shouldn't > > we care if something fails? IOW, perhaps we should have an exit path > > with a WARN_ON_ONCE() or some such to indicate that an unhandled error > > has occurred..? > > Not sure there is much of a point. Basically all errors are either > due to a forced shutdown or cause a forced shutdown anyway, so we'll > already get warnings. Well, what's the point of this change in the first place? I see various error paths that aren't directly related to shutdown. A writeback submission error for instance looks like it will warn, but not necessarily shut down (and the filemap_flush() call is already within a !XFS_FORCED_SHUTDOWN() check). So not all errors are associated with or cause shutdown. I suppose you could audit the various error paths that lead back into this function and document that further if you really wanted to go that route... Also, you snipped the rest of my feedback... how does the fact that the caller doesn't care about errors have anything to do with the fact that the existing logic within this function does? I'm not convinced the changes here are quite correct, but at the very least the commit log description is lacking/misleading. Brian
